Anirudh D - permissions management: Create constants for permission keys to replace passing strings directly into hasPermission() by Pandani07 · Pull Request #2052 · OneCommunityGlobal/HighestGoodNetworkApp

Pandani07 · 2024-03-13T00:15:29Z

Description

Create constants for permission keys to replace passing strings directly into hasPermission()

The permissions were passed into hasPermission() as a hard-coded string in the frontend. Eg - hasPermission('deleteWbs')

To avoid manual error, I have created a separate constants file under src/utils/constants.js and I have created an object that has all the permissions being used. So the above example is changed to - hasPermission(permissions.projects.deleteWbs) and constants.js has the following entry - deleteWbs: 'deleteWbs'

Related PRS (if any):

Use the latest backend
…

Main changes explained:

Changed hard-coded permission strings to constants except in src/components/PermissionsManagement/RolePermissions.jsx

How to test:

check into the current branch on the front end and the development branch on the backend
do npm install and npm run start:local to run this PR locally and do npm install, npm run build && npm run start on the development backend
Clear site data/cache
log in as any user and check if you encounter any bugs. Every user should function as the role it's assigned

Screenshots or videos of changes:

Abi-Liu

Everything looks good!

psharma1984

Hi @Pandani07 I tested the PR with listed steps. I tried login with owner and admin account but when i try to create a user using Create User in User Management it displays a blank page. Please see below video for details

screen-capture.webm

Pandani07 · 2024-03-14T04:28:33Z

Hi @Pandani07 I tested the PR with listed steps. I tried login with owner and admin account but when i try to create a user using Create User in User Management it displays a blank page. Please see below video for details

screen-capture.webm

Hello, what user are you logged in as when you try to create a new user?

SanketKaware

I did not encounter any issues and everything worked as expected.

Additionally, I was able to access the Create New User screen without any problems.

Here is my video review for reference.

Screen.Recording.2024-03-21.at.8.49.12.PM.mov

MQChen211 · 2024-04-03T23:42:05Z

Hi I tested your PR with a volunteer account and an owner account. It works well.

Volunteer account:

Owner Account:

Gayathr-2012 · 2024-04-04T02:48:03Z

Hi
I do not have any problem. I could create a new user. I tested your PR it works as well.

SushmithaPrathap

Hey!
I reviewed your PR and tested it following the steps provided. It works as expected.
I tested it with owner account and volunteer account.
Owner can create new users and volunteer cannot.

Here is a screen recording:

Screen.Recording.2024-04-04.at.8.26.23.PM.mov

hiralsonip · 2024-04-05T21:08:43Z

I've checked out the PR, and everything's good to go. I've successfully created new users as well. Check out my video review here.

PR2052.mp4

p-suram

Changes looks good. I have tested with Admin and Volunteer accounts. Good Work

2024-04-06.22-50-51.mp4

p-suram

Hey, following up with my previous review, I just checked, you missed below 2 files:

src\components\PermissionsManagement\RolePermissions.jsx
const isEditableRole = props.role === 'Owner' ? props.hasPermission('addDeleteEditOwners') : props.auth.user.role !== props.role; const canEditRole = isEditableRole && props.hasPermission('putRole'); const canDeleteRole = isEditableRole && props.hasPermission('deleteRole');
src\components\UserProfile\BlueSquareLayout.jsx
const canManageTimeOffRequests = dispatch(hasPermission('manageTimeOffRequests'));

Parth-tech

Hey Anirudh,

I have checked your PR and here's what I have test:

Owner & Admin account can create users
No other pages are affected my current change
Volunteer must not be able to create new users

Owner Account Video:

Owner.mp4

Admin Account Video:

Admin.mp4

Volunteer Account Video:

Volunteer.mp4

For some reason my Volunteer has the permission to create new users which I am not able to remove for that account but the rest works pretty well.

Parth-tech

Could you help me understand why locally volunteer role has the permission to create new users? Furthermore, I not able to remove that permission either.

jennZhang93

I've checked with an owner account. I can visit user management page, add new user, other features like report page are also functioning. The code changes are very detailed which should've covered all hardcoded strings. Good job!

anandsesha · 2024-04-13T00:42:08Z

Hi, I have tested your changes using owner and admin users and it works as expected. I am able to create a user from user management for both users and view the dashboard and timelog sections. For volunteer users that option is not available as expected.

Owner:

volunteer

KurtisIvey · 2024-04-13T04:00:18Z

Hey pandani, would love to be able to get this merged asap as it looks like you've done a lot of great work here. Would it be possible for you to get the merge conflicts cleared up on this, and I'll wrangle some pull requesters to review it.

Thank you!

j-corp-25

Hi @Pandani07

I followed the instructions and created a user using Amin. No issues popped up. New user shows up when queried.

Video below:

CheckingPermissions.mp4

metaphor987

Hi Anirudh,

The description of the code changes is very clear. I've tested the changes using both admin and volunteer accounts, and the main functionalities work well.

There's only one issue with the "Get Time Zone" feature. After entering the "location" and clicking "Get Time Zone," I received an error message saying "API Key missing." This issue occurred when creating a new user using an admin account and when editing the location in user profiles using either an admin or volunteer account. I'm not completely sure whether this issue is related to the PR, but I think it would be great to check. Thank you!

Screen.Recording.2024-04-13.at.7.31.51.PM.mov

Screen.Recording.2024-04-13.at.8.24.44.PM.mov

Tiantian-C

Hi, I have reviewed this PR and logged in as different users, no errors.

aaryaneil

Tested the PR and the changes work as expected. Attached videos for reference.
Admin account:

Screen.Recording.2024-04-13.at.11.41.21.PM.mov

Volunteer account:

Screen.Recording.2024-04-13.at.11.46.13.PM.mov

Owner account:

Screen.Recording.2024-04-13.at.11.47.39.PM.mov

p-suram

Hey, following up with my previous review, I just checked, you missed below 2 files:

src\components\PermissionsManagement\RolePermissions.jsx
const isEditableRole = props.role === 'Owner' ? props.hasPermission('addDeleteEditOwners') : props.auth.user.role !== props.role; const canEditRole = isEditableRole && props.hasPermission('putRole'); const canDeleteRole = isEditableRole && props.hasPermission('deleteRole');
src\components\UserProfile\BlueSquareLayout.jsx
const canManageTimeOffRequests = dispatch(hasPermission('manageTimeOffRequests'));

Pandani07 · 2024-04-16T04:42:56Z

This PR will likely have a LOT of merge conflicts. Many of them probably breaking things of not merged correctly.

I have resolved the merge conflicts now

Pandani07 · 2024-04-16T04:43:29Z

Hey, following up with my previous review, I just checked, you missed below 2 files:

src\components\PermissionsManagement\RolePermissions.jsx
const isEditableRole = props.role === 'Owner' ? props.hasPermission('addDeleteEditOwners') : props.auth.user.role !== props.role; const canEditRole = isEditableRole && props.hasPermission('putRole'); const canDeleteRole = isEditableRole && props.hasPermission('deleteRole');

src\components\UserProfile\BlueSquareLayout.jsx
const canManageTimeOffRequests = dispatch(hasPermission('manageTimeOffRequests'));

Will check

Pandani07 · 2024-04-16T04:46:25Z

Hey pandani, would love to be able to get this merged asap as it looks like you've done a lot of great work here. Would it be possible for you to get the merge conflicts cleared up on this, and I'll wrangle some pull requesters to review it.

Thank you!

Hello, working on it now. I was not working the last weeks.

cgomezhub

Hi there,

I conducted a test using an admin user. I was successfully able to create a new user, assign them to a team, and add them to a project. However, when creating the new user, I encountered an issue where retrieving their time zone information failed.

Please see the attached images for reference.

nishithaShetty · 2024-05-25T16:33:08Z

Hi Anirudh, Nice work! it works as expected.

Sandhya1236

I checked out the current branch on the frontend and the development branch on the backend. After running npm install and npm run start:local on the frontend and npm install, npm run build, and npm run start on the backend, I cleared the site data/cache. I logged in as various users to ensure there were no bugs and confirmed that each user functioned according to their assigned roles. All steps were followed successfully, verifying that changing hard-coded permission strings to constants did not introduce any issues.

one-community · 2024-05-30T16:19:23Z

Putting a pause on this for the reasons listed here: https://docs.google.com/document/d/1n5NIb3dbiSrIRuOG1ZCwt34pEP1poqddCv1guaQml88/edit#bookmark=id.ujw54u7uztbd

Dnagabahiru

Priority: Low

I reviewed the changes for replacing hard-coded permission strings passed directly into hasPermission() with centralized permission constants.

The approach looks good and should help reduce the possibility of typos or inconsistencies when the same permission keys are reused across different components. Using constants such as permissions.projects.deleteWbs instead of hasPermission('deleteWbs') also makes the permission references easier to maintain going forward.

Since Permissions Management is now sufficiently developed, I think this is a good point to complete this implementation and verify the permissions end-to-end.

Suggested next steps:

Complete replacing the remaining hard-coded permission strings, including the areas currently excluded from the PR.
Confirm that RoutePermissions can be reused where appropriate, such as in the Header.
Test all Permissions Management functionality after the constants are implemented.
Verify that users continue to receive the correct permissions based on their assigned roles.
Pay particular attention to user creation and other areas where permission-related issues were previously observed.

Once the permission functionality has been fully tested and confirmed, this PR should provide a good foundation for consistently handling permission keys going forward.

Dnagabahiru

Priority: Low

I reviewed the changes for replacing hard-coded permission strings passed directly into hasPermission() with centralized permission constants.

The approach looks good and should help reduce the possibility of typos or inconsistencies when the same permission keys are reused across different components. Using constants such as permissions.projects.deleteWbs instead of hasPermission('deleteWbs') also makes the permission references easier to maintain going forward.

Since Permissions Management is now sufficiently developed, I think this is a good point to complete this implementation and verify the permissions end-to-end.

Suggested next steps:

Complete replacing the remaining hard-coded permission strings, including the areas currently excluded from the PR.
Confirm that RoutePermissions can be reused where appropriate, such as in the Header.
Test all Permissions Management functionality after the constants are implemented.
Verify that users continue to receive the correct permissions based on their assigned roles.
Pay particular attention to user creation and other areas where permission-related issues were previously observed.

Once the permission functionality has been fully tested and confirmed, this PR should provide a good foundation for consistently handling permission keys going forward.

nathanah

Priority High:
Looks like !targetIsDevAdminUneditable lost it's negation.
Priority Low:
This is definitely still an incomplete transition, but can complete that later.
Some cleaning out of redundancies.

nathanah · 2026-08-01T22:40:47Z

-  const canManageAdminLinks = props.hasPermission('manageAdminLinks');
+
+  const canManageAdminLinks = props.hasPermission(permissions.userManagement.manageAdminLinks);
+  const canPutUserProfileImportantInfo = props.hasPermission(permissions.userManagement.putUserProfileImportantInfo);


Suggested change

const canPutUserProfileImportantInfo = props.hasPermission(permissions.userManagement.putUserProfileImportantInfo);

Extra?

nathanah · 2026-08-01T22:43:02Z

Imported permissions, but didn't replace permission string literals in this file.

nathanah · 2026-08-01T22:44:59Z

                    !isTeamSaved
                  }
-                  canEditTeamCode={props.hasPermission('editTeamCode') || requestorRole == 'Owner' || !targetIsDevAdminUneditable}
+                  canEditTeamCode={props.hasPermission(permissions.teams.editTeamCode) || requestorRole == 'Owner' || targetIsDevAdminUneditable}


Suggested change

canEditTeamCode={props.hasPermission(permissions.teams.editTeamCode) || requestorRole == 'Owner' || targetIsDevAdminUneditable}

canEditTeamCode={props.hasPermission(permissions.teams.editTeamCode) || !targetIsDevAdminUneditable}

I don't think you meant to remove the negation.

nathanah · 2026-08-01T22:46:33Z

                    }
                    canEditTeamCode={
-                      props.hasPermission('editTeamCode') || requestorRole === 'Owner' ||requestorRole === 'Administrator'
+                      props.hasPermission(permissions.teams.editTeamCode) || requestorRole === 'Owner' ||requestorRole === 'Administrator'


Suggested change

props.hasPermission(permissions.teams.editTeamCode) || requestorRole === 'Owner' ||requestorRole === 'Administrator'

props.hasPermission(permissions.teams.editTeamCode)

These manual role checks are redundant, so we can clean them up while we're at it.

nathanah · 2026-08-01T22:47:20Z

+  // const canChangeUserStatus = props.hasPermission('changeUserStatus');
+  // const canAddDeleteEditOwners = props.hasPermission('addDeleteEditOwners');
+  // const canPutUserProfile = props.hasPermission('putUserProfile');
+  // const canUpdatePassword = props.hasPermission('updatePassword');
+  // const canGetProjectMembers = props.hasPermission('getProjectMembers');
+  // const canChangeRehireableStatus = props.hasPermission('changeUserRehireableStatus');
+  // const canManageAdminLinks = props.hasPermission('manageAdminLinks');;
+  // const canSeeQSC = props.hasPermission('seeQSC');
+


Suggested change

// const canChangeUserStatus = props.hasPermission('changeUserStatus');

// const canAddDeleteEditOwners = props.hasPermission('addDeleteEditOwners');

// const canPutUserProfile = props.hasPermission('putUserProfile');

// const canUpdatePassword = props.hasPermission('updatePassword');

// const canGetProjectMembers = props.hasPermission('getProjectMembers');

// const canChangeRehireableStatus = props.hasPermission('changeUserRehireableStatus');

// const canManageAdminLinks = props.hasPermission('manageAdminLinks');;

// const canSeeQSC = props.hasPermission('seeQSC');

Cleaning things up.

Pandani07 added 2 commits March 12, 2024 13:55

created permissions constants

cb52375

Had to change some more

7241106

Abi-Liu previously approved these changes Mar 13, 2024

View reviewed changes

psharma1984 reviewed Mar 14, 2024

View reviewed changes

fixed issues

67ca12d

Pandani07 dismissed Abi-Liu’s stale review via 67ca12d March 18, 2024 19:29

fixed errors

1542189

SanketKaware previously approved these changes Mar 22, 2024

View reviewed changes

one-community added the High Priority - Please Review First label Apr 4, 2024

SushmithaPrathap previously approved these changes Apr 5, 2024

View reviewed changes

p-suram approved these changes Apr 7, 2024

View reviewed changes

p-suram reviewed Apr 7, 2024

View reviewed changes

Parth-tech previously approved these changes Apr 10, 2024

View reviewed changes

Parth-tech reviewed Apr 10, 2024

View reviewed changes

jennZhang93 previously approved these changes Apr 12, 2024

View reviewed changes

KurtisIvey requested review from Abi-Liu and psharma1984 April 13, 2024 03:58

j-corp-25 previously approved these changes Apr 13, 2024

View reviewed changes

metaphor987 suggested changes Apr 14, 2024

View reviewed changes

Tiantian-C reviewed Apr 14, 2024

View reviewed changes

aaryaneil previously approved these changes Apr 14, 2024

View reviewed changes

one-community changed the title ~~Anirudh permissions mangement~~ Anirudh permissions management: Create constants for permission keys to replace passing strings directly into hasPermission() Apr 15, 2024

p-suram suggested changes Apr 15, 2024

View reviewed changes

Pandani07 dismissed stale reviews from aaryaneil, Parth-tech, j-corp-25, jennZhang93, SushmithaPrathap, and SanketKaware via ddacafc April 16, 2024 04:42

Fixed lint errors

0fb5858

Chuehleo added High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible and removed High Priority - Please Review First labels May 1, 2024

cgomezhub suggested changes May 7, 2024

View reviewed changes

Sandhya1236 previously approved these changes May 26, 2024

View reviewed changes

one-community added do not review Do not review or look at code without full context and removed High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible labels May 30, 2024

check: fixed location not fetching issues

860c00a

Pandani07 dismissed Sandhya1236’s stale review via 860c00a May 30, 2024 16:48

Pandani07 added 4 commits May 30, 2024 21:42

action: fixed merge conflicts with latest development

d67b31e

action: fixed merge conflicts with latest development

c11d744

action: fixed merge conflicts with latest development

ff59782

action: fixed merge conflicts with latest development

5e62394

one-community added the Saved for Later Implementation This is an important fix but it needs to wait for other fixes first label Aug 11, 2024

Dnagabahiru reviewed Aug 1, 2026

View reviewed changes

Dnagabahiru approved these changes Aug 1, 2026

View reviewed changes

nathanah requested changes Aug 1, 2026

View reviewed changes

pixelpix13 mentioned this pull request Aug 6, 2026

Purav-Takingover-PR #2052: Centralize permission keys via permissionKeys constants #5431

Open

14 tasks

pixelpix13 closed this Aug 6, 2026

	canEditTeamCode={props.hasPermission(permissions.teams.editTeamCode) \|\| requestorRole == 'Owner' \|\| targetIsDevAdminUneditable}
	canEditTeamCode={props.hasPermission(permissions.teams.editTeamCode) \|\| !targetIsDevAdminUneditable}

	props.hasPermission(permissions.teams.editTeamCode) \|\| requestorRole === 'Owner' \|\|requestorRole === 'Administrator'
	props.hasPermission(permissions.teams.editTeamCode)

Uh oh!

Uh oh!

Conversation

Pandani07 commented Mar 13, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Related PRS (if any):

Main changes explained:

How to test:

Screenshots or videos of changes:

Uh oh!

Abi-Liu left a comment

Choose a reason for hiding this comment

Uh oh!

psharma1984 left a comment

Choose a reason for hiding this comment

Uh oh!

Pandani07 commented Mar 14, 2024

Uh oh!

SanketKaware left a comment

Choose a reason for hiding this comment

Uh oh!

MQChen211 commented Apr 3, 2024

Uh oh!

Gayathr-2012 commented Apr 4, 2024

Uh oh!

SushmithaPrathap left a comment

Choose a reason for hiding this comment

Uh oh!

hiralsonip commented Apr 5, 2024

Uh oh!

p-suram left a comment

Choose a reason for hiding this comment

Uh oh!

p-suram left a comment

Choose a reason for hiding this comment

Uh oh!

Parth-tech left a comment • edited by aaryaneil Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Parth-tech left a comment

Choose a reason for hiding this comment

Uh oh!

jennZhang93 left a comment

Choose a reason for hiding this comment

Uh oh!

anandsesha commented Apr 13, 2024

Uh oh!

KurtisIvey commented Apr 13, 2024

Uh oh!

j-corp-25 left a comment

Choose a reason for hiding this comment

Uh oh!

metaphor987 left a comment

Choose a reason for hiding this comment

Uh oh!

Tiantian-C left a comment

Choose a reason for hiding this comment

Uh oh!

aaryaneil left a comment

Choose a reason for hiding this comment

Uh oh!

p-suram left a comment

Choose a reason for hiding this comment

Uh oh!

Pandani07 commented Apr 16, 2024

Uh oh!

Pandani07 commented Apr 16, 2024

Uh oh!

Pandani07 commented Apr 16, 2024

Uh oh!

cgomezhub left a comment

Choose a reason for hiding this comment

Uh oh!

nishithaShetty commented May 25, 2024

Uh oh!

Sandhya1236 left a comment

Choose a reason for hiding this comment

Uh oh!

Pandani07 commented Mar 13, 2024 •

edited

Loading

Parth-tech left a comment •

edited by aaryaneil

Loading