Making it possible to skip a timers from the progress tab - #843
Making it possible to skip a timers from the progress tab#843IvarKoreman-Ritense wants to merge 6 commits into
Conversation
…ve the necessary PBAC permission.
|
Warning Review limit reached
Next review available in: 3 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. 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 (4)
📝 WalkthroughWalkthroughAdds authorization support and case-scoped REST endpoints for listing and skipping process timers. Skips execute timer jobs and publish an auditable 🚥 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
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
backend/process-document/src/test/kotlin/com/ritense/processdocument/web/rest/ProcessInspectionResourceTest.kt (1)
99-116: 🎯 Functional Correctness | 🔴 Critical | ⚡ Quick winFix the stale constructor wiring.
Line 99 passes four arguments to
ProcessInstanceCaseAccessService, but it now accepts onlyProcessDocumentAssociationService. Line 106 also omits the requireddocumentServiceandauthorizationServicearguments forProcessInspectionResource; this test source will not compile.Proposed fix
val caseAccessService = ProcessInstanceCaseAccessService( - documentService, - authorizationService, processDocumentAssociationService, - runtimeService, ) resource = ProcessInspectionResource( + documentService = documentService, + authorizationService = authorizationService, caseAccessService = caseAccessService,
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: da40f6af-4d3f-4640-881f-a9d4999221c0
📒 Files selected for processing (27)
backend/apps/dev/src/main/resources/config/global/permission/processinstance.permission.jsonbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/authorization/OperatonExecutionActionProvider.ktbackend/process-document/src/main/java/com/ritense/processdocument/autoconfigure/ProcessDocumentAutoConfiguration.javabackend/process-document/src/main/java/com/ritense/processdocument/security/config/ProcessDocumentHttpSecurityConfigurer.javabackend/process-document/src/main/java/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentAuditService.javabackend/process-document/src/main/kotlin/com/ritense/processdocument/event/ProcessTimerSkippedEvent.ktbackend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessInstanceCaseAccessService.ktbackend/process-document/src/main/kotlin/com/ritense/processdocument/web/rest/ProcessInspectionResource.ktbackend/process-document/src/main/kotlin/com/ritense/processdocument/web/rest/ProcessTimerResource.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/event/ProcessTimerSkippedEventTest.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/service/ProcessInstanceCaseAccessServiceTest.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/service/impl/OperatonProcessJsonSchemaDocumentAuditServiceTest.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/web/rest/ProcessInspectionResourceTest.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/web/rest/ProcessTimerResourceTest.ktdocumentation/features/case/case-detail/tabs/README.mddocumentation/features/process/README.mddocumentation/release-notes/13.x.x/13.39.0/README.mdfrontend/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/case/src/lib/permissions/case-detail.permissions.tsfrontend/projects/valtimo/case/src/lib/services/case-process-timer.service.tsfrontend/projects/valtimo/case/src/lib/services/index.tsfrontend/projects/valtimo/process/src/lib/models/process.model.tsfrontend/projects/valtimo/process/src/lib/process-diagram/process-diagram.component.scssfrontend/projects/valtimo/process/src/lib/process-diagram/process-diagram.component.tsfrontend/projects/valtimo/shared/assets/core/en.jsonfrontend/projects/valtimo/shared/assets/core/nl.json
| * permission on it. The process-instance execution row has `ID_ == PROC_INST_ID_`, so it is loaded by | ||
| * [processInstanceId]. A missing execution means the instance is not active. | ||
| */ | ||
| private fun requireExecutionModifyPermission(processInstanceId: String) { |
There was a problem hiding this comment.
I discussed this with the team, and we think it's best to have a new entity type for this. The risk right now is that we might in the future want to add other ways of changing a running process that would make sense to also put on operaton execution. Users might then get unintended access to new features.
Our suggestion so far is to make an OperatonTimer entity and perform the complete action. That way we could later also have a different feature to change the timer to a different duration as the modify action.
It would make sense to add a mapper from the OperatonTimer to the OperatonExecution class so you can still refer to the path leading up to the case instances for conditions.
As an example on how something like this could be done, you can look at the ZgwDocument class. Used like this
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
frontend/projects/valtimo/case/src/lib/components/case-detail/tab/progress/progress.component.ts (2)
153-164: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winBind the pending timer to its original process context.
The pending state stores only the timer, while Line 161 uses the currently selected process instance and Line 163 reads the current document ID. If the user changes process instance or navigates while the confirmation is open, the request uses the wrong context and the backend rejects the job as not belonging to that process. Store the document/process IDs with the pending timer, or clear the confirmation when either changes.
176-179: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winRefresh process-instance data after a successful skip.
This success path reloads timers and the diagram, but
processDocumentInstances$is driven only by route changes. If skipping a timer changes the process instance’s active/completed state, the Progress tab keeps stale process-instance data until navigation or a full reload. Add a process-instance refresh trigger and emit it here.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: 2db5a724-d738-4779-bd37-c38952ebe01f
📒 Files selected for processing (17)
backend/apps/dev/src/main/resources/config/global/permission/process-timer.permission.jsonbackend/core/src/main/kotlin/com/ritense/valtimo/autoconfiguration/ValtimoOperatonAutoConfiguration.ktbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/authorization/OperatonTimerActionProvider.ktbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/authorization/OperatonTimerExecutionMapper.ktbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/authorization/OperatonTimerSpecification.ktbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/authorization/OperatonTimerSpecificationFactory.ktbackend/core/src/main/kotlin/com/ritense/valtimo/operaton/domain/OperatonTimer.ktbackend/core/src/test/kotlin/com/ritense/valtimo/operaton/authorization/OperatonTimerExecutionMapperTest.ktbackend/process-document/src/main/java/com/ritense/processdocument/autoconfigure/ProcessDocumentAutoConfiguration.javabackend/process-document/src/main/kotlin/com/ritense/processdocument/service/ProcessInstanceCaseAccessService.ktbackend/process-document/src/main/kotlin/com/ritense/processdocument/web/rest/ProcessTimerResource.ktbackend/process-document/src/test/kotlin/com/ritense/processdocument/web/rest/ProcessTimerResourceTest.ktdocumentation/features/access-control/configurable-elements.mddocumentation/features/access-control/container-conditions.mddocumentation/features/process/README.mddocumentation/release-notes/13.x.x/13.39.0/README.mdfrontend/projects/valtimo/case/src/lib/components/case-detail/tab/progress/progress.component.ts
💤 Files with no reviewable changes (1)
- backend/process-document/src/main/java/com/ritense/processdocument/autoconfigure/ProcessDocumentAutoConfiguration.java
Conflicts: documentation/release-notes/13.x.x/13.39.0/README.md
2 participants
Describe the changes
Link to the related Github issue: generiekzaakafhandelcomponent/gzac-issues#465
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