fix(j-s): Subpoena Notifications #16556
Conversation
WalkthroughThe pull request introduces significant changes to the judicial system's backend, particularly focusing on the handling of subpoenas and notifications. Key modifications include renaming and restructuring methods in the Changes
Possibly related PRs
Suggested labels
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
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 20
🧹 Outside diff range and nitpick comments (35)
apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts (2)
5-6: Improve type safety by removinganyInstead of using
any[], we can type the guards array more specifically.- // eslint-disable-next-line @typescript-eslint/no-explicit-any - let guards: any[] + let guards: (new () => SubpoenaExistsOptionalGuard)[]
15-18: Consider adding more test casesWhile the basic guard configuration is tested, consider adding tests for:
- Error scenarios when metadata is missing
- Verification that other endpoints don't use this guard
- Guard execution order if combined with other guards
This would provide better coverage and documentation of the guard's behavior.
Would you like me to help generate these additional test cases?
apps/judicial-system/backend/src/app/modules/notification/notification.module.ts (1)
Line range hint
29-51: Well-structured module configurationThe module structure follows best practices with:
- Clear separation of concerns using distinct notification services
- Proper use of forwardRef() for circular dependencies
- Organized imports and provider registration
Consider documenting the responsibility of each notification service in a README.md to help maintain the clear separation of concerns.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendHeadsUpNotifications.spec.ts (1)
Line range hint
52-67: Consider adding test cases for combined notification scenarios.Given that one of the PR objectives is to fix scenarios where "both the service status and the defender choice were altered in the same call", consider adding test cases to verify this specific fix.
Would you like me to help generate additional test cases for these scenarios?
libs/judicial-system/auth/src/lib/guards/roles.guard.ts (1)
Line range hint
13-39: Consider adding security audit loggingGiven the critical nature of role-based access control, especially in the context of subpoena notifications, consider adding debug-level logging for security audit purposes. This would help track unauthorized access attempts and role validation failures.
canActivate(context: ExecutionContext): boolean { const rolesRules = this.reflector.get<RolesRule[]>( 'roles-rules', context.getHandler(), ) // Deny if no rules if (!rolesRules) { + console.debug('Access denied: No roles rules defined for handler') return false } const request = context.switchToHttp().getRequest() const user: User = request.user // Deny if no user if (!user) { + console.debug('Access denied: No user found in request') return false } // Pick the first matching rule const rule = rolesRules.find((rule) => typeof rule === 'string' ? rule === user.role : rule?.role === user.role, ) // Deny if no rule matches the user's role if (!rule) { + console.debug(`Access denied: No matching rule found for role ${user.role}`) return false }apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverConclusionToCourtOfAppeals.spec.ts (1)
36-39: Consider adding edge cases to test notification handling.While the current test case covers the basic scenario, consider adding tests for:
- Multiple notifications of different types
- Empty notifications array
- Invalid notification types
This would ensure robust handling of various notification scenarios, especially given the PR's focus on fixing notification-related issues.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentDeniedNotifications.spec.ts (1)
65-65: Consider enhancing test coverage for notification types.While the current test correctly uses the new
CaseNotificationType.INDICTMENT_DENIED, consider adding test cases for error scenarios and edge cases.Example test cases to consider:
- Invalid notification type
- Missing required notification data
- Multiple notifications in quick succession
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendCourtDateNotification.spec.ts (1)
Line range hint
1-91: Consider adding test cases for notification fixesWhile the existing tests cover the basic notification flow, consider adding test cases for the specific issues mentioned in the PR objectives:
- Test for preventing excessive notifications
- Test for the scenario where both service status and defender choice are altered simultaneously
Would you like me to help generate additional test cases for these scenarios?
libs/judicial-system/types/src/index.ts (1)
14-20: Consider adding JSDoc comments for the new notification types.Since this is a shared library used across different NextJS apps, adding documentation for the new notification types would improve maintainability and help other developers understand when to use each type.
Example:
/** * Types for different notification scenarios in the judicial system * @export */ export { /** Notifications related to case status changes */ CaseNotificationType, /** Notifications specific to subpoena operations */ SubpoenaNotificationType, /** Base notification type */ NotificationType, /** Notifications for institution-level events */ InstitutionNotificationType, /** Types defining how notifications should be dispatched */ NotificationDispatchType, } from './lib/notification'apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentReturnedNotifications.spec.ts (1)
66-66: Consider adding edge case tests.While the basic notification test is good, consider adding test cases for:
- Multiple police case numbers
- Missing prosecutor email
- Missing court name
Example test structure:
describe('edge cases', () => { it('should handle multiple police case numbers in email subject', () => { // Test with multiple case numbers }); it('should handle missing prosecutor email gracefully', () => { // Test with undefined email }); });apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.spec.ts (1)
78-85: Consider extracting expected message body to improve test readabilityWhile the implementation is correct, the test readability could be improved by extracting the expected message body into a helper constant.
Consider refactoring like this:
+ const createExpectedNotification = (prosecutorsOfficeId: string) => ({ + type: MessageType.NOTIFICATION, + body: { + type: CaseNotificationType.INDICTMENTS_WAITING_FOR_CONFIRMATION, + prosecutorsOfficeId, + }, + }); expect(mockMessageService.sendMessagesToQueue).toHaveBeenCalledWith([ - { - type: MessageType.NOTIFICATION, - body: { - type: CaseNotificationType.INDICTMENTS_WAITING_FOR_CONFIRMATION, - prosecutorsOfficeId: prosecutorsOfficeId1, - }, - }, - { - type: MessageType.NOTIFICATION, - body: { - type: CaseNotificationType.INDICTMENTS_WAITING_FOR_CONFIRMATION, - prosecutorsOfficeId: prosecutorsOfficeId2, - }, - }, + createExpectedNotification(prosecutorsOfficeId1), + createExpectedNotification(prosecutorsOfficeId2), ])apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/update.spec.ts (1)
Based on the search results and the original test file context, I can now provide a final response:
Additional test coverage needed for notification scenarios
The current test suite only covers basic notification scenarios. While there is one test case for preventing duplicate notifications (
only send notification once to defender), it's specific to defender assignment notifications. The test suite should be expanded to include:
- Test cases verifying that excessive notifications are prevented when multiple updates occur in rapid succession
- Test cases ensuring correct notification behavior when both service status and defender choice change simultaneously
- Test cases validating the deduplication logic for all notification types, not just defender assignments
Example test structure:
describe('multiple simultaneous updates', () => { it('should send only one notification when status and defender change together') it('should deduplicate notifications for rapid successive updates') })🔗 Analysis chain
Line range hint
31-121: Consider adding tests for new notification scenarios.Based on the PR objectives, consider adding test cases for:
- Verifying that excessive notifications are prevented
- Ensuring notifications are properly sent when both service status and defender choice are changed simultaneously
Let's check if these scenarios are tested elsewhere:
Let me try a different search strategy to verify if these notification scenarios are tested elsewhere.
Let me try one more search to specifically look for test cases related to preventing duplicate notifications and handling multiple changes:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related test cases in the codebase rg -t ts "describe.*notification.*service.*status.*defender" apps/judicial-system/backendLength of output: 91
Script:
#!/bin/bash # Search for notification-related test cases more broadly rg -t ts "describe.*notification" apps/judicial-system/backend/src -A 5 # Also search for test files containing both status and defender-related tests rg -t ts "describe.*('|\")(service status|defender)" apps/judicial-system/backend/src -A 5 # Look for MessageService usage in tests rg -t ts "MessageService.*notification" apps/judicial-system/backend/src/app/modules/case/test -A 5Length of output: 75618
Script:
#!/bin/bash # Search for test cases related to preventing duplicate notifications rg -t ts "describe.*prevent.*notification|describe.*duplicate.*notification" apps/judicial-system/backend/src # Look for test cases handling multiple changes rg -t ts "describe.*multiple.*changes|describe.*simultaneous.*changes" apps/judicial-system/backend/src # Search for test files that might handle notification deduplication rg -t ts "only send notification once" apps/judicial-system/backend/srcLength of output: 489
apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.ts (1)
89-94: Consider enhancing error handling for invalid notification typesThe type system changes look good, but we could improve the error handling by being more specific about which notification types are supported.
Consider this enhancement:
case InstitutionNotificationType.INDICTMENTS_WAITING_FOR_CONFIRMATION: await this.sendIndictmentsWaitingForConfirmationNotification( prosecutorsOfficeId, ) break default: - throw new InternalServerErrorException( - `Invalid notification type ${type}`, - ) + throw new InternalServerErrorException( + `Unsupported institution notification type: ${type}. Supported types: ${Object.values(InstitutionNotificationType).join(', ')}`, + )apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (1)
42-48: Consider adding field descriptions for better documentation.To improve schema documentation and maintainability, consider adding descriptions to the new fields using GraphQL's description syntax.
Example enhancement:
subpoenas { id created - serviceStatus - serviceDate - servedBy - comment - defenderNationalId - caseId - subpoenaId + serviceStatus # Current status of the subpoena service + serviceDate # Date when the subpoena was served + servedBy # Identity of the person who served the subpoena + comment # Additional notes about the service + defenderNationalId # National ID of the assigned defender + caseId # Reference to the parent case + subpoenaId # Unique identifier for the subpoena }apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts (1)
120-121: Consider documenting error scenarios.The changes correctly enforce subpoena existence validation and make the subpoena parameter required. This aligns well with the PR's goal of fixing service certificate PDF access.
Consider adding API documentation for error scenarios using
@ApiResponsedecorators:@ApiResponse({ status: 404, description: 'Subpoena not found' }) @ApiResponse({ status: 403, description: 'Insufficient permissions to access the subpoena' })Also applies to: 134-134
apps/judicial-system/backend/src/app/modules/notification/baseNotification.service.ts (1)
Line range hint
1-165: Consider implementing a notification type registry pattern.Given the critical nature of judicial system notifications and the ongoing refactoring effort, consider implementing a more robust notification type system using a registry pattern. This would provide:
- Centralized management of notification types
- Runtime validation of notification types
- Type-safe notification handling across the application
- Easy extension for new notification types
Example implementation:
// notificationRegistry.ts export class NotificationRegistry { private static registry = new Map<string, NotificationConfig>(); static register(type: string, config: NotificationConfig) { this.registry.set(type, config); } static isValidType(type: string): type is ValidNotificationType { return this.registry.has(type); } }apps/judicial-system/backend/src/app/formatters/subpoenaPdf.ts (2)
Line range hint
89-93: Consider improving error handling and internationalization.The hardcoded fallback messages should use translation strings for consistency:
'Ekki skráð'should use a translation key- The comment "the latter shouldn't happen" suggests a potential data integrity issue that should be handled more robustly
Consider applying these improvements:
if (theCase.court?.name) { addNormalText( doc, - theCase.court.address || 'Ekki skráð', // the latter shouldn't happen, if it does we have an problem with the court data + theCase.court.address || formatMessage(strings.addressNotRegistered), 'Times-Roman', ) }Also, consider adding validation for court data integrity at the service layer to prevent this edge case.
Line range hint
77-85: Consider adding PDF size limits for security.While the buffer handling is secure, consider adding a maximum size limit for the generated PDFs to prevent potential memory issues with very large documents.
Consider adding a size check:
const doc = new PDFDocument({ size: 'A4', margins: { top: 40, bottom: 60, left: 50, right: 50, }, bufferPages: true, + size: { + maxPages: 100 // Prevent extremely large documents + } })apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealJudgesAssignedNotifications.spec.ts (1)
86-86: LGTM! Consider environment-specific URLs in tests.While the notification type change is correct, the test uses hardcoded localhost URLs in email templates. Consider using environment-specific base URLs to make tests more maintainable across different environments.
Example improvement:
// Define at the top of the test file const BASE_URL = process.env.BASE_URL || 'http://localhost:4200'; // Use in template strings `${BASE_URL}/landsrettur/yfirlit/${caseId}`apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealFilesUpdatedNotifications.spec.ts (1)
Line range hint
97-143: Consider adding negative test cases.While the current tests thoroughly verify successful notification delivery, consider adding test cases for:
- Invalid recipient data (e.g., missing email)
- Failed email delivery scenarios
- Missing or invalid case numbers
This would ensure the notification system handles error cases gracefully.
Example test case structure:
describe('notification failures', () => { it('should handle missing recipient email gracefully', async () => { // Test setup with invalid email const invalidJudge = { ...appealJudge1, email: undefined }; // Verify error handling const then = await givenWhenThen(); expect(then.error).toBeDefined(); expect(mockEmailService.sendEmail).not.toHaveBeenCalled(); }); });apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCaseFilesUpdatedNotifications.spec.ts (1)
Line range hint
95-97: Consider enhancing URL validation in tests.While the tests verify the email content and URLs, consider extracting the base URL (
http://localhost:4200) and role-specific paths into constants. This would make the tests more maintainable and less prone to errors when URLs change.const BASE_URL = 'http://localhost:4200'; const URL_PATHS = { JUDGE: '/domur/akaera/yfirlit', DEFENDER: '/verjandi/akaera', PROSECUTOR: '/akaera/stadfesta' }; // Usage in test expect(mockEmailService.sendEmail).toHaveBeenCalledWith( expect.objectContaining({ html: expect.stringContaining(`${BASE_URL}${URL_PATHS.JUDGE}/${caseId}`), }), );Also applies to: 134-136
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCourtDateNotifications.spec.ts (1)
Based on the search results from all scripts executed, I can now provide a final assessment:
Missing test coverage for service status and defender choice notifications
The codebase shows that SubpoenaNotificationType enum includes three notification types:
- SERVICE_SUCCESSFUL
- SERVICE_FAILED
- DEFENDANT_SELECTED_DEFENDER
However, no test files were found covering these notifications, despite the functionality being implemented in:
- subpoenaNotification.service.ts
- subpoena.service.ts
The lack of test coverage for these specific notifications means:
- Service status notifications (both successful and failed) are untested
- Defender choice notification scenarios are untested
🔗 Analysis chain
Line range hint
1-224: Verify test coverage for notification scenariosWhile the existing tests cover basic court date notifications, defender notifications, and indictment scenarios, we should verify if they adequately test the specific issues mentioned in the PR objectives:
- Excessive notifications
- Missing defender choice notification when both service status and defender choice are changed
Let's check for related test coverage:
Let me try a different search strategy to verify the test coverage for notifications.
This should help us understand:
- What notification types exist in the system
- Which notification test files are present
- The actual test coverage in those files
Let me try a different search strategy to find notification-related test coverage.
This should help us find:
- The SubpoenaNotificationType enum definition and usage
- Any test files related to defender choice and service status notifications
Let me try one more search to specifically look for test files related to SubpoenaNotificationType.
This should help us find any test files specifically covering the SubpoenaNotificationType notifications, which include service status and defender choice notifications.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for test files related to defender choice notifications fd -e ts -e spec.ts -e test.ts | xargs rg "defender.*choice.*notification|service.*status.*notification"Length of output: 106
Script:
#!/bin/bash # Search for notification types and test files echo "=== Searching for notification type definitions ===" rg -A 5 "enum.*NotificationType|CaseNotificationType" echo -e "\n=== Searching for notification test files ===" fd notification.*spec -e ts echo -e "\n=== Looking for notification test cases ===" fd notification.*spec -e ts -x cat {}Length of output: 367722
Script:
#!/bin/bash # Search for SubpoenaNotificationType and test files echo "=== Searching for SubpoenaNotificationType usage ===" rg "SubpoenaNotificationType" -A 5 echo -e "\n=== Searching for notification test files related to defender choice and service status ===" fd -e ts | rg -l "defender.*choice|service.*status"Length of output: 11673
Script:
#!/bin/bash # Search for test files containing SubpoenaNotificationType tests echo "=== Searching for SubpoenaNotification test files ===" fd "subpoena.*notification.*spec" -e ts echo -e "\n=== Looking at test content ===" fd "subpoena.*notification.*spec" -e ts -x cat {}Length of output: 291
apps/judicial-system/backend/src/app/modules/case/test/caseController/getRulingSignatureConfirmation.spec.ts (1)
207-207: Consider adding error scenario tests for notification handling.While the notification type change is correct, consider adding test cases to verify error handling when notifications fail to send, especially for extended LÖKE cases which may have different notification requirements.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealCompletedNotifications.spec.ts (1)
Line range hint
108-109: Consider using environment-specific URLs consistently.The test uses inconsistent base URLs in the email content:
http://localhost:4200in some placeshttps://rettarvorslugatt.island.isin othersThis could cause issues when running tests in different environments.
Consider using a configuration value for the base URL:
- html: `Landsréttur hefur úrskurðað í máli ${appealCaseNumber} (héraðsdómsmál nr. ${courtCaseNumber}). Niðurstaða Landsréttar: Staðfest. Hægt er að nálgast gögn málsins á <a href="http://localhost:4200/krafa/yfirlit/${caseId}">yfirlitssíðu málsins í Réttarvörslugátt</a>.`, + html: `Landsréttur hefur úrskurðað í máli ${appealCaseNumber} (héraðsdómsmál nr. ${courtCaseNumber}). Niðurstaða Landsréttar: Staðfest. Hægt er að nálgast gögn málsins á <a href="${mockConfig.baseUrl}/krafa/yfirlit/${caseId}">yfirlitssíðu málsins í Réttarvörslugátt</a>.`,Also applies to: 186-187
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
Line range hint
23-58: Consider using feature flags for temporary removals.Instead of commenting out code, consider implementing a feature flag system. This would:
- Make it easier to enable/disable features in different environments
- Provide better control over the rollout of incomplete features
- Keep the codebase cleaner without commented sections
Would you like assistance in implementing a feature flag system for this functionality?
apps/judicial-system/web/src/routes/Court/Indictments/Subpoena/Subpoena.tsx (1)
107-110: LGTM! Consider adding type safety for indictmentDecision.The added dependencies are necessary and correctly implemented. They ensure that the callback updates when the court's decision or arraignment schedule changes, fixing the notification issues mentioned in the PR objectives.
Consider adding type safety for
workingCase.indictmentDecisionby creating a type guard:const hasIndictmentDecision = ( workingCase: WorkingCase ): workingCase is WorkingCase & { indictmentDecision: NonNullable<typeof workingCase.indictmentDecision> } => { return workingCase.indictmentDecision !== null && workingCase.indictmentDecision !== undefined; };Then use it in the condition:
const clearedConclusion = isArraignmentScheduled && hasIndictmentDecision(workingCase) ? [{ indictmentDecision: null, // ... rest of the properties }] : undefined;apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts (1)
Line range hint
46-54: Consider refining notification logic to prevent excessive notificationsGiven that one of the PR's objectives is to fix excessive notifications, consider the following improvements to the notification sending logic:
- Add role-based notification filtering to ensure notifications are sent only to relevant users
- Consider consolidating multiple notifications when multiple changes occur simultaneously
- Add rate limiting or debouncing for notifications to prevent notification spam
For example, in the
updatemethod, when bothnationalIdanddefenderEmailchanges occur, two separate notifications are sent. These could potentially be combined.Here's a suggested approach for the
getMessageForSendDefendantsNotUpdatedAtCourtNotificationmethod:private getMessageForSendDefendantsNotUpdatedAtCourtNotification( theCase: Case, user: User, ): Message { + // Determine the target user based on the sender's role + const targetUser = user.role === 'defender' ? getProsecutorForCase(theCase) : user; return { type: MessageType.NOTIFICATION, - user, + user: targetUser, caseId: theCase.id, body: { type: CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURT }, } }Also applies to: 156-173, 251-262, 317-328
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts (1)
Line range hint
31-324: Consider extracting test data to fixturesThe test file contains multiple test cases with similar case data structures. Consider extracting these to shared fixtures to improve maintainability and reduce duplication. This would make it easier to maintain consistent test data across the test suite.
Example structure:
// fixtures/cases.ts export const createTestCase = (overrides: Partial<Case> = {}): Case => ({ id: uuid(), type: CaseType.CUSTODY, state: CaseState.ACCEPTED, courtCaseNumber: '007-2022-07', court: { name: 'Héraðsdómur Reykjavíkur' }, ...overrides })apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendReadyForCourtNotifications.spec.ts (1)
Line range hint
1-398: Consider adding high-level test documentationWhile the test implementation is solid, consider adding a brief JSDoc comment at the file level to document:
- The purpose of these notification tests
- The different notification scenarios covered
- The relationship with the notification type refactoring
Example addition at the file start:
/** * Tests for the Ready for Court notification functionality. * Covers various notification scenarios: * - Initial court notifications * - Subsequent modifications * - Defender notifications * - Indictment-specific notifications * * These tests validate the notification behavior using the CaseNotificationType * system for improved type safety and notification categorization. */apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
Line range hint
507-507: Consider implementing parallel file fetchingThe TODO comment suggests an opportunity for performance optimization. Consider using
Promise.all()to fetch files in parallel:- // TODO: speed this up by fetching all files in parallel - for (const file of caseFilesByCategory) { - await this.awsS3Service - .getObject(theCase.type, file.key) - .then((content) => filesToZip.push({ data: content, name: file.name })) - .catch((reason) => - // Tolerate failure, but log what happened - this.logger.warn( - `Could not get file ${file.id} of case ${file.caseId} from AWS S3`, - { reason }, - ), - ) - } + await Promise.all( + caseFilesByCategory.map((file) => + this.awsS3Service + .getObject(theCase.type, file.key) + .then((content) => filesToZip.push({ data: content, name: file.name })) + .catch((reason) => + // Tolerate failure, but log what happened + this.logger.warn( + `Could not get file ${file.id} of case ${file.caseId} from AWS S3`, + { reason }, + ), + ), + ), + )apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts (1)
156-156: Enhance error logging with additional contextConsider including more details in the error log at line 156 to aid in debugging. Adding the notification type and case ID provides valuable context.
Update the error logging as follows:
- this.logger.error('Failed to send notification', error) + this.logger.error(`Failed to send notification of type ${type} for case ${theCase.id}`, { error })apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (3)
17-21: Ensure consistent import statement formattingThe import statements have been modified. Consider organizing imports by grouping external and internal modules separately and alphabetically sorting them to improve readability.
188-189: Typographical error in commentThere is a typo in the comment: 'for now we want to prentend that it's ok.' It should be 'pretend'.
Apply this diff to correct the typo:
- // for now we want to prentend that it's ok. + // for now we want to pretend that it's okay.
230-231: Typographical error in commentSame typo as above: 'for now we want to prentend that it's ok.' Please correct 'prentend' to 'pretend'.
Apply this diff:
- // for now we want to prentend that it's ok. + // for now we want to pretend that it's okay.apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts (1)
2682-2685: Enhance error handling in the default case ofsendNotificationIn the default case of the
sendNotificationmethod, anInternalServerErrorExceptionis thrown when an invalid notification type is encountered. Consider providing a more descriptive error message to aid in debugging.Suggested change:
- throw new InternalServerErrorException( - `Invalid notification type ${type}`, - ) + throw new InternalServerErrorException( + `Invalid CaseNotificationType '${type}' encountered in sendNotification method.`, + )
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (72)
- apps/judicial-system/api/src/app/modules/file/file.controller.ts (1 hunks)
- apps/judicial-system/backend/src/app/formatters/subpoenaPdf.ts (1 hunks)
- apps/judicial-system/backend/src/app/messages/courtUpload.ts (1 hunks)
- apps/judicial-system/backend/src/app/messages/pdfCore.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/case.service.ts (15 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/caseController/getRulingSignatureConfirmation.spec.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/caseController/transition.spec.ts (14 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/caseController/update.spec.ts (5 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverConclusionToCourtOfAppeals.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/transition.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/update.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/defendant/test/defendantController/delete.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/defendant/test/defendantController/update.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/defendant/test/internalDefendantController/deliverDefendantToCourt.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/baseNotification.service.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts (33 hunks)
- apps/judicial-system/backend/src/app/modules/notification/dto/caseNotification.dto.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/dto/institutionNotification.dto.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/notification/dto/notification.dto.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/notification/dto/notificationDispatch.dto.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/notification/dto/subpoenaNotification.dto.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/notification/guards/rolesRules.ts (6 hunks)
- apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/notification.module.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/notification.service.ts (5 hunks)
- apps/judicial-system/backend/src/app/modules/notification/notificationDispatch.service.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealCompletedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealFilesUpdatedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealJudgesAssignedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealReceivedByCourtNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealStatementNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealToCourtOfAppealsNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealWithdrawnNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCaseFilesUpdatedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCourtDateNotifications.spec.ts (5 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendDefendantsNotUpdatedAtCourtNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendHeadsUpNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentDeniedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentReturnedNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendReadyForCourtNotifications.spec.ts (7 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRevokedNotifications.spec.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAdvocateAssignedNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAppealFilesUpdatedNotification.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAppealJudgesAssignedNotification.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendCaseFilesUpdatedNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendCourtDateNotification.spec.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendHeadsUpNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendReadyForCourtNotifications.spec.ts (4 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/index.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts (5 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (7 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/subpoenaControllerGuards.spec.ts (2 hunks)
- apps/judicial-system/message-handler/src/app/test/handleMessage.spec.ts (1 hunks)
- apps/judicial-system/scheduler/src/app/app.service.ts (2 hunks)
- apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (1 hunks)
- apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (2 hunks)
- apps/judicial-system/web/src/routes/Court/Indictments/Subpoena/Subpoena.tsx (1 hunks)
- apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (2 hunks)
- libs/judicial-system/auth/src/lib/guards/roles.guard.ts (1 hunks)
- libs/judicial-system/message/src/lib/message.ts (2 hunks)
- libs/judicial-system/types/src/index.ts (1 hunks)
- libs/judicial-system/types/src/lib/notification.ts (2 hunks)
✅ Files skipped from review due to trivial changes (1)
- apps/judicial-system/backend/src/app/messages/courtUpload.ts
🧰 Additional context used
📓 Path-based instructions (71)
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/backend/src/app/formatters/subpoenaPdf.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/messages/pdfCore.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/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/getRulingSignatureConfirmation.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/transition.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/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/test/internalCaseController/deliverConclusionToCourtOfAppeals.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/transition.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/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/defendant/defendant.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/defendant/test/defendantController/delete.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/defendant/test/defendantController/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/defendant/test/internalDefendantController/deliverDefendantToCourt.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/notification/baseNotification.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/notification/caseNotification.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/notification/dto/caseNotification.dto.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/notification/dto/institutionNotification.dto.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/notification/dto/notification.dto.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/notification/dto/notificationDispatch.dto.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/notification/dto/subpoenaNotification.dto.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/notification/guards/rolesRules.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/notification/institutionNotification.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/notification/internalNotification.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/notification/notification.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."
apps/judicial-system/backend/src/app/modules/notification/notification.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/notification/notificationDispatch.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/notification/subpoenaNotification.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/notification/test/createTestingNotificationModule.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/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.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/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.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/notification/test/internalNotificationController/sendAppealCompletedNotifications.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/notification/test/internalNotificationController/sendAppealFilesUpdatedNotifications.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/notification/test/internalNotificationController/sendAppealJudgesAssignedNotifications.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/notification/test/internalNotificationController/sendAppealReceivedByCourtNotifications.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/notification/test/internalNotificationController/sendAppealStatementNotifications.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/notification/test/internalNotificationController/sendAppealToCourtOfAppealsNotifications.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/notification/test/internalNotificationController/sendAppealWithdrawnNotifications.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/notification/test/internalNotificationController/sendCaseFilesUpdatedNotifications.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/notification/test/internalNotificationController/sendCourtDateNotifications.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/notification/test/internalNotificationController/sendDefendantsNotUpdatedAtCourtNotifications.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/notification/test/internalNotificationController/sendHeadsUpNotifications.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/notification/test/internalNotificationController/sendIndictmentDeniedNotifications.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/notification/test/internalNotificationController/sendIndictmentReturnedNotifications.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/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.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/notification/test/internalNotificationController/sendReadyForCourtNotifications.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/notification/test/internalNotificationController/sendRevokedNotifications.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/notification/test/internalNotificationController/sendRulingNotifications.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/notification/test/notificationController/sendAdvocateAssignedNotifications.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/notification/test/notificationController/sendAppealFilesUpdatedNotification.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/notification/test/notificationController/sendAppealJudgesAssignedNotification.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/notification/test/notificationController/sendCaseFilesUpdatedNotifications.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/notification/test/notificationController/sendCourtDateNotification.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/notification/test/notificationController/sendHeadsUpNotifications.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/notification/test/notificationController/sendReadyForCourtNotifications.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/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."
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.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/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/test/subpoenaController/getSubpoenaPdfGuards.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/subpoenaController/subpoenaControllerGuards.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/message-handler/src/app/test/handleMessage.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/scheduler/src/app/app.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/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/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/Subpoena/Subpoena.tsx (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
libs/judicial-system/auth/src/lib/guards/roles.guard.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."
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."
libs/judicial-system/types/src/index.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."
libs/judicial-system/types/src/lib/notification.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."
📓 Learnings (13)
apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/notification.module.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/notification.service.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts (3)
Learnt from: oddsson PR: island-is/island.is#16329 File: apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts:143-177 Timestamp: 2024-10-15T21:11:15.520Z Learning: In `apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts`, the method `sendEmails` currently uses `Promise.all` for multiple recipients, and changing it to `Promise.allSettled` is considered for a future iteration.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-10-14T21:34:16.234Z Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCaseFilesUpdatedNotifications.spec.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts (1)
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-10-21T21:14:47.668Z Learning: In `LimitedAccessSubpoenaController`, `SubpoenaExistsOptionalGuard` is still used on specific endpoints not covered by the test file `limitedAccessSubpoenaControllerGuards.spec.ts`.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-10-14T21:34:16.234Z 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/subpoenaController/getSubpoenaPdfGuards.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-10-21T21:14:47.668Z 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-10-17T11:41:45.565Z 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-10-17T11:53:19.983Z 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/subpoenaController/subpoenaControllerGuards.spec.ts (2)
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-10-21T21:14:47.668Z 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/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts:175-185 Timestamp: 2024-10-17T11:53:19.983Z 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/web/src/routes/Court/Indictments/Subpoena/Subpoena.tsx (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-10-14T21:34:16.234Z Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.
🔇 Additional comments (147)
apps/judicial-system/backend/src/app/modules/subpoena/index.ts (1)
3-3: LGTM! Verify guard implementation.The new export follows the module's established patterns and TypeScript best practices.
Let's verify the guard's implementation:
✅ Verification successful
The shell script results provide comprehensive information about the
SubpoenaExistsGuardimplementation and its usage across the codebase. Let me generate the final response:Guard implementation and usage verified - all good! ✅
The guard is properly implemented and used consistently across the codebase:
- Implements
CanActivateinterface with proper dependency injection ofSubpoenaService- Used in multiple controllers for subpoena-related endpoints:
limitedAccessSubpoena.controller.tsinternalSubpoena.controller.tssubpoena.controller.tsinternalNotification.controller.ts- Has a derived
SubpoenaExistsOptionalGuardfor optional validation scenariosThe new export in the barrel file is justified as the guard is widely used across the module and related components.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the SubpoenaExistsGuard implementation and its usage # Check the guard implementation echo "Checking guard implementation..." ast-grep --pattern 'class SubpoenaExistsGuard { $$$ }' # Check where the guard is being used echo "Checking guard usage..." rg -A 5 "SubpoenaExistsGuard" --type tsLength of output: 9307
apps/judicial-system/backend/src/app/modules/notification/dto/subpoenaNotification.dto.ts (3)
5-5: LGTM! Clean import of specialized notification type.The change to import
SubpoenaNotificationTypefrom the shared types package improves type safety and maintains better separation of concerns.
9-11: LGTM! Proper decorator usage with the new enum type.The decorators are correctly configured:
IsEnumensures runtime validationApiPropertymaintains accurate API documentation
Line range hint
1-13: Architectural improvement: Better type granularity.The refactoring from a generic
NotificationTypeto a specificSubpoenaNotificationTypealigns well with the Single Responsibility Principle and improves type safety. This change should help prevent the notification issues mentioned in the PR objectives.Let's verify that this change is consistent across the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information to verify the architectural change:
The refactoring to
SubpoenaNotificationTypeis consistent and well-implemented across the codebaseThe codebase shows a clear separation between different types of notifications:
SubpoenaNotificationTypeis used specifically for subpoena-related notifications (service success/failure, defender selection)CaseNotificationTypehandles case-specific notificationsInstitutionNotificationTypemanages institution-level notificationsThis granular approach improves type safety by preventing mixing of different notification domains, which aligns perfectly with the Single Responsibility Principle.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining generic NotificationType usage that might need updating # Expected: No instances of generic NotificationType in notification-related files # Search for any remaining NotificationType usage rg "NotificationType" --type ts # Search for consistent usage of SubpoenaNotificationType rg "SubpoenaNotificationType" --type tsLength of output: 56940
apps/judicial-system/backend/src/app/modules/notification/dto/notificationDispatch.dto.ts (2)
1-5: LGTM! Well-organized imports.The imports are properly organized following the pattern: external libraries, framework imports, and internal modules, with clear separation between them.
8-12: LGTM! Well-structured DTO property.The
typeproperty is well-implemented with:
- Proper validation using
@IsNotEmpty()and@IsEnum()- API documentation using
@ApiProperty- Type safety with readonly modifier and non-null assertion
apps/judicial-system/backend/src/app/modules/notification/dto/institutionNotification.dto.ts (2)
5-5: LGTM! Clean import of the new notification type.The import statement follows the proper convention and maintains good separation of concerns by importing types from a dedicated package.
9-11: LGTM! Proper enum validation and API documentation.The changes maintain proper type safety and API documentation while transitioning to the more specific
InstitutionNotificationType. The decorators ensure both runtime validation and accurate OpenAPI specification generation.Let's verify that this enum is used consistently across the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information to verify the usage of
InstitutionNotificationType.The enum validation and API documentation are correctly implemented and used consistently
The
InstitutionNotificationTypeenum is properly used across the codebase:
- It's defined in
libs/judicial-system/types/src/lib/notification.ts- It's correctly imported and used in the DTO with proper validation decorators
- The enum is used consistently in related services like
notificationDispatch.service.tsandinstitutionNotification.service.ts- No legacy or inconsistent usage patterns were found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for consistent usage of InstitutionNotificationType # and ensure no old NotificationType references remain # Search for any remaining references to the old NotificationType rg "NotificationType" -g "!*.{md,txt,json}" -g "!{CHANGELOG,README}*" # Search for usage of InstitutionNotificationType to verify consistent adoption rg "InstitutionNotificationType" -g "!*.{md,txt,json}" -g "!{CHANGELOG,README}*"Length of output: 55666
apps/judicial-system/backend/src/app/modules/notification/dto/notification.dto.ts (2)
5-5: LGTM! Clean import of domain-specific typeThe import of
CaseNotificationTypefrom the shared types package follows best practices by maintaining a clear separation of concerns and promoting type reusability across the judicial system.
9-11: LGTM! Consistent enum usage in decoratorsThe update to use
CaseNotificationTypeis properly reflected in both validation (@IsEnum) and API documentation (@ApiProperty) decorators, maintaining type safety and accurate API documentation.Let's verify the consistent usage of this enum across the codebase:
✅ Verification successful
Based on the search results, I can now provide the final response since I have enough information to verify the enum usage:
Verified: CaseNotificationType is consistently used across the codebase
The search results confirm that
CaseNotificationTypeis properly used throughout the codebase:
- The enum is defined in
libs/judicial-system/types/src/lib/notification.ts- It's consistently used in decorators for validation (
@IsEnum) and API documentation (@ApiProperty) across DTOs- All references to notification types in services, controllers, and tests use the
CaseNotificationTypeenum- No legacy or inconsistent enum usage was found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the migration from NotificationType to CaseNotificationType is complete # and there are no lingering references to the old type. # Check for any remaining references to NotificationType echo "Checking for any remaining references to NotificationType..." rg "NotificationType" -g "!*.{md,txt}" -g "!package.json" # Check the usage of CaseNotificationType to ensure consistent implementation echo "Checking usage of CaseNotificationType..." rg "CaseNotificationType" -g "!*.{md,txt}" -g "!package.json"Length of output: 90283
apps/judicial-system/backend/src/app/modules/notification/dto/caseNotification.dto.ts (2)
6-6: LGTM! Import change aligns with notification type refactoring.The update to use
CaseNotificationTypeis consistent with the PR's objective of refactoring notification types for better structure.
15-17: LGTM! Property decorators and type updated correctly.The changes maintain proper validation and API documentation while transitioning to the new notification type system.
Let's verify that all usages of this DTO have been updated to use the new type:
✅ Verification successful
Let me gather more specific information about the CaseNotificationDto usage with a refined search strategy.
This will help us verify if the DTO is being used correctly with the new type system by showing the actual instantiations and the enum definition.
Let me search specifically for the CaseNotificationType enum definition to complete our verification.
This will help us verify that the enum is properly defined and imported consistently across the codebase.
Based on the gathered information, I can now provide a final response:
The CaseNotificationDto implementation is correctly using the new type system
The verification shows that:
- The DTO is properly importing
CaseNotificationTypefrom@island.is/judicial-system/types- All test files and implementations consistently use the same imported type
- The usage patterns in test files demonstrate correct type usage (e.g.,
type: CaseNotificationType.COURT_DATE,type: CaseNotificationType.RULING, etc.)- The property decorators in the DTO are correctly applied for validation and API documentation
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to the old NotificationType # and verify CaseNotificationDto usage with new CaseNotificationType # Search for any remaining references to old NotificationType echo "Checking for old NotificationType references..." rg -l "NotificationType" "apps/judicial-system" # Search for CaseNotificationDto usage to verify correct type usage echo "Checking CaseNotificationDto usage..." ast-grep --pattern 'new CaseNotificationDto({ $$$type: $_$$$})'Length of output: 7725
Script:
#!/bin/bash # Search for CaseNotificationDto usage with context echo "Checking CaseNotificationDto instantiation and usage..." rg -A 3 "CaseNotificationDto" apps/judicial-system/backend # Check the CaseNotificationType enum definition echo -e "\nChecking CaseNotificationType definition..." rg -A 10 "enum CaseNotificationType" apps/judicial-system/backendLength of output: 22091
Script:
#!/bin/bash # Search for the enum definition file and its contents echo "Searching for CaseNotificationType enum definition..." rg --type ts -C 5 "export enum CaseNotificationType" apps/judicial-system/backend # Check imports of CaseNotificationType echo -e "\nChecking CaseNotificationType imports..." rg "import.*CaseNotificationType" apps/judicial-system/backendLength of output: 3090
apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts (1)
8-13: LGTM! Clean setup using metadata reflectionThe
beforeEachsetup correctly retrieves guard metadata using reflection, following established patterns in the codebase.apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/subpoenaControllerGuards.spec.ts (2)
5-5: LGTM! Clean import path using barrel exports.The simplified import path improves maintainability by using the module's barrel exports.
Line range hint
17-27: Verify the removal of SubpoenaExistsOptionalGuard.The guard count has been reduced from 7 to 6, removing the SubpoenaExistsOptionalGuard. Please ensure this removal aligns with the changes in the controller's endpoints.
Let's verify the guard usage in the controller:
apps/judicial-system/backend/src/app/messages/pdfCore.ts (1)
1-1: LGTM! Import consolidation improves dependency management.The consolidation of imports from
react-intlto@formatjs/intlis a good practice as it reduces duplicate dependencies and uses the core library directly.Let's verify this change is consistent across the codebase:
✅ Verification successful
Let me analyze the results from the shell script execution:
The codebase shows extensive usage of both
react-intland@formatjs/intl:
- Many files still import from
react-intl- Several files import from
@formatjs/intl@formatjs/intlis listed as a dependency in package.jsonThe change in
apps/judicial-system/backend/src/app/messages/pdfCore.tsis part of a backend service that generates PDFs, where using@formatjs/intldirectly makes sense since it's not a React environment.Import change is appropriate for this backend context
The change from
react-intlto@formatjs/intlin this specific file is correct because:
- It's in a backend service (
judicial-system/backend)- It's used for PDF generation, not React UI
@formatjs/intlis the core library that powersreact-intl- The package is already a dependency in the project
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining react-intl imports and @formatjs/intl usage # to ensure consistency across the codebase echo "Checking for remaining react-intl imports..." rg "from 'react-intl'" -g '*.ts' -g '*.tsx' echo "Checking for @formatjs/intl usage..." rg "from '@formatjs/intl'" -g '*.ts' -g '*.tsx' # Verify package.json includes @formatjs/intl echo "Checking package.json for @formatjs/intl dependency..." fd package.json --exec jq -r '.dependencies["@formatjs/intl"] // .devDependencies["@formatjs/intl"] // empty'Length of output: 151408
apps/judicial-system/backend/src/app/modules/notification/notification.module.ts (1)
Line range hint
20-26: Verify the notification service transitionWhile CaseNotificationService is added, InternalNotificationController is still present. Let's verify there are no lingering dependencies.
✅ Verification successful
Let me gather more specific information about the notification services and controllers to better understand the transition.
No issues with notification service transition - proper implementation confirmed
The verification shows that the transition from InternalNotificationService to CaseNotificationService is correctly implemented:
- The InternalNotificationController is properly using CaseNotificationService in its constructor and methods
- The notification.module.ts has all required services registered in its providers array
- There are no lingering references to an InternalNotificationService class
- The controller's implementation shows proper usage of the new CaseNotificationService for handling case notifications
The presence of InternalNotificationController is intentional as it serves as the routing layer, while the underlying service implementation has been successfully migrated to CaseNotificationService.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining references to InternalNotificationService # and verify CaseNotificationService usage # Check for InternalNotificationService references echo "Checking for InternalNotificationService references..." rg "InternalNotificationService" -l # Check CaseNotificationService usage echo "Checking CaseNotificationService usage..." ast-grep --pattern 'class $_ extends CaseNotificationService { $$$ }'Length of output: 319
Script:
#!/bin/bash # Check for InternalNotificationController usage and implementation echo "Checking InternalNotificationController implementation..." ast-grep --pattern 'class InternalNotificationController { $$$ }' # Check service injection in the controller echo "Checking service injection in InternalNotificationController..." ast-grep --pattern 'constructor($$$) { $$$ }' apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts # Check providers array in notification module echo "Checking notification module providers..." rg "providers.*\[" -A 10 apps/judicial-system/backend/src/app/modules/notification/notification.module.ts # Check for any internal notification related tests echo "Checking for internal notification tests..." fd "internal.*notification.*test" --type fLength of output: 12548
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendHeadsUpNotifications.spec.ts (1)
6-6: LGTM: Notification type refactoring looks good!The change from
NotificationTypetoCaseNotificationTypealigns with the broader refactoring effort to improve notification type categorization.Also applies to: 45-45
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendHeadsUpNotifications.spec.ts (2)
4-4: LGTM! Consistent enum refactoring across the file.The changes correctly implement the notification type refactoring from
NotificationTypetoCaseNotificationType, maintaining consistency across the import statement and its usage in both the test case and assertions.Also applies to: 40-40, 63-63
Line range hint
4-65: Verify test coverage for other notification types.While this test file correctly handles the HEADS_UP notification type, we should ensure comprehensive test coverage exists for all notification scenarios mentioned in the PR objectives, particularly the service status and defender choice notifications.
🧰 Tools
🪛 Biome
[error] 42-42: 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] 43-43: 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/notification/notificationDispatch.service.ts (4)
11-13: LGTM! Type system improvements enhance code clarity.The introduction of
InstitutionNotificationTypeand renaming toNotificationDispatchTypealigns well with the PR's objective of refactoring notification types for better structure.
34-38: LGTM! Type-safe message construction.The message construction properly utilizes the new
InstitutionNotificationTypeenum, maintaining type safety while improving code clarity.
Line range hint
56-67: LGTM! Robust error handling implementation.The error handling follows best practices with proper logging and consistent error responses.
45-50: Verify notification type usage across the codebase.While the method signature update to use
NotificationDispatchTypeis correct, we should verify that all callers have been updated accordingly.✅ Verification successful
Let me analyze the results and generate one more verification script to ensure we have all the necessary information.
Based on the comprehensive results from our verification scripts, I can now provide the final response:
All notification dispatch type usages are correctly aligned with the updated signature
The verification shows that:
- All calls to
dispatchNotificationare using theNotificationDispatchTypeenum- The DTO (
notificationDispatch.dto.ts) is properly decorated with@ApiProperty({ enum: NotificationDispatchType })- The enum is properly defined in
libs/judicial-system/types/src/lib/notification.ts- All usages across the codebase (in controller, service, tests, and scheduler) consistently use
NotificationDispatchType.INDICTMENTS_WAITING_FOR_CONFIRMATION🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Find all usages of dispatchNotification to ensure they use the new type # Expected: All calls should use NotificationDispatchType # Search for method calls rg -A 2 "dispatchNotification\(" # Search for NotificationDispatchType usage rg "NotificationDispatchType\."Length of output: 2344
Script:
#!/bin/bash # Let's verify the NotificationDispatchnDto type definition to ensure it's using NotificationDispatchType ast-grep --pattern 'interface NotificationDispatchnDto { $$$ }' # Also check for any other potential notification dispatch types or enums rg -g '*.ts' 'enum.*Notification.*Type'Length of output: 1897
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendCaseFilesUpdatedNotifications.spec.ts (1)
4-4: LGTM: Clean migration to CaseNotificationTypeThe changes consistently replace
NotificationTypewithCaseNotificationTypeacross imports and usages, maintaining the test's integrity while aligning with the notification type refactoring initiative.Also applies to: 40-40, 63-63
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAdvocateAssignedNotifications.spec.ts (2)
4-4: LGTM! Import change aligns with notification type refactoring.The change from
NotificationTypetoCaseNotificationTypeis consistent with the broader refactoring effort to improve notification type handling.
42-42: LGTM! Consistent usage of new notification type.The test correctly uses
CaseNotificationType.ADVOCATE_ASSIGNEDin both the notification setup and assertion, maintaining consistency with the refactored type system.Also applies to: 64-64
libs/judicial-system/auth/src/lib/guards/roles.guard.ts (2)
40-40: LGTM: Improved code readabilityThe added blank line enhances code readability by clearly separating different logical blocks.
Line range hint
1-85: Verify guard's behavior with subpoena notification changesSince this PR involves changes to subpoena notifications and service status updates, we should verify that the guard correctly handles access control for these specific scenarios.
✅ Verification successful
Let me gather more specific information about the role definitions and guards in the subpoena controllers to verify the access control behavior.
Let me try a different approach to verify the guard's behavior with subpoena notifications.
Let me check one more thing to verify the specific rules being applied to subpoena notifications.
Based on the results from our verification attempts, I can now provide a final response:
Guard implementation is appropriate for subpoena notification handling
The RolesGuard implementation is properly structured to handle access control for subpoena-related operations. From our analysis:
- The guard is used in both main subpoena controllers (
subpoena.controller.tsandlimitedAccessSubpoena.controller.ts)- Role definitions are clearly defined in the types (
UserRoleenum) with proper segregation between prosecution, court, and defence roles- The guard's implementation supports all necessary rule types (BASIC, FIELD, FIELD_VALUES) needed for fine-grained access control
The current implementation is sufficiently robust to handle the subpoena notification changes safely, as it:
- Properly validates user roles before allowing access
- Supports field-level access control when needed
- Has comprehensive role definitions for all relevant user types
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of RolesGuard in subpoena-related endpoints # and ensure proper access control for notification-related operations # Look for RolesGuard usage in subpoena-related files echo "Checking RolesGuard usage in subpoena-related endpoints..." rg -l "RolesGuard" | rg -i "subpoena" # Look for role definitions related to subpoena notifications echo "Checking role definitions for subpoena notifications..." ast-grep --pattern 'RolesRule[] = [ $$$ { role: $_, type: $_, $$$ } $$$ ]'Length of output: 795
Script:
#!/bin/bash # Check the role decorators and guard usage in subpoena controllers echo "Checking role decorators in subpoena controllers..." rg -A 5 "@UseGuards\(.*RolesGuard.*\)" apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts apps/judicial-system/backend/src/app/modules/subpoena/limitedAccessSubpoena.controller.ts echo "Checking role rules definitions..." rg "RolesRule.*=.*\[" -A 10 apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts apps/judicial-system/backend/src/app/modules/subpoena/limitedAccessSubpoena.controller.ts # Check for notification-related endpoints and their guards echo "Checking notification-related endpoints..." rg -B 2 -A 2 "notification" apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts apps/judicial-system/backend/src/app/modules/subpoena/limitedAccessSubpoena.controller.tsLength of output: 886
Script:
#!/bin/bash # Search for role-related decorators in the entire codebase for subpoena endpoints echo "Searching for role decorators in subpoena-related code..." rg -B 2 -A 5 "@Roles|@SetRoles" apps/judicial-system/backend/src/app/modules/subpoena/ # Look for notification-related types and interfaces echo "Checking notification-related types..." rg -l "notification" libs/judicial-system/types/src/ # Check for role-related constants or enums echo "Checking role definitions..." rg "enum.*Role|const.*Role" -A 5 libs/judicial-system/types/src/Length of output: 7766
Script:
#!/bin/bash # Check for rule definitions and their types echo "Checking rule definitions in subpoena module..." rg -B 2 -A 5 "const.*Rule.*:.*RolesRule" apps/judicial-system/backend/src/app/modules/subpoena/ # Look for notification-related endpoints and their rules echo "Checking notification-related endpoints and their rules..." rg -B 5 -A 2 "notification.*\(" apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts # Check for field-value rules that might be related to notifications echo "Checking for field-value rules..." rg "RulesType.FIELD_VALUES" -B 2 -A 5 apps/judicial-system/backend/src/app/modules/subpoena/Length of output: 610
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAppealJudgesAssignedNotification.spec.ts (2)
4-4: LGTM: Import statement updated correctlyThe change from
NotificationTypetoCaseNotificationTypealigns with the PR's goal of improving notification type specificity.
42-42: LGTM: Consistent usage of new notification typeThe test correctly uses
CaseNotificationType.APPEAL_JUDGES_ASSIGNEDin both the notification sending and message body verification, maintaining test integrity while adopting the new type system.Also applies to: 64-64
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendAppealFilesUpdatedNotification.spec.ts (2)
4-4: LGTM! Consistent usage of CaseNotificationType.The changes correctly implement the notification type refactoring while maintaining the test's effectiveness in validating the appeal files notification flow.
Also applies to: 42-42, 64-64
Line range hint
1-67: Verify test coverage for related notification scenarios.While this test covers the appeal files updated notification, let's verify the coverage of other notification scenarios affected by the type changes.
✅ Verification successful
Let me gather more specific information about the notification types to verify coverage.
Test coverage for notification types is comprehensive
After analyzing the CaseNotificationType enum and test coverage, I can confirm that:
- All notification types defined in the enum are covered by corresponding test files
- Both internal and external notification controllers have dedicated test files
- The APPEAL_CASE_FILES_UPDATED type (the focus of the original review) has proper test coverage in both controllers:
sendAppealFilesUpdatedNotification.spec.tssendAppealFilesUpdatedNotifications.spec.tsThe test structure follows a consistent pattern across all notification types, with proper verification of message queuing and response handling.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other notification-related test files and their coverage of CaseNotificationType scenarios # Search for test files related to notifications echo "Searching for notification-related test files:" fd "notification.*\.spec\.ts$" apps/judicial-system/backend/src # Check usage of CaseNotificationType in test files echo -e "\nChecking CaseNotificationType usage in tests:" rg -l "CaseNotificationType\." apps/judicial-system/backend/src --type ts # Look for potential uncovered notification types echo -e "\nChecking for all available notification types:" ast-grep --pattern 'export enum CaseNotificationType'Length of output: 9516
Script:
#!/bin/bash # Let's check the actual enum definition and compare with test coverage # Find the CaseNotificationType enum definition echo "CaseNotificationType enum definition:" rg "export enum CaseNotificationType" -A 30 --type ts # Check test coverage for each notification type echo -e "\nTest coverage for notification types:" rg "type: CaseNotificationType\." apps/judicial-system/backend/src/app/modules/notification/test --type tsLength of output: 12261
apps/judicial-system/scheduler/src/app/app.service.ts (1)
9-9: LGTM: Import change aligns with notification type refactoringThe change from
NotificationTypetoNotificationDispatchTypeis consistent with the broader refactoring of notification types across the codebase.apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverConclusionToCourtOfAppeals.spec.ts (1)
4-4: LGTM! Import change aligns with notification type refactoring.The change from NotificationType to CaseNotificationType is consistent with the PR's objective to refactor notification types.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentDeniedNotifications.spec.ts (1)
6-6: LGTM! Good refactor to more specific notification types.The change from generic
NotificationTypeto specificCaseNotificationTypeimproves type safety and better represents the domain model.apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendCourtDateNotification.spec.ts (4)
4-4: LGTM: Import statement updated correctlyThe import statement has been properly updated to use
CaseNotificationTypeinstead ofNotificationType, aligning with the notification type refactoring mentioned in the PR objectives.
40-42: LGTM: Notification type usage updated in test caseThe test correctly uses
CaseNotificationType.COURT_DATEin the notification controller call.
64-64: LGTM: Message queue test updatedThe message queue test assertion correctly uses
CaseNotificationType.COURT_DATEin the expected message body.
85-85: Verify advocate assignment notification behaviorThe test expects an
ADVOCATE_ASSIGNEDnotification wheneventOnlyis true. This seems to be testing an important edge case mentioned in the PR objectives about defender choice notifications.Let's verify this behavior is documented:
✅ Verification successful
Let me try a different search strategy to understand the advocate assignment notification behavior better.
Based on the search results, I can now provide a conclusive response about the advocate assignment notification behavior.
The advocate assignment notification test is correctly implemented
The test case is valid and aligns with the actual implementation in the codebase. Here's why:
- The
CaseNotificationType.ADVOCATE_ASSIGNEDis a well-defined notification type used throughout the system for advocate assignment notifications- The implementation in
caseNotification.service.tsshows dedicated methods for handling advocate assignments:
shouldSendAdvocateAssignedNotificationsendAdvocateAssignedNotificationsendAdvocateAssignedNotifications- The notification is used for both defenders and spokespersons, with proper email formatting through
formatAdvocateAssignedEmailNotificationWhile the
eventOnlyparameter isn't directly documented, the test is validating the core advocate assignment notification functionality which is thoroughly implemented in the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for documentation about the eventOnly parameter and its relationship with advocate assignments rg -A 3 "eventOnly.*ADVOCATE_ASSIGNED" --type tsLength of output: 50
Script:
#!/bin/bash # Search for ADVOCATE_ASSIGNED notification type usage and context rg -A 5 "CaseNotificationType.ADVOCATE_ASSIGNED" --type ts # Also search for any documentation or implementation of advocate assignment notifications rg -A 5 "advocate.*notification" --type ts -iLength of output: 21944
libs/judicial-system/types/src/index.ts (1)
14-20: LGTM! The notification types refactoring looks well-structured.The expanded export of notification types aligns with the PR objectives and provides better granularity for different notification scenarios (case, subpoena, institution).
Let's verify the usage of these notification types across the codebase:
✅ Verification successful
Let me analyze the results and generate a final response. The search results show:
- The new notification types (
CaseNotificationType,SubpoenaNotificationType,InstitutionNotificationType) are being used extensively across the codebase, particularly in backend services, DTOs, and test files.- The old
NotificationTypereferences are still present in many files, but looking at the paths, they appear to be using the new exported type (accessing it via dot notation) rather than the old type.- The changes are properly structured with dedicated services for each notification type (caseNotification.service.ts, subpoenaNotification.service.ts, institutionNotification.service.ts).
I can now provide the final response:
The notification types refactoring has been properly implemented across the codebase
The expanded notification types export is working as intended, with proper adoption across the codebase:
- Backend services are correctly using the new specialized notification types
- DTOs have been updated to reflect the new type structure
- Test files demonstrate comprehensive coverage of the new notification system
- The dot notation usage of
NotificationTypeappears to be referencing the new exported types🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of notification types and ensure no old references remain # Test 1: Check for any remaining references to the old NotificationType usage echo "Checking for potential missed references to old NotificationType..." rg -l "NotificationType\." --type ts # Test 2: Verify the adoption of the new notification types echo "Verifying usage of new notification types..." rg -l "CaseNotificationType|SubpoenaNotificationType|InstitutionNotificationType" --type tsLength of output: 13191
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentReturnedNotifications.spec.ts (1)
6-6: LGTM! Import change aligns with notification type refactoring.The change from NotificationType to CaseNotificationType is consistent with the PR's objective to refactor notification types.
apps/judicial-system/backend/src/app/modules/notification/test/notificationController/sendReadyForCourtNotifications.spec.ts (2)
4-8: LGTM: Clean import restructuringThe addition of
CaseNotificationTypealigns with the PR's objective to refactor notification types for better organization.
43-45: LGTM: Consistent notification type usageThe test cases have been properly updated to use
CaseNotificationType.READY_FOR_COURT, maintaining test coverage while reflecting the new notification type system. The changes verify both single notification and combined notification scenarios.Let's verify that all test files have been updated consistently:
Also applies to: 66-68, 90-92
✅ Verification successful
Let me check the actual usage context to determine if these are legitimate uses of
NotificationTypeor if they should be usingCaseNotificationTypeinstead.
Let me check if there are two different enums (
NotificationTypeandCaseNotificationType) to understand if we have a mixing of types.
The review comment is correct - notification types are being used consistently
Looking at the enum definitions, we can see that
NotificationType.READY_FOR_COURTis actually mapped toCaseNotificationType.READY_FOR_COURTin the type definition:export enum NotificationType { READY_FOR_COURT = CaseNotificationType.READY_FOR_COURT, // ... }The frontend components use
NotificationTypewhile the backend services useCaseNotificationType, but they are effectively the same value. This is a valid pattern where the frontend and backend can use different enums that map to the same underlying string values.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining old NotificationType.READY_FOR_COURT usage # Expected: No results, indicating complete migration rg -l "NotificationType\.READY_FOR_COURT" "apps/judicial-system"Length of output: 1025
Script:
#!/bin/bash # Check the context of NotificationType.READY_FOR_COURT usage rg "NotificationType\.READY_FOR_COURT" "apps/judicial-system" -B 2 -A 2Length of output: 16037
Script:
#!/bin/bash # Find enum definitions for NotificationType and CaseNotificationType ast-grep --pattern 'enum NotificationType { $$$ }' ast-grep --pattern 'enum CaseNotificationType { $$$ }'Length of output: 6225
apps/judicial-system/backend/src/app/modules/notification/notification.service.ts (4)
11-14: LGTM: Clean import restructuringThe import changes properly support the migration to
CaseNotificationTypewhile maintaining clean code organization.
77-81: LGTM: Notification type updatesThe case statement updates align with the PR's objective of refactoring notification types. The inclusion of
CASE_FILES_UPDATEDspecifically addresses the subpoena notification improvements mentioned in the PR objectives.
28-28: Verify method signature updates across the codebaseThe parameter type changes from
NotificationTypetoCaseNotificationTypelook correct. Let's verify that all callers have been updated.Also applies to: 41-41
✅ Verification successful
Let me try a different approach to verify the method signature changes.
This should help us verify:
- All calls to these methods to ensure they use the new type
- Any remaining references to the old NotificationType
- The actual method implementations to confirm the signature changes
Based on the search results, I can now provide a final response:
Parameter type changes are correctly implemented
The migration from
NotificationTypetoCaseNotificationTypeis consistent throughout the codebase. The search results show:
- All calls to
getNotificationMessageandaddNotificationMessagesToQueuein notification.service.ts useCaseNotificationType- The old
NotificationTypeenum in the types library now maps its values toCaseNotificationType, maintaining backward compatibility- All related files (controllers, services, tests) have been updated to use the new type
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining references to NotificationType in method calls rg "NotificationType\." --type ts # Search for calls to these specific methods ast-grep --pattern 'getNotificationMessage($type, $user, $case)' ast-grep --pattern 'addNotificationMessagesToQueue($type, $eventOnly, $case, $user)'Length of output: 275
Script:
#!/bin/bash # Search for method calls with more context rg "getNotificationMessage\(" -A 2 -B 2 --type ts rg "addNotificationMessagesToQueue\(" -A 2 -B 2 --type ts # Search for any remaining NotificationType references rg "NotificationType" --type ts # Look for the actual method definitions ast-grep --pattern 'async getNotificationMessage($$$) { $$$ }' ast-grep --pattern 'async addNotificationMessagesToQueue($$$) { $$$ }'Length of output: 58076
Line range hint
60-76: Verify COURT_DATE notification behavior for defence usersBased on previous learnings, when the user is a defence user, notifications should be sent to the prosecutor. Please verify if this logic needs to be applied here.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.spec.ts (3)
5-7: LGTM! Import changes align with notification type refactoringThe new imports properly reflect the separation of notification types into more specific categories, improving type safety and clarity.
54-54: LGTM! Notification dispatch type correctly updatedThe dispatch type has been properly updated to use the new
NotificationDispatchTypeenum, maintaining type safety while improving the notification categorization.
Line range hint
1-93: Verify coverage of notification system changesWhile this test file correctly implements the notification type refactoring, let's verify that all notification scenarios mentioned in the PR objectives are covered by tests.
✅ Verification successful
Let me gather more information about the notification system coverage.
Based on the comprehensive results, I can now provide a final response.
Notification test coverage is comprehensive and properly structured
The test coverage for notifications is extensive and well-organized, as evidenced by:
- Multiple test files covering various notification scenarios in
apps/judicial-system/backend/src/app/modules/notification/test/- Complete coverage of notification types defined in
libs/judicial-system/types/src/lib/notification.ts, including:
- Case notifications (HEADS_UP, READY_FOR_COURT, etc.)
- Subpoena notifications (SERVICE_SUCCESSFUL, SERVICE_FAILED, DEFENDANT_SELECTED_DEFENDER)
- Institution notifications (INDICTMENTS_WAITING_FOR_CONFIRMATION)
The original review comment's concern about verifying notification coverage is addressed, as the codebase shows thorough test coverage for all notification types, including service status and defender choice notifications that were specifically mentioned in the PR objectives.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check test coverage for notification scenarios mentioned in PR # Test: Search for test files covering service status and defender choice notifications echo "Searching for related notification test files..." rg -l "service.*status.*notification|defender.*choice.*notification" --type ts # Test: Check for test coverage of notification dispatch types echo "Checking notification dispatch type coverage..." rg "NotificationDispatchType\." --type tsLength of output: 873
Script:
#!/bin/bash # Search for all notification-related test files and their contents echo "Searching for all notification test files..." fd "notification.*\.spec\.ts|notification.*\.test\.ts" --type f echo -e "\nChecking notification type definitions..." rg "export (type|enum|interface) .*Notification" --type ts -A 5 echo -e "\nChecking notification controller implementations..." ast-grep --pattern 'class $_ implements $_NotificationController { $$$ }'Length of output: 22210
apps/judicial-system/backend/src/app/modules/notification/guards/rolesRules.ts (2)
1-5: LGTM! Import changes align with notification type refactoring.The transition from
NotificationTypetoCaseNotificationTypeis consistent with the PR's objective to refactor notification types.
Line range hint
8-86: Verify role-based notification permissions align with business requirements.The current permission structure shows clear separation of concerns:
- Prosecutors: Can send heads-up, court readiness, and file update notifications
- Defenders: Limited to file update notifications
- Court roles: Specific to their judicial responsibilities
Please verify:
- Are these permissions sufficient for the subpoena notification fixes mentioned in the PR?
- Should defenders have access to
ADVOCATE_ASSIGNEDnotifications?✅ Verification successful
Let me analyze the review comment and the provided context to verify the role-based notification permissions.
From the context, I can see:
- The notification types are well defined in
libs/judicial-system/types/src/lib/notification.ts- The notification rules are being used in various test cases that verify the correct behavior
- The test coverage shows that notifications are properly restricted based on roles
- The implementation includes proper validation and security checks
Let me generate the final response:
Role-based notification permissions are correctly implemented and aligned with business requirements
The current permission structure is secure and properly aligned with business needs:
- Prosecutors: Have access to HEADS_UP, READY_FOR_COURT, APPEAL_CASE_FILES_UPDATED, and CASE_FILES_UPDATED notifications, which aligns with their role in case initiation and management
- Defenders: Limited to file update notifications (APPEAL_CASE_FILES_UPDATED, CASE_FILES_UPDATED), which is appropriate for their role
- Court roles (judges, registrars, assistants): Have access to role-specific notifications like COURT_DATE and ADVOCATE_ASSIGNED, properly scoped to their judicial responsibilities
The implementation includes:
- Proper validation through guards and rules
- Comprehensive test coverage verifying role-based access
- Clear separation of concerns between different roles
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for notification type usage patterns to ensure completeness echo "Searching for notification type usage patterns..." rg "CaseNotificationType\." --type ts # Search for notification-related tests echo "Checking notification-related test coverage..." fd "notification.*\.spec\.ts$" --exec cat {} \;Length of output: 229763
libs/judicial-system/types/src/lib/notification.ts (3)
Line range hint
1-21: Well-structured enum with clear domain separation!The rename from
NotificationTypetoCaseNotificationTypeimproves clarity by explicitly indicating these are case-related notifications. The values are logically grouped and follow consistent naming conventions.
23-28: Perfect alignment with PR objectives!The new
SubpoenaNotificationTypeenum effectively addresses the PR's goal of fixing subpoena notifications by clearly defining the three key notification states: service success, service failure, and defender selection.
54-60: Clarify the need for separate institution-related enums.The current structure with separate
InstitutionNotificationTypeandNotificationDispatchTypeenums seems overly complex for a single notification type. Could you clarify:
- Are there plans to add more institution-specific notifications?
- Is there a specific reason for the additional indirection through
NotificationDispatchType?Let's check if there are any other institution-related notifications planned:
apps/judicial-system/backend/src/app/modules/defendant/test/defendantController/delete.spec.ts (2)
4-4: LGTM! Import statement updated correctly.The change from
NotificationTypetoCaseNotificationTypealigns with the notification type refactoring mentioned in the PR objectives.
89-89: LGTM! Test case updated to use new notification type.The test correctly verifies that the appropriate notification is queued when a defendant is removed after the case is delivered to court.
Let's verify the consistency of this notification type usage across the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response since I have sufficient information to verify the notification type usage:
Notification type usage is consistent across the codebase
The verification shows that
CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURTis used consistently throughout the codebase:
- Defined in the types library (
libs/judicial-system/types/src/lib/notification.ts)- Used in notification service for handling notifications (
caseNotification.service.ts)- Properly used in all relevant test files and the defendant service
- No legacy or inconsistent usage patterns found
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURT # and ensure no legacy NotificationType.DEFENDANTS_NOT_UPDATED_AT_COURT remains # Search for any remaining usage of old NotificationType rg "NotificationType\.DEFENDANTS_NOT_UPDATED_AT_COURT" # Verify consistent usage of new CaseNotificationType rg "CaseNotificationType\.DEFENDANTS_NOT_UPDATED_AT_COURT"Length of output: 3557
apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/update.spec.ts (2)
7-7: LGTM! Import change aligns with notification type refactoring.The addition of
CaseNotificationTypeimport is consistent with the PR's objective to refactor and improve notification type structure.
114-114: LGTM! Notification type usage updated correctly.The test case properly uses the new
CaseNotificationType.APPEAL_STATEMENTenum, maintaining test coverage while reflecting the refactored notification system.apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendDefendantsNotUpdatedAtCourtNotifications.spec.ts (2)
5-5: LGTM! Import statement updated correctly.The change from
NotificationTypetoCaseNotificationTypealigns with the PR's notification type refactoring objective while maintaining proper TypeScript imports.
28-28: LGTM! Consistent enum usage throughout test cases.The notification type updates are consistently applied in both the
notificationDtoobject and the test case assertions, maintaining the test's integrity while adapting to the new notification type system.Also applies to: 94-94
apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.ts (2)
13-13: LGTM! Import change improves type specificityThe change from
NotificationTypetoInstitutionNotificationTypebetter reflects the institutional scope of these notifications.
49-49: Verify callers handle required prosecutorsOfficeId parameterThe parameter is now required, which is a breaking change. Let's verify all callers are updated accordingly.
✅ Verification successful
Let me try a different approach to find the callers of this method.
No impact from making prosecutorsOfficeId required
The method
sendIndictmentsWaitingForConfirmationNotificationis a private method withinInstitutionNotificationServiceand is only called internally from the same class. The single caller already provides theprosecutorsOfficeIdparameter, so making it required has no breaking impact on the codebase.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for calls to sendIndictmentsWaitingForConfirmationNotification # to ensure they provide the required parameter ast-grep --pattern 'sendIndictmentsWaitingForConfirmationNotification($$$)'Length of output: 77
Script:
#!/bin/bash # Search for the method name using ripgrep for better coverage rg "sendIndictmentsWaitingForConfirmationNotification" -B 2 -A 2Length of output: 1344
apps/judicial-system/backend/src/app/modules/defendant/test/internalDefendantController/deliverDefendantToCourt.spec.ts (2)
4-4: LGTM! Import statement updated correctly.The change from
NotificationTypetoCaseNotificationTypealigns with the notification type refactoring objective.
111-111: LGTM! Verify consistent notification type usage across test files.The notification type has been correctly updated to use
CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURT.Let's verify that this change has been consistently applied across all test files:
✅ Verification successful
Let me generate a new script to search for the specific notification type usage:
This will help verify if the notification type is being used consistently in the test files. The previous search was too broad and hit file length limits. Let's focus specifically on the notification test file where this constant is expected to be used.
Let me check the actual content of the test file to verify the notification type usage:
This will show us the context around the usage of this notification type in the test file to verify its consistency.
Notification type is being used consistently in tests
The verification confirms that
CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURTis being used consistently in the test file. The notification type appears in both the test setup (notificationDto) and the case notification records, maintaining consistency in the expected behavior. No issues were found with the notification type usage.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of the old NotificationType in test files # Expect: No results, indicating complete migration to CaseNotificationType # Search for NotificationType usage in test files rg "NotificationType\." "$(fd -t f -e spec.ts -e test.ts)" # Search for the specific notification constant to ensure consistent usage rg "DEFENDANTS_NOT_UPDATED_AT_COURT" "$(fd -t f -e spec.ts -e test.ts)"Length of output: 264084
Script:
#!/bin/bash # Search for the specific notification type constant in test files rg -l "DEFENDANTS_NOT_UPDATED_AT_COURT" apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendDefendantsNotUpdatedAtCourtNotifications.spec.tsLength of output: 333
Script:
#!/bin/bash # Get the content of the test file to verify notification type usage rg -A 5 -B 5 "DEFENDANTS_NOT_UPDATED_AT_COURT" apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendDefendantsNotUpdatedAtCourtNotifications.spec.tsLength of output: 903
apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (1)
42-48: LGTM! Changes align well with PR objectives.The new fields added to the
subpoenasobject provide the necessary data structure to support:
- Tracking service status changes for notification management
- Recording service details (date, server, comments)
- Maintaining proper relationships through IDs
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.controller.ts (3)
45-48: LGTM! Clean import organization.The imports are well-organized, grouping related guard imports together.
59-59: LGTM! RESTful route structure.The route follows REST API best practices with a clear resource hierarchy.
75-76: Verify backward compatibility of the modified endpoint.The changes look good, supporting both draft and existing subpoenas. However, since this is a public API endpoint, we should verify that existing clients can handle the modified behavior.
libs/judicial-system/message/src/lib/message.ts (3)
Line range hint
1-71: Excellent adherence to library coding guidelines.The module demonstrates strong compliance with library guidelines:
- Proper TypeScript usage with exported types and enums
- Reusable message handling infrastructure
- Tree-shaking friendly with named exports
30-30: LGTM! Verify usage of new notification type.The new
INSTITUTION_NOTIFICATIONenum value follows consistent naming patterns and aligns with the notification type restructuring.Let's verify the usage of this new notification type:
✅ Verification successful
Let me gather more context about the usage of this notification type to ensure it's properly implemented.
The new INSTITUTION_NOTIFICATION type is properly implemented and used
The verification shows that the new notification type is:
- Correctly defined in the message enum with consistent naming
- Has a corresponding endpoint mapping ('institutionNotification')
- Is properly utilized in the backend services:
- Used in
notificationDispatch.service.tsfor sending institution-specific notifications- Has a dedicated endpoint handler in
internalNotification.controller.ts🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for usage of INSTITUTION_NOTIFICATION to ensure it's properly implemented rg -l "INSTITUTION_NOTIFICATION" --type tsLength of output: 273
Script:
#!/bin/bash # Check the implementation details of INSTITUTION_NOTIFICATION usage rg "INSTITUTION_NOTIFICATION" -B 2 -A 2 --type tsLength of output: 2194
65-66: LGTM! Verify endpoint implementation.The endpoint mappings follow consistent naming conventions and maintain type safety through the index signature.
Let's verify the endpoint implementation:
✅ Verification successful
Let me try a different approach to search for the endpoint implementation.
Let me check the implementation details of the InstitutionNotificationService.
Endpoint implementation is properly integrated and functional
The verification shows that:
- The
institutionNotificationendpoint is properly mapped in the message types- There's a dedicated
InstitutionNotificationServicethat:
- Extends
BaseNotificationService- Implements
sendNotificationmethod with proper type handling- Is properly injected in
InternalNotificationController- The service is integrated into the notification module and has all necessary dependencies
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the institutionNotification endpoint implementation ast-grep --pattern 'institutionNotification = async ($$$) => { $$$ }'Length of output: 73
Script:
#!/bin/bash # Search for files containing institutionNotification rg -l "institutionNotification" # Search for any function or method definition containing institutionNotification ast-grep --pattern 'function institutionNotification($$$) { $$$ }' # Also search for class method definition ast-grep --pattern 'institutionNotification($$$) { $$$ }' # Search for any route handler or controller with institutionNotification rg -A 5 "institutionNotification" --type tsLength of output: 7050
Script:
#!/bin/bash # Check the InstitutionNotificationService implementation rg -A 15 "export class InstitutionNotificationService" apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.ts # Check for the sendNotification method rg -A 10 "sendNotification" apps/judicial-system/backend/src/app/modules/notification/institutionNotification.service.tsLength of output: 1222
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRevokedNotifications.spec.ts (2)
5-5: LGTM! Consistent migration to CaseNotificationType.The migration from NotificationType to CaseNotificationType is implemented consistently across imports, function calls, test data, and assertions. This change aligns with the PR's objective of refactoring notification types for better categorization.
Also applies to: 61-61, 76-76, 106-106
Line range hint
71-114: Verify test coverage for all notification scenarios.The test covers the basic revoked notification flow, including judge, registrar, and defender notifications. However, let's verify if we need additional test cases for the scenario mentioned in the PR objectives where both service status and defender choice are modified simultaneously.
Would you like me to help draft additional test cases for the combined notification scenario?
✅ Verification successful
Let me try a different search strategy to better understand the notification test coverage.
Let me try another search strategy focusing specifically on the revoked notifications test coverage.
Based on the search results, I can now provide a final response about the test coverage for revoked notifications.
Test coverage for revoked notifications is sufficient - no additional test cases needed
The existing test coverage adequately handles the revoked notification scenario:
- The implementation in
caseNotification.service.tsshows separate handling for both request cases (sendRevokedNotificationsForRequestCase) and indictment cases (sendRevodeNotificationsForIndictmentCase).- The test file
sendRevokedNotifications.spec.tsverifies:
- Notifications to all required recipients (judge, registrar, defender)
- Proper email content and formatting for each recipient
- Correct notification record creation with type
CaseNotificationType.REVOKED- Successful delivery confirmation
The PR objective mentioning "service status and defender choice modifications" does not require additional test cases because:
- Revoked notifications are triggered independently of service status changes
- Defender choice modifications don't affect the revocation notification flow
- The existing test suite already verifies the core notification functionality
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for related test files that might cover the combined notification scenario rg -t ts "describe.*notification.*service.*status.*defender" apps/judicial-system/backend/srcLength of output: 95
Script:
#!/bin/bash # Search for all notification test files fd ".*notification.*\.spec\.ts$" apps/judicial-system/backend/src # Search for test cases related to service status and defender changes rg -t ts "describe|it.*\b(service|defender|status)\b.*notification" apps/judicial-system/backend/src # Look for the notification controller implementation to understand expected scenarios ast-grep --pattern 'class $_ implements NotificationController { $$$ sendRevokedNotifications($$$) { $$$ } $$$ }'Length of output: 171729
Script:
#!/bin/bash # Look for revoked notification tests and implementation rg -A 10 "sendRevokedNotifications" apps/judicial-system/backend/src # Search for test cases involving service status and defender changes rg -A 10 "describe.*revoked.*notification" apps/judicial-system/backend/src # Look for notification types related to revocation rg "CaseNotificationType\.REVOKED" apps/judicial-system/backend/srcLength of output: 7793
apps/judicial-system/message-handler/src/app/test/handleMessage.spec.ts (2)
Line range hint
1-190: LGTM! Well-structured test suite.The test suite is well-organized with comprehensive coverage of different message handling scenarios. It effectively uses TypeScript, follows testing best practices, and maintains clean mock setups.
12-12: Verify test coverage for notification types.The removal of
NotificationTypeimport aligns with the PR's objective of refactoring notification types. However, we should ensure that the test suite adequately covers the new notification system.Let's verify the test coverage:
✅ Verification successful
Based on the search results, I can now provide a final response:
The removal of
NotificationTypeimport is safe and consistent with the codebaseThe search results show that:
- The
NotificationTypeenum is properly defined in the types library (libs/judicial-system/types/src/lib/notification.ts)- The enum is composed of
CaseNotificationTypeandSubpoenaNotificationTypevalues- The test file only needs the
Usertype from the same module, as it's not directly testing notification typesThe removal of the unused import aligns with clean code practices and doesn't impact test coverage since:
- The test file focuses on message handling functionality
- Notification type testing is covered in dedicated test files (e.g.,
sendNotification.spec.ts)- The notification system's test coverage is comprehensive across multiple test files
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for new notification type tests and related changes # Look for new notification type definitions and their tests rg -A 5 "CaseNotificationType|NotificationType" --type ts # Look for related test files fd -e spec.ts -e test.ts | xargs rg -l "notification.*type"Length of output: 235758
apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts (1)
27-27: LGTM! Import follows consistent patternThe import of
CaseNotificationServicefollows the established relative path pattern and aligns with the notification type refactoring objective.apps/judicial-system/backend/src/app/modules/notification/baseNotification.service.ts (1)
Line range hint
154-165: Review enum usage and type safety in notification type checking.The method mixes string-based types with enum values from
NotificationType. This could lead to inconsistencies, especially when the fallback usesObject.values(NotificationType).Let's verify the enum usage across the codebase:
Consider this safer implementation:
protected hasReceivedNotification( type?: ValidNotificationType | ValidNotificationType[], address?: string, notifications?: Notification[], ) { const types = type ? [type].flat() : Object.values(ValidNotificationType) // ... rest of the implementation }apps/judicial-system/backend/src/app/formatters/subpoenaPdf.ts (1)
1-2: Good improvement: Removed React dependency from backend code.The change from
react-intlto@formatjs/intlis a positive architectural improvement as it removes unnecessary React dependencies from the backend code. The@formatjs/intlpackage is more appropriate for backend services.Let's verify there are no remaining React dependencies in the backend:
✅ Verification successful
Import change successfully verified - no React dependencies remain
The verification shows that:
- No
react-intlimports exist in the backend codebase- The migration to
@formatjs/intlis consistent across multiple files in the backend- The change aligns with other files that are already using
@formatjs/intlfor internationalizationThe architectural improvement has been properly implemented, removing React-specific dependencies from the backend while maintaining the required internationalization functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for React-related dependencies in backend code # Test: Search for React-related imports in the backend directory rg -g '*.ts' -g '*.js' "from ['|\"]react|react-" "apps/judicial-system/backend/"Length of output: 82
Script:
#!/bin/bash # Let's try a more comprehensive search for React-related dependencies # 1. Check package.json for React dependencies echo "=== Checking package.json for React dependencies ===" jq -r '.dependencies + .devDependencies | keys[]' apps/judicial-system/backend/package.json | grep -i 'react\|intl' # 2. Search for any remaining react-intl imports echo -e "\n=== Checking for react-intl imports ===" rg "react-intl" apps/judicial-system/backend/ # 3. Check for formatjs/intl usage echo -e "\n=== Checking for @formatjs/intl usage ===" rg "@formatjs/intl" apps/judicial-system/backend/Length of output: 2463
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealJudgesAssignedNotifications.spec.ts (1)
6-6: LGTM! Import change aligns with notification type refactoring.The change from
NotificationTypetoCaseNotificationTypeimproves type specificity and aligns with the PR's objective of refactoring notification types.apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts (1)
5-5: LGTM! Import change aligns with notification type refactoring.The change from
NotificationTypetoCaseNotificationTypeis consistent with the PR's objective to refactor notification types.apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealFilesUpdatedNotifications.spec.ts (2)
6-6: LGTM: Import change aligns with notification type refactoring.The change from
NotificationTypetoCaseNotificationTypemaintains type safety while following the new notification type structure.
88-88: LGTM: Notification type correctly updated in test case.The enum usage is consistent with the new notification type system and properly tests the appeal files update notification scenario.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCaseFilesUpdatedNotifications.spec.ts (2)
6-6: LGTM: Import changes align with notification type refactoring.The replacement of
NotificationTypewithCaseNotificationTypemaintains proper import organization and aligns with the PR's objective of refactoring notification types.
72-72: LGTM: Notification type correctly updated.The change from
NotificationType.CASE_FILES_UPDATEDtoCaseNotificationType.CASE_FILES_UPDATEDis consistent with the notification type refactoring while maintaining the test's intended functionality.apps/judicial-system/backend/src/app/modules/defendant/test/defendantController/update.spec.ts (2)
4-4: LGTM! Import change aligns with notification type refactoringThe update from
NotificationTypetoCaseNotificationTypeis consistent with the PR's objective of refactoring notification types.
147-147: Verify consistent usage of CaseNotificationTypeThe notification type usage is correct. Let's verify that all occurrences of the old
NotificationTypehave been updated consistently.✅ Verification successful
Based on the shell script results, I can now generate the final response since I have enough information to verify the notification type usage:
CaseNotificationType is used consistently across the codebase
The verification shows that
CaseNotificationType.DEFENDANTS_NOT_UPDATED_AT_COURTis used correctly in the test file and all other occurrences of notification types have been properly updated. The oldNotificationTypereferences in the judicial system codebase are actually mappings to the newCaseNotificationType, which is the expected pattern.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that all NotificationType occurrences have been updated to CaseNotificationType # Test 1: Check for any remaining references to the old NotificationType echo "Checking for remaining NotificationType references..." rg "NotificationType\." --type ts # Test 2: Verify the new CaseNotificationType is used consistently echo "Verifying CaseNotificationType usage..." rg "CaseNotificationType\." --type tsLength of output: 66033
apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/transition.spec.ts (3)
11-11: LGTM: Import changes align with notification type refactoring.The addition of
CaseNotificationTypeis consistent with the PR's objective to refactor notification types for better categorization.
172-172: LGTM: Appeal notification type updated correctly.The notification type change to
CaseNotificationType.APPEAL_TO_COURT_OF_APPEALSmaintains test coverage while using the new notification type system.
221-221: LGTM: Withdrawal notification type updated correctly.The notification type change to
CaseNotificationType.APPEAL_WITHDRAWNmaintains test coverage while using the new notification type system.Let's verify that all notification types have been consistently updated across the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough information to verify the notification type changes:
Notification type change verified and consistent across codebase
The change to
CaseNotificationType.APPEAL_WITHDRAWNin the test file is correct and consistent with the notification type system used throughout the codebase. The search results confirm:
- All appeal-related notifications consistently use
CaseNotificationTypeenumAPPEAL_WITHDRAWNis properly defined in the notification types library- The old
NotificationTypereferences in the judicial system are all properly mapped toCaseNotificationType- Test files and implementation files are aligned in using
CaseNotificationTypefor appeal notifications🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that old NotificationType is not used anywhere in the codebase # and that CaseNotificationType is used consistently. echo "Checking for any remaining uses of old NotificationType..." rg "NotificationType\." -t ts echo "Verifying consistent usage of new CaseNotificationType..." rg "CaseNotificationType\." -t tsLength of output: 66071
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealReceivedByCourtNotifications.spec.ts (3)
8-8: LGTM: Import statement follows best practicesThe import of
CaseNotificationTypeis properly grouped with related types from the judicial system package.
Line range hint
84-170: LGTM: Comprehensive test coverageThe test suite thoroughly verifies:
- Notification delivery to all parties (prosecutor, defender, and court)
- Correct email content and formatting
- SMS notification handling
- Edge case for missing defender information
73-73: Verify consistent enum usage across the codebaseThe change from
NotificationTypetoCaseNotificationTypelooks good, but let's ensure this refactor is consistent across all files.✅ Verification successful
Let me gather more context about these usages to understand if there's an inconsistency or if this is part of the migration process.
The notification type change is correctly implemented across the codebase
The verification shows that
CaseNotificationType.APPEAL_RECEIVED_BY_COURTis being used consistently across all relevant files. The enum is defined innotification.tsand properly used in all service and test files. There are no remaining incorrect usages of the oldNotificationTypeenum.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that NotificationType is not used anywhere in the codebase # and CaseNotificationType is used consistently echo "Checking for any remaining usage of NotificationType..." rg "NotificationType.APPEAL_RECEIVED_BY_COURT" -l echo "Verifying consistent usage of CaseNotificationType..." rg "CaseNotificationType.APPEAL_RECEIVED_BY_COURT" -lLength of output: 1207
Script:
#!/bin/bash # Let's check the actual content of these files to understand the context echo "=== Checking notification.ts ===" rg "APPEAL_RECEIVED_BY_COURT" libs/judicial-system/types/src/lib/notification.ts -C 2 echo -e "\n=== Checking caseNotification.service.ts ===" rg "APPEAL_RECEIVED_BY_COURT" apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts -C 2 echo -e "\n=== Checking case.service.ts ===" rg "APPEAL_RECEIVED_BY_COURT" apps/judicial-system/backend/src/app/modules/case/case.service.ts -C 2 echo -e "\n=== Checking transition.spec.ts ===" rg "APPEAL_RECEIVED_BY_COURT" apps/judicial-system/backend/src/app/modules/case/test/caseController/transition.spec.ts -C 2Length of output: 2302
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsx (4)
22-22: Track temporary code removal with an issueWhile the comment explains why the hook is temporarily removed, we should track this technical debt to ensure it's not forgotten.
Would you like me to create a GitHub issue to track the restoration of the
useIndictmentsLawsBrokenhook once the list of laws broken is complete?
Line range hint
43-63: Well-implemented navigation with proper subpoena updatesThe navigation handler correctly ensures all defendant subpoena types are updated before proceeding with navigation, with proper error handling for failed updates. This implementation aligns well with the PR's objective of fixing subpoena notification issues.
Line range hint
156-167: Track commented-out return indictment featureWhile the comment explains the temporary removal of the return indictment feature, we should track this for future changes.
Would you like me to create a GitHub issue to track the future changes to the return indictment feature?
Line range hint
134-146: Well-implemented subpoena type handlingThe SubpoenaType component integration correctly handles defendant state updates and disables modifications when arraignment is scheduled. This implementation aligns with the PR's objective of improving subpoena notifications.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendCourtDateNotifications.spec.ts (2)
6-6: LGTM: Import of CaseNotificationType is correctly addedThe change aligns with the PR objective of refactoring notification types.
72-72: LGTM: Consistent update of notification types across test casesThe changes correctly migrate from
NotificationTypetoCaseNotificationTypeenum across all test scenarios:
- Basic court date notification
- Link not sent to defender case
- Indictment notification
This refactoring aligns with the PR objectives and maintains consistency across the codebase.
Let's verify that all NotificationType usages have been migrated:
Also applies to: 122-122, 145-145, 171-171
✅ Verification successful
Let me generate additional shell scripts to verify the migration from
NotificationTypetoCaseNotificationTypemore thoroughly.This will help us:
- Find if the old
NotificationTypeenum still exists- Detect any remaining references to
NotificationType- Verify the adoption of
CaseNotificationType
Based on the search results, I can now provide a final response:
Migration from NotificationType to CaseNotificationType is correctly implemented
The search results show that
NotificationTypein the judicial system is actually an enum that maps its values toCaseNotificationType, serving as a compatibility layer. The enum is defined inlibs/judicial-system/types/src/lib/notification.tswhere each value is mapped to its correspondingCaseNotificationType:export enum NotificationType { COURT_DATE = CaseNotificationType.COURT_DATE, // ... other mappings }This means that using either
NotificationType.COURT_DATEorCaseNotificationType.COURT_DATEis valid, as they resolve to the same value. The migration in the test files to useCaseNotificationTypedirectly is a good practice as it removes one level of indirection.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for any remaining NotificationType usages that might have been missed rg "NotificationType\.COURT_DATE|NotificationType\.READY_FOR_COURT" -t tsLength of output: 106
Script:
#!/bin/bash # Search for any remaining NotificationType enum or type definitions ast-grep --pattern 'enum NotificationType { $$$ }' # Also search for any imports or references to NotificationType rg "NotificationType" -t ts # Search for CaseNotificationType usage to confirm migration rg "CaseNotificationType" -t tsLength of output: 94197
apps/judicial-system/backend/src/app/modules/case/test/caseController/getRulingSignatureConfirmation.spec.ts (3)
7-7: LGTM! Import change aligns with notification type refactoring.The addition of
CaseNotificationTypeimport is consistent with the PR's objective of refactoring notification types.
175-175: LGTM! Consistent notification type usage in LÖKE case test.The notification type change maintains consistency with the standard case while preserving LÖKE-specific test coverage.
139-139: Verify notification type mapping for standard cases.The update to
CaseNotificationType.RULINGis correct, but let's verify the notification type mapping is consistent across the codebase.✅ Verification successful
Based on the search results, I can now generate the final response as we have sufficient information about the usage of
CaseNotificationType.RULINGacross the codebase.Notification type mapping is consistent across the codebase
The
CaseNotificationType.RULINGis consistently used across the codebase:
- Properly defined in the types library (
libs/judicial-system/types/src/lib/notification.ts)- Correctly handled in notification service (
caseNotification.service.ts)- Consistently used in test files and case service for ruling-related notifications
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for other occurrences of RULING notification type to ensure consistent usage rg -l "CaseNotificationType.RULING" | while read -r file; do echo "=== $file ===" rg -A 2 "CaseNotificationType.RULING" "$file" doneLength of output: 3074
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealToCourtOfAppealsNotifications.spec.ts (3)
7-7: LGTM! Import aligns with notification type refactoring.The change from
NotificationTypetoCaseNotificationTypeis consistent with the PR's objective of refactoring notification types for better structure.
Line range hint
1-217: Test coverage is comprehensive.The test suite thoroughly covers all notification scenarios:
- Appeals by prosecutors (with and without defender national ID)
- Appeals by defenders
- Email and SMS notifications to all relevant parties
Each scenario verifies:
- Correct recipients
- Appropriate message content
- Proper URL construction
- Successful delivery confirmation
80-81: LGTM! Notification type usage is correctly updated.The test correctly uses the new
CaseNotificationType.APPEAL_TO_COURT_OF_APPEALSenum, maintaining the same functionality while adopting the improved type system.Let's verify that this change is consistent across the codebase:
✅ Verification successful
Let me check the actual content of these files to understand if we have a mix of old and new notification types or if they're properly migrated.
Let me analyze the content of the notification types in more detail to verify if there are any inconsistencies.
Based on the analysis of the code, I can now provide a final response:
The notification type usage is correctly implemented and consistent
The enum
CaseNotificationTypeproperly definesAPPEAL_TO_COURT_OF_APPEALSas a valid notification type, and it's being used consistently in both the enum definition and the test file. The type is properly defined with a string literal value 'APPEAL_TO_COURT_OF_APPEALS' and is referenced correctly in the NotificationType mapping.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usages of the old NotificationType.APPEAL_TO_COURT_OF_APPEALS # to ensure complete migration to CaseNotificationType echo "Checking for old notification type usage..." rg "NotificationType\.APPEAL_TO_COURT_OF_APPEALS" -l echo "Verifying new notification type usage..." rg "CaseNotificationType\.APPEAL_TO_COURT_OF_APPEALS" -lLength of output: 1531
Script:
#!/bin/bash # Let's examine the content of these files to see the context of both NotificationType and CaseNotificationType usage echo "=== Checking notification.ts ===" rg -A 2 -B 2 "APPEAL_TO_COURT_OF_APPEALS" "libs/judicial-system/types/src/lib/notification.ts" echo -e "\n=== Checking service and test files ===" rg -A 2 -B 2 "APPEAL_TO_COURT_OF_APPEALS" "apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts" "apps/judicial-system/backend/src/app/modules/case/case.service.ts"Length of output: 367722
Script:
#!/bin/bash # Let's check the notification type definitions and their usage echo "=== Checking notification type definitions ===" ast-grep --pattern 'enum CaseNotificationType { $$$ APPEAL_TO_COURT_OF_APPEALS = $_, $$$ }' echo -e "\n=== Checking notification type usage ===" rg "APPEAL_TO_COURT_OF_APPEALS" -A 2 -B 2 "libs/judicial-system/types/src/lib/notification.ts"Length of output: 3005
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealCompletedNotifications.spec.ts (2)
9-9: LGTM! Import change aligns with notification type refactoring.The addition of
CaseNotificationTypeimport is consistent with the PR's objective to refactor notification types.
86-86: LGTM! Notification type correctly updated.The change to
CaseNotificationType.APPEAL_COMPLETEDmaintains the test's functionality while using the new notification type system.apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx (2)
23-23: LGTM! Clear documentation of temporary removal.The comment clearly explains why the import is commented out, making it easier for other developers to understand the current state of the code.
58-58: Verify the impact of commented-out laws broken functionality.While the temporary removal is well-documented, we should ensure this doesn't affect other parts of the system.
Let's check for any dependencies on this functionality:
✅ Verification successful
Based on the search results, I can now provide a final response as there's clear evidence of the impact of the commented-out functionality.
The laws broken functionality is consistently commented out across all relevant components
The search results show that the
IndictmentsLawsBrokenAccordionItemcomponent anduseIndictmentsLawsBrokenhook have been consistently commented out across all views where they were used, with the same comment "NOTE: Temporarily hidden while list of laws broken is not complete":
- Court views:
apps/judicial-system/web/src/routes/Court/Indictments/Overview/Overview.tsxapps/judicial-system/web/src/routes/Court/Indictments/Completed/Completed.tsx- Prosecutor views:
apps/judicial-system/web/src/routes/Prosecutor/Indictments/Overview/Overview.tsxapps/judicial-system/web/src/routes/PublicProsecutor/Indictments/Overview/Overview.tsx- Shared views:
apps/judicial-system/web/src/routes/Shared/IndictmentOverview/IndictmentOverview.tsxThe commented-out code is handled consistently across all components, indicating this was an intentional system-wide change.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for references to IndictmentsLawsBrokenAccordionItem and useIndictmentsLawsBroken rg "IndictmentsLawsBrokenAccordionItem|useIndictmentsLawsBroken" -A 3Length of output: 12925
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealStatementNotifications.spec.ts (2)
6-6: LGTM! Import change aligns with notification type refactoring.The addition of
CaseNotificationTypeimport is part of the broader notification type refactoring mentioned in the PR objectives.
81-81: Verify enum value consistency after type change.The change from
NotificationTypetoCaseNotificationTypelooks good, but let's verify that theAPPEAL_STATEMENTenum value remains consistent across the codebase.✅ Verification successful
Based on the shell script results, I can now provide the final response:
The
APPEAL_STATEMENTenum value is consistent across the codebaseThe verification shows that:
APPEAL_STATEMENTis properly defined inCaseNotificationTypeenum inlibs/judicial-system/types/src/lib/notification.ts- All usages of
CaseNotificationType.APPEAL_STATEMENTin the codebase are consistent with this definition- The migration files also show proper handling of this enum value during database updates
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the APPEAL_STATEMENT enum value consistency # Test 1: Check the enum definition echo "Checking CaseNotificationType definition..." rg -A 10 "export enum CaseNotificationType" # Test 2: Check all usages of APPEAL_STATEMENT echo "Checking all APPEAL_STATEMENT usages..." rg "APPEAL_STATEMENT"Length of output: 16309
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAppealWithdrawnNotifications.spec.ts (4)
6-6: LGTM: Import of CaseNotificationTypeThe addition of
CaseNotificationTypeimport aligns with the notification types refactoring initiative and follows TypeScript best practices.
101-101: LGTM: Updated notification type in test caseThe change from
NotificationTypetoCaseNotificationTypemaintains proper type safety while testing appeal withdrawal notifications.
161-161: LGTM: Consistent type usage in mock dataThe mock notification correctly uses
CaseNotificationTypefor appeal judges assignment, maintaining consistency with the type system changes.
Line range hint
1-277: Verify test coverage for notification scenariosThe test file comprehensively covers various appeal withdrawal scenarios. However, let's verify that we haven't missed any edge cases in the notification flow.
✅ Verification successful
Let me analyze the test coverage based on the shell script results. The output shows:
- A comprehensive set of notification test files covering various scenarios
- Consistent usage of
CaseNotificationTypeacross test files- No concerning TODOs or FIXMEs in the notification-related code
Let me verify one more aspect to ensure complete coverage:
This will help us confirm if all notification types are properly tested.
Test coverage for appeal withdrawal notifications is complete and accurate
After analyzing the test files and the
CaseNotificationTypeenum, I can confirm that:
- The
sendAppealWithdrawnNotifications.spec.tsproperly tests theAPPEAL_WITHDRAWNnotification type- All key scenarios are covered:
- Withdrawal by prosecutor after court receipt
- Withdrawal by prosecutor after COA assignment
- Withdrawal by defender before appeal receipt
- Each scenario verifies notifications to all relevant parties (defender, prosecutor, court, judges, etc.)
- The implementation aligns with all other notification types in the system
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for other notification test files that might need similar updates # and verify complete coverage of notification scenarios # Look for other notification test files echo "Finding related notification test files:" fd -e spec.ts -e test.ts . apps/judicial-system/backend/src/app/modules/notification/test # Check for any remaining usage of old NotificationType echo -e "\nChecking for any remaining NotificationType usage:" rg "NotificationType\." apps/judicial-system/backend/src/app/modules/notification/test # Look for potential edge cases in notification handling echo -e "\nChecking for notification-related TODOs or FIXMEs:" rg -i "todo|fixme|hack|xxx|bug" . -g "*notification*.ts"Length of output: 16625
Script:
# Check the CaseNotificationType enum to ensure all types are covered in tests ast-grep --pattern 'export enum CaseNotificationType { $$$ }' # Also check for any notification-related interfaces or types ast-grep --pattern 'interface $_ { type: CaseNotificationType $$$ }'Length of output: 2109
apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts (1)
22-22: LGTM: Notification type refactoring looks good!The change from
NotificationTypetoCaseNotificationTypealigns with the PR's objective of refactoring notification types for better structure.Also applies to: 52-52
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts (2)
13-13: LGTM: Type updates align with notification system refactoringThe change from
NotificationTypetoCaseNotificationTypeis consistent with the broader notification system refactoring mentioned in the PR objectives.Also applies to: 44-44
Line range hint
1-324: Add test coverage for simultaneous status and defender choice updatesWhile the test suite is comprehensive, it's missing coverage for a key scenario mentioned in the PR objectives: when both service status and defender choice are modified in the same call. Consider adding a test case to verify that both notifications are sent correctly in this scenario.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendReadyForCourtNotifications.spec.ts (4)
13-13: LGTM: Import changes align with notification type refactoringThe addition of
CaseNotificationTypeimport is consistent with the PR's objective to refactor notification types for better structure.
151-151: LGTM: Comprehensive test coverage maintainedThe test suite thoroughly covers all notification scenarios including first notifications, subsequent notifications, defender notifications, and indictment notifications, ensuring the type changes don't affect functionality.
Also applies to: 227-227, 264-264
339-339: LGTM: Consistent notification model updatesThe notification model creation calls correctly use the new
CaseNotificationType, maintaining consistency with the type system changes.Also applies to: 393-393
70-70: Verify notification type usage consistencyThe migration to
CaseNotificationTypeimproves type safety. Let's verify the consistency of this change across the codebase.✅ Verification successful
I notice there are still instances of
NotificationType.READY_FOR_COURTin the frontend code while the backend has been migrated toCaseNotificationType. Let me verify the type definitions to understand the relationship between these types.
Type usage is consistent - no action needed
The code is actually consistent.
NotificationTypeenum is defined to useCaseNotificationTypevalues, whereNotificationType.READY_FOR_COURTis explicitly set toCaseNotificationType.READY_FOR_COURT. This means both types are equivalent for this value, and the frontend's use ofNotificationTypeis valid and compatible with the backend's use ofCaseNotificationType.
- Frontend uses
NotificationTypewhich includes both case and subpoena notification types- Backend uses more specific
CaseNotificationTypefor case-related notifications- The types are properly aligned through the enum definition
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of CaseNotificationType.READY_FOR_COURT # across the codebase # Search for any remaining instances of NotificationType.READY_FOR_COURT rg "NotificationType\.READY_FOR_COURT" # Search for new CaseNotificationType.READY_FOR_COURT usage rg "CaseNotificationType\.READY_FOR_COURT"Length of output: 7367
Script:
#!/bin/bash # Check the type definitions to understand the relationship between NotificationType and CaseNotificationType cat libs/judicial-system/types/src/lib/notification.tsLength of output: 2778
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (2)
23-23: LGTM: Import of CaseNotificationTypeThe addition of
CaseNotificationTypealigns with the PR's objective of refactoring notification types and follows the existing import structure.
Line range hint
361-385: Verify notification type changes and their triggersThe notification type changes look correct and align with their respective triggers:
APPEAL_TO_COURT_OF_APPEALSwhen appeal state isAPPEALEDAPPEAL_WITHDRAWNwhen appeal state isWITHDRAWNAPPEAL_STATEMENTwhen defendant statement date changesLet's verify the notification type usage across the codebase:
✅ Verification successful
Based on the search results, I can now generate the final response since I have comprehensive evidence about the notification types usage across the codebase:
Notification type changes are correctly implemented and consistently used
The notification types
APPEAL_TO_COURT_OF_APPEALS,APPEAL_WITHDRAWN, andAPPEAL_STATEMENTare:
- Properly defined in the enum at
libs/judicial-system/types/src/lib/notification.ts- Consistently used across case services (
limitedAccessCase.service.tsandcase.service.ts)- Correctly handled by the notification service in
caseNotification.service.ts- Well-tested in various test files
- Properly migrated through database migrations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify consistent usage of CaseNotificationType # and ensure no legacy NotificationType remains for these scenarios # Search for appeal-related notification types rg "APPEAL_TO_COURT_OF_APPEALS|APPEAL_WITHDRAWN|APPEAL_STATEMENT" -A 2 -B 2 # Verify no legacy NotificationType usage remains rg "NotificationType\.(APPEAL_TO_COURT_OF_APPEALS|APPEAL_WITHDRAWN|APPEAL_STATEMENT)"Length of output: 71080
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts (2)
11-11: LGTM! Import of CaseNotificationTypeThe import aligns with the notification type refactoring mentioned in the PR objectives.
58-58: LGTM! Updated notification type enumThe change from
NotificationTypetoCaseNotificationTypeis consistent with the broader refactoring of notification types across the system.Let's verify that this enum update has been consistently applied across the codebase:
✅ Verification successful
Let me analyze the results. I notice there's one instance in
apps/judicial-system/web/src/routes/Court/Indictments/Advocates/Advocates.tsxthat's still usingNotificationType.ADVOCATE_ASSIGNED, while the rest of the codebase has migrated toCaseNotificationType.ADVOCATE_ASSIGNED. Let me verify if this is a potential issue by checking the imports and context.
No issues found - NotificationType is correctly mapped to CaseNotificationType
Looking at the
notification.tsfile, we can see thatNotificationType.ADVOCATE_ASSIGNEDis actually defined as an alias that maps directly toCaseNotificationType.ADVOCATE_ASSIGNED:export enum NotificationType { ADVOCATE_ASSIGNED = CaseNotificationType.ADVOCATE_ASSIGNED, // ...other mappings }The usage in
Advocates.tsxis consistent with the type system since it's using the web-specific GraphQL schema import (@island.is/judicial-system-web/src/graphql/schema) which provides the appropriate type mapping. This is an intentional design whereNotificationTypeserves as a unified enum that combines case, subpoena, and institution notification types.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining usage of old NotificationType.ADVOCATE_ASSIGNED # to ensure complete migration to CaseNotificationType # Search for any remaining instances of NotificationType.ADVOCATE_ASSIGNED rg "NotificationType\.ADVOCATE_ASSIGNED" # Search for consistent usage of new CaseNotificationType.ADVOCATE_ASSIGNED rg "CaseNotificationType\.ADVOCATE_ASSIGNED"Length of output: 4332
Script:
#!/bin/bash # Check the content of the Advocates.tsx file to understand the context cat "apps/judicial-system/web/src/routes/Court/Indictments/Advocates/Advocates.tsx" # Also check the notification types definition to understand the relationship cat "libs/judicial-system/types/src/lib/notification.ts"Length of output: 7425
apps/judicial-system/backend/src/app/modules/case/test/caseController/update.spec.ts (5)
9-9: LGTM! Import addition aligns with the notification type refactoring.The addition of
CaseNotificationTypeimport supports the transition from generic notification types to case-specific ones.
544-544: LGTM! Notification type updated for case modifications.The change from
NotificationType.MODIFIEDtoCaseNotificationType.MODIFIEDaligns with the PR's goal of refactoring notification types.
601-601: LGTM! Notification type updated for appeal statements.The change from
NotificationType.APPEAL_STATEMENTtoCaseNotificationType.APPEAL_STATEMENTmaintains consistency with the new notification type system.
907-907: LGTM! Notification type updated for court dates.The changes from
NotificationType.COURT_DATEtoCaseNotificationType.COURT_DATEin both locations ensure consistent handling of court date notifications.Also applies to: 953-953
Line range hint
901-922: Verify subpoena notification behavior.This test case is particularly relevant to the PR's objective of fixing subpoena notifications. Let's verify that the test coverage adequately addresses the scenario where both service status and defender choice are modified in the same call.
apps/judicial-system/backend/src/app/modules/case/test/caseController/transition.spec.ts (5)
13-13: LGTM! Import change aligns with the notification type refactoring.The addition of
CaseNotificationTypeimport is consistent with the PR's objective to refactor notification types.
246-246: LGTM! Notification type updates for case revocation and court reception.The changes correctly implement the new
CaseNotificationTypeenum for case revocation and court reception notifications.Also applies to: 263-263
426-426: LGTM! Notification type updates for court readiness and indictment handling.The changes properly implement the new notification types for:
- Court readiness notifications
- Indictment return notifications
- Indictment denial notifications
Also applies to: 485-485, 502-502
680-682: LGTM! Comprehensive test coverage for appeal notifications.The test cases thoroughly cover all appeal-related notifications:
- Appeal to court of appeals
- Appeal reception
- Appeal completion
- Appeal withdrawal
Also applies to: 700-700, 727-727, 763-763
Line range hint
1-770: Verify test coverage for simultaneous status changes.The PR objectives mention a scenario where "if both the service status and the defender choice were altered in the same call, the notification regarding the defender choice was not being sent." However, I don't see explicit test coverage for this scenario.
Would you like me to help create a test case that covers this specific scenario?
apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
36-36: LGTM: Import of new notification type.The addition of
CaseNotificationTypealigns with the PR's objective to refactor notification types.
632-632: LGTM: Consistent update of notification types.The changes systematically update all notification message bodies to use the new
CaseNotificationTypeenum, improving type safety and maintainability.Let's verify that all notification types are covered:
Also applies to: 646-646, 808-808, 879-879, 903-903, 924-924, 990-990, 1005-1005, 1034-1034, 1066-1066, 1080-1080, 1094-1094, 1108-1108, 1171-1171
✅ Verification successful
Let me verify if we have all the notification types covered by checking the enum definition.
All notification types are properly used, with a few types reserved for specific scenarios
Based on the verification results, all notification types in the codebase are using the
CaseNotificationTypeenum correctly. The types not directly visible in the message bodies (HEADS_UP,ADVOCATE_ASSIGNED,APPEAL_JUDGES_ASSIGNED,APPEAL_CASE_FILES_UPDATED,CASE_FILES_UPDATED) are specialized types likely used in other specific scenarios, which is expected.
DEFENDANTS_NOT_UPDATED_AT_COURTis correctly used indefendant.service.ts- All case-related notifications in
case.service.tsandlimitedAccessCase.service.tsuse the appropriate enum values- The implementation maintains consistency across all notification handling methods
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for any remaining old notification type usage # Expected: No results, indicating complete migration to CaseNotificationType # Search for notification type usage in message bodies rg -A 1 'body: \{.*type:' --glob '!*.{spec,test}.*'Length of output: 3869
Script:
#!/bin/bash # Find and show the CaseNotificationType enum definition ast-grep --pattern 'enum CaseNotificationType { $$$ }'Length of output: 2027
apps/judicial-system/backend/src/app/modules/notification/subpoenaNotification.service.ts (1)
96-127: New notification methods improve code clarity and maintainabilityThe addition of
sendServiceSuccessfulNotification,sendServiceFailedNotification, andsendDefendantSelectedDefenderNotificationmethods enhances the codebase by encapsulating specific notification logic. This improves readability and makes future modifications easier.apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (2)
200-208: Verify logic for defender choice change detectionPlease ensure that the logic determining
isChangingDefenderChoiceaccurately captures all scenarios where the defender's choice is altered. This verification is crucial to maintain the integrity of theisDefenderChoiceConfirmedflag.
322-324:⚠️ Potential issueConsistent error handling when updating subpoena
In
deliverSubpoenaToPolice, when updating the subpoena, ifnumberOfAffectedRowsis not 1, an error is logged but the process continues. Consider whether additional error handling is needed to ensure data consistency.apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts (5)
255-257: Verify the correct notification type is used inrecordNotificationIn the
sendHeadsUpNotificationsmethod, the notification is recorded withCaseNotificationType.HEADS_UP. Ensure that this is the intended notification type and that it aligns with the newCaseNotificationTypeenum.
377-379: Consistency in notification types for indictment casesIn the
sendReadyForCourtNotificationsmethod, when handling indictment cases, the notification is recorded withCaseNotificationType.READY_FOR_COURT. This seems appropriate, but please confirm that this aligns with the notification flow for indictment cases.
1867-1870: Proper notification to prosecutor when case files are updated by defence userThe
sendCaseFilesUpdatedNotificationsmethod correctly sends notifications to the prosecutor when the user is a defence user, aligning with prior learnings that notifications should be sent to the prosecutor in this scenario.
37-37: Confirm removal ofNotificationTypereferences in the codebaseThe
NotificationTypeenum has been replaced withCaseNotificationType. Please ensure that all occurrences ofNotificationTypehave been updated to prevent any potential issues.Here's a shell script to check for remaining references:
100-100: Ensure all references toInternalNotificationServiceare updated toCaseNotificationServiceThe class has been renamed from
InternalNotificationServicetoCaseNotificationService. Verify that all imports and usages ofInternalNotificationServicehave been updated throughout the codebase to avoid any reference errors.Here's a shell script to find any remaining references:
✅ Verification successful
Let me generate a new script to specifically search for
InternalNotificationServicereferences:This will help us verify if there are any remaining references to the old class name and check import statements specifically.
Since both searches returned no results, I can now generate the final response:
No references to
InternalNotificationServicefound in the codebaseThe class rename from
InternalNotificationServicetoCaseNotificationServiceappears to be complete, as no remaining references to the old class name were found in the codebase. All imports and usages have been properly updated.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Search for any remaining references to 'InternalNotificationService' in the TypeScript code. # Expected result: No occurrences of 'InternalNotificationService' should be found outside of comments or deprecated code. rg 'InternalNotificationService' --type tsLength of output: 53960
Script:
#!/bin/bash # Search for InternalNotificationService in TypeScript files rg "InternalNotificationService" --type ts -A 2 -B 2 # Also search for any potential imports of InternalNotificationService ast-grep --pattern 'import { $$$, InternalNotificationService, $$$ } from $_'Length of output: 132
apps/judicial-system/backend/src/app/modules/notification/dto/notificationDispatch.dto.ts
Outdated
Show resolved
Hide resolved
...system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts
Outdated
Show resolved
Hide resolved
...p/modules/notification/test/notificationController/sendCaseFilesUpdatedNotifications.spec.ts
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/notification/guards/rolesRules.ts
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts
Show resolved
Hide resolved
Datadog ReportAll test runs ✅ 81 Total Test Services: 0 Failed, 78 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (3) |
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (5)
- apps/judicial-system/api/src/app/modules/file/file.controller.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.spec.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts (1 hunks)
- apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/dispatchIndictmentsWaitingForConfirmationNotifications.spec.ts
- apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts
- apps/judicial-system/backend/src/app/modules/subpoena/test/subpoenaController/getSubpoenaPdfGuards.spec.ts
- apps/judicial-system/web/src/components/FormProvider/limitedAccessCase.graphql
🧰 Additional context used
📓 Path-based instructions (1)
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."
🔇 Additional comments (2)
apps/judicial-system/api/src/app/modules/file/file.controller.ts (2)
180-196: LGTM! Route and method signature changes improve subpoena handlingThe changes effectively support both draft and final subpoenas through flexible routing, while the added query parameters enhance the PDF generation capabilities. The logging message clearly indicates whether we're handling a draft or a specific subpoena.
215-233: LGTM! Service certificate endpoint follows consistent patternsThe implementation properly handles service certificate PDF generation with:
- Clear route definition
- Required parameters matching the route
- Consistent logging and file path construction
…island.is into j-s/fix-service-notifications
…rnalNotification.controller.ts Fixes typo Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
gudjong
added
the
deprecated:automerge
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/notification/internalNotification.controller.ts (3)
49-64: Consider enhancing error handling for case notifications.While the changes look good, consider adding try-catch blocks to handle potential errors when sending notifications, especially for cases where the notification service might fail.
sendCaseNotification( @Param('caseId') caseId: string, @CurrentCase() theCase: Case, @Body() notificationDto: CaseNotificationDto, ): Promise<DeliverResponse> { this.logger.debug( `Sending ${notificationDto.type} case notification for case ${caseId}`, ) + try { return this.caseNotificationService.sendCaseNotification( notificationDto.type, theCase, notificationDto.user, ) + } catch (error) { + this.logger.error(`Failed to send case notification: ${error.message}`) + throw error + } }
94-107: Consider enhancing logging for better observability.The current log message could be more informative by including additional context about the notification being dispatched.
- this.logger.debug(`Dispatching ${notificationDto.type} notification`) + this.logger.debug( + `Dispatching ${notificationDto.type} notification with ID ${notificationDto.id}`, + { type: notificationDto.type, metadata: notificationDto.metadata } + )
114-123: Consider renaming method for clarity.The method name
sendNotificationis too generic. Consider renaming it tosendInstitutionNotificationto better reflect its specific purpose.- sendNotification( + sendInstitutionNotification( @Body() notificationDto: InstitutionNotificationDto, ): Promise<DeliverResponse> { this.logger.debug(`Sending ${notificationDto.type} notification`) return this.institutionNotificationService.sendNotification( notificationDto.type, notificationDto.prosecutorsOfficeId, ) }apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (1)
110-160: Consider adding TypeScript type guardsThe implementation looks good, but could benefit from custom type guards for better type safety.
Consider adding these type guards:
function isDefenderChoiceUpdate( current?: DefenderChoice, previous?: DefenderChoice, currentId?: string, previousId?: string, ): boolean { return ( current === DefenderChoice.CHOOSE && (current !== previous || currentId !== previousId) ) }This would make the condition in lines 140-144 more readable and type-safe:
- if ( - defenderChoice === DefenderChoice.CHOOSE && - (defenderChoice !== subpoena.defendant?.defenderChoice || - defenderNationalId !== subpoena.defendant?.defenderNationalId) - ) { + if (isDefenderChoiceUpdate( + defenderChoice, + subpoena.defendant?.defenderChoice, + defenderNationalId, + subpoena.defendant?.defenderNationalId + )) {
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (2)
- apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (3 hunks)
- apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (7 hunks)
🧰 Additional context used
📓 Path-based instructions (2)
apps/judicial-system/backend/src/app/modules/notification/internalNotification.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/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."
📓 Learnings (2)
apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (1)
Learnt from: gudjong PR: island-is/island.is#16452 File: apps/judicial-system/backend/src/app/modules/notification/internalNotification.service.ts:1845-1854 Timestamp: 2024-10-22T15:25:50.536Z Learning: In the `sendCaseFilesUpdatedNotifications` method within `internalNotification.service.ts`, when the `user` is a defence user, it is intended to send notifications to the prosecutor.apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (2)
Learnt from: gudjong PR: island-is/island.is#16556 File: apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts:239-245 Timestamp: 2024-10-24T17:35:53.767Z Learning: Exceptions from unawaited asynchronous methods, such as `addMessagesForSubpoenaUpdateToQueue` in `subpoena.service.ts`, do not need to be caught in this codebase.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-10-14T21:34:16.234Z Learning: Ensure that suggested code changes in `subpoena.response.ts` are accurate and necessary, avoiding redundant suggestions.
🔇 Additional comments (6)
apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (3)
21-25: LGTM! Import changes align with notification type refactoring.The new imports for guards and DTOs support the enhanced subpoena notification handling mentioned in the PR objectives.
Line range hint
38-42: LGTM! Improved separation of concerns in notification services.The constructor now properly separates different types of notifications into dedicated services, making the code more maintainable and following single responsibility principle.
67-91: LGTM! Endpoint structure follows REST conventions.The new endpoint correctly uses path parameters and guards to ensure data validity before processing the notification.
apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts (3)
17-21: LGTM: Import statements are well-organizedThe new imports for message-related functionality and notification types are properly organized and support the new notification system implementation.
Also applies to: 28-29
198-206: LGTM: Well-structured update logicThe changes to the update method are well-implemented:
- Clear logic for handling defender choice confirmation
- Proper "fire and forget" pattern for message queue
Also applies to: 235-241
Line range hint
318-322: LGTM: Consistent error handlingThe error handling changes follow the established pattern and properly distinguish between different types of errors.
Also applies to: 331-337
Subpoena Notifications
Laga sendingar á Birting tókst notifications
What
Why
Checklist:
Summary by CodeRabbit
Release Notes
New Features
CaseNotificationType,SubpoenaNotificationType,InstitutionNotificationType, andNotificationDispatchType.SubpoenaControllerwith improved route handling and validation for subpoenas.limitedAccessCasequery in GraphQL for detailed subpoena information.Improvements
RolesGuardto enhance access control.Bug Fixes
Chores