-
Notifications
You must be signed in to change notification settings - Fork 61
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
chore(j-s): Add more info to new indictment robot emails #17184
Conversation
WalkthroughThis pull request introduces enhancements to 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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverIndictmentInfoToCourt.spec.ts (1)
57-61
: LGTM! Consider using a more realistic test email domain.The addition of the prosecutor's email property aligns with the PR objectives. While the implementation is correct, consider using a more realistic domain for the test email (e.g.,
@domstolar.is
or similar official domain) to better reflect production scenarios.apps/judicial-system/backend/src/app/modules/court/test/createCourtCase.spec.ts (1)
108-108
: LGTM! Consider adding explicit test cases for hyphenated numbers.The implementation correctly removes hyphens from police case numbers across all test cases. However, since this is a new requirement, consider adding specific test cases with hyphenated police case numbers to explicitly verify this behavior.
Example test case addition:
it('should remove hyphens from police case number', () => { const hyphenatedNumber = '123-456-789' policeCaseNumbers = [hyphenatedNumber] // ... test setup ... expect(mockCourtClientService.createCase).toHaveBeenCalledWith( courtId, expect.objectContaining({ sourceNumber: '123456789' }) ) })Also applies to: 149-149, 186-186, 221-221
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (4)
apps/judicial-system/backend/src/app/modules/case/internalCase.service.ts
(1 hunks)apps/judicial-system/backend/src/app/modules/case/test/internalCaseController/deliverIndictmentInfoToCourt.spec.ts
(1 hunks)apps/judicial-system/backend/src/app/modules/court/court.service.ts
(3 hunks)apps/judicial-system/backend/src/app/modules/court/test/createCourtCase.spec.ts
(4 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
apps/judicial-system/backend/src/app/modules/court/test/createCourtCase.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/deliverIndictmentInfoToCourt.spec.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/internalCase.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/court/court.service.ts (1)
Pattern apps/**/*
: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
📓 Learnings (1)
apps/judicial-system/backend/src/app/modules/case/internalCase.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.
🔇 Additional comments (4)
apps/judicial-system/backend/src/app/modules/court/court.service.ts (3)
339-341
: LGTM: Police case number sanitization implemented correctly.
The implementation properly handles the removal of hyphens from police case numbers and includes null safety checks.
Also applies to: 350-350
575-575
: LGTM: Prosecutor email field added and police case number sanitization implemented.
The changes successfully:
- Add the prosecutor's email field to the type definition
- Implement consistent police case number sanitization
Also applies to: 579-581, 585-585
575-585
: Verify the implementation across the codebase.
Let's verify:
- All callers are updated to handle the new prosecutor email field
- Police case number sanitization is consistent
✅ Verification successful
Let me gather more information about the usage of this method to verify all callers.
Implementation verification completed
- The police case number sanitization is consistent across the codebase, with all instances using the same
.replace(/-/g, '')
pattern - The
updateIndictmentCaseWithIndictmentInfo
method is only called frominternalCase.service.ts
, and the prosecutor parameter is optional, so no updates to callers are required - The test file correctly mocks the method call without requiring the optional prosecutor parameter
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for prosecutor email field usage
echo "Checking for prosecutor email field usage..."
rg -p "updateIndictmentCaseWithIndictmentInfo.*prosecutor.*email" apps/
# Check for police case number sanitization
echo "Checking for consistent police case number sanitization..."
rg -p "policeCaseNumber.*replace.*-" apps/
Length of output: 898
Script:
#!/bin/bash
# Check for all usages of updateIndictmentCaseWithIndictmentInfo
echo "Checking for method calls..."
rg -p "updateIndictmentCaseWithIndictmentInfo" apps/ -A 5
# Check the method definition
echo "Checking method definition..."
ast-grep --pattern 'updateIndictmentCaseWithIndictmentInfo($$$) {
$$$
}'
Length of output: 1394
apps/judicial-system/backend/src/app/modules/case/internalCase.service.ts (1)
609-609
: LGTM! Verify court service compatibility.
The addition of prosecutor's email aligns with the PR objective to enhance indictment robot emails.
Let's verify if the court service is ready to handle this new field:
✅ Verification successful
Based on the search results, I can see that the court client service has an updateCaseWithProsecutor
method that handles prosecutor updates to the court, and the email field is a standard string type in user-related interfaces. The court client service implementation shows it properly handles prosecutor updates through its API. Since the court service is already set up to handle prosecutor information updates and email is a basic string field, this change should be safe.
LGTM! Court service is ready to handle prosecutor's email
The court client service has proper infrastructure to handle prosecutor updates including basic fields like email through its updateCaseWithProsecutor
API endpoint.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if court service is updated to handle prosecutor email
# Search for prosecutor email handling in court service
ast-grep --pattern 'updateIndictmentCaseWithIndictmentInfo($$$) {
$$$
prosecutor?: {
$$$
email?: $_
$$$
}
$$$
}'
# Backup: Search using ripgrep
rg -A 5 'updateIndictmentCaseWithIndictmentInfo.*prosecutor.*email'
Length of output: 204
Script:
#!/bin/bash
# Let's try a broader search to find court service interfaces and types
# Search for court service files
fd -t f "court.*\.(ts|js)"
# Search for any interface or type definitions related to indictment cases
rg -A 10 "interface.*Indictment|type.*Indictment"
# Search for prosecutor type definitions
rg -A 5 "interface.*Prosecutor|type.*Prosecutor"
# Search for court API endpoints or services handling indictment
rg -A 5 "updateIndictment|createIndictment"
Length of output: 67004
Script:
#!/bin/bash
# Let's search for court client service and interfaces to check email handling
# Search for court client service
rg -A 10 "class.*CourtClient" libs/judicial-system/court-client/src/lib/courtClient.service.ts
# Search for interfaces or types related to indictment updates in court
rg -A 10 "interface.*UpdateIndictment|type.*UpdateIndictment" libs/judicial-system/types/src/lib/
# Search for prosecutor type definitions
rg -A 5 "interface.*Prosecutor|type.*Prosecutor" libs/judicial-system/types/src/lib/
Length of output: 1717
Script:
#!/bin/bash
# Let's check the court client service implementation for email handling
rg -A 10 "updateCaseWithProsecutor|updateIndictmentCase" libs/judicial-system/court-client/src/lib/
# Check for email-related types and interfaces
rg -A 5 "email.*:.*string" libs/judicial-system/types/src/lib/
# Check for prosecutor-related types in court client
rg -A 5 "prosecutor.*:.*{" libs/judicial-system/court-client/src/lib/
Length of output: 7642
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #17184 +/- ##
==========================================
- Coverage 35.76% 35.76% -0.01%
==========================================
Files 6931 6931
Lines 147974 147965 -9
Branches 42171 42167 -4
==========================================
- Hits 52930 52925 -5
+ Misses 95044 95040 -4
Flags with carried forward coverage won't be shown. Click here to find out more.
... and 4 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ✅ 11 Total Test Services: 0 Failed, 10 Passed Test ServicesThis report shows up to 10 services
|
Upplýsingapóstur - roboti. Taka út bandstrik í löke nr.
Upplýsingapóstur - roboti. Senda netfang sækjanda með nafni á robottann með upplýsingum um ákæru
What
Add prosecutors email address to new indictment robot mail info
Remove dashes from police case number that is sent to courts, both via robot and API
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes
Tests