From 3a90c07e769756a82ed7350a02a93f7212e2c18a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Mon, 7 Jan 2019 11:41:46 +0100 Subject: [PATCH] Fix date issues introduced in woocommerce/woocommerce-admin#1203 (https://github.com/woocommerce/woocommerce-admin/pull/1229) * Revert "Show today charts up to the current hour (https://github.com/woocommerce/woocommerce-admin/pull/1203)" This reverts commit 029ff9cc9c79aae1cf2a1c7de00f44e1b663fdfb. * Add current hour to timestamp of queries ending in today * Fix getCurrentDates not returning moment objects as specified in the docs but returning string dates * Set appendTimestamp( ..., 'now') seconds to 00 * Add test for appendTimestamp( ..., 'now' ) * Don't accept string dates in 'appendTimestamp' * Fix 'moment' dependency deprecation warning --- .../client/store/reports/test/utils.js | 2 +- .../client/store/reports/utils.js | 10 +++-- .../packages/date/src/index.js | 37 +++++++++++-------- .../packages/date/test/index.js | 35 ++++++++++-------- 4 files changed, 49 insertions(+), 35 deletions(-) diff --git a/plugins/woocommerce-admin/client/store/reports/test/utils.js b/plugins/woocommerce-admin/client/store/reports/test/utils.js index 90e0b604c3c..94c83b46ce9 100644 --- a/plugins/woocommerce-admin/client/store/reports/test/utils.js +++ b/plugins/woocommerce-admin/client/store/reports/test/utils.js @@ -327,7 +327,7 @@ describe( 'getSummaryNumbers()', () => { } ); setGetReportStats( ( endpoint, _query ) => { - if ( '2018-10-10T00:00:00' === _query.after ) { + if ( '2018-10-10T00:00:00+00:00' === _query.after ) { return { data: { totals: totals.primary, diff --git a/plugins/woocommerce-admin/client/store/reports/utils.js b/plugins/woocommerce-admin/client/store/reports/utils.js index a81eb662921..fb1ff9379f9 100644 --- a/plugins/woocommerce-admin/client/store/reports/utils.js +++ b/plugins/woocommerce-admin/client/store/reports/utils.js @@ -4,6 +4,7 @@ * External dependencies */ import { find, forEach, isNull } from 'lodash'; +import moment from 'moment'; /** * WooCommerce dependencies @@ -119,15 +120,18 @@ export function isReportDataEmpty( report ) { * @returns {Object} data request query parameters. */ function getRequestQuery( endpoint, dataType, query ) { - const datesFromQuery = getCurrentDates( query, 'YYYY-MM-DDTHH:00:00' ); + const datesFromQuery = getCurrentDates( query ); const interval = getIntervalForQuery( query ); const filterQuery = getFilterQuery( endpoint, query ); + const end = datesFromQuery[ dataType ].before; + const endingTimeOfDay = end.isSame( moment(), 'day' ) ? 'now' : 'end'; + return { order: 'asc', interval, per_page: MAX_PER_PAGE, - after: datesFromQuery[ dataType ].after, - before: datesFromQuery[ dataType ].before, + after: appendTimestamp( datesFromQuery[ dataType ].after, 'start' ), + before: appendTimestamp( end, endingTimeOfDay ), ...filterQuery, }; } diff --git a/plugins/woocommerce-admin/packages/date/src/index.js b/plugins/woocommerce-admin/packages/date/src/index.js index c4501bd9b8f..06bae8d80c6 100644 --- a/plugins/woocommerce-admin/packages/date/src/index.js +++ b/plugins/woocommerce-admin/packages/date/src/index.js @@ -56,18 +56,24 @@ export const periods = [ /** * Adds timestamp to a string date. * - * @param {string} date - Date as a string. - * @param {string} timeOfDay - Either `start` or `end` of the day. + * @param {moment.Moment} date - Date as a moment object. + * @param {string} timeOfDay - Either `start`, `now` or `end` of the day. * @return {string} - String date with timestamp attached. */ export const appendTimestamp = ( date, timeOfDay ) => { + date = date.format( isoDateFormat ); if ( timeOfDay === 'start' ) { return date + 'T00:00:00+00:00'; } + if ( timeOfDay === 'now' ) { + // Set seconds to 00 to avoid consecutives calls happening before the previous + // one finished. + return date + 'T' + moment().format( 'HH:mm:00' ) + '+00:00'; + } if ( timeOfDay === 'end' ) { return date + 'T23:59:59+00:00'; } - throw new Error( 'appendTimestamp requires second parameter to be either `start` or `end`' ); + throw new Error( 'appendTimestamp requires second parameter to be either `start`, `now` or `end`' ); }; /** @@ -172,14 +178,14 @@ export function getLastPeriod( period, compare ) { */ export function getCurrentPeriod( period, compare ) { const primaryStart = moment().startOf( period ); - const primaryEnd = moment().add( 1, 'hour' ); - const hoursSoFar = primaryEnd.diff( primaryStart, 'hours' ); + const primaryEnd = moment(); + const daysSoFar = primaryEnd.diff( primaryStart, 'days' ); let secondaryStart; let secondaryEnd; if ( 'previous_period' === compare ) { secondaryStart = primaryStart.clone().subtract( 1, period ); - secondaryEnd = primaryEnd.clone().subtract( 1, period ).add( 1, 'hour' ); + secondaryEnd = primaryEnd.clone().subtract( 1, period ); } else { secondaryStart = 'week' === period @@ -189,7 +195,7 @@ export function getCurrentPeriod( period, compare ) { .week( primaryStart.week() ) .startOf( 'week' ) : primaryStart.clone().subtract( 1, 'years' ); - secondaryEnd = secondaryStart.clone().add( hoursSoFar + 1, 'hours' ); + secondaryEnd = secondaryStart.clone().add( daysSoFar, 'days' ); } return { primaryStart, @@ -278,10 +284,9 @@ export const getDateParamsFromQuery = ( { period, compare, after, before } ) => * @property {string} [compare] - compare valuer, ie previous_year * @property {string} [after] - date in iso date format, ie 2018-07-03 * @property {string} [before] - date in iso date format, ie 2018-07-03 - * @param {String} dateFormat - format of the dates to return * @return {{primary: DateValue, secondary: DateValue}} - Primary and secondary DateValue objects */ -export const getCurrentDates = ( query, dateFormat = isoDateFormat ) => { +export const getCurrentDates = query => { const { period, compare, after, before } = getDateParamsFromQuery( query ); const { primaryStart, primaryEnd, secondaryStart, secondaryEnd } = getDateValue( period, @@ -294,14 +299,14 @@ export const getCurrentDates = ( query, dateFormat = isoDateFormat ) => { primary: { label: find( presetValues, item => item.value === period ).label, range: getRangeLabel( primaryStart, primaryEnd ), - after: primaryStart.format( dateFormat ), - before: primaryEnd.format( dateFormat ), + after: primaryStart, + before: primaryEnd, }, secondary: { label: find( periods, item => item.value === compare ).label, range: getRangeLabel( secondaryStart, secondaryEnd ), - after: secondaryStart.format( dateFormat ), - before: secondaryEnd.format( dateFormat ), + after: secondaryStart, + before: secondaryEnd, }, }; }; @@ -323,11 +328,11 @@ export const getDateDifferenceInDays = ( date, date2 ) => { * 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 {String|Moment.moment} date1 - primary start + * @param {String|Moment.moment} date2 - secondary start * @param {String} compare - `previous_period` or `previous_year` * @param {String} interval - interval - * @return {String} - Calculated date + * @return {Moment.moment} - Calculated date */ export const getPreviousDate = ( date, date1, date2, compare, interval ) => { const dateMoment = moment( date ); diff --git a/plugins/woocommerce-admin/packages/date/test/index.js b/plugins/woocommerce-admin/packages/date/test/index.js index 80ae76ae364..9cea2010b98 100644 --- a/plugins/woocommerce-admin/packages/date/test/index.js +++ b/plugins/woocommerce-admin/packages/date/test/index.js @@ -25,15 +25,20 @@ import { describe( 'appendTimestamp', () => { it( 'should append `start` timestamp', () => { - expect( appendTimestamp( '2018-01-01', 'start' ) ).toEqual( '2018-01-01T00:00:00+00:00' ); + expect( appendTimestamp( moment( '2018-01-01' ), 'start' ) ).toEqual( '2018-01-01T00:00:00+00:00' ); + } ); + + it( 'should append `now` timestamp', () => { + const nowTimestamp = moment().format( 'HH:mm:00' ); + expect( appendTimestamp( moment( '2018-01-01' ), 'now' ) ).toEqual( '2018-01-01T' + nowTimestamp + '+00:00' ); } ); it( 'should append `end` timestamp', () => { - expect( appendTimestamp( '2018-01-01', 'end' ) ).toEqual( '2018-01-01T23:59:59+00:00' ); + expect( appendTimestamp( moment( '2018-01-01' ), 'end' ) ).toEqual( '2018-01-01T23:59:59+00:00' ); } ); it( 'should throw and error if `timeOfDay` is not valid', () => { - expect( () => appendTimestamp( '2018-01-01' ) ).toThrow( Error ); + expect( () => appendTimestamp( moment( '2018-01-01' ) ) ).toThrow( Error ); } ); } ); @@ -481,16 +486,16 @@ describe( 'getCurrentDates', () => { const currentDates = getCurrentDates( query ); expect( currentDates.primary ).toBeDefined(); - expect( 'string' === typeof currentDates.primary.label ).toBe( true ); - expect( 'string' === typeof currentDates.primary.range ).toBe( true ); - expect( 'string' === typeof currentDates.primary.after ).toBe( true ); - expect( 'string' === typeof currentDates.primary.before ).toBe( true ); + expect( typeof currentDates.primary.label ).toBe( 'string' ); + expect( typeof currentDates.primary.range ).toBe( 'string' ); + expect( moment.isMoment( currentDates.primary.after ) ).toBe( true ); + expect( moment.isMoment( currentDates.primary.before ) ).toBe( true ); expect( currentDates.secondary ).toBeDefined(); - expect( 'string' === typeof currentDates.secondary.label ).toBe( true ); - expect( 'string' === typeof currentDates.secondary.range ).toBe( true ); - expect( 'string' === typeof currentDates.secondary.after ).toBe( true ); - expect( 'string' === typeof currentDates.secondary.before ).toBe( true ); + expect( typeof currentDates.secondary.label ).toBe( 'string' ); + expect( typeof currentDates.secondary.range ).toBe( 'string' ); + expect( moment.isMoment( currentDates.secondary.after ) ).toBe( true ); + expect( moment.isMoment( currentDates.secondary.before ) ).toBe( true ); } ); it( 'should correctly apply default values', () => { @@ -509,12 +514,12 @@ describe( 'getCurrentDates', () => { const currentDates = getCurrentDates( query ); // Ensure default period is 'month' - expect( currentDates.primary.after ).toBe( startOfMonth ); - expect( currentDates.primary.before ).toBe( today ); + expect( currentDates.primary.after.format( isoDateFormat ) ).toBe( startOfMonth ); + expect( currentDates.primary.before.format( isoDateFormat ) ).toBe( today ); // Ensure default compare is `previous_period` - expect( currentDates.secondary.after ).toBe( startOfMonthYearAgo ); - expect( currentDates.secondary.before ).toBe( todayLastYear ); + expect( currentDates.secondary.after.format( isoDateFormat ) ).toBe( startOfMonthYearAgo ); + expect( currentDates.secondary.before.format( isoDateFormat ) ).toBe( todayLastYear ); } ); } );