From 718fe762fae80f5cac9c8732bc5bcc0512522bd2 Mon Sep 17 00:00:00 2001 From: Gan Eng Chin Date: Tue, 19 Mar 2024 01:56:08 +0800 Subject: [PATCH] Add formatting for Cost and Sales numbers in Campaigns card in Marketing page (#44917) * Return formatted cost and sales price for MarketingCampaigns. * Display formatted cost and sales number in Campaigns card. * Use price formatting. * Format decimal places based on currency. * Add changelog. * Fix type issue is useCampaigns.test.ts. * Use wp_strip_all_tags to respect currency symbol positioning settings. Without wp_strip_all_tags, the result contains element, and it causes browser to show the currency symbol in unexpected unwanted position. * Fix PHP linting issue. * Fix tests in useCampaigns.test.ts. * Use html_entity_decode to remove dangerouslySetInnerHTML usage. * Remove unneeded code formatting in Campaigns.tsx. * Add explanation comment for `get_formatted_price`. * Fix PHP lint error. * Use map instead of filter to get price format. * Add code comment. * Get currency info based on user locale or default locale. * Use locales in locale-info.php instead of currency-info.php. Co-authored-by: Bartosz Budzanowski * Code formatting and fix code comment. * Fix lint errors. --------- Co-authored-by: Bartosz Budzanowski --- .../marketing/data-multichannel/types.ts | 2 + .../marketing/hooks/test/useCampaigns.test.ts | 12 ++-- .../client/marketing/hooks/useCampaigns.ts | 9 +-- .../update-marketing-number-formatting | 4 ++ .../src/Admin/API/MarketingCampaigns.php | 63 +++++++++++++++++-- 5 files changed, 73 insertions(+), 17 deletions(-) create mode 100644 plugins/woocommerce/changelog/update-marketing-number-formatting diff --git a/plugins/woocommerce-admin/client/marketing/data-multichannel/types.ts b/plugins/woocommerce-admin/client/marketing/data-multichannel/types.ts index 9f2248e681d..0f3c97a4ab4 100644 --- a/plugins/woocommerce-admin/client/marketing/data-multichannel/types.ts +++ b/plugins/woocommerce-admin/client/marketing/data-multichannel/types.ts @@ -58,10 +58,12 @@ export type Campaign = { cost: { value: string; currency: string; + formatted: string; } | null; sales: { value: string; currency: string; + formatted: string; } | null; }; diff --git a/plugins/woocommerce-admin/client/marketing/hooks/test/useCampaigns.test.ts b/plugins/woocommerce-admin/client/marketing/hooks/test/useCampaigns.test.ts index 244cf1864d6..db17a0de192 100644 --- a/plugins/woocommerce-admin/client/marketing/hooks/test/useCampaigns.test.ts +++ b/plugins/woocommerce-admin/client/marketing/hooks/test/useCampaigns.test.ts @@ -37,18 +37,22 @@ jest.mock( '@wordpress/api-fetch', () => const campaigns: Array< APICampaign > = Array.from( { length } ).map( ( _, index ) => { const id = `${ page }_${ index + 1 }`; + const value = ( ( page * perPage + index ) * 0.25 ).toString(); + return { id, channel: 'extension-foo', title: `Campaign ${ id }`, manage_url: `https://test/extension-foo?path=setup&id=${ id }`, cost: { - value: ( ( page * perPage + index ) * 0.25 ).toString(), + value, currency: 'USD', + formatted: `$${ value }`, }, sales: { - value: ( ( page * perPage + index ) * 0.25 ).toString(), + value, currency: 'USD', + formatted: `$${ value }`, }, }; } @@ -119,8 +123,8 @@ describe( 'useCampaigns', () => { id: 'extension-foo|1_1', title: 'Campaign 1_1', description: '', - cost: 'USD 1.25', - sales: 'USD 1.25', + cost: '$1.25', + sales: '$1.25', manageUrl: 'https://test/extension-foo?path=setup&id=1_1', icon: 'https://test/foo.png', channelName: 'Extension Foo', diff --git a/plugins/woocommerce-admin/client/marketing/hooks/useCampaigns.ts b/plugins/woocommerce-admin/client/marketing/hooks/useCampaigns.ts index b2cd950232d..de76aac7812 100644 --- a/plugins/woocommerce-admin/client/marketing/hooks/useCampaigns.ts +++ b/plugins/woocommerce-admin/client/marketing/hooks/useCampaigns.ts @@ -46,13 +46,8 @@ export const useCampaigns = ( page = 1, perPage = 5 ): UseCampaignsType => { ( el ) => el.slug === campaign.channel ); - const cost = campaign.cost - ? `${ campaign.cost.currency } ${ campaign.cost.value }` - : '-'; - - const sales = campaign.sales - ? `${ campaign.sales.currency } ${ campaign.sales.value }` - : '-'; + const cost = campaign.cost ? campaign.cost.formatted : '-'; + const sales = campaign.sales ? campaign.sales.formatted : '-'; return { id: `${ campaign.channel }|${ campaign.id }`, diff --git a/plugins/woocommerce/changelog/update-marketing-number-formatting b/plugins/woocommerce/changelog/update-marketing-number-formatting new file mode 100644 index 00000000000..697c0a4caa8 --- /dev/null +++ b/plugins/woocommerce/changelog/update-marketing-number-formatting @@ -0,0 +1,4 @@ +Significance: minor +Type: update + +Add formatting for Cost and Sales numbers in Campaigns card in Marketing page. diff --git a/plugins/woocommerce/src/Admin/API/MarketingCampaigns.php b/plugins/woocommerce/src/Admin/API/MarketingCampaigns.php index 7e658ceda02..7f691357498 100644 --- a/plugins/woocommerce/src/Admin/API/MarketingCampaigns.php +++ b/plugins/woocommerce/src/Admin/API/MarketingCampaigns.php @@ -132,6 +132,57 @@ class MarketingCampaigns extends WC_REST_Controller { return $response; } + /** + * Get formatted price based on Price type. + * + * This uses plugins/woocommerce/i18n/currency-info.php and plugins/woocommerce/i18n/locale-info.php to get option object based on $price->currency. + * + * Example: + * + * - When $price->currency is 'USD' and $price->value is '1000', it should return '$1000.00'. + * - When $price->currency is 'JPY' and $price->value is '1000', it should return '¥1,000'. + * - When $price->currency is 'AED' and $price->value is '1000', it should return '5.000,00 د.إ'. + * + * @param Price $price Price object. + * @return String formatted price. + */ + private function get_formatted_price( $price ) { + // Get $num_decimals to be passed to wc_price. + $locale_info_all = include WC()->plugin_path() . '/i18n/locale-info.php'; + $locale_index = array_search( $price->get_currency(), array_column( $locale_info_all, 'currency_code' ), true ); + $locale = array_values( $locale_info_all )[ $locale_index ]; + $num_decimals = $locale['num_decimals']; + + // Get $currency_info based on user locale or default locale. + $currency_locales = $locale['locales']; + $user_locale = get_user_locale(); + $currency_info = $currency_locales[ $user_locale ] ?? $currency_locales['default']; + + // Get $price_format to be passed to wc_price. + $currency_pos = $currency_info['currency_pos']; + $currency_formats = array( + 'left' => '%1$s%2$s', + 'right' => '%2$s%1$s', + 'left_space' => '%1$s %2$s', + 'right_space' => '%2$s %1$s', + ); + $price_format = $currency_formats[ $currency_pos ] ?? $currency_formats['left']; + + $price_value = wc_format_decimal( $price->get_value() ); + $price_formatted = wc_price( + $price_value, + array( + 'currency' => $price->get_currency(), + 'decimal_separator' => $currency_info['decimal_sep'], + 'thousand_separator' => $currency_info['thousand_sep'], + 'decimals' => $num_decimals, + 'price_format' => $price_format, + ) + ); + + return html_entity_decode( wp_strip_all_tags( $price_formatted ) ); + } + /** * Prepares the item for the REST response. * @@ -150,15 +201,17 @@ class MarketingCampaigns extends WC_REST_Controller { if ( $item->get_cost() instanceof Price ) { $data['cost'] = array( - 'value' => wc_format_decimal( $item->get_cost()->get_value() ), - 'currency' => $item->get_cost()->get_currency(), + 'value' => wc_format_decimal( $item->get_cost()->get_value() ), + 'currency' => $item->get_cost()->get_currency(), + 'formatted' => $this->get_formatted_price( $item->get_cost() ), ); } if ( $item->get_sales() instanceof Price ) { $data['sales'] = array( - 'value' => wc_format_decimal( $item->get_sales()->get_value() ), - 'currency' => $item->get_sales()->get_currency(), + 'value' => wc_format_decimal( $item->get_sales()->get_value() ), + 'currency' => $item->get_sales()->get_currency(), + 'formatted' => $this->get_formatted_price( $item->get_sales() ), ); } @@ -257,6 +310,4 @@ class MarketingCampaigns extends WC_REST_Controller { return $params; } - - }