From 842c23201e5f5cb23cad99490d81e0e252140179 Mon Sep 17 00:00:00 2001 From: Paul Sealock Date: Tue, 11 Sep 2018 19:58:11 +1200 Subject: [PATCH] AdvancedFilters: Update Search usage to use 'getLabels' from config --- .../report/orders/{constants.js => config.js} | 6 + .../client/analytics/report/orders/index.js | 2 +- .../products/{constants.js => config.js} | 49 ++++--- .../client/analytics/report/products/index.js | 2 +- .../components/filters/advanced/index.js | 11 +- .../filters/advanced/search-filter.js | 37 +++-- .../components/filters/advanced/test/utils.js | 129 ++++++------------ .../components/filters/advanced/utils.js | 55 ++------ .../client/components/filters/index.js | 1 - 9 files changed, 117 insertions(+), 175 deletions(-) rename plugins/woocommerce-admin/client/analytics/report/orders/{constants.js => config.js} (94%) rename plugins/woocommerce-admin/client/analytics/report/products/{constants.js => config.js} (60%) diff --git a/plugins/woocommerce-admin/client/analytics/report/orders/constants.js b/plugins/woocommerce-admin/client/analytics/report/orders/config.js similarity index 94% rename from plugins/woocommerce-admin/client/analytics/report/orders/constants.js rename to plugins/woocommerce-admin/client/analytics/report/orders/config.js index 251ba6ae2ba..d9c53d8ad19 100644 --- a/plugins/woocommerce-admin/client/analytics/report/orders/constants.js +++ b/plugins/woocommerce-admin/client/analytics/report/orders/config.js @@ -4,6 +4,11 @@ */ import { __ } from '@wordpress/i18n'; +/** + * Internal dependencies + */ +import { getProductLabelsById } from 'analytics/report/products/config'; + export const filters = [ { label: __( 'All Orders', 'wc-admin' ), value: 'all' }, { @@ -55,6 +60,7 @@ export const advancedFilterConfig = { input: { component: 'Search', type: 'products', + getLabels: getProductLabelsById, }, }, code: { diff --git a/plugins/woocommerce-admin/client/analytics/report/orders/index.js b/plugins/woocommerce-admin/client/analytics/report/orders/index.js index e327e574b9b..ecc35b104c0 100644 --- a/plugins/woocommerce-admin/client/analytics/report/orders/index.js +++ b/plugins/woocommerce-admin/client/analytics/report/orders/index.js @@ -13,7 +13,7 @@ import { partial } from 'lodash'; * Internal dependencies */ import { Card, ReportFilters } from '@woocommerce/components'; -import { filters, advancedFilterConfig } from './constants'; +import { filters, advancedFilterConfig } from './config'; import './style.scss'; class OrdersReport extends Component { diff --git a/plugins/woocommerce-admin/client/analytics/report/products/constants.js b/plugins/woocommerce-admin/client/analytics/report/products/config.js similarity index 60% rename from plugins/woocommerce-admin/client/analytics/report/products/constants.js rename to plugins/woocommerce-admin/client/analytics/report/products/config.js index 5f0347c4529..10b7b6068cf 100644 --- a/plugins/woocommerce-admin/client/analytics/report/products/constants.js +++ b/plugins/woocommerce-admin/client/analytics/report/products/config.js @@ -9,6 +9,31 @@ import apiFetch from '@wordpress/api-fetch'; * Internal dependencies */ import { stringifyQuery } from 'lib/nav-utils'; +import { NAMESPACE } from 'store/constants'; + +export const getProductLabelsById = queryString => { + const idList = queryString + .split( ',' ) + .map( id => parseInt( id, 10 ) ) + .filter( Boolean ); + const payload = stringifyQuery( { + include: idList.join( ',' ), + per_page: idList.length, + } ); + return apiFetch( { path: NAMESPACE + 'products' + payload } ); +}; + +export const getCategoryLabelsById = queryString => { + const idList = queryString + .split( ',' ) + .map( id => parseInt( id, 10 ) ) + .filter( Boolean ); + const payload = stringifyQuery( { + include: idList.join( ',' ), + per_page: idList.length, + } ); + return apiFetch( { path: NAMESPACE + 'products/categories' + payload } ); +}; export const filters = [ { label: __( 'All Products', 'wc-admin' ), value: 'all' }, @@ -29,17 +54,7 @@ export const filters = [ settings: { type: 'products', param: 'product', - getLabels: function( queryString ) { - const idList = queryString - .split( ',' ) - .map( id => parseInt( id, 10 ) ) - .filter( Boolean ); - const payload = stringifyQuery( { - include: idList.join( ',' ), - per_page: idList.length, - } ); - return apiFetch( { path: '/wc/v3/products' + payload } ); - }, + getLabels: getProductLabelsById, labels: { title: __( 'Compare Products', 'wc-admin' ), update: __( 'Compare', 'wc-admin' ), @@ -52,17 +67,7 @@ export const filters = [ settings: { type: 'product_cats', param: 'product_cat', - getLabels: function( queryString ) { - const idList = queryString - .split( ',' ) - .map( id => parseInt( id, 10 ) ) - .filter( Boolean ); - const payload = stringifyQuery( { - include: idList.join( ',' ), - per_page: idList.length, - } ); - return apiFetch( { path: '/wc/v3/products/categories' + payload } ); - }, + getLabels: getCategoryLabelsById, labels: { title: __( 'Compare Product Categories', 'wc-admin' ), update: __( 'Compare', 'wc-admin' ), diff --git a/plugins/woocommerce-admin/client/analytics/report/products/index.js b/plugins/woocommerce-admin/client/analytics/report/products/index.js index 3fd9db6ca75..0e51868ce49 100644 --- a/plugins/woocommerce-admin/client/analytics/report/products/index.js +++ b/plugins/woocommerce-admin/client/analytics/report/products/index.js @@ -7,7 +7,7 @@ import { Component, Fragment } from '@wordpress/element'; /** * Internal dependencies */ -import { filters } from './constants'; +import { filters } from './config'; import { ReportFilters } from '@woocommerce/components'; import './style.scss'; diff --git a/plugins/woocommerce-admin/client/components/filters/advanced/index.js b/plugins/woocommerce-admin/client/components/filters/advanced/index.js index 197fac235e7..f20c25ac0fc 100644 --- a/plugins/woocommerce-admin/client/components/filters/advanced/index.js +++ b/plugins/woocommerce-admin/client/components/filters/advanced/index.js @@ -31,11 +31,9 @@ const matches = [ class AdvancedFilters extends Component { constructor( props ) { super( props ); - const activeFiltersFromQuery = getActiveFiltersFromQuery( props.query, props.config ); this.state = { match: matches[ 0 ], - activeFilters: activeFiltersFromQuery, - previousFilters: activeFiltersFromQuery, + activeFilters: getActiveFiltersFromQuery( props.query, props.config ), }; this.filterListRef = createRef(); @@ -107,7 +105,7 @@ class AdvancedFilters extends Component { newFilter.value = filterConfig.input.options[ 0 ].value; } if ( filterConfig.input && 'Search' === filterConfig.input.component ) { - newFilter.value = []; + newFilter.value = ''; } this.setState( state => { return { @@ -129,9 +127,8 @@ class AdvancedFilters extends Component { } getUpdateHref( activeFilters ) { - const { previousFilters } = this.state; - const { path, query } = this.props; - const updatedQuery = getQueryFromActiveFilters( activeFilters, previousFilters ); + const { path, query, config } = this.props; + const updatedQuery = getQueryFromActiveFilters( activeFilters, query, config ); return getNewPath( updatedQuery, path, query ); } diff --git a/plugins/woocommerce-admin/client/components/filters/advanced/search-filter.js b/plugins/woocommerce-admin/client/components/filters/advanced/search-filter.js index 5ad345ada80..3e4be8bbee2 100644 --- a/plugins/woocommerce-admin/client/components/filters/advanced/search-filter.js +++ b/plugins/woocommerce-admin/client/components/filters/advanced/search-filter.js @@ -14,27 +14,38 @@ import PropTypes from 'prop-types'; import Search from 'components/search'; class SearchFilter extends Component { - constructor() { - super(); + constructor( { filter, config } ) { + super( ...arguments ); this.onSearchChange = this.onSearchChange.bind( this ); + this.state = { + selected: [], + }; + + this.updateLabels = this.updateLabels.bind( this ); + + if ( filter.value.length ) { + config.input.getLabels( filter.value ).then( this.updateLabels ); + } + } + + updateLabels( data ) { + const selected = data.map( p => ( { id: p.id, label: p.name } ) ); + this.setState( { selected } ); } onSearchChange( values ) { + this.setState( { + selected: values, + } ); const { filter, onFilterChange } = this.props; - const nextValues = values.map( value => value.id ); - onFilterChange( filter.key, 'value', nextValues ); + const idList = values.map( value => value.id ).join( ',' ); + onFilterChange( filter.key, 'value', idList ); } render() { const { filter, config, onFilterChange } = this.props; - const { key, rule, value } = filter; - const selected = value.map( id => { - // For now - return { - id: parseInt( id, 10 ), - label: id.toString(), - }; - } ); + const { selected } = this.state; + const { key, rule } = filter; return (
{ config.label }
@@ -75,7 +86,7 @@ SearchFilter.propTypes = { filter: PropTypes.shape( { key: PropTypes.string, rule: PropTypes.string, - value: PropTypes.array, + value: PropTypes.string, } ).isRequired, /** * Function to be called on update. diff --git a/plugins/woocommerce-admin/client/components/filters/advanced/test/utils.js b/plugins/woocommerce-admin/client/components/filters/advanced/test/utils.js index 3e5cd8d28a9..e2cc22f2c8f 100644 --- a/plugins/woocommerce-admin/client/components/filters/advanced/test/utils.js +++ b/plugins/woocommerce-admin/client/components/filters/advanced/test/utils.js @@ -9,13 +9,29 @@ /** * Internal dependencies */ -import { - getUrlKey, - getSearchFilterValue, - getActiveFiltersFromQuery, - getUrlValue, - getQueryFromActiveFilters, -} from '../utils'; +import { getUrlKey, getActiveFiltersFromQuery, getQueryFromActiveFilters } from '../utils'; + +const config = { + with_select: { + rules: [ { value: 'is' } ], + input: { + component: 'SelectControl', + options: [ { value: 'pending' } ], + }, + }, + with_search: { + rules: [ { value: 'includes' } ], + input: { + component: 'Search', + }, + }, + with_no_rules: { + input: { + component: 'SelectControl', + options: [ { value: 'pending' } ], + }, + }, +}; describe( 'getUrlKey', () => { it( 'should return a correctly formatted string', () => { @@ -29,51 +45,11 @@ describe( 'getUrlKey', () => { } ); } ); -describe( 'getSearchFilterValue', () => { - it( 'should convert url query param into value readable by Search component', () => { - const str = '1,2,3'; - const values = getSearchFilterValue( str ); - expect( Array.isArray( values ) ).toBeTruthy(); - expect( values[ 0 ] ).toBe( '1' ); - expect( values[ 1 ] ).toBe( '2' ); - expect( values[ 2 ] ).toBe( '3' ); - } ); - - it( 'should convert an empty string into an empty array', () => { - const str = ''; - const values = getSearchFilterValue( str ); - expect( Array.isArray( values ) ).toBeTruthy(); - expect( values.length ).toBe( 0 ); - } ); -} ); - describe( 'getActiveFiltersFromQuery', () => { - const config = { - with_select: { - rules: [ { value: 'is' } ], - input: { - component: 'SelectControl', - options: [ { value: 'pending' } ], - }, - }, - with_search: { - rules: [ { value: 'includes' } ], - input: { - component: 'Search', - }, - }, - with_no_rules: { - input: { - component: 'SelectControl', - options: [ { value: 'pending' } ], - }, - }, - }; - it( 'should return activeFilters from a query', () => { const query = { with_select_is: 'pending', - with_search_includes: '', + with_search_includes: '1,2,3', with_no_rules: 'pending', }; @@ -91,7 +67,7 @@ describe( 'getActiveFiltersFromQuery', () => { const with_search = activeFilters[ 1 ]; expect( with_search.key ).toBe( 'with_search' ); expect( with_search.rule ).toBe( 'includes' ); - expect( with_search.value ).toEqual( [] ); + expect( with_search.value ).toEqual( '1,2,3' ); // with_search const with_no_rules = activeFilters[ 2 ]; @@ -119,28 +95,6 @@ describe( 'getActiveFiltersFromQuery', () => { } ); } ); -describe( 'getUrlValue', () => { - it( 'should pass through a string', () => { - const value = getUrlValue( 'my string' ); - expect( value ).toBe( 'my string' ); - } ); - - it( 'should return null for a non-string value', () => { - const value = getUrlValue( {} ); - expect( value ).toBeNull(); - } ); - - it( 'should return null for an empty array', () => { - const value = getUrlValue( [] ); - expect( value ).toBeNull(); - } ); - - it( 'should return comma separated values when given an array', () => { - const value = getUrlValue( [ 1, 2, 3 ] ); - expect( value ).toBe( '1,2,3' ); - } ); -} ); - describe( 'getQueryFromActiveFilters', () => { it( 'should return a query object from activeFilters', () => { const activeFilters = [ @@ -148,32 +102,27 @@ describe( 'getQueryFromActiveFilters', () => { { key: 'things', rule: 'includes', - value: [ 1, 2, 3 ], + value: '1,2,3', }, { key: 'customer', value: 'new' }, ]; - const query = getQueryFromActiveFilters( activeFilters ); - expect( query.status_is ).toBe( 'open' ); - expect( query.things_includes ).toBe( '1,2,3' ); - expect( query.customer ).toBe( 'new' ); + const query = {}; + const nextQuery = getQueryFromActiveFilters( activeFilters, query, config ); + expect( nextQuery.status_is ).toBe( 'open' ); + expect( nextQuery.things_includes ).toBe( '1,2,3' ); + expect( nextQuery.customer ).toBe( 'new' ); } ); it( 'should remove parameters from the previous filters', () => { - const nextFilters = []; - const previousFilters = [ - { key: 'status', rule: 'is', value: 'open' }, - { - key: 'things', - rule: 'includes', - value: [ 1, 2, 3 ], - }, - { key: 'customer', value: 'new' }, - ]; + const activeFilters = []; + const query = { + with_select_is: 'complete', + with_search_includes: '45', + }; - const query = getQueryFromActiveFilters( nextFilters, previousFilters ); - expect( query.status_is ).toBeUndefined(); - expect( query.things_includes ).toBeUndefined(); - expect( query.customer ).toBeUndefined(); + const nextQuery = getQueryFromActiveFilters( activeFilters, query, config ); + expect( nextQuery.with_select_is ).toBeUndefined(); + expect( nextQuery.with_search_includes ).toBeUndefined(); } ); } ); diff --git a/plugins/woocommerce-admin/client/components/filters/advanced/utils.js b/plugins/woocommerce-admin/client/components/filters/advanced/utils.js index 53c73d27ebf..e5b1d8ebd60 100644 --- a/plugins/woocommerce-admin/client/components/filters/advanced/utils.js +++ b/plugins/woocommerce-admin/client/components/filters/advanced/utils.js @@ -18,23 +18,13 @@ export const getUrlKey = ( key, rule ) => { return key; }; -/** - * Convert url values to array of objects for component - * - * @param {string} str - url query parameter value - * @return {array} - array of Search values - */ -export const getSearchFilterValue = str => { - return str.length ? str.trim().split( ',' ) : []; -}; - /** * Describe activeFilter object. * * @typedef {Object} activeFilter * @property {string} key - filter key. * @property {string} [rule] - a modifying rule for a filter, eg 'includes' or 'is_not'. - * @property {string|array} value - filter value(s). + * @property {string} value - filter value(s). */ /** @@ -54,9 +44,7 @@ export const getActiveFiltersFromQuery = ( query, config ) => { } ); if ( match ) { - const rawValue = query[ getUrlKey( configKey, match.value ) ]; - const value = - 'Search' === filter.input.component ? getSearchFilterValue( rawValue ) : rawValue; + const value = query[ getUrlKey( configKey, match.value ) ]; return { key: configKey, rule: match.value, @@ -76,40 +64,27 @@ export const getActiveFiltersFromQuery = ( query, config ) => { ); }; -/** - * Create a string value for url. Return a string directly or concatenate ids if supplied - * an array of objects. - * - * @param {string|array} value - value of an activeFilter - * @return {string|null} - url query param value - */ -export const getUrlValue = value => { - if ( Array.isArray( value ) ) { - return value.length ? value.join( ',' ) : null; - } - return 'string' === typeof value ? value : null; -}; - /** * Given activeFilters, create a new query object to update the url. Use previousFilters to * Remove unused params. * - * @param {activeFilters[]} nextFilters - activeFilters shown in the UI - * @param {activeFilters[]} previousFilters - filters represented by the current url + * @param {activeFilters[]} activeFilters - activeFilters shown in the UI + * @param {object} query - the current url query object + * @param {object} config - config object * @return {object} - query object representing the new parameters */ -export const getQueryFromActiveFilters = ( nextFilters, previousFilters = [] ) => { - const previousData = previousFilters.reduce( ( query, filter ) => { - query[ getUrlKey( filter.key, filter.rule ) ] = undefined; - return query; +export const getQueryFromActiveFilters = ( activeFilters, query, config ) => { + const previousFilters = getActiveFiltersFromQuery( query, config ); + const previousData = previousFilters.reduce( ( data, filter ) => { + data[ getUrlKey( filter.key, filter.rule ) ] = undefined; + return data; }, {} ); - const data = nextFilters.reduce( ( query, filter ) => { - const urlValue = getUrlValue( filter.value ); - if ( urlValue ) { - query[ getUrlKey( filter.key, filter.rule ) ] = urlValue; + const nextData = activeFilters.reduce( ( data, filter ) => { + if ( filter.value ) { + data[ getUrlKey( filter.key, filter.rule ) ] = filter.value; } - return query; + return data; }, {} ); - return { ...previousData, ...data }; + return { ...previousData, ...nextData }; }; diff --git a/plugins/woocommerce-admin/client/components/filters/index.js b/plugins/woocommerce-admin/client/components/filters/index.js index ff257f64473..2233b1f621f 100644 --- a/plugins/woocommerce-admin/client/components/filters/index.js +++ b/plugins/woocommerce-admin/client/components/filters/index.js @@ -46,7 +46,6 @@ class ReportFilters extends Component { return (