Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Dashboard Sharing: unshared modules are blocked from sharing if owner loses capability #5354

Open
eclarke1 opened this issue Jun 15, 2022 · 10 comments
Assignees
Labels
P1 Medium priority Type: Bug Something isn't working

Comments

@eclarke1
Copy link
Collaborator

eclarke1 commented Jun 15, 2022

Bug Description

Bug bash issue: https://app.asana.com/0/1202258919887896/1202445528223665 please refer to Asana issue for background

If a managing user has their role changed or is deleted by another admin, that admin won't be able to "manage roles" if they are the only remaining admin. If there are 2 site administrators, and one of the administrators is removed or has their WP role downgraded the other admin will not be able to change Dashboard Sharing roles, while the option to manage view access doesn't appear.

Recording of experience

I think we could detect that the "managing user" is no longer an admin and show a different notice here, which would probably be best. It might even warrant a general error notice, not sure 🤔

Steps to reproduce

  1. Setup SK without Analytics (only SC) on a site with 2 administrators
  2. Login as the other administrator (adminB), set up SK and connect the Analytics module
  3. Check the dashboard sharing options. You'll notice that SC is managed by adminA while you (adminB) are managing the Analytics module.
  4. Log back in to the site from adminA and change the role of adminB from "administrator" to "Editor"
  5. Open the dashboard sharing options. It's now not possible to change the dashboard sharing roles, while the "Who can manage view access" options don't appear

image.png


Do not alter or remove anything below. The following sections will be managed by moderators only.

Acceptance criteria

Implementation Brief

Test Coverage

QA Brief

Changelog entry

@eclarke1 eclarke1 added P0 High priority Type: Bug Something isn't working labels Jun 15, 2022
@aaemnnosttv
Copy link
Collaborator

This should be covered by the existing functionality for module recovery. I don't think we should further complicate the interface here to facilitate module recovery but we could adjust the language if the module were recoverable. IMO this isn't launch blocking.

@eclarke1
Copy link
Collaborator Author

Changing to P1 then to reflect this as a nice to have

@eclarke1 eclarke1 added P1 Medium priority and removed P0 High priority labels Jun 15, 2022
@aaemnnosttv aaemnnosttv self-assigned this Jun 23, 2022
@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Jun 23, 2022

@felixarntz @marrrmarrr this is an odd case because the module isn't shared yet, so it isn't recoverable. In the user's experience they'll not be able to share it because it's still technically owned by another user, even though that user is really no longer capable of being the module owner.

In this case, it's more of a case of module take over, similar to how we handle shared ownership modules although we'd only want to allow it if the user had access to the configured entity. If they did have access, we could consider allowing them to take over by enabling sharing. This way they wouldn't be blocked from enabling sharing, but otherwise no takeover would be needed.

Said another way, if a module owner loses their ability to share the module and sharing isn't enabled yet, we could resolve that modules sharing management value to "all admins" allowing any other admin signed in with Google to enable sharing for it via the settings. For these users, we would check if they have access to the current configured entity and if so we could enable the interface for managing shared roles. If they enabled sharing by adding roles, we could make them the new module owner on save. This would probably require tweaking the UI a bit and maybe adding a new case for the language at the bottom but I'm not sure how else we might address this other than perhaps updating the logic for modules that are considered shareable to also require their owner is capable of sharing. The latter is the easier change but makes it harder for users to know what needs to be done to enable sharing for such a module.

@aaemnnosttv
Copy link
Collaborator

As discussed yesterday with @felixarntz and @marrrmarrr, the plan is to first enhance the module settings screens to allow for module takeover, even if the user does not have access to the current configuration. See #5496

Then the sharing settings interface could be enhanced here in such a situation, if the current user had access to the configured entity, there could be a single-click button to take over which would then be replaced by the user role select, allowing the user to enable sharing. If the user did not have access, this could link to the settings edit view for the module instead where it could be reconfigured, which would have the side-effect of taking ownership, that would then allow them to enable sharing if desired.

@FlicHollis
Copy link
Collaborator

Hi @aaemnnosttv what is the next steps here? Can this be moved to IB?

@aaemnnosttv
Copy link
Collaborator

@FlicHollis this is blocked by #5496 so we need to wait for that one to at least get AC and maybe even an IB before we can start moving this one along.

@marrrmarrr
Copy link
Collaborator

@aaemnnosttv left a comment on #5496 on how to handle that corner case. Once we finalise how that should look like, I think we can either link to that state from the dashboard sharing modal, or directly to the "redo setup" flow.

@bethanylang
Copy link
Collaborator

@aaemnnosttv Following up on this one, thanks!

@aaemnnosttv aaemnnosttv changed the title Managing user has role changed, they cannot manage roles as the remaining admin Dashboard Sharing: unshared modules are blocked from sharing if owner loses capability Mar 31, 2023
@aaemnnosttv
Copy link
Collaborator

I've updated the title here to be a bit more clear. #5496 is still the one we need to do first to unblock this one.

@ivonac4
Copy link
Collaborator

ivonac4 commented Jan 8, 2024

@aaemnnosttv Should this one stay assigned to you to work on it soon or you want someone else to pick it up?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P1 Medium priority Type: Bug Something isn't working
Projects
None yet
Development

No branches or pull requests

7 participants