feat(j-s): Allow prosecutor office to change the prosecutor decision #17355
Conversation
…s/add-option-to-change-prosecutor-result
|
Warning There were issues while running some tools. Please review the errors and either fix the tool’s configuration or disable the tool if it’s a critical failure. 🔧 eslint
warning [email protected]: This version is no longer supported. Please see https://eslint.org/version-support for other options. WalkthroughThis pull request introduces changes to the judicial system application, focusing on enhancing the indictment review process. The modifications span multiple files across the backend and frontend, adding a new field for indictment review decisions, creating a new component for reviewer selection, and updating the modal confirmation mechanism. The changes primarily revolve around expanding the capabilities for public prosecutors to manage and review indictment cases. Changes
Sequence DiagramsequenceDiagram
participant User
participant IndictmentReviewerSelector
participant ReviewDecision
participant ConfirmationModal
User->>IndictmentReviewerSelector: Select Reviewer
IndictmentReviewerSelector->>User: Confirm Selection
User->>ReviewDecision: Choose Decision
ReviewDecision->>ConfirmationModal: Trigger Confirmation
ConfirmationModal->>User: Confirm Decision
User->>ReviewDecision: Submit Final Decision
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
...l-system/web/src/routes/PublicProsecutor/Indictments/Overview/IndictmentReviewerSelector.tsx
Outdated
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 10 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
apps/judicial-system/web/src/routes/PublicProsecutor/components/utils.ts
Outdated
Show resolved
Hide resolved
…github.com/island-is/island.is into j-s/add-option-to-change-prosecutor-result
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (8)
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (1)
112-112: Use optional chaining for safety and clarity.
ReplaceonSelect && onSelect(item.value)withonSelect?.(item.value)to reduce complexity.- onSelect && onSelect(item.value) + onSelect?.(item.value)🧰 Tools
🪛 Biome (1.9.4)
[error] 112-112: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (6)
1-3: Remove unused imports if any remain.Double-check whether all imported hooks and modules are referenced. Keeping imports trimmed helps maintain clarity.
5-5: Adhere to naming conventions and imports grouping.Make sure that all UI components from
@island.is/*libraries are consistently grouped to promote clarity in import statements.
46-48: Ensure variable names clearly reflect their purpose.
isReviewedDecisionChangedis descriptive, but consider clarifying or adding comments where used, confirming that it only tracks changes, not final confirmations.
49-52: Refactor modal state to unify handling logic if possible.
confirmationModaland other modal states might benefit from a central state management approach to keep modal usage consistent across the application.
66-66: Notify user or log result after setting reviewer.Adding a toast or log message could help confirm that the reviewer has been successfully assigned, improving user transparency.
143-167: Evaluate form labeling and user feedback.As you toggle between the two sections of the form, consider clarifying button text or disabling conditions so users easily understand why functionality is enabled or disabled.
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (1)
47-50: Centralize modal constants.The constants
CONFIRM_PROSECUTOR_DECISIONandConfirmationModalare helpful. Make sure these remain consistent across all referencing components to avoid confusion.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/judicial-system/backend/src/app/modules/case/guards/rolesRules.ts(1 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/IndictmentReviewerSelector.tsx(1 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts(1 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx(4 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx(7 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/components/utils.ts(1 hunks)apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx(3 hunks)
🧰 Additional context used
📓 Path-based instructions (7)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/guards/rolesRules.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/IndictmentReviewerSelector.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/components/utils.ts (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
📓 Learnings (3)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/IndictmentReviewerSelector.tsx (1)
Learnt from: oddsson
PR: island-is/island.is#16731
File: apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx:172-186
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `IndictmentOverview.tsx`, since the defendants data does not change, using `useMemo` to memoize the filtering logic is unnecessary.
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (3)
Learnt from: thorhildurt
PR: island-is/island.is#17198
File: apps/judicial-system/web/src/routes/Prison/IndictmentOverview/IndictmentOverview.tsx:42-50
Timestamp: 2024-12-11T14:25:44.741Z
Learning: In `IndictmentOverview.tsx`, when updating the punishment type, update the UI state before making the API call to immediately reflect the change.
Learnt from: unakb
PR: island-is/island.is#17112
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:166-178
Timestamp: 2024-12-04T10:10:49.089Z
Learning: In the Judicial System application, error handling for failed case transitions in the `Summary` component (`apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx`) is managed externally via a toaster notification. Therefore, additional error handling within the modal is not necessary.
Learnt from: oddsson
PR: island-is/island.is#16731
File: apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx:172-186
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `IndictmentOverview.tsx`, since the defendants data does not change, using `useMemo` to memoize the filtering logic is unnecessary.
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (4)
Learnt from: oddsson
PR: island-is/island.is#16731
File: apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx:172-186
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `IndictmentOverview.tsx`, since the defendants data does not change, using `useMemo` to memoize the filtering logic is unnecessary.
Learnt from: thorhildurt
PR: island-is/island.is#17198
File: apps/judicial-system/web/src/routes/Prison/IndictmentOverview/IndictmentOverview.tsx:42-50
Timestamp: 2024-12-11T14:25:44.741Z
Learning: In `IndictmentOverview.tsx`, when updating the punishment type, update the UI state before making the API call to immediately reflect the change.
Learnt from: unakb
PR: island-is/island.is#17112
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:166-178
Timestamp: 2024-12-04T10:10:49.089Z
Learning: In the Judicial System application, error handling for failed case transitions in the `Summary` component (`apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx`) is managed externally via a toaster notification. Therefore, additional error handling within the modal is not necessary.
Learnt from: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-11-12T15:15:11.835Z
Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
🪛 Biome (1.9.4)
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx
[error] 112-112: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (20)
apps/judicial-system/web/src/routes/PublicProsecutor/components/utils.ts (3)
1-2: Constants look good and follow naming conventions.
These two constants are clearly named and help keep the code explicit.
4-7: Good use of union type for modal identification.
Exporting a union type of string constants is a clear approach that enforces consistent usage of the modal states in the codebase.
8-11: Functions are concise and effective.
These type guards allow for a clean and type-safe check of modal states.
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts (1)
41-46: Accurate and descriptive message definition.
The new message key and associated text are properly defined to handle the reviewed decision change scenario.
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/IndictmentReviewerSelector.tsx (3)
32-35: Ensure correct error handling or fallback.
The component uses errorPolicy: 'all' and no-cache. Confirm that errors are caught in upstream code or handled gracefully, so the UI doesn't silently fail.
37-53: Memoization is well-placed.
Using useMemo to transform user data ensures that the list is recalculated only when necessary, preventing unnecessary rendering.
75-95: Select component is effectively integrated.
Properly setting the default value and updating state for the selected reviewer aligns with standard React patterns.
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (5)
8-11: Expanded checks for prosecutor roles are correct.
Including isPublicProsecutorUser ensures broader role coverage.
21-21: Well-structured import for the new ConfirmationModal logic.
This clarifies the type usage for modalVisible and unifies modal management.
27-33: Interface updates align with the new modal design.
Shifting from boolean to a union type is an excellent choice for making the state more explicit and ensuring correct usage throughout.
Line range hint 39-54: Initialization with currentDecision keeps the component consistent.
Using state to hold the initially selected decision ensures a smooth user experience.
Line range hint 123-136: Modal usage with typed states is correctly implemented.
By checking isConfirmProsecutorDecisionModal(modalVisible), the code avoids confusion on open/close states.
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (5)
7-10: Verify usage of newly imported judicial-system types.
Ensure that CaseIndictmentRulingDecision and IndictmentCaseReviewDecision are effectively used in the code to avoid any redundancy.
28-34: Check cross-component dependencies for these new imports.
When managing decisions, confirm that ReviewDecision and the related utilities from utils seamlessly integrate with other parts of the system (e.g., state management, error handling).
110-116: Clarify display logic for reviewer selector.
Ensure that the condition !workingCase.indictmentReviewDecision is correct for controlling when the selector is shown, avoiding potential confusion if future decisions are introduced.
117-139: Check the flow for updating the review decision.
- Confirm the
onSelectcallback effectively synchronizes new decisions with backend logic. - Ensure the state's
isReviewedDecisionChangedis reset (if necessary) after successful updates.
169-169: Confirm the side effect of isReviewerAssignedModal.
Validate that the correct modal text is displayed for the assigned reviewer and that the user can successfully proceed after acknowledging.
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (2)
112-114: Validate type safety for modalVisible.
Converting modalVisible from a boolean to ConfirmationModal | undefined is a good approach. Double-check that all related logic properly accounts for this type change.
312-312: Assess next-button click handler.
Tying onNextButtonClick directly to setModalVisible(CONFIRM_PROSECUTOR_DECISION) is straightforward. Ensure there's no unintended side effect on other modals, if any.
apps/judicial-system/backend/src/app/modules/case/guards/rolesRules.ts (1)
60-63: Confirm new field's permission logic.
The addition of 'indictmentReviewDecision' to publicProsecutorFields fits the PR objective. Verify all necessary security checks are in place and that only authorized roles can update this field.
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx
Outdated
Show resolved
Hide resolved
...judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (1)
83-83: Use optional chaining for better code style.Replace the logical AND operator with optional chaining for better readability.
- onSelect && onSelect(item.value) + onSelect?.(item.value)Also applies to: 112-112
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (2)
119-142: Use null coalescing for cleaner default values.Consider using null coalescing operator for cleaner default value handling.
- indictmentAppealDeadline={ - workingCase.indictmentAppealDeadline ?? '' - } - indictmentAppealDeadlineIsInThePast={ - workingCase.indictmentVerdictAppealDeadlineExpired ?? false - } + indictmentAppealDeadline={workingCase.indictmentAppealDeadline ?? ''} + indictmentAppealDeadlineIsInThePast={workingCase.indictmentVerdictAppealDeadlineExpired ?? false}
146-170: Extract complex condition for better readability.Consider extracting the complex
nextIsDisabledcondition into a separate variable for better readability.+ const isReviewerSelectionDisabled = + !selectedIndictmentReviewer || + selectedIndictmentReviewer.value === workingCase.indictmentReviewer?.id || + isLoadingWorkingCase return ( // ... <FormFooter nextButtonIcon="arrowForward" previousUrl={constants.CASES_ROUTE} nextIsLoading={isLoadingWorkingCase} - nextIsDisabled={ - !selectedIndictmentReviewer || - selectedIndictmentReviewer.value === workingCase.indictmentReviewer?.id || - isLoadingWorkingCase - } + nextIsDisabled={isReviewerSelectionDisabled}
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts(1 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx(4 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx(7 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.strings.ts
🧰 Additional context used
📓 Path-based instructions (2)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (1)
Pattern apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
📓 Learnings (1)
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (4)
Learnt from: oddsson
PR: island-is/island.is#16731
File: apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx:172-186
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In `IndictmentOverview.tsx`, since the defendants data does not change, using `useMemo` to memoize the filtering logic is unnecessary.
Learnt from: thorhildurt
PR: island-is/island.is#17198
File: apps/judicial-system/web/src/routes/Prison/IndictmentOverview/IndictmentOverview.tsx:42-50
Timestamp: 2024-12-11T14:25:44.741Z
Learning: In `IndictmentOverview.tsx`, when updating the punishment type, update the UI state before making the API call to immediately reflect the change.
Learnt from: unakb
PR: island-is/island.is#17112
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:166-178
Timestamp: 2024-12-04T10:10:49.089Z
Learning: In the Judicial System application, error handling for failed case transitions in the `Summary` component (`apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx`) is managed externally via a toaster notification. Therefore, additional error handling within the modal is not necessary.
Learnt from: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-11-12T15:15:11.835Z
Learning: User unakb prefers explicit case handling in switch statements for key functionalities like `getRulingDecisionTagColor` to ensure clarity and avoid assumptions that a case was overlooked.
🪛 Biome (1.9.4)
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx
[error] 112-112: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (6)
apps/judicial-system/web/src/routes/PublicProsecutor/components/ReviewDecision/ReviewDecision.tsx (4)
8-11: LGTM! Well-structured imports with proper access control.The addition of
isPublicProsecutorUserand modal-related imports aligns well with the feature requirements.Also applies to: 21-21
27-33: LGTM! Type-safe props interface.The Props interface changes enhance type safety and provide better state management through:
- Strongly typed modal visibility with
ConfirmationModal- Optional
currentDecisionfor existing decisions- Improved callback signature with optional decision parameter
Line range hint
39-54: LGTM! Proper state initialization.The state is correctly initialized with the current decision, enabling the modification of existing decisions.
Line range hint
122-135: LGTM! Type-safe modal handling.The modal implementation demonstrates good practices:
- Type-safe visibility check with
isConfirmProsecutorDecisionModal- Consistent state management across all modal actions
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (2)
46-51: LGTM! Well-structured state management.The state management implementation:
- Tracks decision changes with
isReviewedDecisionChanged- Uses type-safe
ConfirmationModalfor modal state
74-81: Follow codebase style conventions.Based on the codebase conventions:
- Move the function definition above the return statement
- Add parentheses to the if statement
+ const onSelect = (decision?: IndictmentCaseReviewDecision) => { + if (!decision) { + return + } + + const isDecisionChanged = decision !== workingCase.indictmentReviewDecision + setIsReviewedDecisionChanged(isDecisionChanged) + } + return (
What
Why
Screenshots / Gifs
Screen.Recording.2024-12-27.at.10.30.43.mov
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Refactor
Chores