Conversation
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| const paymentAdminVerifyAccess = jest.fn().mockResolvedValue(undefined); | ||
| const engagementApproverVerifyAccess = jest | ||
| .fn() | ||
| .mockRejectedValue(new Error('should not be called')); |
There was a problem hiding this comment.
[💡 readability]
Using mockRejectedValue with an error message like 'should not be called' is a bit misleading. Consider using a more descriptive message or a different approach to ensure clarity in test failures.
|
|
||
| private normalizeRoles(roles: string[] = []): Set<string> { | ||
| return new Set( | ||
| (roles || []).map((role) => role?.trim().toLowerCase()).filter(Boolean), |
There was a problem hiding this comment.
[maintainability]
The use of role?.trim().toLowerCase() is repeated in multiple places. Consider extracting this logic into a helper function to improve maintainability and reduce duplication.
| normalizedRoles: Set<string>, | ||
| ): boolean { | ||
| return ( | ||
| providerRole === Role.EngagementPaymentApprover.trim().toLowerCase() && |
There was a problem hiding this comment.
[❗❗ correctness]
Ensure that Role.EngagementPaymentApprover and Role.PaymentAdmin are always defined and not null or undefined to avoid potential runtime errors.
| } | ||
|
|
||
| async verifyAccess(resourceId: string, userId: string, roles: string[] = []) { | ||
| const normalizedRoles = this.normalizeRoles(roles); |
There was a problem hiding this comment.
[correctness]
The normalizeRoles function is called but its implementation is not shown in the diff. Ensure that it correctly normalizes roles to prevent potential mismatches or errors in access verification.
| const normalizedRole = r?.trim().toLowerCase(); | ||
| if ( | ||
| !normalizedRole || | ||
| this.shouldSkipProvider(normalizedRole, normalizedRoles) |
There was a problem hiding this comment.
[❗❗ correctness]
The shouldSkipProvider function is used to determine if a provider should be skipped. Ensure that this function is well-tested to avoid inadvertently skipping necessary providers, which could lead to incorrect access control.
1 participant
No description provided.