From 266fe7d1d8668007afc1c678014b541cf08c7106 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:10:35 +1000 Subject: [PATCH 1/7] Remove transaction dispute details feature flag to enable feature by default --- client/globals.d.ts | 1 - client/index.js | 12 +- client/payment-details/summary/index.tsx | 7 +- .../summary/test/index.test.tsx | 641 +++++++++--------- client/settings/wcpay-settings-context.js | 1 - includes/admin/class-wc-payments-admin.php | 5 +- includes/class-wc-payments-features.php | 11 - 7 files changed, 319 insertions(+), 359 deletions(-) diff --git a/client/globals.d.ts b/client/globals.d.ts index 8a972874945..897b9e4500c 100644 --- a/client/globals.d.ts +++ b/client/globals.d.ts @@ -15,7 +15,6 @@ declare global { customSearch: boolean; isAuthAndCaptureEnabled: boolean; paymentTimeline: boolean; - isDisputeOnTransactionPageEnabled: boolean; }; fraudServices: unknown[]; testMode: boolean; diff --git a/client/index.js b/client/index.js index 681ff13479b..325fc6bc9d3 100644 --- a/client/index.js +++ b/client/index.js @@ -152,14 +152,8 @@ addFilter( capability: 'manage_woocommerce', } ); - // If disputes on transaction page feature is enabled, set up a soft - // redirect component; otherwise register the (legacy) dispute details page. - const isDisputeOnTransactionPageEnabled = - window.wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled; pages.push( { - container: isDisputeOnTransactionPageEnabled - ? RedirectToTransactionDetails - : LegacyDisputeDetailsPage, + container: RedirectToTransactionDetails, path: '/payments/disputes/details', wpOpenMenu: menuID, breadcrumbs: [ @@ -171,9 +165,7 @@ addFilter( __( 'Dispute details', 'woocommerce-payments' ), ], navArgs: { - id: isDisputeOnTransactionPageEnabled - ? 'wc-payments-disputes-details-legacy-redirect' - : 'wc-payments-disputes-details', + id: 'wc-payments-disputes-details-legacy-redirect', parentPath: '/payments/disputes', }, capability: 'manage_woocommerce', diff --git a/client/payment-details/summary/index.tsx b/client/payment-details/summary/index.tsx index 319b6adcab8..36fd2f4ae05 100644 --- a/client/payment-details/summary/index.tsx +++ b/client/payment-details/summary/index.tsx @@ -167,10 +167,7 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( { charge.currency && balance.currency !== charge.currency; const { - featureFlags: { - isAuthAndCaptureEnabled, - isDisputeOnTransactionPageEnabled, - }, + featureFlags: { isAuthAndCaptureEnabled }, } = useContext( WCPaySettingsContext ); // We should only fetch the authorization data if the payment is marked for manual capture and it is not already captured. @@ -463,7 +460,7 @@ const PaymentDetailsSummary: React.FC< PaymentDetailsSummaryProps > = ( { - { isDisputeOnTransactionPageEnabled && charge.dispute && ( + { charge.dispute && ( <> { isAwaitingResponse( charge.dispute.status ) ? ( { }, featureFlags: { isAuthAndCaptureEnabled: true, - isDisputeOnTransactionPageEnabled: false, }, currencyData: { US: { @@ -461,390 +459,379 @@ describe( 'PaymentDetailsSummary', () => { } ); } ); - describe( 'with feature flag isDisputeOnTransactionPageEnabled', () => { - beforeEach( () => { - global.wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled = true; - } ); - - afterEach( () => { - global.wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled = false; - } ); - - test( 'renders the information of a disputed charge', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'needs_response'; + test( 'renders the information of a disputed charge', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'needs_response'; - renderCharge( charge ); + renderCharge( charge ); - // Dispute Notice - screen.getByText( - /The cardholder claims this is an unauthorized transaction/, - { ignore: '.a11y-speak-region' } - ); + // Dispute Notice + screen.getByText( + /The cardholder claims this is an unauthorized transaction/, + { ignore: '.a11y-speak-region' } + ); - // Don't render the staged evidence message - expect( - screen.queryByText( - /You initiated a challenge to this dispute/, - { ignore: '.a11y-speak-region' } - ) - ).toBeNull(); + // Don't render the staged evidence message + expect( + screen.queryByText( /You initiated a challenge to this dispute/, { + ignore: '.a11y-speak-region', + } ) + ).toBeNull(); - // Dispute Summary Row - expect( - screen.getByText( /Dispute Amount/i ).nextSibling - ).toHaveTextContent( /\$20.00/ ); - expect( - screen.getByText( /Disputed On/i ).nextSibling - ).toHaveTextContent( /Aug 30, 2023/ ); - expect( - screen.getByText( /Reason/i ).nextSibling - ).toHaveTextContent( /Transaction unauthorized/ ); - expect( - screen.getByText( /Respond By/i ).nextSibling - ).toHaveTextContent( /Sep 9, 2023/ ); + // Dispute Summary Row + expect( + screen.getByText( /Dispute Amount/i ).nextSibling + ).toHaveTextContent( /\$20.00/ ); + expect( + screen.getByText( /Disputed On/i ).nextSibling + ).toHaveTextContent( /Aug 30, 2023/ ); + expect( screen.getByText( /Reason/i ).nextSibling ).toHaveTextContent( + /Transaction unauthorized/ + ); + expect( + screen.getByText( /Respond By/i ).nextSibling + ).toHaveTextContent( /Sep 9, 2023/ ); - // Steps to resolve - screen.getByText( /Steps to resolve/i ); - screen.getByRole( 'link', { - name: /Email the customer/i, - } ); - screen.getByRole( 'link', { - name: /guidance on dispute withdrawal/i, - } ); + // Steps to resolve + screen.getByText( /Steps to resolve/i ); + screen.getByRole( 'link', { + name: /Email the customer/i, + } ); + screen.getByRole( 'link', { + name: /guidance on dispute withdrawal/i, + } ); - // Actions - screen.getByRole( 'button', { - name: /Challenge dispute/, - } ); - screen.getByRole( 'button', { - name: /Accept dispute/, - } ); + // Actions + screen.getByRole( 'button', { + name: /Challenge dispute/, } ); + screen.getByRole( 'button', { + name: /Accept dispute/, + } ); + } ); - test( 'renders the information of a disputed charge when the store/charge currency differ', () => { - // True when multi-currency is enabled. - global.wcpaySettings.shouldUseExplicitPrice = true; + test( 'renders the information of a disputed charge when the store/charge currency differ', () => { + // True when multi-currency is enabled. + global.wcpaySettings.shouldUseExplicitPrice = true; - // In this case, charge currency is JPY, but store currency is NOK. - const charge = getBaseCharge(); - charge.currency = 'jpy'; - charge.amount = 10000; - charge.balance_transaction = { - amount: 72581, + // In this case, charge currency is JPY, but store currency is NOK. + const charge = getBaseCharge(); + charge.currency = 'jpy'; + charge.amount = 10000; + charge.balance_transaction = { + amount: 72581, + currency: 'nok', + reporting_category: 'charge', + fee: 4152, + }; + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'needs_response'; + charge.dispute.amount = 10000; + charge.dispute.currency = 'jpy'; + charge.dispute.balance_transactions = [ + { + amount: -72581, currency: 'nok', - reporting_category: 'charge', - fee: 4152, - }; - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'needs_response'; - charge.dispute.amount = 10000; - charge.dispute.currency = 'jpy'; - charge.dispute.balance_transactions = [ - { - amount: -72581, - currency: 'nok', - fee: 15000, - reporting_category: 'dispute', - }, - ]; - renderCharge( charge ); + fee: 15000, + reporting_category: 'dispute', + }, + ]; + renderCharge( charge ); - // Disputed amount should show the store (balance transaction) currency. - expect( - screen.getByText( /Dispute Amount/i ).nextSibling - ).toHaveTextContent( /kr 725.81 NOK/i ); - } ); + // Disputed amount should show the store (balance transaction) currency. + expect( + screen.getByText( /Dispute Amount/i ).nextSibling + ).toHaveTextContent( /kr 725.81 NOK/i ); + } ); - test( 'renders the information of an inquiry when the store/charge currency differ', () => { - // True when multi-currency is enabled. - global.wcpaySettings.shouldUseExplicitPrice = true; + test( 'renders the information of an inquiry when the store/charge currency differ', () => { + // True when multi-currency is enabled. + global.wcpaySettings.shouldUseExplicitPrice = true; - // In this case, charge currency is JPY, but store currency is NOK. - const charge = getBaseCharge(); - charge.currency = 'jpy'; - charge.amount = 10000; - charge.balance_transaction = { - amount: 72581, - currency: 'nok', - reporting_category: 'charge', - fee: 4152, - }; - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'warning_needs_response'; - charge.dispute.amount = 10000; - charge.dispute.currency = 'jpy'; - // Inquiries don't have balance transactions. - charge.dispute.balance_transactions = []; - renderCharge( charge ); - - // Disputed amount should show the dispute/charge currency. - expect( - screen.getByText( /Dispute Amount/i ).nextSibling - ).toHaveTextContent( /¥10,000 JPY/i ); - } ); + // In this case, charge currency is JPY, but store currency is NOK. + const charge = getBaseCharge(); + charge.currency = 'jpy'; + charge.amount = 10000; + charge.balance_transaction = { + amount: 72581, + currency: 'nok', + reporting_category: 'charge', + fee: 4152, + }; + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'warning_needs_response'; + charge.dispute.amount = 10000; + charge.dispute.currency = 'jpy'; + // Inquiries don't have balance transactions. + charge.dispute.balance_transactions = []; + renderCharge( charge ); - test( 'correctly renders dispute details for a dispute with staged evidence', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'needs_response'; - charge.dispute.evidence_details = { - has_evidence: true, - due_by: 1694303999, - past_due: false, - submission_count: 0, - }; - - renderCharge( charge ); - - screen.getByText( - /The cardholder claims this is an unauthorized transaction/, - { ignore: '.a11y-speak-region' } - ); + // Disputed amount should show the dispute/charge currency. + expect( + screen.getByText( /Dispute Amount/i ).nextSibling + ).toHaveTextContent( /¥10,000 JPY/i ); + } ); - // Render the staged evidence message - screen.getByText( /You initiated a challenge to this dispute/, { - ignore: '.a11y-speak-region', - } ); + test( 'correctly renders dispute details for a dispute with staged evidence', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'needs_response'; + charge.dispute.evidence_details = { + has_evidence: true, + due_by: 1694303999, + past_due: false, + submission_count: 0, + }; - screen.getByRole( 'button', { - name: /Continue with challenge/, - } ); - } ); + renderCharge( charge ); - test( 'correctly renders the accept dispute modal and accepts', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'needs_response'; + screen.getByText( + /The cardholder claims this is an unauthorized transaction/, + { ignore: '.a11y-speak-region' } + ); - renderCharge( charge ); + // Render the staged evidence message + screen.getByText( /You initiated a challenge to this dispute/, { + ignore: '.a11y-speak-region', + } ); - const openModalButton = screen.getByRole( 'button', { - name: /Accept dispute/, - } ); + screen.getByRole( 'button', { + name: /Continue with challenge/, + } ); + } ); - // Open the modal - openModalButton.click(); + test( 'correctly renders the accept dispute modal and accepts', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'needs_response'; - screen.getByRole( 'heading', { - name: /Accept the dispute?/, - } ); - screen.getByText( /\$15.00 dispute fee/, { - ignore: '.a11y-speak-region', - } ); + renderCharge( charge ); - screen.getByRole( 'button', { - name: /Cancel/, - } ); - const acceptButton = screen.getByRole( 'button', { - name: /Accept dispute/, - } ); + const openModalButton = screen.getByRole( 'button', { + name: /Accept dispute/, + } ); + + // Open the modal + openModalButton.click(); - // Accept the dispute - acceptButton.click(); + screen.getByRole( 'heading', { + name: /Accept the dispute?/, + } ); + screen.getByText( /\$15.00 dispute fee/, { + ignore: '.a11y-speak-region', + } ); - expect( mockDisputeDoAccept ).toHaveBeenCalledTimes( 1 ); + screen.getByRole( 'button', { + name: /Cancel/, + } ); + const acceptButton = screen.getByRole( 'button', { + name: /Accept dispute/, } ); - test( 'navigates to the dispute challenge screen when the challenge button is clicked', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'needs_response'; - charge.dispute.id = 'dp_test123'; + // Accept the dispute + acceptButton.click(); - renderCharge( charge ); + expect( mockDisputeDoAccept ).toHaveBeenCalledTimes( 1 ); + } ); - const challengeButton = screen.getByRole( 'button', { - name: /Challenge dispute/, - } ); + test( 'navigates to the dispute challenge screen when the challenge button is clicked', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'needs_response'; + charge.dispute.id = 'dp_test123'; - challengeButton.click(); + renderCharge( charge ); - expect( window.location.href ).toContain( - `admin.php?page=wc-admin&path=%2Fpayments%2Fdisputes%2Fchallenge&id=${ charge.dispute.id }` - ); + const challengeButton = screen.getByRole( 'button', { + name: /Challenge dispute/, } ); - test( 'correctly renders dispute details for "won" disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'won'; - charge.dispute.metadata.__evidence_submitted_at = '1693400000'; - renderCharge( charge ); + challengeButton.click(); - screen.getByText( /You won this dispute on/i, { - ignore: '.a11y-speak-region', - } ); - screen.getByRole( 'button', { name: /View dispute details/i } ); + expect( window.location.href ).toContain( + `admin.php?page=wc-admin&path=%2Fpayments%2Fdisputes%2Fchallenge&id=${ charge.dispute.id }` + ); + } ); - // No actions or steps rendered - expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + test( 'correctly renders dispute details for "won" disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'won'; + charge.dispute.metadata.__evidence_submitted_at = '1693400000'; + renderCharge( charge ); + + screen.getByText( /You won this dispute on/i, { + ignore: '.a11y-speak-region', } ); + screen.getByRole( 'button', { name: /View dispute details/i } ); - test( 'correctly renders dispute details for "under_review" disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'under_review'; - charge.dispute.metadata.__evidence_submitted_at = '1693400000'; + // No actions or steps rendered + expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); + } ); - renderCharge( charge ); + test( 'correctly renders dispute details for "under_review" disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'under_review'; + charge.dispute.metadata.__evidence_submitted_at = '1693400000'; - screen.getByText( /You submitted evidence for this dispute/i, { - ignore: '.a11y-speak-region', - } ); - screen.getByRole( 'button', { name: /View submitted evidence/i } ); + renderCharge( charge ); - // No actions or steps rendered - expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + screen.getByText( /You submitted evidence for this dispute/i, { + ignore: '.a11y-speak-region', } ); + screen.getByRole( 'button', { name: /View submitted evidence/i } ); - test( 'correctly renders dispute details for "accepted" disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'lost'; - charge.dispute.metadata.__closed_by_merchant = '1'; - charge.dispute.metadata.__dispute_closed_at = '1693453017'; + // No actions or steps rendered + expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); + } ); - renderCharge( charge ); + test( 'correctly renders dispute details for "accepted" disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'lost'; + charge.dispute.metadata.__closed_by_merchant = '1'; + charge.dispute.metadata.__dispute_closed_at = '1693453017'; - screen.getByText( /This dispute was accepted/i, { - ignore: '.a11y-speak-region', - } ); - // Check for the correct fee amount - screen.getByText( /\$15.00 fee/i, { - ignore: '.a11y-speak-region', - } ); + renderCharge( charge ); - // No actions or steps rendered - expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + screen.getByText( /This dispute was accepted/i, { + ignore: '.a11y-speak-region', + } ); + // Check for the correct fee amount + screen.getByText( /\$15.00 fee/i, { + ignore: '.a11y-speak-region', } ); - test( 'correctly renders dispute details for "lost" disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'lost'; - charge.dispute.metadata.__evidence_submitted_at = '1693400000'; - charge.dispute.metadata.__dispute_closed_at = '1693453017'; + // No actions or steps rendered + expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); + } ); - renderCharge( charge ); + test( 'correctly renders dispute details for "lost" disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'lost'; + charge.dispute.metadata.__evidence_submitted_at = '1693400000'; + charge.dispute.metadata.__dispute_closed_at = '1693453017'; - screen.getByText( /This dispute was lost/i, { - ignore: '.a11y-speak-region', - } ); - // Check for the correct fee amount - screen.getByText( /\$15.00 fee/i, { - ignore: '.a11y-speak-region', - } ); - screen.getByRole( 'button', { name: /View dispute details/i } ); + renderCharge( charge ); - // No actions or steps rendered - expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + screen.getByText( /This dispute was lost/i, { + ignore: '.a11y-speak-region', + } ); + // Check for the correct fee amount + screen.getByText( /\$15.00 fee/i, { + ignore: '.a11y-speak-region', } ); + screen.getByRole( 'button', { name: /View dispute details/i } ); - test( 'correctly renders dispute details for "warning_under_review" inquiry disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'warning_under_review'; - charge.dispute.metadata.__evidence_submitted_at = '1693400000'; + // No actions or steps rendered + expect( screen.queryByText( /Steps to resolve/i ) ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); + } ); - renderCharge( charge ); + test( 'correctly renders dispute details for "warning_under_review" inquiry disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'warning_under_review'; + charge.dispute.metadata.__evidence_submitted_at = '1693400000'; - screen.getByText( /You submitted evidence for this inquiry/i, { - ignore: '.a11y-speak-region', - } ); - screen.getByRole( 'button', { name: /View submitted evidence/i } ); + renderCharge( charge ); - // No actions rendered - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + screen.getByText( /You submitted evidence for this inquiry/i, { + ignore: '.a11y-speak-region', } ); + screen.getByRole( 'button', { name: /View submitted evidence/i } ); - test( 'correctly renders dispute details for "warning_closed" inquiry disputes', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'warning_closed'; - charge.dispute.metadata.__evidence_submitted_at = '1693400000'; - charge.dispute.metadata.__dispute_closed_at = '1693453017'; + // No actions rendered + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); + } ); - renderCharge( charge ); + test( 'correctly renders dispute details for "warning_closed" inquiry disputes', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'warning_closed'; + charge.dispute.metadata.__evidence_submitted_at = '1693400000'; + charge.dispute.metadata.__dispute_closed_at = '1693453017'; - screen.getByText( /This inquiry was closed/i, { - ignore: '.a11y-speak-region', - } ); - screen.getByRole( 'button', { name: /View submitted evidence/i } ); + renderCharge( charge ); - // No actions rendered - expect( - screen.queryByRole( 'button', { - name: /Challenge/i, - } ) - ).toBeNull(); - expect( - screen.queryByRole( 'button', { - name: /Accept/i, - } ) - ).toBeNull(); + screen.getByText( /This inquiry was closed/i, { + ignore: '.a11y-speak-region', } ); + screen.getByRole( 'button', { name: /View submitted evidence/i } ); + + // No actions rendered + expect( + screen.queryByRole( 'button', { + name: /Challenge/i, + } ) + ).toBeNull(); + expect( + screen.queryByRole( 'button', { + name: /Accept/i, + } ) + ).toBeNull(); } ); } ); diff --git a/client/settings/wcpay-settings-context.js b/client/settings/wcpay-settings-context.js index 0966d6e9c47..456ce5cc6c5 100644 --- a/client/settings/wcpay-settings-context.js +++ b/client/settings/wcpay-settings-context.js @@ -9,7 +9,6 @@ const WCPaySettingsContext = createContext( { accountStatus: {}, featureFlags: { isAuthAndCaptureEnabled: false, - isDisputeOnTransactionPageEnabled: false, woopay: false, }, } ); diff --git a/includes/admin/class-wc-payments-admin.php b/includes/admin/class-wc-payments-admin.php index 25b0a0fdc40..3a290ea6d06 100644 --- a/includes/admin/class-wc-payments-admin.php +++ b/includes/admin/class-wc-payments-admin.php @@ -473,12 +473,9 @@ public function add_payments_menu() { ] ); - $is_dispute_on_transaction_page_enabled = WC_Payments_Features::is_dispute_on_transaction_page_enabled(); wc_admin_register_page( [ - 'id' => $is_dispute_on_transaction_page_enabled - ? 'wc-payments-disputes-details-legacy-redirect' - : 'wc-payments-disputes-details', + 'id' => 'wc-payments-disputes-details-legacy-redirect', 'title' => __( 'Dispute details', 'woocommerce-payments' ), 'parent' => 'wc-payments-disputes', 'path' => '/payments/disputes/details', diff --git a/includes/class-wc-payments-features.php b/includes/class-wc-payments-features.php index fb4d8333231..ea76a5116dd 100644 --- a/includes/class-wc-payments-features.php +++ b/includes/class-wc-payments-features.php @@ -22,7 +22,6 @@ class WC_Payments_Features { const WOOPAY_FIRST_PARTY_AUTH_FLAG_NAME = '_wcpay_feature_woopay_first_party_auth'; const AUTH_AND_CAPTURE_FLAG_NAME = '_wcpay_feature_auth_and_capture'; const PROGRESSIVE_ONBOARDING_FLAG_NAME = '_wcpay_feature_progressive_onboarding'; - const DISPUTE_ON_TRANSACTION_PAGE = '_wcpay_feature_dispute_on_transaction_page'; const PAY_FOR_ORDER_FLOW = '_wcpay_feature_pay_for_order_flow'; const DEFERRED_UPE_SERVER_FLAG_NAME = 'is_deferred_intent_creation_upe_enabled'; @@ -247,15 +246,6 @@ public static function is_wcpay_subscriptions_eligible() { return ! empty( $store_base_location['country'] ) && 'US' === $store_base_location['country']; } - /** - * Checks whether Deposits details UI on Transaction Details page is enabled. Disabled by default. - * - * @return bool - */ - public static function is_dispute_on_transaction_page_enabled(): bool { - return '1' === get_option( self::DISPUTE_ON_TRANSACTION_PAGE, '0' ); - } - /** * Checks whether the merchant has chosen Subscription product types during onboarding * WooCommerce and is elible for WCPay Subscriptions, if so, enables the feature flag. @@ -451,7 +441,6 @@ public static function to_array() { 'woopayExpressCheckout' => self::is_woopay_express_checkout_enabled(), 'isAuthAndCaptureEnabled' => self::is_auth_and_capture_enabled(), 'progressiveOnboarding' => self::is_progressive_onboarding_enabled(), - 'isDisputeOnTransactionPageEnabled' => self::is_dispute_on_transaction_page_enabled(), 'isPayForOrderFlowEnabled' => self::is_pay_for_order_flow_enabled(), ] ); From 7cc45460eca10e72f24f7bea2b6995227052053c Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:11:51 +1000 Subject: [PATCH 2/7] Remove redundant payment details summary test --- .../summary/test/index.test.tsx | 19 ------------------- 1 file changed, 19 deletions(-) diff --git a/client/payment-details/summary/test/index.test.tsx b/client/payment-details/summary/test/index.test.tsx index 2b1c3dc6e93..ce2fdb56501 100755 --- a/client/payment-details/summary/test/index.test.tsx +++ b/client/payment-details/summary/test/index.test.tsx @@ -237,25 +237,6 @@ describe( 'PaymentDetailsSummary', () => { expect( container ).toMatchSnapshot(); } ); - test( 'renders the information of a disputed charge', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'under_review'; - charge.dispute.balance_transactions = [ - { - amount: -2000, - fee: 1500, - currency: 'usd', - reporting_category: 'dispute', - }, - ]; - - const container = renderCharge( charge ); - screen.getByText( /Deducted: \$-20.00/i ); - expect( container ).toMatchSnapshot(); - } ); - test( 'renders the information of a dispute-reversal charge', () => { const charge = getBaseCharge(); charge.disputed = true; From 868ab802a32c79a9044820d281866bb90d8e3923 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:13:39 +1000 Subject: [PATCH 3/7] Group dispute-related payment details summary tests --- .../summary/test/index.test.tsx | 164 +++++++++--------- 1 file changed, 82 insertions(+), 82 deletions(-) diff --git a/client/payment-details/summary/test/index.test.tsx b/client/payment-details/summary/test/index.test.tsx index ce2fdb56501..5b576f5a914 100755 --- a/client/payment-details/summary/test/index.test.tsx +++ b/client/payment-details/summary/test/index.test.tsx @@ -237,88 +237,6 @@ describe( 'PaymentDetailsSummary', () => { expect( container ).toMatchSnapshot(); } ); - test( 'renders the information of a dispute-reversal charge', () => { - const charge = getBaseCharge(); - charge.disputed = true; - charge.dispute = getBaseDispute(); - charge.dispute.status = 'won'; - - charge.dispute.balance_transactions = [ - { - amount: -2000, - fee: 1500, - currency: 'usd', - reporting_category: 'dispute', - }, - { - amount: 2000, - fee: -1500, - currency: 'usd', - reporting_category: 'dispute_reversal', - }, - ]; - - const container = renderCharge( charge ); - expect( - screen.queryByText( /Deducted: \$-15.00/i ) - ).not.toBeInTheDocument(); - expect( - screen.queryByRole( 'button', { - name: /Fee breakdown/i, - } ) - ).not.toBeInTheDocument(); - expect( container ).toMatchSnapshot(); - } ); - - test( 'renders the fee breakdown tooltip of a disputed charge', () => { - const charge = { - ...getBaseCharge(), - currency: 'jpy', - amount: 10000, - balance_transaction: { - amount: 2000, - currency: 'usd', - fee: 70, - }, - disputed: true, - dispute: { - ...getBaseDispute(), - amount: 10000, - status: 'under_review', - balance_transactions: [ - { - amount: -1500, - fee: 1500, - currency: 'usd', - reporting_category: 'dispute', - }, - ], - } as Dispute, - }; - - renderCharge( charge ); - - // Open tooltip content - const tooltipButton = screen.getByRole( 'button', { - name: /Fee breakdown/i, - } ); - userEvent.click( tooltipButton ); - - // Check fee breakdown calculated correctly - const tooltipContent = screen.getByRole( 'tooltip' ); - expect( - within( tooltipContent ).getByLabelText( /Transaction fee/ ) - ).toHaveTextContent( /\$0.70/ ); - - expect( - within( tooltipContent ).getByLabelText( /Dispute fee/ ) - ).toHaveTextContent( /\$15.00/ ); - - expect( - within( tooltipContent ).getByLabelText( /Total fees/ ) - ).toHaveTextContent( /\$15.70/ ); - } ); - test( 'renders the Tap to Pay channel from metadata', () => { const charge = getBaseCharge(); const metadata = getBaseMetadata(); @@ -528,6 +446,88 @@ describe( 'PaymentDetailsSummary', () => { ).toHaveTextContent( /kr 725.81 NOK/i ); } ); + test( 'renders the information of a dispute-reversal charge', () => { + const charge = getBaseCharge(); + charge.disputed = true; + charge.dispute = getBaseDispute(); + charge.dispute.status = 'won'; + + charge.dispute.balance_transactions = [ + { + amount: -2000, + fee: 1500, + currency: 'usd', + reporting_category: 'dispute', + }, + { + amount: 2000, + fee: -1500, + currency: 'usd', + reporting_category: 'dispute_reversal', + }, + ]; + + const container = renderCharge( charge ); + expect( + screen.queryByText( /Deducted: \$-15.00/i ) + ).not.toBeInTheDocument(); + expect( + screen.queryByRole( 'button', { + name: /Fee breakdown/i, + } ) + ).not.toBeInTheDocument(); + expect( container ).toMatchSnapshot(); + } ); + + test( 'renders the fee breakdown tooltip of a disputed charge', () => { + const charge = { + ...getBaseCharge(), + currency: 'jpy', + amount: 10000, + balance_transaction: { + amount: 2000, + currency: 'usd', + fee: 70, + }, + disputed: true, + dispute: { + ...getBaseDispute(), + amount: 10000, + status: 'under_review', + balance_transactions: [ + { + amount: -1500, + fee: 1500, + currency: 'usd', + reporting_category: 'dispute', + }, + ], + } as Dispute, + }; + + renderCharge( charge ); + + // Open tooltip content + const tooltipButton = screen.getByRole( 'button', { + name: /Fee breakdown/i, + } ); + userEvent.click( tooltipButton ); + + // Check fee breakdown calculated correctly + const tooltipContent = screen.getByRole( 'tooltip' ); + expect( + within( tooltipContent ).getByLabelText( /Transaction fee/ ) + ).toHaveTextContent( /\$0.70/ ); + + expect( + within( tooltipContent ).getByLabelText( /Dispute fee/ ) + ).toHaveTextContent( /\$15.00/ ); + + expect( + within( tooltipContent ).getByLabelText( /Total fees/ ) + ).toHaveTextContent( /\$15.70/ ); + } ); + test( 'renders the information of an inquiry when the store/charge currency differ', () => { // True when multi-currency is enabled. global.wcpaySettings.shouldUseExplicitPrice = true; From 6c40af9627f78ec28e62f87cfd73a20538ab7743 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Wed, 4 Oct 2023 16:13:57 +1000 Subject: [PATCH 4/7] Update snapshot for dispute-reversal test --- .../test/__snapshots__/index.test.tsx.snap | 305 ++---------------- 1 file changed, 29 insertions(+), 276 deletions(-) diff --git a/client/payment-details/summary/test/__snapshots__/index.test.tsx.snap b/client/payment-details/summary/test/__snapshots__/index.test.tsx.snap index 524678d41c7..4a3bcae9a5c 100644 --- a/client/payment-details/summary/test/__snapshots__/index.test.tsx.snap +++ b/client/payment-details/summary/test/__snapshots__/index.test.tsx.snap @@ -2418,297 +2418,50 @@ exports[`PaymentDetailsSummary renders the information of a dispute-reversal cha - - -`; - -exports[`PaymentDetailsSummary renders the information of a disputed charge 1`] = ` -
-
-
-

- $20.00 - - usd - - - Disputed: Under review - -

-
-

- Deducted: - $-20.00 -

-

- Fees: - $-15.70 - -

- -

- Net: - $-15.70 -

-
+ Learn more about preventing disputes + + .
-
- Payment ID: - ch_38jdHA39KKA -
+ +
- -
- -