From f8374def64d8c5ad9ad344699ffa4b45ddb2567e Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 17 May 2021 14:40:32 -0700 Subject: [PATCH] refactor ListHeader to use replaceNamespacedParams --- .../src/components/ListHeader/ListHeader.jsx | 70 +++++++++---------- .../components/ListHeader/ListHeader.test.jsx | 4 +- awx/ui_next/src/util/qs.js | 5 +- awx/ui_next/src/util/qs.test.js | 9 +++ 4 files changed, 48 insertions(+), 40 deletions(-) diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index d5e359c3bc..3e200a7c45 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -6,11 +6,10 @@ import { Toolbar, ToolbarContent } from '@patternfly/react-core'; import DataListToolbar from '../DataListToolbar'; import { - encodeNonDefaultQueryString, parseQueryString, mergeParams, - replaceParams, removeParams, + replaceNamespacedParams, } from '../../util/qs'; import { QSConfig, SearchColumns, SortColumns } from '../../types'; @@ -38,60 +37,59 @@ class ListHeader extends React.Component { handleSearch(key, value) { const { location, qsConfig } = this.props; - let params = parseQueryString(qsConfig, location.search); - params = mergeParams(params, { [key]: value }); - params = replaceParams(params, { page: 1 }); - this.pushHistoryState(params); + const params = parseQueryString(qsConfig, location.search); + const qs = replaceNamespacedParams(qsConfig, location.search, { + ...mergeParams(params, { [key]: value }), + page: 1, + }); + this.pushHistoryState(qs); } handleReplaceSearch(key, value) { const { location, qsConfig } = this.props; - const oldParams = parseQueryString(qsConfig, location.search); - this.pushHistoryState(replaceParams(oldParams, { [key]: value })); + const qs = replaceNamespacedParams(qsConfig, location.search, { + [key]: value, + }); + this.pushHistoryState(qs); } handleRemove(key, value) { const { location, qsConfig } = this.props; - let oldParams = parseQueryString(qsConfig, location.search); - if (parseInt(value, 10)) { - oldParams = removeParams(qsConfig, oldParams, { - [key]: parseInt(value, 10), - }); - } - this.pushHistoryState(removeParams(qsConfig, oldParams, { [key]: value })); + const oldParams = parseQueryString(qsConfig, location.search); + const updatedParams = removeParams(qsConfig, oldParams, { + [key]: value, + }); + const qs = replaceNamespacedParams( + qsConfig, + location.search, + updatedParams + ); + this.pushHistoryState(qs); } handleRemoveAll() { - // remove everything in oldParams except for page_size and order_by const { location, qsConfig } = this.props; const oldParams = parseQueryString(qsConfig, location.search); - const oldParamsClone = { ...oldParams }; - delete oldParamsClone.page_size; - delete oldParamsClone.order_by; - this.pushHistoryState(removeParams(qsConfig, oldParams, oldParamsClone)); + Object.keys(oldParams).forEach(key => { + oldParams[key] = null; + }); + const qs = replaceNamespacedParams(qsConfig, location.search, oldParams); + this.pushHistoryState(qs); } handleSort(key, order) { const { location, qsConfig } = this.props; - const oldParams = parseQueryString(qsConfig, location.search); - this.pushHistoryState( - replaceParams(oldParams, { - order_by: order === 'ascending' ? key : `-${key}`, - page: null, - }) - ); + const qs = replaceNamespacedParams(qsConfig, location.search, { + order_by: order === 'ascending' ? key : `-${key}`, + page: null, + }); + this.pushHistoryState(qs); } - pushHistoryState(params) { - const { history, qsConfig } = this.props; + pushHistoryState(queryString) { + const { history } = this.props; const { pathname } = history.location; - const nonNamespacedParams = parseQueryString({}, history.location.search); - const encodedParams = encodeNonDefaultQueryString( - qsConfig, - params, - nonNamespacedParams - ); - history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + history.push(queryString ? `${pathname}?${queryString}` : pathname); } render() { diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx index d501418c44..76ddd2b8af 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx @@ -51,7 +51,7 @@ describe('ListHeader', () => { expect(history.location.search).toEqual(''); }); - test('should test clear all', () => { + test('should clear all', () => { const query = '?item.page_size=5&item.name=foo'; const history = createMemoryHistory({ initialEntries: [`/organizations/1/teams${query}`], @@ -71,7 +71,7 @@ describe('ListHeader', () => { expect(history.location.search).toEqual(query); const toolbar = wrapper.find('DataListToolbar'); toolbar.prop('clearAllFilters')(); - expect(history.location.search).toEqual('?item.page_size=5'); + expect(history.location.search).toEqual(''); }); test('should test handle search', () => { diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 75981ce192..fd952cba46 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -164,9 +164,10 @@ export function removeParams(config, oldParams, paramsToRemove) { }; Object.keys(oldParams).forEach(key => { const value = removeParam(oldParams[key], paramsToRemove[key]); - if (value !== null) { - updated[key] = value; + if (value == null && Object.prototype.hasOwnProperty.call(updated, key)) { + return; } + updated[key] = value; }); return updated; } diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index f3886ab7eb..ee37ce05b6 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -341,6 +341,7 @@ describe('qs (qs.js)', () => { baz: 'bar', page: 3, page_size: 15, + bag: null, }); }); @@ -429,6 +430,7 @@ describe('qs (qs.js)', () => { baz: ['bar', 'bang'], page: 3, page_size: 15, + pat: null, }); }); @@ -443,6 +445,7 @@ describe('qs (qs.js)', () => { expect(removeParams(config, oldParams, toRemove)).toEqual({ page: 3, page_size: 15, + baz: null, }); }); @@ -457,6 +460,7 @@ describe('qs (qs.js)', () => { expect(removeParams(config, oldParams, toRemove)).toEqual({ page: 1, page_size: 15, + baz: null, }); }); @@ -526,6 +530,7 @@ describe('qs (qs.js)', () => { baz: ['one', 'two', 'three'], page: 3, page_size: 15, + bag: null, }); }); @@ -546,6 +551,7 @@ describe('qs (qs.js)', () => { baz: ['bar', 'bang'], page: 3, page_size: 15, + pat: null, }); }); @@ -559,6 +565,7 @@ describe('qs (qs.js)', () => { const toRemove = { bag: 'boom' }; expect(removeParams(config, oldParams, toRemove)).toEqual({ baz: '', + bag: null, page: 3, page_size: 15, }); @@ -860,6 +867,8 @@ describe('qs (qs.js)', () => { ); }); + // This fix needed after we're confident refactoring components + // to use replaceNamespacedParams provides equivalent functionality test.skip('should not alter params of other namespaces', () => { const query = 'template.name__icontains=workflow&template.page=2&credential.page=3';