feat(j-s): Show indictment decision type to public prosecutor users #17079
Conversation
WalkthroughThe pull request introduces several modifications across multiple files in the judicial system application. Key changes include the addition of a Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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 ReportAll test runs ✅ 9 Total Test Services: 0 Failed, 9 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (4) |
oddsson
changed the title
![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
🧹 Outside diff range and nitpick comments (9)
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)
37-40: LGTM! Consider updating GraphQL documentation.The addition of the
courtfield is well-structured and aligns with the query's organization. Since this is a schema change, consider updating the GraphQL documentation to reflect this new field.apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.ts (1)
80-82: LGTM! Consider adding test coverage.The court field is properly typed and decorated. Since this is a model change, consider adding test cases to verify the serialization and deserialization of the new field.
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesAwaitingReview.tsx (1)
81-92: Consider accessibility implications of disabled Tag.While the Tag component implementation correctly displays the decision type, using the
disabledprop might affect accessibility by reducing contrast and potentially making it harder to read.Consider removing the
disabledprop and using appropriate styling to maintain visual hierarchy without compromising accessibility:- <Tag variant="darkerBlue" outlined disabled> + <Tag variant="darkerBlue" outlined>apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx (2)
82-96: Consider extracting court abbreviation formatting logic.The court abbreviation formatting logic could be moved to a separate utility function to improve reusability and maintainability.
Consider creating a utility function:
+const formatCourtCaseNumber = (courtName: string | undefined, caseNumber: string | undefined) => { + const courtAbbreviation = districtCourtAbbreviation(courtName) + return `${courtAbbreviation ? `${courtAbbreviation}: ` : ''}${caseNumber ?? ''}` +} // In the component: - cell: (row) => { - const courtAbbreviation = districtCourtAbbreviation( - row.court?.name, - ) - - return ( - <CourtCaseNumber - courtCaseNumber={`${ - courtAbbreviation ? `${courtAbbreviation}: ` : '' - }${row.courtCaseNumber ?? ''}`} - policeCaseNumbers={row.policeCaseNumbers ?? []} - appealCaseNumber={row.appealCaseNumber ?? ''} - /> - ) - }, + cell: (row) => ( + <CourtCaseNumber + courtCaseNumber={formatCourtCaseNumber(row.court?.name, row.courtCaseNumber)} + policeCaseNumbers={row.policeCaseNumbers ?? []} + appealCaseNumber={row.appealCaseNumber ?? ''} + /> + ),
101-112: Consider accessibility implications of disabled Tag.Similar to CasesAwaitingReview.tsx, the Tag component's
disabledprop might affect accessibility.Consider removing the
disabledprop:- <Tag variant="darkerBlue" outlined disabled> + <Tag variant="darkerBlue" outlined>apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (1)
117-147: Consider adding fallback for missing court dataThe implementation correctly displays court abbreviations and ruling decisions. However, consider adding a comment when both court name and court case number are missing.
cell: (row) => { const courtAbbreviation = districtCourtAbbreviation( row.court?.name, ) + if (!courtAbbreviation && !row.courtCaseNumber) { + return <Text color="red">No case number available</Text> + } return ( <CourtCaseNumber courtCaseNumber={`${ courtAbbreviation ? `${courtAbbreviation}: ` : '' }${row.courtCaseNumber ?? ''}`} policeCaseNumbers={row.policeCaseNumbers ?? []} appealCaseNumber={row.appealCaseNumber ?? ''} /> ) },apps/judicial-system/web/src/components/Table/Table.tsx (1)
174-190: Enhance type safety for column key handlingThe
getColumnValuefunction works correctly but could benefit from stronger typing.- column: keyof CaseListEntry, + column: 'defendants' | 'courtCaseNumber' | keyof CaseListEntry,This would make it clearer which columns have special handling and prevent potential runtime errors.
libs/judicial-system/formatters/src/lib/formatters.ts (1)
221-242: Enhance type safety and maintainability of court abbreviationsThe function works correctly but could benefit from TypeScript enums or constants for court names.
+const enum DistrictCourt { + REYKJAVIK = 'Héraðsdómur Reykjavíkur', + REYKJANES = 'Héraðsdómur Reykjaness', + // ... other courts +} + +const COURT_ABBREVIATIONS: Record<DistrictCourt, string> = { + [DistrictCourt.REYKJAVIK]: 'HDR', + [DistrictCourt.REYKJANES]: 'HDRN', + // ... other mappings +} + export const districtCourtAbbreviation = (courtName?: string | null) => { - switch (courtName) { - case 'Héraðsdómur Reykjavíkur': - return 'HDR' - // ... other cases - default: - return '' - } + return COURT_ABBREVIATIONS[courtName as DistrictCourt] ?? '' }This approach would:
- Prevent typos in court names
- Make maintenance easier
- Enable better IDE support
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
412-412: Consider grouping related institution includes together.For better code organization, consider moving the court include next to the prosecutorsOffice include since they are both Institution models.
- { model: Institution, as: 'court' }, { model: Institution, as: 'prosecutorsOffice' }, + { model: Institution, as: 'court' },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (10)
apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/case.service.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts(1 hunks)apps/judicial-system/web/messages/Core/tables.ts(1 hunks)apps/judicial-system/web/src/components/Table/Table.tsx(2 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesAwaitingReview.tsx(5 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx(5 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx(2 hunks)apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql(1 hunks)libs/judicial-system/formatters/src/lib/formatters.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (10)
apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.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/interceptors/caseList.interceptor.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/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."
libs/judicial-system/formatters/src/lib/formatters.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.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/Table/Table.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/messages/Core/tables.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/Tables/CasesAwaitingReview.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/Tables/CasesForReview.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."
📓 Learnings (3)
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (3)
Learnt from: oddsson
PR: island-is/island.is#14673
File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
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#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/PublicProsecutor/Tables/CasesAwaitingReview.tsx (4)
Learnt from: oddsson
PR: island-is/island.is#14673
File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
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: 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.
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/PublicProsecutor/Tables/CasesForReview.tsx (3)
Learnt from: oddsson
PR: island-is/island.is#14673
File: apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesForReview.tsx:79-79
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The implementation of `mapIndictmentCaseStateToTagVariant` in `TagCaseState.tsx`, which maps only `CaseState.ACCEPTED` to a specific tag and defaults others to an "unknown" status, is intentional as per the application's requirements.
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: 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 (9)
apps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts (1)
67-67: Verify that court information is non-sensitive data.
Given the warning comment about sensitive information in this interceptor, please confirm that court information is safe to expose in the case list endpoint.
✅ Verification successful
Let me gather more specific information about the court data sensitivity.
Court information is safe to expose in the case list endpoint
The interceptor file explicitly includes a warning comment about sensitive information and provides a whitelist of safe fields to return. The court field has been deliberately added to this whitelist, indicating it has been reviewed and deemed non-sensitive. The comment also specifies that a separate endpoint should be created if sensitive information needs to be exposed to specific roles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any security-related comments or documentation about court data sensitivity
rg -i "sensitive|security|private" --type ts
Length of output: 67632
Script:
#!/bin/bash
# Search for any security-related comments or documentation specifically about court data
rg -i "court.*sensitive|sensitive.*court" --type ts
# Search for the interceptor file to see the warning comment about sensitive data
cat apps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts
Length of output: 3328
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesAwaitingReview.tsx (3)
5-5: LGTM! Import statements are well-organized.
The new imports are necessary for implementing the indictment decision type display functionality and follow NextJS best practices.
Also applies to: 21-24
54-54: LGTM! Table header addition is appropriate.
The new column for case type enhances the UI by providing clear visibility of the indictment decision type to public prosecutor users, which aligns with the PR objectives.
65-67: LGTM! Context menu implementation is simplified.
The refactoring improves code readability while maintaining the same functionality.
apps/judicial-system/web/messages/Core/tables.ts (1)
137-147: LGTM! Message definitions are well-structured.
The new message definitions for fine and ruling tags follow the established pattern, include clear descriptions, and support proper localization.
apps/judicial-system/web/src/routes/PublicProsecutor/Tables/CasesReviewed.tsx (3)
6-9: LGTM: Import changes are well-organized
The addition of districtCourtAbbreviation is properly grouped with other formatter imports.
95-98: LGTM: Table configuration enhances user experience
The addition of sorting capability for court case numbers and the new case type column improves data organization and visibility.
Also applies to: 106-106
112-114: LGTM: Context menu generation simplified
The refactoring to use arrow function syntax improves code conciseness while maintaining functionality.
apps/judicial-system/web/src/components/Table/Table.tsx (1)
9-12: LGTM: Import changes are well-organized
The addition of districtCourtAbbreviation is properly grouped with other date-related formatters.
oddsson
added
the
deprecated:automerge
…17079) * Add court abbr to getAllCases call * Refactoring * Refactoring * Refactoring * Refactoring * Add indictment decision to tables --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Show indictment decision type to public prosecutor users
Asana
What
Show either FINE or RULING based on the indictment ruling decision for users at the public prosecutors office
Why
For clarity
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
courtreference to the case list entries, enhancing case data representation.Bug Fixes
Documentation
Refactor