From c1a4bf933e0054f1c15c7d0459f38585dac5b839 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:33:51 +1000 Subject: [PATCH 1/8] Add dispute isRefundable util function --- client/disputes/utils.ts | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/client/disputes/utils.ts b/client/disputes/utils.ts index 6e5acf2f437..d4cb2fc52d1 100644 --- a/client/disputes/utils.ts +++ b/client/disputes/utils.ts @@ -72,6 +72,11 @@ export const isInquiry = ( dispute: Pick< Dispute, 'status' > ): boolean => { return dispute.status.startsWith( 'warning' ); }; +export const isRefundable = ( status: DisputeStatus ): boolean => { + // Refundable dispute statuses are one of `warning_needs_response`, `warning_under_review`, `warning_closed` or `won`. + return isInquiry( { status } ) || 'won' === status; +}; + /** * Returns the dispute fee balance transaction for a dispute if it exists * and the deduction has not been reversed. From 2d61468068704db4517b854271d58ba95e7de0f1 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:38:54 +1000 Subject: [PATCH 2/8] Add changelog entry --- changelog/fix-7960-transaction-refund-eligible-disputes-only | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/fix-7960-transaction-refund-eligible-disputes-only diff --git a/changelog/fix-7960-transaction-refund-eligible-disputes-only b/changelog/fix-7960-transaction-refund-eligible-disputes-only new file mode 100644 index 00000000000..e35c11107f7 --- /dev/null +++ b/changelog/fix-7960-transaction-refund-eligible-disputes-only @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Hide the transaction details refund menu for ineligble disputed transactions From b864c62b4745919980f364bc459856e41096d7ff Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 3 Jan 2024 11:50:36 +1000 Subject: [PATCH 3/8] Only show transaction refund menu if dispute is refundable --- client/payment-details/summary/index.tsx | 11 ++++++++++- 1 file changed, 10 insertions(+), 1 deletion(-) diff --git a/client/payment-details/summary/index.tsx b/client/payment-details/summary/index.tsx index 607bacb5f0a..7845632e00a 100644 --- a/client/payment-details/summary/index.tsx +++ b/client/payment-details/summary/index.tsx @@ -48,6 +48,7 @@ import DisputeStatusChip from 'components/dispute-status-chip'; import { getDisputeFeeFormatted, isAwaitingResponse, + isRefundable, } from 'wcpay/disputes/utils'; import { useAuthorization } from 'wcpay/data'; import CaptureAuthorizationButton from 'wcpay/components/capture-authorization-button'; @@ -208,6 +209,14 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( { const disputeFee = charge.dispute && getDisputeFeeFormatted( charge.dispute ); + // If this transaction is disputed, check if it is refundable. If not, we should hide the refund menu. + const isDisputeRefundable = charge.dispute + ? isRefundable( charge.dispute.status ) + : true; + + const showRefundMenu = + charge.captured && ! charge.refunded && isDisputeRefundable; + // Use the balance_transaction fee if available. If not (e.g. authorized but not captured), use the application_fee_amount. const transactionFee = charge.balance_transaction ? { @@ -484,7 +493,7 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( {
- { ! charge?.refunded && charge?.captured && ( + { showRefundMenu && ( Date: Wed, 3 Jan 2024 14:25:09 +1000 Subject: [PATCH 4/8] Add tests for dispute refund menu cases --- .../summary/test/index.test.tsx | 48 +++++++++++++++++++ 1 file changed, 48 insertions(+) diff --git a/client/payment-details/summary/test/index.test.tsx b/client/payment-details/summary/test/index.test.tsx index 522c613d5ff..8f41921aae5 100755 --- a/client/payment-details/summary/test/index.test.tsx +++ b/client/payment-details/summary/test/index.test.tsx @@ -427,6 +427,13 @@ describe( 'PaymentDetailsSummary', () => { screen.getByRole( 'button', { name: /Accept dispute/, } ); + + // Refund menu is not rendered + expect( + screen.queryByRole( 'button', { + name: /Translation actions/i, + } ) + ).toBeNull(); } ); test( 'renders the information of a disputed charge when the store/charge currency differ', () => { @@ -684,6 +691,11 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is rendered + screen.getByRole( 'button', { + name: /Translation actions/i, + } ); } ); test( 'correctly renders dispute details for "under_review" disputes', () => { @@ -712,6 +724,13 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is not rendered + expect( + screen.queryByRole( 'button', { + name: /Translation actions/i, + } ) + ).toBeNull(); } ); test( 'correctly renders dispute details for "accepted" disputes', () => { @@ -744,6 +763,13 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is not rendered + expect( + screen.queryByRole( 'button', { + name: /Translation actions/i, + } ) + ).toBeNull(); } ); test( 'correctly renders dispute details for "lost" disputes', () => { @@ -777,6 +803,13 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is not rendered + expect( + screen.queryByRole( 'button', { + name: /Translation actions/i, + } ) + ).toBeNull(); } ); test( 'correctly renders dispute details for "warning_needs_response" inquiry disputes', () => { @@ -807,6 +840,11 @@ describe( 'PaymentDetailsSummary', () => { screen.getByRole( 'button', { name: /Issue refund/i, } ); + + // Refund menu is rendered + screen.getByRole( 'button', { + name: /Translation actions/i, + } ); } ); test( 'correctly renders dispute details for "warning_under_review" inquiry disputes', () => { @@ -834,6 +872,11 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is rendered + screen.getByRole( 'button', { + name: /Translation actions/i, + } ); } ); test( 'correctly renders dispute details for "warning_closed" inquiry disputes', () => { @@ -862,6 +905,11 @@ describe( 'PaymentDetailsSummary', () => { name: /Accept/i, } ) ).toBeNull(); + + // Refund menu is rendered + screen.getByRole( 'button', { + name: /Translation actions/i, + } ); } ); describe( 'order missing notice', () => { From 006e8e3b6a4337f6facc753cbd679abfbf90cf29 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 3 Jan 2024 14:47:03 +1000 Subject: [PATCH 5/8] Use isRefundable dispute util function for disputed order notice --- client/components/disputed-order-notice/index.js | 10 +++------- 1 file changed, 3 insertions(+), 7 deletions(-) diff --git a/client/components/disputed-order-notice/index.js b/client/components/disputed-order-notice/index.js index 13ea7af71e5..dce0fb2b77f 100644 --- a/client/components/disputed-order-notice/index.js +++ b/client/components/disputed-order-notice/index.js @@ -14,6 +14,7 @@ import { getDetailsURL } from 'wcpay/components/details-link'; import { isAwaitingResponse, isInquiry, + isRefundable, isUnderReview, } from 'wcpay/disputes/utils'; import { useCharge } from 'wcpay/data'; @@ -30,10 +31,7 @@ const DisputedOrderNoticeHandler = ( { chargeId, onDisableOrderRefund } ) => { if ( ! charge?.dispute ) { return; } - // Refunds are only allowed if the dispute is an inquiry or if it's won. - const isRefundable = - isInquiry( dispute ) || [ 'won' ].includes( dispute.status ); - if ( ! isRefundable ) { + if ( ! isRefundable( dispute.status ) ) { onDisableOrderRefund( dispute.status ); } }, [ charge, onDisableOrderRefund ] ); @@ -42,8 +40,6 @@ const DisputedOrderNoticeHandler = ( { chargeId, onDisableOrderRefund } ) => { if ( ! charge?.dispute ) { return null; } - const isRefundable = - isInquiry( dispute ) || [ 'won' ].includes( dispute.status ); // Special case the dispute "under review" notice which is much simpler. // (And return early.) @@ -66,7 +62,7 @@ const DisputedOrderNoticeHandler = ( { chargeId, onDisableOrderRefund } ) => { // This may be dead code. Leaving in for now as this is consistent with // the logic before this PR. // https://github.com/Automattic/woocommerce-payments/pull/7557 - if ( dispute.status === 'lost' && ! isRefundable ) { + if ( dispute.status === 'lost' ) { return ( Date: Wed, 3 Jan 2024 15:05:05 +1000 Subject: [PATCH 6/8] Add placeholders changelog entry --- changelog/dev-refactor-order-dispute-refundable-util | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelog/dev-refactor-order-dispute-refundable-util diff --git a/changelog/dev-refactor-order-dispute-refundable-util b/changelog/dev-refactor-order-dispute-refundable-util new file mode 100644 index 00000000000..7fbc1de10b0 --- /dev/null +++ b/changelog/dev-refactor-order-dispute-refundable-util @@ -0,0 +1,5 @@ +Significance: patch +Type: dev +Comment: Not user-facing: refactor's refund eligibility logic for disputed orders + + From d767dc1c6e42e55b2230479e5955d4c8a32e89b6 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Thu, 4 Jan 2024 09:44:55 +1000 Subject: [PATCH 7/8] Rename var to `showControlMenu` to indicate refund menu may include other actions in the future --- client/payment-details/summary/index.tsx | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/client/payment-details/summary/index.tsx b/client/payment-details/summary/index.tsx index 7845632e00a..58db34e8da3 100644 --- a/client/payment-details/summary/index.tsx +++ b/client/payment-details/summary/index.tsx @@ -209,12 +209,13 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( { const disputeFee = charge.dispute && getDisputeFeeFormatted( charge.dispute ); - // If this transaction is disputed, check if it is refundable. If not, we should hide the refund menu. + // If this transaction is disputed, check if it is refundable. const isDisputeRefundable = charge.dispute ? isRefundable( charge.dispute.status ) : true; - const showRefundMenu = + // Control menu only shows refund actions for now. In the future, it may show other actions. + const showControlMenu = charge.captured && ! charge.refunded && isDisputeRefundable; // Use the balance_transaction fee if available. If not (e.g. authorized but not captured), use the application_fee_amount. @@ -493,7 +494,7 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( {
- { showRefundMenu && ( + { showControlMenu && ( Date: Fri, 5 Jan 2024 09:21:30 +1000 Subject: [PATCH 8/8] Fix changelog --- changelog/dev-refactor-order-dispute-refundable-util | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/changelog/dev-refactor-order-dispute-refundable-util b/changelog/dev-refactor-order-dispute-refundable-util index 7fbc1de10b0..276e36cdcb8 100644 --- a/changelog/dev-refactor-order-dispute-refundable-util +++ b/changelog/dev-refactor-order-dispute-refundable-util @@ -1,5 +1,3 @@ Significance: patch Type: dev -Comment: Not user-facing: refactor's refund eligibility logic for disputed orders - - +Comment: Not user-facing: refactors the refund eligibility logic for disputed orders