From 86e8a8508b9dc0ed679ffa8026cea2f61dfcd64e Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 28 Sep 2023 11:02:49 +1300 Subject: [PATCH 01/24] add a new page component for redirecting disputes => details: - first cut - registers a new handler component for dispute details - RedirectToTransactionDetails - that handler generates the relevant url Still to do: - get the redirect working - get the user experience working smoothly - currently takes time to generate the URL - we may need an "interstitial" "one moment please" UI --- .../redirect-to-transaction-details.tsx | 51 +++++++++++++++++++ client/index.js | 7 +-- 2 files changed, 55 insertions(+), 3 deletions(-) create mode 100644 client/disputes/redirect-to-transaction-details.tsx diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx new file mode 100644 index 00000000000..33b7990fdfa --- /dev/null +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -0,0 +1,51 @@ +/** + * External dependencies + */ +import React from 'react'; + +/** + * Internal dependencies. + */ +import { useDispute } from 'data/index'; +import Page from 'components/page'; +import { Dispute } from 'wcpay/types/disputes'; +import { Charge } from 'wcpay/types/charges'; +import { getAdminUrl } from 'wcpay/utils'; + +const RedirectToTransactionDetails = ( { + query: { id: disputeId }, +}: { + query: { id: string }; +} ): JSX.Element => { + const { dispute, isLoading } = useDispute( disputeId ); + const disputeObject = dispute || ( {} as Dispute ); + const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; + // Why would dispute.charge ever be a string? + const chargeObject = disputeObject.charge as Charge; + + let transactionDetailsUrl = ''; + if ( disputeIsAvailable ) { + const paymentIntentId = disputeObject.charge; + transactionDetailsUrl = getAdminUrl( { + page: 'wc-admin', + path: '/payments/transactions/details', + id: chargeObject.payment_intent, + transaction_id: chargeObject.balance_transaction, + type: 'dispute', + } ); + // window.location = transactionDetailsUrl; + // return null; + } + + return ( + +

+ We gonna redirect to + transaction details +

+ 
{ transactionDetailsUrl }
+ + ); +}; + +export default RedirectToTransactionDetails; diff --git a/client/index.js b/client/index.js index acb980c353e..287aab6c117 100644 --- a/client/index.js +++ b/client/index.js @@ -20,7 +20,8 @@ import DepositDetailsPage from 'deposits/details'; import TransactionsPage from 'transactions'; import PaymentDetailsPage from 'payment-details'; import DisputesPage from 'disputes'; -import DisputeDetailsPage from 'disputes/details'; +// import DisputeDetailsPage from 'disputes/details'; +import RedirectToTransactionDetails from 'disputes/redirect-to-transaction-details'; import DisputeEvidencePage from 'disputes/evidence'; import AdditionalMethodsPage from 'wcpay/additional-methods-setup'; import MultiCurrencySetupPage from 'wcpay/multi-currency-setup'; @@ -151,7 +152,7 @@ addFilter( capability: 'manage_woocommerce', } ); pages.push( { - container: DisputeDetailsPage, + container: RedirectToTransactionDetails, path: '/payments/disputes/details', wpOpenMenu: menuID, breadcrumbs: [ @@ -163,7 +164,7 @@ addFilter( __( 'Dispute details', 'woocommerce-payments' ), ], navArgs: { - id: 'wc-payments-disputes-details', + id: 'wc-payments-disputes-details-legacy-redirect', parentPath: '/payments/disputes', }, capability: 'manage_woocommerce', From 1409a1050c5273e21aeb13c00627a90948bf228f Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Thu, 28 Sep 2023 13:44:50 +1300 Subject: [PATCH 02/24] implement the redirect using history push + + reorg imports (alphabeticish by module) --- client/disputes/redirect-to-transaction-details.tsx | 10 ++++++---- 1 file changed, 6 insertions(+), 4 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index 33b7990fdfa..2a6d240c4cc 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -2,14 +2,15 @@ * External dependencies */ import React from 'react'; +import { getHistory } from '@woocommerce/navigation'; /** * Internal dependencies. */ -import { useDispute } from 'data/index'; import Page from 'components/page'; -import { Dispute } from 'wcpay/types/disputes'; +import { useDispute } from 'data/index'; import { Charge } from 'wcpay/types/charges'; +import { Dispute } from 'wcpay/types/disputes'; import { getAdminUrl } from 'wcpay/utils'; const RedirectToTransactionDetails = ( { @@ -33,8 +34,9 @@ const RedirectToTransactionDetails = ( { transaction_id: chargeObject.balance_transaction, type: 'dispute', } ); - // window.location = transactionDetailsUrl; - // return null; + getHistory().replace( transactionDetailsUrl ); + // Is there a way to return null here? This feels less readable. + return <>; } return ( From 83054c83c937814bae1da41cd2d7716dcf5de087 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 09:36:38 +1300 Subject: [PATCH 03/24] TypeScript tweak - allow null return from component: - when redirecting, we don't need to render anything at all --- client/disputes/redirect-to-transaction-details.tsx | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index 2a6d240c4cc..f6d0208e616 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -17,7 +17,7 @@ const RedirectToTransactionDetails = ( { query: { id: disputeId }, }: { query: { id: string }; -} ): JSX.Element => { +} ): JSX.Element | null => { const { dispute, isLoading } = useDispute( disputeId ); const disputeObject = dispute || ( {} as Dispute ); const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; @@ -35,8 +35,7 @@ const RedirectToTransactionDetails = ( { type: 'dispute', } ); getHistory().replace( transactionDetailsUrl ); - // Is there a way to return null here? This feels less readable. - return <>; + return null; } return ( From 83dcc37ef031e6d120840401628955a44ba7b567 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 09:49:58 +1300 Subject: [PATCH 04/24] use notice and spinner to reassure user during redirect --- client/disputes/redirect-to-transaction-details.tsx | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index f6d0208e616..f82f83a420d 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -3,6 +3,7 @@ */ import React from 'react'; import { getHistory } from '@woocommerce/navigation'; +import { Notice, Spinner } from '@wordpress/components'; /** * Internal dependencies. @@ -40,11 +41,10 @@ const RedirectToTransactionDetails = ( { return ( -

- We gonna redirect to - transaction details -

- 
{ transactionDetailsUrl }
+ + +

Redirecting to transaction…

+ ); }; From 73b5a8f0a322637844db2c253a1390cad5e0f368 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 09:55:03 +1300 Subject: [PATCH 05/24] ensure spinner and text are on same line --- client/disputes/redirect-to-transaction-details.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index f82f83a420d..7cf347af20a 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -43,7 +43,7 @@ const RedirectToTransactionDetails = ( { -

Redirecting to transaction…

+ Redirecting to payment details… ); From 416c7edd6e9a7eb17d9c77c265c7dc387afcbb77 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 11:46:04 +1300 Subject: [PATCH 06/24] use WP Flex components to align redirect notice items --- client/disputes/redirect-to-transaction-details.tsx | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index 7cf347af20a..b9c6e8d8170 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -3,7 +3,7 @@ */ import React from 'react'; import { getHistory } from '@woocommerce/navigation'; -import { Notice, Spinner } from '@wordpress/components'; +import { Notice, Spinner, Flex, FlexItem } from '@wordpress/components'; /** * Internal dependencies. @@ -42,8 +42,14 @@ const RedirectToTransactionDetails = ( { return ( - - Redirecting to payment details… + + + + + + Redirecting to payment details… + + ); From 74267c073a94df195d1cd9964f597ada151e7515 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 12:28:48 +1300 Subject: [PATCH 07/24] ensure correct behaviour with feature flag off: - feature off: render (legacy) dispute details page - feature on: render a redirect to transaction details --- client/index.js | 65 ++++++++++++++++++++++++++++++++++--------------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/client/index.js b/client/index.js index 287aab6c117..bca84c6656b 100644 --- a/client/index.js +++ b/client/index.js @@ -20,7 +20,7 @@ import DepositDetailsPage from 'deposits/details'; import TransactionsPage from 'transactions'; import PaymentDetailsPage from 'payment-details'; import DisputesPage from 'disputes'; -// import DisputeDetailsPage from 'disputes/details'; +import DisputeDetailsPage from 'disputes/details'; import RedirectToTransactionDetails from 'disputes/redirect-to-transaction-details'; import DisputeEvidencePage from 'disputes/evidence'; import AdditionalMethodsPage from 'wcpay/additional-methods-setup'; @@ -151,24 +151,51 @@ addFilter( }, capability: 'manage_woocommerce', } ); - pages.push( { - container: RedirectToTransactionDetails, - path: '/payments/disputes/details', - wpOpenMenu: menuID, - breadcrumbs: [ - rootLink, - [ - '/payments/disputes', - __( 'Disputes', 'woocommerce-payments' ), - ], - __( 'Dispute details', 'woocommerce-payments' ), - ], - navArgs: { - id: 'wc-payments-disputes-details-legacy-redirect', - parentPath: '/payments/disputes', - }, - 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( + isDisputeOnTransactionPageEnabled + ? { + container: RedirectToTransactionDetails, + path: '/payments/disputes/details', + wpOpenMenu: menuID, + breadcrumbs: [ + rootLink, + [ + '/payments/disputes', + __( 'Disputes', 'woocommerce-payments' ), + ], + __( 'Dispute details', 'woocommerce-payments' ), + ], + navArgs: { + id: 'wc-payments-disputes-details-legacy-redirect', + parentPath: '/payments/disputes', + }, + capability: 'manage_woocommerce', + } + : { + container: DisputeDetailsPage, + path: '/payments/disputes/details', + wpOpenMenu: menuID, + breadcrumbs: [ + rootLink, + [ + '/payments/disputes', + __( 'Disputes', 'woocommerce-payments' ), + ], + __( 'Dispute details', 'woocommerce-payments' ), + ], + navArgs: { + id: 'wc-payments-disputes-details', + parentPath: '/payments/disputes', + }, + capability: 'manage_woocommerce', + } + ); + pages.push( { container: DisputeEvidencePage, path: '/payments/disputes/challenge', From 341e7f6a5eaa72aa45df1f9008ce2f3e68d2e193 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 12:30:42 +1300 Subject: [PATCH 08/24] Add a hint and bail early from dispute details page component: - This module should be deleted when we go live with this feature - Adding this as a reminder --- client/disputes/details/index.tsx | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/client/disputes/details/index.tsx b/client/disputes/details/index.tsx index e8e43312fcc..e97cd586fea 100644 --- a/client/disputes/details/index.tsx +++ b/client/disputes/details/index.tsx @@ -27,11 +27,19 @@ const DisputeDetails = ( { query: { id: disputeId }, }: { query: { id: string }; -} ): JSX.Element => { +} ): JSX.Element | null => { const { dispute, isLoading, doAccept } = useDispute( disputeId ); const disputeObject = dispute || ( {} as Dispute ); const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; + // Bail early and return nothing if the feature flag is not enabled. + // Here as a hint/reminder to delete this file when the feature flag is removed. + const isDisputeOnTransactionPageEnabled = + wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled; + if ( isDisputeOnTransactionPageEnabled ) { + return null; + } + const actions = disputeIsAvailable && ( Date: Fri, 29 Sep 2023 12:31:09 +1300 Subject: [PATCH 09/24] remove unused --- client/disputes/redirect-to-transaction-details.tsx | 1 - 1 file changed, 1 deletion(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index b9c6e8d8170..c137441b5c3 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -27,7 +27,6 @@ const RedirectToTransactionDetails = ( { let transactionDetailsUrl = ''; if ( disputeIsAvailable ) { - const paymentIntentId = disputeObject.charge; transactionDetailsUrl = getAdminUrl( { page: 'wc-admin', path: '/payments/transactions/details', From d5d7a2e8c9597e91c96866492cc0bf88516902e1 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Fri, 29 Sep 2023 12:42:33 +1300 Subject: [PATCH 10/24] clarify comment about Dispute.charge cast --- client/disputes/redirect-to-transaction-details.tsx | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details.tsx index c137441b5c3..0e5f3ca95fd 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details.tsx @@ -22,7 +22,8 @@ const RedirectToTransactionDetails = ( { const { dispute, isLoading } = useDispute( disputeId ); const disputeObject = dispute || ( {} as Dispute ); const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; - // Why would dispute.charge ever be a string? + // Dispute type allows charge as nested object or string ID, + // so we have to hint we expect a Charge object here. const chargeObject = disputeObject.charge as Charge; let transactionDetailsUrl = ''; From 500b176c0645fcfd7d26f6126106c6203dc51310 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 2 Oct 2023 11:45:19 +1300 Subject: [PATCH 11/24] switch to centred, on-page spinner + "redirecting": - not appropriate use case for notice - this is more consistent with other page loading UI in woo --- .../index.tsx} | 28 +++++++++++-------- .../style.scss | 9 ++++++ 2 files changed, 26 insertions(+), 11 deletions(-) rename client/disputes/{redirect-to-transaction-details.tsx => redirect-to-transaction-details/index.tsx} (77%) create mode 100644 client/disputes/redirect-to-transaction-details/style.scss diff --git a/client/disputes/redirect-to-transaction-details.tsx b/client/disputes/redirect-to-transaction-details/index.tsx similarity index 77% rename from client/disputes/redirect-to-transaction-details.tsx rename to client/disputes/redirect-to-transaction-details/index.tsx index 0e5f3ca95fd..791c14568f5 100644 --- a/client/disputes/redirect-to-transaction-details.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -3,7 +3,7 @@ */ import React from 'react'; import { getHistory } from '@woocommerce/navigation'; -import { Notice, Spinner, Flex, FlexItem } from '@wordpress/components'; +import { Spinner, Flex, FlexItem } from '@wordpress/components'; /** * Internal dependencies. @@ -14,6 +14,8 @@ import { Charge } from 'wcpay/types/charges'; import { Dispute } from 'wcpay/types/disputes'; import { getAdminUrl } from 'wcpay/utils'; +import './style.scss'; + const RedirectToTransactionDetails = ( { query: { id: disputeId }, }: { @@ -41,16 +43,20 @@ const RedirectToTransactionDetails = ( { return ( - - - - - - - Redirecting to payment details… - - - + + + + + +
+ One moment please +
+
Redirecting to payment details…
+ + ); }; diff --git a/client/disputes/redirect-to-transaction-details/style.scss b/client/disputes/redirect-to-transaction-details/style.scss new file mode 100644 index 00000000000..ca8fc49c8f5 --- /dev/null +++ b/client/disputes/redirect-to-transaction-details/style.scss @@ -0,0 +1,9 @@ +.wcpay-dispute-detail-legacy-redirect { + // Shift the redirect spinner to approx center of viewport. + top: 10vh; + position: relative; + + .components-flex-item { + text-align: center; + } +} From 9fe73b538079f55e5927be7297260a10f43c5ea7 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 2 Oct 2023 12:28:20 +1300 Subject: [PATCH 12/24] streamline conditional page registration: - inline conditional for component - inline conditional for nav ID --- client/index.js | 59 ++++++++-------------- includes/admin/class-wc-payments-admin.php | 7 ++- 2 files changed, 28 insertions(+), 38 deletions(-) diff --git a/client/index.js b/client/index.js index bca84c6656b..39fb5ee8e1d 100644 --- a/client/index.js +++ b/client/index.js @@ -157,43 +157,28 @@ addFilter( const isDisputeOnTransactionPageEnabled = window.wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled; pages.push( - isDisputeOnTransactionPageEnabled - ? { - container: RedirectToTransactionDetails, - path: '/payments/disputes/details', - wpOpenMenu: menuID, - breadcrumbs: [ - rootLink, - [ - '/payments/disputes', - __( 'Disputes', 'woocommerce-payments' ), - ], - __( 'Dispute details', 'woocommerce-payments' ), - ], - navArgs: { - id: 'wc-payments-disputes-details-legacy-redirect', - parentPath: '/payments/disputes', - }, - capability: 'manage_woocommerce', - } - : { - container: DisputeDetailsPage, - path: '/payments/disputes/details', - wpOpenMenu: menuID, - breadcrumbs: [ - rootLink, - [ - '/payments/disputes', - __( 'Disputes', 'woocommerce-payments' ), - ], - __( 'Dispute details', 'woocommerce-payments' ), - ], - navArgs: { - id: 'wc-payments-disputes-details', - parentPath: '/payments/disputes', - }, - capability: 'manage_woocommerce', - } + { + container: isDisputeOnTransactionPageEnabled + ? RedirectToTransactionDetails + : DisputeDetailsPage, + path: '/payments/disputes/details', + wpOpenMenu: menuID, + breadcrumbs: [ + rootLink, + [ + '/payments/disputes', + __( 'Disputes', 'woocommerce-payments' ), + ], + __( 'Dispute details', 'woocommerce-payments' ), + ], + navArgs: { + id: isDisputeOnTransactionPageEnabled + ? 'wc-payments-disputes-details-legacy-redirect' + : 'wc-payments-disputes-details', + parentPath: '/payments/disputes', + }, + capability: 'manage_woocommerce', + } ); pages.push( { diff --git a/includes/admin/class-wc-payments-admin.php b/includes/admin/class-wc-payments-admin.php index 477a9c8ccfe..cc406266a38 100644 --- a/includes/admin/class-wc-payments-admin.php +++ b/includes/admin/class-wc-payments-admin.php @@ -465,14 +465,19 @@ public function add_payments_menu() { 'path' => '/payments/transactions/details', ] ); + + $isDisputeOnTransactionPageEnabled = WC_Payments_Features::is_dispute_on_transaction_page_enabled(); wc_admin_register_page( [ - 'id' => 'wc-payments-disputes-details', + 'id' => $isDisputeOnTransactionPageEnabled + ? 'wc-payments-disputes-details-legacy-redirect' + : 'wc-payments-disputes-details', 'title' => __( 'Dispute details', 'woocommerce-payments' ), 'parent' => 'wc-payments-disputes', 'path' => '/payments/disputes/details', ] ); + wc_admin_register_page( [ 'id' => 'wc-payments-disputes-challenge', From e9777127c29f9623a0b431bf66ceaa17ae207b93 Mon Sep 17 00:00:00 2001 From: Rua Haszard Date: Mon, 2 Oct 2023 12:44:25 +1300 Subject: [PATCH 13/24] remove feature flag bailout in legacy dispute details + rename: - rename "Legacy" so it's obvious this is old code - remove bailout, it's not strictly required and makes the PR more complex for low benefit --- client/disputes/details/index.tsx | 14 ++-------- client/index.js | 46 +++++++++++++++---------------- 2 files changed, 25 insertions(+), 35 deletions(-) diff --git a/client/disputes/details/index.tsx b/client/disputes/details/index.tsx index e97cd586fea..e8af4bbcf97 100644 --- a/client/disputes/details/index.tsx +++ b/client/disputes/details/index.tsx @@ -23,23 +23,15 @@ import { TestModeNotice, topics } from 'components/test-mode-notice'; import '../style.scss'; import { Dispute } from 'wcpay/types/disputes'; -const DisputeDetails = ( { +const LegacyDisputeDetails = ( { query: { id: disputeId }, }: { query: { id: string }; -} ): JSX.Element | null => { +} ): JSX.Element => { const { dispute, isLoading, doAccept } = useDispute( disputeId ); const disputeObject = dispute || ( {} as Dispute ); const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; - // Bail early and return nothing if the feature flag is not enabled. - // Here as a hint/reminder to delete this file when the feature flag is removed. - const isDisputeOnTransactionPageEnabled = - wcpaySettings.featureFlags.isDisputeOnTransactionPageEnabled; - if ( isDisputeOnTransactionPageEnabled ) { - return null; - } - const actions = disputeIsAvailable && ( Date: Mon, 2 Oct 2023 16:09:15 +1300 Subject: [PATCH 14/24] add changelog (placeholder) --- changelog/fix-6891-redirect-dispute-detail-to-transaction | 4 ++++ 1 file changed, 4 insertions(+) create mode 100644 changelog/fix-6891-redirect-dispute-detail-to-transaction diff --git a/changelog/fix-6891-redirect-dispute-detail-to-transaction b/changelog/fix-6891-redirect-dispute-detail-to-transaction new file mode 100644 index 00000000000..1a7a59f3d4e --- /dev/null +++ b/changelog/fix-6891-redirect-dispute-detail-to-transaction @@ -0,0 +1,4 @@ +Significance: patch +Type: dev + +This work is part of a UI improvements to increase disputes response that is behind a feature flag. A changelog entry will be added to represent the work as a whole. From 2c4dbfc7718f6b26498d5ddb92ed82f483b00e07 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 12:45:47 +1000 Subject: [PATCH 15/24] Define component using `React.FC<>` --- client/disputes/redirect-to-transaction-details/index.tsx | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index 791c14568f5..de1c20f0fc9 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -16,11 +16,9 @@ import { getAdminUrl } from 'wcpay/utils'; import './style.scss'; -const RedirectToTransactionDetails = ( { +const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { query: { id: disputeId }, -}: { - query: { id: string }; -} ): JSX.Element | null => { +} ) => { const { dispute, isLoading } = useDispute( disputeId ); const disputeObject = dispute || ( {} as Dispute ); const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; From ad9a2eed849756c6b3c875a4566f52d6104a44ca Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 12:46:50 +1000 Subject: [PATCH 16/24] Remove redundant `let` for `transactionDetailsUrl` --- client/disputes/redirect-to-transaction-details/index.tsx | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index de1c20f0fc9..0f464e787dc 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -26,9 +26,8 @@ const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { // so we have to hint we expect a Charge object here. const chargeObject = disputeObject.charge as Charge; - let transactionDetailsUrl = ''; if ( disputeIsAvailable ) { - transactionDetailsUrl = getAdminUrl( { + const transactionDetailsUrl = getAdminUrl( { page: 'wc-admin', path: '/payments/transactions/details', id: chargeObject.payment_intent, From 45349c1e8fb28cb10f4dbf6d095e5a1da8f8e643 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 13:04:14 +1000 Subject: [PATCH 17/24] Handle redirect in useEffect hook to fix "setState in render" error --- .../redirect-to-transaction-details/index.tsx | 35 ++++++++++--------- 1 file changed, 18 insertions(+), 17 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index 0f464e787dc..cb9d5a317b8 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -1,7 +1,7 @@ /** * External dependencies */ -import React from 'react'; +import React, { useEffect } from 'react'; import { getHistory } from '@woocommerce/navigation'; import { Spinner, Flex, FlexItem } from '@wordpress/components'; @@ -20,23 +20,24 @@ const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { query: { id: disputeId }, } ) => { const { dispute, isLoading } = useDispute( disputeId ); - const disputeObject = dispute || ( {} as Dispute ); - const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; - // Dispute type allows charge as nested object or string ID, - // so we have to hint we expect a Charge object here. - const chargeObject = disputeObject.charge as Charge; - if ( disputeIsAvailable ) { - const transactionDetailsUrl = getAdminUrl( { - page: 'wc-admin', - path: '/payments/transactions/details', - id: chargeObject.payment_intent, - transaction_id: chargeObject.balance_transaction, - type: 'dispute', - } ); - getHistory().replace( transactionDetailsUrl ); - return null; - } + useEffect( () => { + const disputeObject = dispute || ( {} as Dispute ); + const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; + // Dispute type allows charge as nested object or string ID, + // so we have to hint we expect a Charge object here. + const chargeObject = disputeObject.charge as Charge; + if ( disputeIsAvailable ) { + const transactionDetailsUrl = getAdminUrl( { + page: 'wc-admin', + path: '/payments/transactions/details', + id: chargeObject.payment_intent, + transaction_id: chargeObject.balance_transaction, + type: 'dispute', + } ); + getHistory().replace( transactionDetailsUrl ); + } + }, [ dispute, isLoading ] ); return ( From 9a3df7812d22e3f6b274ca7f1ba2ff7ddaf04b31 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 13:35:49 +1000 Subject: [PATCH 18/24] Remove Dispute type cast --- .../redirect-to-transaction-details/index.tsx | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index cb9d5a317b8..9eb493a5954 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -11,7 +11,6 @@ import { Spinner, Flex, FlexItem } from '@wordpress/components'; import Page from 'components/page'; import { useDispute } from 'data/index'; import { Charge } from 'wcpay/types/charges'; -import { Dispute } from 'wcpay/types/disputes'; import { getAdminUrl } from 'wcpay/utils'; import './style.scss'; @@ -22,12 +21,12 @@ const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { const { dispute, isLoading } = useDispute( disputeId ); useEffect( () => { - const disputeObject = dispute || ( {} as Dispute ); - const disputeIsAvailable = ! isLoading && dispute && disputeObject.id; - // Dispute type allows charge as nested object or string ID, - // so we have to hint we expect a Charge object here. - const chargeObject = disputeObject.charge as Charge; + const disputeIsAvailable = ! isLoading && dispute?.charge; + if ( disputeIsAvailable ) { + // Dispute type allows charge as nested object or string ID, + // so we have to hint we expect a Charge object here. + const chargeObject = dispute.charge as Charge; const transactionDetailsUrl = getAdminUrl( { page: 'wc-admin', path: '/payments/transactions/details', From 1a76dcefde87b8ae6f8e2bfefa5dad01b76ddba5 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 13:36:34 +1000 Subject: [PATCH 19/24] Rename var to clarify meaning --- client/disputes/redirect-to-transaction-details/index.tsx | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index 9eb493a5954..a494fa2a752 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -21,9 +21,9 @@ const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { const { dispute, isLoading } = useDispute( disputeId ); useEffect( () => { - const disputeIsAvailable = ! isLoading && dispute?.charge; + const disputeChargeIsAvailable = ! isLoading && dispute?.charge; - if ( disputeIsAvailable ) { + if ( disputeChargeIsAvailable ) { // Dispute type allows charge as nested object or string ID, // so we have to hint we expect a Charge object here. const chargeObject = dispute.charge as Charge; From 54ac2ce36a7ef086eeb073f77d297ebf5df9444b Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 14:40:42 +1000 Subject: [PATCH 20/24] Fix PHP linter error (use snake_case) --- includes/admin/class-wc-payments-admin.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/admin/class-wc-payments-admin.php b/includes/admin/class-wc-payments-admin.php index 97c2fabf46c..25b0a0fdc40 100644 --- a/includes/admin/class-wc-payments-admin.php +++ b/includes/admin/class-wc-payments-admin.php @@ -473,10 +473,10 @@ public function add_payments_menu() { ] ); - $isDisputeOnTransactionPageEnabled = WC_Payments_Features::is_dispute_on_transaction_page_enabled(); + $is_dispute_on_transaction_page_enabled = WC_Payments_Features::is_dispute_on_transaction_page_enabled(); wc_admin_register_page( [ - 'id' => $isDisputeOnTransactionPageEnabled + 'id' => $is_dispute_on_transaction_page_enabled ? 'wc-payments-disputes-details-legacy-redirect' : 'wc-payments-disputes-details', 'title' => __( 'Dispute details', 'woocommerce-payments' ), From bec08461d2b73998ee5789f157379a425b56a68a Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:19:41 +1000 Subject: [PATCH 21/24] Add dispute fetch error wp.data getter/setter --- client/data/disputes/action-types.js | 1 + client/data/disputes/actions.js | 9 +++++++++ client/data/disputes/reducer.js | 8 +++++++- client/data/disputes/resolvers.js | 2 ++ client/data/disputes/selectors.js | 5 +++++ 5 files changed, 24 insertions(+), 1 deletion(-) diff --git a/client/data/disputes/action-types.js b/client/data/disputes/action-types.js index a88cdbdc02c..318d331c84c 100644 --- a/client/data/disputes/action-types.js +++ b/client/data/disputes/action-types.js @@ -2,6 +2,7 @@ export default { SET_DISPUTE: 'SET_DISPUTE', + SET_ERROR_FOR_DISPUTE: 'SET_ERROR_FOR_DISPUTE', SET_DISPUTES: 'SET_DISPUTES', SET_DISPUTES_SUMMARY: 'SET_DISPUTES_SUMMARY', }; diff --git a/client/data/disputes/actions.js b/client/data/disputes/actions.js index 7e099b440f3..4a0f82197e1 100644 --- a/client/data/disputes/actions.js +++ b/client/data/disputes/actions.js @@ -23,6 +23,15 @@ export function updateDispute( data ) { }; } +export function updateErrorForDispute( id, data, error ) { + return { + type: TYPES.SET_ERROR_FOR_DISPUTE, + id, + data, + error, + }; +} + export function updateDisputes( query, data ) { return { type: TYPES.SET_DISPUTES, diff --git a/client/data/disputes/reducer.js b/client/data/disputes/reducer.js index 3312bb2b9f9..f6b88f90452 100644 --- a/client/data/disputes/reducer.js +++ b/client/data/disputes/reducer.js @@ -15,7 +15,7 @@ const defaultState = { byId: {}, queries: {}, summary: {}, cached: {} }; const receiveDisputes = ( state = defaultState, - { type, query = {}, data = [] } + { type, query = {}, data = [], id, error } ) => { const index = getResourceId( query ); @@ -25,6 +25,12 @@ const receiveDisputes = ( ...state, byId: { ...state.byId, [ data.id ]: data }, }; + case TYPES.SET_ERROR_FOR_DISPUTE: + state = { + ...state, + byId: { ...state.byId, [ id ]: { error } }, + }; + break; case TYPES.SET_DISPUTES: return { ...state, diff --git a/client/data/disputes/resolvers.js b/client/data/disputes/resolvers.js index 3676df92d60..ee82f790c51 100644 --- a/client/data/disputes/resolvers.js +++ b/client/data/disputes/resolvers.js @@ -18,6 +18,7 @@ import { updateDispute, updateDisputes, updateDisputesSummary, + updateErrorForDispute, } from './actions'; const formatQueryFilters = ( query ) => ( { @@ -61,6 +62,7 @@ export function* getDispute( id ) { 'createErrorNotice', __( 'Error retrieving dispute.', 'woocommerce-payments' ) ); + yield updateErrorForDispute( id, undefined, e ); } } diff --git a/client/data/disputes/selectors.js b/client/data/disputes/selectors.js index 4bd5347c82a..b592e518298 100644 --- a/client/data/disputes/selectors.js +++ b/client/data/disputes/selectors.js @@ -26,6 +26,11 @@ export const getDispute = ( state, id ) => { return disputeById[ id ]; }; +export const getDisputeError = ( state, id ) => { + const disputeById = getDisputesState( state ).byId || {}; + return disputeById[ id ]?.error; +}; + export const getCachedDispute = ( state, id ) => { const disputeById = getDisputesState( state ).cached || {}; return disputeById[ id ]; From 5e006f810c64372b4b140b734e0a08014245588b Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:28:10 +1000 Subject: [PATCH 22/24] Fix test for getDispute resolved on error --- client/data/disputes/test/resolvers.js | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/client/data/disputes/test/resolvers.js b/client/data/disputes/test/resolvers.js index 5b576a65cf7..cd5538de148 100644 --- a/client/data/disputes/test/resolvers.js +++ b/client/data/disputes/test/resolvers.js @@ -13,6 +13,7 @@ import { updateDispute, updateDisputes, updateDisputesSummary, + updateErrorForDispute, } from '../actions'; import { getDispute, getDisputes, getDisputesSummary } from '../resolvers'; @@ -68,6 +69,9 @@ describe( 'getDispute resolver', () => { expect.any( String ) ) ); + expect( generator.next().value ).toEqual( + updateErrorForDispute( 'dp_mock1', undefined, errorResponse ) + ); } ); } ); } ); From 6111fac994cc6f460ba157a459f280597afb0ced Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:28:22 +1000 Subject: [PATCH 23/24] Add error state to useDispute hook return props --- client/data/disputes/hooks.ts | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/client/data/disputes/hooks.ts b/client/data/disputes/hooks.ts index 256faf30152..e3797499537 100644 --- a/client/data/disputes/hooks.ts +++ b/client/data/disputes/hooks.ts @@ -15,6 +15,7 @@ import type { CachedDisputes, DisputesSummary, } from 'wcpay/types/disputes'; +import type { ApiError } from 'wcpay/types/errors'; import { STORE_NAME } from '../constants'; import { disputeAwaitingResponseStatuses } from 'wcpay/disputes/filters/config'; @@ -25,16 +26,20 @@ import { disputeAwaitingResponseStatuses } from 'wcpay/disputes/filters/config'; export const useDispute = ( id: string ): { - dispute: Dispute; + dispute?: Dispute; + error?: ApiError; isLoading: boolean; doAccept: () => void; } => { - const { dispute, isLoading } = useSelect( + const { dispute, error, isLoading } = useSelect( ( select ) => { - const { getDispute, isResolving } = select( STORE_NAME ); + const { getDispute, getDisputeError, isResolving } = select( + STORE_NAME + ); return { - dispute: getDispute( id ), + dispute: getDispute( id ), + error: getDisputeError( id ), isLoading: isResolving( 'getDispute', [ id ] ), }; }, @@ -44,7 +49,7 @@ export const useDispute = ( const { acceptDispute } = useDispatch( STORE_NAME ); const doAccept = () => acceptDispute( id ); - return { dispute, isLoading, doAccept }; + return { dispute, isLoading, error, doAccept }; }; /** From 6b00887b7c81f36bbd09f857bf336c15c25d3224 Mon Sep 17 00:00:00 2001 From: Eric Jinks <3147296+Jinksi@users.noreply.github.com> Date: Tue, 3 Oct 2023 15:35:29 +1000 Subject: [PATCH 24/24] Handle network error in UI --- .../redirect-to-transaction-details/index.tsx | 43 +++++++++++++------ 1 file changed, 29 insertions(+), 14 deletions(-) diff --git a/client/disputes/redirect-to-transaction-details/index.tsx b/client/disputes/redirect-to-transaction-details/index.tsx index a494fa2a752..87c7e7954a4 100644 --- a/client/disputes/redirect-to-transaction-details/index.tsx +++ b/client/disputes/redirect-to-transaction-details/index.tsx @@ -3,7 +3,7 @@ */ import React, { useEffect } from 'react'; import { getHistory } from '@woocommerce/navigation'; -import { Spinner, Flex, FlexItem } from '@wordpress/components'; +import { Spinner, Icon, Flex, FlexItem } from '@wordpress/components'; /** * Internal dependencies. @@ -14,16 +14,15 @@ import { Charge } from 'wcpay/types/charges'; import { getAdminUrl } from 'wcpay/utils'; import './style.scss'; +import warning from 'wcpay/components/icons/warning'; const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { query: { id: disputeId }, } ) => { - const { dispute, isLoading } = useDispute( disputeId ); + const { dispute, error, isLoading } = useDispute( disputeId ); useEffect( () => { - const disputeChargeIsAvailable = ! isLoading && dispute?.charge; - - if ( disputeChargeIsAvailable ) { + if ( ! isLoading && dispute?.charge ) { // Dispute type allows charge as nested object or string ID, // so we have to hint we expect a Charge object here. const chargeObject = dispute.charge as Charge; @@ -44,15 +43,31 @@ const RedirectToTransactionDetails: React.FC< { query: { id: string } } > = ( { direction="column" className="wcpay-dispute-detail-legacy-redirect" > - - - - -
- One moment please -
-
Redirecting to payment details…
- + { error ? ( + <> + + + + +
+ Error retrieving dispute +
+
Please check your network and try again.
+ + + ) : ( + <> + + + + +
+ One moment please +
+
Redirecting to payment details…
+ + + ) } );