Merge pull request woocommerce/woocommerce-admin#841 from woocommerce/fix/chart-settings-persistence

Chart: Persist interval and chart type

plugins/woocommerce-admin/client/analytics/components/report-chart/index.js

@@ -17,6 +17,7 @@ import {
 	getCurrentDates,
 	getDateFormatsForInterval,
 	getIntervalForQuery,
+	getChartTypeForQuery,
 	getPreviousDate,
 } from '@woocommerce/date';
 
@@ -91,6 +92,7 @@ class ReportChart extends Component {
 				data={ chartData }
 				title={ selectedChart.label }
 				interval={ currentInterval }
+				type={ getChartTypeForQuery( query ) }
 				allowedIntervals={ allowedIntervals }
 				itemsLabel={ itemsLabel }
 				layout={ comparisonChart ? 'comparison' : 'standard' }

plugins/woocommerce-admin/client/analytics/report/products/table.js

@@ -14,7 +14,7 @@ import { get, map, orderBy } from 'lodash';
 import { appendTimestamp, getCurrentDates } from '@woocommerce/date';
 import { Link, TableCard } from '@woocommerce/components';
 import { formatCurrency, getCurrencyFormatDecimal } from '@woocommerce/currency';
-import { getNewPath, getTimeRelatedQuery, onQueryChange } from '@woocommerce/navigation';
+import { getNewPath, getPersistedQuery, onQueryChange } from '@woocommerce/navigation';
 
 /**
  * Internal dependencies
@@ -80,7 +80,7 @@ class ProductsReportTable extends Component {
 	getRowsContent( data = [] ) {
 		const { stockStatuses } = wcSettings;
 		const { query } = this.props;
-		const timeRelatedQuery = getTimeRelatedQuery( query );
+		const persistedQuery = getPersistedQuery( query );
 
 		return map( data, row => {
 			const {
@@ -95,7 +95,7 @@ class ProductsReportTable extends Component {
 				stock_status = 'outofstock', // @TODO
 				stock_quantity = '0', // @TODO
 			} = row;
-			const ordersLink = getNewPath( timeRelatedQuery, 'orders', {
+			const ordersLink = getNewPath( persistedQuery, 'orders', {
 				filter: 'advanced',
 				product_includes: product_id,
 			} );

plugins/woocommerce-admin/client/components/chart/index.js

@@ -63,7 +63,6 @@ class Chart extends Component {
 		this.state = {
 			data: props.data,
 			orderedKeys: getOrderedKeys( props ),
-			type: props.type,
 			visibleData: [ ...props.data ],
 			width: 0,
 		};
@@ -98,8 +97,9 @@ class Chart extends Component {
 	}
 
 	handleTypeToggle( type ) {
-		if ( this.state.type !== type ) {
+		if ( this.props.type !== type ) {
-			this.setState( { type } );
+			const { path, query } = this.props;
+			updateQueryString( { type }, path, query );
 		}
 	}
 
@@ -200,7 +200,7 @@ class Chart extends Component {
 	}
 
 	render() {
-		const { orderedKeys, type, visibleData, width } = this.state;
+		const { orderedKeys, visibleData, width } = this.state;
 		const {
 			dateParser,
 			itemsLabel,
@@ -216,6 +216,7 @@ class Chart extends Component {
 			x2Format,
 			interval,
 			valueType,
+			type,
 		} = this.props;
 		let { yFormat } = this.props;
 		const legendDirection = layout === 'standard' && isViewportWide ? 'row' : 'column';

plugins/woocommerce-admin/client/header/index.js

@@ -12,7 +12,7 @@ import PropTypes from 'prop-types';
 /**
  * WooCommerce dependencies
  */
-import { getNewPath, getTimeRelatedQuery } from '@woocommerce/navigation';
+import { getNewPath, getPersistedQuery } from '@woocommerce/navigation';
 import { Link } from '@woocommerce/components';
 
 /**
@@ -89,7 +89,7 @@ class Header extends Component {
 					{ _sections.map( ( section, i ) => {
 						const sectionPiece = Array.isArray( section ) ? (
 							<Link
-								href={ getNewPath( getTimeRelatedQuery(), section[ 0 ], {} ) }
+								href={ getNewPath( getPersistedQuery(), section[ 0 ], {} ) }
 								type={ isEmbedded ? 'wp-admin' : 'wc-admin' }
 							>
 								{ section[ 1 ] }

plugins/woocommerce-admin/client/layout/controller.js

@@ -9,7 +9,7 @@ import { find, last } from 'lodash';
 /**
  * WooCommerce dependencies
  */
-import { getTimeRelatedQuery, stringifyQuery } from '@woocommerce/navigation';
+import { getPersistedQuery, stringifyQuery } from '@woocommerce/navigation';
 
 /**
  * Internal dependencies
@@ -71,7 +71,7 @@ class Controller extends Component {
 
 // Update links in wp-admin menu to persist time related queries
 window.wpNavMenuUrlUpdate = function( page, query ) {
-	const search = stringifyQuery( getTimeRelatedQuery( query ) );
+	const search = stringifyQuery( getPersistedQuery( query ) );
 
 	Array.from(
 		document.querySelectorAll( `#${ page.wpOpenMenu } a, #${ page.wpClosedMenu } a` )

plugins/woocommerce-admin/packages/components/src/filters/filter/index.js

@@ -12,7 +12,7 @@ import PropTypes from 'prop-types';
 /**
  * WooCommerce dependencies
  */
-import { flattenFilters, getTimeRelatedQuery, updateQueryString } from '@woocommerce/navigation';
+import { flattenFilters, getPersistedQuery, updateQueryString } from '@woocommerce/navigation';
 
 /**
  * Internal dependencies
@@ -92,7 +92,7 @@ class FilterPicker extends Component {
 	update( value, additionalQueries = {} ) {
 		const { path, query, config } = this.props;
 		// Keep only time related queries when updating to a new filter
-		const timeRelatedQuery = getTimeRelatedQuery( query );
+		const persistedQuery = getPersistedQuery( query );
 		const update = {
 			[ config.param ]: 'all' === value ? undefined : value,
 			...additionalQueries,
@@ -101,7 +101,7 @@ class FilterPicker extends Component {
 		config.staticParams.forEach( param => {
 			update[ param ] = query[ param ];
 		} );
-		updateQueryString( update, path, timeRelatedQuery );
+		updateQueryString( update, path, persistedQuery );
 	}
 
 	onTagChange( filter, onClose, tags ) {

plugins/woocommerce-admin/packages/date/src/index.js

@@ -412,6 +412,19 @@ export function getIntervalForQuery( query ) {
 	return current;
 }
 
+/**
+ * Returns the current chart type to use.
+ *
+ * @param  {Object} query Current query
+ * @return {String} Current chart type.
+ */
+export function getChartTypeForQuery( { type } ) {
+	if ( [ 'line', 'bar' ].includes( type ) ) {
+		return type;
+	}
+	return 'line';
+}
+
 export const dayTicksThreshold = 63;
 export const weekTicksThreshold = 9;
 

plugins/woocommerce-admin/packages/date/test/index.js

@@ -22,6 +22,7 @@ import {
 	isoDateFormat,
 	getDateDifferenceInDays,
 	getPreviousDate,
+	getChartTypeForQuery,
 } from '../src';
 
 describe( 'appendTimestamp', () => {
@@ -743,3 +744,23 @@ describe( 'getPreviousDate', () => {
 		expect( previousDate.format( isoDateFormat ) ).toBe( '2017-08-21' );
 	} );
 } );
+
+describe( 'getChartTypeForQuery', () => {
+	it( 'should return allowed type', () => {
+		const query = {
+			type: 'bar',
+		};
+		expect( getChartTypeForQuery( query ) ).toBe( 'bar' );
+	} );
+
+	it( 'should default to line', () => {
+		expect( getChartTypeForQuery( {} ) ).toBe( 'line' );
+	} );
+
+	it( 'should return line for not allowed type', () => {
+		const query = {
+			type: 'burrito',
+		};
+		expect( getChartTypeForQuery( query ) ).toBe( 'line' );
+	} );
+} );

plugins/woocommerce-admin/packages/navigation/src/index.js

@@ -46,13 +46,14 @@ export const getPath = () => history.location.pathname;
 export const stringifyQuery = query => ( isEmpty( query ) ? '' : '?' + stringify( query ) );
 
 /**
- * Gets time related parameters from a query.
+ * Gets query parameters that should persist between screens or updates
+ * to reports, such as filtering.
  *
  * @param {Object} query Query containing the parameters.
- * @return {Object} Object containing the time related queries.
+ * @return {Object} Object containing the persisted queries.
  */
-export const getTimeRelatedQuery = ( query = navUtils.getQuery() ) =>
+export const getPersistedQuery = ( query = navUtils.getQuery() ) =>
-	pick( query, [ 'period', 'compare', 'before', 'after' ] );
+	pick( query, [ 'period', 'compare', 'before', 'after', 'interval', 'type' ] );
 
 /**
  * Get an array of IDs from a comma-separated query parameter.

plugins/woocommerce-admin/packages/navigation/src/test/index.js

@@ -2,7 +2,7 @@
 /**
  * Internal dependencies
  */
-import { getTimeRelatedQuery } from '../index';
+import { getPersistedQuery } from '../index';
 
 jest.mock( '../index', () => ( {
 	...require.requireActual( '../index' ),
@@ -13,18 +13,19 @@ jest.mock( '../index', () => ( {
 		compare: 'previous_year',
 		after: '2018-02-01',
 		before: '2018-01-01',
+		interval: 'day',
 	} ),
 } ) );
 
-describe( 'getTimeRelatedQuery', () => {
+describe( 'getPersistedQuery', () => {
 	it( "should return an empty object it the query doesn't contain any time related parameters", () => {
 		const query = {
 			filter: 'advanced',
 			product_includes: 127,
 		};
-		const timeRelatedQuery = {};
+		const persistedQuery = {};
 
-		expect( getTimeRelatedQuery( query ) ).toEqual( timeRelatedQuery );
+		expect( getPersistedQuery( query ) ).toEqual( persistedQuery );
 	} );
 
 	it( 'should return time related parameters', () => {
@@ -35,25 +36,30 @@ describe( 'getTimeRelatedQuery', () => {
 			compare: 'previous_year',
 			after: '2018-02-01',
 			before: '2018-01-01',
+			type: 'bar',
+			interval: 'day',
 		};
-		const timeRelatedQuery = {
+		const persistedQuery = {
 			period: 'year',
 			compare: 'previous_year',
 			after: '2018-02-01',
 			before: '2018-01-01',
+			type: 'bar',
+			interval: 'day',
 		};
 
-		expect( getTimeRelatedQuery( query ) ).toEqual( timeRelatedQuery );
+		expect( getPersistedQuery( query ) ).toEqual( persistedQuery );
 	} );
 
 	it( 'should get the query from getQuery() when none is provided in the params', () => {
-		const timeRelatedQuery = {
+		const persistedQuery = {
 			period: 'year',
 			compare: 'previous_year',
 			after: '2018-02-01',
 			before: '2018-01-01',
+			interval: 'day',
 		};
 
-		expect( getTimeRelatedQuery() ).toEqual( timeRelatedQuery );
+		expect( getPersistedQuery() ).toEqual( persistedQuery );
 	} );
 } );