feat(j-s): Add punishment type tag column in prison cases overview table #17285
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis pull request introduces enhancements to the judicial system application's case management functionality, focusing on adding support for displaying and tracking punishment types for defendants. The changes span multiple files across the application, including model updates, GraphQL schema modifications, UI components, and utility functions. The primary goal is to provide more detailed information about case and defendant punishment types, enabling better visualization and sorting of case information. Changes
Sequence DiagramsequenceDiagram
participant GraphQL as GraphQL Query
participant Table as Table Component
participant Tags as Punishment Type Tags
GraphQL->>Table: Fetch cases with punishment type
Table->>Tags: Generate punishment type tag
Tags-->>Table: Return tag with color and text
Table->>Table: Render table with punishment type column
Possibly related PRs
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
|
thorhildurt
commented
Dec 18, 2024
apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.ts
Outdated
Show resolved
Hide resolved
thorhildurt
commented
Dec 18, 2024
thorhildurt
commented
Dec 18, 2024
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (4)
apps/judicial-system/web/src/utils/arrayHelpers.ts (1)
4-5: Remove unnecessary optional chainingThe
isPresentArraycheck already guarantees thatarris defined and an array, making the optional chaining redundant.-export const isEmptyArray = <T>(arr: T[] | undefined | null): arr is T[] => - isPresentArray(arr) && arr?.length === 0 +export const isEmptyArray = <T>(arr: T[] | undefined | null): arr is T[] => + isPresentArray(arr) && arr.length === 0apps/judicial-system/web/src/utils/arrayHelpers.spec.ts (1)
11-15: Improve test descriptionsConsider making test descriptions more descriptive of the specific scenario being tested.
- it(`should return ${expected} for input ${input}`, () => { + it(`should return ${expected} when array is ${input === undefined ? 'undefined' : input === null ? 'null' : Array.isArray(input) ? `[${input}]` : input}`, () => {apps/judicial-system/web/src/components/Tags/CaseTag.strings.ts (1)
100-124: Consider improving clarity of punishment type abbreviationsWhile the string definitions are well-structured, the abbreviations in defaultMessage might not be immediately clear to all users:
- 'Óskb.' for imprisonment
- 'Skb.' for probation
- 'VL' for ruling decision fine
- 'ÁS' for signed fine invitation
Consider:
- Adding tooltips to explain abbreviations
- Documenting abbreviations in user documentation
- Translating descriptions to English for better maintainability
apps/judicial-system/web/src/components/Tags/utils.ts (1)
132-147: Consider moving the helper function outside for reusability.The
getPunishmentTypeLabelhelper function is defined insidegetPunishmentTypeTag. Consider moving it outside to make it reusable and improve code organization.+const getPunishmentTypeLabel = (punishmentType?: PunishmentType | null) => { + switch (punishmentType) { + case PunishmentType.IMPRISONMENT: + return strings.punishmentTypeImprisonment + case PunishmentType.PROBATION: + return strings.punishmentTypeProbation + case PunishmentType.FINE: + return strings.punishmentTypeFine + case PunishmentType.INDICTMENT_RULING_DECISION_FINE: + return strings.punishmentTypeIndictmentRulingDecisionFine + case PunishmentType.SIGNED_FINE_INVITATION: + return strings.punishmentTypeSignedFineInvitation + default: + return strings.unknown + } +} export const getPunishmentTypeTag = ( punishmentType?: PunishmentType | null, ): { color: TagVariant text: { id: string; defaultMessage: string; description: string } } | null => { if (!punishmentType) return null - - const getPunishmentTypeLabel = (punishmentType?: PunishmentType | null) => { - switch (punishmentType) { - case PunishmentType.IMPRISONMENT: - return strings.punishmentTypeImprisonment - case PunishmentType.PROBATION: - return strings.punishmentTypeProbation - case PunishmentType.FINE: - return strings.punishmentTypeFine - case PunishmentType.INDICTMENT_RULING_DECISION_FINE: - return strings.punishmentTypeIndictmentRulingDecisionFine - case PunishmentType.SIGNED_FINE_INVITATION: - return strings.punishmentTypeSignedFineInvitation - default: - return strings.unknown - } - }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (10)
apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.ts(2 hunks)apps/judicial-system/web/messages/Core/tables.ts(1 hunks)apps/judicial-system/web/src/components/Table/Table.tsx(1 hunks)apps/judicial-system/web/src/components/Tags/CaseTag.strings.ts(1 hunks)apps/judicial-system/web/src/components/Tags/utils.ts(2 hunks)apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx(4 hunks)apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql(1 hunks)apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql(1 hunks)apps/judicial-system/web/src/utils/arrayHelpers.spec.ts(1 hunks)apps/judicial-system/web/src/utils/arrayHelpers.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/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/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/Shared/Cases/prisonCases.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/Tags/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/Cases/PrisonCases.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/utils/arrayHelpers.spec.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/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/src/components/Tags/CaseTag.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/utils/arrayHelpers.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 (2)
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.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: 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#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: 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.
apps/judicial-system/web/src/components/Table/Table.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 (4)
apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql (1)
29-29: Verify schema compatibility
The addition of punishmentType field looks good. However, we should verify the schema changes across the frontend and backend.
✅ Verification successful
Schema changes are properly implemented and consistent
The verification shows complete and consistent implementation of the punishmentType field across the codebase:
- Properly defined as an enum in
libs/judicial-system/types/src/lib/defendant.ts - Correctly implemented in backend models and DTOs
- Consistently used in frontend components with proper type definitions
- GraphQL schema changes align with both frontend and backend implementations
- Comprehensive UI implementation including translations and display logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistent punishment type field usage across the codebase
# Check backend models and resolvers
echo "Checking backend models and resolvers for punishmentType..."
rg -t ts "punishmentType.*:.*PunishmentType" "apps/judicial-system/api/src"
# Check frontend type definitions and usage
echo "Checking frontend type definitions and usage..."
rg -t ts "punishmentType" "apps/judicial-system/web/src/types"
Length of output: 613
Script:
#!/bin/bash
# Let's verify the PunishmentType definition and its usage more thoroughly
# Check for PunishmentType enum/type definition
echo "Checking PunishmentType definition..."
rg -t ts "enum.*PunishmentType|type.*PunishmentType" -A 5
# Check GraphQL schema definitions for punishment type
echo "Checking GraphQL schema definitions..."
fd -e graphql . -x cat {} \; | grep -A 5 -B 5 "punishmentType"
# Check frontend components using punishmentType
echo "Checking frontend usage..."
rg -t ts "punishmentType" "apps/judicial-system/web/src"
Length of output: 8484
apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql (1)
31-31: Verify query performance with new punishment type fields
While the addition of punishment type fields aligns with the PR objectives, we should verify:
- Impact on query performance with additional fields
- Handling of null/undefined values
- Proper error handling in resolvers
Also applies to: 33-33
apps/judicial-system/web/messages/Core/tables.ts (1)
116-120: LGTM!
The new message for punishment type follows the established pattern and includes all required properties with clear descriptions.
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx (1)
187-190: LGTM!
The new column is properly configured with sorting capability using the defendantsPunishmentType field.
apps/judicial-system/api/src/app/modules/case-list/models/caseList.model.ts
Show resolved
Hide resolved
…s/add-punishment-type-tag-in-prison-cases-overview
Datadog ReportAll test runs ✅ 2 Total Test Services: 0 Failed, 2 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (1)
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17285 +/- ##
==========================================
+ Coverage 35.70% 35.71% +0.01%
==========================================
Files 6923 6947 +24
Lines 148601 148605 +4
Branches 42454 42442 -12
==========================================
+ Hits 53058 53078 +20
+ Misses 95543 95527 -16
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 70 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
gudjong
approved these changes
Dec 19, 2024
|
This PR currently has a merge conflict. Please resolve this and then re-add the |
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/judicial-system/web/src/components/Tags/utils.ts (1)
132-147: Consider extracting the helper function.The nested
getPunishmentTypeLabelfunction could be extracted to module scope for better reusability and testability. This would also improve code organization by separating concerns.+const getPunishmentTypeLabel = (punishmentType?: PunishmentType | null) => { + switch (punishmentType) { + case PunishmentType.IMPRISONMENT: + return strings.punishmentTypeImprisonment + case PunishmentType.PROBATION: + return strings.punishmentTypeProbation + case PunishmentType.FINE: + return strings.punishmentTypeFine + case PunishmentType.INDICTMENT_RULING_DECISION_FINE: + return strings.punishmentTypeIndictmentRulingDecisionFine + case PunishmentType.SIGNED_FINE_INVITATION: + return strings.punishmentTypeSignedFineInvitation + default: + return strings.unknown + } +} export const getPunishmentTypeTag = ( punishmentType?: PunishmentType | null, ): { color: TagVariant text: { id: string; defaultMessage: string; description: string } } | null => { if (!punishmentType) return null - const getPunishmentTypeLabel = (punishmentType?: PunishmentType | null) => { - switch (punishmentType) { - case PunishmentType.IMPRISONMENT: - return strings.punishmentTypeImprisonment - case PunishmentType.PROBATION: - return strings.punishmentTypeProbation - case PunishmentType.FINE: - return strings.punishmentTypeFine - case PunishmentType.INDICTMENT_RULING_DECISION_FINE: - return strings.punishmentTypeIndictmentRulingDecisionFine - case PunishmentType.SIGNED_FINE_INVITATION: - return strings.punishmentTypeSignedFineInvitation - default: - return strings.unknown - } - } return { color: 'red' as TagVariant, text: getPunishmentTypeLabel(punishmentType), } }
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
apps/judicial-system/web/src/components/Tags/utils.ts(2 hunks)apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx(3 hunks)apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql(1 hunks)apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/judicial-system/web/src/routes/Shared/Cases/cases.graphql
- apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql
- apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/web/src/components/Tags/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."
🔇 Additional comments (2)
apps/judicial-system/web/src/components/Tags/utils.ts (2)
5-5: LGTM!
The import of PunishmentType is correctly placed and necessary for the new functionality.
124-153: Verify completeness of punishment type handling.
Let's ensure we're handling all possible punishment types from the enum.
✅ Verification successful
All punishment types from the enum are correctly handled in the switch statement
The verification confirms that the getPunishmentTypeTag function handles all possible values from the PunishmentType enum. The enum defined in libs/judicial-system/types/src/lib/defendant.ts contains exactly the same five values that are handled in the switch statement:
- IMPRISONMENT
- PROBATION
- FINE
- INDICTMENT_RULING_DECISION_FINE
- SIGNED_FINE_INVITATION
Additionally, the function includes a default case that returns an "unknown" string, making it robust against any future enum additions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that all PunishmentType enum values are handled in the switch statement
# Search for the PunishmentType enum definition
ast-grep --pattern 'enum PunishmentType {
$$$
}'
# Compare with current implementation
echo "Current implementation handles:"
echo "- IMPRISONMENT"
echo "- PROBATION"
echo "- FINE"
echo "- INDICTMENT_RULING_DECISION_FINE"
echo "- SIGNED_FINE_INVITATION"
Length of output: 945
kodiakhq
bot
deleted the
j-s/add-punishment-type-tag-in-prison-cases-overview
branch
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What
Why
Screenshots / Gifs
Screen.Recording.2024-12-18.at.16.43.54.mov
Checklist:
Summary by CodeRabbit
New Features
defendantsPunishmentTypeto the case list.Bug Fixes
Tests
Chores