Don't display multiple errors on the main WordPress dashboard

[jamesozzie]

Bug Description

If a server is performing slow an error message can appear that you're offline within the main Site Kit dashboard, within the module widgets.

If this occurs on the main WordPress dashboard, within the Site Kit widget, these notices are stacked, as per the screenshot below.

If possible (should the retry button refresh all data, and not just Analytics or SC data), display only one such error with the retry button.

Steps to reproduce

1. Install a plugin that disables the WP REST API. In my case I used the Disable REST API plugin.
2. Ensure the REST API is blocked for administrators (if using the same plugin "Settings > Disable REST API > Administrator > toggle off the SK endpoints" - screenshot)
3. Visit your main WP dashboard, where the error appears in multiple notices. If the error doesn't appear, wait an hour, or clear your session storage in order for cached responses to expire

Screenshots

Additional Context

• PHP Version:
• OS: [e.g. iOS]
• Browser: [e.g. chrome, safari]
• Plugin Version: [e.g. 22]
• Device: [e.g. iPhone6]

---

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

Acceptance criteria

• The display of error messages on Site Kit WP Dashboard widget should be consolidated in the following way:
  • Errors from same module should be deduplicated - only one error message should be displayed per error per module.
  • Retrying an error should retry all the underlying sources that were consolidated into that error.

Implementation Brief

• In current case, each widget returns the error(s) from state separately. Solution is to show errors in more centralised way in main widget component - assets/js/components/wp-dashboard/WPDashboardWidgets.js.
  • So in that file fetch errors from following module stores: MODULES_SEARCH_CONSOLE, MODULES_ANALYTICS_4, MODULES_ANALYTICS. Creating separate variables that will hold the errors from the state associated with these modules, like searchConsoleErrors, analytics4Errors and analyticsErrors, each would fetch associated error from it's store, like this - https://github.com/google/site-kit-wp/blob/954d8ddd1fa01e09e87a51f8a0e7805d9310f2b6/assets/js/components/wp-dashboard/WPDashboardImpressions.js#L64-L68
  • Then include the ReportError component from assets/js/components/ReportError.js
  • If there is an error originating in one these modules, like MODULES_ANALYTICS_4 for example, it will always be rendered in all it's widgets, due to individual check in these widgets. That way we can use the appropriate error variables to check against errors before we render these widgets, so we can render one ReportError component with consolidated messages, instead of each widget with same error message. For searchConsoleErrors use it to conditionally render search console widgets - https://github.com/google/site-kit-wp/blob/954d8ddd1fa01e09e87a51f8a0e7805d9310f2b6/assets/js/components/wp-dashboard/WPDashboardWidgets.js#L137-L138 , like:

    ( searchConsoleErrors ?
    <ReportError moduleSlug="search-console" error={ error } /> :
    <Fragment>
    <WPDashboardImpressionsWidget />
    <WPDashboardClicksWidget /> 
    </Fragment>
    )

  • Since analytics and analytics4 widgets are spread above and bellow search console widgets, add only analytics4Errors and analyticsErrors conditional rendering for ReportError before first widgets - https://github.com/google/site-kit-wp/blob/954d8ddd1fa01e09e87a51f8a0e7805d9310f2b6/assets/js/components/wp-dashboard/WPDashboardWidgets.js#L122-L135
  • And for all analytics and analytics4 widgets, add one more condition for rendering - if there is no error, like this:

    
    { ( analyticsModuleActiveAndConnected && ! analyticsErrors ) &&
    isGA4DashboardView === false && (
    <Fragment>
        <WPDashboardUniqueVisitorsChartWidget />
        <WPDashboardPopularPagesWidget />
    </Fragment>
    ) }

{ ( isGA4DashboardView && ! analytics4Errors ) && (

) }

  * Do the same for first two analytics and analytics4 widgets
  * `ReportError` component accepts array of errors, it will take care of consolidating the error messages within one error components, and `ReportErrorActions` component inside that handles the Retry action, will handle all underlying sources that were consolidated into that error.

### Test Coverage

* update any failing VRT images

## QA Brief

* <!-- One or more bullet points for how to test that the issue has been resolved. -->

## Changelog entry

* <!-- One sentence summarizing the PR, to be used in the changelog. -->

[wpdarren]

@jamesozzie good catch. I am also wondering if the same issue would occur with the Site Kit admin toolbar? I know from past experiences when the WP Dashboard widget has weird UI issues like this, this usually is the case with the toolbar too. I've not been able to recreate this, so wondered if you had any steps to recreate?

[jamesozzie]

@wpdarren I've never seen it happen with the Site Kit admin toolbar, nor did I come across it in the forums. There are only a few metrics output in the admin toolbar, so I suspect it doesn't happen as they are horizontally aligned and not stacked. I have seen the same error appear in both Analytics metrics for example (Total Users & Total Sessions), but that's as expected.

I'll see if I can consistently reproduce the main issue as described above today and add the steps to recreate. I suspect I can do so by throttling via dev tools.

[jamesozzie]

@wpdarren I've added the steps to recreate stacked errors to the main body of the issue. Let me know if you have any questions or want me to do further testing.

[wpdarren]

@jamesozzie thanks for the info. I had a quick look and using that plugin I don't get the offline error message (tried throttling but that didn't work either) but I see a message for the Rest API and feel the same issue you reported but for the Admin toolbar for Site Kit.

Do you think we need to include this as part of this fix? It would look better as one error like the dashboard widget.

[jamesozzie]

@wpdarren I created the REST related error due to not being able to consistently recreate the offline error. The same issue occurs with errors stacked.

In relation to the admin bar nice find. The horizontal errors don't seem as dramatic as the stacked errors, but if we can improve on this also then great!

[jamesozzie]

Just adding that I encountered this on different sites today, for different services (on the main WP dashboard widget).

[aaemnnosttv]

AC ✅

[techanvil]

Hi @zutigrm, thanks for drafting this IB. It's potentially off to a good start, however, I'm actually thinking we should take a different approach to solving this. Indeed, having dug into this further, in hindsight it probably wasn't such a great candidate for a Good First Issue, so apologies there, although I imagine it's a good one to get into a slightly deeper part of the pool nonetheless.

So, the issue that I see with the proposed approach is:

• It looks like you have a bit of a misunderstanding as to how the errors are managed per module. Taking the following quoted point, actually it's not the case that a given error for a module will be rendered in all of its widgets. In fact, errors are distinguished by their baseName (which is the selector name when associated with a selector) and arguments (e.g. reportArgs in these widgets) - as you can see here in createErrorStore(), where the error key is generated from baseName and args. So, as the widgets don't all check for errors using the same baseName and args, we can have the situation where one widget in a given module is displaying an error, while another in the same module is showing the correct output, if say the underlying reports failed for one and succeeded for another.

  • If there is an error originating in one these modules, like MODULES_ANALYTICS_4 for example, it will always be rendered in all it's widgets, due to individual check in these widgets.

• So, the implication is that we'd need to take a much more granular approach, whereby the WPDashboardWidgets component would need to know which errors are associated with which widgets, in order to correctly render a given widget or not according to its related error. This in turn would imply WPDashboardWidgets having an in-depth understanding of the report arguments and so forth for each widget, which is not so desireable architecturally as it breaks the encapsulation we currently have with each widget entirely responsible for managing its own data.
• Note that even if we were to simplify the UX so that we do decide to group things by module, i.e. don't show all widgets for a module if any of the module's widgets fail, we'd still need WPDashboardWidgets to know about all of the various report args for the individual widgets, which would be nice to avoid.

With the above in mind, I'm thinking we should take a different approach, whereby we leverage our existing infrastructure for combining widgets with a common state. It would involve a slight reorganisation of the widgets, but in a way that's consistent with the broader plugin. However - it would effectively be following the same proposal that I made recently for https://github.com/google/site-kit-wp/issues/7149#issuecomment-1632878316, which @aaemnnosttv felt was too much of an involved change at the time and we ended up reducing the scope of the issue instead.

I would suggest taking a read of the linked comment, notably the third paragraph which summarizes how our "combining widgets" behaviour works.

Essentially my proposal would be as follows:

• Move the Search Console widgets on the WP Dashboard above all of the Analytics widgets, to facilitate combining the error states for adjacent widgets of the same module.
  • This would mean moving Total Impressions and Total Clicks to the top, which would actually be consistent with elsewhere e.g. the Admin Bar and the Search Funnel widgets (see screenshots below).
• Update the Site Kit WP Dashboard app and WPDashboardWidgets to make use of the widget infrastructure, registering a widget context, area & widgets, and making use of WidgetContextRenderer as a rendering entry point.
• Follow the proposal outlined in https://github.com/google/site-kit-wp/issues/7149#issuecomment-1632878316 for combining error states, adapting it for the WP Dashboard case (or with a generic adaptation to be used for any set of widgets).

Now, I realise this would be quite an involved piece of work, and we didn't take that route for Key Metrics widgets, but we did have to be particularly time-conscious there due to the target for delivery of the KM feature. It could be the case that we could allow ourselves to take this route here, with this being a more general, and non-critical issue without the same time pressure.

So, I'm going to ask @aaemnnosttv to take a look here and let us know his thoughts on this once more :)

---

Screenshots re. the proposed widget reorganisation, as can be seen here the WP Dashboard is a bit inconsistent in its layout and we could address this at the same time.

[ivonac4]

@aaemnnosttv This one is waiting for your guidance. Can you take a look anytime soon, please?

[aaemnnosttv]

Thanks @techanvil! I definitely like the idea of using the Widget API for more consistency across screens and I have thought about this a number of times myself. In particular, I like the idea of keeping the module-specific parts in their own bundles – the dashboard widget and admin bar both have problems related to module use being hard-coded within them, which would be great to address as a byproduct.

A few thoughts

• If we do go this route, I still think we should make any API changes separately (if needed)
• The reorganization / grouping of the components on the dashboard makes sense but this would also make it much easier to address without using the Widgets API (we could render each using a module specific wrapper which could manage the combined state)
• The Widgets API has yet to be used in this context where its maximum width may only be a fraction of the total screen (or main content) area. The grid system may not work as expected.
• Enabling errors to be combined would affect the dashboard as well which is something we'll want to account for from the beginning as this is not currently the case and maybe we want to keep it that way? Maybe we could use React context (or some other way) to define areas where error consolidation should happen (i.e. non-dashboard areas) rather than enabling it everywhere.
• There is some work underway right now in #6406 which relates to the retry here
• Naturally if we go this route, it would also make sense to apply the same to the Admin bar app which should actually be a more straightforward application
• Now that both dashboard widget and admin bar app are available to view-only users, their implementation could be further simplified due to the out-of-the-box handling of recoverable module states (or others) which the W-API accounts for

Having written all that out, perhaps a smaller first step would be to tackle the Admin bar first and then revisit this with any learnings we may gain along the way. WDYT?

[techanvil]

Thanks @aaemnnosttv, some great points there.

• If we do go this route, I still think we should make any API changes separately (if needed)

That sounds sensible if we are making a substantial change, but not if say we're just adding a component to SPECIAL_WIDGET_STATES which might be all that is needed here. One to assess once we have the solution more well defined.

• The reorganization / grouping of the components on the dashboard makes sense but this would also make it much easier to address without using the Widgets API (we could render each using a module specific wrapper which could manage the combined state)

I guess this is the top level direction we need to determine. It might be easier to take this approach, although we could find ourselves reimplementing some aspects of the widgets API depending on how we structure things.

OTOH, if we do use the widgets API, it opens up the possibility of combining errors on the dashboard too, which could be useful. It should also address the point you mentioned about hard-coded module use in the WP Dashboard and Admin Bar widgets, and potentially allow further simplification for view-only users as per your related point.

• The Widgets API has yet to be used in this context where its maximum width may only be a fraction of the total screen (or main content) area. The grid system may not work as expected.

This is a good point - the current breakpoints are not aligned for the WP Dashboard. Maybe we could recreate the grid layout with different breakpoints, or we could simply use the grid as-is, providing a fixed layout with either one or two widgets for each row, and disregarding the breakpoints as it wouldn't re-layout anyway...

Alternatively we could use a different set of *Renderer components (maybe refactoring the current ones to be composable or suchlike). That would probably be my preferred option, on the face of it, as we don't need the grid for the WP Dashboard layout anyway.

• Enabling errors to be combined would affect the dashboard as well which is something we'll want to account for from the beginning as this is not currently the case and maybe we want to keep it that way? Maybe we could use React context (or some other way) to define areas where error consolidation should happen (i.e. non-dashboard areas) rather than enabling it everywhere.

We could indeed use React context, another option is to provide a new component e.g. CombinableReportError (which could wrap ReportError), which would be the component we add to SPECIAL_WIDGET_STATES. This way widgets can choose whether their errors should be combinable or not.

• There is some work underway right now in #6406 which relates to the retry here

Thanks for highlighting that, I've left a comment on the issue to point out that we could potentially use the changes here (if we take the approach of allowing ReportErrorActions to invalidate multiple selectors).

• Naturally if we go this route, it would also make sense to apply the same to the Admin bar app which should actually be a more straightforward application
  ...

Having written all that out, perhaps a smaller first step would be to tackle the Admin bar first and then revisit this with any learnings we may gain along the way. WDYT?

That sounds pretty sensible - the Admin Bar is a slightly simpler app in general and if we are taking the widget API route, it further simplifies things as it's already using the grid. So it would be a good iterative route to tackle it first.

I think it would be advisable to decide on the top level direction here, though, before starting on the Admin Bar issue. Otherwise we'll have to refer to this long thread (or risk covering all the same ground again) from that issue.

Personally, taking into account the benefits of reusing the widgets API, I do think this would be a good direction to take here and that we should kick off the Admin Bar issue with that top level steerage. What do you reckon?