google-listings-and-ads icon indicating copy to clipboard operation
google-listings-and-ads copied to clipboard

Onboarding: Clean up settings API for pre-launch checklist items

Open joemcgill opened this issue 1 year ago • 6 comments

Part of https://github.com/woocommerce/google-listings-and-ads/issues/2458

This is a follow-up to #2492. Once the UI for the pre-launch checklist is removed, we can remove any backend functionality that is used for saving these options from the /wc/gla/mc/settings endpoint.

Acceptance Criteria

  • [ ] The schema for the /wp-json/wc/gla/mc/setup endpoint is updated to remove references to pre-launch fields: 'website_live', 'checkout_process_secure', 'payment_methods_visible', 'refund_tos_visible', and 'contact_info_visible' so those values no longer return.
  • [ ] Once the user has successfully added an address and validated a phone number, a GET request to the /wp-json/wc/gla/mc/setup endpoint should return paid_ads to show that the store_requirements step is completed.
  • [ ] The MerchantCenterSettings class is removed, since it's not in use.

Implementation Brief

The REST API controller for these settings is in src/API/Site/Controllers/MerchantCenter/SettingsController.php, specifically the get_schema_properties() method.

MerchantCenterService::get_setup_status() should be updated to remove the checked_pre_launch_checklist() requirement. MerchantCenterService::checked_pre_launch_checklist() is no longer needed and can be removed.

The MerchantCenterSettings class also references these settings but is not in use. We can remove that class as part of this work. See this comment.

Test Coverage

  • PHPUnit tests in tests/Unit/MerchantCenter/MerchantCenterServiceTest.php will need to be updated to remove the need to setup/verify pre-launch checks.
  • The endpoint may be used in E2E tests so those need to be cleaned up as well after removing this logic.

joemcgill avatar Aug 05 '24 15:08 joemcgill

The REST API controller for these settings is in src/API/Site/Controllers/MerchantCenter/SettingsController.php

The MerchantCenterSettings class also references these settings. However it seems this class is intentionally unused as mentioned in PR #255

There are some classes added to the \Automattic\WooCommerce\GoogleListingsAndAds\Value namespace. These are not currently being used, but I would like to see them used in the future. They can be reviewed or ignored, or I can pull them out of this PR and save them for another time since they are not used.

Since it's gone unused for a while, my vote would be towards removing it, we can always revert if it's needed again.

mikkamp avatar Aug 06 '24 09:08 mikkamp

I don't know if it belongs in another separate issue or is more part of #2492

Both the classes PolicyComplianceCheckController and PolicyComplianceCheck are solely used to confirm whether these pre-launch items can be marked as completed. So the UI will call mc/policy_check to check what values they return. This won't be needed/used anymore.

mikkamp avatar Aug 06 '24 10:08 mikkamp

I don't know if it belongs in another separate issue or is more part of https://github.com/woocommerce/google-listings-and-ads/issues/2492

#2458 mentions that we may use them later:

We can check these things post onboarding and surface to merchants if they’re missing something, like we currently do with MC issues.

For that reason, I'm thinking that we not remove the policy check functionality as a part of this issue, however, removing the unused MerchantCenterSettings class makes sense.

joemcgill avatar Aug 08 '24 18:08 joemcgill

@ankitguptaindia this one is ready for QA.

joemcgill avatar Aug 27 '24 21:08 joemcgill

@mikkamp this is ready for review.

joemcgill avatar Sep 01 '24 18:09 joemcgill

@kt-12 just some small feedback to handle on this PR and this should be good to go.

joemcgill avatar Sep 04 '24 19:09 joemcgill

Closing this as completed since it was part of the 2.9 release.

mikkamp avatar Dec 02 '24 15:12 mikkamp