feat(j-s): Notifications to prosecutors office in completed indictments #17174
Conversation
WalkthroughThe pull request introduces several changes across multiple files in the judicial system application. Key modifications include the addition of a new secret for police institution emails, updates to notification handling classes and methods, the introduction of new Data Transfer Objects (DTOs), and enhancements to notification types. Additionally, configuration files for different environments are updated to include the new secret. Overall, these changes aim to improve the notification system and its integration with various components of the application. Changes
Possibly related PRs
Suggested reviewers
📜 Recent review detailsConfiguration used: .coderabbit.yaml 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
🔇 Additional comments (3)charts/judicial-system/values.staging.yaml (3)
The
The secret path follows the established pattern for email-related secrets in the system. Let's verify the secret is consistently configured across environments: ✅ Verification successful✅ Secret configuration is consistent across all environments The
🏁 Scripts executedThe following scripts were executed for the analysis: Script: #!/bin/bash
# Check if the secret is configured consistently across environments
rg "POLICE_INSTITUTIONS_EMAILS" charts/judicial-system/values.*.yaml
Length of output: 521
The significant increase in memory (512Mi → 1024Mi) and CPU (350m → 400m) limits suggests higher resource requirements for the new notification feature. Let's analyze the resource usage patterns: Also applies to: 228-228 Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 17 Total Test Services: 0 Failed, 16 Passed Test ServicesThis report shows up to 10 services
🔻 Code Coverage Decreases vs Default Branch (1)
|
…land-is/island.is into j-s/indictment-complete-modal
…land-is/island.is into j-s/indictment-complete-modal
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Outside diff range and nitpick comments (12)
charts/services/judicial-system-backend/values.dev.yaml (1)
Line range hint
1-6: Note: This is an auto-generated file.As indicated by the header comment, this file should not be edited manually. Instead, use the "yarn charts" command to regenerate it.
charts/judicial-system/values.dev.yaml (1)
228-228: Consider updating documentationSince this is a new configuration for prosecutor notifications, consider updating the relevant documentation to reflect this addition and its purpose.
Would you like me to help create a documentation update task or PR?
apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.service.ts (1)
108-108: Simplify access using optional chainingThe expression
theCase.defendants && theCase.defendants[0].serviceRequirementcan be simplified using optional chaining for better readability and to handle cases wheredefendantsmight be undefined or empty.Apply this diff to simplify the code:
- serviceRequirement: - theCase.defendants && theCase.defendants[0].serviceRequirement, + serviceRequirement: theCase.defendants?.[0]?.serviceRequirement,🧰 Tools
🪛 Biome (1.9.4)
[error] 108-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
apps/judicial-system/backend/src/app/modules/notification/dto/indictmentCaseNotification.dto.ts (1)
7-12: LGTM! Consider enhancing API documentation.The DTO implementation follows best practices with proper validation and type safety.
Consider adding a description to the API property for better documentation:
- @ApiProperty({ enum: IndictmentCaseNotificationType }) + @ApiProperty({ + enum: IndictmentCaseNotificationType, + description: 'The type of indictment case notification to be sent to prosecutors' + })apps/judicial-system/backend/src/app/modules/notification/dto/notificationDispatch.dto.ts (1)
17-22: LGTM! Consider reducing decorator duplication.The implementation is correct and follows established patterns.
Consider creating a decorator factory to reduce duplication between NotificationDispatchDto and EventNotificationDispatchDto:
// Create a utility function for common decorators function createEnumValidation(enumType: any) { return function (target: any, propertyKey: string) { IsNotEmpty()(target, propertyKey); IsEnum(enumType)(target, propertyKey); ApiProperty({ enum: enumType })(target, propertyKey); }; } // Usage in DTOs export class EventNotificationDispatchDto { @createEnumValidation(EventNotificationType) readonly type!: EventNotificationType }apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.strings.ts (1)
4-18: Consider adding more context in notification messagesWhile the messages are well-structured for i18n, consider enhancing them with additional context that might be valuable for prosecutors:
- Include judgment date in the subject
- Add case type or category in the body
- Consider including a direct link to the case in LÖKE system
indictmentCompletedWithRuling: defineMessages({ subject: { id: 'judicial.system.backend:indictment_case_notifications.verdict_service.subject', - defaultMessage: 'Máli lokið {courtCaseNumber}', + defaultMessage: 'Máli lokið {courtCaseNumber} - {judgmentDate}', description: 'Notað sem titill í tilkynningu um stöðu birtingar dóms í lokinni ákæru', }, body: { id: 'judicial.system.backend:indictment_case_notifications.verdict_service.body', defaultMessage: - 'Máli {courtCaseNumber} hjá {courtName} hefur verið lokið.\n\nNiðurstaða: Dómur\n\n{serviceRequirement, select, REQUIRED {Birta skal dómfellda dóminn} NOT_REQUIRED {Birting dóms ekki þörf} NOT_APPLICABLE {Dómfelldi var viðstaddur dómsuppkvaðningu} other {}}\n\n{caseOrigin, select, LOKE {Dómur er aðgengilegur í LÖKE.} other {}}', + 'Máli {courtCaseNumber} ({caseType}) hjá {courtName} hefur verið lokið.\n\nNiðurstaða: Dómur\n\n{serviceRequirement, select, REQUIRED {Birta skal dómfellda dóminn} NOT_REQUIRED {Birting dóms ekki þörf} NOT_APPLICABLE {Dómfelldi var viðstaddur dómsuppkvaðningu} other {}}\n\n{caseOrigin, select, LOKE {Dómur er aðgengilegur í LÖKE: {lokeUrl}} other {}}', description: 'Notað sem body í tilkynningu um stöðu birtingar dóms í lokinni ákæru', },apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/eventNotificationDispatch/eventNotificationDispatch.spec.ts (2)
15-15: Add type validation for the case IDThe case ID is created using uuid() but used without type validation. Consider using a proper type assertion or validation.
- const theCase = { id: uuid() } as Case + const theCase = { id: uuid(), type: 'INDICTMENT' } as Case & { id: string }
65-76: Enhance the validation test for notification typesThe validation test could be more informative by:
- Providing specific error messages
- Logging which types are missing
- Validating the count of notification types
it('will fail if a new EventNotificationType is missing from the tests', () => { const allNotificationTypes = Object.values(EventNotificationType) const testedNotificationTypes = notificationScenarios.map( (scenario) => scenario.notificationType, ) const missingNotificationTypes = allNotificationTypes.filter( (type) => !testedNotificationTypes.includes(type), ) - expect(missingNotificationTypes).toEqual([]) + expect(missingNotificationTypes).toEqual( + [], + `Missing test scenarios for notification types: ${missingNotificationTypes.join(', ')}`, + ) + + // Ensure we're testing all notification types + expect(testedNotificationTypes.length).toBe( + allNotificationTypes.length, + 'Number of test scenarios does not match number of notification types', + ) })libs/judicial-system/types/src/lib/notification.ts (1)
Line range hint
9-9: Remove or update the ADVOCATE_ASSIGNED commentThe comment indicates this will be removed, but it's still being used in the NotificationType enum. Either remove it now or create a tracking issue for its future removal.
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.spec.ts (2)
1-62: LGTM! Consider extracting test data setupThe test setup is well-structured with proper mocking and test utilities. However, the test data setup could be moved to a separate helper file for better reusability across test suites.
Consider creating a separate helper file like
indictmentTestData.tsto store test case data:// indictmentTestData.ts export const createTestCase = (overrides = {}) => ({ id: uuid(), court: { name: uuid() }, origin: CaseOrigin.LOKE, defendants: [/* default test data */], prosecutor: {/* default test data */}, ...overrides });
86-95: Improve test description clarityWhile the test logic is correct, the test description could be more explicit about the business rule being tested.
Consider renaming the test to better reflect the business rule:
- it('should not send a notification if indictment ruling decision is not RULING', async () => { + it('should mark notification as delivered without sending email when indictment ruling decision is not RULING', async () => {apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts (1)
36-36: Enhance type safety for notification servicesThe integration of
IndictmentCaseNotificationServicefollows the established pattern, but we can improve type safety.Consider creating a type for the context object:
interface NotificationTestContext { userService: UserService; emailService: EmailService; // ... other services indictmentCaseNotificationService: IndictmentCaseNotificationService; } export const createTestingNotificationModule = async (): Promise<NotificationTestContext> => { // ... existing implementation }Also applies to: 134-134, 163-165
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (25)
apps/judicial-system/backend/infra/judicial-system-backend.ts(1 hunks)apps/judicial-system/backend/src/app/messages/notifications.ts(1 hunks)apps/judicial-system/backend/src/app/modules/event-log/eventLog.module.ts(1 hunks)apps/judicial-system/backend/src/app/modules/event-log/eventLog.service.ts(4 hunks)apps/judicial-system/backend/src/app/modules/notification/dto/indictmentCaseNotification.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/internalNotification.controller.ts(5 hunks)apps/judicial-system/backend/src/app/modules/notification/notification.config.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/notification.module.ts(2 hunks)apps/judicial-system/backend/src/app/modules/notification/notificationDispatch.service.ts(2 hunks)apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.service.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.strings.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts(3 hunks)apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/eventNotificationDispatch/eventNotificationDispatch.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts(1 hunks)charts/judicial-system/values.dev.yaml(1 hunks)charts/judicial-system/values.prod.yaml(1 hunks)charts/judicial-system/values.staging.yaml(1 hunks)charts/services/judicial-system-backend/values.dev.yaml(1 hunks)charts/services/judicial-system-backend/values.prod.yaml(1 hunks)charts/services/judicial-system-backend/values.staging.yaml(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/notifications.ts
🧰 Additional context used
📓 Path-based instructions (18)
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.config.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/infra/judicial-system-backend.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/event-log/eventLog.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/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/services/indictmentCaseNotification/indictmentCaseNotification.strings.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/test/internalNotificationController/eventNotificationDispatch/eventNotificationDispatch.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/dto/indictmentCaseNotification.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/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/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.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/event-log/eventLog.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."
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."
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."
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."
apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.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."
libs/judicial-system/message/src/lib/message.ts (1)
Pattern libs/**/*: "Confirm that the code adheres to the following:
- Reusability of components and hooks across different NextJS apps.
- TypeScript usage for defining props and exporting types.
- Effective tree-shaking and bundling practices."
apps/judicial-system/backend/src/app/modules/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."
📓 Learnings (5)
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-11-12T15:15:11.835Z
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 (2)
Learnt from: gudjong
PR: island-is/island.is#15393
File: apps/judicial-system/backend/src/app/modules/case/internalCase.controller.ts:28-28
Timestamp: 2024-11-12T15:15:11.835Z
Learning: The `DeliverCancellationNoticeDto` import in `apps/judicial-system/backend/src/app/modules/case/internalCase.controller.ts` is used in the `deliverIndictmentCancellationNoticeToCourt` method.
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-11-12T15:15:11.835Z
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.
libs/judicial-system/types/src/lib/notification.ts (1)
Learnt from: gudjong
PR: island-is/island.is#16556
File: apps/judicial-system/backend/src/app/modules/notification/guards/rolesRules.ts:13-16
Timestamp: 2024-11-12T15:15:26.274Z
Learning: In the `CaseNotificationType` enum in `libs/judicial-system/types/src/lib/notification.ts`, `APPEAL_CASE_FILES_UPDATED` and `CASE_FILES_UPDATED` are distinct notification types that need to remain separate, as they serve different purposes in the notification system.
apps/judicial-system/backend/src/app/modules/notification/services/indictmentCaseNotification/indictmentCaseNotification.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-11-12T15:15:11.835Z
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/notificationDispatch.service.ts (2)
Learnt from: gudjong
PR: island-is/island.is#15393
File: apps/judicial-system/backend/src/app/modules/case/internalCase.service.ts:691-711
Timestamp: 2024-11-12T15:15:20.158Z
Learning: The `deliverIndictmentCancellationNoticeToCourt` method in `InternalCaseService` returns `{ delivered: false }` in case of an exception, as error handling is managed by a message queue handler.
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-11-12T15:15:11.835Z
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.
🪛 Biome (1.9.4)
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.spec.ts
[error] 79-79: 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] 80-80: 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/services/indictmentCaseNotification/indictmentCaseNotification.service.ts
[error] 108-108: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
🔇 Additional comments (21)
apps/judicial-system/backend/src/app/modules/notification/notification.config.ts (1)
24-29: LGTM! Verify email configuration format.
The implementation follows the established pattern and type safety. However, let's verify the expected format of the POLICE_INSTITUTIONS_EMAILS secret.
✅ Verification successful
Format verified: POLICE_INSTITUTIONS_EMAILS follows the expected structure
The test file confirms that the environment variable follows the expected JSON format of {"institution_id": "[email protected]"}, which matches the type definition in the config. This is demonstrated in the test setup:
process.env.POLICE_INSTITUTIONS_EMAILS = `{"${prosecutorInstitutionId}": "${prosecutorsOfficeEmail}"}`🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the format of POLICE_INSTITUTIONS_EMAILS secret
# Expected format: { "institution_id": "[email protected]" }
# Check if the secret exists and its format
gh api graphql -f query='
{
repository(owner: "island-is", name: "island.is") {
object(expression: "HEAD:apps/judicial-system/backend/src/app/modules/notification") {
... on Tree {
entries {
name
object {
... on Blob {
text
}
}
}
}
}
}
}' | jq -r '.data.repository.object.entries[] | select(.name | endswith(".test.ts")) | .object.text' | grep -A 5 -B 5 "POLICE_INSTITUTIONS_EMAILS"
Length of output: 490
Script:
#!/bin/bash
# Let's try a different approach to find test files and configuration usage
rg -A 5 -B 5 "POLICE_INSTITUTIONS_EMAILS" --type ts
# Also search for any test files in the notification module
fd "notification.*\.test\.ts" -t f
Length of output: 4869
apps/judicial-system/backend/infra/judicial-system-backend.ts (1)
71-72: LGTM! Secret configuration is properly structured.
The secret is correctly configured following the established pattern and is appropriately grouped with other email-related secrets.
charts/services/judicial-system-backend/values.dev.yaml (1)
141-141: LGTM! Secret configuration matches infrastructure.
The secret configuration is consistent with the infrastructure setup and follows the established pattern.
✅ Verification successful
Let me verify the actual configuration values across environments to ensure complete consistency.
✅ Secret configuration is consistent across all environments
The POLICE_INSTITUTIONS_EMAILS secret path '/k8s/judicial-system/POLICE_INSTITUTIONS_EMAILS' is identical across dev, staging, and production environments, confirming proper infrastructure alignment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify secret configuration consistency across environments
# Expected: Same secret path in all environment files
# Check if the secret is consistently configured across environments
for env in dev staging prod; do
echo "Checking $env environment..."
fd -t f "values.${env}.yaml" charts/services/judicial-system-backend -x grep -l "POLICE_INSTITUTIONS_EMAILS"
done
Length of output: 784
Script:
#!/bin/bash
# Check the actual secret path configuration across environments
for env in dev staging prod; do
echo "=== ${env} environment ==="
grep "POLICE_INSTITUTIONS_EMAILS:" "charts/services/judicial-system-backend/values.${env}.yaml" -A 1
done
Length of output: 995
charts/services/judicial-system-backend/values.prod.yaml (1)
141-141: Verify the secret exists in production environment
The addition of POLICE_INSTITUTIONS_EMAILS secret aligns with the PR objective to implement notifications to prosecutors' office. However, we should verify that this secret is properly configured in the production environment.
charts/services/judicial-system-backend/values.staging.yaml (1)
141-141: Verify the secret exists in staging environment
The addition of POLICE_INSTITUTIONS_EMAILS secret is consistent with other environments. Let's verify its existence in staging.
charts/judicial-system/values.dev.yaml (1)
228-228: Verify the secret exists in development environment
The addition of POLICE_INSTITUTIONS_EMAILS secret is consistent with staging and production environments. Let's verify its existence in development.
✅ Verification successful
Secret path is consistent across all environments
The secret path /k8s/judicial-system/POLICE_INSTITUTIONS_EMAILS is correctly configured and consistently defined across all environments (development, staging, and production). The implementation follows the established pattern used for other secrets in the system, such as PRISON_ADMIN_EMAIL.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the secret exists in AWS Secrets Manager for development environment
# Note: This helps prevent deployment issues due to missing secrets
aws secretsmanager list-secrets --filters Key=name,Values=/k8s/judicial-system/POLICE_INSTITUTIONS_EMAILS
Length of output: 201
Script:
#!/bin/bash
# Let's check if this secret is referenced in other environment files
fd "values.*yaml" charts/judicial-system/ --exec grep -l "POLICE_INSTITUTIONS_EMAILS"
# Also check the related deployment files
fd "deployment.*yaml" charts/judicial-system/ --exec grep -l "POLICE_INSTITUTIONS_EMAILS"
# Check if there's any documentation or configuration mentioning this secret
rg "POLICE_INSTITUTIONS_EMAILS" charts/judicial-system/ -l
Length of output: 478
Script:
#!/bin/bash
# Let's check the actual content of these files to compare the secret paths
echo "=== Development ==="
grep -A 1 "POLICE_INSTITUTIONS_EMAILS" charts/judicial-system/values.dev.yaml
echo -e "\n=== Staging ==="
grep -A 1 "POLICE_INSTITUTIONS_EMAILS" charts/judicial-system/values.staging.yaml
echo -e "\n=== Production ==="
grep -A 1 "POLICE_INSTITUTIONS_EMAILS" charts/judicial-system/values.prod.yaml
Length of output: 831
charts/judicial-system/values.staging.yaml (1)
228-228: LGTM! Verify secret creation in AWS Parameter Store
The addition of the POLICE_INSTITUTIONS_EMAILS secret is consistent with the feature requirements for prosecutor notifications.
Run the following script to verify the secret exists in the staging environment:
charts/judicial-system/values.prod.yaml (1)
228-228: LGTM! Verify secret creation in AWS Parameter Store
The addition of the POLICE_INSTITUTIONS_EMAILS secret in production matches the staging configuration, maintaining environment parity.
Run the following script to verify the secret exists in the production environment:
apps/judicial-system/backend/src/app/modules/notification/notificationDispatch.service.ts (2)
71-83: Implementation of 'dispatchIndictmentSentToPublicProsecutorNotifications' is correct
The method correctly dispatches indictment notification messages to the queue using the MessageService. The message structure aligns with the expected format, and TypeScript types are appropriately used.
85-108: Proper error handling in 'dispatchEventNotification' method
The dispatchEventNotification method effectively handles different notification types with a switch statement. Errors are caught and logged, and the method returns { delivered: false } when an error occurs, ensuring robustness.
apps/judicial-system/backend/src/app/modules/event-log/eventLog.service.ts (1)
27-33: Effective mapping with 'eventToNotificationMap'
The introduction of eventToNotificationMap provides a clear and maintainable way to map event types to notification types, facilitating future expansions.
apps/judicial-system/backend/src/app/modules/notification/internalNotification.controller.ts (3)
87-109: Well-structured addition of 'sendIndictmentCaseNotification' endpoint
The new sendIndictmentCaseNotification endpoint is appropriately defined with correct routing, guards, and dependency injection. The use of CaseTypeGuard ensures only indictment cases are processed.
194-208: Refactored 'sendInstitutionNotification' method is correctly updated
The method now utilizes the InstitutionNotificationDto and properly calls the institutionNotificationService. Logging and API documentation have been updated accordingly.
210-232: Proper implementation of 'dispatchEventNotification' method
The dispatchEventNotification endpoint is well-implemented with appropriate guards and logging. It correctly delegates the notification dispatching to the notificationDispatchService.
apps/judicial-system/backend/src/app/modules/event-log/eventLog.module.ts (1)
11-14: LGTM! Verify circular dependency necessity.
The implementation correctly uses forwardRef for circular dependency resolution.
Let's verify if the circular dependency is necessary by checking the dependencies between EventLogModule and MessageModule:
apps/judicial-system/backend/src/app/modules/notification/notification.module.ts (1)
23-23: LGTM! Service registration looks good
The IndictmentCaseNotificationService is properly imported and registered in the module's providers array.
Also applies to: 51-51
libs/judicial-system/types/src/index.ts (1)
22-23: LGTM! New notification type exports added correctly
The new notification type exports are properly structured and align with the PR objectives for implementing prosecutor notifications.
libs/judicial-system/types/src/lib/notification.ts (2)
51-81: LGTM! NotificationType enum maintains single source of truth
The NotificationType enum properly references the source enums for each notification type, maintaining a single source of truth and making the code more maintainable.
23-25: LGTM! New notification types align with requirements
The new IndictmentCaseNotificationType and EventNotificationType enums are well-structured and their values clearly indicate their purpose.
Let's verify these notification types are used consistently:
Also applies to: 47-49
✅ Verification successful
Notification types are used consistently across the codebase
The verification shows that both notification types are properly integrated:
INDICTMENT_VERDICT_INFOis correctly used in notification services and testsINDICTMENT_SENT_TO_PUBLIC_PROSECUTORis consistently used in event logs, filters, and frontend components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check usage of new notification types
rg "INDICTMENT_VERDICT_INFO|INDICTMENT_SENT_TO_PUBLIC_PROSECUTOR" -t ts
Length of output: 3621
libs/judicial-system/message/src/lib/message.ts (1)
34-35: LGTM! Message types and endpoints properly defined
The new message types and their corresponding endpoints are well-structured and follow the established patterns in the codebase.
Let's verify the endpoint naming consistency:
Also applies to: 73-74
✅ Verification successful
Endpoint naming follows consistent patterns
The endpoint naming for the new message types follows the established patterns in the codebase:
- Notification-related endpoints use either the
notificationsuffix (e.g.,indictmentCaseNotification) or thedispatchpattern (e.g.,eventNotification/dispatch) - The new endpoints align with existing ones like
notification/dispatchand other notification endpoints
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for similar endpoint patterns in the codebase
rg "notification.*dispatch" -t ts
Length of output: 584
Script:
#!/bin/bash
# Check the endpoint mapping in message.ts to verify consistency
rg -A 5 "messageEndpoint = {" -t ts
Length of output: 139
Script:
#!/bin/bash
# Check the endpoint mapping in message.ts with fixed regex
rg -A 5 "messageEndpoint.*=.*{" libs/judicial-system/message/src/lib/message.ts
Length of output: 187
Script:
#!/bin/bash
# View the content of message.ts to check endpoint mappings
cat libs/judicial-system/message/src/lib/message.ts
Length of output: 4725
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendIndictmentsWaitingForConfirmationNotifications.spec.ts (1)
63-63: LGTM! Method rename improves clarity
The change from sendNotification to sendInstitutionNotification better reflects the domain context and improves code readability.
apps/judicial-system/backend/src/app/modules/event-log/eventLog.service.ts
Outdated
Show resolved
Hide resolved
...t/internalNotificationController/eventNotificationDispatch/eventNotificationDispatch.spec.ts
Show resolved
Hide resolved
...ificationController/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.spec.ts
Show resolved
Hide resolved
...ificationController/indictmentCaseNotification/sendIndictmentVerdictInfoNotification.spec.ts
Show resolved
Hide resolved
oddsson
added
the
deprecated:automerge
…ts (#17174) * fix(j-s): Display confirmation modal before completing case * feat(j-s): Notifications for prosecutor * chore: charts update dirty files * chore: nx format:write update dirty files * fix(j-s): Only send verdict info if there was a ruling * Update eventLog.service.ts --------- Co-authored-by: andes-it <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Tilkynning til lögreglu að máli hafi verið lokið með dóm
What
Send notifications to prosecutors office when indictment has been completed with a judgement
Why
So they can react accordingly if verdict needs servicing
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Tests
IndictmentCaseNotificationServiceandInternalNotificationController.Chores