fix: permissions inherited roles visability

[Nortsova]

Description

Funny thing, this bug has been fixed at least three times, but it’s still present. 😁

Here is previous fixes for reference: #2971 , #3024 , #2485

I decided to use the implementation from my last PR, but if you have any concerns, I’m open to refactoring. 🙌

Testing

Step 1. Assign Admin permissions to fry in Andromeda
Step 2. Assign Payer permissions to fry in General
Step 3. Assign Payer permissions to amy in Andromeda
Step 4. Assign Admin permissions to amy in General
Step 5. Navigate to the permissions page to check fry's and amy's roles in Andromeda
Step 6. fry should be an admin, amy should be an admin with inherited pill

Step 7. Install multi-sig extension
Step 8. Assign Admin permissions to amy in Serenity
Step 9. Assign Payer permissions to amy in General
Step 10. Assign Payer permissions to fry in Serenity
Step 11. Assign Admin permissions to fry in General
Step 12. Navigate to the permissions page to check fry's and amy's roles in Serenity
Step 13. amy should be an admin, fry should be an admin with inherited pill

Step 14. Add Bella as a Payer in team Andromeda
Step 15. Add Bella as an Admin in team Serenity
Step 16. Add Alex as an Owner in team General
Step 17. Change Alex permission in team Andromeda to Arbitration only (Custom)
Step 18. Set the Permissions page filter to All Teams
Step 19. Verify that:

• You don't see the "Inherited" badge on any user tile
• Leela & Alex are both under the Owner category
• Bella is under the Admin category, because this is the highest permission they across all subdomains

Step 20. Set the Permissions page filter to General
Step 21. Verify that:

• You don't see Bella in the list
• Leela & Alex are both under the Owner category and their tiles do not have the "Inherited" badge

Step 22. Set the Permissions page filter to Andromeda
Step 23. Verify that:

• Both Leela and Alex are under the Owner category and they both have the "Inherited" badge
• Bella is under the Payer category and her tile does not have the "inherited" badge

Step 24. Set the Permissions page filter to Serenity
Step 25. Verify that:

• Both Leela and Alex are under the Owner category and they both have the "Inherited" badge
• Bella is under the Admin category and her tile does not have the "inherited" badge

Thanks to @bassgeta for the case with custom roles:

Step 26. Assign fry custom Administration role in General
Step 27. Assign fry Architecture, Arbitration and Funding in Andromeda
Step 28. Set the Permissions page filter to Andromeda
Step 29. Verify that fry is under Admin role with "inherited" badge
Step 30. Set the Permissions page filter to All teams
Step 31. Verify that fry is under Admin role without "inherited" badge

Resolves #2785

[rdig]

Is this ready for review? Or is it still being worked on?

[Nortsova]

Is this ready for review? Or is it still being worked on?

@rdig It is ready now, sorry I spent some time on the test cases. 🙌

[bassgeta]

I still have no idea how this disappeared, but I hope it's gonna stay in here permanently now :D
Alright so, I think you almost got it, we somehow need to merge the roles here and mark them as inherited.
Lemme provide breaking steps:

1. Assign fry just the Administration role in General
   
2. Assign fry Architecture, Arbitration and Funding in Andromeda
   
3. Go to the permissions page and choose Andromeda as the filter, fry is shown as Mod not as an Admin
   

I think the issue is with the line I've dropped a comment on, we need to merge inherited + current domain permissions, and we can still have the isInherited to true

Attaching some logs from my debug session

Log 1 is here

Log 2 is here

However, the popup works correctly for this, so at least we don't to fix that 😅

[bassgeta]

I think we need to merge both currentTeamPermissions with inheritedPermissions here, or somehow return both to that function (something similar to how PermissionTooltip and PermissionTooltipContent work like)

[Nortsova]

yep, it was it, thank you so much for your finding! fixed 🥳

[bassgeta]

Thanks for the quick turnaround on my comment, I think we're basically ready to go 🤞 ??
Thanks for the thorough testing steps, really helped with testing this a ton!
So I won't spam too many images I'll just note down the cases
leela - Owner in General, just Architecture in Andromeda
amy - Payer in General, Admin in Andromeda and just Architecture in Serenity
fry - Admin in General, Payer in Andromeda
chris-coder (the most annoying test case) - Funding and Arbitration in General and then Administration in Andromeda and Administration + Architecture in Serenity

Also should we somehow save this configuration for all further permission combo testing? :D

Alright so test 1, amy should be an admin since it's All teams and chris-coder is and admin somewhere by some combination ✔️

2. amy is a Payer in General and chris-coder is Custom , leela and Owner, fry and Admin ✔️
   

3. In Andromeda fry is an Admin still (even though we "downgraded" him in that domain, he inherits from General) , amy is compounded into an admin due to inheritance, chris-coder is compounded into a payer, leela inherits the owner
   

4. In Serenity fry is an inherited Admin, leela an inherited Owner, chris-coder is compounded into an Admin and so is amy
   

so all in all, great job 😎 I've also checked Multisig, buuuuut I'm not gonna spam it with even more pictures, since the cases are the exact same :D

[rdig]

Pfew! This was a pain in the a to test. I can only imagine you wanting to get in done already as it's hanging about for like 3 months :)

Before I get into it I want to mention that step 12 is wrong

Step 12. Navigate to the permissions page to check fry's and amy's roles in Andromeda

Step 12 is wrong, it should say "Serenity"

Another thing is that I found some bugs, but I just want you to open separate issues for them, not fix them here. (see at the bottom)

As for your PR everything seems to be working as expected, so nice job navigating the intricacies of permissions 💯

Direct and inherited "normal" permissions:

Direct and inherited Multisig Permissions:

Bella and Alex admins and owners:

Fry custom permissions:

---

While reviewing this I found two bugs, but in the spirit of getting this resolved once and for all, I'll point them out, but I'll ask you to open separate issues for each one, so that it doesn't pollute this one.

1. User popover cuts off the reputation percentage points

2. Redo-ing a multisig permissions actions from the list will re-do it with normal permissions, not multisig as expected (but will work correctly when re-doing from the action sidebar)

Screencast.from.2024-10-17.16-13-40.mp4

[Nortsova]

Thanks @rdig ! ✨ Yes, the testing steps were the most painful here 😂

Created issues as you asked 🙌:
#3381
#3382

I also created this bug during current tests:
#3344

[Nortsova]

Hey, I had a call with @olgapod today, and it inspired me to simplify the testing steps so we can transfer these test cases to auto-tests.

Here are 3 cases that should be tested. (Then repeat it with multi-sig 🥲)

1. Roles inheritance

• user has permissions just in the parent domain
• user has permissions just in the child domain (and nothing in the parent)
• user has more permissions in the child domain
• user has more permissions in the parent domain (so we have some inherited)

2. All teams should always show the highest user role

3. Merging roles

• user has 1 permission in the child domain and 1/2 in the parent, which we need to merge
• user has 1/2 permissions in child and another 1/2 in parent, which we need to merge

Step 1. Assign amy Payer permissions in General
Step 2. Assign fry Admin permissions in Andromeda
Step 3. Navigate to the Permissions page and Verify that:
Step 4. All teams: amy payer, fry admin
Step 5. General: amy payer, fry doesn't show up
Step 6. Andromeda: amy payer (inherited), fry admin
Step 7. Serenity: amy payer (inherited), fry doesn't show up

Step 8. Assign amy Admin permissions in Serenity
Step 9. Assign amy Mod permissions in Andromeda
Step 10. Assign fry Owner permissions in General
Step 11. Navigate to the Permissions page and Verify that:
Step 12. All teams: amy admin, and fry owner
Step 13. General: amy payer, fry owner
Step 14. Andromeda: amy payer (inherited), fry admin
Step 15. Serenity: amy admin, fry admin (inherited)
Step 16. Rocinante: amy payer (inherited), fry admin (inherited)

Let's try Custom roles now

Step 17. Assign chris Administration (Custom) permissions in General
Step 18. Navigate to the Permissions page and Verify that:
Step 19. All teams: chris has Mod role
Step 20. Assign chris Funding (Custom) permissions in Andromeda
Step 21. Navigate to the Permissions page and Verify that:
Step 22. All teams: chris has Custom role
Step 23. Assign chris Funding and Arbitration (Custom) permissions in Serenity
Step 24. Navigate to the Permissions page and Verify that:
Step 25. All teams: chris has Payer role
Step 26. Assign chris Architecture, Funding and Arbitration (Custom) permissions in ** Rocinante**
Step 27. Navigate to the Permissions page and Verify that:
Step 28. All teams: chris has Admin role
Step 29. General: chris has Mod role
Step 30. Andromeda: chris has Custom role (inherited)
Step 31. Serenity: chris has Payer role (inherited)
Step 32. Rocinante: chris has Admin role (inherited)

Sorry it is not less, but at least more structured than previous steps.

By the way, there is a bug in Step 14. fry doesn't have an inherited pill, but is located under the owner category, with an Admin role in Andromeda, I raised this question and will inform you about the solution as soon as we find one. So maybe these test steps will be updated.

@bassgeta could you please verify these test cases?

[Nortsova]

Okay, so any subdomain Owners should be shown under the Admin category, so I created a ticket to avoid scope creep here: #3397

Thanks @bassgeta for assistance :)

[iamsamgibbs]

Nice work figuring this out! There are still issues with the "Owner" permissions appearing in the sub-domains as you have already noted which does through off some of the inherited permissions - but otherwise this is working great and we can fine tune it with the next PR.

I won't spam all the screenshots here, but I followed through the testing steps without issue (other than Owner appearing where it shouldn't).

Also did a tiny bit of multi-sig testing too, this is probably worth a more thorough test at some point, but seems to be working well as far as I can tell!