Referral Tracking API added#237
Conversation
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several new referral-related API endpoints to the apiConfig.ts file, covering listing, resolving, importing, and updating referrals, as well as service-specific upload and status routes. Feedback focuses on enhancing security by implementing granular PRIVILEGE_CHECK constraints for each new route and ensuring consistency with existing modules. Additionally, the reviewer suggests including these routes in the academic year validation list where applicable and correcting minor comment formatting for better alignment with the codebase style.
| }, | ||
| }), | ||
|
|
||
| //referral API |
There was a problem hiding this comment.
For consistency with other module comments in this file (e.g., lines 249, 277, 395), please add a space after the comment slashes.
| //referral API | |
| // Referral API |
| '/user/v1/referrals': createRouteObject({ | ||
| post: { | ||
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
The new referral routes are missing PRIVILEGE_CHECK. To maintain consistency with other modules in this repository (such as LMS, Opportunity, and Cohorts) and to enable granular access control, it is recommended to include privilege checks. Since these routes relate to referral tracking, you can leverage the existing privilegeGroup.tracking definitions.
Additionally, if these referral operations are tied to a specific academic year, these routes should be added to the apiListForAcademicYear array at the end of this file to ensure the academicyearid header is validated by the middleware.
'/user/v1/referrals': createRouteObject({
post: {
ROLE_CHECK: rolesGroup.superadmin_regional_admin,
PRIVILEGE_CHECK: privilegeGroup.tracking.create,
},
}),| '/user/v1/referrals/list': createRouteObject({ | ||
| post: { | ||
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
This list endpoint should include a privilege check for reading tracking data to ensure consistent security across the application.
'/user/v1/referrals/list': createRouteObject({
post: {
ROLE_CHECK: rolesGroup.superadmin_regional_admin,
PRIVILEGE_CHECK: privilegeGroup.tracking.read,
},
}),| '/user/v1/referrals/resolve': createRouteObject({ | ||
| get: { | ||
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
The resolve endpoint should also be protected with a read privilege check.
'/user/v1/referrals/resolve': createRouteObject({
get: {
ROLE_CHECK: rolesGroup.superadmin_regional_admin,
PRIVILEGE_CHECK: privilegeGroup.tracking.read,
},
}),| '/user/v1/referrals/:id': createRouteObject({ | ||
| patch: { | ||
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
The patch endpoint for individual referrals should be protected with the corresponding update privilege.
| '/user/v1/referrals/:id': createRouteObject({ | |
| patch: { | |
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | |
| }, | |
| }), | |
| '/user/v1/referrals/:id': createRouteObject({ | |
| patch: { | |
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | |
| PRIVILEGE_CHECK: privilegeGroup.tracking.update, | |
| }, | |
| }), |
| '/aspirespecific/import-users/referrals/upload': createRouteObject({ | ||
| post: { | ||
| ROLE_CHECK: rolesGroup.superadmin_regional_admin, | ||
| }, | ||
| }), |
There was a problem hiding this comment.
The aspire-specific referral upload endpoint should also include a privilege check for consistency and security.
'/aspirespecific/import-users/referrals/upload': createRouteObject({
post: {
ROLE_CHECK: rolesGroup.superadmin_regional_admin,
PRIVILEGE_CHECK: privilegeGroup.tracking.create,
},
}),1 participant
No description provided.