Anthony - Permission Change Logs Table/Manage User Permission Modal Fixes and Feature Additions#3600
Conversation
…for permissions added that are not part of a user's role permissions by default, and added info modal for reset to default to summarize what added permissions will be removed if pressed, and applied some prettier styling to files I change(s) to
…e-PermissionsBeyond-UserRole-DefaultPermissions
…r below 395px, display all changed permissions, whether added or deleted that are not apart of the user's role by default, added red color to star icon for visual indicator of removed default permission
…cent spacing from top and bottom of their button shapes, added a hover over text popup for star icon to give text of permission added or removed, enabled star icon to appear for default permissions that were removed and they appear as red stars, and updated reset to default info modal description to account for both added and removed permissions
… button does not work until clicking reset to default to clear the star icon then the button functions, also included check in checkSubperms so permission categories are now always changeable to green all, gray all, or delete based off of how many of their subperms are added or not added
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
✅ Deploy Preview for highestgoodnetwork-dev ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
TaariqMansurie
left a comment
There was a problem hiding this comment.
Tested everything locally, the permission log table looks clean with proper widths, the new info modal works as expected, star indicators are helpful and show the right tooltips, and the category buttons behave correctly. Also checked on small screen sizes, styling looks good. All set from my side!
I have attached the following screenshots:
nathanah
left a comment
There was a problem hiding this comment.
I think there's room for some small optimizations, but mostly looks good.
…e-PermissionsBeyond-UserRole-DefaultPermissions
…ssionKeys from ./PermissionsConst, and updated the css class used for the star icon so the star displays should work in both light and dark mode, also added 0 padding and changed the background color of the star icons to undo style changes made from recent developement merging
Apologizes for being late, was working on other stuff and delegating some time to review this. I re-entered my branch, and will likely need to do a git merge due to styling now being out of order since I last checked. But the reset to default tooltip seems to be working fine, if you have the time, can you point out perhaps which permissions you're attempting to change with my Anthony Volunteer account, where the reset to default doesn't work properly with? |
…e-PermissionsBeyond-UserRole-DefaultPermissions
…e-PermissionsBeyond-UserRole-DefaultPermissions
…er Permissions if user only has role permissions
…on reconsideration
Hi, thank you for the pointing out of errors, though on double checking on my side, I don't seem to face such errors myself so I'm having trouble trying to reproduce the errors you encountered to investigave properly. For the info icon next to "Reset to Default", it is only meant to open a modal if the current user being viewed had permissions modified from their role. I'm not sure if that's the case, as I only can replicate it not opening if the user doesn't have any added/removed permissions indicated by a star icon. For the 404 AxiosError when attempting to save permission changes on Permission Management, I am able to save changes and get success messages. Might be code I've since merged from development since then, or not, I'll be pushing new changes soon so hopefully that helps fixes potential issues in routing, as I didn't change the endpoint really, only added data to be sent through in the body. However, if you could get that error again through attempting to save permission changes, please right-click, click inspect element and go to console, to view any error messages that appear there, and if you could locate the error matching the AxiosError 404 message, and show a screenshot of the whole body and its data for more clarification, I can look more into it. With the role change on User Profile, I had encountered that before, but have not since, could be because I haven't pushed recent merges with development that could resolve it or not, as currently I am able to perform role changes through a user's profile. As for the dark mode css styling on user profile, that isn't something I really changed much, as I only made changes to convert any css files to module.css at the time, and made adjustments in the className to fit the module.css formatting as required. I'll take a look at them though incase I did mess them up and fix them but if they weren't modified by me recently then I'll be holding off on them as I try not to touch code that I don't need to change unless required. Note: I do most of my testing with my own accounts, there could be cases where attempting to modify other user accounts may run into issues, so as a secondary test, if you could make changes to my test accounts to verify it as well, after I push new changes. The accounts that are safe, are Anthony Volunteer, Anthony Test, and Anthony Test2. Again, thank you for testing my PR, if these issues still persist after I try pushing my more recent changes and merges, then I'll do my best to try fixing them. |
… same for tooltip when hovering over star icon
|
@Anusha-Gali Hi Anusha, Sorry for the ping, but I was hoping to ask for you to review my PR again. I did add the feature for permission change logs to be created from role changes in User Management, so if you could test that and verify it. I did add a note in the description of the PR requiring an issue, so while testing it, refresh the page after saving changes in User Management if you're planning to do multiple tests of it. The issue will eventually be handled by someone, but best to account for it. Also, I modified the styling for the change logs, and the star icon and its tooltip when hovered over in dark mode, so could you check those out in dark mode and ensure they appear as the pictures I added for reference? I didn't add much else, but doing a quick review of the PR overall would be appreciated too incase I missed or had changed anything else that was fine prior. |
HemanthNidamanuru
left a comment
There was a problem hiding this comment.
Hi Anthony,
I tested this PR locally by going through the main flows and verifying the functionality. Overall, everything is working well. Below are a few issues I noticed:
The “Remember to save your changes” dialog box is not very clear in dark mode.
The “Save Changes” button in the user profile looks slightly off in terms of styling in dark mode.
Hi Hemanth, Thanks for reviewing, great to hear generally everything is working. As for the issues you brought up, although I'm not sure if I'm the only one facing issues with not being able to scroll on the dev site but I checked the “Save Changes” button and the “Remember to save your changes” dialog box there as well, with both also being very light as you saw in my PR. So it's not quite my PR changes but still valuable, and maybe I'll add it in so it's fixed at least. Again, thanks for verifying everything else is good though! |
…class name used, adjust dark mode styling for save changes buttons on profile and user management reminder modal
|
For future reviews, on the user profile page, there will be sections that are white and thus not readable in dark mode as the text is white. I fixed basic information and the save changes button. The others at least with badges and the blue square/time off sections are separate files that have css files, so they'd require updating the files to .module.css and updating classnames respectively. I also fixed the reminder modal that pops up in permissions management when you make a change to a user's permissions through "manage user permissions" as an admin/owner, so now its seeable in dark mode as well. Any other styling for fixes to dark mode will be something to report, but also to fix in a separate PR or task as they exist on the development branch deployed site, so any issues you find on my PR, that also exists on the dev site, are not caused by this PR so do not worry about them for the purpose of this PR. Thank you for everyone that reviewed my PR though, helps a lot! |
sayali-2308
left a comment
There was a problem hiding this comment.
Re-Review — PR #3600 + #1447
"Hi Anthony, thank you for the updates! I re-tested all the previously reported issues on branch Anthony/Use-PemissionsData-To-Indicate-PermissionsBeyond-UserRole-DefaultPermissions (frontend) and Anthony/Added-PermissionChanges-For-DefaultPermissions-Of-User (backend). Here is the updated status:
Fixed:
Permission save — no longer returns 404, saves successfully with success toast
Role change save — saves correctly with Success modal
Permission Change Log — new entries created correctly for both permission changes and role changes
Dark mode styling — modal and buttons display correctly in dark mode
Category/permission ℹ️ icons — Permission Info modal opens correctly showing category description
Reset to Default — working correctly
Still Not Fixed:
ℹ️ icon next to the 'Reset to Default' button — clicking it does nothing, no modal or tooltip appears. This was reported in the previous review and still needs to be addressed.
Please fix the remaining info icon issue and I will approve. Great progress overall almost there! Thank you!"
Thanks for the re-review, and sorry for replying late. Just to clarify, are you attempting to click the tooltip icon when the user had no modified permissions? If so, I hadn't setup text to be displayed in a case like that. I'll add something for it of course, I missed it on hindsight, but just want to make sure it was with no permission changes that it doesn't cause a modal to popup. |
… rows for readability, and added info modal for no modified permissions case
|
|




































Description
Permission Change Log Table had a column replaced and more permissions displayed that would include ones provided by a user's role instead of just permissions added/removed that were not originally apart of their role. Added condition checks to display text in Reason column based on how a user's permissions are being changed, whether by the managing their permissions or changing their role. Also, made individual's name be regular text when the user's role is changed/updated, which would be highlight in blue in the reason column. An individual's name would be bold when their permissions was modified, and the text would be bold red if permissions were modified for a Role. Added a dark mode version of highlightRow, to make the rows highlighted more easily readable.
In Manage User Permissions Modal, added a info modal next to Reset To Default that would list the selected user's role, and permissions that would be reset if Reset to Default is used (updated to also display text for if the user has no modified permissions for clarity). Added star icon that appears on same line of a permission that was changed from the default provided by the user's role, red for a deleted default permission and green for an added permission. A paragraph element that pops up when hovering over star icon, for quick understanding of what the star means. Fixed category add/delete button so it adds or delete all sub permissions of a category, or add all remaining unadded permissions of a category with a gray add button. For the reminder modal to save changes, on screen size of 395px or less, fixed styling of the buttons and its div element so they're same size and have consistent padding.
In User Profile, reset user's removedDefaultPermissions to empty and defaultPermissions to the permissions of their new role. Then added a call to the endpoint that saves permission changes and creates a user change log to be shown in the Permission Change Log Table. Will display "See default {role} role permissions" if there is no direct changes to a user's permission if they had no permissions to be changed, or their new role matches it (Example: if they had "Edit Header Message" added and "Edit Team" removed as a Manager, then was changed to Volunteer role, then their change log would show "See default Volunteer role permissions" under Permissions Added since Volunteer role does not have "Edit Team" by default, but would display "Edit Header Message" under Permissions Removed, as it's a permission Volunteers would not have by default). The text would prioritize listing the permissions being modified before displaying the generic text.
New Addition: In User Management, when changing the role field of a user and then saving those changes, it also creates a change log indicating the role change on the Permission Change Log Table in Permissions Management, like for User Profile. Multiple change logs are made if multiple users' roles are changed through this method.
Related PRS (if any):
This backend PR is related to the PR#1447 backend PR.
To test this frontend PR properly you need to checkout the PR#1447 backend PR.
…
How to test:
npm installandnpm run start:localto run this PR locallyPermissions Management
Testing Role Change
Testing Role Changes in User Management
Screenshots or videos of changes:
(Owner Only)

(Dark Mode)



Note
For testing in User Management, there is a leftover issue that wasn't much of an issue prior to this PR, but when you make changes to a field through editing, the change is appended to an array so if you change the role of a user repeatedly, the array will append all of the changes, which when saved, will create multiple change logs for that user, instead of your most recent change.
Also, through the same issue, after you save changes, if you decide to do more testing, via editing more users afterwards, the changes you saved previously would still be retained by the array as well, so if you attempt to save new changes, the previous ones would be saved again, so for role changes, it'd create a new change log for the role you saved earlier too.
For example, say you edited my Anthony Volunteer account to be a Manager, then saved. If you made a new tab to view Permissions Management, you'd see the change log indicating it, but then if you tabbed back to User Management, and edit Anthony Volunteer's role to General, and saved. After refreshing Permissions Management, you'd see two new change logs, one for the Manager, and one for the General role change. Then if you edit Anthony Test, and make them a Volunteer, and saved, you'd again see the two role changes for Anthony Volunteer, and the one role change for Anthony Test.
For a workaround, simply refreshing or leaving the User Management page then going back to it, resets the data array to empty. I informed Jae about it, and he instructed me to create it as a task on the bugs document, low priority for someone else to handle it, so it will eventually get fixed, but this is just to inform you incase you encounter this issue as well.