feat(j-s): Send to prison admin #16831
Conversation
WalkthroughThe pull request introduces several enhancements to the judicial system's backend and frontend components. Key changes include the addition of new fields related to defendants, specifically concerning their status with the prison administration. New models, migrations, and GraphQL schema updates are implemented to support these features. Additionally, various components and services are updated to integrate event logging and improve role-based access control. This update aims to streamline the handling of defendant information and enhance the overall functionality of the judicial system application. Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
Datadog ReportAll test runs ✅ 9 Total Test Services: 0 Failed, 9 Passed Test Services
🔻 Code Coverage Decreases vs Default Branch (2) |
![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 (9)
apps/judicial-system/backend/src/app/modules/defendant/index.ts (1)
5-5: Consider grouping related exports together.The new
DefendantEventLogexport should be grouped with other defendant-related exports for better code organization and maintainability.Consider this reorganization:
export { Defendant } from './models/defendant.model' +export { DefendantEventLog } from './models/defendantEventLog.model' export { DefendantService } from './defendant.service' export { DefendantExistsGuard } from './guards/defendantExists.guard' export { CurrentDefendant } from './guards/defendant.decorator' -export { DefendantEventLog } from './models/defendantEventLog.model' export { CivilClaimant } from './models/civilClaimant.model' export { CivilClaimantService } from './civilClaimant.service'libs/judicial-system/types/src/lib/eventLog.ts (1)
15-20: Consider documenting the event typesSince these event types are used across different parts of the judicial system, it would be helpful to add JSDoc comments explaining when each event type is triggered and its significance.
Example documentation:
export enum DefendantEventType { + /** Logged when a case is sent to prison administration for early sentence processing */ SENT_TO_PRISON_ADMIN = 'SENT_TO_PRISON_ADMIN', }apps/judicial-system/backend/src/app/modules/defendant/models/defendantEventLog.model.ts (2)
57-64: Consider adding a default value for the eventType enum.While the column configuration is correct, consider adding a default value to ensure consistent behavior during record creation.
@Column({ type: DataType.ENUM, allowNull: false, values: Object.values(DefendantEventType), + defaultValue: DefendantEventType.SENT_TO_PRISON_ADMIN, }) @ApiProperty({ enum: DefendantEventType }) eventType!: DefendantEventType
1-64: Consider adding indexes for performance optimization.Given that this model will be queried to find the latest prison admin status, consider adding indexes to improve query performance.
Add the following to your migration file:
- Index on
(defendantId, eventType)for efficient status lookups- Index on
(caseId, eventType)if you need to query by caseapps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts (2)
Line range hint
26-28: Document the security review process for new fieldsThe warning comment highlights the importance of careful consideration when exposing data. Consider enhancing this comment to:
- Document the security review process for adding new fields
- List which fields are considered sensitive
- Reference the relevant security policies
Line range hint
20-71: Consider paginating the case list responseThe interceptor transforms the entire cases array at once. Given that this endpoint returns a list of cases with substantial data per case, consider implementing pagination to:
- Improve performance for large datasets
- Reduce memory usage
- Enhance response times
apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts (1)
204-206: Well-structured relationship definition!The HasMany relationship to DefendantEventLog is correctly implemented and follows best practices by allowing efficient access to event logs through the model relationship instead of direct database queries.
This implementation improves performance by enabling eager loading of event logs when needed, reducing the number of database queries.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
43-43: Consider adding validation for the new fieldThe
defendantWaivesRightToCounselfield is added to track whether a defendant has waived their right to counsel. Consider adding validation to ensure this field can only be set under appropriate conditions.Consider adding a validation check:
if (update.defendantWaivesRightToCounsel !== undefined) { // Validate that this can only be set in appropriate case states if (![CaseState.NEW, CaseState.DRAFT].includes(theCase.state)) { throw new BadRequestException('Cannot modify right to counsel waiver in current case state') } }
294-301: Consider adding an index on eventTypeThe query filters on
eventTypecolumn. Adding an index would improve query performance, especially as the number of event logs grows.Consider adding an index in your database migration:
// In your migration file await queryInterface.addIndex('defendant_event_log', ['event_type'], { name: 'idx_defendant_event_log_event_type' });
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
apps/judicial-system/backend/src/app/modules/case/case.service.ts(3 hunks)apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts(2 hunks)apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts(3 hunks)apps/judicial-system/backend/src/app/modules/defendant/defendant.module.ts(2 hunks)apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts(3 hunks)apps/judicial-system/backend/src/app/modules/defendant/index.ts(1 hunks)apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts(2 hunks)apps/judicial-system/backend/src/app/modules/defendant/models/defendantEventLog.model.ts(1 hunks)libs/judicial-system/types/src/index.ts(1 hunks)libs/judicial-system/types/src/lib/eventLog.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (4)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts
- apps/judicial-system/backend/src/app/modules/defendant/defendant.module.ts
- apps/judicial-system/backend/src/app/modules/defendant/defendant.service.ts
- libs/judicial-system/types/src/index.ts
🧰 Additional context used
📓 Path-based instructions (7)
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/interceptors/case.interceptor.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/interceptors/caseList.interceptor.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/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/defendant/models/defendant.model.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/models/defendantEventLog.model.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/eventLog.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 (4)
apps/judicial-system/backend/src/app/modules/case/case.service.ts (1)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/api/src/app/modules/case/models/defendantEventLog.model.ts:6-22
Timestamp: 2024-11-18T21:50:40.004Z
Learning: In `apps/judicial-system/api/src/app/modules`, new models should be created following the existing patterns used in the API to maintain consistency.
apps/judicial-system/backend/src/app/modules/defendant/index.ts (2)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/api/src/app/modules/case/models/defendantEventLog.model.ts:6-22
Timestamp: 2024-11-18T21:50:40.004Z
Learning: In `apps/judicial-system/api/src/app/modules`, new models should be created following the existing patterns used in the API to maintain consistency.
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts:0-0
Timestamp: 2024-11-19T09:04:44.520Z
Learning: Additional documentation is not required when the code is clear and self-explanatory.
apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts (1)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/api/src/app/modules/case/models/defendantEventLog.model.ts:6-22
Timestamp: 2024-11-18T21:50:40.004Z
Learning: In `apps/judicial-system/api/src/app/modules`, new models should be created following the existing patterns used in the API to maintain consistency.
apps/judicial-system/backend/src/app/modules/defendant/models/defendantEventLog.model.ts (2)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/api/src/app/modules/case/models/defendantEventLog.model.ts:6-22
Timestamp: 2024-11-18T21:50:40.004Z
Learning: In `apps/judicial-system/api/src/app/modules`, new models should be created following the existing patterns used in the API to maintain consistency.
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/backend/migrations/20241114123528-create-defendant-event-log.js:0-0
Timestamp: 2024-11-18T21:57:03.051Z
Learning: In the `apps/judicial-system/backend` codebase, validations are not typically included in Sequelize migration files.
🔇 Additional comments (12)
libs/judicial-system/types/src/lib/eventLog.ts (1)
16-18: LGTM!
The new DefendantEventType enum follows TypeScript best practices and maintains consistency with existing naming patterns.
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts (3)
10-10: LGTM: Imports are properly structured
The new imports for Defendant and DefendantEventLog are correctly added and align with the new prison admin functionality.
26-26: LGTM: Case transformation properly integrated
The integration of transformDefendants into the case transformation is clean and aligns with the PR objectives for prison admin functionality.
Line range hint 35-41: Verify interceptor behavior with transformed data
Please ensure that the interceptor correctly handles all edge cases with the new defendant transformation.
apps/judicial-system/backend/src/app/modules/defendant/models/defendantEventLog.model.ts (3)
1-17: LGTM! Well-organized imports.
The imports are properly organized, with clear separation between external dependencies and internal modules.
18-22: LGTM! Proper table configuration.
The table configuration follows best practices with appropriate naming convention and audit trail support through timestamps.
30-56: LGTM! Well-structured model properties.
The model properties are well-defined with appropriate types, constraints, and documentation.
apps/judicial-system/backend/src/app/modules/case/interceptors/caseList.interceptor.ts (1)
15-15: Verify the security implications of defendant data transformation
The change introduces a new way of processing defendant data. Given the PR's context about prison administration functionality:
- Ensure the
transformDefendantsfunction properly sanitizes sensitive prison-related information - Verify that the transformed data aligns with role-based access control requirements
Also applies to: 33-33
✅ Verification successful
Let's examine the implementation of transformDefendants more closely to understand its security implications.
Based on the code analysis, I can now provide a definitive response about the security implications of the defendant data transformation.
The defendant data transformation is safe and properly scoped
The transformDefendants function performs a simple and secure transformation that:
- Only exposes already existing defendant data through
toJSON() - Adds a single timestamp field
setnToPrisonAdminDatefrom event logs - Does not expose any sensitive prison-related information beyond what's already authorized
The transformation is used consistently in both the case and list interceptors, maintaining the same level of data access control across the application.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Analyze the transformDefendants implementation and its security implications
# Check the implementation of transformDefendants
ast-grep --pattern 'function transformDefendants($_) {
$$$
}'
# Look for security-related tests
rg -l "describe.*transformDefendants" -g "*.spec.ts"
# Check for role-based access control tests
rg "isSentToPrisonAdmin|transformDefendants" -g "*.(spec|test).ts" -A 5
Length of output: 343
Script:
#!/bin/bash
# Let's try a different approach to locate and analyze the transformDefendants function
# Find the file containing transformDefendants
rg -l "transformDefendants" --type ts
# Once found, examine its implementation and usage
rg "transformDefendants" --type ts -A 5
# Look for any security or access control related code nearby
rg "transformDefendants|security|access|permission|role" --type ts -C 5
Length of output: 103588
Script:
#!/bin/bash
# Let's look at the case.interceptor.ts file content to analyze the transformDefendants function
rg -A 10 "transformDefendants" apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts
# Check for any security-related tests
rg -l "transformDefendants.*test|test.*transformDefendants" -g "*.spec.ts"
# Look for DefendantEventLog usage to understand what data is being transformed
rg "DefendantEventLog" -A 5 apps/judicial-system/backend/src/app/modules/case
Length of output: 4984
apps/judicial-system/backend/src/app/modules/defendant/models/defendant.model.ts (2)
26-26: LGTM!
The import follows the established pattern for model imports.
200-202: LGTM!
The column definition follows the established pattern and correctly implements the "Send to prison admin" feature with appropriate type and decorators.
apps/judicial-system/backend/src/app/modules/case/case.service.ts (2)
43-43: LGTM: Import changes are well-organized
The new imports for defendant event types and models are properly organized and support the prison admin feature requirements.
Also applies to: 63-68
294-301: LGTM: Well-implemented event logs inclusion
The DefendantEventLog inclusion is properly implemented with performance considerations (separate: true) and correct ordering.
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/defendant/models/defendantEventLog.model.ts
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/case/filters/test/cases.filter.spec.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts (1)
14-26: Consider implementing event-driven updatesThe current implementation handles the prison admin status through data transformation. For a more scalable solution, consider:
- Implementing an event-driven system for status updates
- Adding a caching layer for frequently accessed defendant statuses
- Creating a dedicated service for prison admin-related operations
This would improve maintainability and prepare the system for future features mentioned in the PR objectives (e.g., document uploads).
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/judicial-system/backend/src/app/modules/case/filters/cases.filter.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts(1 hunks)libs/judicial-system/types/src/lib/eventLog.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- apps/judicial-system/backend/src/app/modules/case/filters/cases.filter.ts
- libs/judicial-system/types/src/lib/eventLog.ts
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.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/backend/src/app/modules/case/interceptors/case.interceptor.ts (2)
10-10: LGTM: Imports are correctly structured
The new imports for Defendant and DefendantEventLog models are necessary and properly organized.
26-26: LGTM: Proper integration of transformDefendants
The transformCase function correctly integrates the new transformDefendants function while maintaining its existing structure.
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts
Outdated
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts
Outdated
Show resolved
Hide resolved
…not been sent to prison admin
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (5)
apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts (2)
Line range hint
42-56: Consider extending test utilities for prison admin functionality.Given that this PR introduces "Send to prison admin" functionality, consider extending the
createTestUsersutility to include prison administrator roles and related test data.Example enhancement:
export const createTestUsers = ( roles: string[], ): Record< string, { id: string name: string email: string mobile: string nationalId: string + isPrisonAdmin?: boolean } > =>
Line range hint
127-129: Extend DefendantService mock for prison admin functionality.The
DefendantServicemock should be extended to include methods related to the new prison admin functionality, such as sending cases to prison admin and revoking them.Update the mock implementation:
{ provide: DefendantService, - useValue: { isDefendantInActiveCustody: jest.fn() }, + useValue: { + isDefendantInActiveCustody: jest.fn(), + sendToPrisonAdmin: jest.fn(), + revokePrisonAdmin: jest.fn(), + }, },apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts (1)
Missing test coverage for prison admin notifications
Based on the verification:
- The codebase has prison admin user filters and access controls implemented
- The
CaseNotificationTypeenum doesn't include any prison admin related notifications- No prison admin notification tests exist in the notification module
This confirms the review comment's concern about missing test coverage for prison admin functionality in the notification system.
Specific areas needing test coverage:
- Notification scenarios when a defendant is sent to prison admin (
isSentToPrisonAdmin: true)- Email notification content and recipients for prison admin cases
- Integration with prison admin access controls
🔗 Analysis chain
Line range hint
1-180: Ensure test coverage for new prison admin functionality.While these tests thoroughly cover defender assignment notifications, they don't include scenarios related to the new "Send to prison admin" feature mentioned in the PR objectives.
Consider organizing prison admin related tests in a separate test file like
prisonAdminNotifications.spec.tsto maintain clear separation of concerns.
Let me gather more information about the prison admin functionality to verify the review comment.
Let me check for the notification types and implementation to complete the verification.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check for other test files that might cover prison admin functionality fd -e spec.ts -e test.ts . | grep -i "prison" # Look for prison admin related types/interfaces ast-grep --pattern 'interface $name { $$$ isSentToPrisonAdmin?: boolean $$$ }'Length of output: 318
Script:
#!/bin/bash # Check content of prison admin related test files cat apps/judicial-system/backend/src/app/modules/case/filters/test/prisonAdminUserFilter.spec.ts cat apps/judicial-system/backend/src/app/modules/case/filters/test/prisonStaffUserFilter.spec.ts # Look for prison admin notification types ast-grep --pattern 'enum $_ { $$$ PRISON_$_ = $_, $$$ }' # Search for prison admin related notification code rg -l "prison.*admin.*notification" -g "*.ts"Length of output: 9300
Script:
#!/bin/bash # Check for notification types rg "enum.*CaseNotificationType" -A 10 # Check the implementation of prison admin notifications rg "PRISON_ADMIN.*notification" -A 5 -g "*.ts" # Look for any prison admin related code in notification module fd . "apps/judicial-system/backend/src/app/modules/notification" -t f -e ts | xargs rg -l "prison.*admin"Length of output: 2390
apps/judicial-system/backend/src/app/formatters/formatters.ts (2)
Line range hint
632-651: LGTM! Well-structured prison administration notification.The function properly handles both new and modified rulings, includes necessary context in notifications, and maintains consistent formatting patterns. The court name transformation from "dómur" to "dómi" ensures grammatically correct messages in the target language.
Consider implementing a reusable helper function for court name transformations, as this pattern appears in multiple places throughout the codebase.
Line range hint
1-1183: Well-structured formatting module with room for optimization.The code demonstrates good practices in terms of type safety and separation of concerns. However, there are opportunities for improvement:
- Consider extracting repeated string transformations (e.g., court name formatting, date formatting) into utility functions.
- The error messages and default values ('NONE') could be centralized in constants.
Example utility function for court name transformation:
const transformCourtName = (courtName?: string, format: 'domi' | 'doms' = 'domi'): string => { if (!courtName) return 'NONE'; const replacement = format === 'domi' ? 'dómi' : 'dóms'; return courtName.replace('dómur', replacement); };
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (11)
apps/judicial-system/backend/src/app/formatters/formatters.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/filters/case.filter.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/filters/test/cases.filter.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts(1 hunks)apps/judicial-system/backend/src/app/modules/defendant/test/civilClaimantController/update.spec.ts(0 hunks)apps/judicial-system/backend/src/app/modules/defendant/test/internalDefendantController/internalDefendantControllerGuards.spec.ts(0 hunks)apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts(0 hunks)apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts(1 hunks)apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts(1 hunks)apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts(2 hunks)apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/SendToPrisonAdmin/SendToPrisonAdmin.tsx(1 hunks)
💤 Files with no reviewable changes (3)
- apps/judicial-system/backend/src/app/modules/defendant/test/civilClaimantController/update.spec.ts
- apps/judicial-system/backend/src/app/modules/defendant/test/internalDefendantController/internalDefendantControllerGuards.spec.ts
- apps/judicial-system/backend/src/app/modules/notification/caseNotification.service.ts
✅ Files skipped from review due to trivial changes (1)
- apps/judicial-system/backend/src/app/modules/subpoena/subpoena.service.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- apps/judicial-system/backend/src/app/modules/case/filters/case.filter.ts
- apps/judicial-system/backend/src/app/modules/case/interceptors/case.interceptor.ts
- apps/judicial-system/web/src/routes/PublicProsecutor/Indictments/SendToPrisonAdmin/SendToPrisonAdmin.tsx
🧰 Additional context used
📓 Path-based instructions (4)
apps/judicial-system/backend/src/app/formatters/formatters.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/filters/test/cases.filter.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/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/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."
🔇 Additional comments (6)
apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts (2)
7-7: LGTM! Clean import optimization.
The removal of unused FormatMessage import while maintaining the necessary internationalization functionality through IntlService is a good cleanup.
Line range hint 156-182: Verify notification test coverage for prison admin events.
The testing context should ensure proper coverage of notifications related to prison admin events (sending/revoking cases).
✅ Verification successful
Test context properly handles prison admin notifications
The test context in createTestingNotificationModule.ts includes all necessary components for testing prison admin notifications:
- The module includes
NotificationService,CaseNotificationService, and other required notification services - Prison-related email configuration is properly handled through
notificationModuleConfig - The context exposes notification controllers and services needed for testing prison admin scenarios
Key evidence from codebase:
- Prison admin notifications are implemented in
CaseNotificationServicewith dedicated methods for:- Sending rulings to prison admin (
prisonAdminEmail) - Handling custody notifications (
prisonEmail) - Managing case revocations
- Sending rulings to prison admin (
- Test module provides all required dependencies including email service mocks and configuration
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for prison admin-related test coverage
# Search for prison admin-related test cases
rg -l "prison.*admin.*test" --type ts
# Check for notification handlers related to prison admin
ast-grep --pattern 'class $_ implements NotificationHandler {
$$$
handle($_, $_) {
$$$
prison
$$$
}
}'
Length of output: 162
Script:
#!/bin/bash
# Let's try a broader search for prison-related notifications and tests
# Search for prison-related notification tests
rg -l "prison.*notification" --type ts
# Search for notification handler implementations
ast-grep --pattern 'class $_ implements NotificationHandler'
# Check the test file content
cat "apps/judicial-system/backend/src/app/modules/notification/test/createTestingNotificationModule.ts"
# Look for prison-related notification types or interfaces
rg "prison" --type ts -C 2
Length of output: 94438
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts (1)
Line range hint 42-180: LGTM: Well-structured test cases.
The test cases are well organized with clear setup, execution, and verification phases. They properly test different scenarios of defender assignment notifications.
apps/judicial-system/backend/src/app/modules/case/filters/test/cases.filter.spec.ts (2)
Line range hint 1-585: LGTM! Well-structured test suite
The test file maintains a consistent pattern for testing role-based filters, with clear separation of concerns and explicit test cases for each user role. The prison admin filter test aligns well with this structure.
394-397: Verify test coverage for prison admin case visibility
The filter correctly checks for cases where is_sent_to_prison_admin = true, but consider adding additional test cases to verify:
- Cases that are not sent to prison admin are filtered out
- Edge cases when
is_sent_to_prison_adminis null or false
✅ Verification successful
Let me check the test files more thoroughly to verify the test coverage.
Test coverage for prison admin case visibility is comprehensive
The test coverage is complete with both positive and negative test cases:
- Tests verify that cases are not accessible when
is_sent_to_prison_adminis false or missing (defendants: [{}]) - Tests verify that cases are accessible when
is_sent_to_prison_adminis true (defendants: [{ isSentToPrisonAdmin: true }]) - Tests cover different case types, states, and decisions for prison admin access
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for comprehensive test coverage of prison admin filtering
# Look for related test files
fd -e spec.ts -e test.ts prison
# Check for test cases covering different states of is_sent_to_prison_admin
rg -A 10 "is_sent_to_prison_admin" -g "*.spec.ts" -g "*.test.ts"
Length of output: 1397
Script:
#!/bin/bash
# Check content of prison admin filter test files
cat apps/judicial-system/backend/src/app/modules/case/filters/test/prisonAdminUserFilter.spec.ts
cat apps/judicial-system/backend/src/app/modules/case/filters/test/prisonStaffUserFilter.spec.ts
# Look for test cases with different states of is_sent_to_prison_admin
rg -B 5 -A 15 "describe.*prison" -g "*.spec.ts"
Length of output: 23983
apps/judicial-system/backend/src/app/formatters/formatters.ts (1)
16-16: LGTM! Clean import statement.
The import statement has been simplified to include only the necessary types (Gender and UserRole), improving code maintainability.
...s/notification/test/internalNotificationController/sendAdvocateAssignedNotifications.spec.ts
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 (1)
apps/judicial-system/backend/src/app/modules/defendant/test/createTestingDefendantModule.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
apps/judicial-system/backend/src/app/modules/defendant/test/createTestingDefendantModule.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/defendant/test/createTestingDefendantModule.ts (1)
Learnt from: oddsson
PR: island-is/island.is#16831
File: apps/judicial-system/api/src/app/modules/case/models/defendantEventLog.model.ts:6-22
Timestamp: 2024-11-18T21:50:40.004Z
Learning: In `apps/judicial-system/api/src/app/modules`, new models should be created following the existing patterns used in the API to maintain consistency.
🔇 Additional comments (1)
apps/judicial-system/backend/src/app/modules/defendant/test/createTestingDefendantModule.ts (1)
23-23: LGTM!
The import follows the established pattern for model imports in the codebase.
apps/judicial-system/backend/src/app/modules/defendant/test/createTestingDefendantModule.ts
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
apps/judicial-system/backend/src/app/modules/file/test/fileController/createCaseFileRolesRules.spec.ts (1)
Line range hint
26-35: Consider adding negative test cases.While the current test verifies the presence of authorized roles, consider adding test cases to ensure that unauthorized roles cannot access the file creation functionality. This would provide more comprehensive test coverage.
Example addition:
it('should not give permission to unauthorized roles', () => { const unauthorizedRole = { name: 'unauthorized' } expect(rules).not.toContain(unauthorizedRole) })apps/judicial-system/backend/src/app/modules/file/test/fileController/createPresignedPostRolesRules.spec.ts (1)
Line range hint
14-36: Consider adding specific test cases for prison admin functionality.While the basic role permission is tested, consider adding test cases that specifically validate the permissions related to the "Send to prison admin" feature, such as:
- Access to prison admin related files
- Permissions for sending/revoking cases to prison administrators
Would you like me to help create these additional test cases?
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts (1)
Line range hint
1-350: Consider adding test cases for new prison admin functionalityGiven that this PR introduces the "Send to prison admin" feature, consider adding test cases for:
- Sending cases to prison admin
- Revoking cases from prison admin
- Edge cases around the timing of sending/revoking
Additionally, the test suite would benefit from:
- Testing error scenarios (e.g., failed email delivery)
- Validating email content for prison admin notifications
- Testing concurrent modifications to case status
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (3)
apps/judicial-system/backend/src/app/modules/file/test/fileController/createCaseFileRolesRules.spec.ts(3 hunks)apps/judicial-system/backend/src/app/modules/file/test/fileController/createPresignedPostRolesRules.spec.ts(3 hunks)apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts(2 hunks)
🧰 Additional context used
📓 Path-based instructions (3)
apps/judicial-system/backend/src/app/modules/file/test/fileController/createCaseFileRolesRules.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/file/test/fileController/createPresignedPostRolesRules.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."
📓 Learnings (2)
apps/judicial-system/backend/src/app/modules/file/test/fileController/createCaseFileRolesRules.spec.ts (1)
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCaseFilesRecordPdfRolesRules.spec.ts:1-1
Timestamp: 2024-11-12T15:15:20.157Z
Learning: The usage of `defenderRule` in the following test files is intentional and should remain unchanged:
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getRulingPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getRequestPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCourtRecordPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getByIdRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getAllFilesZipRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/caseController/getAllRolesRules.spec.ts`
apps/judicial-system/backend/src/app/modules/file/test/fileController/createPresignedPostRolesRules.spec.ts (1)
Learnt from: gudjong
PR: island-is/island.is#16389
File: apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCaseFilesRecordPdfRolesRules.spec.ts:1-1
Timestamp: 2024-11-12T15:15:20.157Z
Learning: The usage of `defenderRule` in the following test files is intentional and should remain unchanged:
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createPresignedPostRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/deleteCaseFileRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/createCaseFileRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/file/test/limitedAccessFileController/getCaseFileSignedUrlRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getRulingPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getRequestPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getCourtRecordPdfRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getByIdRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/limitedAccessCaseController/getAllFilesZipRolesRules.spec.ts`
- `apps/judicial-system/backend/src/app/modules/case/test/caseController/getAllRolesRules.spec.ts`
🔇 Additional comments (4)
apps/judicial-system/backend/src/app/modules/file/test/fileController/createCaseFileRolesRules.spec.ts (1)
10-10: LGTM! The role addition follows best practices.
The addition of publicProsecutorStaffRule and its corresponding test expectations are well-structured and align with the PR objectives for the "Send to prison admin" feature.
Also applies to: 26-26, 35-35
apps/judicial-system/backend/src/app/modules/file/test/fileController/createPresignedPostRolesRules.spec.ts (1)
10-10: LGTM! Verify role scope alignment with requirements.
The addition of publicProsecutorStaffRule and its test assertions are correctly implemented. The changes align with the PR's objective of enabling public prosecutor staff to manage prison admin cases.
Let's verify if this role is consistently applied across related endpoints:
Also applies to: 26-26, 35-35
✅ Verification successful
The search results show that publicProsecutorStaffRule is consistently applied across all relevant file-related endpoints in both the main controllers (file.controller.ts, limitedAccessFile.controller.ts) and their corresponding test files. The role is also properly integrated into other related modules like case, defendant, and subpoena controllers, indicating a comprehensive implementation.
Role scope is properly aligned with requirements
The publicProsecutorStaffRule is correctly applied to:
- File operations (create, get signed URLs)
- Case management endpoints
- Defendant-related operations
- Subpoena handling
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if publicProsecutorStaffRule is consistently applied across related controllers
# Expected: Find all endpoints where this role should be applied for the prison admin feature
# Search for other controller files that might need this role
rg -l "createPresignedPost|getCaseFileSignedUrl|createCaseFile" apps/judicial-system/backend/src/app/modules
# Check existing usage of the role
rg "publicProsecutorStaffRule" apps/judicial-system/backend/src/app
Length of output: 7196
apps/judicial-system/backend/src/app/modules/notification/test/internalNotificationController/sendRulingNotifications.spec.ts (2)
178-178:
Confirm notification behavior for modified rulings
Similar to the restriction case, the test for modified rulings now expects one email instead of two. This change should be validated against the notification requirements for modified rulings.
144-144:
Verify the reduction in email notifications
The test now expects only one email to be sent to the prosecutor instead of two. Please verify that this change aligns with the business requirements and doesn't accidentally drop important notifications.
gudjong
added
the
deprecated:automerge
* Add a send to fmst button that routes to a page * Remove unused code * Add UI * Create event log when indictment is sent to FMSt * Create event log when indictment is sent to FMSt * Create event log when indictment is sent to FMSt * Refactor * Refactor * Refactor * Updates FMST queries * Refactor * Refactor * Refactor * Refactor * Add DefendantEventLog table * Refactor * Send sentToPrisonAdminDate to client * Show sent to prison admin date on info cards * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Frontend: Upload file to prison admin * Frontend: Upload file to prison admin * Add SentToPrisonAdmin tag in cases list for PP * Merge * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Refactor * Remove caseId from defendant event log table * Remove caseId from defendant event log table * Revert * Remove unused code * Refactor * Removes unused event type * Removes the defendant event log from the api * Moves defendant event log to defendant model * Fixes backend case retrieval * Updates unit test * Sets the sent to prison admin date to undefined it the defendant has not been sent to prison admin * Fixes linting issues * Updates unit tests * Updates unit tests * Updates unit tests * Updates unit tests --------- Co-authored-by: Guðjón Guðjónsson <[email protected]> Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Send to prison admin
Asana
What
This is the first draft of our "Send to prison admin" functionality. In this first version, public prosecutor staff can send (and revoke) cases to/from prison admins.
What comes later is being able to upload a document that commonly is sent with these types of requests.
Why
Sometimes, defendants want to start their sentence earlier. This change enables PP to do so.
Screenshots / Gifs
Screen.Recording.2024-11-18.at.14.34.46.mov
Checklist:
Summary by CodeRabbit
Release Notes
New Features
isSentToPrisonAdminandsentToPrisonAdminDateto the defendant and case models, enhancing tracking of defendants sent to prison administration.DefendantEventLogmodel to log events related to defendants.Bug Fixes
Documentation
Style
Tests