feat(j-s): Unserviced tag for indictment cases #17352
Conversation
WalkthroughThis pull request introduces enhancements to the judicial system's backend and web components, focusing on subpoena and case state management. The changes primarily involve updating data retrieval mechanisms, improving case state representation, and adding more detailed information about subpoenas and defendants across multiple files. The modifications aim to provide more comprehensive and context-aware case information throughout the application. Changes
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
|
![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 (2)
apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx (1)
12-12: Consider importing only needed components.Although this import is valid, you could import only the components used in the file to keep the overall bundle size smaller.
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)
33-37: Consider adding field descriptions.To improve maintainability, consider adding GraphQL descriptions for the new fields to document their purpose and possible values (especially for the
serviceStatusfield).subpoenas { + """ + Unique identifier for the subpoena record + """ id + """ + Current service status of the subpoena + """ serviceStatus + """ + Reference to the original subpoena + """ subpoenaId }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (9)
apps/judicial-system/backend/src/app/modules/case/case.service.ts(1 hunks)apps/judicial-system/web/src/components/TagCaseState/TagCaseState.strings.ts(1 hunks)apps/judicial-system/web/src/components/TagCaseState/TagCaseState.tsx(7 hunks)apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx(3 hunks)apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx(2 hunks)apps/judicial-system/web/src/routes/Defender/Cases/defenderCases.graphql(1 hunks)apps/judicial-system/web/src/routes/Shared/Cases/ActiveCases.tsx(2 hunks)apps/judicial-system/web/src/routes/Shared/Cases/MobileCase.tsx(1 hunks)apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql(1 hunks)
🧰 Additional context used
📓 Path-based instructions (9)
apps/judicial-system/web/src/routes/Defender/Cases/defenderCases.graphql (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/components/TagCaseState/TagCaseState.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/Shared/Cases/ActiveCases.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/Shared/Cases/cases.graphql (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/Cases/MobileCase.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/backend/src/app/modules/case/case.service.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/Defender/Cases/components/DefenderCasesTable.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/Court/components/CasesInProgressTable/CasesInProgressTable.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/components/TagCaseState/TagCaseState.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 (7)
apps/judicial-system/web/src/routes/Defender/Cases/defenderCases.graphql (1)
Learnt from: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/components/FormProvider/case.graphql:47-47
Timestamp: 2024-11-27T14:33:19.485Z
Learning: In the `Case` GraphQL query at `apps/judicial-system/web/src/components/FormProvider/case.graphql`, the `defendantId` field is not needed in the `subpoenas` object within the `mergedCases.defendants` array.
apps/judicial-system/web/src/routes/Shared/Cases/ActiveCases.tsx (4)
Learnt from: oddsson
PR: island-is/island.is#16939
File: apps/judicial-system/web/src/components/Table/PastCasesTable/PastCasesTable.tsx:126-131
Timestamp: 2024-11-19T21:47:09.093Z
Learning: The `CaseListEntry` interface in `@island.is/judicial-system-web/src/graphql/schema` includes the properties `isValidToDateInThePast`, `indictmentRulingDecision`, and `indictmentDecision`.
Learnt from: oddsson
PR: island-is/island.is#16939
File: apps/judicial-system/web/src/components/Table/PastCasesTable/PastCasesTable.tsx:136-139
Timestamp: 2024-11-20T10:15:04.980Z
Learning: The properties `isValidToDateInThePast`, `indictmentRulingDecision`, and `indictmentDecision` are defined in the `CaseListEntry` interface used in the `PastCasesTable` component.
Learnt from: thorhildurt
PR: island-is/island.is#17285
File: apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx:223-225
Timestamp: 2024-12-18T17:15:59.181Z
Learning: The system currently only allows adding one defendant, so the punishment type logic in the table can default to the first defendant, with a plan to support multiple defendants in the future.
Learnt from: gudjong
PR: island-is/island.is#16760
File: apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx:112-118
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In `apps/judicial-system/web/src/routes/Prosecutor/components/CasesAwaitingConfirmationTable/CasesAwaitingConfirmationTable.tsx`, `apps/judicial-system/web/src/routes/Court/components/CasesAwaitingAssignmentTable/CasesAwaitingAssignmentTable.tsx`, and one instance in `apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx`, it's correct to pass only the `type` prop to `ColumnCaseType` without the `decision` and `parentCaseId` props.
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)
Learnt from: gudjong
PR: island-is/island.is#16863
File: apps/judicial-system/web/src/components/FormProvider/case.graphql:47-47
Timestamp: 2024-11-27T14:33:19.485Z
Learning: In the `Case` GraphQL query at `apps/judicial-system/web/src/components/FormProvider/case.graphql`, the `defendantId` field is not needed in the `subpoenas` object within the `mergedCases.defendants` array.
apps/judicial-system/web/src/routes/Shared/Cases/MobileCase.tsx (1)
Learnt from: thorhildurt
PR: island-is/island.is#17285
File: apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx:223-225
Timestamp: 2024-12-18T17:15:59.181Z
Learning: The system currently only allows adding one defendant, so the punishment type logic in the table can default to the first defendant, with a plan to support multiple defendants in the future.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
Learnt from: unakb
PR: island-is/island.is#16393
File: apps/judicial-system/digital-mailbox-api/src/app/modules/cases/models/subpoena.response.ts:164-169
Timestamp: 2024-11-12T15:15:11.835Z
Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.
apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx (1)
Learnt from: oddsson
PR: island-is/island.is#16939
File: apps/judicial-system/web/src/components/Table/PastCasesTable/PastCasesTable.tsx:126-131
Timestamp: 2024-11-19T21:47:09.093Z
Learning: The `CaseListEntry` interface in `@island.is/judicial-system-web/src/graphql/schema` includes the properties `isValidToDateInThePast`, `indictmentRulingDecision`, and `indictmentDecision`.
apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx (1)
Learnt from: gudjong
PR: island-is/island.is#16760
File: apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx:112-118
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In `apps/judicial-system/web/src/routes/Prosecutor/components/CasesAwaitingConfirmationTable/CasesAwaitingConfirmationTable.tsx`, `apps/judicial-system/web/src/routes/Court/components/CasesAwaitingAssignmentTable/CasesAwaitingAssignmentTable.tsx`, and one instance in `apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx`, it's correct to pass only the `type` prop to `ColumnCaseType` without the `decision` and `parentCaseId` props.
🔇 Additional comments (21)
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.strings.ts (1)
86-90: Additions look good!
The new "Óbirt" tag entry is consistent with the existing map of case states. No issues found.
apps/judicial-system/web/src/routes/Shared/Cases/MobileCase.tsx (1)
89-89: Ensure defendants data validity
Passing "defendants" to TagCaseState is a solid extension. However, consider verifying that "theCase.defendants" is not null or undefined to avoid potential rendering errors.
apps/judicial-system/web/src/routes/Shared/Cases/ActiveCases.tsx (2)
20-23: Imports are correct
The addition of "Defendant" import is consistent with the usage in this file.
111-111: Prop usage is consistent
Providing "defendants" to TagCaseState aligns with the updated component signature.
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.tsx (9)
8-8: Helper import alignment
"isSuccessfulServiceStatus" is a well-named helper that correctly conveys its purpose.
14-14: Defendant type import
The "Defendant" type import from graphQL schema looks correct and is used properly.
35-36: Optional defendants property
Marking "defendants" as optional helps avoid null reference issues if no defendants exist.
38-50: Good approach to check all subpoena statuses
The function "haveAllSubpoenasBeenServiced" correctly determines if each defendant has at least one successfully serviced subpoena. The logic is solid, and the naming is intuitive.
80-80: Extended mapCaseStateToTagVariant signature
Introducing "defendants" as a parameter broadens the contextual logic for tag display without breaking existing usage.
92-100: Óbirt (Unserviced) case state condition
Adding a condition for indictment cases with a scheduled date but unserviced subpoenas is appropriate. This ensures "Óbirt" is highlighted when needed, enhancing user clarity.
118-118: CaseState.ACCEPTED logic
The combined condition for indictment vs. investigation cases is consistent with existing logic.
160-160: Prop extraction
Destructuring "defendants" from props fosters readability.
174-174: Defendant data usage
Passing "defendants" to "mapCaseStateToTagVariant" is necessary for handling the new "Óbirt" scenario. This flow is coherent.
apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx (4)
39-39: Import for Defendant looks good.
This import correctly aligns with the usage of defendant types in the file.
160-160: Prop caseType ensures consistent display logic.
Passing caseType to the TagCaseState component will help maintain correct labeling and styling for each case state.
164-164: Defendants prop enhances context for TagCaseState.
Providing the defendants array supports logic for displaying un/serviced statuses and ensures the UI has sufficient data to reflect indictment states.
14-14: Validate usage of isSuccessfulServiceStatus.
Please ensure that the function imported from @island.is/judicial-system/types is actually used or intended for future use.
apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx (1)
28-31: Added import of Defendant type.
Importing Defendant alongside CaseListEntry is appropriate as you are now passing the defendants array into TagCaseState.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
429-435: Subpoena retrieval in caseListInclude for defendants.
Including the Subpoena model within defendants allows the UI to display subpoena servicing statuses. This addition seems correctly structured.
apps/judicial-system/web/src/routes/Defender/Cases/defenderCases.graphql (1)
26-30: New subpoenas field in defendants.
Extending the query to fetch subpoenas ensures defender views and other UI components can accurately display the subpoena status of each defendant.
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)
33-37: LGTM! The subpoenas field structure aligns with requirements.
The new subpoenas field in the defendants object provides the necessary data structure to support the "Unserviced" tag feature. The inclusion of serviceStatus will enable proper tracking of subpoena service state.
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 (2) |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.tsx (2)
38-44: Consider renaming function for clarityThe function name
haveAllSubpoenasBeenServicedimplies that all subpoenas must be serviced, but the actual logic checks if each defendant has at least one serviced subpoena. Consider renaming tohasEachDefendantAtLeastOneServicedSubpoenafor better clarity.-const haveAllSubpoenasBeenServiced = (defendants: Defendant[]): boolean => { +const hasEachDefendantAtLeastOneServicedSubpoena = (defendants: Defendant[]): boolean => { return defendants.every((defendant) => { - // if at least one subpoena for each defendant was serviced, we return true + // Returns true if the defendant has at least one successfully serviced subpoena return defendant.subpoenas?.some((subpoena) => isSuccessfulServiceStatus(subpoena.serviceStatus), ) }) }
86-94: Consider simplifying nested conditionsThe nested conditions could be simplified using early returns or combining conditions for better readability.
case CaseState.RECEIVED: { - if (isIndictmentCase(caseType) && defendants) { - if (scheduledDate && !haveAllSubpoenasBeenServiced(defendants)) { - return { - color: 'red', - text: formatMessage(strings.notYetServiced), - } - } + if ( + isIndictmentCase(caseType) && + defendants?.length && + scheduledDate && + !hasEachDefendantAtLeastOneServicedSubpoena(defendants) + ) { + return { + color: 'red', + text: formatMessage(strings.notYetServiced), + } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.tsx(7 hunks)apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/judicial-system/web/src/routes/Court/components/CasesInProgressTable/CasesInProgressTable.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.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."
🔇 Additional comments (3)
apps/judicial-system/web/src/components/TagCaseState/TagCaseState.tsx (3)
8-8: LGTM: Type definitions and imports are well-structured
The new imports and interface changes are properly typed and follow TypeScript best practices with optional chaining support.
Also applies to: 14-14, 35-36
154-154: LGTM: Component changes are consistent
The new defendants prop is properly integrated into the component, maintaining consistency with the existing pattern.
Also applies to: 168-168
38-44: Verify handling of edge cases
Please ensure the code handles these edge cases correctly:
- Empty defendants array
- Null subpoenas array
- Missing serviceStatus
Also applies to: 86-94
✅ Verification successful
Edge cases are properly handled in the implementation
The code safely handles all the mentioned edge cases:
-
Empty defendants array: The
every()method returnstruefor empty arrays by design, which is the correct behavior as there are no defendants to check. -
Null subpoenas array: The optional chaining operator (
?.) indefendant.subpoenas?.some()safely handles null/undefined subpoenas array by returning undefined, which gets coerced tofalsein the boolean context. -
Missing serviceStatus: The
isSuccessfulServiceStatusfunction (from@island.is/judicial-system/types) handles undefined/null serviceStatus values, as seen in its implementation which uses a boolean check:Boolean(status && successfulServiceStatus.includes(status)).
The implementation also correctly uses TypeScript's type system with proper null checks and optional chaining, making it type-safe and runtime-safe.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for test coverage of edge cases in the component tests
# Look for test files
fd "TagCaseState.test" --type f
# Search for test cases handling edge scenarios
rg -A 5 "describe|it|test" --glob "*TagCaseState.test*"
Length of output: 238
Script:
#!/bin/bash
# Let's try a broader search for test files and implementation
# First, find the exact path of the component
fd "TagCaseState" --type f
# Then search for test patterns in the implementation
rg "haveAllSubpoenasBeenServiced" -A 5 -B 5
# Look for type definitions to understand the data structure
rg "interface.*Defendant" -A 5
rg "interface.*Subpoena" -A 5
Length of output: 6572
Script:
#!/bin/bash
# Let's check the test file we found and implementation of isSuccessfulServiceStatus
cat "apps/judicial-system/web/src/components/TagCaseState/TagCaseState.spec.ts"
# Look for the implementation of isSuccessfulServiceStatus
rg "isSuccessfulServiceStatus" -A 5 -B 5
# Check ServiceStatus type definition
rg "type.*ServiceStatus|enum.*ServiceStatus" -A 10
Length of output: 29324
Sjá stöðu/tagg á málalista hvort búið er að birta eða ekki
What
Added a new tag "Óbirt" to be displayed on certain case lists for indictment cases.
Why
So that users can easily see whether an indictment case has been serviced when looking over their case list page.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Chores