14.0.0: formio upgrade - #363
Conversation
Klaas-Ritense
added
the
test env
|
Will create a test environment. This comment will be updated once it is available. This usually takes a few minutes.
Test environment metadata:
Observability: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (3)
frontend/projects/valtimo/zgw/src/lib/modules/documenten-api/formio/documenten-api-uploader/documenten-api-uploader.component.ts (1)
110-134:⚠️ Potential issue | 🟠 MajorFix subscription leak in
documentTypesetter — unsubscribe previous subscription and limit to first emission.The setter creates a new subscription without cleanup, causing memory leaks when the input changes. Additionally, the subscription is not added to the component's
_subscriptionsproperty, so it won't be unsubscribed inngOnDestroy(). Usetake(1)to complete after the firstcaseDefinitionKeyemission (only the current key is needed to resolve the document type), and unsubscribe any previous subscription before creating a new one.Proposed fix
+ private documentTypeSub?: Subscription; + `@Input`() set documentType(defaultValue: string) { this.defaultValues['informatieobjecttype'] = defaultValue; + this.documentTypeSub?.unsubscribe(); + this.documentTypeSub = this.stateService.caseDefinitionKey$ .pipe( filter(caseDefinitionKey => !!caseDefinitionKey), + take(1), switchMap(caseDefinitionKey => this.documentService.getCaseSettings(caseDefinitionKey)), switchMap(caseDefinition => this.documentService.getDocumentTypesForCase( String(caseDefinition.caseDefinitionKey), String(caseDefinition.caseDefinitionVersionTag) ) ), catchError(() => { this.defaultValues['informatieobjecttype'] = defaultValue; return EMPTY; }) ) .subscribe(documentTypes => { const foundDocumentType = documentTypes.find( documentType => documentType.name === defaultValue ); foundDocumentType ? (this.defaultValues['informatieobjecttype'] = foundDocumentType.url) : (this.defaultValues['informatieobjecttype'] = defaultValue); }); }public ngOnDestroy(): void { + this.documentTypeSub?.unsubscribe(); this._subscriptions.unsubscribe(); }frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-uploader/form-io-uploader-edit-form.ts (1)
48-59:⚠️ Potential issue | 🟡 MinorPlaceholder text appears incorrect for the Subtitle field.
The
placeholderon line 53 is set to'Title'but this field is for the Subtitle. This looks like a copy-paste error from the Title field above.Proposed fix
{ type: 'textfield', input: true, key: 'customOptions.subtitle', label: 'Subtitle', - placeholder: 'Title', + placeholder: 'Subtitle', tooltip: 'Leave empty to hide subtitle', weight: 12, validate: { required: false, }, },frontend/projects/valtimo/zgw/src/lib/modules/documenten-api/formio/documenten-api-uploader/documenten-api-uploader.formio.ts (1)
24-33:⚠️ Potential issue | 🟡 MinorUpdate license header year from 2015-2025 to 2015-2026.
The selector change is clean with no stale references remaining. However, the file's copyright year in the license header is outdated and must be updated to 2015-2026 per coding guidelines.
🧹 Nitpick comments (7)
frontend/projects/valtimo/components/src/lib/components/form-io/components/formio-value-resolver-selector/formio-value-resolver-selector.component.ts (1)
36-39: Keep the observable shape stable (avoidundefined).Optional chaining makes
caseDefinitionKey$andcaseDefinitionVersionTag$potentiallyundefined, which can break code that expects an Observable (e.g., template bindings or downstream.pipe(...)). Consider falling back toEMPTY/of(undefined)instead.♻️ Proposed refactor
-import {BehaviorSubject, map} from 'rxjs'; +import {BehaviorSubject, map, of} from 'rxjs'; ... - public readonly caseDefinitionKey$ = formioParams?.pipe(map(params => params?.caseDefinitionKey)); - public readonly caseDefinitionVersionTag$ = formioParams?.pipe( - map(params => params?.caseDefinitionVersionTag) - ); + public readonly caseDefinitionKey$ = + formioParams ? formioParams.pipe(map(params => params?.caseDefinitionKey)) : of(undefined); + public readonly caseDefinitionVersionTag$ = + formioParams ? formioParams.pipe(map(params => params?.caseDefinitionVersionTag)) : of(undefined);backend/app/gzac/src/dev/resources/config/case/formio-test/1-0-0/form/container-test.form.json (1)
8-19: Consider moving validation to the inner textfield.The
requiredvalidation is placed on the container component (line 16-18), but containers don't hold values directly in Form.io. The validation would be more effective on the innercontainerFieldtextfield. If this is intentionally testing container validation behavior, please disregard.💡 Suggested fix to move validation to the inner field
"components": [ { "label": "Container field", "key": "containerField", "type": "textfield", + "validate": { + "required": true + }, "input": true } ], - "validate": { - "required": true - }, "input": truebackend/app/gzac/src/dev/resources/config/case/formio-test/1-0-0/form/documenten-api-uploader-test.form.json (1)
1-27: Use environment variable placeholders for the hardcoded Documenten API URL.The hardcoded
http://localhost:8001endpoint ininformatieobjecttypeshould use the existing placeholder pattern established in related configurations likeformio-test.zgw-document-upload-field.json, which uses${VALTIMO_CATALOGI_API_URL}informatieobjecttypen/${VALTIMO_INFORMATIEOBJECTTYPE}. Multiple files in the test config directory have similar hardcodedlocalhost:8001URLs; adopting the placeholder pattern consistently improves portability across environments.frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-uploader/form-io-uploader-edit-form.ts (1)
96-102: Consider adding a warning for unexpected edit form structure.The null checks prevent runtime errors, but if
tabsComponentordisplayTabis not found, the custom components are silently not added. This could make debugging difficult if the@formio/jslibrary structure changes in a future upgrade.Proposed enhancement with console warning
const tabsComponent = editForm.components.find(component => component.key === 'tabs'); if (tabsComponent) { const displayTab = tabsComponent.components.find(tab => tab.key === 'display'); if (displayTab) { displayTab.components.unshift(...customComponents); + } else { + console.warn('formIoUploaderEditForm: display tab not found in edit form structure'); } + } else { + console.warn('formIoUploaderEditForm: tabs component not found in edit form structure'); }frontend/projects/valtimo/task/src/lib/components/task-detail-content/task-detail-content.component.ts (1)
487-494: Consider renaming method for consistency.The method
setDocumentDefinitionNameInServicestill uses the old naming convention while its implementation now works withcaseDefinitionKey. Consider renaming tosetCaseDefinitionKeyInServicefor consistency with the broader refactoring in this PR.♻️ Suggested refactor
- private setDocumentDefinitionNameInService(task: Task): void { + private setCaseDefinitionKeyInService(task: Task): void { this.documentService .getProcessDefinitionCaseDefinitionFromProcessInstanceId(task.processInstanceId) .subscribe(ProcessDefinitionCaseDefinition => { const caseDefinitionKey = ProcessDefinitionCaseDefinition.id.caseDefinitionId.key; this.modalService.setCaseDefinitionKey(caseDefinitionKey); this.stateService.setCaseDefinitionKey(caseDefinitionKey); }); }Also update the call site at line 274:
- this.setDocumentDefinitionNameInService(task); + this.setCaseDefinitionKeyInService(task);frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-builder/form-io-builder.utils.ts (1)
22-31: Return type mismatch: function declaresvoidbut returns a value.The function signature declares
: voidbut the function returnseditFormon line 30. While this was likely pre-existing, the return type should be corrected to match the actual behavior.🔧 Proposed fix
-const modifyEditFormApiKeyInput = (editForm: any): void => { +const modifyEditFormApiKeyInput = (editForm: any): any => { const keyField = editForm?.components ?.find(element => element?.key === 'tabs') ?.components?.find(element => element?.key === 'api') ?.components?.find(element => element?.key === 'key'); if (keyField) delete keyField.validate; return editForm; };frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-current-user/form-io-current-user-edit-form.ts (1)
21-24: Simplify by returning directly.The intermediate variable
editFormserves no purpose and can be eliminated for conciseness.♻️ Suggested simplification
-export const formIoCurrentUserEditForm = () => { - const editForm = TextfieldEditForm(); - return editForm; -}; +export const formIoCurrentUserEditForm = () => TextfieldEditForm();
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
documentation/release-notes/13.x.x/13.15.0/README.md (1)
23-25: Consider varying the sentence structure (optional style improvement).All three bug fix entries begin with "Fixed." While this is perfectly acceptable for release notes, you could optionally vary the language for stylistic variety (e.g., "Resolved," "Corrected," "Addressed").
…rmio # Conflicts: # frontend/package-lock.json
mbritense
changed the title
Klaas-Ritense
changed the title
…rmio # Conflicts: # documentation/release-notes/13.x.x/13.15.0/README.md # frontend/package-lock.json
Klaas-Ritense
changed the title
…rmio # Conflicts: # frontend/package-lock.json # frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-builder/form-io-builder.component.ts # frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-builder/form-io-builder.utils.ts # frontend/projects/valtimo/components/src/lib/components/form-io/components/formio-value-resolver-selector/formio-value-resolver-selector.component.ts
…rmio # Conflicts: # frontend/package-lock.json # frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-builder/form-io-builder.component.ts # frontend/projects/valtimo/components/src/lib/components/form-io/components/formio-value-resolver-selector/formio-value-resolver-selector.component.ts
…rmio # Conflicts: # frontend/package-lock.json
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (5)
🚧 Files skipped from review as they are similar to previous changes (5)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe frontend upgrades Form.io packages and replaces legacy imports with scoped packages. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (2)
documentation/fundamentals/getting-started/compatibility-matrix.md (1)
13-18: Consider adding a short note forOperatonnaming context.Optional: a one-liner explaining the shift from older
Camundalabels in earlier-major rows would reduce reader confusion.frontend/projects/valtimo/zgw/src/lib/modules/documenten-api/formio/documenten-api-uploader/documenten-api-uploader-edit-form.ts (1)
24-629: Consider extracting shared uploader edit-form constructionLines 24-629 duplicate a substantial uploader-edit configuration/injection pattern also present in
frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-uploader/form-io-uploader-edit-form.ts(custom options +tabs.displayinsertion). Centralizing the shared base builder would reduce drift and make future Form.io upgrades safer.
ℹ️ Review info
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (28)
backend/app/gzac/src/dev/resources/config/case/formio-test/1-0-0/case-definition-process-link/formio-test.case-definition-process-link.jsonbackend/app/gzac/src/dev/resources/config/case/formio-test/1-0-0/form/valtimo-file-uploader-test.form.jsondocumentation/SUMMARY.mddocumentation/fundamentals/getting-started/compatibility-matrix.mddocumentation/release-notes/14.x.x/14.0.0/README.mddocumentation/release-notes/14.x.x/14.0.0/back-end-migration.mddocumentation/release-notes/14.x.x/14.0.0/front-end-migration.mddocumentation/release-notes/14.x.x/README.mdfrontend/package.jsonfrontend/projects/valtimo/components/ng-package.jsonfrontend/projects/valtimo/components/package.jsonfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-builder/form-io-builder.component.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-currency/currency-edit-form.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-current-user/form-io-current-user-edit-form.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-resource-selector/form-io-resource-selector.formio.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-uploader/form-io-uploader-edit-form.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/form-io/form-io.component.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/formio-value-resolver-selector/formio-value-resolver-selector-edit-form.tsfrontend/projects/valtimo/components/src/lib/components/form-io/components/formio-value-resolver-selector/formio-value-resolver-selector.component.tsfrontend/projects/valtimo/components/src/lib/components/form-io/form-io.module.tsfrontend/projects/valtimo/components/src/lib/models/form-io.model.tsfrontend/projects/valtimo/components/src/lib/modules/custom-formio-component/create-custom-component.tsfrontend/projects/valtimo/components/src/lib/modules/custom-formio-component/register-custom-component.tsfrontend/projects/valtimo/form-view-model/ng-package.jsonfrontend/projects/valtimo/form-view-model/package.jsonfrontend/projects/valtimo/form-view-model/src/lib/components/form-view-model/form-view-model.component.tsfrontend/projects/valtimo/zgw/src/lib/modules/documenten-api/formio/documenten-api-uploader/documenten-api-uploader-edit-form.tsfrontend/projects/valtimo/zgw/src/lib/modules/documenten-api/formio/documenten-api-uploader/documenten-api-uploader.formio.ts
…rmio # Conflicts: # frontend/package-lock.json # frontend/projects/valtimo/components/src/lib/components/form-io/components/form-io-uploader/form-io-uploader-edit-form.ts
Bumped in all three manifests that pin formio: the root frontend workspace and the @valtimo/components and @valtimo/form-view-model libraries. Bumping only the root leaves nested copies of the old version under projects/valtimo/<lib>/node_modules/@formio, which collide with the hoisted copy as distinct classes and break FormioModule imports. No application code changes were needed.
…rmio Resolved two integration points with the formio 9->11 upgrade: - form-io.module.ts: both sides added to the FormIoModule constructor. Kept both — bootstrap4 template registration from this branch, and the flatpickr locale wiring from next-major. - formio-flatpickr.ts: next-major imports Formio from 'formiojs', which this branch replaced with '@formio/js'. @formio/angular re-exports Formio straight from @formio/js, so this is the same object the module already uses.
|
Note GitHub couldn't provide a complete incremental comparison for this pull request, so CodeRabbit is performing a full review instead. This review may take a little longer. |
Nightly maintenance run. ng build @valtimo/components failed with TS4029: public property 'submission$' "is using name 'FormioSubmissionState' ... but cannot be named". @formio/angular 11 declares FormioSubmission<T = any, stateType = FormioSubmissionState> and does not export FormioSubmissionState. Inferring the property type resolves that default, so declaration emit has to name a type that is not exported. Annotating the property explicitly makes TypeScript emit the written type instead of the resolved one. Type-only change; no behaviour change.
Nightly maintenance run. - frontend/package.json, projects/valtimo/components, projects/valtimo/form-view-model: @formio/js 5.5.0 -> 5.5.1 - @formio/angular already on latest (11.0.5)
2 participants
generiekzaakafhandelcomponent/gzac-issues#611
Notes:
next-major