Improve app access setting flow

Follow-up from !42 (comment 46903):

So I tried it again and this is my observation:

1. Start with a user with no app access at all
2. Set user admin role --> All app access roles are set to admin
3. Set user user role --> All app access roles are still set to admin

This is concerning because when you don't pay much attention, and only focus on taking away user admin role for a user, the user would then still have admin roles for all apps.

So this means by setting the user admin role the app access roles are persisted to the db. I thought they weren't and the backend would just grant all admin users admin access on the fly, no matter what app access was configured. I'd like to hear a second opinion on the desired workflow here. @arie ?

@arie wrote:

Yes I do agree: I would prefer if switching to admin role (or back to regular user) would not make any changes to app access items in the database. In my opinion that would require another UI change though, where the whole list of apps and access permissions is hidden in case of an admin user, and only the new banner with the message is shown -- otherwise this would be too confusing for the user.

I thought they weren't and the backend would just grant all admin users admin access on the fly, no matter what app access was configured.

If I understand correctly, that's still what happens in the backend code. However it additionally sets all app permissions to "Admin", I guess because then the interface actually reflects the "virtual"/"bypass" admin access.

@davor wrote:

Yes I agree with all said. That's why I said to hide all apps permission when the user is admin so that it doesn't confuse the user. But it comes to some UX practices

mentioned in issue #72 (closed)

mentioned in issue #69

changed milestone to %1.0

added weight::0.5 label

mentioned in issue stackspin#1214 (closed)

@maarten do you want to

• change all roles according to Dashboard role
  • when the user is admin - all app roles are set to admin and cannot change
  • when dashboard role is set to user - all app roles are set to user but you can change it
  • when dashboard role is set to no access - all app roles are set to no access but you can change it

Tell me what do you think.

when dashboard role is set to no access

This situation is impossible, dashboard roles can only be "user" or "admin"

• when the user is admin - all app roles are set to admin and cannot change
• when dashboard role is set to user - all app roles are set to user but you can change it

Yes

Yes, it can't be no access for dashboard. Ok, so if this is ok, I can start working on this after stackspin#1214 (closed) if that's ok.

yes, that's great

I've assigned this issue to you to make that clear.

Is it ok if this implementation is also a part of batch create or do you want me to create separate MR for that? There is just a small change on FE.

Because it's just you an me working on this (mostly just you ) doing both in 1 MR is fine IMO

assigned to @davor

added Doing label

mentioned in merge request !48 (merged)

We discussed this at length and then concluded that this issue as a "what if?" scenario is not super important for us to focus on right now. We'll await user feedback to see what seems logical and what doesn't.

Should I remove changes in !48 (e830a095) for this?

Current flow:

• user dashboard role is set to user -> all app access roles are set to user but it can be changed
• user dashboard role is changed to admin -> all app access roles are set to admin and it can't be changed
• user dashboard role is changed to user -> all app access roles are set to admin but it can be changed

Flow in the MR:

• user dashboard role is set to user -> all app access roles are set to user but it can be changed
• user dashboard role is changed to admin -> all app access roles are set to admin and it can't be changed
• user dashboard role is changed to user -> all app access roles are set to user but it can be changed

Thanks, but no.

I think it's fine now that it's already implemented. I think !48 (merged) makes it a bit more intuitive than the situation described in this issue and we'll keep it at that for now.

unassigned @davor

removed Doing label

changed milestone to %Backlog

closed

Closed by !48 (merged)