Visual difference in the view only vs standard SK dashboard with number of KMW tiles shown

[jamesozzie]

Bug Description

We seem to be doing something differently when it comes to the view only SK dashboard compared to the standard dashboard if there are less than the maximum 4 key metric tiles added. "View only" users can also select the key metrics tiles to be viewed, but there are no placeholder "Add a metric" tiles for these users. Screenshots with examples below.

Note that it's possible that for view only users, tiles should not be available for selection, in particular if it requires the creation of custom dimensions.

Standard SK dashboard below - which shows placeholder tiles in place if less than 4 key metrics are selection

View only dashboard via Dashboard Sharing below - When 2 or 3 tiles are displayed, they stretch to the full width of the page and there is no additionl placeholder "Add metric" tile

Steps to reproduce

1. Activate 2 or 3 key metric tiles and ensure all is working as expected.
2. Via dashboard sharing, grant access to GA for other users BUT NOT search console.
3. Login to the site from another user account (ie. An editor role), and access the view only SK dashboard
4. Notice there is no "Add a metric" tile, yet there is the expected "Change metrics" link

Additional Context

• SK 1.126.0

On some further investigation by @jimmymadon, the reason for the Add a metric tile not appearing is because Search Console was not shared via Dashboard Sharing. The "Add a metric" tiles are registered with the modules property set to search-console. So when search-console is not shared, the "Add a metric" tile is not rendered.

site-kit-wp/assets/js/googlesitekit/widgets/register-defaults.js

Line 338 in 7fb7fb9

modules: [ 'search-console' ],

---

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

Acceptance criteria

• The "Add a metric" tile(s) should be visible at all times when there are less than 4 metrics available on the "View Only" dashboard for a user. This should be the case even when only analytics is shared but not search console.

Implementation Brief

• In assets/js/googlesitekit/widgets/register-defaults.js, remove the modules property when registering the 'Add a metric' widgets:

site-kit-wp/assets/js/googlesitekit/widgets/register-defaults.js

Line 338 in 7fb7fb9

modules: [ 'search-console' ],

Essentially, having either search-console or analytics active as a 'requirement' doesn't matter for these tiles because:

• If search-console is the only module that is shared, then key-metrics will not be shown (since there is only one SC tile available and we need at least 2 tiles for the widget to show).
• If analytics is shared, the isActive callback will still guard against the tile showing when it shouldn't.

Test Coverage

• No new tests required.

QA Brief

Changelog entry

[jimmymadon]

@jamesozzie I've tried to reproduce your situation but there is definitely something else going on here. View only users should be able to select any of their own metrics when dashboard sharing is enabled. I am able to see the "Add a metric" CTA tile when logged in as a view only user.

[jamesozzie]

View only users can select their own metrics via the "Change metrics" link only in my case, but without any "add a metric" box. Tested this on two sites today. Happy to jump on a call. I also sent you an "editor" profile login so you can view this issue on a live site @jimmymadon.

[tofumatt]

ACs here make sense, I think this should be fixable by setting the module to be Analytics instead, as long as that won't break things for users who only have Search Console shared. If not I guess it needs to be an OR type thing, which might be more complicated (unless we already have that and I forgot 😅).

Moving to IB 👍🏻

[tofumatt]

@jimmymadon The approach here will work, but thinking about it, it seems to sort of "hide" the intention of the modules attribute there. We don't really want/need to register those widgets if neither module is available, and that's helpful context to have in the code. (Otherwise we would just rely on isActive checks for all widgets. 😅)

I guess given there's the isActive check here though it's fine, and I'm probably overthinking it.

Maybe it'd be nice to add a check for at least one of either Search Console or Analytics being active in the isActive check, but I don't think it's needed. It just makes it a bit easier to grok the intent of the code, but functionally it won't matter 😄

IB ✅