From 0d5999862533128b3fcc3c13a0cdaea35cf62476 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Wed, 14 Nov 2018 14:45:05 +1300 Subject: [PATCH] Persist chart settings --- .../components/report-chart/index.js | 2 ++ .../client/analytics/report/products/table.js | 6 ++--- .../client/components/chart/index.js | 9 ++++---- .../woocommerce-admin/client/header/index.js | 4 ++-- .../client/layout/controller.js | 4 ++-- .../components/src/filters/filter/index.js | 6 ++--- .../packages/date/src/index.js | 13 +++++++++++ .../packages/date/test/index.js | 21 ++++++++++++++++++ .../packages/navigation/src/index.js | 9 ++++---- .../packages/navigation/src/test/index.js | 22 ++++++++++++------- 10 files changed, 70 insertions(+), 26 deletions(-) diff --git a/plugins/woocommerce-admin/client/analytics/components/report-chart/index.js b/plugins/woocommerce-admin/client/analytics/components/report-chart/index.js index 1be79ccd34b..e342922a2ca 100644 --- a/plugins/woocommerce-admin/client/analytics/components/report-chart/index.js +++ b/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' } diff --git a/plugins/woocommerce-admin/client/analytics/report/products/table.js b/plugins/woocommerce-admin/client/analytics/report/products/table.js index a4bf5b894b9..853015e106d 100644 --- a/plugins/woocommerce-admin/client/analytics/report/products/table.js +++ b/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, } ); diff --git a/plugins/woocommerce-admin/client/components/chart/index.js b/plugins/woocommerce-admin/client/components/chart/index.js index 2e48dcd66f2..b92636a856e 100644 --- a/plugins/woocommerce-admin/client/components/chart/index.js +++ b/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 ) { - this.setState( { type } ); + if ( this.props.type !== 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'; diff --git a/plugins/woocommerce-admin/client/header/index.js b/plugins/woocommerce-admin/client/header/index.js index 858f1c49250..c8a9353f8f2 100644 --- a/plugins/woocommerce-admin/client/header/index.js +++ b/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 ) ? ( { section[ 1 ] } diff --git a/plugins/woocommerce-admin/client/layout/controller.js b/plugins/woocommerce-admin/client/layout/controller.js index 024ea1138ff..7050d60ff78 100644 --- a/plugins/woocommerce-admin/client/layout/controller.js +++ b/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` ) diff --git a/plugins/woocommerce-admin/packages/components/src/filters/filter/index.js b/plugins/woocommerce-admin/packages/components/src/filters/filter/index.js index a955cf0bf92..02721950532 100644 --- a/plugins/woocommerce-admin/packages/components/src/filters/filter/index.js +++ b/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 ) { diff --git a/plugins/woocommerce-admin/packages/date/src/index.js b/plugins/woocommerce-admin/packages/date/src/index.js index e678f59215d..288f742f487 100644 --- a/plugins/woocommerce-admin/packages/date/src/index.js +++ b/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; diff --git a/plugins/woocommerce-admin/packages/date/test/index.js b/plugins/woocommerce-admin/packages/date/test/index.js index e2a5f527ae3..752fa789ec6 100644 --- a/plugins/woocommerce-admin/packages/date/test/index.js +++ b/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' ); + } ); +} ); diff --git a/plugins/woocommerce-admin/packages/navigation/src/index.js b/plugins/woocommerce-admin/packages/navigation/src/index.js index c2eae888ebd..9eab62b978b 100644 --- a/plugins/woocommerce-admin/packages/navigation/src/index.js +++ b/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() ) => - pick( query, [ 'period', 'compare', 'before', 'after' ] ); +export const getPersistedQuery = ( query = navUtils.getQuery() ) => + pick( query, [ 'period', 'compare', 'before', 'after', 'interval', 'type' ] ); /** * Get an array of IDs from a comma-separated query parameter. diff --git a/plugins/woocommerce-admin/packages/navigation/src/test/index.js b/plugins/woocommerce-admin/packages/navigation/src/test/index.js index deaf5ff647a..371e37839b3 100644 --- a/plugins/woocommerce-admin/packages/navigation/src/test/index.js +++ b/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 ); } ); } );