site-kit-wp icon indicating copy to clipboard operation
site-kit-wp copied to clipboard

Auto-submit the Setup Component form having returned from the OAuth flow

Open techanvil opened this issue 2 years ago • 1 comments

Feature Description

Making use of the state persistence implemented in #5278, ensure the create/connect property execution is automatically continued when returning from the OAuth flow.

An example of this can be seen in the usage of the autoSubmit value in the Analytics SetupForm component.

  • Design Doc: https://docs.google.com/document/d/1DeWo38lcV7lvLFfcmt-mQ0iaxzB4qfiPArs_yZzYkIM/edit#heading=h.2g0r7ejoqctz
  • Figma: https://www.figma.com/file/vMaCWwr6lpk4PrJWb7jIpz/GA4-Banner-Input?node-id=1082%3A1526

Acceptance criteria

  • Having initiated the OAuth flow to grant the edit scope, via pressing the Create property CTA on the Setup Component, upon returning from the the OAuth flow the Setup Component should be displayed and the property creation should automatically continue, without the user having to press Create property again.
  • The effect should be as though the user has pressed Create property i.e. the activity spinner will be shown in the button while the property and measurement ID are created and connected.

Implementation Brief

The Setup Component is being Implemented in #5270 and #5275. Add the following logic to the Setup Component:

  • When the user returns from the OAuth flow, the Setup Component should be displayed using an autoSubmit state.
  • Get autoSubmit state of CORE_FORMS store by passing FORM_SETUP and 'autoSubmit' as a string.
  • Create a submitForm async callback using the useCallback hook with the following:
    • It receives the event as an argument and prevents the event's default behavior.
    • It should submit the changes by dispatching the submitChanges() action of the modules/analytics-4 store.
    • Check for permission scope error using isPermissionScopeError helper and pass the error object returned from the submitChanges() action.
    • If there that returns true, dispatch the setValues action of the CORE_FORMS store with autoSubmit set to true.
    • If there is no error, dispatch the setValues action of the CORE_FORMS store with autoSubmit set to false.
    • If there is no error, call the finishSetup callback.
  • Using the useEffect hook, call the submitForm callback when the autoSubmit state changes and is true.
  • If hasEditScope was already available, the condition in the useEffect hook should check for autoSubmit AND hasEditScope to call the submitForm callback.
  • Pass preventDefault as an empty method within an object as the event argument to the submitForm callback.
  • It should be similar to the autoSubmit logic in the Analytics SetupForm component. See: https://github.com/google/site-kit-wp/blob/ebbe2b8ab57ee87215f28f0395da1268fdf667f2/assets/js/modules/analytics/components/setup/SetupForm.js#L60-L90

Test Coverage

  • No new tests are to be added.

QA Brief

Changelog entry

techanvil avatar May 25 '22 17:05 techanvil

IB ✅

aaemnnosttv avatar Jul 25 '22 20:07 aaemnnosttv

QA Update: ❌

@techanvil I am seeing odd behaviour and not what the AC/QAB details.

Here are the steps I took to test this and the outcome:

  1. Created new site on InstaWP. Connected Site Kit and created a UA and GA4 property.
  2. I went into Analytics to trash the GA4 property and it shows that this was successful.
  3. I ran the SQL command DELETE FROM wp_options WHERE option_name = 'googlesitekit_analytics-4_settings'; to delete the GA4 account from Site Kit. That ran successfully.
  4. Next, I went into the DB and deleted the wp_googlesitekit_additional_auth_scopes meta key for my user. That was successful.

Here is the flow that I went through:

Step 1. On the Site Kit Dashboard, I saw the banner No existing GA4 banner found. As you can see from the screenshot, there's the message appearing that I will need to give permission to create an Analytics property. So the steps above seem to have worked as expected.

image

Step 2. I clicked on the Create Property CTA button and was redirected to the Google account slection screen to choose an account, which I did. This is the same account that I used to set up Site Kit initially.

Step 3. I was redirected to the oAuth screen and I gave permission.

Step 4. I was then redirected to Site Kit Dashboard and a banner notification, congratulating me for completing the setup of Site Kit. I was confused by the banner, so went to Settings for Analytics and noticed the Connect Google Analytics 4 button. What's interesting is that when I checked my Google Analytics account, a GA4 property was created during this process but Site Kit is not showing that.

Step 5. When the the Connect Google Analytics 4 button is clicked in settings, I was then sent to connect Analytics.

image

image

image

wpdarren avatar Sep 14 '22 10:09 wpdarren

Hi @wpdarren, thanks for spotting this.

The problem here is that the "Congrats on completing the setup for Site Kit!" success notification is presently hard wired to show up when returning from the OAuth flow, regardless of where it was initiated. This notification can be dismissed and will then not show up next time OAuth is transited. Once it's dismissed, or if it doesn't display due to a previous dismissal, the GA4 Activation Banner flow will continue as expected.

What we need to do is to prevent this notification from showing up at all when OAuth is initiated from the GA4 Activation Banner. However the solution for this should go through the usual review process. As such I have created a separate issue to fix it - https://github.com/google/site-kit-wp/issues/5837.

I would suggest that this current issue could be QA'd taking into account the success notification as described (i.e. it should essentially work when dismissing the notification, and also it should work without interruption having previously dismissed it). Alternatively, we can hold off until #5837 has been done.

Hopefully that all makes sense, please let me know what you think or if anything could do with a bit more clarification.

techanvil avatar Sep 14 '22 12:09 techanvil

Just an update note to confirm that will continue with testing this ticket when #5837 is merged.

wpdarren avatar Sep 14 '22 13:09 wpdarren

QA Update: ❌

@techanvil the issue reported above is fixed but I have two new observations:

  1. After giving permission via the oAuth screen there's a weird screen shift. Then the dashboard and GA4 banner notification load. You can see this in the screencast below around 18/19 seconds.

  2. When running through the flow as per the AC/QAB, everything looks perfect. I see the You have Successfully set up your GA4 property notification. When I go to Analytics I see the new GA4 property created, but, when I go to settings, I see the Connect Google Analytics 4 button hasn't been set up successfully.

I also tested this in a scenario where the user does not go through the oAuth because they have given permission and the same issue occurs. I am unsure if it is this ticket that has broken things here, or, #5837 which did pass the QAB. I'll move this ticket back to execution so it can be investigated.

https://user-images.githubusercontent.com/73545194/190840679-d4ecfa0d-d888-444b-9ebf-322abc702591.mp4

Screenshots

image image image image

wpdarren avatar Sep 17 '22 04:09 wpdarren

Thanks, @wpdarren. I have fixed observation #2 in this follow-up PR. As for the first one, I think it could be part of #5837. cc: @techanvil

hussain-t avatar Sep 19 '22 11:09 hussain-t

QA Update: ✅

Verified:

  • Upon clicking the Create property CTA, the user is redirected to the OAuth flow and grant access upon the return.
    • The user lands on the same setup after the successful OAuth flow and grant access.
    • The form is auto-submitted after returning from the OAuth flow.
    • The spinner is displayed while auto-submitting the form.
    • The property was created after the form auto-submit.
    • The issue above is fixed. GA4 property is created and connected successfully.

https://user-images.githubusercontent.com/73545194/191232611-31de7e5a-deeb-4608-9f8c-f3ad05fe3198.mp4

wpdarren avatar Sep 20 '22 10:09 wpdarren