From 147a4fc73a0237f79128656e5cd5e07abc6ae5a5 Mon Sep 17 00:00:00 2001 From: Shendy <73803630+shendy-a8c@users.noreply.github.com> Date: Thu, 21 Jul 2022 12:56:12 +0700 Subject: [PATCH] Fix pagination issue with dispute count in task list on overview page (#4388) * Expose an API to get dispute status counts. * Action type, action, and type for dispute status counts. * Fix the order of dispute routes. * Fix reference to api client. * Make sure passed statuses parameter to the dispute/status_counts API are valid statuses. * Front-end changes to fetch from dispute/status_counts API and display the number of disputes that need response to the task list on the overview page. * Fix 'Maximum update depth exceeded' error. * Pass number of total disputes, so we can have a complete state for the task list. - Remove accepting parameter statuses in dispute/status_counts API. - Fix task list unit test. * Use camel case to as the state property to store dispute status counts data. * Fix pslam error about the return type of the dispute status counts API. * Changelog for dispute task list pagination and grammar fix. * Change changelog entry because grammar fix and link change to the awaiting_response disputes list filter is done in another PR, https://github.com/Automattic/woocommerce-payments/pull/4414. * Unit test for dispute reducer, resolver, and selector. * Unit test for disputes rest controller. * Use number of disputes needing response to decide singular or plurality of the sentence in dispute task list and use the number of total disputes if there is 0 dispute needing response. * Only show dispute task list if there is only awaiting response disputes. * Pass the number of disputes needing response from PHP as part of wcpaySettings instead of fetching it asynchronously from the frontend. * Remove statusCounts from test. Co-authored-by: James Allan --- changelog/fix-4123-dispute-count | 4 + client/overview/index.js | 6 +- client/overview/task-list/tasks.js | 30 +++----- client/overview/task-list/test/tasks.js | 75 ++----------------- includes/admin/class-wc-payments-admin.php | 53 ++++++------- ...s-wc-rest-payments-disputes-controller.php | 1 - 6 files changed, 48 insertions(+), 121 deletions(-) create mode 100644 changelog/fix-4123-dispute-count diff --git a/changelog/fix-4123-dispute-count b/changelog/fix-4123-dispute-count new file mode 100644 index 00000000000..d51ff4e88a9 --- /dev/null +++ b/changelog/fix-4123-dispute-count @@ -0,0 +1,4 @@ +Significance: patch +Type: fix + +Correctly show the number of disputes that need to be responded in task list. diff --git a/client/overview/index.js b/client/overview/index.js index 551adb1dcec..92270cb4902 100644 --- a/client/overview/index.js +++ b/client/overview/index.js @@ -20,7 +20,6 @@ import ErrorBoundary from 'components/error-boundary'; import TaskList from './task-list'; import { getTasks, taskSort } from './task-list/tasks'; import InboxNotifications from './inbox-notifications'; -import { useDisputes } from 'data'; import JetpackIdcNotice from 'components/jetpack-idc-notice'; import './style.scss'; @@ -34,8 +33,8 @@ const OverviewPage = () => { wpcomReconnectUrl, featureFlags: { accountOverviewTaskList }, needsHttpsSetup, + numDisputesNeedingResponse, } = wcpaySettings; - const { disputes, isLoading } = useDisputes( getQuery() ); const { isLoading: settingsIsLoading, settings } = useSettings(); const tasksUnsorted = getTasks( { @@ -43,7 +42,7 @@ const OverviewPage = () => { showUpdateDetailsTask, wpcomReconnectUrl, needsHttpsSetup, - disputes, + numDisputesNeedingResponse, } ); const tasks = Array.isArray( tasksUnsorted ) && tasksUnsorted.sort( taskSort ); @@ -138,7 +137,6 @@ const OverviewPage = () => { { !! accountOverviewTaskList && 0 < tasks.length && - ! isLoading && ! accountRejected && ( { - if ( ! disputes ) { - return 0; - } - const incompleteDisputes = disputes.filter( ( { status } ) => { - return [ 'warning_needs_response', 'needs_response' ].includes( - status - ); - } ); - return incompleteDisputes.length; -}; - export const getTasks = ( { accountStatus, showUpdateDetailsTask, wpcomReconnectUrl, isAccountOverviewTasksEnabled, needsHttpsSetup, - disputes = [], + numDisputesNeedingResponse = 0, } ) => { const { status, currentDeadline, pastDue, accountLink } = accountStatus; const accountRestrictedSoon = 'restricted_soon' === status; const accountDetailsPastDue = 'restricted' === status && pastDue; let accountDetailsTaskDescription; - const isDisputeTaskVisible = 0 < disputes.length; - const disputesToResolve = getDisputesToResolve( disputes ); + const isDisputeTaskVisible = 0 < numDisputesNeedingResponse; if ( accountRestrictedSoon ) { accountDetailsTaskDescription = sprintf( @@ -135,15 +122,16 @@ export const getTasks = ( { _n( '1 disputed payment needs your response', '%s disputed payments need your response', - disputesToResolve, + numDisputesNeedingResponse, 'woocommerce-payments' ), - disputesToResolve ? disputesToResolve : disputes.length + numDisputesNeedingResponse ), - additionalInfo: disputesToResolve - ? __( 'View and respond', 'woocommerce-payments' ) - : '', - completed: 0 === disputesToResolve, + additionalInfo: + 0 < numDisputesNeedingResponse + ? __( 'View and respond', 'woocommerce-payments' ) + : '', + completed: 0 === numDisputesNeedingResponse, isDeletable: true, isDismissable: true, allowSnooze: true, diff --git a/client/overview/task-list/test/tasks.js b/client/overview/task-list/test/tasks.js index 49169acab8b..300e0c7d485 100644 --- a/client/overview/task-list/test/tasks.js +++ b/client/overview/task-list/test/tasks.js @@ -144,7 +144,7 @@ describe( 'getTasks()', () => { } ); it( 'should not include the dispute resolution task', () => { - const disputes = []; + const numDisputesNeedingResponse = 0; const actual = getTasks( { accountStatus: { status: 'restricted_soon', @@ -152,21 +152,13 @@ describe( 'getTasks()', () => { pastDue: false, accountLink: 'http://example.com', }, - disputes, + numDisputesNeedingResponse, } ); expect( actual ).toEqual( [] ); } ); it( 'should include the dispute resolution task', () => { - const disputes = [ - { - id: 123, - amount: 10, - currency: 'USD', - evidence_details: { due_by: 1624147199 }, - status: 'needs_response', - }, - ]; + const numDisputesNeedingResponse = 1; const actual = getTasks( { accountStatus: { status: 'restricted_soon', @@ -174,7 +166,7 @@ describe( 'getTasks()', () => { pastDue: false, accountLink: 'http://example.com', }, - disputes, + numDisputesNeedingResponse, } ); expect( actual ).toEqual( @@ -187,66 +179,11 @@ describe( 'getTasks()', () => { ] ) ); } ); - it( 'should include the dispute resolution task as completed', () => { - const disputes = [ - { - id: 456, - amount: 10, - currency: 'USD', - evidence_details: { due_by: 1624147199 }, - status: 'another_status', - }, - { - id: 789, - amount: 10, - currency: 'USD', - evidence_details: { due_by: 1624147199 }, - status: 'won', - }, - ]; - const actual = getTasks( { - accountStatus: { - status: 'restricted_soon', - currentDeadline: 1620857083, - pastDue: false, - accountLink: 'http://example.com', - }, - disputes, - } ); - - expect( actual ).toEqual( - expect.arrayContaining( [ - expect.objectContaining( { - key: 'dispute-resolution-task', - completed: true, - level: 3, - title: '2 disputed payments need your response', - } ), - ] ) - ); - } ); } ); describe( 'taskSort()', () => { it( 'should sort the tasks', () => { - /*eslint-disable camelcase*/ - const disputes = [ - { - id: 123, - amount: 10, - currency: 'USD', - evidence_details: { due_by: 1624147199 }, - status: 'won', - }, - { - id: 456, - amount: 10, - currency: 'USD', - evidence_details: { due_by: 1624147199 }, - status: 'needs_response', - }, - ]; - /*eslint-enable camelcase*/ + const numDisputesNeedingResponse = 1; const unsortedTasks = getTasks( { accountStatus: { status: 'restricted_soon', @@ -255,7 +192,7 @@ describe( 'taskSort()', () => { accountLink: 'http://example.com', }, isAccountOverviewTasksEnabled: true, - disputes, + numDisputesNeedingResponse, } ); unsortedTasks.unshift( { key: 'test-element', diff --git a/includes/admin/class-wc-payments-admin.php b/includes/admin/class-wc-payments-admin.php index 9bd855eb37c..2e24af6a2b1 100644 --- a/includes/admin/class-wc-payments-admin.php +++ b/includes/admin/class-wc-payments-admin.php @@ -450,46 +450,47 @@ public function register_payments_scripts() { } $wcpay_settings = [ - 'connectUrl' => WC_Payments_Account::get_connect_url(), - 'connect' => [ + 'connectUrl' => WC_Payments_Account::get_connect_url(), + 'connect' => [ 'country' => WC()->countries->get_base_country(), 'availableCountries' => WC_Payments_Utils::supported_countries(), 'availableStates' => WC()->countries->get_states(), ], - 'testMode' => $this->wcpay_gateway->is_in_test_mode(), + 'testMode' => $this->wcpay_gateway->is_in_test_mode(), // set this flag for use in the front-end to alter messages and notices if on-boarding has been disabled. - 'onBoardingDisabled' => WC_Payments_Account::is_on_boarding_disabled(), - 'errorMessage' => $error_message, - 'featureFlags' => $this->get_frontend_feature_flags(), - 'isSubscriptionsActive' => class_exists( 'WC_Subscriptions' ) && version_compare( WC_Subscriptions::$version, '2.2.0', '>=' ), + 'onBoardingDisabled' => WC_Payments_Account::is_on_boarding_disabled(), + 'errorMessage' => $error_message, + 'featureFlags' => $this->get_frontend_feature_flags(), + 'isSubscriptionsActive' => class_exists( 'WC_Subscriptions' ) && version_compare( WC_Subscriptions::$version, '2.2.0', '>=' ), // used in the settings page by the AccountFees component. - 'zeroDecimalCurrencies' => WC_Payments_Utils::zero_decimal_currencies(), - 'fraudServices' => $this->account->get_fraud_services_config(), - 'isJetpackConnected' => $this->payments_api_client->is_server_connected(), - 'isJetpackIdcActive' => Jetpack_Identity_Crisis::has_identity_crisis(), - 'accountStatus' => $this->account->get_account_status_data(), - 'accountFees' => $this->account->get_fees(), - 'accountLoans' => $this->account->get_capital(), - 'accountEmail' => $this->account->get_account_email(), - 'showUpdateDetailsTask' => get_option( 'wcpay_show_update_business_details_task', 'no' ), - 'wpcomReconnectUrl' => $this->payments_api_client->is_server_connected() && ! $this->payments_api_client->has_server_connection_owner() ? WC_Payments_Account::get_wpcom_reconnect_url() : null, - 'additionalMethodsSetup' => [ + 'zeroDecimalCurrencies' => WC_Payments_Utils::zero_decimal_currencies(), + 'fraudServices' => $this->account->get_fraud_services_config(), + 'isJetpackConnected' => $this->payments_api_client->is_server_connected(), + 'isJetpackIdcActive' => Jetpack_Identity_Crisis::has_identity_crisis(), + 'accountStatus' => $this->account->get_account_status_data(), + 'accountFees' => $this->account->get_fees(), + 'accountLoans' => $this->account->get_capital(), + 'accountEmail' => $this->account->get_account_email(), + 'showUpdateDetailsTask' => get_option( 'wcpay_show_update_business_details_task', 'no' ), + 'wpcomReconnectUrl' => $this->payments_api_client->is_server_connected() && ! $this->payments_api_client->has_server_connection_owner() ? WC_Payments_Account::get_wpcom_reconnect_url() : null, + 'additionalMethodsSetup' => [ 'isUpeEnabled' => WC_Payments_Features::is_upe_enabled(), ], - 'multiCurrencySetup' => [ + 'multiCurrencySetup' => [ 'isSetupCompleted' => get_option( 'wcpay_multi_currency_setup_completed' ), ], - 'needsHttpsSetup' => $this->wcpay_gateway->needs_https_setup(), - 'isMultiCurrencyEnabled' => WC_Payments_Features::is_customer_multi_currency_enabled(), - 'shouldUseExplicitPrice' => WC_Payments_Explicit_Price_Formatter::should_output_explicit_price(), - 'overviewTasksVisibility' => [ + 'needsHttpsSetup' => $this->wcpay_gateway->needs_https_setup(), + 'isMultiCurrencyEnabled' => WC_Payments_Features::is_customer_multi_currency_enabled(), + 'shouldUseExplicitPrice' => WC_Payments_Explicit_Price_Formatter::should_output_explicit_price(), + 'overviewTasksVisibility' => [ 'dismissedTodoTasks' => get_option( 'woocommerce_dismissed_todo_tasks', [] ), 'deletedTodoTasks' => get_option( 'woocommerce_deleted_todo_tasks', [] ), 'remindMeLaterTodoTasks' => get_option( 'woocommerce_remind_me_later_todo_tasks', [] ), ], - 'currentUserEmail' => $current_user_email, - 'currencyData' => $currency_data, - 'restUrl' => get_rest_url( null, '' ), // rest url to concatenate when merchant use Plain permalinks. + 'currentUserEmail' => $current_user_email, + 'currencyData' => $currency_data, + 'restUrl' => get_rest_url( null, '' ), // rest url to concatenate when merchant use Plain permalinks. + 'numDisputesNeedingResponse' => $this->get_disputes_awaiting_response_count(), ]; wp_localize_script( diff --git a/includes/admin/class-wc-rest-payments-disputes-controller.php b/includes/admin/class-wc-rest-payments-disputes-controller.php index 4ae0149eeea..70b8acceaad 100644 --- a/includes/admin/class-wc-rest-payments-disputes-controller.php +++ b/includes/admin/class-wc-rest-payments-disputes-controller.php @@ -68,7 +68,6 @@ public function register_routes() { 'permission_callback' => [ $this, 'check_permission' ], ] ); - register_rest_route( $this->namespace, '/' . $this->rest_base . '/(?P\w+)/close',