-
Notifications
You must be signed in to change notification settings - Fork 68
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
Changes from 45 commits
21bca52
e193b47
c373834
01caf0f
a7ee915
dda6ec7
136826f
de0e7de
ec32eca
5332d66
42eca84
93bd362
3339ca1
24291f6
03eb0a8
7050415
841f239
a17efe9
18b5b7e
bd11978
43c053f
504b101
2d537f8
cdd4b77
f09f300
df1db46
5b2aa41
06917f0
378df7b
dbb53a3
e3b97a9
78a2459
f48b1f2
1e929d3
2b813b9
b95d3c2
b586758
81c3d61
83b3dd4
9b38479
120a8c2
7a64f27
fb20b89
4b92383
23b58e9
167f5f8
02cbd09
caac016
8c9056a
ff1996b
21bf451
f4bb038
6549cb2
4efceec
b10377c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
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. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -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'; | ||
|
||
|
@@ -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; | ||
}; | ||
|
||
/** | ||
|
@@ -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". | ||
); | ||
|
||
|
@@ -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', | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -316,7 +316,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' ) ); | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||||
|
@@ -354,7 +354,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. | ||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 ?). There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice catch! I've explicitly set this to You can check this is working by setting the TZ to e.g.
Updated in 02cbd09. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I realise this will be impacted by daylight savings... fix incoming. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Set to UTC-5 (EST / non-daylight-savings) in caac016. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't be affected by daylight savings as the dates are consistent. There was a problem hiding this comment. Choose a reason for hiding this commentThe 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', | ||||
} ), | ||||
] ) | ||||
|
@@ -413,7 +413,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', | ||||
} ), | ||||
] ) | ||||
|
@@ -444,7 +444,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', | ||||
} ), | ||||
] ) | ||||
|
There was a problem hiding this comment.
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.