From c061a0874a4af2d3dbe25a88c6cca44015d02a5a Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 6 Nov 2024 11:35:33 +1000 Subject: [PATCH] Use 'Withdrawal' and 'Deducted' labels when referring to withdrawal deposits, to more accurately communicate the type of transaction that has occurred (#9500) --- .../fix-7802-deposit-withdrawal-text-changes | 4 + .../components/deposit-status-chip/index.tsx | 20 +- .../deposit-status-chip/test/index.test.tsx | 18 +- .../recent-deposits-list.tsx | 3 +- .../test/__snapshots__/index.tsx.snap | 50 ++++- .../deposits-overview/test/index.tsx | 9 + client/deposits/details/index.tsx | 78 ++++++-- .../details/test/__snapshots__/index.tsx.snap | 76 +++++++- client/deposits/details/test/index.tsx | 26 ++- client/deposits/filters/config.js | 17 +- .../filters/test/__snapshots__/index.js.snap | 2 +- client/deposits/list/index.tsx | 12 +- .../list/test/__snapshots__/index.tsx.snap | 178 +++++++++++++++++- client/deposits/list/test/index.tsx | 13 +- client/deposits/strings.ts | 13 +- client/transactions/list/index.tsx | 8 +- .../list/test/__snapshots__/index.tsx.snap | 4 +- client/transactions/list/test/index.tsx | 3 +- 18 files changed, 474 insertions(+), 60 deletions(-) create mode 100644 changelog/fix-7802-deposit-withdrawal-text-changes diff --git a/changelog/fix-7802-deposit-withdrawal-text-changes b/changelog/fix-7802-deposit-withdrawal-text-changes new file mode 100644 index 00000000000..cf32df2a4f7 --- /dev/null +++ b/changelog/fix-7802-deposit-withdrawal-text-changes @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Use 'Withdrawal' and 'Deducted' labels when referring to withdrawal deposits, to more accurately communicate the type of transaction that has occurred diff --git a/client/components/deposit-status-chip/index.tsx b/client/components/deposit-status-chip/index.tsx index 9d70fdfbe7a..8ca6fe6bad6 100644 --- a/client/components/deposit-status-chip/index.tsx +++ b/client/components/deposit-status-chip/index.tsx @@ -6,9 +6,9 @@ import * as React from 'react'; /** * Internal dependencies */ -import { displayStatus } from 'deposits/strings'; import Chip, { ChipType } from 'components/chip'; -import type { DepositStatus } from 'wcpay/types/deposits'; +import type { CachedDeposit, DepositStatus } from 'wcpay/types/deposits'; +import { depositStatusLabels } from 'wcpay/deposits/strings'; /** * Maps a DepositStatus to a ChipType. @@ -23,13 +23,17 @@ const mappings: Record< DepositStatus, ChipType > = { /** * Renders a deposits status chip. - * - * @return {JSX.Element} Deposit status chip. */ const DepositStatusChip: React.FC< { - status: DepositStatus; -} > = ( { status } ): JSX.Element => ( - -); + deposit: Pick< CachedDeposit, 'status' | 'type' >; +} > = ( { deposit } ) => { + let message = depositStatusLabels[ deposit.status ]; + + // Withdrawals are displayed as 'Deducted' instead of 'Paid'. + if ( deposit.type === 'withdrawal' && deposit.status === 'paid' ) { + message = depositStatusLabels.deducted; + } + return ; +}; export default DepositStatusChip; diff --git a/client/components/deposit-status-chip/test/index.test.tsx b/client/components/deposit-status-chip/test/index.test.tsx index 54abc52546c..e96bd9961f5 100644 --- a/client/components/deposit-status-chip/test/index.test.tsx +++ b/client/components/deposit-status-chip/test/index.test.tsx @@ -11,18 +11,28 @@ import DepositStatusChip from '..'; describe( 'Deposits status chip renders', () => { test( 'Renders "Pending" status chip.', () => { - const { getByText } = render( ); + const { getByText } = render( + + ); expect( getByText( 'Pending' ) ).toBeTruthy(); } ); test( 'Renders "Paid" status chip.', () => { - const { getByText } = render( ); - expect( getByText( 'Paid' ) ).toBeTruthy(); + const { getByText } = render( + + ); + expect( getByText( 'Completed (paid)' ) ).toBeTruthy(); } ); test( 'Renders "In transit" status chip.', () => { const { getByText } = render( - + ); expect( getByText( 'In transit' ) ).toBeTruthy(); } ); diff --git a/client/components/deposits-overview/recent-deposits-list.tsx b/client/components/deposits-overview/recent-deposits-list.tsx index 88555793d0d..6bb50a0a6c2 100644 --- a/client/components/deposits-overview/recent-deposits-list.tsx +++ b/client/components/deposits-overview/recent-deposits-list.tsx @@ -11,7 +11,6 @@ import { } from '@wordpress/components'; import { calendar } from '@wordpress/icons'; import { Link } from '@woocommerce/components'; -import { Fragment } from '@wordpress/element'; import { __ } from '@wordpress/i18n'; /** @@ -51,7 +50,7 @@ const RecentDepositsList: React.FC< RecentDepositsProps > = ( { - + { formatCurrency( deposit.amount, deposit.currency ) } diff --git a/client/components/deposits-overview/test/__snapshots__/index.tsx.snap b/client/components/deposits-overview/test/__snapshots__/index.tsx.snap index e38209eb60e..a29a1d9064f 100644 --- a/client/components/deposits-overview/test/__snapshots__/index.tsx.snap +++ b/client/components/deposits-overview/test/__snapshots__/index.tsx.snap @@ -275,7 +275,7 @@ exports[`Deposits Overview information Component Renders 1`] = ` - Paid + Completed (paid)
+
+
+ + + January 4, 2020 + +
+
+ + Completed (deducted) + +
+
+ $40.00 +
+
= ( { status } ) => ( - -); +const DepositStatusIndicator: React.FC< { + deposit: Pick< CachedDeposit, 'status' | 'type' >; +} > = ( { deposit } ) => { + let displayStatusMap = depositStatusLabels; + + // Withdrawals are displayed as 'Deducted' instead of 'Paid' when the status is 'paid'. + if ( deposit.type === 'withdrawal' ) { + displayStatusMap = { + ...displayStatusMap, + paid: displayStatusMap.deducted, + }; + } + + return ( + + ); +}; interface SummaryItemProps { label: string; @@ -102,9 +119,15 @@ export const DepositOverview: React.FC< DepositOverviewProps > = ( { ); } - const depositDateLabel = deposit.automatic - ? __( 'Deposit date', 'woocommerce-payments' ) - : __( 'Instant deposit date', 'woocommerce-payments' ); + const isWithdrawal = deposit.type === 'withdrawal'; + + let depositDateLabel = __( 'Deposit date', 'woocommerce-payments' ); + if ( ! deposit.automatic ) { + depositDateLabel = __( 'Instant deposit date', 'woocommerce-payments' ); + } + if ( isWithdrawal ) { + depositDateLabel = __( 'Withdrawal date', 'woocommerce-payments' ); + } const depositDateItem = ( = ( { true // TODO Change call to gmdateI18n and remove this deprecated param once WP 5.4 support ends. ) } - value={ } + value={ } detail={ deposit.bankAccount } /> ); @@ -138,16 +161,30 @@ export const DepositOverview: React.FC< DepositOverviewProps > = ( { ) : ( { () => [ depositDateItem, = ( { />, - Paid + Completed (paid)
@@ -72,6 +72,78 @@ exports[`Deposit overview renders automatic deposit correctly 1`] = ` `; +exports[`Deposit overview renders automatic withdrawal correctly 1`] = ` +
+
+
+
+
    +
  • +
    +
    + Withdrawal date: Jan 2, 2020 +
    +
    +
    +
    + + Completed (deducted) +
    +
    +
    +
    + MOCK BANK •••• 1234 (USD) +
    +
    +
    • +
  • + -$20.00 +
    • +
+
+ +
+`; + exports[`Deposit overview renders instant deposit correctly 1`] = `
- Paid + Completed (paid)
diff --git a/client/deposits/details/test/index.tsx b/client/deposits/details/test/index.tsx index f6fd4d98116..5d4452e9998 100644 --- a/client/deposits/details/test/index.tsx +++ b/client/deposits/details/test/index.tsx @@ -25,6 +25,19 @@ const mockDeposit = { currency: 'USD', } as CachedDeposit; +const mockWithdrawal = { + id: 'po_mock', + date: '2020-01-02 17:46:02', + type: 'withdrawal', + amount: -2000, + status: 'paid', + bankAccount: 'MOCK BANK •••• 1234 (USD)', + automatic: true, + fee: 30, + fee_percentage: 1.5, + currency: 'USD', +} as CachedDeposit; + declare const global: { wcpaySettings: { zeroDecimalCurrencies: string[]; @@ -58,9 +71,20 @@ describe( 'Deposit overview', () => { } ); test( 'renders automatic deposit correctly', () => { - const { container: overview } = render( + const { container: overview, getByText } = render( ); + getByText( /Deposit date:/ ); + getByText( 'Completed (paid)' ); + expect( overview ).toMatchSnapshot(); + } ); + + test( 'renders automatic withdrawal correctly', () => { + const { container: overview, getByText } = render( + + ); + getByText( /Withdrawal date:/ ); + getByText( 'Completed (deducted)' ); expect( overview ).toMatchSnapshot(); } ); diff --git a/client/deposits/filters/config.js b/client/deposits/filters/config.js index c63435bcaca..90d850c7f2f 100644 --- a/client/deposits/filters/config.js +++ b/client/deposits/filters/config.js @@ -7,11 +7,20 @@ import { getSetting } from '@woocommerce/settings'; /** * Internal dependencies */ -import { displayStatus } from 'deposits/strings'; +import { depositStatusLabels } from 'deposits/strings'; -const depositStatusOptions = Object.entries( - displayStatus -).map( ( [ status, label ] ) => ( { label, value: status } ) ); +const depositStatusOptions = Object.entries( depositStatusLabels ) + // Ignore the 'deducted' status, which is only a display status and not to be used in filters. + .filter( ( [ status ] ) => status !== 'deducted' ) + .map( ( [ status, label ] ) => { + if ( status === 'paid' ) { + return { + label: __( 'Completed', 'woocommerce-payments' ), + value: 'paid', + }; + } + return { label, value: status }; + } ); export const filters = [ { diff --git a/client/deposits/filters/test/__snapshots__/index.js.snap b/client/deposits/filters/test/__snapshots__/index.js.snap index 5f02b248a6c..962eccde043 100644 --- a/client/deposits/filters/test/__snapshots__/index.js.snap +++ b/client/deposits/filters/test/__snapshots__/index.js.snap @@ -5,7 +5,7 @@ HTMLOptionsCollection [ ,
@@ -933,7 +1020,7 @@ exports[`Deposits list renders correctly with multiple currencies 1`] = ` - Paid + Completed (paid) @@ -1037,6 +1124,93 @@ exports[`Deposits list renders correctly with multiple currencies 1`] = ` + + + + + + + + + + + + + Jan 4, 2020 + + + + + Withdrawal + + + + + $40.00 + + + + + + Completed (deducted) + + + + + + MOCK BANK •••• 1234 (USD) + + + diff --git a/client/deposits/list/test/index.tsx b/client/deposits/list/test/index.tsx index 7c6ea8f9d17..322ca965f23 100644 --- a/client/deposits/list/test/index.tsx +++ b/client/deposits/list/test/index.tsx @@ -63,6 +63,15 @@ const mockDeposits = [ bankAccount: 'MOCK BANK •••• 1234 (USD)', currency: 'USD', } as CachedDeposit, + { + id: 'po_mock3', + date: '2020-01-04 17:46:02', + type: 'withdrawal', + amount: 4000, + status: 'paid', + bankAccount: 'MOCK BANK •••• 1234 (USD)', + currency: 'USD', + } as CachedDeposit, ]; declare const global: { @@ -317,7 +326,9 @@ describe( 'Deposits list', () => { csvFirstDeposit[ 3 ] ) ).not.toBe( -1 ); // amount - expect( csvFirstDeposit[ 4 ] ).toBe( displayFirstDeposit[ 3 ] ); // status + expect( csvFirstDeposit[ 4 ] ).toBe( + `"${ displayFirstDeposit[ 3 ] }"` + ); // status expect( csvFirstDeposit[ 5 ] ).toBe( `"${ displayFirstDeposit[ 4 ] }"` ); // bank account diff --git a/client/deposits/strings.ts b/client/deposits/strings.ts index f4cbfbfb3a6..03a8c55198d 100644 --- a/client/deposits/strings.ts +++ b/client/deposits/strings.ts @@ -16,8 +16,17 @@ export const displayType = { withdrawal: __( 'Withdrawal', 'woocommerce-payments' ), }; -export const displayStatus: Record< DepositStatus, string > = { - paid: __( 'Paid', 'woocommerce-payments' ), +/** + * Labels to display for each deposit status. + * + * 'deducted' represents a deposit of the type 'withdrawal' and status 'paid'. + */ +export const depositStatusLabels: Record< + DepositStatus | 'deducted', + string +> = { + paid: __( 'Completed (paid)', 'woocommerce-payments' ), + deducted: __( 'Completed (deducted)', 'woocommerce-payments' ), pending: __( 'Pending', 'woocommerce-payments' ), in_transit: __( 'In transit', 'woocommerce-payments' ), canceled: __( 'Canceled', 'woocommerce-payments' ), diff --git a/client/transactions/list/index.tsx b/client/transactions/list/index.tsx index e123d0319f5..7907fd83007 100644 --- a/client/transactions/list/index.tsx +++ b/client/transactions/list/index.tsx @@ -42,7 +42,7 @@ import RiskLevel, { calculateRiskMapping } from 'components/risk-level'; import ClickableCell from 'components/clickable-cell'; import { getDetailsURL } from 'components/details-link'; import { displayType } from 'transactions/strings'; -import { displayStatus as displayDepositStatus } from 'deposits/strings'; +import { depositStatusLabels } from 'deposits/strings'; import { formatStringValue, isExportModalDismissed, @@ -454,7 +454,7 @@ export const TransactionsList = ( ); const depositStatus = txn.deposit_status - ? displayDepositStatus[ txn.deposit_status ] + ? depositStatusLabels[ txn.deposit_status ] : ''; // Map transaction into table row. @@ -597,9 +597,7 @@ export const TransactionsList = ( 'woocommerce-payments' ); - const title = props.depositId - ? __( 'Deposit transactions', 'woocommerce-payments' ) - : __( 'Transactions', 'woocommerce-payments' ); + const title = __( 'Transactions', 'woocommerce-payments' ); const downloadable = !! rows.length; diff --git a/client/transactions/list/test/__snapshots__/index.tsx.snap b/client/transactions/list/test/__snapshots__/index.tsx.snap index e638122bba2..13aebe63c2a 100644 --- a/client/transactions/list/test/__snapshots__/index.tsx.snap +++ b/client/transactions/list/test/__snapshots__/index.tsx.snap @@ -2069,7 +2069,7 @@ exports[`Transactions list renders correctly when filtered by deposit 1`] = ` data-wp-c16t="true" data-wp-component="Text" > - Deposit transactions + Transactions
- Deposit transactions + Transactions diff --git a/client/transactions/list/test/index.tsx b/client/transactions/list/test/index.tsx index 8d97397ce01..19c4652a7de 100644 --- a/client/transactions/list/test/index.tsx +++ b/client/transactions/list/test/index.tsx @@ -267,10 +267,11 @@ describe( 'Transactions list', () => { isLoading: false, } ); - const { container } = render( + const { container, getByRole } = render( ); expect( container ).toMatchSnapshot(); + getByRole( 'heading', { name: 'Transactions' } ); expect( mockUseTransactions.mock.calls[ 0 ][ 1 ] ).toBe( 'po_mock' ); } );