Commit 26055e39755 for woocommerce
commit 26055e397557b942648a388defb388e7cae1d0c0
Author: Peter Petrov <peter.petrov89@gmail.com>
Date: Wed Sep 16 15:08:03 2026 +0300
Fix analytics chart previous year data offset by a leap day (#68497)
* Fix analytics chart previous year data offset by a leap day
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Fold the previous year 29th Feb into the 28th on day charts
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Skip the leap day fold when both ranges already have it aligned
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Remove the unused dataContainsLeapYear chart helper
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Limit the leap day chart alignment to previous year comparisons
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Match comparison chart days by date instead of array position
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Drop chart leap day tests the range matrix already covers
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Keep positional chart labels and refuse to fold percent metrics
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Include the secondary shift in the getCurrentDates memo key
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* End a year shifted current period on the same calendar day a year earlier
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Show the clamped previous year day when a chart range starts on 29th Feb
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Name the previous year comparison range fix in the changelogs
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Name every surface the previous year comparison range fix moves
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Shorten the changelog entries to one sentence
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Shift comparison dates by calendar date so a DST change cannot drop a day
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Accept any moment input in getPreviousDate like its callers already pass
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Build the DST test fixtures with parseZone so they hold in every browser zone
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
* Derive the previous period of the last year from the store clock
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
---------
Co-authored-by: Claude Fable 5.1 <noreply@anthropic.com>
diff --git a/packages/js/components/changelog/fix-wooplug-2204-chart-label-date-end b/packages/js/components/changelog/fix-wooplug-2204-chart-label-date-end
new file mode 100644
index 00000000000..ae1169110eb
--- /dev/null
+++ b/packages/js/components/changelog/fix-wooplug-2204-chart-label-date-end
@@ -0,0 +1,4 @@
+Significance: minor
+Type: add
+
+Add support for an optional `labelDateEnd` on Chart data points so a tooltip and screen reader label can describe a point covering a range of dates.
diff --git a/packages/js/components/src/chart/d3chart/utils/bar-chart.js b/packages/js/components/src/chart/d3chart/utils/bar-chart.js
index e4f1993d0b5..3479b5cf628 100644
--- a/packages/js/components/src/chart/d3chart/utils/bar-chart.js
+++ b/packages/js/components/src/chart/d3chart/utils/bar-chart.js
@@ -5,6 +5,11 @@ import { get } from 'lodash';
import { event as d3Event } from 'd3-selection';
import moment from 'moment';
+/**
+ * Internal dependencies
+ */
+import { getDateLabel } from './index';
+
export const drawBars = ( node, data, params, scales, formats, tooltip ) => {
const height = scales.yScale.range()[ 0 ];
const barGroup = node
@@ -78,8 +83,10 @@ export const drawBars = ( node, data, params, scales, formats, tooltip ) => {
let label = d.label || d.key;
if ( params.mode === 'time-comparison' ) {
const dayData = data.find( ( e ) => e.date === d.date );
- label = formats.screenReaderFormat(
- moment( dayData[ d.key ].labelDate ).toDate()
+ label = getDateLabel(
+ formats.screenReaderFormat,
+ dayData[ d.key ].labelDate,
+ dayData[ d.key ].labelDateEnd
);
}
return `${ label } ${ tooltip.valueFormat( d.value ) }`;
diff --git a/packages/js/components/src/chart/d3chart/utils/index.js b/packages/js/components/src/chart/d3chart/utils/index.js
index 7ed84a50752..9fb88dce7e7 100644
--- a/packages/js/components/src/chart/d3chart/utils/index.js
+++ b/packages/js/components/src/chart/d3chart/utils/index.js
@@ -4,6 +4,7 @@
import { isNil } from 'lodash';
import { format as d3Format } from 'd3-format';
import { utcParse as d3UTCParse } from 'd3-time-format';
+import moment from 'moment';
/**
* Allows an overriding formatter or defaults to d3Format or d3TimeFormat
@@ -61,6 +62,26 @@ export const getUniqueDates = ( data, dateParser ) => {
return [ ...dates ].sort( ( a, b ) => parseDate( a ) - parseDate( b ) );
};
+/**
+ * Formats the date label of a data point. A point can cover a range of dates,
+ * for example a previous year 28th and 29th February folded into one point,
+ * in which case both ends are formatted and joined.
+ *
+ * @param {Function} formatter - date formatting function.
+ * @param {Date|string} labelDate - date of the data point.
+ * @param {Date|string} labelDateEnd - optional last date the data point covers.
+ * @return {string} Formatted label.
+ */
+export const getDateLabel = ( formatter, labelDate, labelDateEnd ) => {
+ const toDate = ( date ) =>
+ date instanceof Date ? date : moment( date ).toDate();
+ const label = formatter( toDate( labelDate ) );
+ if ( labelDateEnd ) {
+ return `${ label } - ${ formatter( toDate( labelDateEnd ) ) }`;
+ }
+ return label;
+};
+
/**
* Check whether data is empty.
*
diff --git a/packages/js/components/src/chart/d3chart/utils/line-chart.js b/packages/js/components/src/chart/d3chart/utils/line-chart.js
index dd508eca683..5389f5d3484 100644
--- a/packages/js/components/src/chart/d3chart/utils/line-chart.js
+++ b/packages/js/components/src/chart/d3chart/utils/line-chart.js
@@ -10,6 +10,7 @@ import { first, get } from 'lodash';
* Internal dependencies
*/
import { smallBreak, wideBreak } from './breakpoints';
+import { getDateLabel } from './index';
/**
* Describes getDateSpaces
@@ -97,6 +98,7 @@ export const getLineData = ( data, orderedKeys ) =>
date: d.date,
// To have actual date for the screenReader, we need to use label date.
labelDate: d[ row.key ].labelDate,
+ labelDateEnd: d[ row.key ].labelDateEnd,
focus: row.focus,
value: get( d, [ row.key, 'value' ], 0 ),
visible: row.visible,
@@ -174,10 +176,10 @@ export const drawLines = ( node, data, params, scales, formats, tooltip ) => {
.attr( 'tabindex', '0' )
.attr( 'role', 'graphics-symbol' )
.attr( 'aria-label', ( d ) => {
- const label = formats.screenReaderFormat(
- d.labelDate instanceof Date
- ? d.labelDate
- : moment( d.labelDate ).toDate()
+ const label = getDateLabel(
+ formats.screenReaderFormat,
+ d.labelDate,
+ d.labelDateEnd
);
return `${ label } ${ tooltip.valueFormat( d.value ) }`;
} )
diff --git a/packages/js/components/src/chart/d3chart/utils/test/index.js b/packages/js/components/src/chart/d3chart/utils/test/index.js
index 791591885dc..c6af45e2487 100644
--- a/packages/js/components/src/chart/d3chart/utils/test/index.js
+++ b/packages/js/components/src/chart/d3chart/utils/test/index.js
@@ -8,11 +8,38 @@ import { utcParse as d3UTCParse } from 'd3-time-format';
*/
import dummyOrders from './fixtures/dummy-orders';
import orderedKeys from './fixtures/dummy-ordered-keys';
-import { getOrderedKeys, isDataEmpty } from '../index';
+import { getDateLabel, getOrderedKeys, isDataEmpty } from '../index';
const parseDate = d3UTCParse( '%Y-%m-%dT%H:%M:%S' );
const testOrderedKeys = getOrderedKeys( dummyOrders );
+describe( 'getDateLabel', () => {
+ const formatter = ( date ) =>
+ `${ date.getFullYear() }-${ date.getMonth() + 1 }-${ date.getDate() }`;
+
+ it( 'formats a single date given as a string', () => {
+ expect( getDateLabel( formatter, '2020-02-28 00:00:00' ) ).toEqual(
+ '2020-2-28'
+ );
+ } );
+
+ it( 'formats a single date given as a Date', () => {
+ expect( getDateLabel( formatter, new Date( 2020, 1, 28 ) ) ).toEqual(
+ '2020-2-28'
+ );
+ } );
+
+ it( 'joins both ends when the point covers a date range', () => {
+ expect(
+ getDateLabel(
+ formatter,
+ '2020-02-28 00:00:00',
+ '2020-02-29 00:00:00'
+ )
+ ).toEqual( '2020-2-28 - 2020-2-29' );
+ } );
+} );
+
describe( 'parseDate', () => {
it( 'correctly parse date in the expected format', () => {
const testDate = parseDate( '2018-06-30T00:00:00' );
diff --git a/packages/js/components/src/chart/d3chart/utils/tooltip.js b/packages/js/components/src/chart/d3chart/utils/tooltip.js
index c5f5ca1c6fc..35efb73daf0 100644
--- a/packages/js/components/src/chart/d3chart/utils/tooltip.js
+++ b/packages/js/components/src/chart/d3chart/utils/tooltip.js
@@ -4,6 +4,11 @@
import { select as d3Select } from 'd3-selection';
import moment from 'moment';
+/**
+ * Internal dependencies
+ */
+import { getDateLabel } from './index';
+
class ChartTooltip {
constructor() {
this.ref = null;
@@ -109,10 +114,9 @@ class ChartTooltip {
}
getTooltipRowLabel( d, row ) {
- if ( d[ row.key ].labelDate ) {
- return this.labelFormat(
- moment( d[ row.key ].labelDate ).toDate()
- );
+ const { labelDate, labelDateEnd } = d[ row.key ];
+ if ( labelDate ) {
+ return getDateLabel( this.labelFormat, labelDate, labelDateEnd );
}
return row.label || row.key;
}
diff --git a/packages/js/date/changelog/add-wooplug-2204-secondary-range-shift b/packages/js/date/changelog/add-wooplug-2204-secondary-range-shift
new file mode 100644
index 00000000000..9d57b69f251
--- /dev/null
+++ b/packages/js/date/changelog/add-wooplug-2204-secondary-range-shift
@@ -0,0 +1,4 @@
+Significance: minor
+Type: fix
+
+Fix to date previous year ranges ending a day early or late around 29th February, and expose how the secondary range was derived via `shift`. `getPreviousDate` now shifts by calendar dates, so previous period ranges starting on different sides of a DST change no longer come out a day short, and the previous period of the last year follows the store clock rather than the browser clock.
diff --git a/packages/js/date/src/index.ts b/packages/js/date/src/index.ts
index 3d14066b500..a4890864ad2 100644
--- a/packages/js/date/src/index.ts
+++ b/packages/js/date/src/index.ts
@@ -19,32 +19,46 @@ export const defaultDateTimeFormat = 'YYYY-MM-DDTHH:mm:ss';
* DateValue Object
*
* @typedef {Object} DateValue - DateValue data about the selected period.
- * @property {moment.Moment} primaryStart - Primary start of the date range.
- * @property {moment.Moment} primaryEnd - Primary end of the date range.
- * @property {moment.Moment} secondaryStart - Secondary start of the date range.
- * @property {moment.Moment} secondaryEnd - Secondary End of the date range.
+ * @property {moment.Moment} primaryStart - Primary start of the date range.
+ * @property {moment.Moment} primaryEnd - Primary end of the date range.
+ * @property {moment.Moment} secondaryStart - Secondary start of the date range.
+ * @property {moment.Moment} secondaryEnd - Secondary End of the date range.
+ * @property {SecondaryShift} [secondaryShift] - How the secondary range was derived from the primary one.
*/
export type DateValue = {
primaryStart: moment.Moment;
primaryEnd: moment.Moment;
secondaryStart: moment.Moment;
secondaryEnd: moment.Moment;
+ secondaryShift?: SecondaryShift;
};
+/**
+ * How a secondary (comparison) date range relates to the primary one.
+ *
+ * `year`: the same calendar dates one year earlier, so a primary date maps to
+ * the secondary date with the same month and day.
+ * `offset`: shifted back by the distance between the two range starts, so a
+ * primary date maps to the secondary date the same number of days earlier.
+ */
+export type SecondaryShift = 'year' | 'offset';
+
/**
* DataPickerOptions Object
*
* @typedef {Object} DataPickerOptions - Describes the date range supplied by the date picker.
- * @property {string} label - The translated value of the period.
- * @property {string} range - The human readable value of a date range.
- * @property {moment.Moment} after - Start of the date range.
- * @property {moment.Moment} before - End of the date range.
+ * @property {string} label - The translated value of the period.
+ * @property {string} range - The human readable value of a date range.
+ * @property {moment.Moment} after - Start of the date range.
+ * @property {moment.Moment} before - End of the date range.
+ * @property {SecondaryShift} [shift] - Secondary range only: how it was derived from the primary one.
*/
export type DataPickerOptions = {
label: string;
range: string;
after: moment.Moment;
before: moment.Moment;
+ shift?: SecondaryShift;
};
/**
@@ -418,6 +432,7 @@ function anchorRangeToStoreTimeZone( range: DateValue ): DateValue {
primaryEnd: anchorToStoreTimeZone( range.primaryEnd ),
secondaryStart: anchorToStoreTimeZone( range.secondaryStart ),
secondaryEnd: anchorToStoreTimeZone( range.secondaryEnd ),
+ secondaryShift: range.secondaryShift,
};
}
@@ -468,11 +483,15 @@ export function getLastPeriod(
const primaryEnd = primaryStart.clone().endOf( period );
let secondaryStart;
let secondaryEnd;
+ let secondaryShift: SecondaryShift = 'year';
if ( compare === 'previous_period' ) {
if ( period === 'year' ) {
- // Subtract two entire periods for years to take into account leap year
- secondaryStart = moment().startOf( period ).subtract( 2, period );
+ // Subtract a whole year rather than the primary day count, so a leap
+ // year cannot shift the range. Derive it from the primary start: the
+ // browser clock can sit in a different year than the store clock
+ // around New Year.
+ secondaryStart = primaryStart.clone().subtract( 1, period );
secondaryEnd = secondaryStart.clone().endOf( period );
} else {
// Otherwise, use days in primary period to figure out how far to go back
@@ -480,6 +499,7 @@ export function getLastPeriod(
const daysDiff = primaryEnd.diff( primaryStart, 'days' );
secondaryEnd = primaryStart.clone().subtract( 1, 'days' );
secondaryStart = secondaryEnd.clone().subtract( daysDiff, 'days' );
+ secondaryShift = 'offset';
}
} else if ( period === 'week' ) {
secondaryStart = primaryStart.clone().subtract( 1, 'years' );
@@ -499,6 +519,7 @@ export function getLastPeriod(
primaryEnd,
secondaryStart,
secondaryEnd,
+ secondaryShift,
} );
}
@@ -519,26 +540,26 @@ export function getCurrentPeriod(
const primaryStart = getStoreTimeZoneMoment().startOf( period );
const primaryEnd = getStoreTimeZoneMoment();
- const daysSoFar = primaryEnd.diff( primaryStart, 'days' );
let secondaryStart;
let secondaryEnd;
+ let secondaryShift: SecondaryShift = 'year';
if ( compare === 'previous_period' ) {
secondaryStart = primaryStart.clone().subtract( 1, period );
secondaryEnd = primaryEnd.clone().subtract( 1, period );
+ if ( period !== 'year' ) {
+ secondaryShift = 'offset';
+ }
} else {
secondaryStart = primaryStart.clone().subtract( 1, 'years' );
- // Set the end time to 23:59:59.
- secondaryEnd = secondaryStart
- .clone()
- .add( daysSoFar + 1, 'days' )
- .subtract( 1, 'seconds' );
+ secondaryEnd = primaryEnd.clone().subtract( 1, 'years' ).endOf( 'day' );
}
return anchorRangeToStoreTimeZone( {
primaryStart,
primaryEnd,
secondaryStart,
secondaryEnd,
+ secondaryShift,
} );
}
@@ -597,6 +618,7 @@ const getDateValue = memoize<
primaryEnd: before,
secondaryStart,
secondaryEnd,
+ secondaryShift: 'offset',
};
}
return {
@@ -604,6 +626,7 @@ const getDateValue = memoize<
primaryEnd: before,
secondaryStart: after.clone().subtract( 1, 'years' ),
secondaryEnd: before.clone().subtract( 1, 'years' ),
+ secondaryShift: 'year',
};
}
},
@@ -727,6 +750,7 @@ export const getDateParamsFromQuery = (
* @param {Object} primaryEnd - primary query start DateTime, in Moment instance.
* @param {Object} secondaryStart - secondary query start DateTime, in Moment instance.
* @param {Object} secondaryEnd - secondary query start DateTime, in Moment instance.
+ * @param {SecondaryShift} secondaryShift - how the secondary range was derived from the primary one.
* @return {{primary: DataPickerOptions, secondary: DataPickerOptions}} - Primary and secondary DataPickerOptions objects
*/
const getCurrentDatesMemoized = memoize<
@@ -738,6 +762,7 @@ const getCurrentDatesMemoized = memoize<
moment.Moment,
moment.Moment,
moment.Moment,
+ SecondaryShift | undefined,
]
) => {
primary: DataPickerOptions;
@@ -750,7 +775,8 @@ const getCurrentDatesMemoized = memoize<
primaryStart,
primaryEnd,
secondaryStart,
- secondaryEnd
+ secondaryEnd,
+ secondaryShift
) => {
const primaryItem = find(
presetValues,
@@ -779,6 +805,7 @@ const getCurrentDatesMemoized = memoize<
range: getRangeLabel( secondaryStart, secondaryEnd ),
after: secondaryStart,
before: secondaryEnd,
+ shift: secondaryShift,
},
};
},
@@ -788,7 +815,8 @@ const getCurrentDatesMemoized = memoize<
primaryStart,
primaryEnd,
secondaryStart,
- secondaryEnd
+ secondaryEnd,
+ secondaryShift
) =>
[
period,
@@ -797,6 +825,7 @@ const getCurrentDatesMemoized = memoize<
primaryEnd && primaryEnd.format(),
secondaryStart && secondaryStart.format(),
secondaryEnd && secondaryEnd.format(),
+ secondaryShift,
].join( ':' )
);
@@ -826,8 +855,13 @@ export const getCurrentDates = (
throw Error( 'Invalid date range' );
}
- const { primaryStart, primaryEnd, secondaryStart, secondaryEnd } =
- dateValue;
+ const {
+ primaryStart,
+ primaryEnd,
+ secondaryStart,
+ secondaryEnd,
+ secondaryShift,
+ } = dateValue;
return getCurrentDatesMemoized(
period,
@@ -835,7 +869,8 @@ export const getCurrentDates = (
primaryStart,
primaryEnd,
secondaryStart,
- secondaryEnd
+ secondaryEnd,
+ secondaryShift
);
};
@@ -858,31 +893,42 @@ export const getDateDifferenceInDays = (
/**
* Get the previous date for either the previous period of year.
*
- * @param {string} date - Base date
- * @param {string} date1 - primary start
- * @param {string} date2 - secondary start
+ * @param {moment.MomentInput} date - Base date
+ * @param {moment.MomentInput} date1 - primary start
+ * @param {moment.MomentInput} date2 - secondary start
* @param {string} compare - `previous_period` or `previous_year`
* @param {moment.unitOfTime.Diff} interval - interval
+ * @param {SecondaryShift} [shift] - how the secondary range was derived, see `DataPickerOptions.shift`. Takes precedence over `compare` when given.
* @return {Object} - Calculated date
*/
export const getPreviousDate = (
- date: string,
- date1: string,
- date2: string,
+ date: moment.MomentInput,
+ date1: moment.MomentInput,
+ date2: moment.MomentInput,
compare = 'previous_year',
- interval: moment.unitOfTime.Diff | moment.DurationInputArg2
+ interval: moment.unitOfTime.Diff | moment.DurationInputArg2,
+ shift?: SecondaryShift
) => {
const dateMoment = moment( date );
+ const yearShifted = shift ? shift === 'year' : compare === 'previous_year';
- if ( compare === 'previous_year' ) {
+ if ( yearShifted ) {
return dateMoment.clone().subtract( 1, 'years' );
}
- const _date1 = moment( date1 );
- const _date2 = moment( date2 );
- const difference = _date1.diff( _date2, interval );
-
- return dateMoment.clone().subtract( difference, interval );
+ // Range boundaries are anchored to their own UTC offset, so two starts on
+ // different sides of a DST change differ by an hour as instants and the
+ // diff floors to one interval short. Compare and shift the wall-clock
+ // dates instead, then return a local moment like the year shift does.
+ const wallClock = ( value: moment.MomentInput ) =>
+ moment.utc( moment( value ).format( defaultDateTimeFormat ) );
+ const difference = wallClock( date1 ).diff( wallClock( date2 ), interval );
+
+ return moment(
+ wallClock( date )
+ .subtract( difference, interval )
+ .format( defaultDateTimeFormat )
+ );
};
/**
diff --git a/packages/js/date/src/test/index.ts b/packages/js/date/src/test/index.ts
index b6a790d5ed3..1de3cd60128 100644
--- a/packages/js/date/src/test/index.ts
+++ b/packages/js/date/src/test/index.ts
@@ -1516,7 +1516,158 @@ describe( 'getDateDifferenceInDays', () => {
} );
} );
+describe( 'secondary range shift', () => {
+ afterEach( () => {
+ jest.useRealTimers();
+ } );
+
+ it( 'is a year shift for previous year and for the year presets, an offset otherwise', () => {
+ jest.useFakeTimers().setSystemTime( new Date( '2025-03-15T12:00:00' ) );
+
+ expect( getLastPeriod( 'month', 'previous_year' ).secondaryShift ).toBe(
+ 'year'
+ );
+ expect(
+ getCurrentPeriod( 'week', 'previous_year' ).secondaryShift
+ ).toBe( 'year' );
+ expect(
+ getLastPeriod( 'year', 'previous_period' ).secondaryShift
+ ).toBe( 'year' );
+ expect(
+ getCurrentPeriod( 'year', 'previous_period' ).secondaryShift
+ ).toBe( 'year' );
+ expect(
+ getLastPeriod( 'month', 'previous_period' ).secondaryShift
+ ).toBe( 'offset' );
+ expect(
+ getCurrentPeriod( 'quarter', 'previous_period' ).secondaryShift
+ ).toBe( 'offset' );
+ } );
+
+ it( 'ends a year shifted current period on the same calendar day a year earlier', () => {
+ jest.useFakeTimers().setSystemTime( new Date( '2025-06-15T12:00:00' ) );
+ const { secondaryStart, secondaryEnd } = getCurrentPeriod(
+ 'year',
+ 'previous_year'
+ );
+ expect( secondaryStart.format( isoDateFormat ) ).toBe( '2024-01-01' );
+ expect( secondaryEnd.format( 'YYYY-MM-DD HH:mm:ss' ) ).toBe(
+ '2024-06-15 23:59:59'
+ );
+ } );
+
+ it( 'keeps the previous period of the last year on the store clock around New Year', () => {
+ // 09:30 UTC on 1st January: every browser zone east of -09:30 is
+ // already in 2027 while a Honolulu store is still on 31st December.
+ jest.useFakeTimers().setSystemTime(
+ new Date( '2027-01-01T09:30:00Z' )
+ );
+ const previousWcSettings = global.window.wcSettings;
+ global.window.wcSettings = {
+ ...previousWcSettings,
+ timeZone: 'Pacific/Honolulu',
+ };
+
+ try {
+ const { primaryStart, secondaryStart, secondaryEnd } =
+ getLastPeriod( 'year', 'previous_period' );
+ expect( primaryStart.format( isoDateFormat ) ).toBe( '2025-01-01' );
+ expect( secondaryStart.format( isoDateFormat ) ).toBe(
+ '2024-01-01'
+ );
+ expect( secondaryEnd.format( 'YYYY-MM-DD HH:mm:ss' ) ).toBe(
+ '2024-12-31 23:59:59'
+ );
+ } finally {
+ global.window.wcSettings = previousWcSettings;
+ }
+ } );
+
+ it( 'is exposed on the secondary date picker options', () => {
+ jest.useFakeTimers().setSystemTime( new Date( '2026-09-09T12:00:00' ) );
+ expect(
+ getCurrentDates( {
+ period: 'last_year',
+ compare: 'previous_period',
+ } ).secondary.shift
+ ).toBe( 'year' );
+
+ const custom = {
+ period: 'custom',
+ after: '2024-12-01',
+ before: '2025-12-01',
+ };
+
+ expect(
+ getCurrentDates( { ...custom, compare: 'previous_year' } ).secondary
+ .shift
+ ).toBe( 'year' );
+ expect(
+ getCurrentDates( { ...custom, compare: 'previous_period' } )
+ .secondary.shift
+ ).toBe( 'offset' );
+ } );
+} );
+
describe( 'getPreviousDate', () => {
+ it( 'should use the shift over the compare value when given', () => {
+ const yearShifted = getPreviousDate(
+ '2024-03-01',
+ '2024-01-01',
+ '2023-01-01',
+ 'previous_period',
+ 'day',
+ 'year'
+ );
+ expect( yearShifted.format( isoDateFormat ) ).toBe( '2023-03-01' );
+
+ const offset = getPreviousDate(
+ '2025-03-01',
+ '2024-12-01',
+ '2023-12-01',
+ 'previous_year',
+ 'day',
+ 'offset'
+ );
+ expect( offset.format( isoDateFormat ) ).toBe( '2024-02-29' );
+ } );
+ it( 'should shift by calendar dates when the range starts sit on different sides of a DST change', () => {
+ // Last quarter (Q2) against the previous period on a New York store:
+ // April starts in EDT, the last day of December in EST.
+ const primaryStart = moment.parseZone( '2026-04-01T00:00:00-04:00' );
+ const secondaryStart = moment.parseZone( '2025-12-31T00:00:00-05:00' );
+
+ expect(
+ getPreviousDate(
+ '2026-04-01 00:00:00',
+ primaryStart,
+ secondaryStart,
+ 'previous_period',
+ 'day',
+ 'offset'
+ ).format( isoDateFormat )
+ ).toBe( '2025-12-31' );
+ expect(
+ getPreviousDate(
+ '2026-04-01 00:00:00',
+ primaryStart,
+ secondaryStart,
+ 'previous_period',
+ 'week'
+ ).format( isoDateFormat )
+ ).toBe( '2025-12-31' );
+
+ // Quarter to date against the previous period: April against January.
+ expect(
+ getPreviousDate(
+ '2026-05-01 00:00:00',
+ primaryStart,
+ moment.parseZone( '2026-01-01T00:00:00-05:00' ),
+ 'previous_period',
+ 'month'
+ ).format( isoDateFormat )
+ ).toBe( '2026-02-01' );
+ } );
it( 'should return valid date for previous period by days', () => {
const date = '2018-08-21';
const primaryStart = '2018-08-25';
diff --git a/plugins/woocommerce/changelog/fix-wooplug-2204-analytics-leap-day-previous-year b/plugins/woocommerce/changelog/fix-wooplug-2204-analytics-leap-day-previous-year
new file mode 100644
index 00000000000..56d13b5068d
--- /dev/null
+++ b/plugins/woocommerce/changelog/fix-wooplug-2204-analytics-leap-day-previous-year
@@ -0,0 +1,4 @@
+Significance: patch
+Type: fix
+
+Fix analytics previous year comparisons around 29th February: chart days no longer shift by a day, to date ranges compare against the same calendar dates a year earlier, and previous period day charts line up across a DST change.
diff --git a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/index.js b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/index.js
index 573cb5d4587..6cd97d28831 100644
--- a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/index.js
+++ b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/index.js
@@ -102,7 +102,8 @@ export class ReportChart extends Component {
secondary,
query.compare,
selectedChart.key,
- currentInterval
+ currentInterval,
+ selectedChart.type
);
}
diff --git a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/test/utils.js b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/test/utils.js
index 2de19062fd1..0c4401e5d77 100644
--- a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/test/utils.js
+++ b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/test/utils.js
@@ -1,7 +1,19 @@
+/**
+ * External dependencies
+ */
+import moment from 'moment';
+import {
+ getAllowedIntervalsForQuery,
+ getCurrentDates,
+ getCurrentPeriod,
+ getLastPeriod,
+ getPreviousDate,
+} from '@woocommerce/date';
+
/**
* Internal dependencies
*/
-import { buildChartData, dataContainsLeapYear } from '../utils';
+import { buildChartData } from '../utils';
function generateDateInterval( interval, startDate, endDate, subtotals ) {
const subtotalsDefault = {
@@ -25,6 +37,68 @@ function generateDateInterval( interval, startDate, endDate, subtotals ) {
};
}
+function formatDay( date ) {
+ const month = String( date.getMonth() + 1 ).padStart( 2, '0' );
+ const day = String( date.getDate() ).padStart( 2, '0' );
+ return `${ date.getFullYear() }-${ month }-${ day }`;
+}
+
+function generateDayIntervals(
+ startDate,
+ endDate,
+ valuesByDate = {},
+ key = 'orders_count'
+) {
+ const intervals = [];
+ const day = new Date( `${ startDate }T00:00:00` );
+ const end = new Date( `${ endDate }T00:00:00` );
+ while ( day <= end ) {
+ const date = formatDay( day );
+ intervals.push(
+ generateDateInterval( date, date, date, {
+ [ key ]: valuesByDate[ date ] || 0,
+ } )
+ );
+ day.setDate( day.getDate() + 1 );
+ }
+ return intervals;
+}
+
+function buildDayChartData(
+ primaryIntervals,
+ secondaryIntervals,
+ key = 'orders_count',
+ type = 'number'
+) {
+ return buildChartData(
+ { data: { totals: {}, intervals: primaryIntervals } },
+ { data: { totals: {}, intervals: secondaryIntervals } },
+ {
+ label: 'Custom',
+ range: '',
+ after: primaryIntervals[ 0 ].date_start,
+ before: '',
+ },
+ {
+ label: 'Previous year',
+ range: '',
+ after: secondaryIntervals[ 0 ].date_start,
+ before: '',
+ shift: 'year',
+ },
+ 'previous_year',
+ key,
+ 'day',
+ type
+ );
+}
+
+function secondaryByDate( chartData, date ) {
+ const entry = chartData.find( ( d ) => d.date === `${ date }T00:00:00` );
+ const { labelDate, labelDateEnd, value } = entry.secondary;
+ return { labelDate, labelDateEnd, value };
+}
+
describe( 'buildChartData', () => {
test( 'should bump up data since 29th Feb for previous year and compare by day', () => {
const primaryData = {
@@ -267,70 +341,469 @@ describe( 'buildChartData', () => {
},
] );
} );
-} );
-describe( 'dataContainsLeapYear', () => {
- it( 'should return false when intervals are empty', () => {
- const data = {
- data: {
- intervals: [],
- },
- };
- expect( dataContainsLeapYear( data ) ).toBe( false );
+ test( 'should fold the 29th Feb of the previous year into the 28th when the primary range has no leap day', () => {
+ const primary = generateDayIntervals( '2021-02-27', '2021-03-02' );
+ const secondary = generateDayIntervals( '2020-02-27', '2020-03-02', {
+ '2020-02-28': 5,
+ '2020-02-29': 1,
+ '2020-03-01': 2,
+ '2020-03-02': 3,
+ } );
+
+ const chartData = buildDayChartData( primary, secondary );
+
+ expect( chartData ).toHaveLength( 4 );
+ expect( secondaryByDate( chartData, '2021-02-27' ) ).toEqual( {
+ labelDate: '2020-02-27 00:00:00',
+ value: 0,
+ } );
+ expect( secondaryByDate( chartData, '2021-02-28' ) ).toEqual( {
+ labelDate: '2020-02-28 00:00:00',
+ labelDateEnd: '2020-02-29 00:00:00',
+ value: 6,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-01' ) ).toEqual( {
+ labelDate: '2020-03-01 00:00:00',
+ value: 2,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-02' ) ).toEqual( {
+ labelDate: '2020-03-02 00:00:00',
+ value: 3,
+ } );
} );
- it( 'should return false when intervals are undefined', () => {
- const data = {
- data: {},
- };
- expect( dataContainsLeapYear( data ) ).toBe( false );
+ test.each( [
+ [ 'avg_items_per_order', 'average' ],
+ [ 'avg_order_value', 'currency' ],
+ [ 'conversion_rate', 'percent' ],
+ ] )(
+ 'should not fold the 29th Feb of the previous year for the %s average',
+ ( key, type ) => {
+ const primary = generateDayIntervals( '2021-02-27', '2021-03-02' );
+ const secondary = generateDayIntervals(
+ '2020-02-27',
+ '2020-03-02',
+ {
+ '2020-02-28': 5,
+ '2020-02-29': 1,
+ '2020-03-01': 2,
+ '2020-03-02': 3,
+ },
+ key
+ );
+
+ const chartData = buildDayChartData(
+ primary,
+ secondary,
+ key,
+ type
+ );
+
+ expect( secondaryByDate( chartData, '2021-02-28' ) ).toEqual( {
+ labelDate: '2020-02-28 00:00:00',
+ value: 5,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-01' ) ).toEqual( {
+ labelDate: '2020-03-01 00:00:00',
+ value: 2,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-02' ) ).toEqual( {
+ labelDate: '2020-03-02 00:00:00',
+ value: 3,
+ } );
+ }
+ );
+
+ test( 'should show zero for a day missing from the comparison data without shifting the rest', () => {
+ const primary = generateDayIntervals( '2021-02-27', '2021-03-02' );
+ const secondary = generateDayIntervals( '2020-02-27', '2020-03-02', {
+ '2020-02-27': 1,
+ '2020-02-28': 2,
+ '2020-02-29': 3,
+ '2020-03-01': 4,
+ '2020-03-02': 5,
+ } ).filter(
+ ( interval ) => ! interval.date_start.startsWith( '2020-02-28' )
+ );
+
+ const chartData = buildDayChartData( primary, secondary );
+
+ expect( secondaryByDate( chartData, '2021-02-27' ) ).toEqual( {
+ labelDate: '2020-02-27 00:00:00',
+ value: 1,
+ } );
+ expect( secondaryByDate( chartData, '2021-02-28' ) ).toEqual( {
+ labelDate: '2020-02-28 00:00:00',
+ value: 0,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-01' ) ).toEqual( {
+ labelDate: '2020-03-01 00:00:00',
+ value: 4,
+ } );
+ expect( secondaryByDate( chartData, '2021-03-02' ) ).toEqual( {
+ labelDate: '2020-03-02 00:00:00',
+ value: 5,
+ } );
} );
+} );
- it( 'should return false when interval does not include a leap year', () => {
- const data = {
- data: {
- intervals: [
- { date_start: '2019-01-01', date_end: '2019-01-01' },
- { date_start: '2019-12-31', date_end: '2019-12-31' },
- ],
- },
+describe( 'buildChartData across every date range shape', () => {
+ // Every day gets a value only it can produce, so a chart value can be
+ // traced back to the day it came from.
+ const valueOf = ( day ) => Number( day.replace( /-/g, '' ) );
+
+ const builders = {
+ today: ( compare ) => getCurrentPeriod( 'day', compare ),
+ yesterday: ( compare ) => getLastPeriod( 'day', compare ),
+ week: ( compare ) => getCurrentPeriod( 'week', compare ),
+ last_week: ( compare ) => getLastPeriod( 'week', compare ),
+ month: ( compare ) => getCurrentPeriod( 'month', compare ),
+ last_month: ( compare ) => getLastPeriod( 'month', compare ),
+ quarter: ( compare ) => getCurrentPeriod( 'quarter', compare ),
+ last_quarter: ( compare ) => getLastPeriod( 'quarter', compare ),
+ year: ( compare ) => getCurrentPeriod( 'year', compare ),
+ last_year: ( compare ) => getLastPeriod( 'year', compare ),
+ };
+ const clocks = [
+ '2021-02-28T12:00:00',
+ '2024-02-29T12:00:00',
+ '2024-03-15T12:00:00',
+ '2025-03-15T12:00:00',
+ '2025-12-31T12:00:00',
+ '2026-01-01T02:00:00',
+ '2026-09-09T12:00:00',
+ ];
+ const customRanges = [
+ [ '2021-01-01', '2021-03-31' ],
+ [ '2021-02-01', '2021-02-28' ],
+ [ '2024-02-01', '2024-03-31' ],
+ [ '2024-01-01', '2025-01-31' ],
+ [ '2024-12-01', '2025-12-01' ],
+ [ '2016-01-01', '2019-12-31' ],
+ ];
+ const compares = [ 'previous_period', 'previous_year' ];
+ // Zones on both hemispheres, so a preset crosses a DST change in some
+ // clock whichever direction the clocks move. A store without a zone keeps
+ // the plain browser time path covered.
+ const storeTimeZones = [
+ undefined,
+ 'America/New_York',
+ 'Australia/Sydney',
+ ];
+
+ function pickerFor( start, end, shift ) {
+ return {
+ label: '',
+ range: '',
+ after: start.clone().startOf( 'day' ),
+ before: end.clone().startOf( 'day' ),
+ shift,
};
- expect( dataContainsLeapYear( data ) ).toBe( false );
+ }
+
+ function daysBetween( picker ) {
+ const days = [];
+ const day = picker.after.clone();
+ while ( ! day.isAfter( picker.before, 'day' ) ) {
+ days.push( day.format( 'YYYY-MM-DD' ) );
+ day.add( 1, 'days' );
+ }
+ return days;
+ }
+
+ function intervalsFor( picker ) {
+ return daysBetween( picker ).map( ( day ) =>
+ generateDateInterval( day, day, day, {
+ orders_count: valueOf( day ),
+ } )
+ );
+ }
+
+ function problemsFor( name, primary, secondary, compare ) {
+ const chartData = buildChartData(
+ { data: { totals: {}, intervals: intervalsFor( primary ) } },
+ { data: { totals: {}, intervals: intervalsFor( secondary ) } },
+ primary,
+ secondary,
+ compare,
+ 'orders_count',
+ 'day',
+ 'number'
+ );
+ const secondaryDays = daysBetween( secondary );
+ const problems = [];
+
+ chartData.forEach( ( point ) => {
+ const { labelDate, labelDateEnd, value } = point.secondary;
+ let expected = 0;
+ [ labelDate, labelDateEnd ]
+ .filter( Boolean )
+ .map( ( date ) => date.slice( 0, 10 ) )
+ .forEach( ( day ) => {
+ if ( secondaryDays.includes( day ) ) {
+ expected += valueOf( day );
+ }
+ } );
+ if ( value !== expected ) {
+ problems.push(
+ `${ name } ${
+ point.date
+ }: shows ${ value } under "${ labelDate }${
+ labelDateEnd ? ` - ${ labelDateEnd }` : ''
+ }", expected ${ expected }`
+ );
+ }
+ } );
+
+ // No comparison day up to the last labelled one may go missing. Under
+ // a year shift the 29th Feb right after the last label is folded into
+ // it, so it counts too.
+ const labels = chartData
+ .map( ( point ) => point.secondary.labelDate )
+ .filter( ( labelDate ) => labelDate !== '-' )
+ .map( ( labelDate ) => labelDate.slice( 0, 10 ) )
+ .sort();
+ const lastLabel = labels[ labels.length - 1 ] || '';
+ // A comparison range labelled entirely outside its own days would pass
+ // the total check below with zero on both sides.
+ if ( labels.length && ! secondaryDays.includes( labels[ 0 ] ) ) {
+ problems.push(
+ `${ name }: first label ${
+ labels[ 0 ]
+ } is outside the comparison range ${
+ secondaryDays[ 0 ]
+ } - ${ secondaryDays.at( -1 ) }`
+ );
+ }
+ const yearShifted = secondary.shift === 'year';
+ const expectedTotal = secondaryDays
+ .filter(
+ ( day ) =>
+ day <= lastLabel ||
+ ( yearShifted &&
+ day.slice( 5 ) === '02-29' &&
+ moment( day )
+ .subtract( 1, 'days' )
+ .format( 'YYYY-MM-DD' ) <= lastLabel )
+ )
+ .reduce( ( total, day ) => total + valueOf( day ), 0 );
+ const chartTotal = chartData.reduce(
+ ( total, point ) => total + point.secondary.value,
+ 0
+ );
+ if ( chartTotal !== expectedTotal ) {
+ problems.push(
+ `${ name }: chart total ${ chartTotal }, comparison days total ${ expectedTotal }`
+ );
+ }
+
+ return problems;
+ }
+
+ const originalSettings = global.window.wcSettings;
+
+ afterEach( () => {
+ jest.useRealTimers();
+ global.window.wcSettings = originalSettings;
} );
- // Test with multiple intervals where none include a leap year
- it( 'should return false when no intervals include a leap year', () => {
- const data = {
- data: {
- intervals: [
- { date_start: '2019-01-01', date_end: '2019-06-30' },
- { date_start: '2019-07-01', date_end: '2019-12-31' },
- ],
- },
- };
- expect( dataContainsLeapYear( data ) ).toBe( false );
+ test( 'every preset at every clock and store time zone shows each comparison value under its own date', () => {
+ const problems = [];
+
+ storeTimeZones.forEach( ( timeZone ) => {
+ global.window.wcSettings = { ...originalSettings, timeZone };
+ clocks.forEach( ( clock ) => {
+ jest.useFakeTimers().setSystemTime( new Date( clock ) );
+ Object.entries( builders ).forEach( ( [ preset, build ] ) => {
+ compares.forEach( ( compare ) => {
+ const range = build( compare );
+ problems.push(
+ ...problemsFor(
+ `${ preset }/${ compare } at ${ clock } in ${
+ timeZone || 'browser time'
+ }`,
+ pickerFor(
+ range.primaryStart,
+ range.primaryEnd
+ ),
+ pickerFor(
+ range.secondaryStart,
+ range.secondaryEnd,
+ range.secondaryShift
+ ),
+ compare
+ )
+ );
+ } );
+ } );
+ } );
+ } );
+
+ expect( problems ).toEqual( [] );
} );
- // Test with multiple intervals where one includes a leap year
- it( 'should return true when any interval includes a leap year', () => {
- const data = {
- data: {
- intervals: [
- { date_start: '2020-01-01', date_end: '2020-01-01' },
- { date_start: '2020-01-02', date_end: '2020-01-02' },
- ],
- },
- };
- expect( dataContainsLeapYear( data ) ).toBe( true );
+ test( 'pins a comparison value to a hardcoded calendar day', () => {
+ jest.useFakeTimers().setSystemTime( new Date( '2026-09-09T12:00:00' ) );
+ const range = builders.last_year( 'previous_period' );
+ const primary = pickerFor( range.primaryStart, range.primaryEnd );
+ const secondary = pickerFor(
+ range.secondaryStart,
+ range.secondaryEnd,
+ range.secondaryShift
+ );
+ const chartData = buildChartData(
+ { data: { totals: {}, intervals: intervalsFor( primary ) } },
+ { data: { totals: {}, intervals: intervalsFor( secondary ) } },
+ primary,
+ secondary,
+ 'previous_period',
+ 'orders_count',
+ 'day',
+ 'number'
+ );
+
+ expect( secondaryByDate( chartData, '2025-03-01' ) ).toEqual( {
+ labelDate: '2024-03-01 00:00:00',
+ value: 20240301,
+ } );
+ expect( secondaryByDate( chartData, '2025-12-31' ) ).toEqual( {
+ labelDate: '2024-12-31 00:00:00',
+ value: 20241231,
+ } );
} );
- // Test with malformed date formats
- it( 'should handle invalid date formats gracefully', () => {
- const data = {
- data: {
- intervals: [ { date_start: null, date_end: '2020-99-99' } ],
- },
- };
- expect( dataContainsLeapYear( data ) ).toBe( false );
+ test( 'every custom range shows each comparison value under its own date', () => {
+ const problems = [];
+
+ customRanges.forEach( ( [ after, before ] ) => {
+ compares.forEach( ( compare ) => {
+ const { primary, secondary } = getCurrentDates( {
+ period: 'custom',
+ compare,
+ after,
+ before,
+ } );
+ problems.push(
+ ...problemsFor(
+ `custom ${ after }..${ before }/${ compare }`,
+ primary,
+ secondary,
+ compare
+ )
+ );
+ } );
+ } );
+
+ expect( problems ).toEqual( [] );
+ } );
+
+ // Intervals coarser than a day are matched by position, so every point
+ // must be labelled with the interval it was actually plotted from.
+ function bucketIntervals( start, end, interval ) {
+ const intervals = [];
+ let bucketStart = start.clone();
+ while ( ! bucketStart.isAfter( end ) ) {
+ const nextStart = bucketStart
+ .clone()
+ .startOf( interval )
+ .add( 1, interval );
+ intervals.push( {
+ date_start: bucketStart.format( 'YYYY-MM-DD HH:mm:ss' ),
+ date_end: moment
+ .min( nextStart.clone().subtract( 1, 'seconds' ), end )
+ .format( 'YYYY-MM-DD HH:mm:ss' ),
+ subtotals: {
+ orders_count: Number( bucketStart.format( 'YYYYMMDDHH' ) ),
+ },
+ } );
+ bucketStart = nextStart;
+ }
+ return intervals;
+ }
+
+ test( 'every preset at every coarser interval labels the interval it plots', () => {
+ const problems = [];
+
+ clocks.forEach( ( clock ) => {
+ jest.useFakeTimers().setSystemTime( new Date( clock ) );
+ Object.entries( builders ).forEach( ( [ preset, build ] ) => {
+ compares.forEach( ( compare ) => {
+ const range = build( compare );
+ const primary = {
+ label: '',
+ range: '',
+ after: range.primaryStart,
+ before: range.primaryEnd,
+ };
+ const secondary = {
+ label: '',
+ range: '',
+ after: range.secondaryStart,
+ before: range.secondaryEnd,
+ shift: range.secondaryShift,
+ };
+ getAllowedIntervalsForQuery( { period: preset, compare } )
+ .filter( ( interval ) => interval !== 'day' )
+ .forEach( ( interval ) => {
+ const secondaryIntervals = bucketIntervals(
+ secondary.after,
+ secondary.before,
+ interval
+ );
+ const chartData = buildChartData(
+ {
+ data: {
+ totals: {},
+ intervals: bucketIntervals(
+ primary.after,
+ primary.before,
+ interval
+ ),
+ },
+ },
+ {
+ data: {
+ totals: {},
+ intervals: secondaryIntervals,
+ },
+ },
+ primary,
+ secondary,
+ compare,
+ 'orders_count',
+ interval,
+ 'number'
+ );
+ chartData.forEach( ( point, index ) => {
+ const plotted = secondaryIntervals[ index ];
+ // Positional intervals keep the label they always had:
+ // the compare based one, with no shift applied.
+ const expectedLabel = getPreviousDate(
+ point.primary.labelDate,
+ primary.after,
+ secondary.after,
+ compare,
+ interval
+ ).format( 'YYYY-MM-DD HH:mm:ss' );
+ const expectedValue = plotted
+ ? plotted.subtotals.orders_count
+ : 0;
+ if (
+ point.secondary.labelDate !==
+ expectedLabel ||
+ point.secondary.value !== expectedValue
+ ) {
+ problems.push(
+ `${ preset }/${ compare }/${ interval } at ${ clock } ${ point.date }: label ${ point.secondary.labelDate }, value ${ point.secondary.value }, expected ${ expectedLabel }`
+ );
+ }
+ } );
+ } );
+ } );
+ } );
+ } );
+
+ expect( problems ).toEqual( [] );
} );
} );
diff --git a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/utils.js b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/utils.js
index 92a8426e03f..f9b04253d7d 100644
--- a/plugins/woocommerce/client/admin/client/analytics/components/report-chart/utils.js
+++ b/plugins/woocommerce/client/admin/client/analytics/components/report-chart/utils.js
@@ -2,13 +2,10 @@
* External dependencies
*/
import { find, get } from 'lodash';
+import moment from 'moment';
import { flattenFilters } from '@woocommerce/navigation';
import { format as formatDate } from '@wordpress/date';
-import {
- containsLeapYear,
- getPreviousDate,
- isLeapYear,
-} from '@woocommerce/date';
+import { getPreviousDate } from '@woocommerce/date';
export const DEFAULT_FILTER = 'all';
@@ -55,22 +52,19 @@ export function createDateFormatter( format ) {
}
/**
- * Returns true if the data contains a leap year.
+ * Returns true if the values of a chart metric can be added up across days.
+ * Averages and percentages cannot. Core names averages `avg_*` and types most
+ * of them as `average`, but `avg_order_value` is typed as `currency` for
+ * formatting, so both signals are checked.
*
- * @param {Object} data Chart interval data
- * @return {boolean} True if data contains a leap year.
+ * @param {string} key Chart key, e.x: `orders_count`
+ * @param {string} type Chart type, e.x: `number`
+ * @return {boolean} True if the values can be summed.
*/
-export function dataContainsLeapYear( data ) {
- if ( data?.data?.intervals?.length > 1 ) {
- const start = data.data.intervals[ 0 ].date_start;
- const end =
- data.data.intervals[ data.data.intervals.length - 1 ].date_end;
-
- if ( containsLeapYear( start, end ) ) {
- return true;
- }
- }
- return false;
+function isAdditiveMetric( key, type ) {
+ return (
+ type !== 'average' && type !== 'percent' && ! key.startsWith( 'avg_' )
+ );
}
/**
@@ -83,6 +77,7 @@ export function dataContainsLeapYear( data ) {
* @param {string} comparison Comparison type, e.x: `previous_year`
* @param {string} selectedChartKey Chart key, e.x: `orders_count`
* @param {string} currentInterval Chart interval, e.x: `day`
+ * @param {string} selectedChartType Chart type, e.x: `number`
* @return {Object} Chart data
*/
export function buildChartData(
@@ -92,12 +87,31 @@ export function buildChartData(
secondaryDatePicker,
comparison,
selectedChartKey,
- currentInterval
+ currentInterval,
+ selectedChartType
) {
- const primarydataContainsLeapYear = dataContainsLeapYear( primaryData );
- const secondarydataContainsLeapYear = dataContainsLeapYear( secondaryData );
- const primaryDataIntervals = [ ...primaryData.data.intervals ];
- const secondaryDataIntervals = [ ...secondaryData.data.intervals ];
+ const primaryDataIntervals = primaryData.data.intervals;
+ const secondaryDataIntervals = secondaryData.data.intervals;
+ const shift = secondaryDatePicker.shift;
+ const yearShifted = shift
+ ? shift === 'year'
+ : comparison === 'previous_year';
+
+ // Day intervals are matched by date, so a secondary range that is a leap
+ // day longer or shorter than the primary one cannot push later days out of
+ // line. Coarser intervals do not start on comparable dates (a week starts
+ // on its own weekday), so those keep matching by position.
+ const matchByDate = currentInterval === 'day';
+ const secondaryIntervalsByDate = new Map(
+ matchByDate
+ ? secondaryDataIntervals.map( ( secondaryInterval ) => [
+ moment( secondaryInterval.date_start ).format(
+ 'YYYY-MM-DD'
+ ),
+ secondaryInterval,
+ ] )
+ : []
+ );
const chartData = [];
@@ -112,63 +126,63 @@ export function buildChartData(
const primaryLabelDate = interval.date_start;
const primaryValue = interval.subtotals[ selectedChartKey ] || 0;
- const secondaryInterval = secondaryDataIntervals[ index ];
const secondaryLabel = `${ secondaryDatePicker.label } (${ secondaryDatePicker.range })`;
-
const secondaryDateMoment = getPreviousDate(
interval.date_start,
primaryDatePicker.after,
secondaryDatePicker.after,
comparison,
- currentInterval
+ currentInterval,
+ matchByDate ? shift : undefined
);
let secondaryLabelDate = secondaryDateMoment.format(
'YYYY-MM-DD HH:mm:ss'
);
+ let secondaryLabelDateEnd;
+ let secondaryInterval;
+
+ if ( ! matchByDate ) {
+ secondaryInterval = secondaryDataIntervals[ index ];
+ } else if (
+ yearShifted &&
+ index > 0 &&
+ secondaryDateMoment.date() !== moment( interval.date_start ).date()
+ ) {
+ // A primary 29th February has no counterpart a year earlier: moment
+ // clamps it to the 28th, which already belongs to the primary 28th.
+ // The label renders as "Invalid date", which is desirable since
+ // 29th February is not a valid date for non-leap years.
+ secondaryLabelDate = '-';
+ } else {
+ secondaryInterval = secondaryIntervalsByDate.get(
+ secondaryDateMoment.format( 'YYYY-MM-DD' )
+ );
+ }
+
let secondaryValue =
( secondaryInterval &&
secondaryInterval.subtotals[ selectedChartKey ] ) ||
0;
- if ( currentInterval === 'day' ) {
- if (
- primarydataContainsLeapYear &&
- ! secondarydataContainsLeapYear &&
- secondaryDataIntervals?.[ index ]
- ) {
- // Only fix the data if the date is in 29th Feb and secondary data is in 1st March,
- // which signifies incorrect comparison.
- const primaryDate = new Date( interval.date_start );
- const secondaryDate = new Date(
- secondaryDataIntervals[ index ].date_start
- );
- if (
- isLeapYear( primaryDate.getFullYear() ) &&
- primaryDate.getMonth() === 1 &&
- primaryDate.getDate() === 29 &&
- secondaryDate.getMonth() === 2 &&
- secondaryDate.getDate() === 1
- ) {
- // This is going to be displayed as "Invalid date" label from D3.js, but desirable imo since
- // 29th February is not a valid date for non-leap years.
- secondaryLabelDate = '-';
- secondaryValue = 0;
-
- // Move the data up by 1 day for the missing leap day
- // so everything else is shifted to the right correctly.
- secondaryDataIntervals.splice(
- index,
- 0,
- secondaryDataIntervals[ index ]
- );
- }
- } else if (
- ! primarydataContainsLeapYear &&
- secondarydataContainsLeapYear
- ) {
- // Todo: Do something about secondary data having leap year while first does not.
- // Currently, there are issues to render chart where primary data does not have the date since
- // the x-axis is based on primary data.
+ if (
+ matchByDate &&
+ yearShifted &&
+ secondaryInterval &&
+ isAdditiveMetric( selectedChartKey, selectedChartType )
+ ) {
+ // A secondary 29th February has no column on a non-leap primary axis.
+ // Fold it into the 28th so the line still adds up to the legend total.
+ const dayAfter = secondaryDateMoment.clone().add( 1, 'days' );
+ const leapDayInterval =
+ dayAfter.month() === 1 && dayAfter.date() === 29
+ ? secondaryIntervalsByDate.get(
+ dayAfter.format( 'YYYY-MM-DD' )
+ )
+ : undefined;
+ if ( leapDayInterval ) {
+ secondaryValue +=
+ leapDayInterval.subtotals[ selectedChartKey ] || 0;
+ secondaryLabelDateEnd = leapDayInterval.date_start;
}
}
@@ -182,6 +196,9 @@ export function buildChartData(
secondary: {
label: secondaryLabel,
labelDate: secondaryLabelDate,
+ ...( secondaryLabelDateEnd && {
+ labelDateEnd: secondaryLabelDateEnd,
+ } ),
value: secondaryValue,
},
} );