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, #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 <[email protected]>
Automattic · Jul 21, 2022 · 147a4fc · 147a4fc
1 parent 0b67331
commit 147a4fc
Show file tree

Hide file tree

Showing 6 changed files with 48 additions and 121 deletions.
diff --git a/changelog/fix-4123-dispute-count 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
@@ -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,16 +33,16 @@ const OverviewPage = () => {
 		wpcomReconnectUrl,
 		featureFlags: { accountOverviewTaskList },
 		needsHttpsSetup,
+		numDisputesNeedingResponse,
 	} = wcpaySettings;
-	const { disputes, isLoading } = useDisputes( getQuery() );
 	const { isLoading: settingsIsLoading, settings } = useSettings();
 
 	const tasksUnsorted = getTasks( {
 		accountStatus,
 		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 && (
 					<ErrorBoundary>
 						<TaskList

diff --git a/client/overview/task-list/tasks.js b/client/overview/task-list/tasks.js
@@ -13,33 +13,20 @@ import moment from 'moment';
 import wcpayTracks from 'tracks';
 import { getAdminUrl } from 'wcpay/utils';
 
-const getDisputesToResolve = ( disputes ) => {
-	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
@@ -144,37 +144,29 @@ describe( 'getTasks()', () => {
 	} );
 
 	it( 'should not include the dispute resolution task', () => {
-		const disputes = [];
+		const numDisputesNeedingResponse = 0;
 		const actual = getTasks( {
 			accountStatus: {
 				status: 'restricted_soon',
 				currentDeadline: 1620857083,
 				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',
 				currentDeadline: 1620857083,
 				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
@@ -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
@@ -68,7 +68,6 @@ public function register_routes() {
 				'permission_callback' => [ $this, 'check_permission' ],
 			]
 		);
-
 		register_rest_route(
 			$this->namespace,
 			'/' . $this->rest_base . '/(?P<dispute_id>\w+)/close',