Fix: Add form validation when removing permissions #2749

rumzledz · 2024-07-19T18:53:58Z

Description

Note

This PR is meant for actions made via the Permissions Decision Method
There are master bugs related to the Reputation Decision Method which are present on this PR and they will be dealt with separately
Multi-Sig will be dealt with separately

Removing Root permissions from someone in a Parent domain

Removing permissions from someone in a subdomain

Testing

Important

Make sure you have at least 2 Owners in your Colony's Parent domain because if you accidentally remove Root permissions from the only Owner in your Colony, you'll have to restart your env again
Give Fry the much coveted Owner permissions in team General
Give alex-the-ace the Payer permissions in team General
Give Amy the Admin permissions in team Andromeda
Give diana-dynamo these Custom permissions in team General
- Administration
- Arbitration
Give eddy-edge these Custom permissions in Serenity
- Architecture
- Arbitration

Note

The Redo action is now disabled for actions that have explicitly removed permissions from a user
When removing Root permissions from someone in a parent domain, we will now notify the user of the consequences and have them acknowledge our message before they can submit the form
Toggles for inherited permissions are now disabled

1. Selecting Remove Permissions for a member who has permissions for the selected team (Fry)

Step	Expected Result
Set up the following fields - Team: General - Member: Fry - Permissions: Remove permissions:	You should not see an error message

2. Selecting Remove Permissions for a member who does not have permissions for the selected team (jasmine-jolt)

Step	Expected Result
Set up the following fields - Team: General - Member: jasmine-jolt - Permissions: Remove permissions:	Error message: "Member does not have permissions in this team"

3. Checking that the Remove permission option persists when changing the Team & Member fields (Fry)

Step	Expected Result
1. Set up the following fields - Team: General - Member: Fry - Permissions: Remove permissions:
2. Set the Team field to Andromeda	The Permissions field should still be set to Remove permissions
3. Set the Member field to Leela	The Permissions field should still be set to Remove permissions

4. Removing permissions from someone who has inherited permissions from the Parent domain (Fry)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: Fry - Permissions: Remove permissions:	Error message: "Permissions inherited from a parent team, select the parent team to remove permissions."

5. Upgrading a user's role, when the user has an inherited role (alex-the-ace)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: alex-the-ace - Permissions: Admin	Error message: "This member already has Payer permissions inherited from a parent team. You can select the Custom permission type and enable the Architecture permission to have the required permissions in this team."

6. Downgrading a user's role, when the user has an inherited role (alex-the-ace)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: alex-the-ace - Permissions: Mod:	Error message: "Permissions inherited from a parent team, select the parent team to remove permissions."

7. Applying the same inherited role for a user (alex-the-ace)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: alex-the-ace - Permissions: Payer:	Error message: "This member already has Payer permissions inherited from the parent team"

8. Upgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: diana-dynamo - Permissions: Admin	Error message: "This member already has Custom (Arbitration and Administration) permissions inherited from a parent team. You can select the Custom permission type and enable the Architecture and Funding permissions to have the required permissions in this team."

9. Downgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: diana-dynamo - Permissions: Mod:	Error message: "Permissions inherited from a parent team, select the parent team to remove permissions."

10. Applying the same inherited permissions for a user (diana-dynamo)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: diana-dynamo - Permissions: Custom:
2. Submit the form	Error message: "This member already has these custom permissions inherited from the parent team"

11. Checking if the inherited permission toggles are disabled (diana-dynamo)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: diana-dynamo - Permissions: Custom:
2. Hover over the Administration and Arbitration toggles	You should see a tooltip that says: "Permission already inherited from a parent team"
3. Click the Administration and Arbitration toggles	Nothing should happen as they are disabled

12. Removing Root permissions from someone who does not have inherited permissions from a Parent domain (Amy)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: Amy - Permissions: Remove permissions:	You should see a table with the following UI - it's basically the bullet-pointed list of Colony actions pertinent to the user's Role, in this case it's Admin - The table body copy should say "Removal of the following Colony actions" - The table header should say "Remove {role} type"
2. Fill in all other required fields
3. Submit the form	You should see a table with the following UI - The Redo action button should not be available - You should see the same table UI as you did prior to submission

13. Removing Root permissions from someone in a Parent domain (Fry)

Step	Expected Result
1. Set up the following fields - Team: General - Member: Fry - Permissions: Custom permissions:
2. Fill in all other required fields
3. In the Custom Permissions table, toggle off Root
4. Submit the form	You should see the following Modal: - You should not be able to click the "Update permissions" button
5. Close the modal
6. Set the Permissions field to Remove permissions
7. Submit the form	You should see the following Modal: - You should not be able to click the "Update permissions" button
8. Set the Permissions field to Admin
9 Submit the form	You should see the following Modal: - You should not be able to click the "Update permissions" button - Basically, you should see the permissions you will lose, in this case, downgrading to Admin permissions will take away your Root & Recover permissions
8. Tick the "I understand the risk and want to remove Root permissions" checkbox	The Update permissions button should now be enabled
9. Click the Update permissions button	The modal closes and the form is submitted

14. Upgrading Custom permissions to a Role (diana-dynamo)

Step	Expected Result
1. Set up the following fields - Team: Andromeda - Member: diana-dynamo - Permissions: Custom - Toggle on Funding	The copy underneath the Action title should say "Assign Payer permissions for diana-dynamo in Andromeda by leela"
2. Submit the form
3. Redo the action
4. Wait for the Action form to come up	The permissions field should say "Payer"
5. Submit the form	Error message: "This member already has these permissions" This should not say inherited anymore since the Custom Permissions are customised on Team Andromeda
6. Set the Permissions field to Custom	- Error message: "This member already has these permissions" - The Administration & Arbitration toggles are still disabled because these are inherited from the Parent domain - The Funding toggle is switched on and it should be editable - The Architecture toggle is switched off but you should be able to edit it
7. Set the Team field to Serenity	Error message: "This member already has these custom permissions inherited from the parent team" Which makes sense because the Custom Permissions for this domain are left untouched and are the same as the Parent domain

15. Checking the disabled state of Permissions toggles in the Completed Action component (eddy-edge)

Step	Expected Result
1. Set up the following fields - Team: Serenity - Member: eddy-edge - Permissions: Custom:
2. Switch off the Architecture toggle
3. Submit the form	- On the Completed Action component, you should only see Arbitration switched on - Even though the Arbitration toggle is disabled, verify that it is not greyed out

Diffs

Changes 🏗

Exposed the Action Form's Primary button props via getFormOptions
There's a mixture of references to Colony Roles, most notably "roles" or "permissions". In this PR, I have decided to refer to Colony Roles as "Permissions" to help differentiate it from a User Role which is our syntactic sugar to describe a set of permissions
With the Manage Permissions form, we have a couple of instances where we have to reference a user's role from a DB along with the current role that's selected on the form. So I decided to prepend role/permissions-related variables with either "db" or "form" to differentiate their source I believe this helps with the readability.

Resolves #2339
Resolves #2241
Resolves #2755
Resolves #3395

arrenv

@rumzledz Great work on this, I appreciate your hard work and for working through the cases.

Everything seems to work as expected, in which the warning and confirmation modal appears in the situation whenever the Root permission is removed.

My only request would be to close down the modal on clicking the confirmation. That way we return to the action panel, and users can access their Userhub without having to have it float over the top.

mmioana

Awesome work and a great deal of effort on this one @rumzledz 👏 🥇

Went through all your well documented testing steps and can confirm everything works as expected

Setting up the dev env

Selecting Remove Permissions for a member who has permissions for the selected team (Fry)
Selecting Remove Permissions for a member who does not have permissions for the selected team (jasmine-jolt)
Checking that the Remove permission option persists when changing the Team & Member fields (Fry)

Screen.Recording.2024-10-28.at.14.03.39.mov

Removing permissions from someone who has inherited permissions from the Parent domain (Fry)
Upgrading a user's role, when the user has an inherited role (alex-the-ace)
Downgrading a user's role, when the user has an inherited role (alex-the-ace)
Applying the same inherited role for a user (alex-the-ace)
Upgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)
Downgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)
Applying the same inherited permissions for a user (diana-dynamo)

Screen.Recording.2024-10-28.at.14.07.23.mov

Checking if the inherited permission toggles are disabled (diana-dynamo)

Screen.Recording.2024-10-28.at.14.08.09.mov

Removing Root permissions from someone who does not have inherited permissions from a Parent domain (Amy)
Removing Root permissions from someone in a Parent domain (Fry)
Upgrading Custom permissions to a Role (diana-dynamo)

Screen.Recording.2024-10-28.at.14.15.37.mov

Checking the disabled state of Permissions toggles in the Completed Action component (eddy-edge)

Left a small refactoring comment, but that can be treated as a separate issue/improvement 👍 @rumzledz

mmioana · 2024-10-28T13:29:48Z

src/utils/strings.ts

I'm wondering if it would make sense to make use of react-intl pluralisation feature and also create a message descriptor for the and?

mmioana · 2024-10-28T16:13:12Z

src/components/v5/common/ActionSidebar/hooks/useActionFormProps.ts

    validationSchema: ACTION_BASE_VALIDATION_SCHEMA,
+    onSuccess: () => {},


Please add noop here

Spinto!! 🚀

mmioana

It still works as expected after rebasing @rumzledz let's merge it! ✨

Screen.Recording.2024-10-28.at.17.27.43.mov

iamsamgibbs

Gone through most of the testing steps again since the rebase and all still working well! 🚢

rdig

Retested all the steps that I couldn't before and everything now works as you laid out in your testing plan. Very nice.

Thank for your patience and determination to see this to the end.

💯

Setup Phase

Test 10

Test 12

Test 13

Test 15

Nortsova

I did small retest and it seems it still working as expected 👍

Lest merge 🎉 🥳

Not tested again, but there are 4 approvals.

rumzledz self-assigned this Jul 19, 2024

rumzledz force-pushed the fix/2339-remove-permissions-validation branch 17 times, most recently from 91b37c5 to bb4f647 Compare July 22, 2024 15:40

arrenv previously requested changes Jul 22, 2024

View reviewed changes

rumzledz force-pushed the fix/2339-remove-permissions-validation branch 8 times, most recently from b3b6c7d to 9d8aa99 Compare July 22, 2024 17:40

rumzledz requested a review from arrenv July 22, 2024 17:40

rumzledz force-pushed the fix/2339-remove-permissions-validation branch 2 times, most recently from c52ec6c to c9327f9 Compare July 22, 2024 18:14

rumzledz added 2 commits October 28, 2024 21:14

fix: add remove permission validation

1e45598

fix: add prompt when removing roles from the root domain

d313e15

mmioana previously approved these changes Oct 28, 2024

View reviewed changes

rumzledz added 5 commits October 28, 2024 23:07

fix: update modal copies and add a checkbox for acknowledgement

624db84

fix: trigger removal modal in root domain based on role diff

8ccda2d

fix: differentiate same permissions error between domain and inherited

cf97385

fix: update the permissions table UI & logic

aabead8

fix: update getUserRolesForDomain usage

6e6973b

rumzledz dismissed stale reviews from mmioana, Nortsova, and iamsamgibbs via e4cb922 October 28, 2024 15:50

rumzledz force-pushed the fix/2339-remove-permissions-validation branch from 779d049 to e4cb922 Compare October 28, 2024 15:50

rumzledz requested review from Nortsova, iamsamgibbs and mmioana October 28, 2024 15:50

mmioana reviewed Oct 28, 2024

View reviewed changes

fix: rebase fixes

8b81398

rumzledz force-pushed the fix/2339-remove-permissions-validation branch from e4cb922 to 8b81398 Compare October 28, 2024 16:18

iamsamgibbs mentioned this pull request Oct 28, 2024

[Member permissions] 403 Error when assigning duplicate permissions to a user #3395

Closed

rumzledz requested a review from mmioana October 28, 2024 17:06

mmioana approved these changes Oct 28, 2024

View reviewed changes

iamsamgibbs approved these changes Oct 28, 2024

View reviewed changes

iamsamgibbs linked an issue Oct 28, 2024 that may be closed by this pull request

[Member permissions] 403 Error when assigning duplicate permissions to a user #3395

Closed

rdig approved these changes Oct 28, 2024

View reviewed changes

Nortsova approved these changes Oct 28, 2024

View reviewed changes

rumzledz merged commit e9ae47c into master Oct 29, 2024
4 of 6 checks passed

rumzledz deleted the fix/2339-remove-permissions-validation branch October 29, 2024 09:48

iamsamgibbs mentioned this pull request Nov 4, 2024

fix: always use roles passed to contract for motions #3490

Merged

olgapod mentioned this pull request Dec 18, 2024

[Permissions] Redo actions button is shown on the action completed screen #3968

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix: Add form validation when removing permissions #2749

Fix: Add form validation when removing permissions #2749

rumzledz commented Jul 19, 2024 •

edited by rdig

Loading

arrenv left a comment

mmioana left a comment

mmioana Oct 28, 2024

mmioana Oct 28, 2024

rumzledz Oct 28, 2024

mmioana left a comment

iamsamgibbs left a comment

rdig left a comment

Nortsova left a comment

		validationSchema: ACTION_BASE_VALIDATION_SCHEMA,
		onSuccess: () => {},

Fix: Add form validation when removing permissions #2749

Fix: Add form validation when removing permissions #2749

Conversation

rumzledz commented Jul 19, 2024 • edited by rdig Loading

Description

Removing Root permissions from someone in a Parent domain

Removing permissions from someone in a subdomain

Testing

1. Selecting Remove Permissions for a member who has permissions for the selected team (Fry)

2. Selecting Remove Permissions for a member who does not have permissions for the selected team (jasmine-jolt)

3. Checking that the Remove permission option persists when changing the Team & Member fields (Fry)

4. Removing permissions from someone who has inherited permissions from the Parent domain (Fry)

5. Upgrading a user's role, when the user has an inherited role (alex-the-ace)

6. Downgrading a user's role, when the user has an inherited role (alex-the-ace)

7. Applying the same inherited role for a user (alex-the-ace)

8. Upgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)

9. Downgrading a user's role, when the user has inherited Custom permissions (diana-dynamo)

10. Applying the same inherited permissions for a user (diana-dynamo)

11. Checking if the inherited permission toggles are disabled (diana-dynamo)

12. Removing Root permissions from someone who does not have inherited permissions from a Parent domain (Amy)

13. Removing Root permissions from someone in a Parent domain (Fry)

14. Upgrading Custom permissions to a Role (diana-dynamo)

15. Checking the disabled state of Permissions toggles in the Completed Action component (eddy-edge)

Diffs

arrenv left a comment

Choose a reason for hiding this comment

mmioana left a comment

Choose a reason for hiding this comment

mmioana Oct 28, 2024

Choose a reason for hiding this comment

mmioana Oct 28, 2024

Choose a reason for hiding this comment

rumzledz Oct 28, 2024

Choose a reason for hiding this comment

mmioana left a comment

Choose a reason for hiding this comment

iamsamgibbs left a comment

Choose a reason for hiding this comment

rdig left a comment

Choose a reason for hiding this comment

Nortsova left a comment

Choose a reason for hiding this comment

rumzledz commented Jul 19, 2024 •

edited by rdig

Loading