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

Extend Site Health info for Dashboard Sharing #4534

Closed
aaemnnosttv opened this issue Dec 9, 2021 · 11 comments
Closed

Extend Site Health info for Dashboard Sharing #4534

aaemnnosttv opened this issue Dec 9, 2021 · 11 comments
Labels
P0 High priority PHP Type: Enhancement Improvement of an existing feature

Comments

@aaemnnosttv
Copy link
Collaborator

aaemnnosttv commented Dec 9, 2021

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

  • Add a row after "Active Modules":
    • Recoverable Modules: {comma-separated list of recoverable module names}
      • The debug value should be a comma-separated list of module slugs
  • Add rows for all shareable modules after the "Features" row and before the rows for each module
    • {Module Name} Shared Roles: {comma-separated list of role names}
      If there are no shared roles, use None (be sure to use proper i18n, e.g. "Active Modules")
      • The debug value should be a comma-separated list of role IDs
    • {Module Name} Management: {"Owner" or "All admins signed in with Google"}
      • The debug value should be the raw setting value, or owner if none

Implementation Brief

  • Within Debug_Data.php:
    • In the get_fields() method, after the call to get_active_modules_field(), add a call to a new method get_recoverable_modules_field() that adds recoverable modules to $fields array as per the AC.
    • This new method should be similar to the get_active_modules_field(). Use Modules::get_recoverable_modules() and format the modules as per the AC.
    • Create another method, say get_module_sharing_settings_fields() and fetch all shareable modules using Modules::get_shareable_modules(). For each of these modules, create two arrays, each containing the label, value and debug keys. The first array will contain the Shared Roles and the second should contain the Management setting, formatted as per the AC. To find the shared roles and management settings, use Module_Sharing_Settings::get(). Return an array of all the arrays generated above (2 arrays per module). Essentially, the resultant array should be formatted such that it can be directly used in the array_merge() call within the get_fields() method. Merge the new array of fields between the existing $fields and $this->get_module_fields() within that array_merge(). This will add 2 new rows for every shareable module as per the AC.

Test Coverage

  • No new tests are required, but the existing Debug_DataTest.php will have to be updated to cater for the new debug fields.

QA Brief

  • Enable the dashboardSharing feature flag.
  • Change the sharing settings for the modules.
  • Go to WordPress Site Health ( Tools -> Site Health) -> Site Kit by Google.
  • Check the info is displayed as per the AC.

Changelog entry

  • Add Dashboard Sharing's Active Modules and Sharable Modules to Site Health.
@mxbclang
Copy link

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

  • ** Product *** dashboard sharing active: Yes || No
  • ** Product *** dashboard sharing - who can view: NA || Administrators, Editors, Authors, Contributors
  • ** Product *** dashboard sharing - manage view access: NA || Only me || Admins
  • ** Product *** dashboard sharing - module owner:

@aaemnnosttv
Copy link
Collaborator Author

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
Copy link
Collaborator Author

@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
Copy link
Member

@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
Copy link
Collaborator Author

@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 eclarke1 added P0 High priority and removed P1 Medium priority labels Jun 16, 2022
@eclarke1
Copy link
Collaborator

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
Copy link
Member

@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
Copy link
Collaborator Author

I added another row to list recoverable modules but otherwise this should be good to go 👍

@tofumatt tofumatt self-assigned this Jun 20, 2022
@tofumatt
Copy link
Collaborator

IB ✅

@tofumatt tofumatt removed their assignment Jun 21, 2022
@asvinb asvinb self-assigned this Jun 21, 2022
@aaemnnosttv
Copy link
Collaborator Author

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

@asvinb asvinb removed their assignment Jun 23, 2022
@jimmymadon jimmymadon assigned jimmymadon and asvinb and unassigned jimmymadon Jun 23, 2022
@asvinb asvinb assigned aaemnnosttv and unassigned asvinb Jun 24, 2022
@eclarke1 eclarke1 assigned tofumatt and unassigned aaemnnosttv Jun 27, 2022
@tofumatt tofumatt removed their assignment Jun 27, 2022
@wpdarren wpdarren self-assigned this Jun 28, 2022
@wpdarren
Copy link
Collaborator

QA Update: ✅

Verified:

  • After "Active Modules" there is a new row titled Recoverable Modules, which when the recovery alert appears, is populated with either or the Idea Hub and PSI slugs. When there are no recoverable modules then 'None' appears.

  • After the "Features" row there is a new row titled Shared Roles: for each module connected, which has a comma-separated list of role names. When there are no shared modules then 'None' appears.

  • After the "Shared Roles" row this is a new roe titled Management: for each module connected, which has an entry depending if the access is given to other administrators or not. The entries appear as "Owner" or "All admins signed in with Google" based on the dashboard settings modal.

Screenshots

image
image
image

@wpdarren wpdarren removed their assignment Jun 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
P0 High priority PHP Type: Enhancement Improvement of an existing feature
Projects
None yet
Development

No branches or pull requests

9 participants