feat: 802 combobox - #852
Conversation
📝 WalkthroughSummary by CodeRabbit
WalkthroughThe case progress process selector was migrated to a Carbon combo box, with selection handling updated to extract process instance identifiers from list items. The migration screen replaced native selects with combo boxes for definitions, versions, and task mappings. Migration component state now maintains combo box items, refreshes selections during loading and migration, and caches filtered task mapping options. The migration module imports Carbon’s combo box module. 🚥 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: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: b317145c-88da-422f-bdb6-5e73c41c7032
📒 Files selected for processing (5)
frontend/projects/valtimo/case/src/lib/components/case-detail/tab/progress/progress.component.htmlfrontend/projects/valtimo/case/src/lib/components/case-detail/tab/progress/progress.component.tsfrontend/projects/valtimo/migration/src/lib/migration.component.htmlfrontend/projects/valtimo/migration/src/lib/migration.component.tsfrontend/projects/valtimo/migration/src/lib/migration.module.ts
| this.processService | ||
| .getProcessDefinitionVersions(key) | ||
| .subscribe((processDefinitionVersions: ProcessDefinition[]) => { | ||
| this.selectedVersions[type] = processDefinitionVersions; | ||
| this.refreshVersionItems(type); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Discard stale version responses.
Selecting definition A and then B can let A’s slower request overwrite B’s version list. That can subsequently load a version unrelated to the displayed definition.
Proposed fix
.getProcessDefinitionVersions(key)
.subscribe((processDefinitionVersions: ProcessDefinition[]) => {
+ if (this.fields[type].definition !== key) {
+ return;
+ }
this.selectedVersions[type] = processDefinitionVersions;
this.refreshVersionItems(type);
});📝 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.processService | |
| .getProcessDefinitionVersions(key) | |
| .subscribe((processDefinitionVersions: ProcessDefinition[]) => { | |
| this.selectedVersions[type] = processDefinitionVersions; | |
| this.refreshVersionItems(type); | |
| }); | |
| this.processService | |
| .getProcessDefinitionVersions(key) | |
| .subscribe((processDefinitionVersions: ProcessDefinition[]) => { | |
| if (this.fields[type].definition !== key) { | |
| return; | |
| } | |
| this.selectedVersions[type] = processDefinitionVersions; | |
| this.refreshVersionItems(type); | |
| }); |
1 participant
Describe the changes
Link to the related Github issue: generiekzaakafhandelcomponent/gzac-issues#802
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