fix(j-s): Indictment description #17501
Conversation
WalkthroughThe pull request introduces modifications to the indictment count handling in the Judicial System web application. The changes primarily focus on refining how subtypes are processed and integrated into the indictment count logic across multiple components. The modifications affect the way incident descriptions are generated, how subtypes are managed, and how the legal arguments placeholder is defined, with subtle adjustments to the control flow and data processing related to indictment counts. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Finishing Touches
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
|
Datadog ReportBranch report: ✅ 0 Failed, 343 Passed, 0 Skipped, 55.97s Total Time |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17501 +/- ##
=======================================
Coverage 35.61% 35.61%
=======================================
Files 7015 7015
Lines 150339 150344 +5
Branches 42981 42983 +2
=======================================
+ Hits 53541 53543 +2
- Misses 96798 96801 +3
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
![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 (4)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx (1)
296-301: LGTM! Consider adding type annotation for better type safety.The changes correctly address the indictment description update issue by ensuring subtypes are properly included. The implementation maintains immutability and follows React best practices.
Consider adding a type annotation to
updatedIndictmentCountfor better type safety:- const updatedIndictmentCount = { + const updatedIndictmentCount: typeof indictmentCount = { ...indictmentCount, indictmentCountSubtypes: subtypes?.[policeCaseNumber], }apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (1)
315-318: Consider improving readability of the condition.The logic is correct and aligns with the requirement to reset the description when subtypes are removed. However, the condition could be more readable.
Consider this more readable version:
- subtypes.length === 0 || - (subtypes.length > 1 && - (!indictmentCountSubtypes?.length || - indictmentCountSubtypes.length === 0)) + subtypes.length === 0 || + (subtypes.length > 1 && !indictmentCountSubtypes?.length)The second part of the OR condition can be simplified since both checks (
!indictmentCountSubtypes?.lengthandindictmentCountSubtypes.length === 0) are effectively checking the same thing.apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx (2)
Line range hint
254-269: Optimize subtype removal logic and improve error handling.The current implementation:
- Duplicates the filter operation unnecessarily
- Could be simplified for better maintainability
Consider this optimization:
- const remainingSubtypes = subtypes.filter( - (s) => s !== subtype, - ) - + const remainingSubtypes = subtypes.filter((s) => s !== subtype) updatePoliceCase(index, { policeCaseNumber: policeCaseNumbers[index], - subtypes: subtypes.filter((s) => s !== subtype), + subtypes: remainingSubtypes, }) updateIndictmentCount( policeCaseNumbers[index], crimeScene || {}, { - [policeCaseNumbers[index]]: - remainingSubtypes.length === 1 ? remainingSubtypes : [], + [policeCaseNumbers[index]]: remainingSubtypes, }, )The suggested changes:
- Remove duplicate filter operation
- Simplify the logic by always passing remainingSubtypes
- Reuse the remainingSubtypes variable for both updates
Line range hint
226-269: Overall implementation feedback.The changes effectively handle the indictment description update requirements from the PR objectives. The implementation follows React and TypeScript patterns well, but could benefit from:
- Consistent error handling for undefined values
- Simplified logic for subtype management
- Better type safety for the crimeScene object
Consider extracting the subtype management logic into a custom hook to improve reusability and testability. This would also make it easier to maintain consistent behavior across different components.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.tsx(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx(3 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts(1 hunks)apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx(1 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.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/Prosecutor/Indictments/Defendant/Defendant.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/Prosecutor/Indictments/Indictment/IndictmentCount.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/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.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 (4)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (6)
Learnt from: oddsson
PR: island-is/island.is#17192
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx:579-587
Timestamp: 2024-12-17T20:28:47.549Z
Learning: In the `IndictmentCount.tsx` file of the `apps/judicial-system/web` project, constraints on the vehicle registration number are planned to be removed. Avoid suggesting changes related to vehicle registration number validation for now.
Learnt from: oddsson
PR: island-is/island.is#17192
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx:700-758
Timestamp: 2024-12-17T14:33:50.960Z
Learning: In the file `apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx`, the user prefers not to add range validation for the blood alcohol content input.
Learnt from: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx:194-200
Timestamp: 2024-11-27T14:34:38.028Z
Learning: In `Overview.tsx`, the `defendant.subpoenas` array never contains null or undefined `subpoena` objects, so additional null checks are not needed.
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#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.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/Defendant.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: oddsson
PR: island-is/island.is#17192
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx:579-587
Timestamp: 2024-12-17T20:28:47.549Z
Learning: In the `IndictmentCount.tsx` file of the `apps/judicial-system/web` project, constraints on the vehicle registration number are planned to be removed. Avoid suggesting changes related to vehicle registration number validation for now.
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: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx:194-200
Timestamp: 2024-11-27T14:34:38.028Z
Learning: In `Overview.tsx`, the `defendant.subpoenas` array never contains null or undefined `subpoena` objects, so additional null checks are not needed.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts (1)
Learnt from: oddsson
PR: island-is/island.is#17192
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx:579-587
Timestamp: 2024-12-17T20:28:47.549Z
Learning: In the `IndictmentCount.tsx` file of the `apps/judicial-system/web` project, constraints on the vehicle registration number are planned to be removed. Avoid suggesting changes related to vehicle registration number validation for now.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx (4)
Learnt from: oddsson
PR: island-is/island.is#17192
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx:579-587
Timestamp: 2024-12-17T20:28:47.549Z
Learning: In the `IndictmentCount.tsx` file of the `apps/judicial-system/web` project, constraints on the vehicle registration number are planned to be removed. Avoid suggesting changes related to vehicle registration number validation for now.
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#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.
Learnt from: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx:194-200
Timestamp: 2024-11-27T14:34:38.028Z
Learning: In `Overview.tsx`, the `defendant.subpoenas` array never contains null or undefined `subpoena` objects, so additional null checks are not needed.
🔇 Additional comments (3)
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.tsx (1)
315-318: LGTM! The changes effectively handle description resets.The implementation correctly ensures that indictment descriptions are reset when subtypes are removed, preventing stale or incorrect descriptions from persisting. This aligns well with the PR's objective of improving description management when subtypes change.
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Indictment/IndictmentCount.strings.ts (1)
180-181: Consider migration impact of versioned translation ID.The changes improve the placeholder text to be more specific and aligned with legal terminology. However, the versioned ID (
_v1suffix) indicates a breaking change in translations.Run the following script to check for any unmigrated usages of the old translation ID:
✅ Verification successful
Translation ID change is safely migrated
No instances of the old translation ID were found in the codebase, indicating a complete migration to the new versioned ID. The change is safe to proceed.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usages of the old translation ID # to ensure complete migration rg "judicial\.system\.core:indictments_indictment\.indictment_count\.legal_arguments_placeholder[^_]" \ --type ts \ --type tsxLength of output: 152
Script:
#!/bin/bash # Search for old translation ID in all TypeScript files rg "judicial\.system\.core:indictments_indictment\.indictment_count\.legal_arguments_placeholder[^_]" \ -g "*.{ts,tsx}" # Find all files that import the strings file rg "from.*IndictmentCount\.strings" \ -g "*.{ts,tsx}" \ -A 5Length of output: 1934
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Defendant/PoliceCaseInfo/PoliceCaseInfo.tsx (1)
227-235: Improve type safety and error handling for indictment count updates.The current implementation has potential issues:
- Using an empty object as fallback for undefined crimeScene could lead to data loss.
- The logic only updates indictment count when the first subtype is added.
Consider verifying if this behavior aligns with the business requirements. Should the indictment count be updated for subsequent subtype additions as well?
- if (subtypes.length === 1) { + // Always update indictment count when subtypes change updateIndictmentCount( policeCaseNumbers[index], - crimeScene || {}, + crimeScene ?? { place: '', date: null }, { [policeCaseNumbers[index]]: subtypes, }, ) - }✅ Verification successful
The review comment is correct about both type safety and update behavior issues.
- The
crimeScenefallback should match the expected type structure withplaceanddatefields.- The indictment count should be updated for all subtype changes, not just when
subtypes.length === 1, to maintain consistency with other usage patterns in the codebase.The suggested fix in the review comment is appropriate:
- if (subtypes.length === 1) { updateIndictmentCount( policeCaseNumbers[index], - crimeScene || {}, + crimeScene ?? { place: '', date: null }, { [policeCaseNumbers[index]]: subtypes, }, ) - }🏁 Scripts executed
The following scripts were executed for the analysis:
Error: No shell scripts found
Script:
#!/bin/bash # Find type definitions related to crimeScene ast-grep --pattern 'interface $_ { $$$ crimeScene $$$ }' # Find type definitions that might contain crimeScene ast-grep --pattern 'type $_ = { $$$ crimeScene $$$ }' # Find the implementation of updateIndictmentCount ast-grep --pattern 'function updateIndictmentCount($$$) { $$$ }' # Find other usages of updateIndictmentCount rg "updateIndictmentCount" -A 3Length of output: 24225
oddsson
added
the
deprecated:automerge
Fix indictment description
Asana
What
This PR fixes several issues in generating the
IndictmentDescriptionin combo-indictments. When adding/removing subtypes in indictment counts, the indictment description would not always update accordingly. To fix this, we now remove the indictment description when a subtype is removed, so the user needs to "start over".Why
This PR fixes the issues above.
Screenshots / Gifs
Screen.Recording.2025-01-15.at.10.19.26.mov
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
The changes improve the precision of indictment count tracking and provide more contextual guidance for legal argument input in the judicial system web application.