Improve module Edit Settings form when owner loses access to the module

[jamesozzie]

Bug Description

If an user doesn't have access to the connected SC property, they'll see a warning on the Site Kit dashboard as per the below:

If they try to edit the Search Console module, and if this is the only Search Console property available, there will be a dropdown with nothing available for selection. It's not possible to confirm any changes.

Improve this scenario by adding some textual context, or a "Request accsses" button as it appears on the dashboard.

Steps to reproduce

1. Set up Site Kit with Analytics on a site that already is verified in Search Console (ie. http://mysite.com). Use this verified property alone, with no other verified versions of that site (ie. the insecure property or the www prefixed property are not verified independently of Site Kit). Site Kit will use this existing verification method.
2. Ensure data appears on the Site Kit dashboard.
3. Access Search Console and unverify your Google account from the property.
4. Revisit your Site Kit dashboard. An expected Search Console permissions error will appear, with a request access button.
5. Visit the Search Console settings page and a a dropdown will appear which isn't very informative of the issue.

Screenshots

Additional Context

• Site Kit 1.109.0
• This was not raised in the support forums
• Search Console permissions errors are not common for admins, but they do come up in the forums

---

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

Acceptance criteria

• If the current logged in user is the owner of connected modules, but somehow loses their access to the service, then the "owned settings" in the Edit Settings form should be disabled, similar to the behaviour implemented in #4825.
• When disabled, raw values can be displayed as necessary in place of the usual "rich" display value where API entity access is needed (e.g. a property ID in place of the ID + name) but should still appear as the same type of input (e.g. a disabled Select with single selected value).
• An info notice should be displayed as follows:

You configured {module name} but you don’t have access to this {connection entity} anymore. Request access to this {connection entity} or disconnect and reconnect the module with another {connection entity}.

• The words "Request access" should be a link that points to the same URL used in the "Request access" on the Insufficient Permissions error being displayed in this instance on the dashboard.

Implementation Brief

Test Coverage

QA Brief

Changelog entry

[jimmymadon]

I have renamed this issue as this affects all modules and not simply Search Console. It is possible that the current owner of a module, i.e. the person that first set up the module, can lose access to the module for which the "Insufficient Permissions" errors are displayed in widgets on the dashboard.

However, when VIEWING settings, certain modules like Analytics do throw an error whereas Search Console does not as the current saved settings are simply displayed from the database.

Now, when EDITING settings, all drop-downs end up being blank and there is no proper error message on some forms.

If the logged in user is not the owner of a module, we always display a well designed error message and simply display the saved settings values (and no additional information which cannot be fetched if the user has no permissions).

This is because in https://github.com/google/site-kit-wp/issues/4825 and other related issues, we first check if the current logged in user is the owner of a module. If they are, we do NOT check if the user has access to this module or not. This is then used to display the above behaviour (error + saved settings values only).

The module owner check was done mainly to avoid changing all our tests/E2E tests (see point 2 in this comment). Should we handle the case where the current logged in user themselves don't have access?

[tofumatt]

@jimmymadon Checking to ensure the current user doesn't have access makes sense. It'd be great if we could do it only when there's an error if it'll save an otherwise redundant request (that's probably rare), but that's more of an IB detail.

I think it makes sense to write up ACs for that scenario 👍🏻

[jimmymadon]

@tofumatt Thanks - I've drafted some ACs here. c.c.ing @aaemnnosttv as he drafted the original ACs for #4825 and advocated for a "module owner" check BEFORE doing a "module access" check.

[eclarke1]

Can this please have a priority label added @jimmymadon or @tofumatt thank you

[tofumatt]

ACs here work for me, moving this to IB 🙂

[jimmymadon]

@aaemnnosttv I'm moving this back to ACR as when I was doing the IB for this issue, I realise this does make the code a lot more complex, affects many tests and will overall be a 15 or 19 if we do this carefully for all modules. So I'd like to discuss if this rather rare case is worth the effort of fixing.

c.c. @tofumatt