From b18819489b269b36bd6fe2e266a905b67feb9394 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Albert=20Juh=C3=A9=20Lluveras?= Date: Tue, 26 Feb 2019 11:06:37 +0100 Subject: [PATCH] Don't store searchWords in the query prop (https://github.com/woocommerce/woocommerce-admin/pull/1689) --- .../client/analytics/report/index.js | 12 ++--- .../packages/components/src/table/index.js | 5 +- .../packages/navigation/CHANGELOG.md | 3 ++ .../packages/navigation/src/index.js | 20 ++++++++ .../packages/navigation/src/test/index.js | 48 ++++++++++++++++++- 5 files changed, 77 insertions(+), 11 deletions(-) diff --git a/plugins/woocommerce-admin/client/analytics/report/index.js b/plugins/woocommerce-admin/client/analytics/report/index.js index 5acb3434acf..3b8bee28708 100644 --- a/plugins/woocommerce-admin/client/analytics/report/index.js +++ b/plugins/woocommerce-admin/client/analytics/report/index.js @@ -13,7 +13,7 @@ import { find } from 'lodash'; * WooCommerce dependencies */ import { useFilters } from '@woocommerce/components'; -import { getQuery } from '@woocommerce/navigation'; +import { getQuery, getSearchWords } from '@woocommerce/navigation'; /** * Internal dependencies @@ -144,14 +144,15 @@ Report.propTypes = { export default compose( useFilters( REPORTS_FILTER ), withSelect( ( select, props ) => { - const { search } = getQuery(); + const query = getQuery(); + const { search } = query; if ( ! search ) { return {}; } const { report } = props.params; - const searchWords = search.split( ',' ).map( searchWord => searchWord.replace( '%2C', ',' ) ); + const searchWords = getSearchWords( query ); const itemsResult = searchItemsByString( select, report, searchWords ); const { isError, isRequesting, items } = itemsResult; const ids = Object.keys( items ); @@ -159,10 +160,6 @@ export default compose( return { isError, isRequesting, - query: { - ...props.query, - search: searchWords, - }, }; } @@ -171,7 +168,6 @@ export default compose( isRequesting, query: { ...props.query, - search: searchWords, [ report ]: ids.join( ',' ), }, }; diff --git a/plugins/woocommerce-admin/packages/components/src/table/index.js b/plugins/woocommerce-admin/packages/components/src/table/index.js index f0a1f47ae2a..9dcaa203f43 100644 --- a/plugins/woocommerce-admin/packages/components/src/table/index.js +++ b/plugins/woocommerce-admin/packages/components/src/table/index.js @@ -17,7 +17,7 @@ import { generateCSVDataFromTable, generateCSVFileName, } from '@woocommerce/csv-export'; -import { getIdsFromQuery, updateQueryString } from '@woocommerce/navigation'; +import { getIdsFromQuery, getSearchWords, updateQueryString } from '@woocommerce/navigation'; /** * Internal dependencies @@ -246,7 +246,8 @@ class TableCard extends Component { totalRows, } = this.props; const { selectedRows, showCols } = this.state; - const searchedLabels = Array.isArray( query.search ) ? query.search.map( v => ( { id: v, label: v } ) ) : []; + const searchWords = getSearchWords( query ); + const searchedLabels = searchWords.map( v => ( { id: v, label: v } ) ); const allHeaders = this.props.headers; let headers = this.getVisibleHeaders(); let rows = this.getVisibleRows(); diff --git a/plugins/woocommerce-admin/packages/navigation/CHANGELOG.md b/plugins/woocommerce-admin/packages/navigation/CHANGELOG.md index f1ba2aca27a..625744d2c0c 100644 --- a/plugins/woocommerce-admin/packages/navigation/CHANGELOG.md +++ b/plugins/woocommerce-admin/packages/navigation/CHANGELOG.md @@ -1,3 +1,6 @@ +# (unreleased) +- New method `getSearchWords` that extracts search words given a query object. + # 2.0.0 - Replace `history` export with `getHistory` (allows for lazy-create of history) diff --git a/plugins/woocommerce-admin/packages/navigation/src/index.js b/plugins/woocommerce-admin/packages/navigation/src/index.js index 02f465b8667..19202a8a4c4 100644 --- a/plugins/woocommerce-admin/packages/navigation/src/index.js +++ b/plugins/woocommerce-admin/packages/navigation/src/index.js @@ -70,6 +70,26 @@ export function getIdsFromQuery( queryString = '' ) { ); } +/** + * Get an array of searched words given a query. + * + * @param {Object} query Query object. + * @return {Array} List of search words. + */ +export function getSearchWords( query = navUtils.getQuery() ) { + if ( typeof query !== 'object' ) { + throw new Error( 'Invalid parameter passed to getSearchWords, it expects an object or no parameters.' ); + } + const { search } = query; + if ( ! search ) { + return []; + } + if ( typeof search !== 'string' ) { + throw new Error( 'Invalid \'search\' type. getSearchWords expects query\'s \'search\' property to be a string.' ); + } + return search.split( ',' ).map( searchWord => searchWord.replace( '%2C', ',' ) ); +} + /** * Return a URL with set query parameters. * diff --git a/plugins/woocommerce-admin/packages/navigation/src/test/index.js b/plugins/woocommerce-admin/packages/navigation/src/test/index.js index 371e37839b3..481e328f097 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 { getPersistedQuery } from '../index'; +import { getPersistedQuery, getSearchWords } from '../index'; jest.mock( '../index', () => ( { ...require.requireActual( '../index' ), @@ -14,6 +14,7 @@ jest.mock( '../index', () => ( { after: '2018-02-01', before: '2018-01-01', interval: 'day', + search: 'lorem', } ), } ) ); @@ -63,3 +64,48 @@ describe( 'getPersistedQuery', () => { expect( getPersistedQuery() ).toEqual( persistedQuery ); } ); } ); + +describe( 'getSearchWords', () => { + it( 'should get the search words from a query object', () => { + const query = { + search: 'lorem,dolor sit', + }; + const searchWords = [ 'lorem', 'dolor sit' ]; + + expect( getSearchWords( query ) ).toEqual( searchWords ); + } ); + + it( 'should parse `%2C` as commas', () => { + const query = { + search: 'lorem%2Cipsum,dolor sit', + }; + const searchWords = [ 'lorem,ipsum', 'dolor sit' ]; + + expect( getSearchWords( query ) ).toEqual( searchWords ); + } ); + + it( 'should return an empty array if the query has no `search` property', () => { + const query = {}; + const searchWords = []; + + expect( getSearchWords( query ) ).toEqual( searchWords ); + } ); + + it( 'should use the persisted query when it receives no params', () => { + const searchWords = [ 'lorem' ]; + + expect( getSearchWords() ).toEqual( searchWords ); + } ); + + it( 'should throw an error if the param is not an object', () => { + expect( () => getSearchWords( 'lorem' ) ).toThrow( Error ); + } ); + + it( 'should throw an error if the `search` property is not a string', () => { + const query = { + search: new Object(), + }; + + expect( () => getSearchWords( query ) ).toThrow( Error ); + } ); +} );