google / site-kit-wp

Site Kit is a one-stop solution for WordPress users to use everything Google has to offer to make them successful on the web.
https://sitekit.withgoogle.com
Apache License 2.0
1.25k stars 290 forks source link

Extend Site Health info for Dashboard Sharing #4534

Closed aaemnnosttv closed 2 years ago

aaemnnosttv commented 2 years ago

Feature Description

Site Kit's site health report contains valuable information for our support team to help users troubleshoot problems they are experiencing. Including dashboard sharing configuration in the report will be essential to providing support once the feature is rolled out.


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

Acceptance criteria

If dashboardSharing is enabled, Site Kit's site health information should be extended with the following information

Implementation Brief

Test Coverage

QA Brief

Changelog entry

mxbclang commented 2 years ago

@aaemnnosttv Noting that James had created a similar issue, #4440, which requested these specific details in SH for support:

aaemnnosttv commented 2 years ago

Thanks @bethanylang, I wasn't aware of that one but I see you've closed it so we'll plan on using this one going forward. This issue is just a placeholder for now but we'll be sure to work with you to clarify what exactly we want to include here before we start working on it šŸ‘

aaemnnosttv commented 2 years ago

@felixarntz I've added some initial ACs here to get this going, let me know what you think.

As for capabilities, which do you think we should include here? I feel it would probably be too much to list out all of the read/manage/delegate meta caps for each shareable module (at least on individual lines). Maybe we could aggregate these into a single line for each?

Site Health is limited to admins as well so we wouldn't be able to capture it for a view-only user.

felixarntz commented 2 years ago

@aaemnnosttv ACs so far lgtm. I think it would be okay to omit the module-specific capabilities entirely, since to a degree they would be represented already indirectly by what roles each module is shared with.

I think regarding capabilities for the current user it would be enough to ensure all higher-level (i.e. not meta capabilities) are present, including the newer ones like VIEW_AUTHENTICATED_DASHBOARD and VIEW_SHARED_DASHBOARD.

aaemnnosttv commented 2 years ago

@felixarntz actually, regarding the capabilities, these are all currently included when dashboard sharing is enabled. Should we instead change this to limit the capabilities listed here to not include the meta caps as you said?

image
eclarke1 commented 2 years ago

In the bug bash review meeting yesterday, we agreed for this one to be a launch blocker, so I have updated to P0. It is currently in Backlog, so I have moved to ACs. Please let me know if there's any concerns with this.

felixarntz commented 2 years ago

@aaemnnosttv Sorry for the late reply, I had missed your latest comment.

That's neat that these are already included. In that case, I'd say no need to intentionally exclude them. I don't think they're as critical, but arguably for Site Health the more information the better.

aaemnnosttv commented 2 years ago

I added another row to list recoverable modules but otherwise this should be good to go šŸ‘

tofumatt commented 2 years ago

IB āœ…

aaemnnosttv commented 2 years ago

@asvinb please note the change to the label for management as this has been changed in the UI as well.

wpdarren commented 2 years ago

QA Update: āœ…

Verified:

Screenshots ![image](https://user-images.githubusercontent.com/73545194/176167822-2d5b0129-d050-4ced-a671-4d6dbe31a348.png) ![image](https://user-images.githubusercontent.com/73545194/176167841-17588330-9997-48c7-a9e8-642e83342388.png) ![image](https://user-images.githubusercontent.com/73545194/176167871-b2060c3e-b478-4b0a-a544-e296099c9505.png)