Repository navigation
Purav Finish PR #2052 Centralize permission keys for hasPermission - #5431
Conversation
…le components for improved maintainability and consistency. Updated permission checks in BMHeader, BadgeDevelopmentTable, BadgeReport, JobFormBuilder, CPHeader, Header, EnhancedJobFormBuilder, and various Project components.
…stants Co-authored-by: Cursor <cursoragent@cursor.com>
…stants Co-authored-by: Cursor <cursoragent@cursor.com>
…nstants. Co-authored-by: Cursor <cursoragent@cursor.com>
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…rmission vars, add sort comparator. Co-authored-by: Cursor <cursoragent@cursor.com>
Adit0717
left a comment
There was a problem hiding this comment.
Tested this locally on the "feature/Purav-takingover-permission-constants" branch. Verified the following routes using both Admin and Volunteer roles:
Admin role:
/projects - accessible
/usermanagement - accessible
/badgemanagement - accessible
/teams - accessible
/permissionsmanagement - accessible
/announcements - redirected to /dashboard
Volunteer role:
/projects, /usermanagement, /badgemanagement, /teams, /permissionsmanagement - all redirected to /dashboard
DeepighaJ
left a comment
There was a problem hiding this comment.
I see after toggle on/off the persmission management table is not updated, it is intended behavior?
Admin:
Toggle Permission



Assign badge is toggled off and assign badge is enabled and it allows to select badges and on clicking confirm it throws below error. The message should state no permission/permission toggled off something like that.

Create team is not available as its toggled off

JobAnalytic:





Volunteer:
nathanah
left a comment
There was a problem hiding this comment.
I think 'update:badges' is the wrong key.
I caught some redundant role checks for owner that we can clean up while we're at it.
Could reorganize the groups to be more useful, but this is fine.
Otherwise, looks good.
| const canUpdateBadges = hasPermission(permissions['update:badges']); | ||
| const canDeleteBadges = hasPermission(permissions['delete:badges']); |
There was a problem hiding this comment.
| const canUpdateBadges = hasPermission(permissions['update:badges']); | |
| const canDeleteBadges = hasPermission(permissions['delete:badges']); | |
| const canUpdateBadges = hasPermission(permissions.updateBadges); | |
| const canDeleteBadges = hasPermission(permissions.deleteBadges); |
Should it be this?
|
|
||
| const canDeleteQuestionSets = useMemo( | ||
| () => userRole === 'Owner' || dispatch(hasPermission('deleteFormQuestions')), | ||
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.deleteFormQuestions)), |
There was a problem hiding this comment.
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.deleteFormQuestions)), | |
| () => dispatch(hasPermission(permissions.deleteFormQuestions)), |
|
|
||
| const canEditQuestionSets = useMemo( | ||
| () => userRole === 'Owner' || dispatch(hasPermission('editFormQuestions')), | ||
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.editFormQuestions)), |
There was a problem hiding this comment.
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.editFormQuestions)), | |
| () => dispatch(hasPermission(permissions.editFormQuestions)), |
|
|
||
| const canCreateQuestionSets = useMemo( | ||
| () => userRole === 'Owner' || dispatch(hasPermission('createFormQuestions')), | ||
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.createFormQuestions)), |
There was a problem hiding this comment.
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.createFormQuestions)), | |
| () => dispatch(hasPermission(permissions.createFormQuestions)), |
|
|
||
| const canCreateQuestionSets = useMemo( | ||
| () => userRole === 'Owner' || dispatch(hasPermission('createFormQuestions')), | ||
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.createFormQuestions)), |
There was a problem hiding this comment.
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.createFormQuestions)), | |
| () => dispatch(hasPermission(permissions.createFormQuestions)), |
|
|
||
| const canManageForms = useMemo( | ||
| () => userRole === 'Owner' || dispatch(hasPermission('manageJobForms')), | ||
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.manageJobForms)), |
There was a problem hiding this comment.
| () => userRole === 'Owner' || dispatch(hasPermission(permissions.manageJobForms)), | |
| () => dispatch(hasPermission(permissions.manageJobForms)), |
| props.hasPermission(RoutePermissions.permissionsManagement, !isAuthUser) || | ||
| props.hasPermission(RoutePermissions.userPermissionsManagement, !isAuthUser); |
There was a problem hiding this comment.
| props.hasPermission(RoutePermissions.permissionsManagement, !isAuthUser) || | |
| props.hasPermission(RoutePermissions.userPermissionsManagement, !isAuthUser); | |
| props.hasPermission(RoutePermissions.permissionsManagement, !isAuthUser); |
…checks, simplify Header permissionsManagement access. Co-authored-by: Cursor <cursoragent@cursor.com>
|
@DeepighaJ can you tell me what role did you use to test? |
…on-only access pattern. Co-authored-by: Cursor <cursoragent@cursor.com>
linlin-husky
left a comment
There was a problem hiding this comment.
I tried to login with Volunteer and admin Role: for both roles, Route protection works as expected. Directly navigating to admin routes (/projects, /usermanagement, /badgemanagement, /teams, /permissionsmanagement) correctly redirects back to /dashboard. Also for admin role, Core management pages and header navigation load properly with the updated permission constants.
However, when I navigated to check Badge Management, when I clicked the Edit button in badge management, the page crashed.
Attaching the crash screenshot. Could you please take a look at the modal trigger and permission checks in BadgeDevelopmentTable.jsx?
Use props.hasPermission in BadgeDevelopmentTable and null-safe badge field loading in EditBadgePopup so null API values no longer throw on .length. Co-authored-by: Cursor <cursoragent@cursor.com>
There was a problem hiding this comment.
Review Summary & Regression Findings
Tested locally on the latest commit d3e6750 (feature/Purav-takingover-permission-constants).
While the previous white-screen crash on clicking "Edit" is resolved, two blocking functional issues were identified during testing:
1. Badge Management: Edits fail to update/persist
- Steps to Reproduce:
- Go to Badge Management -> Badge Development.
- Click Edit on any badge (e.g., "0 Hours for 6 Week Streak").
- Modify any field (Name, Description, Type, etc.) in the
Edit Badgepopup and click Update.
- Issue: The modal closes/submits, but the changes do not persist or reflect in the table.
pls check the below two images
Because I just got the owner access from jae, so i get to check in owner role
2. Permissions Management: when logging in as owner User permissions are non-interactive
- Steps to Reproduce:
- Go to Permissions Management -> Manage User Permissions.
- Select any active user
- Issue: All action buttons (Delete / Toggle) on the right side are permanently disabled/grayed out, and clicking the permissions produces no state updates.
- Note: Please check
PermissionList.jsx,PermissionListItem.jsx, andPermissionsConst.jsfor key mismatch issues with the refactored constants.
Thank you!
DeMoliT1on
left a comment
There was a problem hiding this comment.
Apart from single suggestion rest looks good to me
| import EditBadgePopup from './EditBadgePopup'; | ||
| import DeleteBadgePopup from './DeleteBadgePopup'; | ||
| import hasPermission from '../../utils/permissions'; | ||
| import { permissions } from '../../utils/constants'; |
There was a problem hiding this comment.
| import { permissions } from '../../utils/constants'; | |
| import { permissions } from '~/utils/constants'; |
I would encourage you to use absolute imports rather than relative ones wherever possible, this would make them much more readable
nathanah
left a comment
There was a problem hiding this comment.
One more small change, then I'll approve.
Co-authored-by: Nathan Hoffman <nathanahoffman@gmail.com>
…tor. Gate Select Featured on updateBadges and modifyBadgeAmount permissions per Nathan review feedback. Co-authored-by: Cursor <cursoragent@cursor.com>
DeMoliT1on
left a comment
There was a problem hiding this comment.
Tested again on local for frontend PR changes branch feature/Purav-takingover-permission-constants with related backend PR and observed the following things.
Issue 1 – "Create New Badge" action missing frontend permission check
Observed:
- Logged in using a user who only has "See Badges" permission and navigate to
Badge Managementpage - The "Create New Badge" button remains clickable even if the logged-in user lacks badge creation permissions.
- Clicking the button allows the user to open the "New Badge" modal and submit the form, which only fails after triggering an authorization error from the backend API (
"You are not authorized to create new badges.").
Expected Behavior
- The "Create New Badge" button should be disabled on the frontend using
hasPermission(permissions.createBadges)if the user does not possess the requisite permission.
Screenshots
Overall Assessment
Verified Working
- Permission Constants Implementation: Replaced hardcoded permission string literals across components (Header, User Management, Projects/WBS, Teams, Timelog, Badges, Permissions Management, etc.) with centralized
permissions.*constants exported frompermissionKeys.js. - Route Permissions & Checks: Updated
routePermissions.jsandhasPermission()utility functions to support array-style checks seamlessly. - Header Navigation Parity: Verified Header links (User Management, Reports, Projects, Teams, Badges, Permissions Management, etc.) render as expected based on assigned user role permissions without runtime console errors.
- Permission Toggle Functionality: Tested turning permissions ON and OFF for role configurations in Permissions Management - UI controls correctly show/hide/disable based on updated constant-based permission checks.
- Dark Mode Parity: Confirmed existing UI layout and theme display remain intact with no styling regressions in dark mode across updated pages.
Requesting Changes
Please address the missing frontend permission check on the "Create New Badge" button before merging:
- "Create New Badge" button should use
hasPermission(permissions.createBadges)so unauthorized users cannot invoke the modal or submit the request.
Resolve routePermissions conflict using permissions.* constants including rescheduleEvent. Gate Create New Badge with createBadges permission and fix Edit Badge form sync so Update enables when badge data loads correctly. Co-authored-by: Cursor <cursoragent@cursor.com>
Hey @linlin-husky, I think it's because the image URL field is empty |
|
❌ The last analysis has failed. |
Co-authored-by: Cursor <cursoragent@cursor.com>
MahalakshmiPFW
left a comment
There was a problem hiding this comment.
Hi! Everything was tested locally and seems to be working fine. Admin and Volunteer route permissions are behaving as expected.
muthapriyanka
left a comment
There was a problem hiding this comment.
Tested PR https://github.com/OneCommunityGlobal/HighestGoodNetworkApp/pull/5431locally.
Updated some permissions and badge creation.
One suggestion is the permission management page has a grid around it which is hardly visible in dark mode may be in future PR we can improve that.




shubhamjakhete
left a comment
There was a problem hiding this comment.
Hi,
I checked the key mapping mechanically rather than only by clicking through the UI.
All constants resolve. Every permission key referenced on this branch checks out against Permissions.json or LEGACY_OR_EXTRA_KEYS. Nothing evaluates to undefined, which would deny silently with no error.
One behaviour change worth noting. development uses hasPermission('update:badges') at BadgeDevelopmentTable.jsx:30, and neither update:badges nor delete:badges exists in Permissions.json, so those checks have never returned true for anyone. updateBadges and deleteBadges do exist, making the switch a real fix rather than a rename. Badge update and delete controls will start appearing for users who have never seen them. Worth updating the description, since "runtime permission values are unchanged" isn't quite accurate here.
HemanthVarupula
left a comment
There was a problem hiding this comment.
Hi @pixelpix13
Tested the pr #5431 locally as admin
App loads successfully
Header loads as intended all the links works
just you can look into font and color , apart from that everything works as intended
Resolve EditBadgePopup conflict by keeping null-safe edit loading plus missing-field warning, and extend permissions management route access using RoutePermissions constants. Co-authored-by: Cursor <cursoragent@cursor.com>
be5872f
Remove unused userRole in JobFormbuilder and use RegExp.exec in badge error parsing. Co-authored-by: Cursor <cursoragent@cursor.com>
|
|
Thank you all, merging! |





Description
Implements permission-key constants so components no longer pass raw strings into

hasPermission()(PR #2052 / task: Finish PR2052 – Create constants for permission keys).This is a frontend refactor only (takeover of Anirudh’s WIP
Anirudh_permissionsMangement). There is no new UI feature. Runtime permission values are unchanged (permissions.postProject === 'postProject'), so behavior should matchdevelopmentwhen the same permissions are assigned.Motivation: Reused hardcoded strings across many components are typo-prone and hard to maintain. Centralizing keys in
permissionKeys.js(sourced fromPermissions.json+ legacy keys) and usingRoutePermissionsin Header improves consistency and maintainability going forward (per Jae/Diego notes on PR #2052).Fixes / Implements: PR #2052 (PRIORITY LOW – Finish constants for permission keys / WIP Purav #2052)
Related PRS (if any):
Anirudh_permissionsMangement(never merged); reimplemented on currentdevelopmentwith flatpermissions.*keys +RoutePermissions(instead of nested hand-written constants).Main changes explained:
src/utils/permissionKeys.js— builds frozenpermissionsmap fromPermissions.json+LEGACY_OR_EXTRA_KEYS; also exportspermissionCategories.src/utils/constants.js— re-exportspermissions/permissionCategories.src/utils/routePermissions.js— route/nav permission lists usepermissions.*constants.src/utils/permissions.js— supports array-style OR checks used byRoutePermissions.hasPermission(permissions.X)/RoutePermissions.*instead of string literals.viewWBSpermissionsRequired, routes Blue Square Email,frontPermissions.includes(...), etc.) to use the same constants.origin/development(Header / UserManagement conflicts resolved while keeping constants).hasPermission('postProject')hasPermission(permissions.postProject)hasPermission(RoutePermissions.userManagement)etc.permissionKeys.jsHow to test:
feature/Purav-takingover-permission-constantsnpm install(Node v20.19.4 recommended), then start locally:npm run start:local(app athttp://localhost:5173)Smoke (Admin / Volunteer)
Permission ON / OFF regression (Owner recommended)
Toggle permissions in Permissions Management → Save → hard refresh / re-login → confirm UI:
getJobReports), Weekly Summaries bio highlight filterPass criteria: ON → control available and works; OFF → hidden/disabled/blocked; no white screens.
Known note to verify
RoutePermissions.badgeManagement, which includesassignBadges(development Header OR-chain did not). Confirm whether users with only Assign Badges should see that nav item.Screenshots or videos of changes:
pr.5431_video.mp4
Note:
development, not look for new screens.postRole/putRole/deleteRole). Admin alone may not be able to add/edit/delete roles.