fix(j-s): Generated PDFs #16625
fix(j-s): Generated PDFs #16625
Conversation
WalkthroughThe pull request introduces changes across three components: 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
|
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #16625 +/- ##
=======================================
Coverage 36.78% 36.79%
=======================================
Files 6855 6855
Lines 142315 142283 -32
Branches 40584 40571 -13
=======================================
Hits 52348 52348
+ Misses 89967 89935 -32
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportBranch report: ✅ 0 Failed, 338 Passed, 0 Skipped, 1m 15.48s Total Time 🔻 Code Coverage Decreases vs Default Branch (1)
|
gudjong
added
automerge
![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
🧹 Outside diff range and nitpick comments (2)
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (1)
Line range hint 72-91: LGTM: Enhanced access control logic with improved naming.
The updated logic correctly implements the requirement to show generated PDF files to prosecutors while maintaining existing access for defenders and civil claimants. The variable renaming improves consistency.
Consider extracting complex conditions into separate helper functions for better readability:
const hasDefenderAccess = (defendant: Defendant) =>
defendant.isDefenderChoiceConfirmed &&
defendant.caseFilesSharedWithDefender &&
defendant.defenderNationalId &&
normalizeAndFormatNationalId(user?.nationalId).includes(
defendant.defenderNationalId,
)
const hasCivilClaimantAccess = (civilClaimant: CivilClaimant) =>
civilClaimant.hasSpokesperson &&
civilClaimant.isSpokespersonConfirmed &&
civilClaimant.caseFilesSharedWithSpokesperson &&
civilClaimant.spokespersonNationalId &&
normalizeAndFormatNationalId(user?.nationalId).includes(
civilClaimant.spokespersonNationalId,
)
const shouldDisplayGeneratedPdfFiles =
isProsecutionUser(user) ||
workingCase.defendants?.some(hasDefenderAccess) ||
workingCase.civilClaimants?.some(hasCivilClaimantAccess)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
168-170: Consider adding a comment explaining the unconditional rendering.
To improve code maintainability, consider adding a comment explaining why the IndictmentCaseFilesList is rendered unconditionally, as this is a significant change from the previous conditional rendering approach.
<Box component="section" marginBottom={5}>
+ {/* Always render the files list to ensure visibility of generated PDFs,
+ empty states are handled within the component */}
<IndictmentCaseFilesList workingCase={workingCase} />
</Box>📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
- apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (1 hunks)
- apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1 hunks)
- apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (3 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.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/Indictments/Overview/Overview.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/IndictmentOverview/IndictmentOverview.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 (2)
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (2)
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-06-27T14:37:07.823Z
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: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-10-08T15:39:04.351Z
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/Indictments/Overview/Overview.tsx (2)
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-06-27T14:37:07.823Z
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: unakb
PR: island-is/island.is#15378
File: apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx:86-100
Timestamp: 2024-10-08T15:39:04.351Z
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.
🔇 Additional comments (6)
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (2)
135-137: LGTM! Good fix for the PDF visibility issue.
The unconditional rendering of IndictmentCaseFilesList is a clean solution that properly delegates empty state handling to the component itself, improving separation of concerns.
135-137: Verify empty state handling in IndictmentCaseFilesList.
Since we're now always rendering the component, let's ensure it properly handles cases where workingCase.caseFiles is empty or undefined.
✅ Verification successful
Let me gather more information about the IndictmentCaseFilesList component to verify its empty state handling.
Let's examine the actual implementation of the IndictmentCaseFilesList component.
Empty state handling is properly implemented in IndictmentCaseFilesList
The component safely handles empty or undefined caseFiles through:
- Optional chaining when filtering case files (e.g.,
cf?.filter()) - Conditional rendering with null checks (e.g.,
indictments && indictments.length > 0) - Each section only renders when its corresponding files exist and have items
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if IndictmentCaseFilesList handles empty states
# Test: Look for empty state handling in the component
ast-grep --pattern 'export const IndictmentCaseFilesList = ({ workingCase }: Props) => {
$$$
if (!workingCase.caseFiles $$$) {
$$$
}
$$$
}'
# Test: Check for null checks or optional chaining
rg -A 3 'workingCase\?.caseFiles|workingCase\.caseFiles\?' apps/judicial-system/web/src/components
Length of output: 771
Script:
#!/bin/bash
# Find and examine the IndictmentCaseFilesList component implementation
rg -l "IndictmentCaseFilesList" apps/judicial-system/web/src/
Length of output: 829
Script:
#!/bin/bash
# Get the implementation of IndictmentCaseFilesList component
cat apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.tsx
# Also check its test file for empty state scenarios
cat apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.spec.tsx
Length of output: 10806
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (3)
11-11: LGTM: Import addition aligns with role-based access control requirements.
The addition of isProsecutionUser import is well-placed alongside other role-checking utilities and follows TypeScript conventions.
176-176: LGTM: Consistent prop naming.
The prop name update maintains consistency with the renamed variable shouldDisplayGeneratedPdfFiles.
184-192: Verify the unconditional rendering of IndictmentCaseFilesList.
The component is now rendered unconditionally, which aligns with the PR objective but should be verified to ensure this doesn't cause issues when workingCase has no files.
Let's verify the component's behavior:
✅ Verification successful
Unconditional rendering of IndictmentCaseFilesList is safe and appropriate
The component implementation shows proper handling of empty or undefined cases:
- All file type sections (indictments, criminalRecords, etc.) are rendered conditionally using
&&operators - Each section checks for both existence and non-empty array before rendering
- The component uses TypeScript Props interface ensuring
workingCaseis always provided - The implementation includes proper null checks on optional fields like
workingCase.defendants
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for IndictmentCaseFilesList implementation to verify null/empty case handling
ast-grep --pattern 'const IndictmentCaseFilesList: FC = ({ workingCase, displayGeneratedPDFs }) => {
$$$
}'
Length of output: 111
Script:
#!/bin/bash
# Let's try a different pattern to find the IndictmentCaseFilesList component
ast-grep --pattern 'const IndictmentCaseFilesList = ({$_}) => {
$$$
}'
# Also search for any imports or references to this component
rg "IndictmentCaseFilesList" -A 3
Length of output: 9873
Script:
#!/bin/bash
# Let's check the component implementation and its test file
cat apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.tsx
cat apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.spec.tsx
Length of output: 10806
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
168-170: LGTM! Please verify empty state handling.
The unconditional rendering of IndictmentCaseFilesList aligns with the PR objective to fix PDF visibility issues. This change simplifies the component logic by delegating empty state handling to the child component.
Let's verify the empty state handling in IndictmentCaseFilesList:
✅ Verification successful
Empty state handling is properly implemented - change is safe
The IndictmentCaseFilesList component safely handles empty states through:
- Null checks with optional chaining for
workingCase.caseFiles - Conditional rendering using
&&operators for each file category section - Empty array handling through
.lengthchecks - Type-safe filtering of case files by category
The unconditional rendering in Overview.tsx is appropriate as the component internally manages all possible states of workingCase.caseFiles.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for empty state handling in IndictmentCaseFilesList
ast-grep --pattern 'export const IndictmentCaseFilesList = ({ workingCase }) => {
$$$
}'
Length of output: 92
Script:
#!/bin/bash
# Let's try finding the IndictmentCaseFilesList component with a broader search
fd "IndictmentCaseFilesList" --type f --exec cat {}
Length of output: 12890
Generated PDFs
Yfirlit sækjanda að máli loknu - engin generated skjöl birtast
What
Why
Screenshots / Gifs
Checklist:
Summary by CodeRabbit
New Features
IndictmentCaseFilesListcomponent in the Indictment Overview, improving visibility of case files.Bug Fixes
IndictmentCaseFilesList, ensuring consistent behavior regardless of case file presence.Documentation