Implement the unhappy paths for the Setup CTA Banner

[techanvil]

Feature Description

Implement the unhappy paths for the Setup CTA Banner, making use of the Error Modal to display them. This includes handling errors returned by the OAuth flow.

See setup CTA banner > setup logic, OAuth errors and error modal in the design doc.

---

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

Acceptance criteria

• Having clicked on Enable groups in the Setup CTA Banner, if an error occurs, whether it's an error returned from the OAuth flow, or from an API call made to create the audiences and/or custom dimension, the Error Modal should be displayed (see #8110).
  • If an error is returned from the OAuth flow:
    • The OAuth error variant of the Error Modal should be displayed.
    • It should follow the Figma design, with the following copy:
      • Title: Analytics update failed
      • Description: Setup was interrupted because you did not grant the necessary permissions. Get help
    • The Retry button should take the user back to the OAuth flow to retry the setup from that point.
    • The Cancel button should close the modal.
    • The default OAuth error notice that Site Kit usually displays when the OAuth flow returns an error should not be shown.
  • If a permission error is returned by one of the API calls:
    • The "insufficient permissions" variant of the Error Modal should be displayed.
    • It should follow the Figma design, with the following copy:
      • Title: Insufficient permissions
      • Description: You’ll need to contact your administrator. Trouble getting access? Get help
    • The Request access button should open the URL for the currently connected web data stream in the Analytics UI, allowing the user to request access.
    • The Cancel button should close the modal.
  • If any other error is returned by one of the API calls:
    • The generic error variant of the Error Modal should be displayed.
    • It should follow the Figma design, with the following copy:
      • Title: Failed to set up visitor groups
      • Description: Oops! Something went wrong. Retry enabling groups.
    • The Retry button should close the modal, retry the failed API call(s) and continue setup from that point.
    • The Cancel button should close the modal.
• The Get help links should open the Analytics support page in a new tab, scrolled to the insufficient permissions section.

---

• Note that all copy should be verified with Figma as the source of truth.

Implementation Brief

Update the enableAudienceGroup action in assets/js/modules/analytics-4/datastore/actions.js:

• [x] Update Function (Action) Signature
  • [x] Modify the enableAudienceGroup action to accept an object parameter that includes failedSiteKitAudienceResourceNames - A list of audiences that failed to be created by Site Kit.
• [x] Return Error Objects
  • [x] Identify all the places in the action where there are TODO comments for error handling.
  • [x] Replace each TODO comment with a return statement that returns an error object. For example, if there is an error in synchronizing available audiences, return the error object immediately.
• [x] Handle Site Kit-Created Audience Failures
  • [x] Add a conditional check at the beginning of the audience creation logic to see if failedSiteKitAudienceResourceNames is provided.
  • [x] If failedSiteKitAudienceResourceNames is provided, skip the creation of new audiences and proceed with the logic.
• [x] Update Audience Creation Logic
  • [x] Modify the existing audience creation logic to handle retries by using the failedSiteKitAudienceResourceNames parameter.
  • [x] Define the audiences to be created based on whether failedSiteKitAudienceResourceNames is provided or use the default list of audiences to create.
• [x] Handle Creation Results
  • [x] After attempting to create the audiences, track which audiences need to be retried if their creation fails.
  • [x] Use an array (e.g., needsRetry) to store the slugs of the audiences that failed to be created.
  • [x] If any audiences need to be retried, return an object with failedSiteKitAudienceResourceNames containing the slugs of the failed audiences.
• Syncing the available audiences, updating the configured audiences, and saving the audience settings should be kept as is.
• Check this commit for reference on a POC branch as a starting point.

Update the ModalDialog component in assets/js/components/ModalDialog.js:

• [x] Add a new prop buttonLink to handle opening a link in a new tab.
• [x] If buttonLink is provided, update the SpinnerButton component props to include href and target attributes with the buttonLink and _blank values, respectively.

Create AudienceErrorModal component in assets/js/modules/analytics-4/components/audience-segmentation/dashboard:

• [x] It should receive hasOAuthError, apiErrors and onRetry as props. Prop names can be adjusted as needed during implementation.
• [x] Use the ModalDialog component to render the error modal.
• [x] Pass the buttonLink prop to the ModalDialog component for OAuth and insufficient permissions errors and other appropriate props.
• [x] OAuth error variant:
  • [x] If hasOAuthError is true, render the OAuth error variant of the Error Modal. See the Figma design for details.
  • [x] Reuse the same callback logic to retry the setup from the OAuth flow, which is being implemented in #8132. Potentially, create a reusable action for this.
• [x] Insufficient permissions error variant:
  • [x] If apiErrors contains an insufficient permissions error, render the insufficient permissions error variant of the Error Modal. See the Figma design for details.
• [x] Generic error variant:
  • [x] If apiErrors contains any other error, render the generic error variant of the Error Modal. See the Figma design for details.
  • [x] Call the onRetry prop when the Retry button is clicked within a callback function. The parent component should pass the appropriate retry function (i.e., enableAudienceGroup action with the failedSiteKitAudienceResourceNames parameter).
• [x] See ReportErrorActions for an example of how to determine the insufficient permissions error and generic error variants.
• Figma designs should be the source of truth for copy and design details.

Update the AudienceSegmentationSetupCTAWidget component in assets/js/modules/analytics-4/components/audience-segmentation/dashboard/AudienceSegmentationSetupCTAWidget.js:

• [x] Create a component state to store the API errors.
• [x] Create a component state to store the failed Site Kit audience resource names from the enableAudienceGroup action response.
• [x] Get the { error, response } object from the enableAudienceGroup action response within the onEnableGroups callback function.
• [x] Set the error state based on the response from the enableAudienceGroup action.
• [x] Set the failed Site Kit audience resource names state based on the response from the enableAudienceGroup action.
• [x] Check if there is an OAuth error using the getSetupErrorCode selector.
• [x] If there is an OAuth error or API error, render the AudienceErrorModal component with the appropriate props.
• [x] Pass a callback function to the onRetry prop of the AudienceErrorModal component to retry the enableAudienceGroup action with the failed audience resource names.
• [x] Set Skip Error Notification Flag:
  • [x] When triggering the OAuth flow, pass skipDefaultErrorNotifications to true while dispatching the setPermissionScopeError action.

Prevent Rendering Other Setup Errors in assets/js/components/notifications/ErrorNotifications.js and assets/js/components/notifications/SetupErrorNotification.js:

• [x] Modify the ErrorNotifications and SetupErrorNotification components to check if temporaryPersistedPermissionsError?.data?.skipDefaultErrorNotifications is set.
• [x] If skipDefaultErrorNotifications is set, do not render the error notifications. i.e., return null.
• See how the temporary persisted permissions error is retrieved in the ErrorNotifications: https://github.com/google/site-kit-wp/blob/17215a06bf2cd00d4ac8f2595e69b59413bb6251/assets/js/components/notifications/ErrorNotifications.js#L56-L61

Test Coverage

• Add storybook stories for all the error variants for the AudienceErrorModal component.
• Add test coverage for all the error variants for the AudienceErrorModal component.
• Add test coverage for the changes made to the enableAudienceGroup action.
• Fix any broken stories in the AudienceSegmentationSetupCTAWidget component.
• Fix any failing tests.

QA Brief

Prerequisites

1. Setup Analytics and enable the audienceSegmentation feature flag.
2. Ensure no audiences have been created or configured.
3. Verify the Audience Segmentation setup CTA banner is displayed.

Testing the Audience Error Modal Variants

There are three Audience Error Modal variants to test:

1. OAuth error variant.
2. Insufficient permissions error variant.
3. Generic error variant.

OAuth Error Variant

To simulate the OAuth error variant:

1. Prepare the Environment:

   • Delete the wp_googlesitekit_additional_auth_scopes entry from the wp_usermeta table if it exists. This entry contains the https://www.googleapis.com/auth/analytics.edit scope.

2. Trigger the OAuth Error:

   • Click on the Enable groups button in the Audience Segmentation setup CTA banner.
   • In the OAuth flow, when prompted to grant permissions, cancel the flow.
   • Ensure the OAuth error variant of the Error Modal is displayed.
   • Click the Retry button to trigger the OAuth flow again.
   • Complete the OAuth flow and grant permissions.
   • Ensure the setup continues successfully after granting permissions.

Insufficient Permissions Error Variant

To simulate the Insufficient Permissions error variant:

1. Install and Configure Tweak Extension:

   • Install the Tweak Chrome extension.
   • Add a new rule to block the sync-audiences request:
     • URL: .*/wp-json/google-site-kit/v1/modules/analytics-4/data/sync-audiences*
     • Enable the use of regular expression (.*)
     • HTTP Method: POST
     • Status code: 403
     • Response payload:

       {
         "code": 403,
         "message": "Insufficient Permissions Test Error",
         "data": {
           "status": 403,
           "reason": "insufficientPermissions"
         }
       }
       

2. Trigger the Insufficient Permissions Error:

   • Click on the Enable groups button in the Audience Segmentation setup CTA banner.
   • Ensure the Insufficient Permissions error variant of the Error Modal is displayed.
   • Click on the Request access button and verify the Analytics console opens in a new tab.

Generic Error Variant

To simulate the Generic Error variant, there are two scenarios:

1. Other API Errors:

   • Simulate Sync Available Audiences API Error:

     • Add a new rule in the Tweak extension to block the sync-audiences request:
       • URL: .*/wp-json/google-site-kit/v1/modules/analytics-4/data/sync-audiences*
       • Enable the use of regular expression (.*)
       • HTTP Method: POST
       • Status code: 500
       • Response payload:

         {
           "code": "internal_server_error",
           "message": "Internal server error",
           "data": { "status": 500 }
         }
         

   • Trigger the Generic Error:

     • Click on the Enable groups button in the Audience Segmentation setup CTA banner.
     • Ensure the Generic error variant of the Error Modal is displayed.
     • Disable the rule to unblock the request in the Tweak extension.
     • Click the Retry button to ensure the setup continues successfully.

2. Audience Creation Failure:

   • Simulate Audience Creation Failure:
     • Manually create new-visitors and returning-visitors audiences in the Analytics console.
     • Click on the Enable groups button in the Audience Segmentation setup CTA banner.
     • Ensure the Generic error variant of the Error Modal is displayed.
     • Delete the new-visitors and returning-visitors audiences in the Analytics console.
     • Click the Retry button to ensure the setup continues successfully.

Changelog entry

• Handle errors in the Audience Segmentation setup flow, showing an error modal allowing the setup to be retried, or relevant permissions to be requested.

[techanvil]

Hi @hussain-t, just letting you know I've moved this back to Backlog as the final in-progress changes to the design doc, relating to audience caching, will probably affect the AC for this one (we probably need to handle an audience sync error here). Please feel free to get preliminary AC ready in the meantime.

[techanvil]

Hi once again, @hussain-t! Again, as the audience caching aspect of the design doc has been sufficiently finalised, I've moved this back to AC.

[techanvil]

Hi @hussain-t, this IB is off to a good start. A few points:

• I realise the name siteKitCreatedAudienceFailures is taken from the POC, but I would suggest a slightly different name. How about failedSiteKitAudienceResourceNames? The current name suggests the variable contains "failures", i.e. error objects or something like that, rather than the audience names.

---

• See ReportErrorActions to determine the insufficient permissions error and generic error variants

• A minor point but this should say something like "See ReportErrorActions for an example of how to determine the insufficient permissions error and generic error variants" and it would be nice to provide a permalink to the example.

---

• Update the enableAudienceGroup action call to pass the siteKitCreatedAudienceFailures parameter.

• This doesn't sound right in isolation. The current dispatch of enableAudienceGroup() is in response to the setup CTA click. We need a way of triggering it in response to clicking the Retry CTA in the error modal.

---

• If there are OAuth errors, dispatching the setPermissionScopeError action with the error is being implemented in #8132.

• I am not sure about the relevance/accuracy of this point. The implementation of #8132 will dispatch setPermissionScopeError() in order to trigger the OAuth flow rather than dispatching it when there is an OAuth error. Maybe the point needs to be rephrased?

---

Prevent Rendering Other Setup Errors in assets/js/components/notifications/ErrorNotifications.js and assets/js/components/notifications/SetupErrorNotification.js

• [ ] Modify the ErrorNotifications and SetupErrorNotification components to check if the autoSubmit property is set for the AUDIENCE_SEGMENTATION_SETUP_FORM key of the CORE_FORMS store. Setting the store is being implemented in #8132.
• [ ] If the autoSubmit property is set, do not render the error notifications. i.e., return null as want to show the AudienceErrorModal component.

• It would be nice to abstract this a little to future proof it - there will be other scenarios where we want to handle an OAuth error inline and suppress these default handler components. We could consider:
  • Encapsulating the check for the autoSubmit in a selector, which we could then add more conditions to as they arise.
  • Using a separate piece of Redux state as a flag for suppressing these components and toggling this flag in the components which are aware of the autoSubmit.
• Adding a selector is probably a more straightforward approach, but please see what you think.

[hussain-t]

Thanks for your valuable review, @techanvil.

I am not sure about the relevance/accuracy of this point. The implementation of https://github.com/google/site-kit-wp/issues/8132 will dispatch setPermissionScopeError() in order to trigger the OAuth flow rather than dispatching it when there is an OAuth error. Maybe the point needs to be rephrased?

This whole point is unnecessary. So, I went ahead and removed it.

It would be nice to abstract this a little to future proof it - there will be other scenarios where we want to handle an OAuth error inline and suppress these default handler components.

I believe we can encapsulate this when necessary. As a general principle, we can extract reusable code when needed.

Encapsulating the check for the autoSubmit in a selector, which we could then add more conditions to as they arise.

If it involves retrieving the value from the state and returning it, we can do this within the component itself, correct?

Using a separate piece of Redux state as a flag for suppressing these components and toggling this flag in the components which are aware of the autoSubmit.

Do you mean setting a state in the core/forms store or creating a separate partial store with its own setter and getter? If it’s the latter, it might be a bit overkill. Instead, we could simply use the getValue selector inside the components.

Please correct me if I’m wrong. My understanding is that encapsulating these checks or creating a separate store might be an over-engineering solution at this stage.

[techanvil]

Thanks @hussain-t.

It would be nice to abstract this a little to future proof it - there will be other scenarios where we want to handle an OAuth error inline and suppress these default handler components.

I believe we can encapsulate this when necessary. As a general principle, we can extract reusable code when needed.

That's fair enough, it's something we could potentially tackle on-the-fly during execution, or iterate on later.

That said - thinking about it more has given me an alternative idea on how we could handle it, which I'll outline after replying to the rest of your comment.

Encapsulating the check for the autoSubmit in a selector, which we could then add more conditions to as they arise.

If it involves retrieving the value from the state and returning it, we can do this within the component itself, correct?

That's right, it's nothing that we have to do outside the component. I was simply thinking this would be one of those places where a bit of DRY up-front could be useful so we don't have to remember to update in two places/refactor later.

Using a separate piece of Redux state as a flag for suppressing these components and toggling this flag in the components which are aware of the autoSubmit.

Do you mean setting a state in the core/forms store or creating a separate partial store with its own setter and getter? If it’s the latter, it might be a bit overkill. Instead, we could simply use the getValue selector inside the components.

Please correct me if I’m wrong. My understanding is that encapsulating these checks or creating a separate store might be an over-engineering solution at this stage.

I'd imagined something in core/forms or similar, rather than a new store which I agree would be overkill.

---

Anyhow, to my new idea. How about we manage this via another optional flag that we pass into setPermissionScopeError()? We can currently pass skipModal - we could provide support for, say, skipDefaultErrorNotifications at the same level.

https://github.com/google/site-kit-wp/blob/0ea003eb33a2bfd79846ac0c227432429532f4c5/assets/js/components/KeyMetrics/MetricsSelectionPanel/Footer.js#L176

This would allow us to clearly specify the desired behaviour at the point where we trigger the OAuth flow, in a manner that's consistent with the existing code.

The permissions error object is snapshotted prior to navigating to OAuth:

https://github.com/google/site-kit-wp/blob/0ea003eb33a2bfd79846ac0c227432429532f4c5/assets/js/components/PermissionsModal/AuthenticatedPermissionsModal.js#L67-L72

And is already retrieved in ErrorNotifications:

https://github.com/google/site-kit-wp/blob/0ea003eb33a2bfd79846ac0c227432429532f4c5/assets/js/components/notifications/ErrorNotifications.js#L56-L61

We could simply check if temporaryPersistedPermissionsError?.data?.skipDefaultErrorNotifications is set and return null if so, and add a similar check to SetupErrorNotification.

To my mind this is preferable to having to check for autoSubmit, as it is more of a declarative/intentful approach, while also avoiding having to set an additional keyed piece of state in core/forms or what not.

---

Lastly - I'm not sure that AudienceErrorModal should have a call to enableAudienceGroup() baked into it. Seeing as AudienceErrorModal will be reused in https://github.com/google/site-kit-wp/issues/8153 with some different retry logic (not clearly specced in the IB yet, but it should only attempt to retry creating the custom dimension in that issue), it seems we should probably pass in an onRetry prop or suchlike and not have any default retry behaviour built into it.

Again, this is something we could potentially iterate on, but with the direction here to cover AudienceErrorModal in tests, I think it would be more efficient to figure this out up-front rather than spend time writing tests for something liable to change in the near future.

Interested in your thoughts on all the above.

[hussain-t]

Thanks so much, @techanvil. That's a brilliant idea. By utilizing that, we can avoid setting unnecessary states.

Just to let you know, I have also updated the IB for #8153 to include the onRetry prop with a callback function.

[techanvil]

Thanks @hussain-t, thanks great. This IB LGTM :white_check_mark:

[techanvil]

Hey @hussain-t, happy to report I've just merged the PR for this and moved the issue to QA. Just dropping you a heads up that I made a small tweak to the QAB which you'll see in the edit history.

[hussain-t]

Apologies, I missed updating the QAB. Thanks for updating it, @techanvil 👍

[techanvil]

Back to you @kelvinballoo! :)

[kelvinballoo]

QA Update ❌

I have tested this and I have a few items to raise:

ITEM 1: Trigger the OAuth Error: When the modal appears, the 'Get Help' is highlighted already. Notice the dotted box around it. By right it should not be focused on.

____________________________________________________________

ITEM 2: Trigger the OAuth Error: The modal size is not correct. It should have width of 386px and not 502px.

Figma: 386 x 224 px

Implementation: 502 x 224px

---

ITEM 3: Insufficient Permissions Error Variant The comments from item 1 and 2 are applicable for this variant as well.> On top of that, I'd like to add that the second sentence should be on a new line.

Figma:

Implementation:

---

ITEM 4: Generic Error Variant Size of modal should be 402 instead of 422

Figma: 402 px wide

Implementation: 422 px wide

---

ITEM 5: Generic Error Variant

1. Clicking outside of modal makes enable disabled. Video is attached for reference.
2. With the Tweak extension on, it seems like clicking on 'Retry' doesn't do much. Is that expected?

https://github.com/google/site-kit-wp/assets/125576926/66891b54-8565-4dca-bb1f-0f98b37546a5

____________________________________________________________

ITEM 6: Generic Error Variant When checking the scenario 2 for Audience Creation Failure, is that still applicable? I believe after the changes done through this slack conversation, this is no longer applicable. Can you confirm please?

---

ITEM 7: Generic Error Variant When the error module popped up, I can see the grey background on the cancel button. Based on figma, there is no background there (unless maybe upon hover?)

Figma:

Implementation:

[hussain-t]

Thanks for your observations, @kelvinballoo.

ITEM 1: Trigger the OAuth Error: When the modal appears, the 'Get Help' is highlighted already. Notice the dotted box around it. By right it should not be focused on.

ITEM 2: Trigger the OAuth Error: The modal size is not correct. It should have width of 386px and not 502px.

ITEM 4: Generic Error Variant Size of modal should be 402 instead of 422

ITEM 7: Generic Error Variant When the error module popped up, I can see the grey background on the cancel button. Based on figma, there is no background there (unless maybe upon hover?)

We are using the common ModalDialog component. It seems buttons and links are focused on the modal component, which is consistent across all our models. For more details, explore the ModalDialog stories. If needed, feel free to create a separate ticket.

Regarding sizes, as mentioned earlier, we use the standard ModalDialog component, which adjusts to the content size. You can see examples in the ModalDialog stories.

ITEM 5: Generic Error Variant Clicking outside of modal makes enable disabled. Video is attached for reference. With the Tweak extension on, it seems like clicking on 'Retry' doesn't do much. Is that expected?

That's expected due to the network blocking using the Tweak extension.

ITEM 3: Insufficient Permissions Error Variant The comments from item 1 and 2 are applicable for this variant as well.> On top of that, I'd like to add that the second sentence should be on a new line.

If you inspect, you'll see that it's a single sentence, not two. This follows the convention we use elsewhere for similar Insufficient Permissions Errors. You can review the AuthenticatedPermissionsModal stories for reference.

ITEM 6: Generic Error Variant When checking the scenario 2 for Audience Creation Failure, is that still applicable? I believe after the changes done through this slack conversation, this is no longer applicable. Can you confirm please?

It's not applicable. I've updated the QAB.

[wpdarren]

QA Update: ⚠️

I have flagged my concerns about these UX/UI differences as @kelvinballoo has compared these with figma designs. These designs are either incorrect, or, we need to make some changes in this ticket. Its important that Figma design are accurate as we will be uding these for bug bashing.

@hussain-t is going to sync with @techanvil about this.

[techanvil]

Hi @kelvinballoo and @wpdarren, thanks for raising this.

The general design philosophy of Site Kit is to reuse generic components to achieve a consistent look and feel. So, reusing the ModalDialog component, which takes its width from its content, does seem like the right way to go here. However, I realise that results in a discrepancy with Figma. I have therefore asked @sigal-teller to comment on this particular case in a thread on Figma.

The question about the line break on the "insufficient permissions" variant is related and I've asked Sigal to clarify that too.

I don't think these issues need to hold this issue up, though - the current implementation is as I'd expect to see it, reusing ModalDialog in the same way we are elsewhere in the plugin, as specced in the IB. I'd suggest this can pass QA on the above points and we can raise a followup issue to address anything that may come out of Sigal's replies on Figma.

Cc @hussain-t

[hussain-t]

Thanks so much for the clarification, @techanvil ➕

[hussain-t]

On further note, the ModelDialog component was modified as part of #8110, as mentioned in the AC.

[kelvinballoo]

QA Update ✅

This has been tested good:

• The OAuth Error is triggered under the circumstances highlighted.

  

• If a permission error is returned by one of the API calls, the "insufficient permissions" variant of the Error Modal is displayed.

  

• If any other error is returned by one of the API calls, the generic error variant of the Error Modal is displayed.

  

• The Get help links to the Analytics support page in a new tab, scrolled to the insufficient permissions section.

  

Moving ticket to Approval.

[hussain-t]

Hi @kelvinballoo, after some investigation, I can confirm that 5.1 is an issue. As a result, I've created a follow-up PR to fix it. I will get back to you for another pass shortly. Thanks!

ITEM 5: Generic Error Variant

1. Clicking outside of modal makes enable disabled. Video is attached for reference.

[kuasha420]

Thanks @hussain-t, PR is now merged. Back to you @kelvinballoo for another pass. :)

[kelvinballoo]

QA Update ✅

This has been tested good after the latest fix:

• The OAuth Error is triggered under the circumstances highlighted.

  

• If a permission error is returned by one of the API calls, the "insufficient permissions" variant of the Error Modal is displayed.

  

• If any other error is returned by one of the API calls, the generic error variant of the Error Modal is displayed.

  

• The Get help links to the Analytics support page in a new tab, scrolled to the insufficient permissions section.

  

Only issue found was about a hover effect which is not applicable here. Raised a ticket under https://github.com/google/site-kit-wp/issues/9007

Moving this ticket to Approval.