feat(j-s): Service Certificate to Court #17904
Merged
+334
−20
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
WalkthroughThis pull request introduces modifications to the service certificate PDF generation, delivery, and messaging workflow within the judicial system backend. The changes update the PDF formatting and defendant information presentation, add a method for sending messages related to indictment arraignment completion, implement new delivery endpoints in both controller and service layers, and extend the messaging enum and endpoint. New test suites also verify the functionality and guard configurations of the added methods. Changes
Sequence Diagram(s)sequenceDiagram
participant C as Client
participant ISC as InternalSubpoenaController
participant SS as SubpoenaService
participant PS as PdfService
participant CS as CourtService
C->>ISC: POST deliver service certificate
ISC->>SS: deliverServiceCertificateToCourt(params)
SS->>PS: getServiceCertificatePdf(case, defendant, subpoena)
PS-->>SS: PDF document
SS->>CS: createDocument(fileName, PDF)
CS-->>SS: Delivery result
SS-->>ISC: DeliverResponse
ISC-->>C: Response
sequenceDiagram
participant CSvc as CaseService
participant MQ as MessageQueue
participant D as Defendant
CSvc->>CSvc: update(case)
alt indictmentDecision added
CSvc->>CSvc: addMessagesForIndictmentArraignmentCompletionToQueue(case, user)
loop For each Defendant
CSvc->>D: check serviceStatus
alt Successful service (defender/electronic/in-person)
CSvc->>MQ: send DELIVERY_TO_COURT_SERVICE_CERTIFICATE message
end
end
end
Possibly related PRs
Suggested labels
Suggested reviewers
✨ Finishing Touches
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
|
…r-service-certificate-to-court
|
View your CI Pipeline Execution ↗ for commit 466d59d.
☁️ Nx Cloud last updated this comment at |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourt.spec.ts (1)
79-112: Add test cases for error scenarios.While error handling is set up in the mocks, there are no test cases verifying the behavior when PDF generation or document creation fails.
Consider adding these test cases:
describe('service certificate delivered to court', () => { // ... existing test ... }) + +describe('when PDF generation fails', () => { + let then: Then + + beforeEach(async () => { + const mockGetServiceCertificatePdf = mockPdfService.getServiceCertificatePdf as jest.Mock + mockGetServiceCertificatePdf.mockRejectedValue(new Error('PDF generation failed')) + + then = await givenWhenThen() + }) + + it('should propagate the error', () => { + expect(then.error).toBeInstanceOf(Error) + expect(then.error.message).toBe('PDF generation failed') + }) +}) + +describe('when court document creation fails', () => { + let then: Then + + beforeEach(async () => { + const mockGetServiceCertificatePdf = mockPdfService.getServiceCertificatePdf as jest.Mock + mockGetServiceCertificatePdf.mockResolvedValue('pdf-content') + const mockCreateDocument = mockCourtService.createDocument as jest.Mock + mockCreateDocument.mockRejectedValue(new Error('Document creation failed')) + + then = await givenWhenThen() + }) + + it('should propagate the error', () => { + expect(then.error).toBeInstanceOf(Error) + expect(then.error.message).toBe('Document creation failed') + }) +})apps/judicial-system/backend/src/app/formatters/serviceCertificatePdf.ts (1)
142-149: Extract indentation logic into a helper function.The comment suggests this indentation logic should be moved to a helper function for reusability and consistency.
Consider adding this helper:
+const addIndentedText = ( + doc: PDFKit.PDFDocument, + text: string, + labelWidth: number, + options?: { newLine?: boolean } +) => { + doc.text(text, 50 + labelWidth, doc.y + 5) + if (options?.newLine) { + doc.text('', 50) + } +} + // Consider adding indentation to helper functions -doc.text( - ` ${defendant.address ?? 'Ekki skráð'}`, - 50 + doc.widthOfString(defendantLabel), - doc.y + 5, -) -doc.text('', 50) +addIndentedText(doc, ` ${defendant.address ?? 'Ekki skráð'}`, doc.widthOfString(defendantLabel), { newLine: true })
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (7)
apps/judicial-system/backend/src/app/formatters/serviceCertificatePdf.ts(8 hunks)apps/judicial-system/backend/src/app/modules/case/case.service.ts(4 hunks)apps/judicial-system/backend/src/app/modules/subpoena/internalSubpoena.controller.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourt.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourtGuards.spec.ts(1 hunks)libs/judicial-system/message/src/lib/message.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
`apps/**/*`: "Confirm that the code adheres to the following...
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/deliverServiceCertificateToCourt.spec.tsapps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.tsapps/judicial-system/backend/src/app/modules/subpoena/internalSubpoena.controller.tsapps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourtGuards.spec.tsapps/judicial-system/backend/src/app/formatters/serviceCertificatePdf.tsapps/judicial-system/backend/src/app/modules/case/case.service.ts
`libs/**/*`: "Confirm that the code adheres to the following...
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."
libs/judicial-system/message/src/lib/message.ts
🧠 Learnings (2)
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/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourtGuards.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: 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`.
🪛 Biome (1.9.4)
apps/judicial-system/backend/src/app/formatters/serviceCertificatePdf.ts
[error] 214-215: Useless case clause.
because the default clause is present:
Unsafe fix: Remove the useless case.
(lint/complexity/noUselessSwitchCase)
⏰ Context from checks skipped due to timeout of 90000ms (5)
- GitHub Check: linting (judicial-system-backend,judicial-system-message,judicial-system-message-handler,judicial...
- GitHub Check: formatting
- GitHub Check: build (judicial-system-message-handler,judicial-system-scheduler)
- GitHub Check: tests (judicial-system-message,judicial-system-message-handler,judicial-system-scheduler)
- GitHub Check: tests (judicial-system-backend)
🔇 Additional comments (6)
apps/judicial-system/backend/src/app/modules/subpoena/test/internalSubpoenaController/deliverServiceCertificateToCourtGuards.spec.ts (1)
8-29: LGTM! The guard configuration test is thorough and well-structured.The test effectively verifies both the presence and order of all required guards, ensuring proper access control and data validation.
libs/judicial-system/message/src/lib/message.ts (1)
15-15: LGTM! The message type and endpoint are properly defined.The new message type and endpoint follow the established naming pattern and correctly map to the controller method.
Also applies to: 58-58
apps/judicial-system/backend/src/app/modules/subpoena/internalSubpoena.controller.ts (1)
165-199: LGTM! Implementation follows established patterns.The new endpoint for delivering service certificates to court is well-implemented with:
- Proper authorization guards
- Consistent parameter handling
- Standard response type
- Appropriate debug logging
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (1)
434-467: LGTM! Robust error handling and consistent implementation.The service certificate delivery implementation:
- Follows established error handling patterns
- Uses consistent file naming
- Provides appropriate logging
apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
1404-1432: LGTM! Correctly handles service certificate message generation.The method properly:
- Filters for successfully served subpoenas
- Uses appropriate message type for service certificate delivery
- Follows message queue patterns
1645-1651: LGTM! Appropriate trigger for service certificate delivery.The changes correctly trigger service certificate delivery when:
- An indictment decision is newly set
- The arraignment is completed
apps/judicial-system/backend/src/app/formatters/serviceCertificatePdf.ts
Show resolved
Hide resolved
oddsson
reviewed
Feb 14, 2025
oddsson
approved these changes
Feb 14, 2025
…:island-is/island.is into j-s/deliver-service-certificate-to-court
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
1363-1363: Simplify the subpoena comparison logic.The subpoena comparison can be simplified using optional chaining and nullish coalescing.
- )?.subpoenas?.[0]?.id !== updatedDefendant.subpoenas?.[0]?.id, // Only deliver new subpoenas + )?.subpoenas?.[0]?.id ?? null !== updatedDefendant.subpoenas?.[0]?.id ?? null, // Only deliver new subpoenas
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
apps/judicial-system/backend/src/app/modules/case/case.service.ts(4 hunks)apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
`apps/**/*`: "Confirm that the code adheres to the following...
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.tsapps/judicial-system/backend/src/app/modules/case/case.service.ts
🔇 Additional comments (3)
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (1)
435-468: LGTM! Well-structured implementation of service certificate delivery.The implementation follows the established patterns in the codebase, with proper error handling and logging.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
1413-1420: Consider creating a helper function for service status check.The service status check logic could be extracted into a helper function for better readability and reusability.
+ private isSuccessfulServiceStatus(serviceStatus: ServiceStatus | undefined): boolean { + return Boolean(serviceStatus && [ + ServiceStatus.DEFENDER, + ServiceStatus.ELECTRONICALLY, + ServiceStatus.IN_PERSON, + ].includes(serviceStatus)) + } + private addMessagesForIndictmentArraignmentCompletionToQueue( theCase: Case, user: TUser, ): Promise<void> { const messages: Message[] = [] theCase.defendants?.forEach((defendant) => { const subpoena = defendant.subpoenas?.[0] - const hasSubpoenaBeenSuccessfullyServedToDefendant = - subpoena?.serviceStatus && - [ - ServiceStatus.DEFENDER, - ServiceStatus.ELECTRONICALLY, - ServiceStatus.IN_PERSON, - ].includes(subpoena.serviceStatus) + const hasSubpoenaBeenSuccessfullyServedToDefendant = + this.isSuccessfulServiceStatus(subpoena?.serviceStatus)
1646-1652: LGTM! Clear and concise indictment decision check.The condition for triggering the service certificate delivery is clear and follows the pattern of other message queue triggers in the codebase.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Service Certificate to Court
Skila birtingavottorði í Auði + bæta við verjanda upplýsingum
What
Why
Screenshots / Gifs
Checklist:
Summary by CodeRabbit