fix(eform-cases): resolve case id from an input, not the router URL - #8033
Merged
Conversation
ElementPictureComponent read its case id only from ActivatedRoute.params
('id', falling back to 'sdkCaseId'). Inside a MatDialog the injected route
is the route the dialog was opened from, so on a route declaring neither
param both lookups yield NaN. That NaN was posted as the literal string
CaseId=NaN, bound to 0 server-side (EFormFilesController has no
[ApiController] and there is no ModelState filter), and picture upload
failed with CaseNotFound -- Danish "Sagen blev ikke fundet".
Thread an optional caseId down the shared chain instead:
case-edit-element -> case-edit-switch -> element-picture
The bound input wins; the route lookup stays as a fallback, so the three
routed consumers (cases/edit/:id, compliance/:sdkCaseId, and
backend-configuration-case/:id) are behaviourally unchanged -- none binds
the input, and a non-finite or <= 0 value falls through to the route.
Also close the nesting gaps in the same chain, all of which dropped the
value one level down:
- element-container (the FieldContainer branch) never received or
forwarded caseId, so a Picture inside a question group reproduced the
same NaN bug one level deeper.
- element-container and the recursive case-edit-element forwarded no
(needUpdate), so a nested picture would upload but never refresh the
host gallery. Only reachable once the id was fixed.
Extra pictures deliberately get [caseId] but no [fieldId]: they are
ExtraFieldValue rows with no FieldId column and serialize as fieldId 0,
and element-picture gates its card on *ngIf="fieldId != 0", so binding
that would hide the card and its existing thumbnails. Tracked in #8032.
Refs #8031
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ
There was a problem hiding this comment.
🟡 Changes recommended
ActivatedRoute.params is subscribed from ngOnChanges in a way that can leak subscriptions when fieldValues changes more than once.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR fixes picture uploads when the case editor is rendered inside a MatDialog by allowing the case id to be passed via component inputs (instead of relying solely on route params), while preserving route-param fallback behavior for existing routed pages.
Changes:
- Added an optional
caseIdinput and “input-first, route-fallback” resolution inElementPictureComponent. - Threaded
caseIddown the shared case-edit component chain, including nestedFieldContainerrendering. - Restored propagation of
(needUpdate)from nested/recursive elements so picture uploads can refresh the host gallery.
File summaries
| File | Description |
|---|---|
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-picture/element-picture.component.ts | Adds optional caseId input and resolves case id from input first, with route fallback. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-container/element-container.component.ts | Adds optional caseId input and re-exposes (needUpdate) for nested containers. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-elements/element-container/element-container.component.html | Forwards [caseId] and (needUpdate) into the nested switch. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.ts | Accepts optional caseId and continues emitting (needUpdate). |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.html | Passes [caseId] to element-picture and into nested element-container. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-switch/case-edit-switch.component.spec.ts | Updates the picture stub to accept the new [caseId] input. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-element/case-edit-element.component.ts | Adds optional caseId input at the top of the chain. |
| eform-client/src/app/common/modules/eform-cases/components/case-edit/case-edit-element/case-edit-element.component.html | Threads [caseId] through the switch, extra-picture slot, and recursive child elements; forwards (needUpdate) recursively. |
Review details
- Files reviewed: 8/8 changed files
- Comments generated: 1
- 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
+59
to
+66
| if (!this.hasCaseIdInput) { | ||
| this.activatedRouteSub$ = this.activateRoute.params.subscribe((params) => { | ||
| this.routeCaseId = +params['id']; | ||
| if (isNaN(this.routeCaseId)) { | ||
| this.routeCaseId = +params['sdkCaseId']; | ||
| } | ||
| }); | ||
| } |
renemadsen
added a commit
to microting/eform-backendconfiguration-plugin
that referenced
this pull request
Sep 3, 2026
…logs (#1159) Completing a calendar event whose eForm has a picture field failed on upload with the toast "Sagen blev ikke fundet" (CaseNotFound). The case was never missing. PrepareComplete materialises the Compliance and returns a real SdkCaseId, and the modal already holds it -- it uses it for getCase() and for the save. It simply never reached the picture component, which resolved its case id from the router URL. Inside a dialog the injected ActivatedRoute is the route the dialog was opened from, and backend-configuration-pn/calendar declares neither :id nor :sdkCaseId, so the id came out NaN and bound to 0 server-side. Bind the id the modals already have. compliance-case-modal carries the same latent defect -- currently unreachable from the web calendar since the combined-complete modal superseded it, but real -- so it is fixed here too. Both bound values are the SDK cases.Id that AddNewImage resolves, taken from Cases.FirstOrDefaultAsync(c => c.Id == compliance.MicrotingSdkCaseId) in PrepareComplete and ToggleComplete respectively -- not a Compliance, planning or occurrence id. Requires the shared-chain change in microting/eform-angular-frontend#8033, which adds the optional caseId input. That must merge first or this fails to compile. Refs #1155 Claude-Session: https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
This was referenced Sep 3, 2026
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.
Fixes #8031
Problem
ElementPictureComponentread its case id only fromActivatedRoute.params(id, falling back tosdkCaseId). Inside aMatDialogthe injected route is the route the dialog was opened from, so on a route declaring neither param both lookups yieldNaN.That
NaNis posted as the literal stringCaseId=NaN;EFormFilesControllerhas no[ApiController]and there is no ModelState filter, so it binds to0, matches no case, and picture upload fails withCaseNotFound— Danish "Sagen blev ikke fundet".Every working consumer is a routed page that happens to carry the id in its URL. The Backend Configuration calendar moved eForm filling from a routed page into a dialog, and the id never got threaded through.
Change
Thread an optional
caseIddown the shared chain:case-edit-element → case-edit-switch → element-picture.The bound input wins; the route lookup stays as a fallback. The three routed consumers (
cases/edit/:id,compliance/:sdkCaseId,backend-configuration-case/:id) are behaviourally unchanged — none binds the input, andhasCaseIdInputrejects non-finite and<= 0values so they fall through to the route.Also closes the nesting gaps in the same chain, both of which dropped the value one level down:
element-container(theFieldContainerbranch) never received or forwardedcaseId, so aPictureinside a question group reproduced the sameNaNbug one level deeper. Found in review.element-containerand the recursivecase-edit-elementforwarded no(needUpdate), so a nested picture would upload successfully but never refresh the host gallery. Only reachable because the id fix made nested upload work.Deliberately not done
Extra pictures get
[caseId]but no[fieldId]. They areExtraFieldValuerows — noFieldIdcolumn — that serialize asfieldId: 0, andelement-picture.component.html:1gates its card on*ngIf="fieldId != 0", so binding that would hide the card along with already-uploaded thumbnails. Extra-picture upload needs a server path writingExtraFieldValuerather thanFieldValue; split out to #8032.Downstream — merge order matters
Plugin CI pins the frontend to
stable. This PR must merge before microting/eform-backendconfiguration-plugin#1155, which binds[caseId]in the two calendar dialogs — otherwise that build fails to compile, not merely to test.Verification
Purely additive; the new inputs are optional and no existing consumer is forced to change. No API, base or SDK change, no migration.
tsc --noEmit: 0 errors in all changed.tsfiles (the repo-wide errors are a pre-existing jest-vs-Chaitypesmisconfiguration present in untouched files).ng serverebuilt cleanly after each edit batch, no error lines.Reviewed by subagent; the
element-containergap above was a review finding, verified against the code before fixing.🤖 Generated with Claude Code
https://claude.ai/code/session_015sXLtgzZU8QL9m84GqMkoJ