feat(j-s): Deliver Subpoena to Court #17239
Conversation
WalkthroughThe pull request introduces several changes across multiple files in the judicial system application. Key modifications include the restructuring of message handling for subpoenas, the addition of new guards in the controller methods, and enhancements to the test suites to ensure comprehensive coverage of the updated functionalities. New methods for delivering subpoenas to court are added, alongside updates to existing service methods and the introduction of new enumerations in the messaging system, facilitating improved case and document management. 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
|
| @@ -1369,7 +1380,10 @@ export class CaseService { | |||
| await this.addMessagesForCourtCaseConnectionToQueue(updatedCase, user) | |||
| } | |||
| } else { | |||
| if (updatedCase.prosecutorId !== theCase.prosecutorId) { | |||
| if ( | |||
| !isIndictment && | |||
There was a problem hiding this comment.
This is not part of the current task. I just noticed this a fixed it. The prosecutor is deliverd to the district court in a different way for indictment cases.
| @UseGuards( | ||
| CaseExistsGuard, | ||
| new CaseTypeGuard([...restrictionCases, ...investigationCases]), | ||
| ) |
There was a problem hiding this comment.
This is not part of the current task. I just noticed this a fixed it. The prosecutor is deliverd to the district court in a different way for indictment cases.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (6)
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToPoliceGuards.spec.ts (1)
8-17: Consider using a more specific type for guards array.While the test setup follows the accepted pattern, we could improve type safety by replacing
any[]with a more specific type.- // eslint-disable-next-line @typescript-eslint/no-explicit-any - let guards: any[] + let guards: Array<new () => unknown>apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.spec.ts (1)
1-1: Consider importing specific case types.Instead of importing all
indictmentCases, consider importing only the specific case types needed for this feature.-import { indictmentCases } from '@island.is/judicial-system/types' +import { CaseType } from '@island.is/judicial-system/types'apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToPolice.spec.ts (3)
77-77: Complete the test implementation.The TODO comment indicates incomplete test coverage. Please implement the remaining test cases for indictment generation.
Would you like me to help implement the remaining test cases?
43-54: Improve promise chain readability.The current promise chain with assignments in expressions can be refactored for better readability using async/await:
- await internalSubpoenaController - .deliverSubpoenaToPolice( - caseId, - defendantId, - subpoenaId, - theCase, - defendant, - subpoena, - dto, - ) - .then((result) => (then.result = result)) - .catch((error) => (then.error = error)) + try { + then.result = await internalSubpoenaController.deliverSubpoenaToPolice( + caseId, + defendantId, + subpoenaId, + theCase, + defendant, + subpoena, + dto, + ); + } catch (error) { + then.error = error; + }🧰 Tools
🪛 Biome (1.9.4)
[error] 53-53: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 54-54: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
24-27: Consider using test data factories.Instead of inline test data creation, consider implementing test data factories for consistent test data across the test suite:
// testDataFactories.ts export const createTestCase = (overrides = {}) => ({ id: uuid(), defendants: [], ...overrides }); export const createTestUser = (overrides = {}) => ({ id: uuid(), ...overrides });apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.spec.ts (1)
78-110: Add test cases for edge cases.Consider adding test cases for:
- Invalid case type scenarios
- Missing court ID
- Missing court case number
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
apps/judicial-system/backend/src/app/modules/case/case.service.ts(2 hunks)apps/judicial-system/backend/src/app/modules/case/internalCase.controller.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/test/caseController/update.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverProsecutorToCourtGuards.spec.ts(2 hunks)apps/judicial-system/backend/src/app/modules/court/court.service.ts(2 hunks)apps/judicial-system/backend/src/app/modules/subpoena/internalSubpoena.controller.ts(3 hunks)apps/judicial-system/backend/src/app/modules/subpoena/subpoena.module.ts(2 hunks)apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts(3 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/createTestingSubpoenaModule.ts(5 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToPoliceGuards.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToPolice.spec.ts(2 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/updateSubpoeanaGuards.spec.ts(1 hunks)libs/judicial-system/message/src/lib/message.ts(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/updateSubpoeanaGuards.spec.ts
🧰 Additional context used
📓 Path-based instructions (14)
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.module.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."
libs/judicial-system/message/src/lib/message.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverProsecutorToCourtGuards.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/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.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/subpoena/test/internalSubpoenaController/deliverSubpoanaToPoliceGuards.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/subpoena/subpoena.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/subpoena/internalSubpoena.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/subpoena/test/createTestingSubpoenaModule.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/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.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/subpoena/test/internalSubpoenaController/deliverSubpoenaToPolice.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/court/court.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/update.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/internalCase.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."
📓 Learnings (5)
apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverProsecutorToCourtGuards.spec.ts (2)
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.spec.ts:24-25
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In certain scenarios within the judicial-system backend, the `RolesGuard` may intentionally follow the `CaseExistsGuard` when specific roles rules require the guard order to be reversed, as seen in tests like `getIndictmentPdfGuards.spec.ts`.
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts:175-185
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In the Jest tests for the `LimitedAccessViewCaseFileGuard` in `apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts`, code duplication in the `beforeEach` blocks is acceptable and should remain unchanged.
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.spec.ts (3)
Learnt from: oddsson
PR: island-is/island.is#16463
File: apps/judicial-system/backend/src/app/modules/subpoena/test/limitedAccessSubpoenaController/limitedAccessSubpoenaControllerGuards.spec.ts:17-17
Timestamp: 2024-11-12T15:15:20.158Z
Learning: In `LimitedAccessSubpoenaController`, `SubpoenaExistsOptionalGuard` is still used on specific endpoints not covered by the test file `limitedAccessSubpoenaControllerGuards.spec.ts`.
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.spec.ts:24-25
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In certain scenarios within the judicial-system backend, the `RolesGuard` may intentionally follow the `CaseExistsGuard` when specific roles rules require the guard order to be reversed, as seen in tests like `getIndictmentPdfGuards.spec.ts`.
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts:175-185
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In the Jest tests for the `LimitedAccessViewCaseFileGuard` in `apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts`, code duplication in the `beforeEach` blocks is acceptable and should remain unchanged.
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToPoliceGuards.spec.ts (3)
Learnt from: oddsson
PR: island-is/island.is#16463
File: apps/judicial-system/backend/src/app/modules/subpoena/test/limitedAccessSubpoenaController/limitedAccessSubpoenaControllerGuards.spec.ts:17-17
Timestamp: 2024-11-12T15:15:20.158Z
Learning: In `LimitedAccessSubpoenaController`, `SubpoenaExistsOptionalGuard` is still used on specific endpoints not covered by the test file `limitedAccessSubpoenaControllerGuards.spec.ts`.
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getIndictmentPdfGuards.spec.ts:24-25
Timestamp: 2024-11-12T15:15:11.835Z
Learning: In certain scenarios within the judicial-system backend, the `RolesGuard` may intentionally follow the `CaseExistsGuard` when specific roles rules require the guard order to be reversed, as seen in tests like `getIndictmentPdfGuards.spec.ts`.
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts:175-185
Timestamp: 2024-11-12T15:15:20.157Z
Learning: In the Jest tests for the `LimitedAccessViewCaseFileGuard` in `apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts`, code duplication in the `beforeEach` blocks is acceptable and should remain unchanged.
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (1)
Learnt from: unakb
PR: island-is/island.is#16393
File: apps/judicial-system/digital-mailbox-api/src/app/modules/cases/models/subpoena.response.ts:164-169
Timestamp: 2024-11-12T15:15:11.835Z
Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
Learnt from: unakb
PR: island-is/island.is#16393
File: apps/judicial-system/digital-mailbox-api/src/app/modules/cases/models/subpoena.response.ts:164-169
Timestamp: 2024-11-12T15:15:11.835Z
Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.
🪛 Biome (1.9.4)
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.spec.ts
[error] 71-71: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToPolice.spec.ts
[error] 53-53: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 54-54: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (21)
apps/judicial-system/backend/src/app/modules/subpoena/test/createTestingSubpoenaModule.ts (5)
16-16: LGTM: Import statement follows established patterns
The CourtService import is correctly placed and follows the same pattern as other service imports.
36-36: LGTM: Mock declaration is consistent
The court service mock follows the established pattern and is correctly placed with other service mocks.
56-56: LGTM: Provider registration is appropriate
CourtService is correctly registered as a provider, maintaining consistency with other service registrations.
100-101: LGTM: Service retrieval is well-implemented
The CourtService instance is correctly retrieved using the established pattern, with proper type annotations.
126-126: Verify the usage of exposed courtService in tests
While the courtService is correctly added to the return object, we should ensure it's being utilized in the test files.
✅ Verification successful
courtService is properly utilized in tests
The courtService is correctly exposed in the testing module and is actively used in the deliverSubpoenaToCourt.spec.ts test file. The service is destructured from the testing module and used to mock the createDocument method for testing error scenarios.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if courtService is used in test files
# Search for courtService usage in test files
rg -l "courtService" "apps/judicial-system/backend/src/app/modules/subpoena/**/*.spec.ts"
Length of output: 196
Script:
#!/bin/bash
# Let's first verify if the test files exist and their correct location
fd "\.spec\.ts$" apps/judicial-system/backend/src/app/modules/subpoena/
# Then search for courtService usage in all test files within the module
rg -l "courtService" apps/judicial-system/backend/src/app/modules/subpoena/
Length of output: 2154
Script:
#!/bin/bash
# Let's check how courtService is used in the test file
rg "courtService" -B 2 -A 2 apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.spec.ts
Length of output: 532
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToPoliceGuards.spec.ts (3)
1-6: LGTM! Imports are well-organized.
The imports follow TypeScript best practices with a clear separation between external and internal dependencies.
19-28: LGTM! Guard configuration is well-tested.
The test thoroughly verifies both the presence and correct configuration of all required guards.
Since this PR implements subpoena delivery to court, let's verify if similar tests exist for the court delivery endpoint:
✅ Verification successful
Guard configuration tests are consistent across both police and court delivery endpoints
The verification confirms that similar guard tests exist for both endpoints with identical guard configurations:
- Both test files verify 4 guards in the same order
- Both endpoints use CaseExistsGuard → CaseTypeGuard → DefendantExistsGuard → SubpoenaExistsGuard
- Both have the same configuration for CaseTypeGuard with
indictmentCases
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for court delivery guard tests
rg -l "Deliver.*[Ss]ubpoena.*[Cc]ourt.*guards" --type ts
Length of output: 181
Script:
#!/bin/bash
# Check the content of the court delivery guard test file
cat apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.spec.ts
Length of output: 1213
1-29: Verify coverage of PR objectives.
While this test file is well-implemented, it focuses on police delivery guards. Since the PR's main objective is implementing court delivery functionality, ensure that equivalent test coverage exists for the court delivery feature.
Let's verify the implementation of the court delivery feature:
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (2)
34-34: Importing CourtService and CourtDocumentFolder
The import statement correctly adds CourtService and CourtDocumentFolder, which are essential for integrating court document functionalities into the SubpoenaService.
97-97: Injecting CourtService into SubpoenaService
Adding CourtService to the constructor allows SubpoenaService to interact with court-related services, adhering to dependency injection best practices in NestJS.
apps/judicial-system/backend/src/app/modules/case/test/caseController/update.spec.ts (1)
900-917: Verify messages are correctly queued for subpoena delivery
The test correctly verifies that messages for both DELIVERY_TO_POLICE_SUBPOENA and DELIVERY_TO_COURT_SUBPOENA are sent for each defendant when the arraignment date is updated. This ensures that both the police and the court are notified about the subpoenas, aligning with the intended functionality.
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoanaToCourtGuards.spec.ts (1)
8-29: LGTM! Guard configuration is well-structured and comprehensive.
The test suite properly verifies:
- Correct number and order of guards
- Proper guard types and configurations
- Case type restrictions for indictment cases
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.module.ts (1)
27-27: Verify circular dependency handling for CourtModule.
Other module imports use forwardRef() to handle circular dependencies. Consider whether CourtModule needs similar treatment:
- CourtModule,
+ forwardRef(() => CourtModule),libs/judicial-system/message/src/lib/message.ts (1)
14-14: LGTM! Message type and endpoint mapping added correctly.
The new message type DELIVERY_TO_COURT_SUBPOENA and its corresponding endpoint mapping follow the established naming conventions and maintain proper ordering.
Also applies to: 52-52
apps/judicial-system/backend/src/app/modules/subpoena/internalSubpoena.controller.ts (2)
70-70: LGTM! Improved clarity in documentation and logging.
The description and logging message have been enhanced to be more precise.
Also applies to: 82-82
93-127: LGTM! Court delivery implementation follows established patterns.
The implementation:
- Uses appropriate guards for security
- Follows consistent parameter patterns
- Implements proper logging
- Maintains error handling through service layer
Let's verify the service method exists:
✅ Verification successful
LGTM! The service implementation is complete and properly handles court delivery.
The verification confirms that:
- The
deliverSubpoenaToCourtmethod exists in the service with the correct signature - The implementation follows proper error handling with try-catch
- It uses the PDF service to generate the document
- It properly integrates with the court service for document delivery
- It returns the expected
DeliverResponsetype
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify the service method implementation exists
ast-grep --pattern 'deliverSubpoenaToCourt(theCase: $_, defendant: $_, subpoena: $_, user: $_)'
Length of output: 97
Script:
#!/bin/bash
# Let's try a broader search for the method name
rg "deliverSubpoenaToCourt" -A 5
Length of output: 4407
Script:
#!/bin/bash
# Search for the service class definition
ast-grep --pattern 'class SubpoenaService {
$$$
}'
Length of output: 36017
Script:
#!/bin/bash
# Look for files that might contain the service implementation
fd "subpoena.service" --type f
Length of output: 104
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverSubpoenaToCourt.spec.ts (1)
91-109: LGTM! Test verifies core functionality.
The test properly verifies:
- PDF generation
- Document creation with correct parameters
- Success response
apps/judicial-system/backend/src/app/modules/court/court.service.ts (1)
37-37: LGTM: Enum addition for subpoena documents
The new enum value SUBPOENA_DOCUMENTS with Icelandic value 'Boðanir' is appropriately added to support subpoena document categorization in the court system.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (3)
1258-1280: LGTM: Enhanced subpoena message handling
The changes properly implement dual message generation for subpoenas, ensuring both police and court systems are notified. The flat array structure improves message queue processing.
1383-1386: LGTM: Refined prosecutor change condition
The condition correctly checks for non-indictment cases before processing prosecutor changes, preventing unnecessary message generation.
1269-1270: Verify message type usage across the codebase
Let's confirm that the DELIVERY_TO_COURT_SUBPOENA message type is properly defined and handled.
✅ Verification successful
Message type implementation is properly integrated
The verification shows that DELIVERY_TO_COURT_SUBPOENA is:
- Correctly defined in the message types enum
- Has a corresponding endpoint mapping (
deliverSubpoenaToCourt) - Is properly used in the case service and controller
- Has test coverage in the update specs
The implementation appears to be complete and consistent across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the message type definition and handler implementation
# Check for message type definition
echo "Checking message type definition..."
rg "DELIVERY_TO_COURT_SUBPOENA.*=.*" -A 2
# Check for message handler implementation
echo "Checking message handler implementation..."
rg "case.*MessageType\.DELIVERY_TO_COURT_SUBPOENA:" -A 5
Length of output: 623
Script:
#!/bin/bash
# Let's check for any message handling related to DELIVERY_TO_COURT_SUBPOENA
echo "Checking message handling implementation..."
rg "DELIVERY_TO_COURT_SUBPOENA" -B 2 -A 5
# Check the message template or content definition
echo "Checking message template/content..."
rg "template.*DELIVERY_TO_COURT_SUBPOENA" -A 3
# Check for any notification or message sending logic
echo "Checking notification logic..."
ast-grep --pattern 'case MessageType.DELIVERY_TO_COURT_SUBPOENA: {
$$$
}'
Length of output: 5688
| async deliverSubpoenaToCourt( | ||
| theCase: Case, | ||
| defendant: Defendant, | ||
| subpoena: Subpoena, | ||
| user: TUser, | ||
| ): Promise<DeliverResponse> { | ||
| return this.pdfService | ||
| .getSubpoenaPdf(theCase, defendant, subpoena) | ||
| .then(async (pdf) => { | ||
| const fileName = `Fyrirkall - ${defendant.name}` | ||
|
|
||
| return this.courtService.createDocument( | ||
| user, | ||
| theCase.id, | ||
| theCase.courtId, | ||
| theCase.courtCaseNumber, | ||
| CourtDocumentFolder.SUBPOENA_DOCUMENTS, | ||
| fileName, | ||
| `${fileName}.pdf`, | ||
| 'application/pdf', | ||
| pdf, | ||
| ) | ||
| }) | ||
| .then(() => ({ delivered: true })) | ||
| .catch((reason) => { | ||
| // Tolerate failure, but log error | ||
| this.logger.warn( | ||
| `Failed to upload subpoena ${subpoena.id} pdf to court for defendant ${defendant.id} of case ${theCase.id}`, | ||
| { reason }, | ||
| ) | ||
|
|
||
| return { delivered: false } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor to use async/await for improved readability
The deliverSubpoenaToCourt method uses promise chaining with .then() and .catch() inside an async function, which can be refactored using async/await for better readability and maintainability. This aligns with modern TypeScript best practices.
Apply this diff to refactor the method:
async deliverSubpoenaToCourt(
theCase: Case,
defendant: Defendant,
subpoena: Subpoena,
user: TUser,
): Promise<DeliverResponse> {
- return this.pdfService
- .getSubpoenaPdf(theCase, defendant, subpoena)
- .then(async (pdf) => {
- const fileName = `Fyrirkall - ${defendant.name}`
-
- return this.courtService.createDocument(
- user,
- theCase.id,
- theCase.courtId,
- theCase.courtCaseNumber,
- CourtDocumentFolder.SUBPOENA_DOCUMENTS,
- fileName,
- `${fileName}.pdf`,
- 'application/pdf',
- pdf,
- )
- })
- .then(() => ({ delivered: true }))
- .catch((reason) => {
- // Tolerate failure, but log error
- this.logger.warn(
- `Failed to upload subpoena ${subpoena.id} pdf to court for defendant ${defendant.id} of case ${theCase.id}`,
- { reason },
- )
-
- return { delivered: false }
- })
+ try {
+ const pdf = await this.pdfService.getSubpoenaPdf(theCase, defendant, subpoena)
+ const fileName = `Fyrirkall - ${defendant.name}`
+
+ await this.courtService.createDocument(
+ user,
+ theCase.id,
+ theCase.courtId,
+ theCase.courtCaseNumber,
+ CourtDocumentFolder.SUBPOENA_DOCUMENTS,
+ fileName,
+ `${fileName}.pdf`,
+ 'application/pdf',
+ pdf,
+ )
+ return { delivered: true }
+ } catch (reason) {
+ // Tolerate failure, but log error
+ this.logger.warn(
+ `Failed to upload subpoena ${subpoena.id} pdf to court for defendant ${defendant.id} of case ${theCase.id}`,
+ { reason },
+ )
+ return { delivered: false }
+ }
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| async deliverSubpoenaToCourt( | |
| theCase: Case, | |
| defendant: Defendant, | |
| subpoena: Subpoena, | |
| user: TUser, | |
| ): Promise<DeliverResponse> { | |
| return this.pdfService | |
| .getSubpoenaPdf(theCase, defendant, subpoena) | |
| .then(async (pdf) => { | |
| const fileName = `Fyrirkall - ${defendant.name}` | |
| return this.courtService.createDocument( | |
| user, | |
| theCase.id, | |
| theCase.courtId, | |
| theCase.courtCaseNumber, | |
| CourtDocumentFolder.SUBPOENA_DOCUMENTS, | |
| fileName, | |
| `${fileName}.pdf`, | |
| 'application/pdf', | |
| pdf, | |
| ) | |
| }) | |
| .then(() => ({ delivered: true })) | |
| .catch((reason) => { | |
| // Tolerate failure, but log error | |
| this.logger.warn( | |
| `Failed to upload subpoena ${subpoena.id} pdf to court for defendant ${defendant.id} of case ${theCase.id}`, | |
| { reason }, | |
| ) | |
| return { delivered: false } | |
| }) | |
| } | |
| async deliverSubpoenaToCourt( | |
| theCase: Case, | |
| defendant: Defendant, | |
| subpoena: Subpoena, | |
| user: TUser, | |
| ): Promise<DeliverResponse> { | |
| try { | |
| const pdf = await this.pdfService.getSubpoenaPdf(theCase, defendant, subpoena) | |
| const fileName = `Fyrirkall - ${defendant.name}` | |
| await this.courtService.createDocument( | |
| user, | |
| theCase.id, | |
| theCase.courtId, | |
| theCase.courtCaseNumber, | |
| CourtDocumentFolder.SUBPOENA_DOCUMENTS, | |
| fileName, | |
| `${fileName}.pdf`, | |
| 'application/pdf', | |
| pdf, | |
| ) | |
| return { delivered: true } | |
| } catch (reason) { | |
| // Tolerate failure, but log error | |
| this.logger.warn( | |
| `Failed to upload subpoena ${subpoena.id} pdf to court for defendant ${defendant.id} of case ${theCase.id}`, | |
| { reason }, | |
| ) | |
| return { delivered: false } | |
| } | |
| } |
| @UseGuards( | ||
| CaseExistsGuard, | ||
| new CaseTypeGuard([...restrictionCases, ...investigationCases]), | ||
| ) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid instantiating guards directly in @UseGuards decorator
Instantiating CaseTypeGuard directly within the @UseGuards decorator can interfere with NestJS's dependency injection and lifecycle management. Instead, you should use the class reference and pass parameters using a custom decorator or metadata.
Consider refactoring as follows:
- Create a custom decorator to apply the guard with parameters:
// case-type.decorator.ts
import { SetMetadata } from '@nestjs/common';
import { CaseTypeGuard } from '../guards/caseType.guard';
export const ALLOWED_CASE_TYPES_KEY = 'allowedCaseTypes';
export const CaseType = (...allowedCaseTypes: string[]) => SetMetadata(ALLOWED_CASE_TYPES_KEY, allowedCaseTypes);- Update the guard to retrieve the metadata:
// caseType.guard.ts
import { CanActivate, ExecutionContext, Injectable } from '@nestjs/common';
import { Reflector } from '@nestjs/core';
import { ALLOWED_CASE_TYPES_KEY } from '../decorators/case-type.decorator';
@Injectable()
export class CaseTypeGuard implements CanActivate {
constructor(private reflector: Reflector) {}
canActivate(context: ExecutionContext): boolean {
const allowedCaseTypes = this.reflector.get<string[]>(ALLOWED_CASE_TYPES_KEY, context.getHandler());
// ... guard logic using allowedCaseTypes
}
}- Apply the decorator in your controller:
@UseGuards(CaseExistsGuard, CaseTypeGuard)
@CaseType(...restrictionCases, ...investigationCases)This approach keeps the guard injection consistent and leverages NestJS features for passing parameters.
| it('should have the right guard configuration', () => { | ||
| expect(guards).toHaveLength(2) | ||
| expect(new guards[0]()).toBeInstanceOf(CaseExistsGuard) | ||
| expect(guards[1]).toBeInstanceOf(CaseTypeGuard) | ||
| expect(guards[1]).toEqual({ | ||
| allowedCaseTypes: [...restrictionCases, ...investigationCases], |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Ensure proper instantiation when testing guard configuration
In the test case, comparing guards[1] directly to an object may not accurately verify the guard's configuration. Since guards[1] is a class, instantiate it to check its properties and ensure accurate testing.
Update the test as follows:
it('should have the right guard configuration', () => {
expect(guards).toHaveLength(2)
expect(new guards[0]()).toBeInstanceOf(CaseExistsGuard)
- expect(guards[1]).toBeInstanceOf(CaseTypeGuard)
- expect(guards[1]).toEqual({
- allowedCaseTypes: [...restrictionCases, ...investigationCases],
- })
+ const caseTypeGuard = new guards[1]()
+ expect(caseTypeGuard).toBeInstanceOf(CaseTypeGuard)
+ expect(caseTypeGuard.allowedCaseTypes).toEqual([...restrictionCases, ...investigationCases])
})This modification ensures that you're testing the actual instance of the guard and its allowedCaseTypes property effectively.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| it('should have the right guard configuration', () => { | |
| expect(guards).toHaveLength(2) | |
| expect(new guards[0]()).toBeInstanceOf(CaseExistsGuard) | |
| expect(guards[1]).toBeInstanceOf(CaseTypeGuard) | |
| expect(guards[1]).toEqual({ | |
| allowedCaseTypes: [...restrictionCases, ...investigationCases], | |
| it('should have the right guard configuration', () => { | |
| expect(guards).toHaveLength(2) | |
| expect(new guards[0]()).toBeInstanceOf(CaseExistsGuard) | |
| const caseTypeGuard = new guards[1]() | |
| expect(caseTypeGuard).toBeInstanceOf(CaseTypeGuard) | |
| expect(caseTypeGuard.allowedCaseTypes).toEqual([...restrictionCases, ...investigationCases]) |
| .then((result) => (then.result = result)) | ||
| .catch((error) => (then.error = error)) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Refactor assignments in then/catch blocks.
The assignments in expressions can be confusing and are flagged by static analysis. Consider refactoring to improve readability.
- .then((result) => (then.result = result))
- .catch((error) => (then.error = error))
+ .then((result) => {
+ then.result = result;
+ })
+ .catch((error) => {
+ then.error = error;
+ })📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| .then((result) => (then.result = result)) | |
| .catch((error) => (then.error = error)) | |
| .then((result) => { | |
| then.result = result; | |
| }) | |
| .catch((error) => { | |
| then.error = error; | |
| }) |
🧰 Tools
🪛 Biome (1.9.4)
[error] 71-71: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
[error] 72-72: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.
(lint/suspicious/noAssignInExpressions)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17239 +/- ##
==========================================
- Coverage 35.76% 35.74% -0.03%
==========================================
Files 6932 6932
Lines 147982 148289 +307
Branches 42171 42301 +130
==========================================
+ Hits 52931 53002 +71
- Misses 95051 95287 +236
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 ReportAll test runs ✅ 4 Total Test Services: 0 Failed, 4 Passed Test Services
|
Deliver Subpoena to Court
Skila staðfestu fyrirkalli í Auði
What
Why
Checklist:
Summary by CodeRabbit
Release Notes
New Features
Enhancements
Bug Fixes
Tests