New Permissions Pages

[alxnddr]

Description

This PR replaces the old Permissions UI with the new one that should make viewing/editing permissions easier especially for customers with a lot of data. It should behave the same in terms of business logic, only the UI has changed.

Screenshots

Collections permissions

Before

After

Data permissions

Before

After — groups focus

After — databases focus

Collection permissions modal

Before

After

Snippet folder permissions modal

Before

After

Warnings

Closes #4186
Closes #8165
Closes #10874
Closes #10920
Closes #12489
Closes #14669
Closes #14671
Closes #17185
Closes #15233

[alxnddr]

@metabase-bot run visual tests

[alxnddr]

@metabase-bot run visual tests

[kdoh]

Overall looking quite solid. There are a couple things I noticed while playing with this that I'd consider tweaking.

1. I think we need a more obvious sense that the database names in the top level group based view are clickable. Think it could be worth using text-decoration: underline here just to be super obvious that you can drill down further.

2. It's not clear which database I'm looking at when I've drilled through into a DB or schema in the group view. We might want to consider adding breadcrumbs here above the "Schema name" label just to help make it easier to scoot around. This is especially true when you get linked to that screen when selecting which tables you want to limit for a particular group as I mention in the next point.

3. I've noticed some jumpiness in the top of the permission page when I select "Limit access" and then I get taken over to the group view. It's like the confirm save bar pops in for a second and then disappears. I tried capturing it in this recording.

permissions-ui-flashing.mov

[alxnddr]

@kdoh I've addressed all the points, thanks!

[codecov]

Codecov Report

Merging #17328 (8b72fd5) into master (4b0afb6) will decrease coverage by 24.30%.
The diff coverage is 24.65%.

@@             Coverage Diff             @@
##           master   #17328       +/-   ##
===========================================
- Coverage   62.46%   38.15%   -24.31%     
===========================================
  Files        1639     1240      -399     
  Lines       65735    33141    -32594     
  Branches     7258     4918     -2340     
===========================================
- Hits        41061    12646    -28415     
+ Misses      21507    19772     -1735     
+ Partials     3167      723     -2444     

Flag	Coverage Δ
back-end	?
front-end	38.14% <24.65%> (+0.08%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
...ntend/src/metabase-enterprise/sandboxes/actions.js	0.00% <0.00%> (ø)
...base-enterprise/sandboxes/components/GTAPModal.jsx	0.00% <0.00%> (ø)
...rontend/src/metabase-enterprise/sandboxes/index.js	0.00% <0.00%> (ø)
...frontend/src/metabase-enterprise/snippets/index.js	0.00% <ø> (ø)
...ionPermissionsModal/CollectionPermissionsModal.jsx	0.00% <0.00%> (ø)
...issionsModal/CollectionPermissionsModal.styled.jsx	0.00% <0.00%> (ø)
...components/PermissionsEditor/PermissionsEditor.jsx	0.00% <0.00%> (ø)
...ents/PermissionsEditor/PermissionsEditor.styled.js	0.00% <0.00%> (ø)
...PermissionsEditor/PermissionsEditorBreadcrumbs.jsx	0.00% <0.00%> (ø)
...sionsEditor/PermissionsEditorBreadcrumbs.styled.js	0.00% <0.00%> (ø)
... and 529 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4b0afb6...8b72fd5. Read the comment docs.

[alxnddr]

@metabase/frontend-developers it is a big one but I split it into granular commits to make the review a bit easier. Please tell if you'd prefer me to create multiple PRs

[flamber]

Just did a quick test of e44177c - here's what I've found:
Looks really nice, but takes a while to get used to it.

1. Clicking the database/schema/table links fails - they are /admin/permissions/data/database1 instead of /admin/permissions/data/database/1
   
2. The table search also search schema, but it's a bit strange, when there's overlaps
   
   
3. I'm still seeing Weird UX around setting limited data permissions #15233
4. If I redo Sandbox modal "cancel" doesn't work correctly #14669, then I can get to a state, where the table is sandboxed, without me defining anything in the sandbox (clicked cancel) and then saved permissions, but now it's not possible to edit the sandbox settings - nothing opens. This might have something to do with sandbox modal sends a GTAP request, which is separate from the overall permission Save request, so it's possible to get into a weird state (though this has always existed and doesn't expose any data incorrectly).

[kulyk]

Tremendous work @alxnddr 👏
Have some general UX feedback here (cc @kdoh)

Discard changes popup

The popup is a little bit confusing. If I have unsaved changes and try to leave a page or change a tab, I see a popup saying "your changes will be discarded if you leave" with two options: "Cancel" and "Discard changes". I expected "Cancel" to close the popup, so I can save my changes and "Discard changes" to leave the page without saving the changes, but it works vice-versa. I guess it doesn't align with the common UX pattern for confirmations like these

Video Demo

confusing-discard-popup.mov

Updating permissions for child collections

When I change permission for a collection, the popover has a toggle to apply the changes to child collections too. The toggle is placed below the permission options, so I tend to select the permission first and then select the toggle. However:

1. The popover closes once I select the permission level, so I need to open it for the second time to change the toggle
2. If I actually reopen the popover and click the toggle, it won't apply the change to children. It only works if I first change the toggle and then change the permission level. I'm just worried it might confuse people

Video Demo

children-update-ux.mov

[alxnddr]

@metabase-bot run visual tests

[alxnddr]

@flamber @daltojohnso @kulyk thank you for reviewing this PR and for your feedback, I hope it was not that painful. I addressed your comments, could you please take a look again?

@kulyk yes, this toggle is really inconvenient, thanks for pointing. If you don't mind, I'll fix it in a separate PR, because it worked the same way before.

@daltojohnso I added a bit of tests on selectors, but previously they contained some mutating logic. Instead, I have cypress specs for that, but the coverage can be increased. Would you mind if I add more tests in a follow-up PR?

[flamber]

Testing 909e299
When I try to edit a sandbox, then it show 404 and changes to URL to this:
/undefined/segmented/group/[object Object]

[alxnddr]

@flamber oh, seems like I broke it during some refactoring while addressing review comments. Fixed, added a spec to prevent breaking it again. Thanks for catching it!