Merge pull request woocommerce/woocommerce-admin#196 from woocommerce/fix/getCurrentDates-defaults

Datepicker: fix getCurrentDates to always reflect defaults

plugins/woocommerce-admin/client/components/date-picker/index.js

@@ -6,7 +6,6 @@ import { Component, Fragment } from '@wordpress/element';
 import { __ } from '@wordpress/i18n';
 import { Dropdown } from '@wordpress/components';
 import { stringify as stringifyQueryObject } from 'qs';
-import moment from 'moment';
 import PropTypes from 'prop-types';
 
 /**
@@ -15,12 +14,12 @@ import PropTypes from 'prop-types';
 import './style.scss';
 import DropdownButton from 'components/dropdown-button';
 import DatePickerContent from './content';
-import { getCurrentDates, isoDateFormat } from 'lib/date';
+import { getCurrentDates, getDateParamsFromQuery, isoDateFormat } from 'lib/date';
 
 class DatePicker extends Component {
 	constructor( props ) {
 		super( props );
-		this.state = this.addQueryDefaults( props.query );
+		this.state = getDateParamsFromQuery( props.query );
 		this.select = this.select.bind( this );
 		this.getUpdatePath = this.getUpdatePath.bind( this );
 		this.isValidSelection = this.isValidSelection.bind( this );
@@ -28,16 +27,7 @@ class DatePicker extends Component {
 
 	// @TODO change this to `getDerivedStateFromProps` in React 16.4
 	UNSAFE_componentWillReceiveProps( nextProps ) {
-		this.setState( this.addQueryDefaults( nextProps.query ) );
-	}
-
-	addQueryDefaults( { period, compare, after, before } ) {
-		return {
-			period: period || 'today',
-			compare: compare || 'previous_period',
-			after: after ? moment( after ) : null,
-			before: before ? moment( before ) : null,
-		};
+		this.setState( getDateParamsFromQuery( nextProps.query ) );
 	}
 
 	select( update ) {
@@ -68,8 +58,7 @@ class DatePicker extends Component {
 	}
 
 	getButtonLabel() {
-		const queryWithDefaults = this.addQueryDefaults( this.props.query );
-		const { primary, secondary } = getCurrentDates( queryWithDefaults );
+		const { primary, secondary } = getCurrentDates( this.props.query );
 		return [
 			`${ primary.label } (${ primary.range })`,
 			`${ __( 'vs.', 'wc-admin' ) } ${ secondary.label } (${ secondary.range })`,

plugins/woocommerce-admin/client/lib/date/index.js

@@ -23,6 +23,16 @@ import { compareValues } from 'components/date-picker/compare-periods';
  * @property {moment.Moment} before - End of the date range.
  */
 
+/**
+ * DateParams Object
+ *
+ * @typedef {Object} dateParams - date parameters derived from query parameters.
+ * @property {string} period - period value, ie `last_week`
+ * @property {string} compare - compare valuer, ie previous_year
+ * @param {moment.Moment|null} after - If the period supplied is "custom", this is the after date
+ * @param {moment.Moment|null} before - If the period supplied is "custom", this is the before date
+ */
+
 export const isoDateFormat = 'YYYY-MM-DD';
 
 /**
@@ -207,16 +217,36 @@ function getDateValue( period, compare, after, before ) {
 	}
 }
 
+/**
+ * Add default date-related parameters to a query object
+ *
+ * @param {string} [period] - period value, ie `last_week`
+ * @param {string} [compare] - compare valuer, ie previous_year
+ * @param {string} [after] - date in iso date format, ie 2018-07-03
+ * @param {string} [before] - date in iso date format, ie 2018-07-03
+ * @return {DateParams} - date parameters derived from query parameters with added defaults
+ */
+export const getDateParamsFromQuery = ( { period, compare, after, before } ) => {
+	return {
+		period: period || 'today',
+		compare: compare || 'previous_period',
+		after: after ? moment( after ) : null,
+		before: before ? moment( before ) : null,
+	};
+};
+
 /**
  * Get Date Value Objects for a primary and secondary date range
  *
- * @param {string} period - Indicates period, 'last_week', 'quarter', or 'custom'
- * @param {string} compare - Indicates the period to compare against, 'previous_period', previous_year'
- * @param {moment.Moment} [after] - If the period supplied is "custom", this is the after date
- * @param {moment.Moment} [before] - If the period supplied is "custom", this is the before date
+ * @param {Object} query - date parameters derived from query parameters
+ * @property {string} [period] - period value, ie `last_week`
+ * @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
  * @return {{primary: DateValue, secondary: DateValue}} - Primary and secondary DateValue objects
  */
-export const getCurrentDates = ( { period, compare, after, before } ) => {
+export const getCurrentDates = query => {
+	const { period, compare, after, before } = getDateParamsFromQuery( query );
 	const { primaryStart, primaryEnd, secondaryStart, secondaryEnd } = getDateValue(
 		period,
 		compare,
@@ -240,6 +270,10 @@ export const getCurrentDates = ( { period, compare, after, before } ) => {
 	};
 };
 
+/**
+ * Gutenberg's moment instance is loaded with i18n values. If the locale isn't english
+ * we can use that data and enhance it with additional translations
+ */
 export function loadLocaleData() {
 	const { date } = wcSettings;
 	const settings = getSettings();

plugins/woocommerce-admin/client/lib/date/test/index.js

@@ -9,7 +9,14 @@ import { setLocaleData } from '@wordpress/i18n';
 /**
  * Internal dependencies
  */
-import { toMoment, getLastPeriod, getCurrentPeriod, getRangeLabel, loadLocaleData } from 'lib/date';
+import {
+	toMoment,
+	getLastPeriod,
+	getCurrentPeriod,
+	getRangeLabel,
+	loadLocaleData,
+	getCurrentDates,
+} from 'lib/date';
 
 describe( 'toMoment', () => {
 	it( 'should pass through a valid Moment object as an argument', () => {
@@ -545,3 +552,37 @@ describe( 'loadLocaleData', () => {
 		expect( moment.localeData()._week.dow ).toBe( 5 );
 	} );
 } );
+
+describe( 'getCurrentDates', () => {
+	it( 'should return a correctly shaped object', () => {
+		const query = {};
+		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( 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( moment.isMoment( currentDates.secondary.after ) ).toBe( true );
+		expect( moment.isMoment( currentDates.secondary.before ) ).toBe( true );
+	} );
+
+	it( 'should correctly apply default values', () => {
+		const query = {};
+		const today = moment();
+		const yesterday = today.clone().subtract( 1, 'day' );
+		const currentDates = getCurrentDates( query );
+
+		// Ensure default period is 'today'
+		expect( today.isSame( currentDates.primary.after, 'day' ) ).toBe( true );
+		expect( today.isSame( currentDates.primary.before, 'day' ) ).toBe( true );
+
+		// Ensure default compare is `previous_period`
+		expect( yesterday.isSame( currentDates.secondary.after, 'day' ) ).toBe( true );
+		expect( yesterday.isSame( currentDates.secondary.before, 'day' ) ).toBe( true );
+	} );
+} );