From 315f77dd1d465048bb80cc42b5e49ff8b6197612 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Fri, 20 Dec 2024 14:39:54 +0800 Subject: [PATCH 01/10] Remove the `hideTaxRates` prop from the `SetupFreeListings` component and the related components and functions. --- .../configure-product-listings/checkErrors.js | 4 +--- .../free-listings/setup-free-listings/form-content.js | 5 +---- .../free-listings/setup-free-listings/index.js | 10 ++-------- .../onboarding/setup-stepper/saved-setup-stepper.js | 1 - 4 files changed, 4 insertions(+), 16 deletions(-) diff --git a/js/src/components/free-listings/configure-product-listings/checkErrors.js b/js/src/components/free-listings/configure-product-listings/checkErrors.js index 20b9e663ee..b7c194a64a 100644 --- a/js/src/components/free-listings/configure-product-listings/checkErrors.js +++ b/js/src/components/free-listings/configure-product-listings/checkErrors.js @@ -12,8 +12,7 @@ const checkErrors = ( values, shippingTimes, finalCountryCodes, - storeCountryCode, - hideTaxRates = false + storeCountryCode ) => { const errors = {}; @@ -106,7 +105,6 @@ const checkErrors = ( * Check tax rate (required for U.S. only). */ if ( - ! hideTaxRates && ( storeCountryCode === 'US' || finalCountryCodes.includes( 'US' ) ) && ! validTaxRateSet.has( values.tax_rate ) ) { diff --git a/js/src/components/free-listings/setup-free-listings/form-content.js b/js/src/components/free-listings/setup-free-listings/form-content.js index 7000c62255..12d83ed86f 100644 --- a/js/src/components/free-listings/setup-free-listings/form-content.js +++ b/js/src/components/free-listings/setup-free-listings/form-content.js @@ -25,16 +25,13 @@ import isNonFreeShippingRate from '~/utils/isNonFreeShippingRate'; * * @param {Object} props React props. * @param {string} [props.submitLabel="Complete setup"] Submit button label. - * @param {boolean} [props.hideTaxRates] Whether to hide tax rate section. */ const FormContent = ( { submitLabel = __( 'Complete setup', 'google-listings-and-ads' ), - hideTaxRates, } ) => { const { values, isValidForm, handleSubmit, adapter } = useAdaptiveFormContext(); - const displayTaxRate = useDisplayTaxRate( adapter.audienceCountries ); - const shouldDisplayTaxRate = ! hideTaxRates && displayTaxRate; + const shouldDisplayTaxRate = useDisplayTaxRate( adapter.audienceCountries ); const shouldDisplayShippingTime = values.shipping_time === 'flat'; const shouldDisplayOrderValueCondition = values.shipping_rate === 'flat' && diff --git a/js/src/components/free-listings/setup-free-listings/index.js b/js/src/components/free-listings/setup-free-listings/index.js index 8b08c1779a..ab8e4ec019 100644 --- a/js/src/components/free-listings/setup-free-listings/index.js +++ b/js/src/components/free-listings/setup-free-listings/index.js @@ -69,7 +69,6 @@ const getSettings = ( values ) => { * @param {() => void} [props.onContinue] Callback called once continue button is clicked. Could be async. While it's being resolved the form would turn into a saving state. * @param {string} [props.submitLabel] Submit button label, to be forwarded to `FormContent`. * @param {JSX.Element} props.headerTitle Title in the header block of this setup. - * @param {boolean} [props.hideTaxRates=false] Whether to hide tax rate section, to be forwarded to `FormContent`. */ const SetupFreeListings = ( { targetAudience, @@ -84,7 +83,6 @@ const SetupFreeListings = ( { onContinue = noop, submitLabel, headerTitle, - hideTaxRates = false, } ) => { const formRef = useRef(); const { code: storeCountryCode } = useStoreCountry(); @@ -101,8 +99,7 @@ const SetupFreeListings = ( { values, shippingTimesData, countries, - storeCountryCode, - hideTaxRates + storeCountryCode ); }; @@ -233,10 +230,7 @@ const SetupFreeListings = ( { validate={ handleValidate } onSubmit={ onContinue } > - + ); diff --git a/js/src/pages/onboarding/setup-stepper/saved-setup-stepper.js b/js/src/pages/onboarding/setup-stepper/saved-setup-stepper.js index 2aed34243a..8001fd6790 100644 --- a/js/src/pages/onboarding/setup-stepper/saved-setup-stepper.js +++ b/js/src/pages/onboarding/setup-stepper/saved-setup-stepper.js @@ -165,7 +165,6 @@ const SavedSetupStepper = ( { savedStep } ) => { targetAudience={ initTargetAudience } settings={ initSettings } shippingRates={ initShippingRates } - hideTaxRates={ true } shippingTimes={ initShippingTimes } resolveFinalCountries={ getFinalCountries } onTargetAudienceChange={ handleFormChange.bind( From 630d1f361f660d04ff60365899387e8d4299dfac Mon Sep 17 00:00:00 2001 From: Eason Su Date: Fri, 20 Dec 2024 17:34:25 +0800 Subject: [PATCH 02/10] Remove the tax rate setting from the `SetupFreeListings` component and the related components and functions. --- .../__snapshots__/checkErrors.test.js.snap | 8 --- .../configure-product-listings/checkErrors.js | 21 +----- .../checkErrors.test.js | 69 ------------------- .../setup-free-listings/form-content.js | 10 +-- .../setup-free-listings/index.js | 12 +--- .../minimum-order-card.test.js | 1 - .../shipping-rate-section.test.js | 1 - 7 files changed, 5 insertions(+), 117 deletions(-) diff --git a/js/src/components/free-listings/configure-product-listings/__snapshots__/checkErrors.test.js.snap b/js/src/components/free-listings/configure-product-listings/__snapshots__/checkErrors.test.js.snap index 5138c253eb..32f5668b0c 100644 --- a/js/src/components/free-listings/configure-product-listings/__snapshots__/checkErrors.test.js.snap +++ b/js/src/components/free-listings/configure-product-listings/__snapshots__/checkErrors.test.js.snap @@ -6,14 +6,6 @@ exports[`checkErrors Audience When the audience location option is an invalid va exports[`checkErrors Audience When the audience location option is an invalid value or missing, should not pass 2`] = `"Please select a location option."`; -exports[`checkErrors For tax rate, if store country code or selected country codes include 'US' When the tax rate option is an invalid value or missing, should not pass 1`] = `"Please specify tax rate option."`; - -exports[`checkErrors For tax rate, if store country code or selected country codes include 'US' When the tax rate option is an invalid value or missing, should not pass 2`] = `"Please specify tax rate option."`; - -exports[`checkErrors For tax rate, if store country code or selected country codes include 'US' When the tax rate option is an invalid value or missing, should not pass 3`] = `"Please specify tax rate option."`; - -exports[`checkErrors For tax rate, if store country code or selected country codes include 'US' When the tax rate option is an invalid value or missing, should not pass 4`] = `"Please specify tax rate option."`; - exports[`checkErrors Offer free shipping With flat shipping rate option When there are some non-free shipping rates, and offer free shipping is checked, and there is no minimum order amount for non-free shipping rates, should not pass 1`] = `"Please enter minimum order for free shipping."`; exports[`checkErrors Shipping rates For flat type When there are any selected countries with shipping rates not set, should not pass 1`] = `"Please specify estimated shipping rates for all the countries, and the rate cannot be less than 0."`; diff --git a/js/src/components/free-listings/configure-product-listings/checkErrors.js b/js/src/components/free-listings/configure-product-listings/checkErrors.js index b7c194a64a..200a59a5b7 100644 --- a/js/src/components/free-listings/configure-product-listings/checkErrors.js +++ b/js/src/components/free-listings/configure-product-listings/checkErrors.js @@ -6,14 +6,8 @@ import { __ } from '@wordpress/i18n'; const validlocationSet = new Set( [ 'all', 'selected' ] ); const validShippingRateSet = new Set( [ 'automatic', 'flat', 'manual' ] ); const validShippingTimeSet = new Set( [ 'flat', 'manual' ] ); -const validTaxRateSet = new Set( [ 'destination', 'manual' ] ); -const checkErrors = ( - values, - shippingTimes, - finalCountryCodes, - storeCountryCode -) => { +const checkErrors = ( values, shippingTimes, finalCountryCodes ) => { const errors = {}; // Check audience. @@ -101,19 +95,6 @@ const checkErrors = ( ); } - /** - * Check tax rate (required for U.S. only). - */ - if ( - ( storeCountryCode === 'US' || finalCountryCodes.includes( 'US' ) ) && - ! validTaxRateSet.has( values.tax_rate ) - ) { - errors.tax_rate = __( - 'Please specify tax rate option.', - 'google-listings-and-ads' - ); - } - return errors; }; diff --git a/js/src/components/free-listings/configure-product-listings/checkErrors.test.js b/js/src/components/free-listings/configure-product-listings/checkErrors.test.js index 52e37e4151..5e2debb65e 100644 --- a/js/src/components/free-listings/configure-product-listings/checkErrors.test.js +++ b/js/src/components/free-listings/configure-product-listings/checkErrors.test.js @@ -41,7 +41,6 @@ describe( 'checkErrors', () => { countries: [ 'US', 'JP' ], shipping_rate: 'flat', shipping_time: 'flat', - tax_rate: 'manual', shipping_country_rates: toRates( [ 'US', 10 ], [ 'JP', 30, 88 ] ), offer_free_shipping: true, }; @@ -436,72 +435,4 @@ describe( 'checkErrors', () => { } ); } ); } ); - - describe( `For tax rate, if store country code or selected country codes include 'US'`, () => { - let codes; - - beforeEach( () => { - codes = [ 'US' ]; - } ); - - it( `When the tax rate option is an invalid value or missing, should not pass`, () => { - // Not set yet - let errors = checkErrors( defaultFormValues, [], codes ); - - expect( errors ).toHaveProperty( 'tax_rate' ); - expect( errors.tax_rate ).toMatchSnapshot(); - - errors = checkErrors( defaultFormValues, [], [], 'US' ); - - expect( errors ).toHaveProperty( 'tax_rate' ); - expect( errors.tax_rate ).toMatchSnapshot(); - - // Invalid value - errors = checkErrors( - { ...defaultFormValues, tax_rate: true }, - [], - codes - ); - - expect( errors ).toHaveProperty( 'tax_rate' ); - expect( errors.tax_rate ).toMatchSnapshot(); - - // Invalid value - errors = checkErrors( - { ...defaultFormValues, tax_rate: 'invalid' }, - [], - codes - ); - - expect( errors ).toHaveProperty( 'tax_rate' ); - expect( errors.tax_rate ).toMatchSnapshot(); - } ); - - it( 'When the tax rate option is a valid value, should pass', () => { - // Selected destination - const destinationTaxRate = { - ...defaultFormValues, - tax_rate: 'destination', - }; - - let errors = checkErrors( destinationTaxRate, [], codes ); - - expect( errors ).not.toHaveProperty( 'tax_rate' ); - - errors = checkErrors( destinationTaxRate, [], [], 'US' ); - - expect( errors ).not.toHaveProperty( 'tax_rate' ); - - // Selected manual - const manualTaxRate = { ...defaultFormValues, tax_rate: 'manual' }; - - errors = checkErrors( manualTaxRate, [], codes ); - - expect( errors ).not.toHaveProperty( 'tax_rate' ); - - errors = checkErrors( destinationTaxRate, [], [], 'US' ); - - expect( errors ).not.toHaveProperty( 'tax_rate' ); - } ); - } ); } ); diff --git a/js/src/components/free-listings/setup-free-listings/form-content.js b/js/src/components/free-listings/setup-free-listings/form-content.js index 12d83ed86f..34d92a5278 100644 --- a/js/src/components/free-listings/setup-free-listings/form-content.js +++ b/js/src/components/free-listings/setup-free-listings/form-content.js @@ -10,13 +10,10 @@ import { useAdaptiveFormContext } from '~/components/adaptive-form'; import StepContent from '~/components/stepper/step-content'; import StepContentActions from '~/components/stepper/step-content-actions'; import StepContentFooter from '~/components/stepper/step-content-footer'; -import TaxRate from '~/pages/settings/setup-tax-rate/tax-rate'; -import useDisplayTaxRate from '~/pages/settings/setup-tax-rate/useDisplayTaxRate'; import ChooseAudienceSection from '~/components/free-listings/choose-audience-section'; import ShippingRateSection from '~/components/shipping-rate-section'; import ShippingTimeSection from '~/components/free-listings/configure-product-listings/shipping-time-section'; import AppButton from '~/components/app-button'; -import ConditionalSection from '~/components/conditional-section'; import OrderValueConditionSection from '~/components/order-value-condition-section'; import isNonFreeShippingRate from '~/utils/isNonFreeShippingRate'; @@ -31,14 +28,14 @@ const FormContent = ( { } ) => { const { values, isValidForm, handleSubmit, adapter } = useAdaptiveFormContext(); - const shouldDisplayTaxRate = useDisplayTaxRate( adapter.audienceCountries ); + const shouldDisplayShippingTime = values.shipping_time === 'flat'; const shouldDisplayOrderValueCondition = values.shipping_rate === 'flat' && values.shipping_country_rates.some( isNonFreeShippingRate ); const handleSubmitClick = ( event ) => { - if ( shouldDisplayTaxRate !== null && isValidForm ) { + if ( isValidForm ) { return handleSubmit( event ); } @@ -53,9 +50,6 @@ const FormContent = ( { ) } { shouldDisplayShippingTime && } - - - { const formRef = useRef(); - const { code: storeCountryCode } = useStoreCountry(); if ( ! ( targetAudience && settings && shippingRates && shippingTimes ) ) { return ; @@ -95,12 +93,7 @@ const SetupFreeListings = ( { const countries = resolveFinalCountries( values ); const { shipping_country_times: shippingTimesData } = values; - return checkErrors( - values, - shippingTimesData, - countries, - storeCountryCode - ); + return checkErrors( values, shippingTimesData, countries ); }; const handleChange = ( change, values ) => { @@ -217,7 +210,6 @@ const SetupFreeListings = ( { // These are the fields for settings. shipping_rate: settings.shipping_rate, shipping_time: settings.shipping_time, - tax_rate: settings.tax_rate, // This is used in UI only, not used in API. offer_free_shipping: getOfferFreeShippingInitialValue( shippingRates ), diff --git a/js/src/components/order-value-condition-section/minimum-order-card/minimum-order-card.test.js b/js/src/components/order-value-condition-section/minimum-order-card/minimum-order-card.test.js index 5813e59b94..7b316c0d99 100644 --- a/js/src/components/order-value-condition-section/minimum-order-card/minimum-order-card.test.js +++ b/js/src/components/order-value-condition-section/minimum-order-card/minimum-order-card.test.js @@ -43,7 +43,6 @@ const adaptiveFormContextDefaultValues = { shipping_country_times: [], shipping_rate: true, shipping_time: true, - tax_rate: null, }; const adaptiveFormInputPropsDefaultValues = { diff --git a/js/src/components/shipping-rate-section/shipping-rate-section.test.js b/js/src/components/shipping-rate-section/shipping-rate-section.test.js index 053fa09fb0..9d54c2cc1b 100644 --- a/js/src/components/shipping-rate-section/shipping-rate-section.test.js +++ b/js/src/components/shipping-rate-section/shipping-rate-section.test.js @@ -40,7 +40,6 @@ jest.mock( '~/components/adaptive-form', () => ( { shipping_country_times: [], shipping_rate: 'flat', shipping_time: 'flat', - tax_rate: null, }, }; } ), From ad50a3cdad895974b224cb5fc9f4338b403af56e Mon Sep 17 00:00:00 2001 From: Eason Su Date: Thu, 2 Jan 2025 14:53:46 +0800 Subject: [PATCH 03/10] Remove the validation error message from `TaxRate` component and disable options during submission. --- js/src/pages/settings/setup-tax-rate/tax-rate.js | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/js/src/pages/settings/setup-tax-rate/tax-rate.js b/js/src/pages/settings/setup-tax-rate/tax-rate.js index 1d2fa86a6e..66a0ea1d8b 100644 --- a/js/src/pages/settings/setup-tax-rate/tax-rate.js +++ b/js/src/pages/settings/setup-tax-rate/tax-rate.js @@ -22,7 +22,7 @@ import VerticalGapLayout from '~/components/vertical-gap-layout'; const TaxRate = () => { const { getInputProps, - adapter: { renderRequestedValidation }, + adapter: { isSubmitting }, } = useAdaptiveFormContext(); return ( @@ -62,6 +62,7 @@ const TaxRate = () => { ) } value="destination" collapsible + disabled={ isSubmitting } > { __( @@ -78,6 +79,7 @@ const TaxRate = () => { ) } value="manual" collapsible + disabled={ isSubmitting } > { createInterpolateElement( @@ -98,7 +100,6 @@ const TaxRate = () => { - { renderRequestedValidation( 'tax_rate' ) } From 14e707021243dc4fd3ac624bf6429b9fb0f0d4f3 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Thu, 2 Jan 2025 14:56:20 +0800 Subject: [PATCH 04/10] Adjust `TaxRate` component to render children within its section wrapper and below its card. --- js/src/pages/settings/setup-tax-rate/tax-rate.js | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/js/src/pages/settings/setup-tax-rate/tax-rate.js b/js/src/pages/settings/setup-tax-rate/tax-rate.js index 66a0ea1d8b..6474ea9d61 100644 --- a/js/src/pages/settings/setup-tax-rate/tax-rate.js +++ b/js/src/pages/settings/setup-tax-rate/tax-rate.js @@ -19,7 +19,13 @@ import VerticalGapLayout from '~/components/vertical-gap-layout'; * @fires gla_documentation_link_click with `{ context: 'setup-mc-tax-rate', link_id: 'tax-rate-manual', href: 'https://www.google.com/retail/solutions/merchant-center/' }` */ -const TaxRate = () => { +/** + * Renders the options of tax rate. + * + * @param {Object} props React props. + * @param {JSX.Element} [props.children] Children to be rendered below the card. + */ +const TaxRate = ( { children } ) => { const { getInputProps, adapter: { isSubmitting }, @@ -102,6 +108,7 @@ const TaxRate = () => { + { children } ); }; From e14ab9f22d62f31620c17941a81721e976794950 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Thu, 2 Jan 2025 15:33:58 +0800 Subject: [PATCH 05/10] Add a new component to save and sync the tax rate setup individually. --- js/src/pages/settings/setup-tax-rate/index.js | 1 + .../settings/setup-tax-rate/setup-tax-rate.js | 117 ++++++++++++++++++ 2 files changed, 118 insertions(+) create mode 100644 js/src/pages/settings/setup-tax-rate/index.js create mode 100644 js/src/pages/settings/setup-tax-rate/setup-tax-rate.js diff --git a/js/src/pages/settings/setup-tax-rate/index.js b/js/src/pages/settings/setup-tax-rate/index.js new file mode 100644 index 0000000000..541e3d049a --- /dev/null +++ b/js/src/pages/settings/setup-tax-rate/index.js @@ -0,0 +1 @@ +export { default } from './setup-tax-rate'; diff --git a/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js b/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js new file mode 100644 index 0000000000..8920bf5a92 --- /dev/null +++ b/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js @@ -0,0 +1,117 @@ +/** + * External dependencies + */ +import { __ } from '@wordpress/i18n'; +import { Flex } from '@wordpress/components'; + +/** + * Internal dependencies + */ +import Section from '~/components/section'; +import AppSpinner from '~/components/app-spinner'; +import AppButton from '~/components/app-button'; +import AdaptiveForm from '~/components/adaptive-form'; +import TaxRate from './tax-rate'; +import useSettings from '~/hooks/useSettings'; +import useDisplayTaxRate from './useDisplayTaxRate'; +import useTargetAudienceFinalCountryCodes from '~/hooks/useTargetAudienceFinalCountryCodes'; +import { handleApiError } from '~/utils/handleError'; + +const validTaxRateSet = new Set( [ 'destination', 'manual' ] ); + +/** + * Renders the tax rate setup if the current target audience requires it. + * + * This component won't display the validation error message on UI, + * because it should be obvious to the user that they have to select + * one of the radio options to continue the submission. + */ +export default function SetupTaxRate() { + const { settings, saveSettings, syncSettings } = useSettings(); + const { data: audienceCountries } = useTargetAudienceFinalCountryCodes(); + const shouldDisplayTaxRate = useDisplayTaxRate( audienceCountries ); + + if ( ! shouldDisplayTaxRate || ! settings?.hasOwnProperty( 'tax_rate' ) ) { + if ( shouldDisplayTaxRate === false ) { + return null; + } + + return ( +
+ +
+ ); + } + + const handleValidate = ( values ) => { + const errors = {}; + + if ( ! validTaxRateSet.has( values.tax_rate ) ) { + errors.tax_rate = __( + 'Please specify tax rate option.', + 'google-listings-and-ads' + ); + } + + return errors; + }; + + const handleSubmit = async ( values ) => { + const nextSettings = { + ...settings, + tax_rate: values.tax_rate, + }; + + return saveSettings( nextSettings ) + .then( syncSettings, ( error ) => { + handleApiError( + error, + __( + 'There was an error saving tax rate.', + 'google-listings-and-ads' + ) + ); + } ) + .catch( ( error ) => { + handleApiError( + error, + __( + 'There was an error synchronizing tax rate to Google Merchant Center.', + 'google-listings-and-ads' + ) + ); + } ); + }; + + return ( + + { ( formContext ) => { + const { values, isValidForm } = formContext; + const taxRate = values.tax_rate; + const disabled = ! isValidForm || taxRate === settings.tax_rate; + + return ( + + + + { __( + 'Save tax rate', + 'google-listings-and-ads' + ) } + + + + ); + } } + + ); +} From 3e59f95be1872d83084f9e74cddf84c002894f20 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Thu, 2 Jan 2025 17:10:10 +0800 Subject: [PATCH 06/10] Add the tax rate setup to the Settings page. --- js/src/pages/settings/index.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/js/src/pages/settings/index.js b/js/src/pages/settings/index.js index 39338c24ab..2058a8b610 100644 --- a/js/src/pages/settings/index.js +++ b/js/src/pages/settings/index.js @@ -13,6 +13,7 @@ import useGoogleAccount from '~/hooks/useGoogleAccount'; import useUpdateRestAPIAuthorizeStatusByUrlQuery from '~/hooks/useUpdateRestAPIAuthorizeStatusByUrlQuery'; import { subpaths, getReconnectAccountUrl } from '~/utils/urls'; import { ContactInformationPreview } from '~/components/contact-information'; +import SetupTaxRate from './setup-tax-rate'; import LinkedAccounts from './linked-accounts'; import ReconnectWPComAccount from './reconnect-wpcom-account'; import ReconnectGoogleAccount from './reconnect-google-account'; @@ -65,6 +66,7 @@ const Settings = () => { + ); From fdd48bd32b629bcb72283c252f0eebf5ab91088e Mon Sep 17 00:00:00 2001 From: Eason Su Date: Thu, 2 Jan 2025 17:13:10 +0800 Subject: [PATCH 07/10] Remove unused `ConditionalSection` component. --- js/src/components/conditional-section.js | 34 ------------------------ 1 file changed, 34 deletions(-) delete mode 100644 js/src/components/conditional-section.js diff --git a/js/src/components/conditional-section.js b/js/src/components/conditional-section.js deleted file mode 100644 index 57a9bc7ccd..0000000000 --- a/js/src/components/conditional-section.js +++ /dev/null @@ -1,34 +0,0 @@ -/** - * Internal dependencies - */ -import Section from '~/components/section'; -import AppSpinner from '~/components/app-spinner'; - -/** - * Component to conditionally render a section. - * If `show` is set to - * - `true` returns given children - * - `false` returns null - * - `null` - unspecified, render section preloader - * - * @param {Object} props React props - * @param {boolean | null} props.show Flag to indicate whether the element should be shown. - * @param {Array} props.children Content to be rendered. - * @return {Array | null} Children, preloader section, or null. - */ -const ConditionalSection = ( { show, children } ) => { - switch ( show ) { - case true: - return children; - case false: - return null; - default: - return ( -
- -
- ); - } -}; - -export default ConditionalSection; From 1eda2d7bd1272f59775aa22f8b54502c73d56665 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Fri, 3 Jan 2025 11:49:04 +0800 Subject: [PATCH 08/10] Update E2E tests. --- .../dashboard/edit-free-listings.test.js | 1 - tests/e2e/specs/settings/settings.test.js | 99 +++++++++++++++++++ tests/e2e/utils/pages/edit-free-listings.js | 11 --- tests/e2e/utils/pages/settings.js | 29 ++++++ 4 files changed, 128 insertions(+), 12 deletions(-) create mode 100644 tests/e2e/specs/settings/settings.test.js diff --git a/tests/e2e/specs/dashboard/edit-free-listings.test.js b/tests/e2e/specs/dashboard/edit-free-listings.test.js index 2695856e5a..52db0baefa 100644 --- a/tests/e2e/specs/dashboard/edit-free-listings.test.js +++ b/tests/e2e/specs/dashboard/edit-free-listings.test.js @@ -76,7 +76,6 @@ test.describe( 'Edit Free Listings', () => { test( 'Check recommended shipping settings', async () => { await editFreeListingsPage.checkRecommendShippingSettings(); await editFreeListingsPage.fillCountriesShippingTimeInput( '5', '10' ); - await editFreeListingsPage.checkDestinationBasedTaxRates(); const saveChangesButton = await editFreeListingsPage.getSaveChangesButton(); await expect( saveChangesButton ).toBeEnabled(); diff --git a/tests/e2e/specs/settings/settings.test.js b/tests/e2e/specs/settings/settings.test.js new file mode 100644 index 0000000000..a8b5468602 --- /dev/null +++ b/tests/e2e/specs/settings/settings.test.js @@ -0,0 +1,99 @@ +/** + * External dependencies + */ +import { expect, test } from '@playwright/test'; + +/** + * Internal dependencies + */ +import { clearOnboardedMerchant, setOnboardedMerchant } from '../../utils/api'; +import SettingsPage from '../../utils/pages/settings'; + +test.use( { storageState: process.env.ADMINSTATE } ); + +test.describe.configure( { mode: 'serial' } ); + +/** + * @type {import('../../utils/pages/settings.js').default} settingsPage + */ +let settingsPage = null; + +/** + * @type {import('@playwright/test').Page} page + */ +let page = null; + +test.describe( 'Settings', () => { + test.beforeAll( async ( { browser } ) => { + page = await browser.newPage(); + settingsPage = new SettingsPage( page ); + + await setOnboardedMerchant(); + await settingsPage.mockRequests(); + } ); + + test.afterAll( async () => { + await clearOnboardedMerchant(); + await page.close(); + } ); + + test.describe( 'Tax rate setup', () => { + test( 'Should not show the setup when selling in regions unrelated to the US', async () => { + // Mock the country where the store is located as outside of the US. + const once = settingsPage.fulfillTimes( 1 ); + await once.fulfillRequest( + /\/wc-admin\/options\?options=woocommerce_default_country\b/, + { woocommerce_default_country: 'JP' } + ); + await settingsPage.mockTargetAudienceCountries( 'JP' ); + await settingsPage.goto(); + + await expect( + page.getByRole( 'heading', { name: 'Settings' } ) + ).toBeVisible(); + + await expect( + page.locator( '.woocommerce-spinner' ).first() + ).not.toBeVisible(); + + await expect( + page.getByText( 'Tax rate (required for U.S. only)' ) + ).not.toBeVisible(); + } ); + + test( 'Update the setting', async () => { + await settingsPage.mockTargetAudienceCountries(); + await settingsPage.goto(); + + await expect( + page.getByText( 'Tax rate (required for U.S. only)' ) + ).toBeVisible(); + + const saveButton = page.getByRole( 'button', { + name: 'Save tax rate', + } ); + const saveSpinner = saveButton.locator( '.woocommerce-spinner' ); + const option = page.getByRole( 'radio', { checked: false } ); + const optionValue = option.getAttribute( 'value' ); + + // Save button will become clickable after selecting another option. + await expect( saveButton ).toBeDisabled(); + await option.check(); + await expect( saveButton ).toBeEnabled(); + + // Submit the change, and then the save button will go through loading state + // and stay disabled both during and after submission. + await saveButton.click(); + await expect( saveSpinner ).toBeVisible(); + await expect( saveButton ).toBeDisabled(); + await expect( saveSpinner ).not.toBeVisible(); + await expect( saveButton ).toBeDisabled(); + + // Reload to assert the setting has been actually saved. + await page.reload(); + await expect( + page.getByRole( 'radio', { checked: true } ) + ).toHaveAttribute( 'value', optionValue ); + } ); + } ); +} ); diff --git a/tests/e2e/utils/pages/edit-free-listings.js b/tests/e2e/utils/pages/edit-free-listings.js index 0c086cdeea..c05c49e039 100644 --- a/tests/e2e/utils/pages/edit-free-listings.js +++ b/tests/e2e/utils/pages/edit-free-listings.js @@ -59,17 +59,6 @@ export default class EditFreeListingsPage extends MockRequests { await timesLocator.last().fill( max ); } - /** - * Check the destination based tax rates. - * - * @return {Promise} - */ - async checkDestinationBasedTaxRates() { - await this.page - .locator( 'text=My store uses destination-based tax rates.' ) - .check(); - } - /** * Mock the successful saving settings response. * diff --git a/tests/e2e/utils/pages/settings.js b/tests/e2e/utils/pages/settings.js index 4ae90c4385..f66acdec07 100644 --- a/tests/e2e/utils/pages/settings.js +++ b/tests/e2e/utils/pages/settings.js @@ -34,6 +34,35 @@ export default class SettingsPage extends MockRequests { ); } + /** + * Mock all requests related to external accounts such as Merchant Center, Google, etc. + * + * @return {Promise} + */ + async mockRequests() { + await this.mockJetpackConnected(); + await this.mockGoogleConnected(); + await this.mockMCConnected(); + await this.mockAdsAccountConnected(); + await this.mockContactInformation(); + await this.mockSuccessfulSettingsSyncRequest(); + } + + /** + * Mock the target audience request with the given countries. + * + * @param {Array} [countries=['US']] country codes to be mocked. + * @return {Promise} + */ + async mockTargetAudienceCountries( ...countries ) { + await this.fulfillTargetAudience( { + location: 'selected', + countries: countries.length ? countries : [ 'US' ], + locale: 'en_US', + language: 'English', + } ); + } + /** * Get the Grant Access Button. * From 1132178a92fd52c7034a134cd1a08eee8a65c3a4 Mon Sep 17 00:00:00 2001 From: Eason Date: Mon, 6 Jan 2025 14:32:26 +0800 Subject: [PATCH 09/10] Update the description for a test case in `tests/e2e/specs/settings/settings.test.js` MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address: https://github.com/woocommerce/google-listings-and-ads/pull/2749#discussion_r1901736413 Co-authored-by: Miguel Pérez Pellicer <5908855+puntope@users.noreply.github.com> --- tests/e2e/specs/settings/settings.test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/e2e/specs/settings/settings.test.js b/tests/e2e/specs/settings/settings.test.js index a8b5468602..16a1bdcf89 100644 --- a/tests/e2e/specs/settings/settings.test.js +++ b/tests/e2e/specs/settings/settings.test.js @@ -61,7 +61,7 @@ test.describe( 'Settings', () => { ).not.toBeVisible(); } ); - test( 'Update the setting', async () => { + test( 'Should show the setup when selling to the US and can update the setting', async () => { await settingsPage.mockTargetAudienceCountries(); await settingsPage.goto(); From fdae02a8d50106f2589b741f985a9450e113be88 Mon Sep 17 00:00:00 2001 From: Eason Su Date: Mon, 6 Jan 2025 14:43:12 +0800 Subject: [PATCH 10/10] Add the confirmation notification after the tax rate setting is successfully saved and synced. Address: https://github.com/woocommerce/google-listings-and-ads/pull/2749#issuecomment-2569183046 --- .../pages/settings/setup-tax-rate/setup-tax-rate.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js b/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js index 8920bf5a92..fda0022392 100644 --- a/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js +++ b/js/src/pages/settings/setup-tax-rate/setup-tax-rate.js @@ -15,6 +15,7 @@ import TaxRate from './tax-rate'; import useSettings from '~/hooks/useSettings'; import useDisplayTaxRate from './useDisplayTaxRate'; import useTargetAudienceFinalCountryCodes from '~/hooks/useTargetAudienceFinalCountryCodes'; +import useDispatchCoreNotices from '~/hooks/useDispatchCoreNotices'; import { handleApiError } from '~/utils/handleError'; const validTaxRateSet = new Set( [ 'destination', 'manual' ] ); @@ -30,6 +31,7 @@ export default function SetupTaxRate() { const { settings, saveSettings, syncSettings } = useSettings(); const { data: audienceCountries } = useTargetAudienceFinalCountryCodes(); const shouldDisplayTaxRate = useDisplayTaxRate( audienceCountries ); + const { createNotice } = useDispatchCoreNotices(); if ( ! shouldDisplayTaxRate || ! settings?.hasOwnProperty( 'tax_rate' ) ) { if ( shouldDisplayTaxRate === false ) { @@ -80,6 +82,15 @@ export default function SetupTaxRate() { 'google-listings-and-ads' ) ); + } ) + .then( () => { + createNotice( + 'success', + __( + 'Your change to tax rate has been saved and will be synced to your Google Merchant Center.', + 'google-listings-and-ads' + ) + ); } ); };