fix(j-s): Merge Case Access #16053
fix(j-s): Merge Case Access #16053
Conversation
WalkthroughThe changes involve enhancements to the judicial system's API and frontend components to support the handling of merged cases. Key modifications include the introduction of optional parameters for merged case IDs in various service methods, controllers, and input types. Additionally, new guards have been implemented to validate requests related to merged cases, and the UI components have been updated to accommodate the display of connected case files and laws broken. These changes aim to improve the flexibility and functionality of case management processes. 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 ReportAll test runs ✅ 3 Total Test Services: 0 Failed, 3 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (3) |
… and have been sent to the public prosecutor
![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 (4)
apps/judicial-system/backend/src/app/modules/case/guards/mergedCaseExists.guard.ts (1)
1-44: Implementation ofMergedCaseExistsGuardlooks good!The guard correctly checks for the existence of a merged case within a given case context and handles the following scenarios appropriately:
- Missing
Caseobject: Throws anInternalServerErrorException.- Missing
mergedCaseId: Allows the request to proceed.mergedCaseIdnot found: Throws aBadRequestException.mergedCaseIdfound: Updates the request parameters and allows the request to proceed.The guard adheres to the NestJS best practices by implementing the
CanActivateinterface and using dependency injection forCaseService.Here are a few suggestions to further improve the code:
Consider adding JSDoc comments to the
canActivatemethod to provide a clear description of its functionality and parameters.Use optional chaining (
?.) consistently when accessingmergedCasesto avoid potentialundefinederrors:const mergedCase = theCase.mergedCases?.find( (mergedCase) => mergedCase.id === mergedCaseId, )
- Consider extracting the logic for finding the merged case into a separate private method to improve readability and maintainability:
private findMergedCase(theCase: Case, mergedCaseId: string) { return theCase.mergedCases?.find( (mergedCase) => mergedCase.id === mergedCaseId, ) }Then, use this method in the
canActivatemethod:const mergedCase = this.findMergedCase(theCase, mergedCaseId)apps/judicial-system/api/src/app/modules/file/file.resolver.ts (1)
88-96: LGTM! The code changes align with the PR objective and should improve data accessibility for merged cases.The addition of the
mergedCaseIdparameter to thegetSignedUrlmethod and the corresponding update to thebackendService.getCaseFileSignedUrlmethod call should enable retrieving signed URLs for files associated with merged cases. This enhancement aligns with the PR objective of ensuring access to data from combined cases for both the defender and the applicant.The code changes are focused and unlikely to introduce any breaking changes or negatively impact existing functionality.
Suggestions for further improvement:
- Consider adding a brief comment explaining the purpose of the
mergedCaseIdparameter to improve code readability and maintainability.- Verify that the
backendService.getCaseFileSignedUrlmethod has been updated to handle themergedCaseIdparameter correctly and that appropriate tests have been added to validate the behavior.apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
Line range hint
1-1138: Consider refactoring the CaseService class to improve maintainability.The
CaseServiceclass has grown quite large and has multiple responsibilities. Here are a few suggestions to improve its design and maintainability:
Break down the class into smaller, more focused classes or services that adhere to the Single Responsibility Principle (SRP). This will make the codebase more modular and easier to understand, test, and maintain.
Extract complex logic from large methods into separate methods or utility functions. This will improve code readability, reusability, and testability.
Add more comments or documentation to explain the purpose and behavior of critical methods and decision points. This will help future maintainers understand the code flow and the rationale behind certain choices.
By applying these refactorings, the
CaseServiceclass can become more maintainable and easier to work with in the long run.
Line range hint
1-1138: Ensure comprehensive test coverage for the CaseService class.Given the critical nature of the functionalities provided by the
CaseServiceclass, it is crucial to have a robust test suite in place. Here are a few recommendations:
Write comprehensive unit tests to cover the various methods and scenarios in the
CaseServiceclass. This will help catch any regressions and ensure the expected behavior of each method.Develop integration tests to verify the interaction with external services and libraries, such as the
SigningService,AwsS3Service, etc. These tests will give confidence that the integration points are working as expected.Test the queueing functionality to ensure that the correct messages are added to the queue for different case events and updates. This will help prevent any issues related to missed or incorrect notifications.
By having a strong test suite, you can maintain the reliability and correctness of the
CaseServiceclass as the codebase evolves.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (34)
- apps/judicial-system/api/src/app/modules/backend/backend.service.ts (2 hunks)
- apps/judicial-system/api/src/app/modules/file/dto/getSignedUrl.input.ts (2 hunks)
- apps/judicial-system/api/src/app/modules/file/file.controller.ts (2 hunks)
- apps/judicial-system/api/src/app/modules/file/file.resolver.ts (1 hunks)
- apps/judicial-system/api/src/app/modules/file/limitedAccessFile.controller.ts (2 hunks)
- apps/judicial-system/api/src/app/modules/file/limitedAccessFile.resolver.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/case.controller.ts (5 hunks)
- apps/judicial-system/backend/src/app/modules/case/case.service.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/filters/case.filter.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/filters/cases.filter.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/filters/test/cases.filter.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/guards/mergedCaseExists.guard.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/caseController/getCaseFilesRecordPdfGuards.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/caseController/getIndictmentPdfGuards.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCaseFilesRecordPdfGuards.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/file/file.controller.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/test/fileController/getCaseFileSignedUrlGuards.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlGuards.spec.ts (2 hunks)
- apps/judicial-system/web/src/components/AccordionItems/ConnectedCaseFilesAccordionItem/ConnectedCaseFilesAccordionItem.tsx (2 hunks)
- apps/judicial-system/web/src/components/AppealCaseFilesOverview/AppealCaseFilesOverview.tsx (0 hunks)
- apps/judicial-system/web/src/components/FormProvider/case.graphql (1 hunks)
- apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (2 hunks)
- apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.tsx (9 hunks)
- apps/judicial-system/web/src/components/PdfButton/PdfButton.tsx (3 hunks)
- apps/judicial-system/web/src/routes/Court/Indictments/Completed/Completed.tsx (4 hunks)
- apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (1 hunks)
- apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx (1 hunks)
- apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (3 hunks)
- apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (3 hunks)
- apps/judicial-system/web/src/utils/hooks/useFileList/index.ts (2 hunks)
Files not reviewed due to no reviewable changes (1)
- apps/judicial-system/web/src/components/AppealCaseFilesOverview/AppealCaseFilesOverview.tsx
Additional context used
Path-based instructions (33)
apps/judicial-system/api/src/app/modules/backend/backend.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."
apps/judicial-system/api/src/app/modules/file/dto/getSignedUrl.input.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/api/src/app/modules/file/file.controller.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/api/src/app/modules/file/file.resolver.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/api/src/app/modules/file/limitedAccessFile.controller.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/api/src/app/modules/file/limitedAccessFile.resolver.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/case.controller.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/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."
apps/judicial-system/backend/src/app/modules/case/filters/case.filter.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/filters/cases.filter.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/filters/test/cases.filter.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/backend/src/app/modules/case/guards/mergedCaseExists.guard.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/limitedAccessCase.controller.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/limitedAccessCase.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."
apps/judicial-system/backend/src/app/modules/case/test/caseController/getCaseFilesRecordPdfGuards.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/backend/src/app/modules/case/test/caseController/getIndictmentPdfGuards.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/backend/src/app/modules/case/test/limitedAccessCaseController/getCaseFilesRecordPdfGuards.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/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.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/backend/src/app/modules/file/file.controller.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/file/limitedAccessFile.controller.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/file/test/fileController/getCaseFileSignedUrlGuards.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/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlGuards.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/AccordionItems/ConnectedCaseFilesAccordionItem/ConnectedCaseFilesAccordionItem.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/FormProvider/case.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/FormProvider/limitedAccessCase.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/IndictmentCaseFilesList/IndictmentCaseFilesList.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/PdfButton/PdfButton.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/Court/Indictments/Completed/Completed.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/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/Court/Indictments/Summary/Summary.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/Prosecutor/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."
apps/judicial-system/web/src/utils/hooks/useFileList/index.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 (1)
apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx (1)
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.
Additional comments not posted (94)
apps/judicial-system/api/src/app/modules/file/dto/getSignedUrl.input.ts (1)
14-18: LGTM!The addition of the optional
mergedCaseIdproperty to theGetSignedUrlInputclass is a valuable enhancement that improves the flexibility of the input type. The use of the@IsOptional()decorator and the@Fielddecorator withnullable: trueensures that the property is correctly defined as an optional field that can be null in GraphQL queries.The changes adhere to the existing coding style and conventions used in the file and are unlikely to introduce any breaking changes or compatibility issues. The expanded capabilities of the input type accommodate scenarios where a case may not be merged or where the merged case ID is not applicable, thereby improving the overall functionality and usability of the API.
Great work on this enhancement!
apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlGuards.spec.ts (3)
2-2: LGTM!The import statement for
MergedCaseExistsGuardis correctly added from thecasemodule.
19-19: LGTM!The test assertion is correctly updated to expect 4 guards instead of 3, reflecting the addition of the new
MergedCaseExistsGuard.
21-23: Verify the impact of the change in the order of the guards.The order of the guards is changed, with
MergedCaseExistsGuardadded as the second guard. This suggests that the existence of a merged case is now checked before verifying the existence of the case file.Please ensure that this change in the order of the guards does not introduce any unintended consequences or break the access control logic for case files.
Run the following script to verify the impact of the change:
Verification successful
Verification: Guard order change is consistent and intentional
The change in the order of guards, specifically the addition of
MergedCaseExistsGuardas an early check, is consistently implemented across different controllers in the codebase. This suggests that the change is a deliberate architectural decision rather than an isolated modification.Key observations:
- In the LimitedAccessFileController,
MergedCaseExistsGuardis the second guard, consistent with the original change.- In the FileController,
MergedCaseExistsGuardis also placed early in the guard chain (fourth out of six guards).The consistent implementation across different controllers indicates that this change is unlikely to introduce unintended consequences. It appears to be a systematic update to the access control logic for case files, possibly to ensure that merged case scenarios are handled correctly before proceeding with other checks.
Given the consistency of the implementation and the absence of any apparent issues, the change in the order of guards seems to be a valid and intentional improvement to the system's architecture.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the impact of the change in the order of the guards. # Test 1: Search for the usage of the guards in the codebase. # Expect: The order of the guards should be consistent across the codebase. rg --type typescript -A 5 $'CaseReadGuard|MergedCaseExistsGuard|CaseFileExistsGuard|LimitedAccessViewCaseFileGuard' # Test 2: Search for any potential issues or TODOs related to the change in the order of the guards. # Expect: No potential issues or TODOs should be found. rg --type typescript $'TODO|FIXME|XXX|HACK|BUG'Length of output: 235
Script:
#!/bin/bash # Description: Verify the impact of the change in the order of the guards. # Test 1: Search for the usage of the guards in the codebase. # Expect: The order of the guards should be consistent across the codebase. rg -g '*.ts' -A 5 'CaseReadGuard|MergedCaseExistsGuard|CaseFileExistsGuard|LimitedAccessViewCaseFileGuard' # Test 2: Search for any potential issues or TODOs related to the change in the order of the guards. # Expect: No potential issues or TODOs should be found. rg -g '*.ts' 'TODO.*guard|FIXME.*guard|XXX.*guard|HACK.*guard|BUG.*guard' # Test 3: Search for the specific file and surrounding context # Expect: To see the implementation and usage of the guards in the relevant file rg -g '*getCaseFileSignedUrlGuards.ts' -A 10 -B 10 'CaseReadGuard|MergedCaseExistsGuard|CaseFileExistsGuard|LimitedAccessViewCaseFileGuard'Length of output: 77208
Script:
#!/bin/bash # Description: Locate and display the content of getCaseFileSignedUrlGuards.ts file # Search for the file file_path=$(fd -t f getCaseFileSignedUrlGuards.ts) if [ -n "$file_path" ]; then echo "File found at: $file_path" echo "File contents:" cat "$file_path" else echo "File getCaseFileSignedUrlGuards.ts not found." echo "Searching for similar files:" fd -t f -e ts | grep -i "getCaseFileSignedUrl.*Guard" fiLength of output: 516
Script:
#!/bin/bash # Description: Display relevant parts of the getCaseFileSignedUrlGuards test files echo "Contents of limitedAccessFileController/getCaseFileSignedUrlGuards.spec.ts:" awk '/describe.*Get case file signed url guards/,/^})/' apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlGuards.spec.ts echo -e "\n\nContents of fileController/getCaseFileSignedUrlGuards.spec.ts:" awk '/describe.*Get case file signed url guards/,/^})/' apps/judicial-system/backend/src/app/modules/file/test/fileController/getCaseFileSignedUrlGuards.spec.tsLength of output: 2089
apps/judicial-system/backend/src/app/modules/file/test/fileController/getCaseFileSignedUrlGuards.spec.ts (2)
6-6: LGTM!The import statement for
MergedCaseExistsGuardis correctly added from the appropriate path in thecasemodule.
22-29: Great job on improving the test case!The updated test case is more comprehensive and efficient:
- It checks for the correct configuration of all six guards in a single test case.
- It ensures that each guard is instantiated correctly and is of the expected type.
- The addition of
MergedCaseExistsGuardto the test case is consistent with the changes mentioned in the PR summary.This change enhances the clarity and maintainability of the guard verification process within the
FileControllertests.apps/judicial-system/backend/src/app/modules/case/test/caseController/getCaseFilesRecordPdfGuards.spec.ts (2)
8-8: LGTM!The import statement for the new guard
MergedCaseExistsGuardfollows the correct syntax and naming conventions. The relative import path also seems correct.
Line range hint
22-31: Test case updated correctly!The test case has been updated correctly to include the new guard
MergedCaseExistsGuard:
- The expected number of guards has been increased from 5 to 6.
- The new guard is instantiated and checked at the correct index (5) in the guards array.
- The test case follows the existing pattern of instantiating and checking each guard.
apps/judicial-system/web/src/components/AccordionItems/ConnectedCaseFilesAccordionItem/ConnectedCaseFilesAccordionItem.tsx (3)
4-4: LGTM!Importing UI components from a shared library is a good practice to maintain consistency across the application.
11-11: LGTM!The new prop
connectedCaseParentIdis correctly typed and follows the naming convention.
Line range hint
15-41: Excellent structural enhancement!Wrapping the
AccordionIteminside anAccordioncomponent is a great way to improve the organization of the accordion items. This change enhances the component's capability to manage and display connected case files more effectively.Also, passing the
connectedCaseParentIdprop to theIndictmentCaseFilesListcomponent ensures that the necessary data is available for its functionality.The JSX structure follows React best practices and is syntactically correct.
apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.spec.ts (2)
7-7: LGTM!The import statement for
MergedCaseExistsGuardis correctly added, following the existing pattern.
Line range hint
22-31: LGTM!The test configuration is correctly updated to include the new
MergedCaseExistsGuard. The changes follow the existing pattern and the new expectation verifies that the guard is correctly instantiated.apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCaseFilesRecordPdfGuards.spec.ts (2)
7-7: LGTM!The import statement for
MergedCaseExistsGuardis correctly added, which aligns with the PR objective of handling merged cases.
22-22: Test case updated correctly!The test case is correctly updated to:
- Increase the expected number of guards from 5 to 6.
- Add an assertion to check if the 6th guard is an instance of
MergedCaseExistsGuard.These changes ensure that the test case validates the presence and type of the newly added guard.
Also applies to: 31-31
apps/judicial-system/backend/src/app/modules/case/test/caseController/getIndictmentPdfGuards.spec.ts (2)
10-10: LGTM!The import statement for
MergedCaseExistsGuardis correctly added, following the existing import style and using the correct relative path.
23-33: Test case refactoring looks good!The test case has been appropriately modified to accommodate the addition of
MergedCaseExistsGuard. The test now checks for a total of 6 guards and verifies the instance types of each guard, including the new guard.The refactoring also improves the clarity and efficiency of the test by consolidating the individual guard tests into a single test case. The test case name accurately reflects its purpose.
apps/judicial-system/web/src/components/PdfButton/PdfButton.tsx (4)
10-11: LGTM!The changes to the
Propsinterface enhance the flexibility of thePdfButtoncomponent by:
- Making
caseIdoptional, allowing for scenarios where a case ID may not be provided.- Adding
connectedCaseParentId, which can be used to determine the URL structure when generating the PDF link.These changes align with the PR objective of improving data accessibility in merged cases.
31-31: LGTM!Adding
connectedCaseParentIdto the destructured props allows the component to access the new optional property, which is necessary to utilize it in the component's logic.
44-46: LGTM!The updated logic for constructing the URL prefix allows the component to generate appropriate PDF links based on the provided properties by conditionally including a segment for
mergedCaseifconnectedCaseParentIdis present.This change enhances the component's functionality in the judicial system application and aligns with the PR objective of improving data accessibility in merged cases.
49-51: LGTM!The updated final URL construction ensures that the appropriate case ID is used in the generated PDF link by using
connectedCaseParentIdif it exists, falling back tocaseIdif it doesn't.This change enhances the component's ability to handle cases where a parent case ID is relevant and aligns with the PR objective of improving data accessibility in merged cases.
apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (4)
Line range hint
13-23: Approved: The new attributes enhance case file data granularity.The addition of the
modified,type,state, andsizefields to thecaseFilesfield provides more detailed information about each case file. This enhancement enables advanced querying and filtering capabilities, such as:
- Sorting files by their last modification date using the
modifiedfield.- Grouping files by their format using the
typefield.- Filtering out archived or inactive files using the
statefield.- Estimating storage requirements and optimizing file retrieval performance using the
sizefield.These improvements contribute to a better user experience and more efficient data management.
171-183: Approved: TheindictmentCountsfield provides valuable granular data.The introduction of the
indictmentCountsfield is a significant enhancement to the case data model. By including attributes such ascaseId,policeCaseNumber,offenses,substances,lawsBroken,incidentDescription, andlegalArguments, this field enables users to access and analyze indictment data at a granular level.The
caseIdandpoliceCaseNumberattributes establish a clear link between indictments and their associated cases, while theoffenses,substances, andlawsBrokenattributes provide crucial information about the nature of the indictments. Furthermore, theincidentDescriptionandlegalArgumentsattributes allow for a more in-depth understanding of the circumstances surrounding each indictment.This level of detail empowers users to make informed decisions and gain valuable insights into the legal proceedings associated with each case.
184-204: Approved: ThemergedCasesfield provides a comprehensive view of related cases.The introduction of the
mergedCasesfield is a valuable addition to the case data model, as it allows users to retrieve information about related cases that have been merged with the primary case. The nested structure withinmergedCasesincludes attributes such asid,courtCaseNumber,type,policeCaseNumbers, andindictmentSubtypes, providing a comprehensive view of each merged case.The presence of the
caseFilesfield within the nested structure is particularly noteworthy, as it ensures that users can access detailed information about the files associated with each merged case. By mirroring the structure of the originalcaseFilesfield, the query maintains consistency and familiarity for users, reducing cognitive load and promoting ease of use.This holistic approach to representing merged cases empowers users to gain a thorough understanding of the relationships between cases and their associated documentation, ultimately facilitating more informed decision-making and analysis.
Line range hint
1-206: Approved: The file adheres to NextJS best practices and leverages GraphQL effectively.Upon reviewing the entire file, it is evident that it adheres to the additional instructions provided for files matching the pattern
apps/**/*. The use of GraphQL in this NextJS application follows best practices and offers several benefits:
The structure of the query is well-defined, with a clear input type (
CaseQueryInput) and a specific set of fields to be returned. This promotes maintainability and scalability.GraphQL allows for efficient data retrieval by enabling clients to request only the data they need, minimizing over-fetching or under-fetching of data. This can improve application performance by reducing the amount of data transferred over the network.
The
.graphqlfile extension suggests that the query is written in the GraphQL schema language, which is compatible with TypeScript. This compatibility allows for the generation of TypeScript types based on the GraphQL schema, enhancing type safety and catching potential errors during development.While the query itself does not directly interact with state management or server-side rendering, it is important to note that proper implementation of these techniques in other parts of the NextJS application contributes to a smooth user experience and optimal performance.
Overall, the use of GraphQL in this NextJS application demonstrates adherence to best practices and sets the foundation for an efficient and maintainable system.
apps/judicial-system/web/src/utils/hooks/useFileList/index.ts (5)
17-17: LGTM!The addition of the optional
connectedCaseParentIdproperty to theParametersinterface is a good change that will enable theuseFileListhook to handle scenarios involving connected or merged cases. This aligns well with the PR objective of improving data visibility in such cases.
20-20: LGTM!The destructuring of
connectedCaseParentIdfrom theParametersobject is consistent with the updated interface and will allow the hook to access the parent case ID when provided. The destructuring syntax is correct.
104-112: LGTM!The updated
queryfunction call effectively leverages theconnectedCaseParentIdto determine the appropriatecaseIdandmergedCaseIdvalues for the GraphQL query. This dynamic adjustment of query parameters based on the case relationship aligns well with the PR objective of improving data visibility in merged cases.The conditional setting of
mergedCaseIdensures that it is only included when both the parent and child case IDs are available, likely to handle specific merged case scenarios. The changes are logically sound and should enable the desired behavior.
114-120: LGTM!The updated dependency array of the memoized
onOpenfunction is comprehensive and should ensure that the function behaves correctly in response to changes in its dependencies.The inclusion of
caseIdandconnectedCaseParentIdis crucial to ensure that the function always uses the most up-to-date case IDs when making the GraphQL query. This is important for maintaining the accuracy and relevance of the query in scenarios involving merged or connected cases.The addition of
getSignedUrl,limitedAccess, andlimitedAccessGetSignedUrlto the dependency array is also necessary, as these values are used within the function body and any changes to them should trigger a re-creation of the memoized function.Overall, the updated dependency array is well-constructed and should provide the desired behavior.
Line range hint
1-134: Overall assessment: LGTM!The code changes in the
useFileListhook are well-designed and effectively contribute to the PR objective of improving data visibility in merged cases. The introduction of theconnectedCaseParentIdparameter and the corresponding adjustments to the GraphQL query logic enable the hook to dynamically adapt its behavior based on the case relationship.The changes are logically sound, maintainable, and align with the goal of enhancing the user experience in scenarios involving connected or merged cases. The updated dependency array of the memoized
onOpenfunction ensures that it remains in sync with its dependencies, providing accurate and relevant behavior.Overall, the code changes in this file are of high quality and are a positive contribution to the codebase.
apps/judicial-system/api/src/app/modules/file/limitedAccessFile.resolver.ts (1)
87-98: LGTM! Verify the backend service method.The changes look good and align with the PR objective of improving data accessibility in merged cases. The addition of the
mergedCaseIdparameter enables the resolver method to handle requests involving merged cases.Please ensure that:
- The
backendService.limitedAccessGetCaseFileSignedUrlmethod signature has been updated to accept the newmergedCaseIdparameter.- The behavior of the backend service method has been thoroughly tested for both regular and merged cases.
You can use the following script to verify the backend service method signature:
Verification successful
Backend service method signature verified and updated correctly.
The
limitedAccessGetCaseFileSignedUrlmethod in the backend service has been successfully updated to include themergedCaseIdparameter. Key points:
- Method location:
./apps/judicial-system/api/src/app/modules/backend/backend.service.ts- Updated signature:
limitedAccessGetCaseFileSignedUrl(caseId: string, id: string, mergedCaseId?: string): Promise<SignedUrl>- The
mergedCaseIdparameter is optional, allowing the method to handle both merged and non-merged cases.The resolver in
limitedAccessFile.resolver.tscorrectly calls this method with all three parameters.Recommendation:
- Ensure thorough testing for both merged and non-merged case scenarios, given the optional nature of the
mergedCaseIdparameter.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the signature of the `limitedAccessGetCaseFileSignedUrl` method in the backend service. # Test: Search for the method declaration. Expect: The method accepts `caseId`, `id`, and `mergedCaseId` parameters. rg --type typescript -A 5 $'limitedAccessGetCaseFileSignedUrl\(caseId, id, mergedCaseId\)'Length of output: 126
Script:
#!/bin/bash # Description: Verify the signature of the `limitedAccessGetCaseFileSignedUrl` method in the backend service. # Attempt 1: Use rg without specifying file type echo "Attempt 1: Using rg without file type specification" rg -A 5 'limitedAccessGetCaseFileSignedUrl\(caseId, id, mergedCaseId\)' # Attempt 2: Use ast-grep to search for the method echo "Attempt 2: Using ast-grep" ast-grep --lang typescript --pattern 'limitedAccessGetCaseFileSignedUrl($_,$_,$_) { $$$ }' # Attempt 3: Use find and grep as a fallback echo "Attempt 3: Using find and grep" find . -type f -name "*.ts" -exec grep -n -A 5 'limitedAccessGetCaseFileSignedUrl' {} +Length of output: 2204
apps/judicial-system/backend/src/app/modules/file/limitedAccessFile.controller.ts (2)
39-39: LGTM!The import statement is correct and follows the project's conventions.
111-118: The changes align with the PR objectives and adhere to the project's best practices.The addition of the
MergedCaseExistsGuardand the modified endpoint route effectively extend the functionality to support merged cases while maintaining the existing security measures. The code segment follows the NextJS best practices, maintains a consistent structure, and leverages TypeScript for type safety.Great work on enhancing the access control logic and providing flexibility for merged case scenarios!
apps/judicial-system/api/src/app/modules/file/limitedAccessFile.controller.ts (3)
55-58: LGTM!The updated
@Getdecorator correctly defines the new route pattern for handling merged case requests. This change aligns with the PR objective of improving access to merged case data.
62-62: Looks good!The addition of the
mergedCaseIdparameter and the conditional file path construction logic correctly handle requests for merged cases. This change aligns with the PR objective of improving access to merged case data.Also applies to: 70-73, 78-78
152-152: Looks good!The changes to the
getIndictmentPdfmethod, including the updated@Getdecorator, the newmergedCaseIdparameter, and the conditional file path construction logic, correctly handle requests for merged cases. These changes align with the PR objective of improving access to merged case data and maintain consistency with thegetCaseFilesRecordPdfmethod.Also applies to: 156-156, 163-166, 171-171
apps/judicial-system/api/src/app/modules/file/file.controller.ts (4)
57-60: LGTM!The updated
@Getdecorator for thegetCaseFilesRecordPdfmethod now supports two URL patterns, allowing for retrieving case files associated with both standard and merged cases. The changes enhance the flexibility of the API while maintaining backward compatibility.
64-64: LGTM!The introduction of the
mergedCaseIdparameter in thegetCaseFilesRecordPdfmethod allows for handling requests for merged cases. The conditional logic ensures that the correct file path is constructed based on the presence ofmergedCaseId, maintaining the existing functionality for standard cases while adding support for merged cases. The changes are backward compatible and do not break existing functionality.Also applies to: 72-75, 80-80
154-154: LGTM!The updated
@Getdecorator for thegetIndictmentPdfmethod now supports two URL patterns, allowing for retrieving indictments associated with both standard and merged cases. The changes enhance the flexibility of the API while maintaining backward compatibility.
158-158: LGTM!The introduction of the
mergedCaseIdparameter in thegetIndictmentPdfmethod allows for handling requests for merged cases. The conditional logic ensures that the correct file path is constructed based on the presence ofmergedCaseId, maintaining the existing functionality for standard cases while adding support for merged cases. The changes are backward compatible and do not break existing functionality.Also applies to: 165-168, 173-173
apps/judicial-system/web/src/components/FormProvider/case.graphql (1)
302-311: LGTM! The additional fields enhance the case file data.The newly added fields
modified,type,category,userGeneratedFilename,displayDate, andsubmittedByprovide more comprehensive information about each case file in merged cases. This aligns well with the PR objective of improving data accessibility.Specifically, the
displayDateandsubmittedByfields can help users better understand the context and timeline of the case files.apps/judicial-system/web/src/routes/Court/Indictments/Summary/Summary.tsx (1)
133-136: Verify if the ConnectedCaseFilesAccordionItem component handles the new prop correctly.The change looks good as it provides the parent case ID to the
ConnectedCaseFilesAccordionItemcomponent, which could enable better management of merged cases.Please ensure that the
ConnectedCaseFilesAccordionItemcomponent is equipped to handle the newconnectedCaseParentIdprop to avoid any unintended behavior. You can use the following script to verify:Verification successful
The new prop is correctly handled in the ConnectedCaseFilesAccordionItem component.
The
connectedCaseParentIdprop is properly defined in the component's interface and correctly passed down to theIndictmentCaseFilesListcomponent. This implementation aligns with the intended use of providing the parent case ID for better management of merged cases.
- The prop is defined in the
Propsinterface- It's correctly destructured in the component's function parameters
- The prop is passed down to the
IndictmentCaseFilesListcomponent as expectedThe change has been successfully implemented and appears to work as intended.
Scripts executed
The following scripts were executed for the analysis:
Script:
# Search for the usage of `connectedCaseParentId` prop within the `ConnectedCaseFilesAccordionItem` component. # Expect: The prop is being used correctly without any errors. rg --type typescript -A 5 $'connectedCaseParentId' apps/judicial-system/web/src/components/ConnectedCaseFilesAccordionItemLength of output: 156
Script:
#!/bin/bash # Check if the file exists echo "Checking if the file exists:" fd ConnectedCaseFilesAccordionItem.tsx apps/judicial-system/web/src/components # List TypeScript files in the components directory echo -e "\nListing TypeScript files in the components directory:" fd -e ts -e tsx . apps/judicial-system/web/src/components # If the file exists, display its content echo -e "\nDisplaying the content of ConnectedCaseFilesAccordionItem (if it exists):" fd ConnectedCaseFilesAccordionItem.tsx apps/judicial-system/web/src/components -x cat {}Length of output: 18635
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (1)
111-114: LGTM! The addition ofconnectedCaseParentIdprop enhances data accessibility.Passing the parent case ID to the
ConnectedCaseFilesAccordionItemcomponent through theconnectedCaseParentIdprop is a good approach. It provides the child component with additional context about the parent case, which can be leveraged for further logic or display purposes.This change aligns with the PR objective of improving data accessibility in merged cases and facilitates better communication and understanding between the involved parties.
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsx (4)
13-13: LGTM!The import statement for
ConnectedCaseFilesAccordionItemis correctly added.
68-70: LGTM!The introduction of
hasLawsBrokenandhasMergeCasesconstants improves code readability by abstracting the conditions for checking laws broken and merged cases. The logic for these checks is implemented correctly.
129-142: LGTM!The conditional rendering logic for displaying the
IndictmentsLawsBrokenAccordionItemandConnectedCaseFilesAccordionItembased on the presence of laws broken and merged cases is implemented correctly. The mapping overworkingCase.mergedCasesand rendering aConnectedCaseFilesAccordionItemfor each merged case inside aBoxwith a uniquekeyprop is also done correctly.
Line range hint
1-200: Code adheres to NextJS best practices and TypeScript usage.The component follows the recommended functional component syntax, uses hooks correctly, and is properly typed using
FCand imported types from the schema. The file is located in theroutesdirectory, which aligns with the NextJS file structure for pages.apps/judicial-system/backend/src/app/modules/file/file.controller.ts (2)
52-52: LGTM!The import statement for the
MergedCaseExistsGuardis syntactically correct. The purpose of the guard is clear based on its name.
Line range hint
132-148: Great work on enhancing the functionality to support merged cases!The
MergedCaseExistsGuardis used appropriately to ensure that a merged case exists before allowing access to the route. The modified route path follows the correct syntax and supports the required endpoints for both standard file requests and those associated with merged cases.These changes align perfectly with the PR objective of improving data accessibility in merged cases, thereby facilitating better communication and understanding between the involved parties. Well done!
apps/judicial-system/web/src/components/IndictmentCaseFilesList/IndictmentCaseFilesList.tsx (5)
35-35: LGTM!The new optional prop
connectedCaseParentIdis a clear and logical addition to thePropsinterface. It follows the existing naming convention and its purpose is easily understandable.
43-43: Simplifies the component interface.The removal of the unused
workingCaseprop and the modification of theRenderFilescomponent to only acceptRenderFilesPropssimplifies the component's interface and improves clarity.
Line range hint
119-130: Consistently passes down theconnectedCaseParentIdprop.The
connectedCaseParentIdprop is consistently passed down to theRenderFilesandPdfButtoncomponents in multiple places. This ensures that the prop is available for any future functionality related to connected cases. The changes follow a consistent pattern and do not introduce any apparent issues.Also applies to: 143-155, 163-171, 171-182, 199-204, 217-217
Line range hint
1-234: Adheres to NextJS best practices and optimally uses TypeScript.The
IndictmentCaseFilesListcomponent follows NextJS best practices by:
- Using a functional component with hooks for state management and side effects.
- Placing the component file in the
componentsdirectory, aligning with the recommended file structure.- Efficiently managing global state with the
useContexthook.The component also optimally uses TypeScript by:
- Defining prop types and other type definitions to ensure type safety.
- Catching potential type-related issues at compile time.
66-72: Prepares the component for future functionality.The addition of the
connectedCaseParentIdprop to theIndictmentCaseFilesListcomponent and its usage in theuseFileListhook prepares the component for potential future functionality related to connected cases.Verify that the
useFileListhook correctly handles theconnectedCaseParentIdprop:Verification successful
Verification successful:
connectedCaseParentIdis correctly handledThe
useFileListhook inapps/judicial-system/web/src/utils/hooks/useFileList/index.tscorrectly handles theconnectedCaseParentIdprop:
- It's used as an optional parameter, maintaining backward compatibility.
- The hook uses it to determine the correct case ID for fetching files, supporting both regular and merged cases.
- It allows for fetching files from either the main case or the connected parent case.
The
IndictmentCaseFilesListcomponent correctly passes theconnectedCaseParentIdto theuseFileListhook, and the usage is consistent across multiple components (e.g.,PdfButton,ConnectedCaseFilesAccordionItem), indicating a well-integrated feature.This change successfully prepares the component for future functionality related to connected cases.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `useFileList` hook correctly handles the `connectedCaseParentId` prop. # Test: Search for the usage of `connectedCaseParentId` in the `useFileList` hook. Expect: The prop is used to fetch files related to the connected parent case. rg --type typescript -A 5 $'connectedCaseParentId' apps/judicial-system/web/src/utils/hooksLength of output: 125
Script:
#!/bin/bash # Description: Locate the useFileList hook and verify the usage of connectedCaseParentId # Step 1: Find the file containing the useFileList hook echo "Step 1: Locating useFileList hook" fd -e ts -e tsx useFileList apps/judicial-system/web/src # Step 2: Search for connectedCaseParentId in the found file(s) echo "Step 2: Searching for connectedCaseParentId usage" fd -e ts -e tsx useFileList apps/judicial-system/web/src | xargs rg -A 5 "connectedCaseParentId"Length of output: 13895
apps/judicial-system/backend/src/app/modules/case/filters/cases.filter.ts (3)
80-92: LGTM!The additional filter conditions enhance the specificity of the cases returned for public prosecution users by ensuring only cases with
FINEorRULINGdecisions andINDICTMENT_SENT_TO_PUBLIC_PROSECUTORevent type are included.Note: As mentioned in the comment, filtering out other event types should be acceptable for the public prosecutor's case list. However, it's worth verifying that the case list does not rely on any other event types to avoid unintended omissions.
206-206: Looks good!The simplification of the case type filter condition for cases in the
ACCEPTEDstate improves code readability while maintaining the intended functionality.
Line range hint
209-214: Looks good!The explicit checks for
indictment_ruling_decisionasRULINGandindictment_review_decisionasACCEPTprovide clarity on the specific criteria for completed indictment cases returned for prison admin users.Note: It's worth verifying that the additional conditions align with the intended behavior and do not exclude any necessary cases that may have different ruling or review decisions.
apps/judicial-system/backend/src/app/modules/case/filters/case.filter.ts (1)
91-111: Excellent work on enhancing the access control for public prosecution users!The additional checks for
indictmentRulingDecisionand the presence of theINDICTMENT_SENT_TO_PUBLIC_PROSECUTORevent log ensure that public prosecution users can only access cases that meet the necessary criteria. This strengthens the security and correctness of the access control mechanism.The changes adhere to the principle of least privilege by granting access only when the required conditions are met, preventing unauthorized access to sensitive case information.
apps/judicial-system/web/src/routes/Court/Indictments/Completed/Completed.tsx (5)
21-21: LGTM!The import statement looks good. It follows the correct naming convention and imports from the appropriate path.
27-27: LGTM!The import statement for the custom hook
useIndictmentsLawsBrokenlooks good. It follows the correct naming convention and imports from the appropriate path.
53-53: LGTM!The
lawsBrokenconstant is declared correctly using theuseIndictmentsLawsBrokenhook with theworkingCaseas an argument. The constant name is descriptive and conveys its purpose.
132-134: LGTM!The
hasLawsBrokenandhasMergeCasesconstants are declared correctly with descriptive names. The conditions used to determine their values are logically sound and will help in conditionally rendering UI elements.
153-168: LGTM!The conditional rendering block is implemented correctly. It uses the
hasLawsBrokenandhasMergeCasesconstants to determine whether to render theIndictmentsLawsBrokenAccordionItemandConnectedCaseFilesAccordionItemcomponents respectively. The rendering logic is clean and easy to understand.apps/judicial-system/api/src/app/modules/backend/backend.service.ts (2)
267-276: LGTM!The changes to
getCaseFileSignedUrlcorrectly handle the optionalmergedCaseIdparameter and construct the URL path accordingly. This enhances the API's capability to retrieve signed URLs for files associated with merged cases while maintaining the existing functionality for standard cases.The code adheres to NextJS best practices and efficiently uses TypeScript for type safety.
407-415: LGTM!The changes to
limitedAccessGetCaseFileSignedUrlcorrectly handle the optionalmergedCaseIdparameter and construct the URL path accordingly. This enhances the API's capability to retrieve signed URLs for files associated with merged cases in a limited access context while maintaining the existing functionality for standard cases.The code adheres to NextJS best practices and efficiently uses TypeScript for type safety.
apps/judicial-system/backend/src/app/modules/case/filters/test/cases.filter.spec.ts (5)
310-311: LGTM!The filter conditions correctly select only completed indictment cases for the public prosecutor role.
313-316: Looks good!The filter condition correctly selects only indictment cases with a ruling decision of either
FINEorRULINGfor the public prosecutor role.
319-320: Looks good!The filter condition correctly selects only indictment cases that have an event log of type
INDICTMENT_SENT_TO_PUBLIC_PROSECUTORfor the public prosecutor role.
381-381: LGTM!The filter condition correctly includes
CaseType.PAROLE_REVOCATIONin the list of restriction cases for the prison admin staff role.
384-384: Looks good!The filter conditions correctly select only completed indictment cases with a ruling decision and an accepted review decision for the prison admin staff role.
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts (11)
Line range hint
88-106: LGTM!The function is well-structured and uses appropriate guards and interceptors to ensure proper access control and data transformation. The logic for updating the case when opened by the defender is correct.
Line range hint
108-132: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. The logic for updating the
defendantStatementDateis correct.
Line range hint
134-178: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. The logic for updating the case state based on the provided transition is correct, including handling special cases for appeal-related transitions.
Line range hint
180-189: LGTM!The function is simple and uses an appropriate guard to ensure proper access control. It correctly delegates the actual retrieval to the
limitedAccessCaseService.
Line range hint
191-216: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly delegates the actual PDF generation to the
pdfService.
Line range hint
218-255: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly validates that the provided police case number is associated with the case and delegates the actual PDF generation to the
pdfService. The updated route definition allows handling requests for merged cases.
Line range hint
257-282: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly delegates the actual PDF generation to the
pdfService.
Line range hint
284-308: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly delegates the actual PDF generation to the
pdfService.
Line range hint
310-345: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly validates that the case is in the accepted state before generating the custody notice and delegates the actual PDF generation to the
pdfService.
Line range hint
347-410: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly delegates the actual PDF generation to the
pdfService. The updated route definition allows handling requests for merged cases.
Line range hint
412-440: LGTM!The function is well-structured and uses appropriate guards to ensure proper access control and validation. It correctly delegates the actual ZIP generation to the
limitedAccessCaseService.apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsx (3)
18-18: LGTM!The import statement for
ConnectedCaseFilesAccordionItemfollows the project's naming convention and is correctly referenced from the@island.is/judicial-system-web/src/componentsdirectory.
157-159: LGTM!The new variables
hasLawsBrokenandhasMergeCasesare correctly implemented to check for the existence of broken laws and merged cases, respectively. The variable names are descriptive and follow the project's naming convention.
216-229: LGTM!The conditional rendering logic for displaying the
IndictmentsLawsBrokenAccordionItemandConnectedCaseFilesAccordionItemcomponents based on thehasLawsBrokenandhasMergeCasesvariables is implemented correctly. Thekeyprop is properly set for each renderedConnectedCaseFilesAccordionItem, and the necessary props are passed correctly.apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (6)
173-173: LGTM!The additions to the
includearray enhance the data retrieval capabilities by fetchingindictmentCountsandmergedCasesalong with the case data. This aligns with the PR objective of improving data accessibility in merged cases.Also applies to: 223-252
Line range hint
288-380: Skipped review.This function is not directly related to the changes made in this PR. Reviewing it is not necessary.
Line range hint
382-400: Skipped review.This function is not directly related to the changes made in this PR. Reviewing it is not necessary.
Line range hint
402-434: Skipped review.This function is not directly related to the changes made in this PR. Reviewing it is not necessary.
Line range hint
436-467: Skipped review.This function is not directly related to the changes made in this PR. Reviewing it is not necessary.
Line range hint
469-516: Skipped review.This function is not directly related to the changes made in this PR. Reviewing it is not necessary.
apps/judicial-system/backend/src/app/modules/case/case.controller.ts (2)
563-566: LGTM!The changes to the
getCaseFilesRecordPdfmethod look good. The addition of theMergedCaseExistsGuardenhances the validation and security of the endpoint by ensuring that the provided merged case ID exists and is associated with the given case ID.
723-726: LGTM!The changes to the
getIndictmentPdfmethod look good. The addition of theMergedCaseExistsGuardenhances the validation and security of the endpoint by ensuring that the provided merged case ID exists and is associated with the given case ID.apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
323-326: LGTM!The addition of new case file categories to include for merged cases looks good. It aligns with the PR objective of enhancing data visibility for both parties in merged cases.
399-399: Looks good!Including
eventLogsin thelistOrderarray ensures that case list results are ordered considering the chronology of events. This can provide a useful view of case activities.
apps/judicial-system/backend/src/app/modules/case/guards/mergedCaseExists.guard.ts
Show resolved
Hide resolved
...omponents/AccordionItems/ConnectedCaseFilesAccordionItem/ConnectedCaseFilesAccordionItem.tsx
Outdated
Show resolved
Hide resolved
Good comments, I have addressed them all. |
* Fixes db queries * Shows merged cases to prosecutors and defenders * Only returns completed indictments that ended with a fine or a ruling and have been sent to the public prosecutor * Updates unit tests * Opens case files from merged cases * Opens generated indictments and case files records of merged cases * Updates unit tests * Removes console log * Fixes broken web build * Fixes accordion declarations and adds comments --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Merge Case Access
Sameina mál - Verjandi/sækjandi á að sjá gögn úr sameinuðum málum
What
Why
Screenshots / Gifs
Screen.Recording.2024-09-19.at.14.22.54.mov
Checklist:
Summary by CodeRabbit
Release Notes
New Features
modified,type, andsubmittedBy.MergedCaseExistsGuardfor improved access control on merged cases.Improvements
Bug Fixes