Invalid language in "Insufficient permissions" errors for non-authenticated users

[jamesozzie]

Bug Description

When sharing a view only version of a dashboard, Analytics permission errors (and possibly others) are output in the view only dashboard. This includes for administrators (if they selected the view only dashboard) and for non administrator roles.

lz8fRYyLcb.mp4

This is similar to what was reported previously: #5551

Steps to reproduce

1. Setup SK and connect Analytics. When connecting Analytics connect it to a property that another user has access to.
2. Ensure the dashboard loads as expected, showing Analytics data or the zero state.
3. From analytics.google.com remove access to the connected property (from another Google account, alternatively delete the connected property)
4. Reload the Site Kit dashboard (you may need to wait 1 hour due to caches responses. You an switch the reporting period if so)
5. You'll notice the permissions error.
6. Share the dashboard (specifically Analytics) with other users (admins or other roles)
7. Login as one of those users. The error appears

Screenshots

Additional Context

• SK 1.88.0
• Latest version of the Tester plugin
• For administrators the "Request access" button also appears, even though the administrators are not signed into Google
• For editors the "Request access" button does not appear

---

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

Acceptance criteria

• The error shown for insufficient permissions on the dashboard should use alternate language for non-authenticated users:

Heading

Access lost to {Module Name}

Body

The administrator sharing this module with you has lost access to the {Google Service} service, so you won’t be able to see stats from it on the Site Kit dashboard. You can contact them or another administrator to restore access.

• For authenticated admins the language should remain as today
• The button to Request access from a report error should only be shown to authenticated admins

Implementation Brief

• In assets/js/components/ReportError.js:
  • Update the getMessage() function:
    • Before executing the existing functionality, check if the current user is a view-only user. This can be done by using the useViewOnly() hook. If so:
      • Set the title variable as mentioned in the ACs as the heading (translatable).
      • From the function, return the string as mentioned in the ACs as the body (translatable).
      • Both {Module Name} and {Google Service} placeholders in the ACs should be replaced with the name of the module (module.name).
  • Inside the return statement, display the div.googlesitekit-error-cta-wrapper element only if the current user is not a view-only user.
  • Add a Storybook story for the case where there is a ReportError with insufficient permissions but for a view-only user.

Test Coverage

• In assets/js/components/ReportError.test.js:
  • Add test case(s) to cover the above scenario.

QA Brief

• Following Steps to reproduce make the error appear
• Check new text for view-only users
• Verify the "Request access" button doesn't appear for a non-authenticated administrator
• Verify that the language change only shows up if it is an "Insufficient permissions" error.

Changelog entry

• Fix presentation of errors from Google APIs shown to view-only users.

[jamesozzie]

I'm unable to recreate this on other sites, nor can @adamdunnage . Tested on TasteWP and InstaWP setups. Happy to share a login to a site to the site where this does appear.

[jamesozzie]

Steps to reproduce updated, with recording of experience below:

ApqL3qrNy2.mp4

[marrrmarrr]

@aaemnnosttv IIRC the general direction we took with regard to a module becoming "unshared" after it's been shared for view-only previously was to just remove it from the view-only dashboard (without showing a notice that "admin XYZ has unshared this" or similar). So having this error deviates from this direction.

Or is your thinking more that this is not "unsharing", due to e.g. the admin account being deleted or themselves losing access for whatever reason, so won't fall in the set of cases described above? If that is so, then at least we should point people to some way out. E.g. "contact another admin of the site to see if they can restore access" or something along these lines?

[aaemnnosttv]

IIRC the general direction we took with regard to a module becoming "unshared" after it's been shared for view-only previously was to just remove it from the view-only dashboard (without showing a notice that "admin XYZ has unshared this" or similar). So having this error deviates from this direction.

It's easy for some of the language to get a bit mixed as there are some similar but different things happening conceptually so I'll try to clarify.

When you say "unshared" – to me that means that the module owner went into the sharing settings and removed a shared role from the module. In that case, the view only users would no longer see anything on their dashboard from that module, so that much has not changed.

When a shared module is no longer shareable (i.e. sharing is enabled but interrupted), based on information that we have in WP alone (not considering external API access), then we consider that module to be recoverable. In that case, the module is still shown on the dashboard but with informational (non-error) placeholders. See #5376.

This issue is about a separate scenario from those above where sharing is enabled, and the module owner has lost access to the external entity on the Google service. This means the module owner would see the same insufficient permissions error as well. We don't store the concept of whether or not a user's access token has access to a given service/entity as this would further complicate things in a non-trivial way.

Or is your thinking more that this is not "unsharing", due to e.g. the admin account being deleted or themselves losing access for whatever reason, so won't fall in the set of cases described above? If that is so, then at least we should point people to some way out. E.g. "contact another admin of the site to see if they can restore access" or something along these lines?

Yes – this is an external condition that we won't know for sure until requests are made. As for guiding the user, that's mostly what this issue is about (adjusting the language) so I'm open to any suggestions you have for what you'd prefer to see there. The current language can be seen here https://google.github.io/site-kit-wp/storybook/develop/?path=/story/components-reporterror--report-error-with-insufficient-permissions which does offer some guidance but is specific to "your Google account" which is of course invalid in the context of a view-only user. I've suggested we reword this to be "The sharing administrator's Google account" with the rest of the message the same. Let me know what you think and how we can improve this?

[marrrmarrr]

In that case I think we can adapt the language here to make it clear it's not about "insufficient permissions" (since the users never had these permissions to begin with, and the point of dashboard sharing is that they don't need these permissions to see the dashboard).
So how about this:
Heading: Access lost to $MODULE
Text: The administrator who originally shared this module with you has lost access to the $SERVICE service, so you won’t be able to see stats from it on the Site Kit dashboard. You can contact them or another administrator to restore access and share again.

[aaemnnosttv]

Thanks @marrrmarrr – a few suggestions for your consideration:

Heading: The administrator who originally shared this module with you

This is potentially inaccurate because the administrator who originally shared the module could be different than the admin who is currently sharing (e.g. changing connection or recovering the module).

How about: The administrator sharing this module with you ?

You can contact them or another administrator to restore access and share again.

Sharing doesn't necessarily need to be done again so we could simplify it to:

You can contact them or another administrator to restore access.

[marrrmarrr]

@aaemnnosttv thanks for clarifying, both suggestions SG. So we're left with:
Access lost to $MODULE
The administrator sharing this module with you has lost access to the $SERVICE service, so you won’t be able to see stats from it on the Site Kit dashboard. You can contact them or another administrator to restore access.

[aaemnnosttv]

Great, I'll update the AC with this and move it forward! Thanks @marrrmarrr 👍

[eugene-manuilov]

IB ✔️

[felixarntz]

@sashadoes @nfmohit @techanvil Looking at https://github.com/google/site-kit-wp/pull/6292/files#diff-b95a49b3fe9c5fd7c5b5f782eef76c5c71fd74299f4bdce4a7156144db48d621R90, I'm questioning whether the logic there is right? The ACs state that "The error shown for insufficient permissions" should change. But the logic in the PR puts the check before the check for whether the error even is an insufficient permissions error.

To be fair, that part already seems incorrect in the IB. Please take a look.

cc @eugene-manuilov @aaemnnosttv

[nfmohit]

Thank you for noticing and for pointing it out, @felixarntz! I've opened #6337 to address this.

[mohitwp]

QA Update ❌

@nfmohit

I've noticed that for authenticated users if user lost access to internet then the error description is correct -

Heading - Data error in Analytics
Body - You are probably offline.

But for non authenticated users (View -only dashboard) error description is same as we are showing in case of insufficient permission error and Retry button is not showing. Also, the title is not bold.

Authenticatd user—Signed in Admin

Non Authenticated user (View only dashboard)

**On Latest- error appears in case of Data error.

[aaemnnosttv]

@mohitwp @nfmohit this does look like it needs to be fixed. The changes here should only affect the insufficient permissions case, not other errors.

[techanvil]

Hi @mohitwp, I have taken a look but am unable to reproduce this. For me, the correct "You are probably offline" error message still appears on the view-only Dashboard. I've tried with both an admin and a non-admin user, here's a screenshot for the admin user:

Please can you double-check the steps you are taking to reproduce this? If you are still able to then please document the steps to reproduce in a bit more detail.

[mohitwp]

QA Update ✅

Thank you, @techanvil. I'm also not able to replicate this error now.

• Verified on dev.
• Verified for authenticated user - admin user when signed in.
• Admin user when access dashboard in view only.
• Verified for non authenticated users in view only.
• For non-authenticated users, new error message showing as per AC.
• For authenticated current error message showing with 'Request Access' button.
• For non-authenticated users, "Request access" button is not appearing.

Authenticated - Logged-in user Admin

Non-Authenticated user - View only dashboard