Sai preetham tracking management #3091

preethamdnr · 2025-01-28T18:29:49Z

Description

This PR implements a new "Tracking Management" section within the permissions module, allowing owners to grant specific tracking-related permissions (Tracking Button, <+/->, etc.) to other users.

Related PRS (if any):

none.

Main changes explained:

Introduced a new "Tracking Management" section in Permission Management, allowing owners to grant tracking-related permissions (e.g., addWarningTracker, deactivateWarningTracker) to users.
Updated Warning.jsx and WarningItem.jsx to manage permission-based visibility and enable/disable buttons (Issue Warning, -Issue Blue Square, Delete Warning) based on user roles.
Enhanced WarningTrackerModal.jsx to control actions like adding, deactivating, reactivating, and deleting warning trackers with permission checks.
Added new permissions in PermissionsConst.js with detailed descriptions and updated Warnings.test.js to validate the new features.

How to test:

check into current branch
do npm install and ... to run this PR locally
Clear site data/cache
log as Owner/admin user
Navigate to Dashboard → Other Links → Permission Management → Manage User Permissions.
Select your volunteer account → Permissions → Tracking Management. Assign specific permissions under "Tracking Management" (e.g., View Tracking Overview, Issue a Tracking Warnings, Add a New Warning Tracker, Delete a Warning Tracker)→ Save changes→ Submit.
Log in as the volunteer and verify the assigned permissions are reflected in their account.
Test all permissions in different scenarios.

Screenshots or videos of changes:

video1708959534.mp4

… for Tracking Management.

netlify · 2025-01-28T18:30:09Z

✅ Deploy Preview for highestgoodnetwork-dev ready!

Name	Link
🔨 Latest commit	`59f1189`
🔍 Latest deploy log	https://app.netlify.com/sites/highestgoodnetwork-dev/deploys/67a16c8b3982a20008ff7f5e
😎 Deploy Preview	https://deploy-preview-3091--highestgoodnetwork-dev.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

fangedShadow · 2025-01-29T17:42:30Z

the code works for majority of the part. I had issue regarding issuing a blue square or a warning when you make a new title. Also it let me add a blue square when i didn't have the permission to do it. please check the video i did multiple checks to confirm it. other than this everything else works how it is mentioned above.

20250129-1717-21.9006812.1.mp4

nathanah

Is there a backend PR for this? You'll need backend changes to implement these permissions.

nathanah · 2025-02-01T10:45:48Z

src/components/Warnings/WarningItem.jsx

+  const rolesAllowedToTracking = ['Administrator', 'Owner'];
+
+  const canIssueTrackingWarnings =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('issueTrackingWarnings'));
+  const canIssueBlueSquare =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('issueBlueSquare'));
+  const canDeleteWarning =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('deleteWarning'));


Suggested change

const rolesAllowedToTracking = ['Administrator', 'Owner'];

const canIssueTrackingWarnings =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('issueTrackingWarnings'));

const canIssueBlueSquare =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('issueBlueSquare'));

const canDeleteWarning =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('deleteWarning'));

const canIssueTrackingWarnings = dispatch(hasPermission('issueTrackingWarnings'));

const canIssueBlueSquare = dispatch(hasPermission('issueBlueSquare'));

const canDeleteWarning = dispatch(hasPermission('deleteWarning'));

hasPermission() will check for role permissions too, so you can remove rolesAllowedToTracking. If you want to guarantee that Admins and Owners have this permission, you can add it to CreateInitialPermissions.js on the backend and those permissions will be guaranteed for the roles you add it to (they will be re-added to the roles every time the server restarts).

nathanah · 2025-02-01T10:48:30Z

src/components/Warnings/Warnings.jsx

+  const rolesAllowedToTracking = ['Administrator', 'Owner'];
+  const isAllowedToTracking =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('viewTrackingOverview'));
+  const canViewTrackerButton =
+    rolesAllowedToTracking.includes(userRole) ||
+    dispatch(hasPermission('addWarningTracker')) ||
+    dispatch(hasPermission('deactivateWarningTracker')) ||
+    dispatch(hasPermission('deleteWarningTracker'));


Suggested change

const rolesAllowedToTracking = ['Administrator', 'Owner'];

const isAllowedToTracking =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('viewTrackingOverview'));

const canViewTrackerButton =

rolesAllowedToTracking.includes(userRole) ||

dispatch(hasPermission('addWarningTracker')) ||

dispatch(hasPermission('deactivateWarningTracker')) ||

dispatch(hasPermission('deleteWarningTracker'));

const isAllowedToTracking = dispatch(hasPermission('viewTrackingOverview'));

const canViewTrackerButton =

dispatch(hasPermission('addWarningTracker')) ||

dispatch(hasPermission('deactivateWarningTracker')) ||

dispatch(hasPermission('deleteWarningTracker'));

Same thing here.

nathanah · 2025-02-01T10:49:47Z

src/components/Warnings/WarningItem.jsx

@@ -13,7 +14,19 @@ function WarningItem({
  username,
  handleDeleteWarning,
  submitWarning,
+  userRole,


Suggested change

userRole,

Don't need to pass in userRole if you rely on hasPermission().

nathanah · 2025-02-01T10:50:17Z

src/components/Warnings/Warnings.jsx

@@ -94,11 +102,12 @@ export default function Warning({ personId, username, userRole, displayUser }) {
          username={username}
          submitWarning={handlePostWarningDetails}
          handleDeleteWarning={handleDeleteWarning}
+          userRole={userRole}


Suggested change

userRole={userRole}

Don't need to pass in userRole if you rely on hasPermission().

nathanah · 2025-02-01T10:50:32Z

src/components/Warnings/Warnings.jsx

@@ -126,6 +135,7 @@ export default function Warning({ personId, username, userRole, displayUser }) {
            personId={personId}
            setToggleWarningTrackerModal={setToggleWarningTrackerModal}
            getUsersWarnings={fetchUsersWarningsById}
+            userRole={userRole}


Suggested change

userRole={userRole}

Same here

nathanah · 2025-02-01T10:52:03Z

src/components/Warnings/modals/WarningTrackerModal.jsx

@@ -34,6 +33,7 @@ function WarningTrackerModal({
  toggleWarningTrackerModal,
  getUsersWarnings,
  setToggleWarningTrackerModal,
+  userRole,


Suggested change

userRole,

Same here.

nathanah · 2025-02-01T10:52:47Z

src/components/Warnings/modals/WarningTrackerModal.jsx

+  const rolesAllowedToTracking = ['Administrator', 'Owner'];
+  const canAddWarningTracker =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('addWarningTracker'));
+  const canDeactivateWarningTracker =
+    rolesAllowedToTracking.includes(userRole) ||
+    dispatch(hasPermission('deactivateWarningTracker'));
+  const canReactivateWarningTracker =
+    rolesAllowedToTracking.includes(userRole) ||
+    dispatch(hasPermission('reactivateWarningTracker'));
+  const canDeleteWarningTracker =
+    rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('deleteWarningTracker'));


Suggested change

const rolesAllowedToTracking = ['Administrator', 'Owner'];

const canAddWarningTracker =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('addWarningTracker'));

const canDeactivateWarningTracker =

rolesAllowedToTracking.includes(userRole) ||

dispatch(hasPermission('deactivateWarningTracker'));

const canReactivateWarningTracker =

rolesAllowedToTracking.includes(userRole) ||

dispatch(hasPermission('reactivateWarningTracker'));

const canDeleteWarningTracker =

rolesAllowedToTracking.includes(userRole) || dispatch(hasPermission('deleteWarningTracker'));

const canAddWarningTracker = dispatch(hasPermission('addWarningTracker'));

const canDeactivateWarningTracker = dispatch(hasPermission('deactivateWarningTracker'));

const canReactivateWarningTracker = dispatch(hasPermission('reactivateWarningTracker'));

const canDeleteWarningTracker = dispatch(hasPermission('deleteWarningTracker'));

Same here.

preethamdnr · 2025-02-04T01:32:50Z

the code works for majority of the part. I had issue regarding issuing a blue square or a warning when you make a new title. Also it let me add a blue square when i didn't have the permission to do it. please check the video i did multiple checks to confirm it. other than this everything else works how it is mentioned above.

20250129-1717-21.9006812.1.mp4
Hi @fangedShadow,

Thanks for your feedback! I've made the requested changes to ensure permissions are correctly enforced for adding blue squares and warnings. Everything is working as expected now. Could you please review it again and let me know your thoughts?

Regards,
Sai Preetham

preethamdnr · 2025-02-04T01:40:00Z

Hi @nathanah,

Thank you for the suggestions! Regarding your feedback:

UserRole:
Passing userRole is necessary as per the task requirements, as it provides flexibility to assign these permissions beyond just Admin and Owner roles. This allows for easier customization of permissions when needed.

Backend Changes:
Backend updates are not required for this issue since the permissions are being handled entirely on the frontend. The existing API structure is sufficient to store and manage these permissions without introducing new endpoints.

Let me know if you have further questions or would like me to clarify anything else!

Best regards,
Sai Preetham Dongari

nathanah · 2025-02-04T02:53:25Z

UserRole: Passing userRole is necessary as per the task requirements, as it provides flexibility to assign these permissions beyond just Admin and Owner roles. This allows for easier customization of permissions when needed.

With this implementation, Admin and Owner will look like they don't have permission when you check in Permission Management, but still have access to the buttons. My suggestion is to, instead of your current implementation of adding an additional check on the front-end, add this permission on the back end (in CreateInitialPermissions.js) so the permission is guaranteed for these roles. This has the same flexibility for adding the permissions to any role and simplifies logic to be in alignment with how we do permission checks throughout the codebase.

Backend Changes: Backend updates are not required for this issue since the permissions are being handled entirely on the frontend. The existing API structure is sufficient to store and manage these permissions without introducing new endpoints.

I'm not necessarily suggesting adding new endpoints, just adding in permission checks to the existing ones. Without backend checks, someone could circumvent the frontend permission checks by manually sending custom API requests (using something like Postman).

I think the backend controller is the currentWarningsController? I'm saying to add a permission check to each relevant API to reject requests from users without the relevant permission.
Example:

const deleteWarningDescription = async (req, res) => {
+    if(!(await hasPermission(req.body.requestor, 'deleteWarning'))){
+        res.status(403).send({error: 'You are not authorized to delete warnings'})
+    }
...

Ideally, the permissions should be 1:1 with the APIs IMO since it makes these changes trivial. Reusing APIs makes the checks have to be more complicated so that they only have access to exactly what their permissions allow.

Let me know if you have any other questions.

luisarevalo21

Hello @preethamdnr,

I reviewed your PR but I had issues when I tried assigning the permission to my volunteer account.
Since my volunteer account doesn't have the permission to see all the users, I first added that task then the tracking permission, but it fails, and crashes on the backend as well.
I am checked in the development backend branch as well. My previous PR about the tracking might be a cause as well as I see an error related to the enum.

I also agree with @nathanah . You should have a backend route since you are updating the user's permissions. Otherwise anyone can just adjust their account on the frontend and be

displayed information they shouldn't. You probably don't need a new route, as theres probably already a route that can update a user's permission you'll probably need to use that as Nathan suggested too.

Also, the backend displaying a ton of data too, there seems to be a log somewhere in the backend.

Here are the errors I got.

preethamdnr added 3 commits December 13, 2024 16:17

Added Tracking management to the permissionslist

8f0da1e

Latest pull from development

e67754e

Saipreetham_Implemented permission-based visibility and functionality…

6bc2178

… for Tracking Management.

preethamdnr added medium priority high priority and removed medium priority high priority labels Jan 28, 2025

latest dev changes

da542e4

SaumitC requested a review from nathanah January 31, 2025 17:03

nathanah requested changes Feb 1, 2025

View reviewed changes

preethamdnr added 2 commits February 3, 2025 19:05

Preetham_fixed the issue 2 blue square for overview permission.

a33abfa

merge conflicts resolved

59f1189

preethamdnr requested a review from nathanah February 4, 2025 01:40

one-community added High Priority - Please Review First This is an important PR we'd like to get merged as soon as possible and removed medium priority labels Feb 4, 2025

luisarevalo21 requested changes Feb 4, 2025

View reviewed changes

preethamdnr 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 Feb 4, 2025

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sai preetham tracking management #3091

Sai preetham tracking management #3091

preethamdnr commented Jan 28, 2025 •

edited

Loading

netlify bot commented Jan 28, 2025 •

edited

Loading

fangedShadow commented Jan 29, 2025

nathanah left a comment

nathanah Feb 1, 2025

nathanah Feb 1, 2025

nathanah Feb 1, 2025

nathanah Feb 1, 2025

nathanah Feb 1, 2025

nathanah Feb 1, 2025

nathanah Feb 1, 2025

preethamdnr commented Feb 4, 2025

preethamdnr commented Feb 4, 2025

nathanah commented Feb 4, 2025 •

edited

Loading

luisarevalo21 left a comment

Sai preetham tracking management #3091

Are you sure you want to change the base?

Sai preetham tracking management #3091

Conversation

preethamdnr commented Jan 28, 2025 • edited Loading

Description

Related PRS (if any):

Main changes explained:

How to test:

Screenshots or videos of changes:

netlify bot commented Jan 28, 2025 • edited Loading

✅ Deploy Preview for highestgoodnetwork-dev ready!

fangedShadow commented Jan 29, 2025

nathanah left a comment

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

nathanah Feb 1, 2025

Choose a reason for hiding this comment

preethamdnr commented Feb 4, 2025

preethamdnr commented Feb 4, 2025

nathanah commented Feb 4, 2025 • edited Loading

luisarevalo21 left a comment

Choose a reason for hiding this comment

preethamdnr commented Jan 28, 2025 •

edited

Loading

netlify bot commented Jan 28, 2025 •

edited

Loading

nathanah commented Feb 4, 2025 •

edited

Loading