chore(j-s): Prison users access to appeal case files #15844
Conversation
WalkthroughThe changes involve significant modifications to various components within the judicial system's backend. Key updates include the replacement of interceptors in the Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (3)
apps/judicial-system/backend/src/app/modules/case/interceptors/completedAppealAccessed.interceptor.ts (1)
Line range hint
23-50: Consider enhancing type safety and code organization.While the implementation is correct, there are opportunities to improve type safety and code organization:
- Consider extracting the condition for logging the event into a separate method for better readability.
- Use type guards to narrow down the user type and avoid type assertions.
- Consider using an enum for user roles instead of string literals.
Here's a suggested refactoring:
import { Observable } from 'rxjs' import { map } from 'rxjs/operators' import { CallHandler, ExecutionContext, Injectable, NestInterceptor, } from '@nestjs/common' import { CaseAppealState, EventType, InstitutionType, User, UserRole, } from '@island.is/judicial-system/types' import { EventLogService } from '../../event-log' import { Case } from '../models/case.model' @Injectable() export class CompletedAppealAccessedInterceptor implements NestInterceptor { constructor(private readonly eventLogService: EventLogService) {} intercept(context: ExecutionContext, next: CallHandler): Observable<Case> { const request = context.switchToHttp().getRequest() const user = request.user as User return next.handle().pipe( map((data: Case) => { if (this.shouldLogEvent(data, user)) { this.logEvent(data, user) } return data }), ) } private shouldLogEvent(data: Case, user: User): boolean { return ( data.appealState === CaseAppealState.COMPLETED && (this.isLegalUser(user) || this.isPrisonStaff(user)) ) } private isLegalUser(user: User): boolean { return [UserRole.PROSECUTOR, UserRole.DEFENDER].includes(user.role) } private isPrisonStaff(user: User): boolean { return ( user.role === UserRole.PRISON_SYSTEM_STAFF && user.institution?.type === InstitutionType.PRISON ) } private logEvent(data: Case, user: User): void { this.eventLogService.create({ eventType: EventType.APPEAL_RESULT_ACCESSED, caseId: data.id, nationalId: user.nationalId, userRole: user.role, }) } }This refactoring improves readability, maintainability, and type safety while preserving the original functionality.
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (2)
69-73: Approved with suggestion: Prison system user access conditionThe updated condition for prison system users is more flexible and robust. It now checks against multiple categories and ensures
caseFile.categoryis defined before the inclusion check.Consider reordering the conditions for better performance:
if (isPrisonSystemUser(user) && caseFile.category) { if ( - isCompletedCase(theCase.state) && - prisonSystemCaseFileCategories.includes(caseFile.category) + prisonSystemCaseFileCategories.includes(caseFile.category) && + isCompletedCase(theCase.state) ) { return true } }This change puts the more specific condition first, potentially avoiding unnecessary checks of
isCompletedCasewhen the category doesn't match.
Line range hint
1-80: Suggestions for improving code organization and testabilityThe
LimitedAccessViewCaseFileGuardis well-structured, but consider the following improvements:
- Extract the role-specific logic into separate methods for better readability and testability.
- Use dependency injection for the imported constants to make the guard more flexible and easier to test.
- Consider adding unit tests for this guard to ensure all conditions are correctly handled.
Here's an example of how you could refactor the
canActivatemethod:@Injectable() export class LimitedAccessViewCaseFileGuard implements CanActivate { constructor( @Inject('DEFENDER_CATEGORIES_RESTRICTION') private defenderCategoriesRestriction: CaseFileCategory[], @Inject('DEFENDER_CATEGORIES_INDICTMENT') private defenderCategoriesIndictment: CaseFileCategory[], @Inject('PRISON_SYSTEM_CATEGORIES') private prisonSystemCategories: CaseFileCategory[], ) {} canActivate(context: ExecutionContext): boolean { const { user, theCase, caseFile } = this.extractRequestData(context); if (isDefenceUser(user) && caseFile.category) { return this.checkDefenceUserAccess(user, theCase, caseFile); } if (isPrisonSystemUser(user) && caseFile.category) { return this.checkPrisonSystemUserAccess(user, theCase, caseFile); } throw new ForbiddenException(`Forbidden for ${user.role}`); } private extractRequestData(context: ExecutionContext): { user: User; theCase: Case; caseFile: CaseFile } { const request = context.switchToHttp().getRequest(); const user: User = request.user; const theCase: Case = request.case; const caseFile: CaseFile = request.caseFile; if (!user) throw new InternalServerErrorException('Missing user'); if (!theCase) throw new InternalServerErrorException('Missing case'); if (!caseFile) throw new InternalServerErrorException('Missing case file'); return { user, theCase, caseFile }; } private checkDefenceUserAccess(user: User, theCase: Case, caseFile: CaseFile): boolean { if ( isRequestCase(theCase.type) && isCompletedCase(theCase.state) && this.defenderCategoriesRestriction.includes(caseFile.category) ) { return true; } if ( isIndictmentCase(theCase.type) && this.defenderCategoriesIndictment.includes(caseFile.category) ) { return true; } return false; } private checkPrisonSystemUserAccess(user: User, theCase: Case, caseFile: CaseFile): boolean { return ( this.prisonSystemCategories.includes(caseFile.category) && isCompletedCase(theCase.state) ); } }This refactoring improves readability, testability, and maintainability of the guard.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (11)
- apps/judicial-system/backend/src/app/modules/case/case.controller.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/guards/limitedAccessCaseExists.guard.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/interceptors/completedAppealAccessed.interceptor.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/caseFileCategory.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (2 hunks)
- libs/judicial-system/types/src/index.ts (1 hunks)
- libs/judicial-system/types/src/lib/file.ts (1 hunks)
- libs/judicial-system/types/src/lib/user.ts (1 hunks)
Files skipped from review due to trivial changes (3)
- apps/judicial-system/backend/src/app/modules/case/guards/limitedAccessCaseExists.guard.ts
- libs/judicial-system/types/src/index.ts
- libs/judicial-system/types/src/lib/file.ts
Additional context used
Path-based instructions (8)
apps/judicial-system/backend/src/app/modules/file/guards/caseFileCategory.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/completedAppealAccessed.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/caseFile.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/file/guards/limitedAccessViewCaseFile.guard.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/user.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/case/limitedAccessCase.controller.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
apps/judicial-system/backend/src/app/modules/case/case.controller.ts (1)
Pattern
apps/**/*: "Confirm that the code adheres to the following:
- NextJS best practices, including file structure, API routes, and static generation methods.
- Efficient state management and server-side rendering techniques.
- Optimal use of TypeScript for component and utility type safety."
Additional comments not posted (12)
apps/judicial-system/backend/src/app/modules/file/guards/caseFileCategory.ts (1)
26-26: LGTM! Consider minor formatting adjustment.The addition of
prisonSystemCaseFileCategoriesaligns well with the PR objectives. It correctly restricts prison system access to only the appeal ruling, as specified in the requirements.For consistency with other constants in this file, consider adding a line break after the opening bracket:
-export const prisonSystemCaseFileCategories = [CaseFileCategory.APPEAL_RULING] +export const prisonSystemCaseFileCategories = [ + CaseFileCategory.APPEAL_RULING, +]Let's verify the usage of this new constant in other parts of the codebase:
Verification successful
Usage of
prisonSystemCaseFileCategoriesverified successfully.The constant
prisonSystemCaseFileCategoriesis correctly utilized in the codebase, aligning with the PR objectives. It is used in:
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.tsfor conditional logic.apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.tsas an import.These usages confirm its role in managing access to appeal rulings as intended.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of prisonSystemCaseFileCategories in the codebase # Test: Search for imports or usage of prisonSystemCaseFileCategories # Expect: At least one occurrence in a file other than the current one rg --type typescript -g '!**/caseFileCategory.ts' 'prisonSystemCaseFileCategories'Length of output: 117
Script:
#!/bin/bash # Description: Check for usage of prisonSystemCaseFileCategories in the codebase # Test: Search for imports or usage of prisonSystemCaseFileCategories # Expect: At least one occurrence in a file other than the current one rg --type-add 'ts:*.ts' -g '!**/caseFileCategory.ts' 'prisonSystemCaseFileCategories'Length of output: 530
apps/judicial-system/backend/src/app/modules/case/interceptors/completedAppealAccessed.interceptor.ts (1)
23-23: Improved class name for better clarity.The change from
CaseInterceptortoCompletedAppealAccessedInterceptorbetter reflects the specific purpose of this interceptor. This naming convention aligns well with the Single Responsibility Principle.apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.ts (3)
1-19: Imports look good.The necessary imports for RxJS, NestJS, and custom types are present and appropriate for the interceptor's functionality.
1-53: Overall, the interceptor implements the required functionality with room for improvement.The
CaseFileInterceptorsuccessfully filters case files based on user roles and appeal state, aligning with the PR objectives. The implementation ensures that prison admins (FMST) can access appeal rulings for completed appeals, while prison staff have no access to appeal case files.However, the suggested refactoring would improve code quality, readability, and maintainability. It's important to implement these changes carefully and verify that they don't introduce any regressions in the existing functionality.
After implementing the suggested changes, please run comprehensive tests to ensure that the access control logic works as expected for all user roles and appeal states.
21-53: Interceptor logic aligns with PR objectives, but consider some improvements.The interceptor successfully filters case files based on user roles and appeal state. However, there are a few points to consider:
- The logic for filtering case files could be simplified for better readability.
- Modifying the original data object directly might have unintended side effects.
- The
returnDatavariable is not consistently used throughout the method.- There's no error handling for potential undefined values.
Consider refactoring the interceptor logic as follows:
@Injectable() export class CaseFileInterceptor implements NestInterceptor { intercept(context: ExecutionContext, next: CallHandler): Observable<Case> { const request = context.switchToHttp().getRequest() const user: User = request.user return next.handle().pipe( map((data: Case) => { if (!data.caseFiles) { return data } if (isPrisonStaffUser(user) || data.appealState !== CaseAppealState.COMPLETED) { return { ...data, caseFiles: [] } } if (isPrisonSystemUser(user)) { return { ...data, caseFiles: data.caseFiles.filter( (cf) => cf.category === CaseFileCategory.APPEAL_RULING ), } } return data }) ) } }This refactored version:
- Simplifies the logic and improves readability.
- Creates a new object instead of modifying the original data.
- Consistently returns a new or modified object.
- Adds a check for undefined
caseFiles.To ensure this refactoring doesn't break existing functionality, please run the following test:
This will help us understand how the interceptor is used and ensure our changes don't introduce any regressions.
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (1)
24-24: LGTM: Import of prisonSystemCaseFileCategoriesThe addition of
prisonSystemCaseFileCategoriesimport is appropriate and consistent with the existing import pattern for case file categories.libs/judicial-system/types/src/lib/user.ts (1)
117-122: LGTM! Consider adding a clarifying comment.The implementation of
isPrisonStaffUserlooks good and is consistent with other user role check functions in the file. It's correctly exported and uses TypeScript for parameter typing, making it reusable across different NextJS apps.Consider adding a comment to clarify the difference between
isPrisonStaffUserandisPrisonSystemUser. For example:// Checks if the user is a prison staff member (excludes prison admin) export const isPrisonStaffUser = (user: InstitutionUser): boolean => Boolean( user.role && prisonSystemRoles.includes(user.role) && user.institution?.type === InstitutionType.PRISON, )Let's verify the usage of this new function in the codebase:
Verification successful
Function
isPrisonStaffUseris actively used in the codebase.The
isPrisonStaffUserfunction is integrated and utilized in multiple parts of the codebase, confirming its role in determining user roles related to prison staff. It is used in the following locations:
libs/judicial-system/types/src/index.tsapps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.tsapps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for usage of isPrisonStaffUser function # Expected: No results if the function is not yet used rg --type typescript "isPrisonStaffUser"Length of output: 75
Script:
#!/bin/bash # Description: Check for usage of isPrisonStaffUser function # Expected: No results if the function is not yet used rg --type ts "isPrisonStaffUser"Length of output: 537
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts (2)
56-57: New interceptors imported: Verify implementationsThe addition of
CaseFileInterceptorandCompletedAppealAccessedInterceptoraligns with the changes in the@UseInterceptorsdecorator. These custom interceptors likely enhance the access control for case files and completed appeals.Please ensure that the implementations of these interceptors in their respective files (
caseFile.interceptor.tsandcompletedAppealAccessed.interceptor.ts) align with the PR objectives, particularly regarding access rules for prison admins and staff.Verification successful
Interceptors Implementations Verified
The implementations of
CaseFileInterceptorandCompletedAppealAccessedInterceptoralign with the PR objectives. They effectively manage access control and event logging for case files and completed appeals, supporting the access rules for prison admins and staff.
CaseFileInterceptorclears or filters case files based on user roles and appeal states.CompletedAppealAccessedInterceptorlogs access events for completed appeals for specific user roles.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of the new interceptors # Test: Check the content of the new interceptor files echo "Checking CaseFileInterceptor implementation:" cat $(fd --type f --glob '**/caseFile.interceptor.ts') echo "\nChecking CompletedAppealAccessedInterceptor implementation:" cat $(fd --type f --glob '**/completedAppealAccessed.interceptor.ts')Length of output: 3154
89-89: Updated interceptors: Verify access control implementationThe
@UseInterceptorsdecorator has been updated to useCompletedAppealAccessedInterceptorandCaseFileInterceptorinstead of the previousCaseInterceptor. This change appears to implement more specific access control mechanisms.Please verify that this change correctly implements the access rules described in the PR objectives:
- Prison admins (FMST) should have access to view appeal rulings only for completed appeals.
- Prison staff should not have access to any appeal case files.
Run the following test to check the implementation:
If the test doesn't return any results, consider implementing explicit role checks in the
getByIdmethod or ensure that the interceptors handle this logic appropriately.Verification successful
Interceptors Implement Access Control Correctly
The
CompletedAppealAccessedInterceptorandCaseFileInterceptoreffectively enforce the access control rules as intended. The interceptors ensure that:
- Prison admins (FMST) can view appeal rulings only for completed appeals.
- Prison staff do not have access to any appeal case files.
These implementations align with the PR objectives and correctly handle the specified access control logic.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the implementation of access control in the getById method # Test: Check for role-based access control in the getById method echo "Checking role-based access control in getById method:" ast-grep --lang typescript --pattern 'getById($$$) { $$$ if ($user.role === "PRISON_ADMIN") { $$$ } else if ($user.role === "PRISON_STAFF") { $$$ } $$$ }' apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.tsLength of output: 929
Script:
#!/bin/bash # Description: Verify the implementation of access control in the getById method by checking for individual role checks # Test: Check for PRISON_ADMIN role access control in the getById method echo "Checking for PRISON_ADMIN role access control in getById method:" ast-grep --lang typescript --pattern 'if ($user.role === "PRISON_ADMIN") { $$$ }' apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts # Test: Check for PRISON_STAFF role access control in the getById method echo "Checking for PRISON_STAFF role access control in getById method:" ast-grep --lang typescript --pattern 'if ($user.role === "PRISON_STAFF") { $$$ }' apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.tsLength of output: 609
Script:
#!/bin/bash # Description: Locate and examine the implementation of CompletedAppealAccessedInterceptor and CaseFileInterceptor # Find the implementation of CompletedAppealAccessedInterceptor echo "Searching for CompletedAppealAccessedInterceptor implementation:" ast-grep --lang typescript --pattern 'class CompletedAppealAccessedInterceptor { $$$ }' # Find the implementation of CaseFileInterceptor echo "Searching for CaseFileInterceptor implementation:" ast-grep --lang typescript --pattern 'class CaseFileInterceptor { $$$ }'Length of output: 412
Script:
#!/bin/bash # Description: Search for any files related to interceptors in the codebase # Find files that might contain interceptor implementations echo "Searching for interceptor-related files:" fd interceptor --type fLength of output: 1654
Script:
#!/bin/bash # Description: Display the contents of the interceptors to verify access control logic # Display the contents of completedAppealAccessed.interceptor.ts echo "Contents of completedAppealAccessed.interceptor.ts:" cat apps/judicial-system/backend/src/app/modules/case/interceptors/completedAppealAccessed.interceptor.ts # Display the contents of caseFile.interceptor.ts echo "Contents of caseFile.interceptor.ts:" cat apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.tsLength of output: 3001
apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.service.ts (1)
19-22: Verify usage of new imports and consider adding comments.New imports have been added for prison-related user checks and case file categories:
isPrisonStaffUserandisPrisonSystemUserfrom '@island.is/judicial-system/types'prisonSystemCaseFileCategoriesfrom '../file/guards/caseFileCategory'These imports suggest new functionality related to prison system users and case file categories. However, they are not used in the visible part of this file.
To ensure these imports are necessary and used correctly, please run the following script:
If these imports are used in this file or closely related files, consider adding comments to explain their purpose and how they relate to the changes in prison user access to appeal case files mentioned in the PR objectives.
Also applies to: 43-43
Verification successful
Imports are used correctly across the codebase.
The imports
isPrisonStaffUser,isPrisonSystemUser, andprisonSystemCaseFileCategoriesare utilized in multiple files, confirming their necessity and relevance to the functionality related to prison system users and case file categories.
isPrisonStaffUserandisPrisonSystemUserare used in:
limitedAccessCase.service.tsviewCaseFile.guard.tslimitedAccessViewCaseFile.guard.tscase.filter.tscases.filter.tscaseFile.interceptor.ts
prisonSystemCaseFileCategoriesis used in:
limitedAccessViewCaseFile.guard.ts- Defined in
caseFileCategory.tsConsider adding comments to these imports to clarify their purpose and how they relate to the changes in prison user access to appeal case files.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of new imports in the codebase # Test 1: Check for usage of isPrisonStaffUser and isPrisonSystemUser echo "Checking usage of isPrisonStaffUser and isPrisonSystemUser:" rg --type typescript "isPrisonStaffUser|isPrisonSystemUser" apps/judicial-system/backend/src # Test 2: Check for usage of prisonSystemCaseFileCategories echo "Checking usage of prisonSystemCaseFileCategories:" rg --type typescript "prisonSystemCaseFileCategories" apps/judicial-system/backend/srcLength of output: 489
Script:
#!/bin/bash # Description: Verify the usage of new imports in the codebase with corrected file type # Test 1: Check for usage of isPrisonStaffUser and isPrisonSystemUser echo "Checking usage of isPrisonStaffUser and isPrisonSystemUser:" rg --type ts "isPrisonStaffUser|isPrisonSystemUser" apps/judicial-system/backend/src # Test 2: Check for usage of prisonSystemCaseFileCategories echo "Checking usage of prisonSystemCaseFileCategories:" rg --type ts "prisonSystemCaseFileCategories" apps/judicial-system/backend/srcLength of output: 2563
apps/judicial-system/backend/src/app/modules/case/case.controller.ts (2)
103-103: New import added for CompletedAppealAccessedInterceptor.The import statement for
CompletedAppealAccessedInterceptorhas been added, which aligns with the changes in thegetByIdmethod.
471-471: Interceptor changed for getById method.The
CaseInterceptorhas been replaced withCompletedAppealAccessedInterceptorfor thegetByIdmethod. This change appears to be related to the new functionality for allowing prison users to access appeal case files under specific conditions.To ensure that the new interceptor correctly implements the access rules described in the PR objectives, we need to review its implementation. Let's check the contents of the
CompletedAppealAccessedInterceptor:Verification successful
Interceptor Implementation Verified
The
CompletedAppealAccessedInterceptorclass is implemented in the codebase and includes logic related to handling completed appeal cases. This aligns with the review comment's description of its purpose. The interceptor checks if theappealStateof aCaseisCOMPLETED, which suggests it is implementing the intended access rules.
- Location:
apps/judicial-system/backend/src/app/modules/case/interceptors/completedAppealAccessed.interceptor.tsScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for the CompletedAppealAccessedInterceptor implementation ast-grep --lang typescript --pattern 'class CompletedAppealAccessedInterceptor { $$$ }'Length of output: 91
Script:
#!/bin/bash # Search for the CompletedAppealAccessedInterceptor class definition in the codebase rg 'class CompletedAppealAccessedInterceptor' --type=ts -A 10Length of output: 1634
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #15844 +/- ##
=======================================
Coverage 36.93% 36.93%
=======================================
Files 6681 6682 +1
Lines 136472 136505 +33
Branches 38756 38769 +13
=======================================
+ Hits 50401 50415 +14
- Misses 86071 86090 +19
Flags with carried forward coverage won't be shown. Click here to find out more.
Continue to review full report in Codecov by Sentry.
|
Datadog ReportAll test runs ❌ 14 Total Test Services: 1 Failed, 13 Passed Test ServicesThis report shows up to 10 services
❌ Failed Tests (16)
🔻 Code Coverage Decreases vs Default Branch (5) |
apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.ts
Show resolved
Hide resolved
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (7)
- apps/judicial-system/backend/src/app/modules/case/case.controller.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts (2 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/caseFileCategory.ts (1 hunks)
- apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (3 hunks)
- libs/judicial-system/types/src/index.ts (1 hunks)
- libs/judicial-system/types/src/lib/user.ts (1 hunks)
Files skipped from review as they are similar to previous changes (5)
- apps/judicial-system/backend/src/app/modules/case/case.controller.ts
- apps/judicial-system/backend/src/app/modules/case/interceptors/caseFile.interceptor.ts
- apps/judicial-system/backend/src/app/modules/case/limitedAccessCase.controller.ts
- apps/judicial-system/backend/src/app/modules/file/guards/caseFileCategory.ts
- libs/judicial-system/types/src/index.ts
Additional context used
Path-based instructions (2)
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.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/user.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."
Additional comments not posted (3)
apps/judicial-system/backend/src/app/modules/file/guards/limitedAccessViewCaseFile.guard.ts (1)
68-74: LGTM!The code changes implement the access control logic for prison admin users correctly, addressing the suggestions from the previous review.
libs/judicial-system/types/src/lib/user.ts (2)
117-122: LGTM!The code changes are approved. The
isPrisonAdminUserfunction follows the existing pattern of user role validation functions and correctly checks if a user is a prison admin.
124-129: LGTM!The code changes are approved. The
isPrisonStaffUserfunction follows the existing pattern of user role validation functions and correctly checks if a user is a prison staff.
gudjong
added
the
deprecated:automerge
…d.is into j-s/limited-access-files
oddsson
removed
the
deprecated:automerge
oddsson
added
the
deprecated:automerge
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts (1 hunks)
Additional context used
Path-based instructions (1)
apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.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 not posted (5)
apps/judicial-system/backend/src/app/modules/file/guards/test/limitedAccessViewCaseFileGuard.spec.ts (5)
Line range hint
1-1: Overall structure and setup are appropriate for the test suite.The use of Jest for mocking and the
givenWhenThenpattern are well-suited for testing the guard functionality. The setup is clean and follows good practices in behavior-driven development (BDD).
Line range hint
1-1: Comprehensive test coverage for defence users.The parameterized tests for defence users are thorough, covering both allowed and disallowed case file categories across various case types and states. This ensures robust testing of guard conditions.
Line range hint
1-1: Test case for prison system users aligns with PR objectives.The focus on testing access rights for prison admins, specifically for the
APPEAL_RULINGcategory, is crucial and aligns with the PR's goal of restricting access to sensitive information. The tests are well-structured to ensure that security measures are correctly enforced.
Line range hint
1-1: Robust test coverage for remaining users.The tests comprehensively cover various roles and institutions, ensuring that access controls are strictly enforced. This is crucial for the security and integrity of the system.
Line range hint
1-1: Important tests for error handling in cases of missing data.The tests for scenarios where critical information is missing are crucial. They ensure that the system throws appropriate exceptions, which is vital for robust error handling and security.
* Create CaseFileInterceptor * Filter case files in interceptor * Only send appeal ruling for completed appeals * Refactor * Remove unused code * Refactor * Allow defence users to see all files * Refactor * Fix tests --------- Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com>
Prison users access to appeal case files
Asana
What
This PR implements access for prison users to appeal case files. The following rules apply
Checklist:
Summary by CodeRabbit
New Features
Bug Fixes