site-kit-wp: Add "Retry" button to regular JS error notices in the plugin

Follow-up to #5236: In addition to the ReportErrors, a retry button should also be added to similar JS-based errors that appear outside of Site Kit widgets - specifically in module setup flows and module settings.


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

Acceptance criteria

  • The ErrorNotice component (primarily used throughout setup flows and module settings) should also display a “Retry” button if the provided error comes with the necessary selectorData defined on it.
  • The logic for whether to display the “Retry” button or not should be similar to #5236.
  • When the user clicks the button, the corresponding store selector should also be invalidated.

Implementation Brief

Test Coverage

  • Add tests to ensure the error notice renders/doesn’t render the button and runs the callback based on the error passed to it.

QA Brief

  • An example of a retryable ErrorNotice can be found in the Tag Manager setup page.
  • Using Tweak:
    • Set up a rule for the URL http://sitekit.10uplabs.com/wp-json/google-site-kit/v1/modules/tagmanager/data/accounts?_locale=user, substituting your protocol://host as needs be.
    • Set the status code to be an error e.g. 403.
    • Set the response payload to something like this:
{
  "code": "foo",
  "message": "Mock error fetching tagmanager accounts!",
  "data": {}
}
  • Start the Tab Manager setup flow.
  • On the first page of it, the error should appear with a Retry button.
  • Disable the Tweak rule and press retry, the error should resolve.

image

Changelog entry

  • Add a “Retry” button for most errors in the plugin, except for some auth and other select errors.

About this issue

  • Original URL
  • State: closed
  • Created 2 years ago
  • Comments: 20 (6 by maintainers)

Commits related to this issue

Most upvoted comments

Approval ✅

Thanks @techanvil @tofumatt @wpdarren! Technically looks all good to me, just visually it’s a bit odd having a primary CTA button in that inline notice, especially since there’ll often be another actual primary CTA button in related UI.

That is not super critical and also wasn’t outlined in the ACs here, so I’ve opened a separate follow-up issue #6055 to address it.

Hi @makiost @aaemnnosttv @techanvil I am removing this ticket from Sprint 83(and will not add to Sprint 84) as we are still waiting on the resolution on the AC in 5618. We can add this to a sprint once 5618 is in execution is that sounds ok to everyone? Thanks cc @eclarke1

@eclarke1 @FlicHollis This is a follow-up task for #5236 that we had forgotten about somewhat. It should be fairly straightforward to address from an engineering perspective. With that said, this is not blocking anything, so it can happen individually without directly being tied to another effort. If we could get this added in the next month would be great.