-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat(j-s): Add court name abbr. to prison cases table #17171
Conversation
WalkthroughThe pull request introduces modifications to the 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx (1)
192-206
: Simplify the court case number formatting logic.While the implementation is functional, the nested template literals could be simplified for better readability.
Consider this cleaner implementation:
cell: (row) => { const courtAbbreviation = districtCourtAbbreviation( row.court?.name, ) + const prefix = courtAbbreviation ? `${courtAbbreviation}: ` : '' + const caseNumber = row.courtCaseNumber ?? '' return ( <CourtCaseNumber - courtCaseNumber={`${ - courtAbbreviation ? `${courtAbbreviation}: ` : '' - }${row.courtCaseNumber ?? ''}`} + courtCaseNumber={`${prefix}${caseNumber}`} policeCaseNumbers={row.policeCaseNumbers ?? []} appealCaseNumber={row.appealCaseNumber ?? ''} /> ) },
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx
(3 hunks)apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql
(1 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
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/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."
📓 Learnings (1)
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.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 (5)
apps/judicial-system/web/src/routes/Shared/Cases/prisonCases.graphql (1)
47-50
: LGTM! The court field addition is well-structured.
The new court
field with id
and name
subfields is appropriately placed and provides the necessary data for displaying court abbreviations.
apps/judicial-system/web/src/routes/Shared/Cases/PrisonCases.tsx (4)
6-9
: LGTM! Clean import restructuring.
The imports are well-organized and properly structured using named imports.
172-175
: LGTM! Good addition of sorting capability.
Adding sorting for the case number column improves user experience and follows the established pattern for sortable columns.
Line range hint 65-206
: LGTM! Excellent performance optimization.
The use of useMemo
for table rendering functions is well-implemented with correct dependencies, preventing unnecessary re-renders of complex table structures.
Line range hint 1-206
: Verify the court abbreviation display in the UI.
The implementation successfully adds court name abbreviations to the prison cases table. To ensure the feature works as expected, please verify:
- Court abbreviations are correctly displayed in the UI
- Sorting works properly with the new court prefix
✅ Verification successful
Court abbreviation implementation is correctly integrated
Based on the verification results, the implementation is complete and correct:
- The
districtCourtAbbreviation
formatter is properly implemented with mappings for all district courts (HDR, HDRN, HDV, etc.) - The formatter is correctly imported and used in the PrisonCases component
- The court abbreviation is properly concatenated with the case number in the format
{abbreviation}: {caseNumber}
- The implementation is consistent with other components in the codebase that use the same pattern
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the implementation of court abbreviations
# Check for the formatter function implementation
echo "Checking for district court abbreviation formatter..."
rg -A 5 "export.*districtCourtAbbreviation"
# Verify court name mapping
echo "Checking court name mapping..."
rg -A 10 "court.*name.*mapping|COURT.*MAPPING"
Length of output: 813
Script:
#!/bin/bash
# Let's verify the complete implementation by checking:
# 1. The GraphQL query to ensure court field is included
# 2. The full formatter implementation
# 3. Usage in the component
# Check the GraphQL query
echo "Checking GraphQL query for court field..."
rg -A 10 "prisonCasesQuery|PrisonCasesQuery" --type ts
# Check the complete formatter implementation
echo "Checking complete district court abbreviation implementation..."
rg -A 15 "districtCourtAbbreviation" --type ts
# Check the component's usage
echo "Checking usage in the component..."
ast-grep --pattern 'const courtAbbreviation = districtCourtAbbreviation($_)'
Length of output: 22172
Script:
#!/bin/bash
# Let's verify the GraphQL query definition to ensure court field is included
echo "Checking GraphQL query definition..."
rg -A 5 "query PrisonCases" --type graphql
# Check if the court field is included in the CaseListEntry type
echo "Checking CaseListEntry type definition..."
rg -A 15 "type CaseListEntry" --type graphql
Length of output: 331
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17171 +/- ##
==========================================
+ Coverage 35.75% 35.77% +0.01%
==========================================
Files 6935 6933 -2
Lines 148065 148001 -64
Branches 42211 42181 -30
==========================================
+ Hits 52942 52946 +4
+ Misses 95123 95055 -68
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 24 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 97 Total Test Services: 0 Failed, 89 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Add court name abbr. to prison cases table
Asana
What
Add court name abbreviations to court case column in the indictments cases table for prisons.
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes