Include process links when exporting and importing a system process - #869
Include process links when exporting and importing a system process#869maarten-ritense wants to merge 6 commits into
Conversation
…481) A process that is not linked to a case definition can now be exported together with its process links and imported on another environment, so changes tested on one environment no longer have to be reconnected by hand elsewhere. During the import, plugin links can be pointed at the plugin configurations of the target environment, and a summary reports which referenced forms, form flows, decision tables and called sub-processes are missing here. Those are exported and imported separately, because they can be shared with other processes. Process links supplied through config/global are now leading: a link that is not in the file is removed. Importing over a system process that may not be updated is refused, and importing over an existing process asks for confirmation first, like uploading a single bpmn file already did. That plain export now names the file after the process instead of diagram.bpmn.
|
Warning Review limit reached
Next review available in: 27 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (6)
📝 WalkthroughSummary by CodeRabbit
WalkthroughThis change adds global process-definition and process-link export support, ZIP import previewing, missing-reference detection, plugin-configuration mapping, and authoritative process-link imports. Backend REST endpoints expose export, preview, and import operations with authorization rules. The frontend adds process-package upload steps, replacement handling, missing-reference summaries, plugin mapping controls, download behavior, service APIs, translations, tests, and documentation updates. 🚥 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (4)
frontend/projects/valtimo/plugin/src/lib/components/plugin-configuration-mapping/plugin-configuration-mapping.component.html (1)
43-51: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueHide decorative SVG from assistive tech.
The arrow icon is purely decorative (visual separator between source/target). Add
aria-hidden="true"so screen readers skip it.♿ Proposed fix
<svg class="plugin-configuration-mapping__arrow" height="16" viewBox="0 0 32 32" width="16" + aria-hidden="true" xmlns="http://www.w3.org/2000/svg" >frontend/projects/valtimo/process-management/src/lib/components/process-management-upload/process-management-upload.component.ts (1)
279-283: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueUnnecessary Blob re-wrap.
fileis already aBlob; wrapping it innew Blob([file], {type: file.type})copies the bytes for no benefit.♻️ Proposed simplification
private toFormData(file: File): FormData { const formData = new FormData(); - formData.append('file', new Blob([file], {type: file.type}), file.name); + formData.append('file', file, file.name); return formData; }backend/form/src/main/kotlin/com/ritense/form/mapper/FormProcessLinkMapper.kt (1)
176-193: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate form-lookup logic with
resolveFormDefinition.The blueprintId-conditional
getFormDefinitionByNamelookup here duplicates the same branching already inresolveFormDefinition(Lines 194-199). Extract a shared helper to avoid the two diverging if they're changed independently.♻️ Proposed refactor
+ private fun findFormDefinition(formName: String, blueprintId: BlueprintId?): java.util.Optional<FormIoFormDefinition> = + if (blueprintId != null) { + formDefinitionService.getFormDefinitionByName(formName, blueprintId) + } else { + formDefinitionService.getFormDefinitionByName(formName) + } + override fun getMissingReference(deployDto: ProcessLinkDeployDto, blueprintId: BlueprintId?): MissingReferenceDto? { deployDto as FormProcessLinkDeployDto - val formDefinition = if (blueprintId != null) { - formDefinitionService.getFormDefinitionByName(deployDto.formDefinitionName, blueprintId) - } else { - formDefinitionService.getFormDefinitionByName(deployDto.formDefinitionName) - } - return if (formDefinition.isPresent) { + return if (findFormDefinition(deployDto.formDefinitionName, blueprintId).isPresent) { null } else { MissingReferenceDto( type = MissingReferenceType.FORM, reference = deployDto.formDefinitionName, activityId = deployDto.activityId, ) } } private fun resolveFormDefinition(formName: String, blueprintId: BlueprintId?): FormIoFormDefinition { - val result = if (blueprintId != null) { - formDefinitionService.getFormDefinitionByName(formName, blueprintId) - } else { - formDefinitionService.getFormDefinitionByName(formName) - } - `@Suppress`("UNCHECKED_CAST") - return (result as java.util.Optional<FormIoFormDefinition>).orElseThrow { + return findFormDefinition(formName, blueprintId).orElseThrow { IllegalStateException("Form definition $formName not found") } }backend/process-link/src/main/kotlin/com/ritense/processlink/service/ProcessDefinitionImportPreviewService.kt (1)
213-235: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPreserve the original exception as cause.
ImportServiceException("Archive could not be read: ${e.message}")discards the original stack trace, matching the detektSwallowedExceptionhint. SinceImportServiceExceptiononly takes a message, useinitCauseto retain diagnosability for production import failures.♻️ Proposed fix
} catch (e: Exception) { - throw ImportServiceException("Archive could not be read: ${e.message}") + throw ImportServiceException("Archive could not be read: ${e.message}").apply { initCause(e) } }Source: Linters/SAST tools
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 9c75a645-46f1-4b95-93ce-60d3493840db
⛔ Files ignored due to path filters (6)
documentation/.gitbook/assets/read-only-system-process.pngis excluded by!**/*.pngdocumentation/.gitbook/assets/setting-system-process-property.pngis excluded by!**/*.pngdocumentation/.gitbook/assets/system-process.pngis excluded by!**/*.pngdocumentation/using-valtimo/process/img/read-only-system-process.pngis excluded by!**/*.pngdocumentation/using-valtimo/process/img/setting-system-process-property.pngis excluded by!**/*.pngdocumentation/using-valtimo/process/img/system-process.pngis excluded by!**/*.png
📒 Files selected for processing (46)
backend/core/src/main/kotlin/com/ritense/valtimo/autoconfiguration/ExportAutoConfiguration.ktbackend/core/src/main/kotlin/com/ritense/valtimo/exporter/GlobalProcessDefinitionExporter.ktbackend/exporter/src/main/kotlin/com/ritense/exporter/request/GlobalProcessDefinitionExportRequest.ktbackend/form-flow/src/main/kotlin/com/ritense/formflow/mapper/FormFlowProcessLinkMapper.ktbackend/form/src/main/kotlin/com/ritense/form/mapper/FormProcessLinkMapper.ktbackend/form/src/test/kotlin/com/ritense/form/mapper/FormProcessLinkMapperTest.ktbackend/importer/src/main/kotlin/com/ritense/importer/ImportService.ktbackend/importer/src/main/kotlin/com/ritense/importer/ValtimoImportService.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/configuration/ProcessLinkAutoConfiguration.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/exporter/GlobalProcessLinkExporter.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/importer/GlobalProcessLinkImporter.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/importer/ProcessLinkImporter.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/mapper/ProcessLinkMapper.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/security/config/ProcessLinkHttpSecurityConfigurer.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/service/ProcessDefinitionImportPreviewService.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/web/rest/ProcessLinkResource.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/web/rest/dto/MissingReferenceDto.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/web/rest/dto/ProcessDefinitionImportPreviewResponseDto.ktbackend/process-link/src/main/kotlin/com/ritense/processlink/web/rest/dto/ProcessDefinitionImportResponseDto.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/exporter/GlobalProcessLinkExporterIntTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/exporter/GlobalProcessLinkExporterTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/importer/GlobalProcessDefinitionRoundTripIntTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/importer/GlobalProcessLinkImporterIntTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/importer/GlobalProcessLinkImporterTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/service/ProcessDefinitionImportPreviewServiceIntTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/service/ProcessDefinitionImportPreviewServiceTest.ktbackend/process-link/src/test/kotlin/com/ritense/processlink/web/rest/ProcessLinkResourceTest.ktdocumentation/features/process/systemprocesses.mddocumentation/release-notes/13.x.x/13.40.0/README.mdfrontend/projects/valtimo/plugin/src/lib/components/plugin-configuration-mapping/plugin-configuration-mapping.component.htmlfrontend/projects/valtimo/plugin/src/lib/components/plugin-configuration-mapping/plugin-configuration-mapping.component.scssfrontend/projects/valtimo/plugin/src/lib/components/plugin-configuration-mapping/plugin-configuration-mapping.component.spec.tsfrontend/projects/valtimo/plugin/src/lib/components/plugin-configuration-mapping/plugin-configuration-mapping.component.tsfrontend/projects/valtimo/plugin/src/public-api.tsfrontend/projects/valtimo/process-management/src/lib/components/process-management-builder/process-management-builder.component.htmlfrontend/projects/valtimo/process-management/src/lib/components/process-management-builder/process-management-builder.component.tsfrontend/projects/valtimo/process-management/src/lib/components/process-management-upload/process-management-upload.component.htmlfrontend/projects/valtimo/process-management/src/lib/components/process-management-upload/process-management-upload.component.scssfrontend/projects/valtimo/process-management/src/lib/components/process-management-upload/process-management-upload.component.spec.tsfrontend/projects/valtimo/process-management/src/lib/components/process-management-upload/process-management-upload.component.tsfrontend/projects/valtimo/process-management/src/lib/constants/process-management.test-ids.tsfrontend/projects/valtimo/process-management/src/lib/models/index.tsfrontend/projects/valtimo/process-management/src/lib/models/process-definition-import.model.tsfrontend/projects/valtimo/process-management/src/lib/services/process-management.service.tsfrontend/projects/valtimo/shared/assets/core/en.jsonfrontend/projects/valtimo/shared/assets/core/nl.json
| this.pluginManagementService | ||
| .getPluginDefinitions() | ||
| .pipe(take(1)) | ||
| .subscribe(definitions => { | ||
| this.loadConfigurations(mappableConfigurations, new Set(definitions.map(({key}) => key))); | ||
| }); | ||
| } | ||
|
|
||
| private loadConfigurations( | ||
| configurations: PluginConfigurationPreview[], | ||
| installedKeys: Set<string> | ||
| ): void { | ||
| const installableKeys = [ | ||
| ...new Set(configurations.map(({pluginDefinitionKey}) => pluginDefinitionKey)), | ||
| ].filter(key => !!key && installedKeys.has(key)); | ||
|
|
||
| if (installableKeys.length === 0) { | ||
| this.buildRows(configurations, new Map(), installedKeys); | ||
| return; | ||
| } | ||
|
|
||
| const requests = installableKeys.reduce( | ||
| (accumulator, key) => ({ | ||
| ...accumulator, | ||
| [key]: this.pluginManagementService | ||
| .getPluginConfigurationsByPluginDefinitionKey(key) | ||
| .pipe(take(1)), | ||
| }), | ||
| {} as Record<string, Observable<PluginConfiguration[]>> | ||
| ); | ||
|
|
||
| forkJoin(requests) | ||
| .pipe(take(1)) | ||
| .subscribe(results => { | ||
| this.buildRows(configurations, new Map(Object.entries(results)), installedKeys); | ||
| }); | ||
| } |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Unhandled errors on plugin-lookup calls silently break the mapping step.
getPluginDefinitions() and getPluginConfigurationsByPluginDefinitionKey() have no error handler. If either call fails, rows$ stays empty and the user is never told; the import can then proceed with an incomplete plugin mapping with no warning.
🔧 Proposed fix
this.pluginManagementService
.getPluginDefinitions()
.pipe(take(1))
- .subscribe(definitions => {
- this.loadConfigurations(mappableConfigurations, new Set(definitions.map(({key}) => key)));
- });
+ .subscribe({
+ next: definitions => {
+ this.loadConfigurations(mappableConfigurations, new Set(definitions.map(({key}) => key)));
+ },
+ error: () => this.rows$.next([]),
+ });Similarly add an error callback to the forkJoin(requests) subscription in loadConfigurations(), and surface a notification/error state to the user.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| this.pluginManagementService | |
| .getPluginDefinitions() | |
| .pipe(take(1)) | |
| .subscribe(definitions => { | |
| this.loadConfigurations(mappableConfigurations, new Set(definitions.map(({key}) => key))); | |
| }); | |
| } | |
| private loadConfigurations( | |
| configurations: PluginConfigurationPreview[], | |
| installedKeys: Set<string> | |
| ): void { | |
| const installableKeys = [ | |
| ...new Set(configurations.map(({pluginDefinitionKey}) => pluginDefinitionKey)), | |
| ].filter(key => !!key && installedKeys.has(key)); | |
| if (installableKeys.length === 0) { | |
| this.buildRows(configurations, new Map(), installedKeys); | |
| return; | |
| } | |
| const requests = installableKeys.reduce( | |
| (accumulator, key) => ({ | |
| ...accumulator, | |
| [key]: this.pluginManagementService | |
| .getPluginConfigurationsByPluginDefinitionKey(key) | |
| .pipe(take(1)), | |
| }), | |
| {} as Record<string, Observable<PluginConfiguration[]>> | |
| ); | |
| forkJoin(requests) | |
| .pipe(take(1)) | |
| .subscribe(results => { | |
| this.buildRows(configurations, new Map(Object.entries(results)), installedKeys); | |
| }); | |
| } | |
| this.pluginManagementService | |
| .getPluginDefinitions() | |
| .pipe(take(1)) | |
| .subscribe({ | |
| next: definitions => { | |
| this.loadConfigurations(mappableConfigurations, new Set(definitions.map(({key}) => key))); | |
| }, | |
| error: () => this.rows$.next([]), | |
| }); | |
| } | |
| private loadConfigurations( | |
| configurations: PluginConfigurationPreview[], | |
| installedKeys: Set<string> | |
| ): void { | |
| const installableKeys = [ | |
| ...new Set(configurations.map(({pluginDefinitionKey}) => pluginDefinitionKey)), | |
| ].filter(key => !!key && installedKeys.has(key)); | |
| if (installableKeys.length === 0) { | |
| this.buildRows(configurations, new Map(), installedKeys); | |
| return; | |
| } | |
| const requests = installableKeys.reduce( | |
| (accumulator, key) => ({ | |
| ...accumulator, | |
| [key]: this.pluginManagementService | |
| .getPluginConfigurationsByPluginDefinitionKey(key) | |
| .pipe(take(1)), | |
| }), | |
| {} as Record<string, Observable<PluginConfiguration[]>> | |
| ); | |
| forkJoin(requests) | |
| .pipe(take(1)) | |
| .subscribe(results => { | |
| this.buildRows(configurations, new Map(Object.entries(results)), installedKeys); | |
| }); | |
| } |
Always export a process link file, also when the process has no process links: an importer only removes the process links of a process it receives a file for, so a process without links left the process links of the target environment in place. Forget the previewed package when another file is uploaded afterwards. Confirming the replacement of a bpmn file that was selected after previewing a package imported that package instead. Return a bad request instead of failing when the plugin configuration mappings cannot be read, keep the cause of an unreadable archive, hide the decorative arrow of the plugin mapping from assistive technology and stop rewrapping the uploaded file in a blob.
An exported system process now describes itself in a valtimo-manifest.json, like an exported case definition and building block already did. The process is the artifact of the export, of the new type PROCESS_DEFINITION, and the plugins of its process links are its dependencies. A bpmn file has no field a manifest can reference, so the title and the version of the artifact are written as literal values. The version tag of the model is preferred over the version of the deployment, which differs per environment. A version tag that encodes a case or building block definition is ignored, so that encoding does not leak into a manifest. The export service collected the manifest dependencies of non-root exporters only, which assumes a single exporter per root request. The bpmn and the process links of a process are exported by two exporters of the same request, so the plugins of the process links were dropped. Dependencies are now collected from every result that does not itself contribute the artifact, which is the reason for the original exception: the dependencies of the exporter of the artifact describe that artifact, for when it is pulled in as a dependency of another export.
The types declared in plugin-configuration-mapping.component.ts now live in plugin/src/lib/models/plugin-configuration-mapping.ts, which makes the component's input type part of the @valtimo/plugin public surface.
| /** | ||
| * Exports the BPMN of a process definition that is not part of a case definition. | ||
| * | ||
| * Unlike [ProcessDefinitionExporter] this exporter does not create related export requests for |
There was a problem hiding this comment.
Why do we not want to export called subprocesses/decision definitions?
There was a problem hiding this comment.
Discussed it a bit more internally, we would like to see this included before we accept it into the product. People will get wrong expectations if they export a process definition that references other process or decision definitions.
There was a problem hiding this comment.
The idea is/was to mimic the existing export of a 'system' process with its process links included. As the existing system process export only exports the bpmn definition and not also its called elements I made the decision for this new export to exclude these as well.
Including these will extend the scope significantly and introduces more complexity if you ask me. It will do a lot more than the existing 'simple' system process export. What should the scope be then, include all called bpmn elements including and referenced form definitions too? What if called elements are used by other bpmn's as well, same question for forms.
Maybe we should discuss this a little more or it could be an iteration of this in the (near) future if there is demand for this.
|
|
||
| override fun getMissingReference(deployDto: ProcessLinkDeployDto, blueprintId: BlueprintId?): MissingReferenceDto? { | ||
| deployDto as FormFlowProcessLinkDeployDto | ||
| val definition = when (blueprintId) { |
There was a problem hiding this comment.
You're calling the same method twice, why are you using a when for this?
|
Testing finding: Screen.Recording.2026-08-04.at.15.35.03.mov |
Yes, that should be the case. Only non 'read-only' system processes can be replaced / updated by an import. A toggle exists (see this page https://docs.valtimo.nl/features/process/systemprocesses#make-system-process-updatable in the docs) to disable this and then also read-only processes can be replaced / updated. |
…e-process-links Resolved a conflict in the release notes: 13.40.0 has been released on next-minor, so the notes for the system process export and import moved to the 13.41.0 skeleton.
3 participants
Describe the changes
Link to the related Github issue: generiekzaakafhandelcomponent/gzac-issues#481
A process that is not linked to a case definition (the processes on
/processes) could only be moved between environments as a bare BPMN file, so its process links had to be reconnected by hand on every next environment. This adds an export that includes the process links, and an import for that package.The import half already existed and was simply unreachable:
GlobalProcessDefinitionImporter,GlobalProcessLinkImporterandValtimoImportService.importGlobalalready consumeconfig/global/bpmn/*.bpmnandconfig/global/process-link/*.process-link.json— that is how the dev app deploysdocument-uploadand its plugin links at startup. What was missing is the export side (no exporter wrote intoconfig/global), the REST endpoints and the UI.Specify the code branch location:
story/481-system-process-export-include-process-links→next-minorRelevant comments:
Breaking changes
Documentation
New features or changes that have been introduced have been documented.
Tests
Unit tests have been added that cover these changes
Integration tests have been added that cover these changes
Describe the testing steps
Export with process linkson/processes/document-uploaddownloads<key>_v<version>_<timestamp>.process.zipcontaining onlyconfig/global/bpmn/document-upload.bpmn,config/global/process-link/document-upload.process-link.jsonandvaltimo-manifest.json, and the process-link JSON matches the checked-in dev-app filevaltimo-manifest.jsonnames the process (Documenten API upload document, typePROCESS_DEFINITION) and listsdocumentenapiandzakenapiasPLUGINdependencies.valtimoVersionis empty when running frombootRun:DefaultValtimoVersionResolverreads it from the JAR manifest, which is absent outside a packaged buildUploadcompletes the importUploaddisabledBackfrom the review step keeps the selected file and a usableUploadbutton.bpmnbehaves exactly as before.bpmn/.zipin Firefox and Safari — cannot be automated,setInputFilesbypasses the dialogSecurity
The Secure by Design principle has been applied to these changes
The import refuses a package before applying any of it when it would fail or when it would overwrite a process that is managed by configuration, so no partially imported state can result. The preview and the import both derive their decisions from the archive contents server-side; nothing from the client decides what may be replaced.
Added or changed REST API endpoints have authentication and authorization in place
All three endpoints are restricted to
ADMINinProcessLinkHttpSecurityConfigurer. They are covered byProcessLinkSecuritySmokeIntegrationTest.Valtimo access control checks have been implemented
These are ADMIN-only management endpoints that run under
runWithoutAuthorization, exactly like the existing case-definition export and import. There is no per-resource permission model for process definitions to check against.Dependencies
Newly added dependencies do not introduce known vulnerabilities/CVE's and are in line with the Valtimo license
No dependencies were added or changed.