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 Run ID: 📒 Files selected for processing (1)
✅ Files skipped from review due to trivial changes (1)
📝 WalkthroughSummary by CodeRabbit
WalkthroughReplaces package ✨ Finishing Touches🧪 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
2 participants
generiekzaakafhandelcomponent/gzac-issues#611
Note: target branch is
next-major