woocommerce / google-listings-and-ads

Sync your store with Google to list products for free, run paid ads, and track performance straight from your store dashboard.
https://woo.com/products/google-listings-and-ads/
GNU General Public License v3.0
45 stars 21 forks source link

Onboarding: Default stores to destination-based tax rates during onboarding #2491

Open joemcgill opened 1 month ago

joemcgill commented 1 month ago

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

During the 2nd page of the onboarding process, we're hiding the setting that let's users configure the tax rates for the store (#2490). Therefore, we want to always default the tax rate to "destination" on initial setup. The user will be able to change this later, but it is not necessary to configure during onboarding.

Acceptance Criteria

Implementation Brief

When tax rates are changed, the frontend sends a REST API request to the /wp-json/wc/gla/mc/settings endpoint with updated values for the settings (including the tax_rate setting. The controller for that API endpoint is located in src/API/Site/Controllers/MerchantCenter/SettingsController.php and uses a schema to define and validate the settings values used. We may be able to simply update this schema SettingsController::get_schema_properties to set a default value for the tax_rate setting.

After setting a default, we may still need to update the controller to pre-fill the default value, see https://github.com/woocommerce/google-listings-and-ads/issues/2491#issuecomment-2277592379.

Test Coverage

Add new PHPUnit tests class for the SettingsController class in the tests/Unit/API/Site/Controllers/MerchantCenter/ folder. Include a test that ensures that when the settings are saved without passing a tax_rate that the return value will show the tax_rate set to "destination" (the default) and ensure that the underlying WP option is updated to include the default value.

Definition Questions

  1. Are there any other places besides the REST request that we need to modify to ensure this tax rate setting is set as a default to "destination"?
  2. We currently don't have any product direction for how someone would change this setting after we set the default. How are we handling this?
joemcgill commented 1 month ago

@mikkamp do you have any insight into this questin?

Are there any other places besides the REST request that we need to modify to ensure this tax rate setting is set as a default to "destination"?

mikkamp commented 1 month ago

Are there any other places besides the REST request that we need to modify to ensure this tax rate setting is set as a default to "destination"?

We have other locations where we set a default in the schema. However if I recall correctly, the WP_REST_Request class doesn't automatically extract this default value. For the reports we used the function get_default_params.

Once it's accepted as the default in the REST API request to /wp-json/wc/gla/mc/settings then there isn't anywhere else where it will need to be set. Some of the unit tests / E2E tests might need to get adjusted to no longer handle this setting.

We currently don't have any product direction for how someone would change this setting after we set the default. How are we handling this?

If we don't remove it, the setting will still be available after onboarding by going to Dashboard > Programs > Free listings > Edit, so for now we can leave it there until we decide to move some of those settings (there are suggestions for moving the shipping settings, but nothing finalized yet).

joemcgill commented 1 month ago

If we don't remove it, the setting will still be available after onboarding by going to Dashboard > Programs > Free listings > Edit, so for now we can leave it there until we decide to move some of those settings (there are suggestions for moving the shipping settings, but nothing finalized yet).

Thanks! Eason had also pointed this out in #2490 so I think we're all good here.