-
Notifications
You must be signed in to change notification settings - Fork 21
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adjust auto-sync shipping option #2526
Changes from 7 commits
4ef12ec
c73ccf9
7702c35
5b6b038
3c17297
4d90755
53d8f4a
c238ba1
8f3231f
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -14,6 +14,8 @@ import RadioHelperText from '.~/wcdl/radio-helper-text'; | |
import AppDocumentationLink from '.~/components/app-documentation-link'; | ||
import VerticalGapLayout from '.~/components/vertical-gap-layout'; | ||
import FlatShippingRatesInputCards from './flat-shipping-rates-input-cards'; | ||
import useSettings from '.~/components/free-listings/configure-product-listings/useSettings'; | ||
import useMCSetup from '.~/hooks/useMCSetup'; | ||
|
||
/** | ||
* @fires gla_documentation_link_click with `{ context: 'setup-mc-shipping', link_id: 'shipping-read-more', href: 'https://support.google.com/merchants/answer/7050921' }` | ||
|
@@ -22,8 +24,18 @@ import FlatShippingRatesInputCards from './flat-shipping-rates-input-cards'; | |
|
||
const ShippingRateSection = () => { | ||
const { getInputProps, values } = useAdaptiveFormContext(); | ||
const { settings } = useSettings(); | ||
const { hasFinishedResolution, data: mcSetup } = useMCSetup(); | ||
const inputProps = getInputProps( 'shipping_rate' ); | ||
|
||
// Hide the automatic shipping rate option if there are no shipping rates and the merchant is onboarding. | ||
const hideAutomatticShippingRate = | ||
! settings?.shipping_rates_count && | ||
hasFinishedResolution && | ||
mcSetup?.status === 'incomplete' | ||
? true | ||
: false; | ||
|
||
return ( | ||
<Section | ||
title={ __( 'Shipping rates', 'google-listings-and-ads' ) } | ||
|
@@ -51,27 +63,29 @@ const ShippingRateSection = () => { | |
<Section.Card> | ||
<Section.Card.Body> | ||
<VerticalGapLayout size="large"> | ||
<AppRadioContentControl | ||
{ ...inputProps } | ||
label={ createInterpolateElement( | ||
__( | ||
'<strong>Recommended:</strong> Automatically sync my store’s shipping settings to Google.', | ||
'google-listings-and-ads' | ||
), | ||
{ | ||
strong: <strong></strong>, | ||
} | ||
) } | ||
value="automatic" | ||
collapsible | ||
> | ||
<RadioHelperText> | ||
{ __( | ||
'My current settings and any future changes to my store’s shipping rates and classes will be automatically synced to Google Merchant Center.', | ||
'google-listings-and-ads' | ||
{ ! hideAutomatticShippingRate && ( | ||
<AppRadioContentControl | ||
{ ...inputProps } | ||
label={ createInterpolateElement( | ||
__( | ||
'Automatically sync my store’s shipping settings to Google.', | ||
'google-listings-and-ads' | ||
), | ||
{ | ||
strong: <strong></strong>, | ||
} | ||
) } | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since we dont use anymore tag we can omit createInterpolateElement and just use label={
__(
'Automatically sync my store’s shipping settings to Google.',
'google-listings-and-ads'
)
}
|
||
</RadioHelperText> | ||
</AppRadioContentControl> | ||
value="automatic" | ||
collapsible | ||
> | ||
<RadioHelperText> | ||
{ __( | ||
'My current settings and any future changes to my store’s shipping rates and classes will be automatically synced to Google Merchant Center.', | ||
'google-listings-and-ads' | ||
) } | ||
</RadioHelperText> | ||
</AppRadioContentControl> | ||
) } | ||
<AppRadioContentControl | ||
{ ...inputProps } | ||
label={ __( | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,211 @@ | ||
/** | ||
* External dependencies | ||
*/ | ||
import '@testing-library/jest-dom'; | ||
import { render } from '@testing-library/react'; | ||
|
||
/** | ||
* Internal dependencies | ||
*/ | ||
import useSettings from '.~/components/free-listings/configure-product-listings/useSettings'; | ||
import useMCSetup from '.~/hooks/useMCSetup'; | ||
import ShippingRateSection from './shipping-rate-section'; | ||
//import FlatShippingRatesInputCards from './flat-shipping-rates-input-cards'; | ||
|
||
jest.mock( './flat-shipping-rates-input-cards', () => () => <></> ); | ||
|
||
jest.mock( '.~/components/adaptive-form', () => ( { | ||
useAdaptiveFormContext: jest | ||
.fn() | ||
.mockName( 'useAdaptiveFormContext' ) | ||
.mockImplementation( () => { | ||
return { | ||
getInputProps: () => { | ||
return { | ||
checked: true, | ||
className: '', | ||
help: null, | ||
onBlur: () => {}, | ||
onChange: () => {}, | ||
selected: 'flat', | ||
value: 'flat', | ||
}; | ||
}, | ||
values: { | ||
countries: [ 'ES' ], | ||
language: 'English', | ||
locale: 'en_US', | ||
location: 'selected', | ||
offer_free_shipping: false, | ||
shipping_country_rates: [], | ||
shipping_country_times: [], | ||
shipping_rate: 'flat', | ||
shipping_time: 'flat', | ||
tax_rate: null, | ||
}, | ||
}; | ||
} ), | ||
} ) ); | ||
|
||
jest.mock( | ||
'.~/components/free-listings/configure-product-listings/useSettings' | ||
); | ||
jest.mock( '.~/hooks/useMCSetup' ); | ||
|
||
describe( 'ShippingRateSection', () => { | ||
it( 'shouldnt render automatic rates if there are not shipping rates and it is onboarding', () => { | ||
useMCSetup.mockImplementation( () => { | ||
return { | ||
hasFinishedResolution: true, | ||
data: { | ||
status: 'incomplete', | ||
}, | ||
}; | ||
} ); | ||
|
||
useSettings.mockImplementation( () => { | ||
return { | ||
settings: { | ||
shipping_rates_count: 0, | ||
}, | ||
}; | ||
} ); | ||
|
||
const { getByText, queryByRole } = render( <ShippingRateSection /> ); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are simple. I can manually estimate flat shipping rates.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are complex. I will enter my shipping rates and times manually in Google Merchant Center.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
queryByRole( | ||
'Automatically sync my store’s shipping settings to Google.' | ||
) | ||
).not.toBeInTheDocument(); | ||
} ); | ||
|
||
it( 'should render automatic rates if there are shipping rates and it is onboarding', () => { | ||
useMCSetup.mockImplementation( () => { | ||
return { | ||
hasFinishedResolution: true, | ||
data: { | ||
status: 'incomplete', | ||
}, | ||
}; | ||
} ); | ||
|
||
useSettings.mockImplementation( () => { | ||
return { | ||
settings: { | ||
shipping_rates_count: 1, | ||
}, | ||
}; | ||
} ); | ||
|
||
const { getByText } = render( <ShippingRateSection /> ); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are simple. I can manually estimate flat shipping rates.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are complex. I will enter my shipping rates and times manually in Google Merchant Center.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'Automatically sync my store’s shipping settings to Google.' | ||
) | ||
).toBeInTheDocument(); | ||
} ); | ||
|
||
it( 'should render automatic rates if there are not shipping rates and it is not onboarding', () => { | ||
useMCSetup.mockImplementation( () => { | ||
return { | ||
hasFinishedResolution: true, | ||
data: { | ||
status: 'completed', | ||
}, | ||
}; | ||
} ); | ||
|
||
useSettings.mockImplementation( () => { | ||
return { | ||
settings: { | ||
shipping_rates_count: 0, | ||
}, | ||
}; | ||
} ); | ||
|
||
const { getByText } = render( <ShippingRateSection /> ); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are simple. I can manually estimate flat shipping rates.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are complex. I will enter my shipping rates and times manually in Google Merchant Center.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'Automatically sync my store’s shipping settings to Google.' | ||
) | ||
).toBeInTheDocument(); | ||
} ); | ||
|
||
it( 'should render automatic rates if there are shipping rates and it is not onboarding', () => { | ||
useMCSetup.mockImplementation( () => { | ||
return { | ||
hasFinishedResolution: true, | ||
data: { | ||
status: 'completed', | ||
}, | ||
}; | ||
} ); | ||
|
||
useSettings.mockImplementation( () => { | ||
return { | ||
settings: { | ||
shipping_rates_count: 1, | ||
}, | ||
}; | ||
} ); | ||
|
||
const { getByText } = render( <ShippingRateSection /> ); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are simple. I can manually estimate flat shipping rates.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'My shipping settings are complex. I will enter my shipping rates and times manually in Google Merchant Center.' | ||
) | ||
).toBeInTheDocument(); | ||
|
||
expect( | ||
getByText( | ||
'Automatically sync my store’s shipping settings to Google.' | ||
) | ||
).toBeInTheDocument(); | ||
} ); | ||
} ); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -6,6 +6,8 @@ | |
use Automattic\WooCommerce\GoogleListingsAndAds\API\Site\Controllers\BaseOptionsController; | ||
use Automattic\WooCommerce\GoogleListingsAndAds\API\TransportMethods; | ||
use Automattic\WooCommerce\GoogleListingsAndAds\Options\OptionsInterface; | ||
use Automattic\WooCommerce\GoogleListingsAndAds\Shipping\ShippingZone; | ||
use Automattic\WooCommerce\GoogleListingsAndAds\Proxies\RESTServer; | ||
use WP_REST_Request as Request; | ||
|
||
defined( 'ABSPATH' ) || exit; | ||
|
@@ -17,6 +19,22 @@ | |
*/ | ||
class SettingsController extends BaseOptionsController { | ||
|
||
/** | ||
* @var ShippingZone | ||
*/ | ||
protected $shipping_zone; | ||
|
||
/** | ||
* SettingsController constructor. | ||
* | ||
* @param RESTServer $server | ||
* @param ShippingZone $shipping_zone | ||
*/ | ||
public function __construct( RESTServer $server, ShippingZone $shipping_zone ) { | ||
parent::__construct( $server ); | ||
$this->shipping_zone = $shipping_zone; | ||
} | ||
|
||
/** | ||
* Register rest routes with WordPress. | ||
*/ | ||
|
@@ -46,9 +64,10 @@ public function register_routes(): void { | |
*/ | ||
protected function get_settings_endpoint_read_callback(): callable { | ||
return function () { | ||
$data = $this->options->get( OptionsInterface::MERCHANT_CENTER, [] ); | ||
$schema = $this->get_schema_properties(); | ||
$items = []; | ||
$data = $this->options->get( OptionsInterface::MERCHANT_CENTER, [] ); | ||
$data['shipping_rates_count'] = $this->shipping_zone->get_shipping_rates_count(); | ||
$schema = $this->get_schema_properties(); | ||
$items = []; | ||
foreach ( $schema as $key => $property ) { | ||
$items[ $key ] = $data[ $key ] ?? $property['default'] ?? null; | ||
} | ||
|
@@ -71,6 +90,9 @@ protected function get_settings_endpoint_edit_callback(): callable { | |
} | ||
|
||
foreach ( $schema as $key => $property ) { | ||
if ( ! in_array( 'edit', $property['context'] ?? [], true ) ) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added this so it skips only view properties; otherwise, it would save them as options, like the |
||
continue; | ||
} | ||
$options[ $key ] = $request->get_param( $key ) ?? $options[ $key ] ?? $property['default'] ?? null; | ||
} | ||
|
||
|
@@ -178,6 +200,16 @@ protected function get_schema_properties(): array { | |
'validate_callback' => 'rest_validate_request_arg', | ||
'default' => false, | ||
], | ||
'shipping_rates_count' => [ | ||
'type' => 'number', | ||
'description' => __( | ||
'The number of shipping rates in WC ready to be used in the Merchant Center.', | ||
'google-listings-and-ads' | ||
), | ||
'context' => [ 'view' ], | ||
'validate_callback' => 'rest_validate_request_arg', | ||
'default' => 0, | ||
], | ||
]; | ||
} | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This can be simplified as just
mcSetup?.status === 'incomplete'