Save the draft version name and use it in case management title, brea… - #860
Save the draft version name and use it in case management title, brea…#860sofiaIvarsRitense wants to merge 1 commit into
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe backend draft creation path now falls back to the base case definition name when no name is provided, with tests for explicit and fallback names. Case management fetches and exposes the selected case definition for page titles and breadcrumbs, clears breadcrumbs on teardown, caches definition results, and refreshes global-active state and menus after updates. Sidebar menu iterations now use stable tracking keys. Release notes document the related fixes. 🚥 Pre-merge checks | ✅ 2✅ Passed checks (2 passed)
✨ 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 |
![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
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 4e18f115-fa55-4c4e-9308-00aec9d96f28
📒 Files selected for processing (9)
backend/case/src/main/kotlin/com/ritense/case/service/CaseDefinitionService.ktbackend/case/src/test/kotlin/com/ritense/case/service/CaseDefinitionServiceTest.ktdocumentation/release-notes/13.x.x/13.39.0/README.mdfrontend/projects/valtimo/case-management/src/lib/components/case-management-deployment/case-management-deployment.component.tsfrontend/projects/valtimo/case-management/src/lib/components/case-management-detail-actions/case-management-detail-actions.component.tsfrontend/projects/valtimo/case-management/src/lib/components/case-management-detail/case-management-detail.component.tsfrontend/projects/valtimo/case-management/src/lib/services/case-detail.service.tsfrontend/projects/valtimo/components/src/lib/components/left-sidebar/left-sidebar.component.htmlfrontend/projects/valtimo/components/src/lib/components/left-sidebar/left-sidebar.component.ts
| * **Case menu and version indicator now update when a version is made globally active** | ||
|
|
||
| Making a version globally active now updates the case menu and the *set as globally active* action immediately. | ||
| Previously the menu kept showing the previously active version and its name until the page was reloaded. |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Avoid repeating “previously” in the release note.
The sentence is clearer as “Before this, the menu kept showing the version that had been active and its name until the page was reloaded.”
🧰 Tools
🪛 LanguageTool
[style] ~100-~100: This adverb was used twice in the sentence. Consider removing one of them or replacing them with a synonym.
Context: ... Previously the menu kept showing the previously active version and its name until the p...
(ADVERB_REPETITION_PREMIUM)
Source: Linters/SAST tools
| public ngOnDestroy(): void { | ||
| this._subscriptions.unsubscribe(); | ||
| this.breadcrumbService.clearThirdBreadcrumb(); | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Do not clear the shared breadcrumb from the deployment child.
CaseManagementDetailComponent also owns and sets the same third breadcrumb. When deployment is a child tab, leaving it destroys this component but leaves the parent mounted; this teardown then removes the parent’s breadcrumb, which is not re-emitted until the case definition changes.
Let the parent own breadcrumb cleanup, or scope cleanup to the route that owns the breadcrumb.
| public readonly globalActiveVersion$: Observable<string | null> = combineLatest([ | ||
| this.caseDefinitionKey$, | ||
| this._refreshGlobalActiveCase$, | ||
| ]).pipe( | ||
| switchMap(([caseDefinitionKey]) => |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Keep the global-active refresh stream alive for null responses.
The inner pipeline can emit null through catchError(() => of(null)), but the following mapping dereferences result.caseDefinitionVersionTag. A failed refresh or no-active-version response will throw and permanently terminate globalActiveVersion$, leaving the active-version indicator and button state stale.
Suggested fix
switchMap(([caseDefinitionKey]) =>
- this.caseManagementService.getGlobalActiveCase(caseDefinitionKey).pipe(
- map(result => result.caseDefinitionVersionTag),
+ this.caseManagementService.getGlobalActiveCase(caseDefinitionKey).pipe(
+ map(result => result?.caseDefinitionVersionTag ?? null),
catchError(() => of(null))
)
)📝 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.
| public readonly globalActiveVersion$: Observable<string | null> = combineLatest([ | |
| this.caseDefinitionKey$, | |
| this._refreshGlobalActiveCase$, | |
| ]).pipe( | |
| switchMap(([caseDefinitionKey]) => | |
| switchMap(([caseDefinitionKey]) => | |
| this.caseManagementService.getGlobalActiveCase(caseDefinitionKey).pipe( | |
| map(result => result?.caseDefinitionVersionTag ?? null), | |
| catchError(() => of(null)) | |
| ) | |
| ) |
| private openBreadcrumbSubscription(): void { | ||
| this._subscriptions.add( | ||
| this.caseDetailService.caseDefinition$.subscribe(caseDefinition => { | ||
| if (!caseDefinition) return; | ||
|
|
||
| const route = `/case-management/case/${caseDefinition.caseDefinitionKey}/version/${caseDefinition.caseDefinitionVersionTag}`; | ||
|
|
||
| this.breadcrumbService.setThirdBreadcrumb({ | ||
| route: [route], | ||
| content: caseDefinition.name, | ||
| href: route, | ||
| }); | ||
| }) | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Clear the breadcrumb when the case-definition lookup returns null.
CaseDetailService maps lookup failures to null, but this handler returns without clearing the existing third breadcrumb. A failed load or version switch can therefore leave the previous version’s name and URL visible.
Suggested fix
this.caseDetailService.caseDefinition$.subscribe(caseDefinition => {
- if (!caseDefinition) return;
+ if (!caseDefinition) {
+ this.breadcrumbService.clearThirdBreadcrumb();
+ return;
+ }📝 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.
| private openBreadcrumbSubscription(): void { | |
| this._subscriptions.add( | |
| this.caseDetailService.caseDefinition$.subscribe(caseDefinition => { | |
| if (!caseDefinition) return; | |
| const route = `/case-management/case/${caseDefinition.caseDefinitionKey}/version/${caseDefinition.caseDefinitionVersionTag}`; | |
| this.breadcrumbService.setThirdBreadcrumb({ | |
| route: [route], | |
| content: caseDefinition.name, | |
| href: route, | |
| }); | |
| }) | |
| ); | |
| private openBreadcrumbSubscription(): void { | |
| this._subscriptions.add( | |
| this.caseDetailService.caseDefinition$.subscribe(caseDefinition => { | |
| if (!caseDefinition) { | |
| this.breadcrumbService.clearThirdBreadcrumb(); | |
| return; | |
| } | |
| const route = `/case-management/case/${caseDefinition.caseDefinitionKey}/version/${caseDefinition.caseDefinitionVersionTag}`; | |
| this.breadcrumbService.setThirdBreadcrumb({ | |
| route: [route], | |
| content: caseDefinition.name, | |
| href: route, | |
| }); | |
| }) | |
| ); |
1 participant
…dcrumb and menu
Describe the changes
Link to the related Github issue: generiekzaakafhandelcomponent/gzac-issues#395
Specify the code branch location:
Relevant 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
Security
The Secure by Design principle has been applied to these changes
Added or changed REST API endpoints have authentication and authorization in place
Valtimo access control checks have been implemented
Dependencies
Newly added dependencies do not introduce known vulnerabilities/CVE's and are in line with the Valtimo license