diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index 4974c001c9..92dcad3a3e 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -9,7 +9,8 @@ import FilterTags from '@components/FilterTags'; import { encodeNonDefaultQueryString, parseQueryString, - addParams, + mergeParams, + replaceParams, removeParams, } from '@util/qs'; import { QSConfig } from '@types'; @@ -48,7 +49,7 @@ class ListHeader extends React.Component { const { history, qsConfig } = this.props; const { search } = history.location; const oldParams = parseQueryString(qsConfig, search); - this.pushHistoryState(addParams(qsConfig, oldParams, { [key]: value })); + this.pushHistoryState(mergeParams(oldParams, { [key]: value })); } handleRemove(key, value) { @@ -67,7 +68,7 @@ class ListHeader extends React.Component { const { search } = history.location; const oldParams = parseQueryString(qsConfig, search); this.pushHistoryState( - addParams(qsConfig, oldParams, { + replaceParams(oldParams, { order_by: order === 'ascending' ? key : `-${key}`, page: null, }) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index d238595212..12751eacdf 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -15,7 +15,7 @@ import DataListToolbar from '@components/DataListToolbar'; import { encodeNonDefaultQueryString, parseQueryString, - addParams, + replaceParams, } from '@util/qs'; import { pluralize, ucFirst } from '@util/strings'; @@ -34,16 +34,14 @@ class PaginatedDataList extends React.Component { const { history, qsConfig } = this.props; const { search } = history.location; const oldParams = parseQueryString(qsConfig, search); - this.pushHistoryState(addParams(qsConfig, oldParams, { page: pageNumber })); + this.pushHistoryState(replaceParams(oldParams, { page: pageNumber })); } handleSetPageSize(event, pageSize) { const { history, qsConfig } = this.props; const { search } = history.location; const oldParams = parseQueryString(qsConfig, search); - this.pushHistoryState( - addParams(qsConfig, oldParams, { page_size: pageSize }) - ); + this.pushHistoryState(replaceParams(oldParams, { page_size: pageSize })); } pushHistoryState(params) { diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 1a169399e9..75df652120 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -97,51 +97,6 @@ export const encodeNonDefaultQueryString = (config, params) => { .join('&'); }; -/** - * Merges existing params of query string with new ones and returns the - * updated list of params - * @param {object} qs config object (used for getting defaults, current query params etc.) - * @param {object} object with params from existing query - * @param {object} object with new params to add - * @return {object} query param object - */ -export function addParams(config, oldParams, paramsToAdd) { - // const oldParamsMinusDefaults = getNonDefaultParams(oldParams, config.defaultParams); - // const merged = mergeParams(oldParamsMinusDefaults, paramsToAdd); - // console.log(oldParamsMinusDefaults, merged); - // - // return { - // ...config.defaultParams, - // ...merged, - // ...getRemainingNewParams(merged, paramsToAdd), - // }; - - const namespacedOldParams = namespaceParams(config.namespace, oldParams); - const namespacedParamsToAdd = namespaceParams(config.namespace, paramsToAdd); - const namespacedDefaultParams = namespaceParams( - config.namespace, - config.defaultParams - ); - - const namespacedOldParamsNotDefaults = getNonDefaultParams( - namespacedOldParams, - namespacedDefaultParams - ); - const namespacedMergedParams = mergeParams( - namespacedOldParamsNotDefaults, - namespacedParamsToAdd - ); - - // return updated params. - // If newParams includes updates to the defaults, they will be replaced, - // not concatenated. - return denamespaceParams(config.namespace, { - ...getDefaultParams(namespacedOldParams, namespacedDefaultParams), - ...namespacedMergedParams, - ...getRemainingNewParams(namespacedMergedParams, namespacedParamsToAdd), - }); -} - /** * Removes params from the search string and returns the updated list of params * @param {object} qs config object (used for getting defaults, current query params etc.) @@ -199,7 +154,7 @@ function parseValue(config, key, rawValue) { if (config.integerFields && config.integerFields.some(v => v === key)) { return parseInt(rawValue, 10); } - // TODO: parse date fields + // TODO: parse dateFields into date format? return decodeURIComponent(rawValue); } @@ -246,23 +201,6 @@ const namespaceParams = (namespace, params = {}) => { return namespaced || {}; }; -/** - * helper function to remove namespace from params object - * @param {string} namespace to remove from params - * @param {object} params object to append namespace to - * @return {object} params object with non-namespaced keys - */ -const denamespaceParams = (namespace, params = {}) => { - if (!namespace) return params; - - const denamespaced = {}; - Object.keys(params).forEach(key => { - denamespaced[key.substr(namespace.length + 1)] = params[key]; - }); - - return denamespaced; -}; - /** * helper function to check the namespace of a param is what you expec * @param {string} namespace to append to params @@ -297,46 +235,16 @@ const paramValueIsEqual = (one, two) => { }; /** - * helper function to get params that are defaults - * @param {object} namespaced params object - * @param {object} namespaced params object of default params - * @return {object} namespaced params object of only defaults - */ -const getDefaultParams = (params, defaults) => - arrayToObject( - Object.keys(params) - .filter(key => Object.keys(defaults).indexOf(key) > -1) - .map(key => [key, params[key]]) - ); - -/** - * helper function to get params that are not defaults - * @param {object} namespaced params object - * @param {object} namespaced params object of default params - * @return {object} namespaced params object of non-defaults - */ -const getNonDefaultParams = (params, defaults) => - arrayToObject( - Object.keys(params) - .filter(key => Object.keys(defaults).indexOf(key) === -1) - .map(key => [key, params[key]]) - ); - -/** - * helper function to merge old and new params together - * @param {object} namespaced params object old params with defaults filtered out + * Merge old and new params together, joining values into arrays where necessary + * @param {object} namespaced params object of old params * @param {object} namespaced params object of new params * @return {object} merged namespaced params object */ -// TODO: BUG? newParam that doesn't exist in oldParams isn't added(?) -const mergeParams = (oldParams, newParams) => { - const merged = arrayToObject( - Object.keys(oldParams).map(key => { - const oldVal = oldParams[key]; - const newVal = newParams[key]; - return [key, mergeParam(oldVal, newVal)]; - }) - ); +export function mergeParams(oldParams, newParams) { + const merged = {}; + Object.keys(oldParams).forEach(key => { + merged[key] = mergeParam(oldParams[key], newParams[key]); + }); Object.keys(newParams).forEach(key => { if (!merged[key]) { merged[key] = newParams[key]; @@ -344,7 +252,6 @@ const mergeParams = (oldParams, newParams) => { }); return merged; } -export { mergeParams as _mergeParams }; function mergeParam(oldVal, newVal) { if (!newVal) { @@ -353,22 +260,33 @@ function mergeParam(oldVal, newVal) { if (!oldVal) { return newVal; } + let merged; if (Array.isArray(oldVal)) { - oldVal.push(newVal); - return oldVal; + merged = oldVal.concat(newVal); + } else { + merged = ([oldVal]).concat(newVal); } - return [oldVal, newVal]; + return dedupeArray(merged); +} + +function dedupeArray(arr) { + const deduped = [...new Set(arr)]; + if (deduped.length === 1) { + return deduped[0]; + } + return deduped; } /** - * helper function to get new params that are not in merged params - * @param {object} namespaced params object of merged params + * Join old and new params together, replacing old values with new ones where + * necessary + * @param {object} namespaced params object of old params * @param {object} namespaced params object of new params - * @return {object} remaining new namespaced params object + * @return {object} joined namespaced params object */ -const getRemainingNewParams = (mergedParams, newParams) => - arrayToObject( - Object.keys(newParams) - .filter(key => Object.keys(mergedParams).indexOf(key) === -1) - .map(key => [key, newParams[key]]) - ); +export function replaceParams(oldParams, newParams) { + return { + ...oldParams, + ...newParams, + }; +} diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index d97ebcffd8..c0acb8fbd1 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -7,7 +7,8 @@ import { removeParams, _stringToObject, _addDefaultsToObject, - _mergeParams, + mergeParams, + replaceParams, } from './qs'; describe('qs (qs.js)', () => { @@ -91,7 +92,7 @@ describe('qs (qs.js)', () => { const params = { page: 1, foo: 'bar', - } + }; expect(encodeNonDefaultQueryString(conf, params)).toEqual('item.foo=bar'); }); @@ -287,179 +288,6 @@ describe('qs (qs.js)', () => { }); }); - describe('addParams', () => { - test('should add query params', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: 'bar', page: 3, page_size: 15 }; - const newParams = { bag: 'boom' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: 'bar', - bag: 'boom', - page: 3, - page_size: 15, - }); - }); - - test('should add query params with duplicates', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { baz: 'boom' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang', 'boom'], - page: 3, - page_size: 15, - }); - }); - - test('should replace query params that have defaults', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { page: 5 }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang'], - page: 5, - page_size: 15, - }); - }); - - test('should replace query params that match defaults', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { page_size: 5 }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang'], - page: 3, - page_size: 5, - }); - }); - - test('should add multiple params', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { baz: 'bust', pat: 'pal' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang', 'bust'], - pat: 'pal', - page: 3, - page_size: 15, - }); - }); - - test('should convert param to array when merging', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: 'bar', page: 3, page_size: 15 }; - const newParams = { baz: 'bust', pat: 'pal' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bust'], - pat: 'pal', - page: 3, - page_size: 15, - }); - }); - - test('should add namespaced query params', () => { - const config = { - namespace: 'item', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: 'bar', page: 3, page_size: 15 }; - const newParams = { bag: 'boom' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: 'bar', - bag: 'boom', - page: 3, - page_size: 15, - }); - }); - - test('should not include other namespaced query params when adding', () => { - const config = { - namespace: 'item', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: 'bar', page: 1, page_size: 15 }; - const newParams = { bag: 'boom' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: 'bar', - bag: 'boom', - page: 1, - page_size: 15, - }); - }); - - test('should add namespaced query params with duplicates', () => { - const config = { - namespace: 'item', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { baz: 'boom' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang', 'boom'], - page: 3, - page_size: 15, - }); - }); - - test('should replace namespaced query params that are defaults', () => { - const config = { - namespace: 'item', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { page: 5 }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang'], - page: 5, - page_size: 15, - }); - }); - - test('should add multiple namespaced params', () => { - const config = { - namespace: 'item', - defaultParams: { page: 1, page_size: 15 }, - integerFields: ['page', 'page_size'], - }; - const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; - const newParams = { baz: 'bust', pat: 'pal' }; - expect(addParams(config, oldParams, newParams)).toEqual({ - baz: ['bar', 'bang', 'bust'], - pat: 'pal', - page: 3, - page_size: 15, - }); - }); - }); - describe('removeParams', () => { test('should remove query params', () => { const config = { @@ -726,7 +554,7 @@ describe('qs (qs.js)', () => { const newParams = { foo: 'two', }; - expect(_mergeParams(oldParams, newParams)).toEqual({ + expect(mergeParams(oldParams, newParams)).toEqual({ foo: ['one', 'two'], }); }); @@ -734,28 +562,137 @@ describe('qs (qs.js)', () => { it('should retain unaltered params', () => { const oldParams = { foo: 'one', - bar: 'baz' + bar: 'baz', }; const newParams = { foo: 'two', }; - expect(_mergeParams(oldParams, newParams)).toEqual({ + expect(mergeParams(oldParams, newParams)).toEqual({ foo: ['one', 'two'], bar: 'baz', }); }); - it('should merge objects', () => { + it('should gather params from both objects', () => { const oldParams = { one: 'one', }; const newParams = { two: 'two', }; - expect(_mergeParams(oldParams, newParams)).toEqual({ + expect(mergeParams(oldParams, newParams)).toEqual({ one: 'one', two: 'two', }); }); + + it('should append to array', () => { + const oldParams = { + foo: ['one', 'two'], + }; + const newParams = { + foo: 'three', + }; + expect(mergeParams(oldParams, newParams)).toEqual({ + foo: ['one', 'two', 'three'], + }); + }); + + it('should append array to existing value', () => { + const oldParams = { + foo: 'one', + }; + const newParams = { + foo: ['two', 'three'], + }; + expect(mergeParams(oldParams, newParams)).toEqual({ + foo: ['one', 'two', 'three'], + }); + }); + + it('should merge two arrays', () => { + const oldParams = { + foo: ['one', 'two'], + }; + const newParams = { + foo: ['three', 'four'], + }; + expect(mergeParams(oldParams, newParams)).toEqual({ + foo: ['one', 'two', 'three', 'four'], + }); + }); + + it('should prevent exact duplicates', () => { + const oldParams = { foo: 'one' }; + const newParams = { foo: 'one' }; + expect(mergeParams(oldParams, newParams)).toEqual({ foo: 'one' }); + }); + + it('should prevent exact duplicates in arrays', () => { + const oldParams = { foo: ['one', 'three'] }; + const newParams = { foo: ['one', 'two'] }; + expect(mergeParams(oldParams, newParams)).toEqual({ + foo: ['one', 'three', 'two'], + }); + }); + + it('should add multiple params', () => { + const oldParams = { baz: ['bar', 'bang'], page: 3, page_size: 15 }; + const newParams = { baz: 'bust', pat: 'pal' }; + expect(mergeParams(oldParams, newParams)).toEqual({ + baz: ['bar', 'bang', 'bust'], + pat: 'pal', + page: 3, + page_size: 15, + }); + }) + }); + + describe('replaceParams', () => { + it('should collect params into one object', () => { + const oldParams = { foo: 'one' }; + const newParams = { bar: 'two' }; + expect(replaceParams(oldParams, newParams)).toEqual({ + foo: 'one', + bar: 'two', + }); + }); + + it('should retain unaltered params', () => { + const oldParams = { + foo: 'one', + bar: 'baz', + }; + const newParams = { foo: 'two' }; + expect(replaceParams(oldParams, newParams)).toEqual({ + foo: 'two', + bar: 'baz', + }); + }); + + it('should override old values with new ones', () => { + const oldParams = { + foo: 'one', + bar: 'three', + }; + const newParams = { + foo: 'two', + baz: 'four', + }; + expect(replaceParams(oldParams, newParams)).toEqual({ + foo: 'two', + bar: 'three', + baz: 'four', + }); + }); + + it('should handle exact duplicates', () => { + const oldParams = { foo: 'one' }; + const newParams = { foo: 'one', bar: 'two' }; + expect(replaceParams(oldParams, newParams)).toEqual({ + foo: 'one', + bar: 'two', + }); + }); }); });