-
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): Allow defenders to filter cases by court date #17232
base: main
Are you sure you want to change the base?
Conversation
WalkthroughThe changes in this pull request focus on 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
|
Datadog ReportBranch report: ✅ 0 Failed, 332 Passed, 0 Skipped, 55.03s Total Time 🔻 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.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx (1)
Line range hint
1-1
: Add documentation and tests for the court date sorting featureWhile the implementation looks good, please consider:
- Adding JSDoc comments to document the sorting behavior for court dates
- Adding unit tests to verify:
- Sorting of court dates including null/undefined values
- Integration with the existing table functionality
Would you like me to help generate the test cases for this feature?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
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."
🔇 Additional comments (1)
apps/judicial-system/web/src/routes/Defender/Cases/components/DefenderCasesTable.tsx (1)
135-141
: Verify sorting behavior for null court dates
The implementation follows the existing pattern, but there are two considerations:
- The title uses "hearingArrangementDate" which might be confusing for users expecting "Court Date".
- The sorting implementation needs to handle cases where courtDate is null/undefined.
Let's verify the sorting implementation:
if (column === 'courtDate') { | ||
return entry.courtDate | ||
} |
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.
🛠️ Refactor suggestion
Consider improving the courtDate handling logic
The current implementation has a few potential issues:
- The raw
courtDate
is returned without any formatting, which might be inconsistent with other date displays in the table. - The fallback to
entry.created
seems incorrect for columns other than 'courtDate' and 'defendants'.
Consider applying this improvement:
if (column === 'courtDate') {
- return entry.courtDate
+ return entry.courtDate ?? null
}
- return entry.created
+ return entry[column] ?? null
Committable suggestion skipped: line range outside the PR's diff.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17232 +/- ##
==========================================
- Coverage 35.74% 35.74% -0.01%
==========================================
Files 6937 6937
Lines 148167 148170 +3
Branches 42250 42251 +1
==========================================
Hits 52969 52969
- Misses 95198 95201 +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.
|
Verjandi geti raðað eftir fyrirtöku á málalista eins og önnur hlutverk geta gert
What
Allow defenders to filter case list by court date
Why
It's convenient
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes