fix(eform-files): make picture upload atomic and idempotent; localize its error (#8036) - #8039
Merged
Merged
Conversation
… its error (#8036) POST /api/template-files/image persisted UploadedData and FieldValue before the step that can fail (thumbnail generation), returned an untranslated resource key on failure, and the client let the user re-submit the same file on every further click. One transient failure became a raw-key toast and N duplicate picture cards after N retries. The endpoint also stored Extension without the leading dot, so every web-uploaded picture was unreachable through get-image/{fileName}.{ext} and invisible in the compliance image gallery. Backend (EFormFilesController.AddNewImage) - Thumbnails are derived to temp names before any row is written; the UploadedData/FieldValue creation and the storage puts run in one transaction. The SDK context uses EnableRetryOnFailure, so the unit runs through Database.CreateExecutionStrategy().ExecuteAsync with the entities built inside the delegate and the change tracker cleared on entry, so a transient retry cannot re-insert a failed attempt. - Idempotency guard: a non-removed FieldValue on (CaseId, FieldId) whose non-removed UploadedData has the same checksum returns success without creating anything; delete-then-reupload still creates a new row. - Extension stored with the leading dot (".png"); FileName keeps the {id}_{md5}.{ext} shape. - Temp files (GUID stem) removed in finally on every path; the exception is logged with its stack trace and structured case/field ids. - SharedResource.resx / .da.resx: ErrorWhileUpdateImage and ImageNotFound (neutral English + real Danish); no placeholders in other locales. Frontend (element-picture) - Host buttons and the dialog's Save are locked while the upload is in flight; released on success:false and on error so the user can retry. - "Add new image" label translated; AddPictureDialog renders Cancel before Save. Verified against the local dev stack: dotted extension, idempotent re-POST, thumbnails served under the dotted names, delete-then-reupload creates a new row, corrupt file yields the translated error with no rows and no temp leftovers. Co-Authored-By: Claude Fable 5.1 <[email protected]> Claude-Session: https://claude.ai/code/session_018qJL2WhHwhZ5CGZehZF2ro
There was a problem hiding this comment.
🔵 Needs a closer look
It changes transactional semantics and storage/thumbnail handling in a critical upload endpoint, warranting careful human validation of edge cases and deployment behavior.
Pull request overview
Improves picture upload reliability and user experience across the backend upload endpoint and the Angular picture element by making uploads transactionally safer, adding idempotency guards, and ensuring error messages are localized.
Changes:
- Backend:
AddNewImagenow generates thumbnails before persisting DB rows, wraps DB writes + storage uploads in an execution-strategy transaction, adds a checksum-based idempotency guard, and cleans up temp files reliably. - Backend: Adds missing localization keys for image-upload errors (neutral + Danish).
- Frontend: Prevents duplicate submissions by locking host/dialog actions during upload; translates the “Add new image” label and updates dialog button ordering/disabled state.
File summaries
| File | Description |
|---|---|
| eFormAPI/eFormAPI.Web/Resources/SharedResource.resx | Adds localized strings for upload/update failure and image-not-found cases. |
| eFormAPI/eFormAPI.Web/Resources/SharedResource.da.resx | Adds Danish translations for the new image error keys. |
| eFormAPI/eFormAPI.Web/Controllers/Eforms/EFormFilesController.cs | Makes AddNewImage atomic-ish (execution strategy + transaction), idempotent by checksum, and ensures temp file cleanup and better logging. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.ts | Adds upload locking (buttonsLocked/saving) to prevent duplicate POSTs while in-flight and allow retry after failure. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.html | Translates the “Add new image” button label. |
Review details
- Files reviewed: 5/5 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+310
to
+314
| // Idempotency guard: the same file already attached to this | ||
| // (case, field) is a no-op success. Both "not removed" conditions | ||
| // are load-bearing — DeleteImage soft-deletes only the | ||
| // UploadedData, so a delete-then-reupload must create a new row. | ||
| var alreadyAttached = await sdkDbContext.FieldValues |
Comment on lines
+418
to
+419
| decimal currentRation = image.Height / (decimal) image.Width; | ||
| int newHeight = (int) Math.Round((currentRation * newWidth)); |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #8036. Part of microting/eform-backendconfiguration-plugin#1183 (customer bug list, PDF pages 1–2 "Kan ikke uploade billede").
What
Backend —
EFormFilesController.AddNewImageMagickImagestep can no longer leave rows behind.UploadedDatacreate/update,FieldValuecreate and the three storage puts run in one transaction. The SDK'sMicrotingDbContextFactoryenablesEnableRetryOnFailure, and EF Core refuses user-initiated transactions under a retrying strategy, so the unit runs throughDatabase.CreateExecutionStrategy().ExecuteAsync(...)with the entities constructed inside the delegate andChangeTracker.Clear()on entry (a retried attempt must not re-insert a failed attempt's tracked entity).FieldValueon(CaseId, FieldId)whose non-removedUploadedDatahas the same checksum → plain success, no rows (decision 1). Both non-removed conditions are load-bearing:DeleteImagesoft-deletes onlyUploadedData, so delete-then-reupload creates a new row.Extensionstored with the leading dot (.png), matching the SDK device path and the plugin upload path, so derived thumbnail names{id}_700_{md5}.pngmatchget-image/{fileName}.{ext}and the compliance gallery can fetch them.FileName = {id}_{md5}.{ext}unchanged (flutter-eform parity harness shape preserved).finallyon every path; exception logged with stack trace and structured{CaseId}/{FieldId}.SharedResource.resx+SharedResource.da.resx:ErrorWhileUpdateImage("Error while uploading image" / "Fejl under upload af billede"),ImageNotFound("Image not found" / "Billedet blev ikke fundet"). No entries added to the other 23 locale files (translation tooling);IStringLocalizerfalls back to the neutral resx. Also fixes the siblingUpdateImagetoast.Frontend —
element-pictureaddPicture()locks the host buttons and the dialog's Save (saving) before the request; released onsuccess:falseand onerror, so the sameFilecan never be re-POSTed while in flight and retry after an error works.Add new imagelabel now translated;AddPictureDialogComponentrenders Cancel before Save (btn-cancel/btn-primary).Decisions recorded
UploadedDatas.Extension NOT LIKE '.%') stay invisible in the compliance gallery. If ops wants them back, it is a one-off script, not a migration:UPDATE UploadedDatas SET Extension = CONCAT('.', Extension) WHERE Extension NOT LIKE '.%'and copy the S3 objects{id}_300_{md5}{ext}/{id}_700_{md5}{ext}to the dotted keys (the SDK report path reads by storedExtension, so the DB change alone would break it for old rows).DeleteImagealso soft-deleting the orphanedFieldValue): left as is.DeletePictureDialogComponent(same file) still renders Delete before Cancel; out of this issue's scope, andcheck-button-conventions.jsdoes not scan inline templates.Verified (local dev stack, worktree backend on a spare port against the dev DB + S3)
Extension='.png',FileName={id}_{md5}.png, one FieldValueget-image/{id}_700_{md5}.pngand_300_→ 200 PNG 700×525 / 300×225Error while uploading image/Fejl under upload af billede, no rows, stack trace loggedCase not found, no rows/tmp/cases-temp-filesempty after every callNot verified here: the compliance gallery end-to-end (plugin), and the exact 922 exception.
Notes
work-items-planning-containertag.🤖 Generated with Claude Code
https://claude.ai/code/session_018qJL2WhHwhZ5CGZehZF2ro