API Pull - Grant access to Google WPCOM app in onboarding flow #2337

ianlin · 2024-03-27T10:56:32Z

Changes proposed in this Pull Request:

Project: pcTzPl-1SM-p2
Figma: JjJwsuOziVFv6eP1HNFhU8-fi-18069-355

Part of #2146 and similar to #2311. This PR adds the granting access to Google WPCOM app flow in the onboarding flow.

What has been changed:

Add a shared hook to handle REST API auth URL redirect, both settings page and onboarding can use this hook.
Add a new setup mc step grant_rest_api_access. Like the step accounts, its index is also 1 as we want the UI stays on the set up accounts page when the step grant_rest_api_access is still incomplete.
- This step is only valid when notification service is enabled (by setting the filter woocommerce_gla_notifications_enabled to true)
Remove a prop hideNotificationService in the component ConnectedGoogleMCAccountCard as we would always show it on both settings and onboarding pages.
Add a prop hideDisableProductFetch in the component ConnectedGoogleMCAccountCard as we are not showing Disable product data fetch in the onboarding flow when the granting access is approved.
- This is open for discussion, whether we'd want to show it in the onboarding flow.

Screenshots:

Detailed test instructions:

Approved

Checkout this branch
Set the filter woocommerce_gla_notifications_enabled to true
Make sure the option gla_wpcom_rest_api_status is not set
Delete all connected accounts, go to the onboarding flow by going to /wp-admin/admin.php?page=wc-admin&path=/google/setup-mc
Connect Google account
Connect MC account
After MC account is connected, it will automatically be redirect to the WPCOM app granting access page
Finish the auth step, it will eventually be redirect to https://woo.com
- Because currently we are using a test WPCOM app, in the future this will the Google's WPCOM app and they will help us to redirect back to the merchant site.
Go back to setup mc page with an extra query parameter google_wpcom_app_status=approved. E.g.
- /wp-admin/admin.php?page=wc-admin&path=/google/setup-mc&google_wpcom_app_status=approved
The UI shows MC account connected and also Google has been granted access to fetch your product data.
The UI does not show Disable product data fetch like that in the settings page. API Pull - Grant access to google WPCOM app in settings page #2311

Disapproved or Error

Disconnect MC account by calling DELETE /wc/gla/mc/connection, and delete the option gla_wpcom_rest_api_status
Go to setup mc page
Reconnect MC account, and finish the granting access flow
Go back to setup mc page with an extra query parameter google_wpcom_app_status=error or google_wpcom_app_status=disapproved. E.g.
- /wp-admin/admin.php?page=wc-admin&path=/google/setup-mc&google_wpcom_app_status=disapproved
The UI shows MC account connected but There was an issue granting access to Google for fetching your products.
Click Grant access will redirect to WPCOM app granting access page

Notifications Service is not Enabled

Set the filter woocommerce_gla_notifications_enabled to false
Delete all accounts and delete the option gla_wpcom_rest_api_status
Connect MC account, it will not be redirected to WPCOM granting access page
Set the option gla_wpcom_rest_api_status to approved, disapproved, or error
The MC card will not display new product data fetch notice.

Additional details

Verifying who can set the REST API status by setting a query parameter google_wpcom_app_status will be implemented in another PR.

Changelog entry

…roduct sync button

…ding setup mc

codecov · 2024-03-27T11:00:08Z

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (feature/google-api-project-phase-2@b51c815). Click here to learn what that means.

❗ Current head 43104de differs from pull request most recent head 07be97d. Consider uploading reports for the commit 07be97d to get more accurate results

Additional details and impacted files

@@                         Coverage Diff                          @@
##             feature/google-api-project-phase-2   #2337   +/-   ##
====================================================================
  Coverage                                      ?   62.8%           
  Complexity                                    ?    4481           
====================================================================
  Files                                         ?     765           
  Lines                                         ?   22616           
  Branches                                      ?     543           
====================================================================
  Hits                                          ?   14195           
  Misses                                        ?    7964           
  Partials                                      ?     457

Flag	Coverage Δ
js-unit-tests	`55.3% <ø> (?)`
php-unit-tests	`64.2% <100.0%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

Files	Coverage Δ
src/MerchantCenter/MerchantCenterService.php	`94.4% <100.0%> (ø)`

…-app-in-setting-page' into add/grant-access-to-google-wpcom-app-in-onboarding-flow

If no `step` query param provided, use the step from MC setup.

…setup mc" This reverts commit 712f38c.

Note that this step shares the same key with the step `accounts` defined in `js/src/setup-mc/setup-stepper/stepNameKeyMap.js`, so when this step hasn't finished, the UI will stay on the setup accounts page.

…ogle-wpcom-app-in-onboarding-flow

Because we wouldn't want the merchant to disable it during onboarding. Also remove hideNotificationService since we will always show it in both onboarding and settings page.

…ogle-wpcom-app-in-onboarding-flow

puntope · 2024-04-03T13:39:24Z

Just a note here: If this is for next version I guess we should create a separate branch than feature/google-api-project as this one is for the MVP.

puntope · 2024-04-04T19:44:07Z

❓ Doesn't matter if the grant access gets an error or is approved. You can always continue the setup. I assume this is the expected behaviour

puntope · 2024-04-04T19:51:13Z

Wonder if we might add some events/tracks regarding the final setup. (ie. If the auth is granted)

puntope · 2024-04-04T19:51:31Z

What about adding some tests for the UI as well?

puntope · 2024-04-04T19:18:33Z

js/src/components/google-mc-account-card/connected-google-mc-account-card.js

@@ -40,12 +40,12 @@ import { getSettingsUrl } from '.~/utils/urls';
 * @param {Object} props React props.
 * @param {{ id: number }} props.googleMCAccount A data payload object containing the user's Google Merchant Center account ID.
 * @param {boolean} [props.hideAccountSwitch=false] Indicate whether hide the account switch block at the card footer.
- * @param {boolean} [props.hideNotificationService=true] Indicate whether hide the enable Notification service block at the card footer.
+ * @param {boolean} [props.hideDisableProductFetch=true] Indicate whether hide the disable product fetch block at the card footer.


I guess this should be [props.hideDisableProductFetch=false]

puntope · 2024-04-04T19:19:09Z

js/src/hooks/useRestAPIAuthURLRedirect.js

+import useApiFetchCallback from '.~/hooks/useApiFetchCallback';
+import useDispatchCoreNotices from '.~/hooks/useDispatchCoreNotices';
+
+const useRestAPIAuthURLRedirect = () => {


Maybe we can add some JSDOCS for this Hook?

puntope · 2024-04-04T19:29:42Z

src/MerchantCenter/MerchantCenterService.php


-				if ( $this->is_mc_contact_information_setup() && $this->checked_pre_launch_checklist() ) {
-					$step = 'paid_ads';
+			if ( ! $notifications_enabled || $this->is_rest_api_granted() ) {


instead of $this->is_rest_api_granted() we might use NotificationsService::is_ready() that does the same so we can avoid adding more code doing the same.

…ss-to-google-wpcom-app-in-onboarding-flow

ianlin added 4 commits March 27, 2024 14:58

Add a hook to handle REST API auth URL redirect

bab0207

Use useRestAPIAuthURLRedirect to handle auth URL redirect in enable p…

fa23f4d

…roduct sync button

Redirect to REST API auth url seamlessly after MC connected in onboar…

0d21366

…ding setup mc

Use a shared hook to update rest api auth status in onboarding setup mc

e0a7099

github-actions bot added changelog: add A new feature, function, or functionality was added. type: enhancement The issue is a request for an enhancement. labels Mar 27, 2024

ianlin added 6 commits March 29, 2024 15:38

Merge remote-tracking branch 'origin/add/grant-access-to-google-wpcom…

b69b9ea

…-app-in-setting-page' into add/grant-access-to-google-wpcom-app-in-onboarding-flow

Prevent fetchRestAPIAuthorize from being called multiple times

e55a9d8

Show product sync status notice in GMC card in onboarding flow

c24f44f

Use query param step to navigate to the step in onboarding setup mc

712f38c

If no `step` query param provided, use the step from MC setup.

Revert "Use query param step to navigate to the step in onboarding …

9f12af8

…setup mc" This reverts commit 712f38c.

Add a setup MC step grant_rest_api_access

446271f

Note that this step shares the same key with the step `accounts` defined in `js/src/setup-mc/setup-stepper/stepNameKeyMap.js`, so when this step hasn't finished, the UI will stay on the setup accounts page.

Base automatically changed from add/grant-access-to-google-wpcom-app-in-setting-page to feature/google-api-project April 2, 2024 08:47

ianlin added 5 commits April 2, 2024 16:48

Merge branch 'feature/google-api-project' into add/grant-access-to-go…

0c8a46e

…ogle-wpcom-app-in-onboarding-flow

Merge branch 'feature/google-api-project' into add/grant-access-to-go…

8087258

…ogle-wpcom-app-in-onboarding-flow

Add a prop hideDisableProductFetch in ConnectedGoogleMCAccountCard

bfa5b79

Because we wouldn't want the merchant to disable it during onboarding. Also remove hideNotificationService since we will always show it in both onboarding and settings page.

Merge branch 'feature/google-api-project' into add/grant-access-to-go…

ebc4984

…ogle-wpcom-app-in-onboarding-flow

Skip the step grant_rest_api_access if notifications is not enabled

43104de

ianlin requested a review from a team April 3, 2024 12:35

ianlin self-assigned this Apr 3, 2024

ianlin marked this pull request as ready for review April 3, 2024 12:35

puntope reviewed Apr 4, 2024

View reviewed changes

ianlin changed the base branch from feature/google-api-project to feature/google-api-project-phase-2 April 8, 2024 07:39

Merge branch 'feature/google-api-project-phase-2' into add/grant-acce…

07be97d

…ss-to-google-wpcom-app-in-onboarding-flow

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

API Pull - Grant access to Google WPCOM app in onboarding flow #2337

API Pull - Grant access to Google WPCOM app in onboarding flow #2337

ianlin commented Mar 27, 2024 •

edited

codecov bot commented Mar 27, 2024 •

edited

puntope commented Apr 3, 2024

puntope commented Apr 4, 2024

puntope commented Apr 4, 2024

puntope commented Apr 4, 2024

puntope Apr 4, 2024

puntope Apr 4, 2024

puntope Apr 4, 2024

API Pull - Grant access to Google WPCOM app in onboarding flow #2337

Are you sure you want to change the base?

API Pull - Grant access to Google WPCOM app in onboarding flow #2337

Conversation

ianlin commented Mar 27, 2024 • edited

Changes proposed in this Pull Request:

Screenshots:

Detailed test instructions:

Approved

Disapproved or Error

Notifications Service is not Enabled

Additional details

Changelog entry

codecov bot commented Mar 27, 2024 • edited

Codecov Report

puntope commented Apr 3, 2024

puntope commented Apr 4, 2024

puntope commented Apr 4, 2024

puntope commented Apr 4, 2024

puntope Apr 4, 2024

Choose a reason for hiding this comment

puntope Apr 4, 2024

Choose a reason for hiding this comment

puntope Apr 4, 2024

Choose a reason for hiding this comment

ianlin commented Mar 27, 2024 •

edited

codecov bot commented Mar 27, 2024 •

edited