Anthony/added permission changes for default permissions of user#1447
Anthony/added permission changes for default permissions of user#1447AnthonyWeathers wants to merge 34 commits intodevelopmentfrom
Conversation
… accurately show both default and non-default changes in added or removed columns in permission change log table
…efaultPermissions-Of-User
…efaultPermissions-Of-User
| userId, | ||
| individualName: `INDIVIDUAL: ${firstName} ${lastName}`, | ||
| permissions: Permissions, | ||
| permissions: changedPermissions, // changed from Permissions to changedPermissions, to track changes for default and non-default permissions |
There was a problem hiding this comment.
Since changedPermissions is defined as const changedPermissions = [...Permissions, ...removedPermissions];, it doesn't distinguish between if the edited permissions are added or removed from the role permissions. This can be interpreted by cross-referencing the role permissions. Either consider them added if not found in the role permissions, or removed if they are found in the role permissions.
However, this cross-reference check could be broken if the user's role changes (along with their role permissions) or if the role's permissions change. A role change would thus break our ability to interpret old logs. I think it needs to have removedRolePermissions as a labeled entry to keep this context.
There was a problem hiding this comment.
Or it's cross-referenced with the permissionsAdded and permissionsRemoved fields of all previous entries for this user?
| permissionsRemoved = [ | ||
| ...removedPermissions.filter((item) => !docPermissions.includes(item)), //saves new removed defaults | ||
| ...docPermissions.filter( | ||
| // saves changes of only removed non-default role permissions for user | ||
| (item) => !defaultPermissions.includes(item) && !Permissions.includes(item), | ||
| ), | ||
| ]; | ||
| permissionsAdded = [ | ||
| ...Permissions.filter((item) => !docPermissions.includes(item)), // saves new added permissions | ||
| ...docPermissions.filter( | ||
| // saves changes of only removed default permissions being added back | ||
| (item) => defaultPermissions.includes(item) && !removedPermissions.includes(item), | ||
| ), | ||
| ]; |
There was a problem hiding this comment.
Adding/removing a permission to a user, then mirroring that change to the role will keep showing that change as new with every update.
| permissionsRemoved = [ | |
| ...removedPermissions.filter((item) => !docPermissions.includes(item)), //saves new removed defaults | |
| ...docPermissions.filter( | |
| // saves changes of only removed non-default role permissions for user | |
| (item) => !defaultPermissions.includes(item) && !Permissions.includes(item), | |
| ), | |
| ]; | |
| permissionsAdded = [ | |
| ...Permissions.filter((item) => !docPermissions.includes(item)), // saves new added permissions | |
| ...docPermissions.filter( | |
| // saves changes of only removed default permissions being added back | |
| (item) => defaultPermissions.includes(item) && !removedPermissions.includes(item), | |
| ), | |
| ]; | |
| permissionsRemoved = [ | |
| ...removedPermissions.filter((item) => !prevRemovedPermissions.includes(item)), //saves new removed defaults | |
| ...prevAddedPermissions.filter((item) =>!Permissions.includes(item)), //removed user permissions | |
| ]; | |
| permissionsAdded = [ | |
| ...Permissions.filter((item) => !prevAddedPermissions.includes(item)), // saves new added permissions | |
| ...prevRemovedPermissions.filter((item) => !removedPermissions.includes(item)), //removed permissions added back | |
| ]; |
There was a problem hiding this comment.
I understand that concern, I'll have to take a look into how changing the role of a user with added permissions or removed role permissions affects things with the logs before I can see if it causes issues, and then try implementing the changes you suggested to see if that helps resolve it.
Currently though, when I try modifying the role of my alt manager account on the development live app or locally, I receive an error. This is when I open the profile of my alt, while on my owner account, scroll to their Role, open the role menu and select administrator or volunteer, just to try changing for the changes in permissions, and receive the error I shared below. Does this occur for you? Otherwise if its just my end, I am unsure how to resolve it so I can test the potential issue with role changes with my code. Thank you for bringing up the potential issues though!
There was a problem hiding this comment.
Nevermind, it was an error that occurred during work on another PR, after deleting the incorrect data, I was able to change the role of my alt to test your concerns. You are right that role changes does impact the change log, and your suggested code to handle permissions added and remove does seem better, but in the case of prevAddedPermissions, if we add a permission, say 'Edit Task' to a user, then updated their role to be higher, which includes 'Edit Task' as a default, if a new change log is made for the user, then ...prevAddedPermissions.filter((item) =>!Permissions.includes(item)) in setting permissionsRemoved would be triggered, as I checked, and on the frontend ui for the permission list, through console logs, when I checked the permissions of my alt, its permissions were set to the role's default, so the respective permissions and removedPermissions become blank at first. So with 'Edit Task', it would no be in Permissions if it becomes a default permission due to role, thus being counted as one to be listed under permissionsRemoved. So I should also include a check to see if the item in question, is listed in the role's default list?
…ked checks to address cases for role change for a user
|
@nathanah Been a bit, but I've revised my code, and added a removedRolePermissions field to the UserPermissionChangeLog schema, so could you check it out and verify if my code would work fine? I tested it using my alt, and it seemed to work with role changes while still performing the original job with just changing permissions. I could record a demo to showcase it if needed. |
nathanah
left a comment
There was a problem hiding this comment.
Sorry for the delay. LGTM
…efaultPermissions to userProfileSchema
… to make use of it
…ready in dev branch
Anusha-Gali
left a comment
There was a problem hiding this comment.
Hi Anthony,
I have reviewed your PR locally and have mentioned my comments in frontend: OneCommunityGlobal/HighestGoodNetworkApp#3600 (review)

sayali-2308
left a comment
There was a problem hiding this comment.
PR #1447 (Anthony - Added PermissionChanges For DefaultPermissions Of User): Backend #1447: Reviewed in conjunction with frontend PR #3600
Checked out backend branch Anthony/Added-PermissionChanges-For-DefaultPermissions-Of-User, ran npm install, started backend server. Tested with frontend branch.
CRITICAL BACKEND ISSUES:
- Permission Update Endpoint - 404 Error:
- Frontend sends request to save permission changes
- Backend returns: "AxiosError: Request failed with status code 404"
- API endpoint for updating user permissions appears to be missing or incorrectly configured
- Prevents any permission modifications from being saved
- This is a critical backend failure blocking core functionality
- Role Change Endpoint - Save Error:
- Frontend sends request to save user role change (Volunteer → Manager)
- Backend returns error: "Sorry an error occured while trying to save"
- Role changes cannot be saved
- Prevents creation of role change log entries in Permission Change Logs Table
- Critical backend failure
Backend Endpoints to Verify:
- Permission update endpoint (currently returning 404)
- Role change/update endpoint (currently failing with error)
- Permission Change Log creation endpoint (cannot test due to save failures)
- User profile update endpoint (failing when role is changed)
Required Changes:
- Fix permission update API endpoint - resolve 404 error
- Fix role change save functionality - resolve save error
- Verify Permission Change Log entries are created correctly when permissions/roles are modified
- Ensure all API responses return proper error messages and status codes
- Test complete flow: modify permission → save → create log entry → display in table
…check to ensure length of arrays matched
|



Description
Modified logUserPermissionChangeByAccount.js to work with frontend PR#3600 and update the permission change log table in Permissions Management for Owner users
Related PRS (if any):
This backend PR is related to the PR#3600 frontend PR.
To test this backend PR you need to checkout the PR#3600 frontend PR.
…
Main changes explained:
How to test:
npm install, thennpm run build, and lastlynpm startto run this PR locallyNote:
If a user you're changing, already had permissions removed or added prior to this PR pair, indicator by a star icon listed in the frontend PR, then changes to those permissions might not be saved properly. I tried addressing it, so hopefully it does work, but let me know if anything unexpected occurs though.
There's a current bug in User Management, if you make changes to a user, the array containing that change, will retain all changes you make, whether you changed the role of a user repeatedly or not, same for if you saved some changes, the array will still keep them until you refresh the page or leave it and return. If you make multiple changes without refreshing or leaving the page, then you may/will see repeated change logs for a user if you've changed their role more than once, or if you saved the change, then modified the role of another user, or a different field of the user.