Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WC Home active disputes task: improve wording and reduce due_by threshold to align with Payments Overview task #6548

Merged
merged 55 commits into from
Jun 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
55 commits
Select commit Hold shift + click to select a range
21bca52
WIP showing disputes in WC home task list.
shendy-a8c Jun 17, 2023
e193b47
Merge branch 'develop' into update/6505-dispute-task-wc-home
shendy-a8c Jun 17, 2023
c373834
TODO function doc-block.
shendy-a8c Jun 17, 2023
01caf0f
Remove unused use Database_Cache.
shendy-a8c Jun 17, 2023
a7ee915
Add TODO to decide how to access api client and database cache.
shendy-a8c Jun 17, 2023
dda6ec7
Strip HTML tags from wc_price().
shendy-a8c Jun 17, 2023
136826f
TODO as a reminder to add a constant in Database_Cache.
shendy-a8c Jun 17, 2023
de0e7de
Fix to return response's data.
shendy-a8c Jun 17, 2023
ec32eca
Use dummy test disputes data.
shendy-a8c Jun 17, 2023
5332d66
Fix date diff logic.
shendy-a8c Jun 17, 2023
42eca84
Add docblocks and auto-fix formatting
Jinksi Jun 19, 2023
93bd362
Fix translators comment to resolve phpcs error
Jinksi Jun 19, 2023
3339ca1
Use util function to format amounts consistently
Jinksi Jun 19, 2023
24291f6
Fix translator comment
Jinksi Jun 19, 2023
03eb0a8
Merge branch 'develop' into update/6505-dispute-task-wc-home
Jinksi Jun 19, 2023
7050415
Update get_disputes() function docblock
Jinksi Jun 19, 2023
841f239
Fix task subtitle to reflect correct multi-dispute string
Jinksi Jun 20, 2023
a17efe9
Add Database_Cache active disputes key constant
Jinksi Jun 20, 2023
18b5b7e
Ensure time is displayed in format `11:59 PM`
Jinksi Jun 20, 2023
bd11978
Use `$due_by` variable to simplify code
Jinksi Jun 20, 2023
43c053f
Fix string formatting for single dispute > 24h subtitle
Jinksi Jun 20, 2023
504b101
Fix time to display in format `11:59 PM`
Jinksi Jun 20, 2023
2d537f8
Ensure due_by datetimes are compared and displayed in user TZ
Jinksi Jun 20, 2023
cdd4b77
Fix for currency amount returning *100
Jinksi Jun 20, 2023
f09f300
Fix strings: "respond to N disputes", not "for"
Jinksi Jun 20, 2023
df1db46
Remove test case code comments
Jinksi Jun 20, 2023
5b2aa41
Update Disputes Task unit tests
Jinksi Jun 20, 2023
06917f0
Return empty array if `$response?.['data']` is null-ish
Jinksi Jun 20, 2023
378df7b
Remove resolved TODO comment
Jinksi Jun 20, 2023
dbb53a3
Update docblock to improve clarity
Jinksi Jun 20, 2023
e3b97a9
Add changelog entry
Jinksi Jun 20, 2023
78a2459
Add `use` statements to fix PHP linter errors
Jinksi Jun 20, 2023
f48b1f2
Update unit tests to reduce flakiness with relative dates
Jinksi Jun 20, 2023
1e929d3
Update unit test to reduce flakiness with relative dates
Jinksi Jun 20, 2023
2b813b9
Include all active disputes in total calculations
Jinksi Jun 20, 2023
b95d3c2
Fix accuracy of "last week to respond" dispute count
Jinksi Jun 21, 2023
b586758
Fix wording of Overview disputes task subtitle
Jinksi Jun 21, 2023
81c3d61
Sort active disputes by due_by date ascending
Jinksi Jun 21, 2023
83b3dd4
Compare Overview and WC task `due_by` dates with UTC
Jinksi Jun 21, 2023
9b38479
Format datetimes consistently in local timezone
Jinksi Jun 21, 2023
120a8c2
Remove TODO comment
Jinksi Jun 21, 2023
7a64f27
Create/get DB & API class directly from task constructor
Jinksi Jun 21, 2023
fb20b89
Merge branch 'develop' into update/6505-dispute-task-wc-home
Jinksi Jun 21, 2023
4b92383
Remove cache force refresh used for testing
Jinksi Jun 21, 2023
23b58e9
Merge branch 'develop' into update/6505-dispute-task-wc-home
Jinksi Jun 21, 2023
167f5f8
Invalidate cache when disputes are received/updated
Jinksi Jun 21, 2023
02cbd09
Explicitly set moment timezone before task-list tests
Jinksi Jun 22, 2023
caac016
Set test timezone to UTC-5 to avoid daylight savings time
Jinksi Jun 22, 2023
8c9056a
Update comment to improve readability
Jinksi Jun 22, 2023
ff1996b
Use the existing WC_Payments API client rather than recreate
Jinksi Jun 22, 2023
21bf451
Use `is_array` to clarify statement purpose
Jinksi Jun 22, 2023
f4bb038
Use store's date/time formatting
Jinksi Jun 22, 2023
6549cb2
Update tests after datetime formatting change
Jinksi Jun 22, 2023
4efceec
Cache active disputes on task init
Jinksi Jun 22, 2023
b10377c
Correct docblocks to account for null initial value
Jinksi Jun 22, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions changelog/update-6505-dispute-task-wc-home
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
Significance: minor
Type: update

Improve the wording of the "Active Disputes" task list item on the WooCommerce → Home screen to better communicate the urgency of resolving these disputes.
30 changes: 18 additions & 12 deletions client/overview/task-list/dispute-tasks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
* External dependencies
*/
import { __, sprintf } from '@wordpress/i18n';
import { dateI18n } from '@wordpress/date';
import moment from 'moment';
import { getHistory } from '@woocommerce/navigation';

Expand All @@ -28,12 +29,10 @@ const isDueWithin = ( dispute: CachedDispute, days: number ) => {
if ( dispute.due_by === '' ) {
return false;
}
const now = moment();
const dueBy = moment( dispute.due_by );
return (
dueBy.diff( now, 'hours' ) > 0 &&
dueBy.diff( now, 'hours' ) <= 24 * days
);
// Get current time in UTC.
const now = moment().utc();
const dueBy = moment.utc( dispute.due_by );
return dueBy.diff( now, 'hours' ) > 0 && dueBy.diff( now, 'days' ) <= days;
Comment on lines +32 to +35
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comparisons are handled in UTC time for more robust logic consistent with PHP logic in this PR.

};

/**
Expand Down Expand Up @@ -153,15 +152,22 @@ export const getDisputeResolutionTask = (
numDisputesDueWithin24h >= 1
? sprintf(
__( 'Respond today by %s', 'woocommerce-payments' ),
// Show due_by time in local time.
moment( dispute.due_by ).format( 'h:mm A' ) // E.g. "11:59 PM".
// Show due_by time in local timezone: e.g. "11:59 PM".
dateI18n(
'g:i A',
moment.utc( dispute.due_by ).local().toISOString()
)
)
: sprintf(
__(
'By %s – %s left to respond',
'woocommerce-payments'
),
moment( dispute.due_by ).format( 'MMM D, YYYY' ), // E.g. "Jan 1, 2021".
// Show due_by date in local timezone: e.g. "Jan 1, 2021".
dateI18n(
'M j, Y',
moment.utc( dispute.due_by ).local().toISOString()
),
moment( dispute.due_by ).fromNow( true ) // E.g. "2 days".
);

Expand Down Expand Up @@ -204,18 +210,18 @@ export const getDisputeResolutionTask = (
disputeTotalAmounts
);
disputeTask.content =
// Final day / Last week to respond for N of the disputes
// Final day / Last week to respond to N of the disputes
numDisputesDueWithin24h >= 1
? sprintf(
__(
'Final day to respond for %d of the disputes',
'Final day to respond to %d of the disputes',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated this Payments Overview dispute task wording to match the design.

'woocommerce-payments'
),
numDisputesDueWithin24h
)
: sprintf(
__(
'Last week to respond for %d of the disputes',
'Last week to respond to %d of the disputes',
'woocommerce-payments'
),
numDisputesDueWithin7Days
Expand Down
21 changes: 17 additions & 4 deletions client/overview/task-list/test/tasks.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,11 @@
/** @format */

/**
* External dependencies
*/

import moment from 'moment';

/**
* Internal dependencies
*/
Expand Down Expand Up @@ -97,7 +103,13 @@ const mockActiveDisputes = [
];

describe( 'getTasks()', () => {
// Get current timezone
const currentTimezone = moment.tz.guess();

beforeEach( () => {
// set local timezone to EST (not daylight savings time)
// Note Etc/GMT+5 === UTC-5
moment.tz.setDefault( 'Etc/GMT+5' );
// mock Date.now that moment library uses to get current date for testing purposes
Date.now = jest.fn( () => new Date( '2023-02-01T08:00:00.000Z' ) );

Expand All @@ -122,6 +134,7 @@ describe( 'getTasks()', () => {
afterEach( () => {
// roll it back
Date.now = () => new Date();
moment.tz.setDefault( currentTimezone );
} );
it( 'should include business details when flag is set', () => {
const actual = getTasks( {
Expand Down Expand Up @@ -316,7 +329,7 @@ describe( 'getTasks()', () => {

it( 'should not include the dispute resolution task if dispute due_by > 7 days', () => {
// Set Date.now to - 7 days to reduce urgency of disputes.
Date.now = jest.fn( () => new Date( '2023-01-25T08:00:00.000Z' ) );
Date.now = jest.fn( () => new Date( '2023-01-24T08:00:00.000Z' ) );
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed tests that were incorrect due to UTC/local time comparison bugs.

const actual = getTasks( {
accountStatus: {
status: 'restricted_soon',
Expand Down Expand Up @@ -354,7 +367,7 @@ describe( 'getTasks()', () => {
completed: false,
level: 1,
title: 'Respond to a dispute for $10.00 – Last day',
content: 'Respond today by 11:59 PM',
content: 'Respond today by 6:59 PM', // shown in local timezone.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I had a look and couldn't find the local timezone set anywhere. This could cause failures if the timezone is different from the expected (EST ?).

Copy link
Member

@Jinksi Jinksi Jun 22, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! I've explicitly set this to America/New_York for this test suite, reverting afterAll.

You can check this is working by setting the TZ to e.g. Australia/Brisbane, tests will then fail.

moment.tz.setDefault( 'America/New_York' );

Updated in 02cbd09.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I realise this will be impacted by daylight savings... fix incoming.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Set to UTC-5 (EST / non-daylight-savings) in caac016.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't be affected by daylight savings as the dates are consistent.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, yes, right!

I'm having flashbacks of all of the failing GH checks last time we changed to/from daylight savings. I'll just leave it in, at least it's explicit.

actionLabel: 'Respond now',
} ),
] )
Expand Down Expand Up @@ -413,7 +426,7 @@ describe( 'getTasks()', () => {
level: 1,
title:
'Respond to 3 active disputes for a total of $20.00, €10.00',
content: 'Final day to respond for 1 of the disputes',
content: 'Final day to respond to 1 of the disputes',
actionLabel: 'See disputes',
} ),
] )
Expand Down Expand Up @@ -444,7 +457,7 @@ describe( 'getTasks()', () => {
level: 1,
title:
'Respond to 3 active disputes for a total of $20.00, €10.00',
content: 'Last week to respond for 1 of the disputes',
content: 'Last week to respond to 2 of the disputes',
actionLabel: 'See disputes',
} ),
] )
Expand Down
Loading