From 9bbaa6993f1f7001b5809b617d27dec816dc9447 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Fri, 14 May 2021 15:12:53 -0700 Subject: [PATCH 1/9] refactor param changes to use new util function --- .../PaginatedDataList/PaginatedDataList.jsx | 28 ++++----- .../components/PaginatedTable/HeaderRow.jsx | 14 +---- .../PaginatedTable/PaginatedTable.jsx | 32 ++++------ .../InventoryList/useWsInventories.js | 15 ++--- awx/ui_next/src/util/qs.js | 19 ++++++ awx/ui_next/src/util/qs.test.js | 61 +++++++++++++++++++ awx/ui_next/src/util/useRequest.js | 10 ++- 7 files changed, 114 insertions(+), 65 deletions(-) diff --git a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx index aaeed5455b..6a30d821b0 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -13,11 +13,7 @@ import ContentLoading from '../ContentLoading'; import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; -import { - encodeNonDefaultQueryString, - parseQueryString, - replaceParams, -} from '../../util/qs'; +import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; import { QSConfig, SearchColumns, SortColumns } from '../../types'; @@ -40,7 +36,6 @@ function PaginatedDataList({ pluralizedItemName, showPageSizeOptions, location, - renderToolbar, }) { const { search, pathname } = useLocation(); @@ -51,22 +46,21 @@ function PaginatedDataList({ }; const handleSetPage = (event, pageNumber) => { - const oldParams = parseQueryString(qsConfig, search); - pushHistoryState(replaceParams(oldParams, { page: pageNumber })); + const encodedParams = replaceNamespacedParams(qsConfig, search, { + page: pageNumber, + }); + pushHistoryState(encodedParams); }; const handleSetPageSize = (event, pageSize, page) => { - const oldParams = parseQueryString(qsConfig, search); - pushHistoryState(replaceParams(oldParams, { page_size: pageSize, page })); + const encodedParams = replaceNamespacedParams(qsConfig, search, { + page_size: pageSize, + page, + }); + pushHistoryState(encodedParams); }; - const pushHistoryState = params => { - const nonNamespacedParams = parseQueryString({}, history.location.search); - const encodedParams = encodeNonDefaultQueryString( - qsConfig, - params, - nonNamespacedParams - ); + const pushHistoryState = encodedParams => { history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); }; diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index a7b076da57..368c3eac23 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -3,11 +3,7 @@ import React from 'react'; import { useLocation, useHistory } from 'react-router-dom'; import { Thead, Tr, Th as PFTh } from '@patternfly/react-table'; import styled from 'styled-components'; -import { - encodeNonDefaultQueryString, - parseQueryString, - replaceParams, -} from '../../util/qs'; +import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; const Th = styled(PFTh)` --pf-c-table--cell--Overflow: initial; @@ -25,16 +21,10 @@ export default function HeaderRow({ const params = parseQueryString(qsConfig, location.search); const onSort = (key, order) => { - const newParams = replaceParams(params, { + const encodedParams = replaceNamespacedParams(qsConfig, location.search, { order_by: order === 'asc' ? key : `-${key}`, page: null, }); - const nonNamespacedParams = parseQueryString({}, history.location.search); - const encodedParams = encodeNonDefaultQueryString( - qsConfig, - newParams, - nonNamespacedParams - ); history.push( encodedParams ? `${location.pathname}?${encodedParams}` diff --git a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx index 5bced479a6..d73082162a 100644 --- a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx +++ b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx @@ -4,7 +4,7 @@ import PropTypes from 'prop-types'; import { TableComposable, Tbody } from '@patternfly/react-table'; import { t } from '@lingui/macro'; -import { useHistory } from 'react-router-dom'; +import { useLocation, useHistory } from 'react-router-dom'; import ListHeader from '../ListHeader'; import ContentEmpty from '../ContentEmpty'; @@ -14,11 +14,7 @@ import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; import LoadingSpinner from '../LoadingSpinner'; -import { - encodeNonDefaultQueryString, - parseQueryString, - replaceParams, -} from '../../util/qs'; +import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; import { QSConfig, SearchColumns } from '../../types'; function PaginatedTable({ @@ -35,32 +31,30 @@ function PaginatedTable({ toolbarRelatedSearchableKeys, pluralizedItemName, showPageSizeOptions, - renderToolbar, emptyContentMessage, ouiaId, }) { + const { search, pathname } = useLocation(); const history = useHistory(); - const pushHistoryState = params => { - const { pathname, search } = history.location; - const nonNamespacedParams = parseQueryString({}, search); - const encodedParams = encodeNonDefaultQueryString( - qsConfig, - params, - nonNamespacedParams - ); + const pushHistoryState = encodedParams => { history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); }; const handleSetPage = (event, pageNumber) => { - const oldParams = parseQueryString(qsConfig, history.location.search); - pushHistoryState(replaceParams(oldParams, { page: pageNumber })); + const encodedParams = replaceNamespacedParams(qsConfig, search, { + page: pageNumber, + }); + pushHistoryState(encodedParams); }; const handleSetPageSize = (event, pageSize, page) => { - const oldParams = parseQueryString(qsConfig, history.location.search); - pushHistoryState(replaceParams(oldParams, { page_size: pageSize, page })); + const encodedParams = replaceNamespacedParams(qsConfig, search, { + page_size: pageSize, + page, + }); + pushHistoryState(encodedParams); }; const searchColumns = toolbarSearchColumns.length diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js index 7797256064..c975100906 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js +++ b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js @@ -1,10 +1,6 @@ import { useState, useEffect } from 'react'; import { useLocation, useHistory } from 'react-router-dom'; -import { - parseQueryString, - replaceParams, - encodeNonDefaultQueryString, -} from '../../../util/qs'; +import { parseQueryString, replaceNamespacedParams } from '../../../util/qs'; import useWebsocket from '../../../util/useWebsocket'; import useThrottle from '../../../util/useThrottle'; @@ -90,12 +86,9 @@ export default function useWsInventories( ) { // We've deleted the last inventory on this page so we'll // try to navigate back to the previous page - const newParams = encodeNonDefaultQueryString( - qsConfig, - replaceParams(params, { - page: params.page - 1, - }) - ); + const newParams = replaceNamespacedParams(qsConfig, location.search, { + page: params.page - 1, + }); history.push(`${location.pathname}?${newParams}`); return; } diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 729a28790d..75981ce192 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -246,3 +246,22 @@ export function replaceParams(oldParams, newParams) { ...newParams, }; } + +/** + * Update namespaced param(s), returning a new query string. Leaves params + * from other namespaces unaltered + * @param {object} qs config object for namespacing params, filtering defaults + * @param {string} the url query string to update + * @param {object} namespaced params to add or update + * @return {string} url query string + */ +export function replaceNamespacedParams(config, queryString, newParams) { + const oldParams = parseQueryString(config, queryString); + const updatedParams = replaceParams(oldParams, newParams); + const nonNamespacedParams = parseQueryString({}, queryString); + return encodeNonDefaultQueryString( + config, + updatedParams, + nonNamespacedParams + ); +} diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index 763b07434c..f3886ab7eb 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -8,6 +8,7 @@ import { _addDefaultsToObject, mergeParams, replaceParams, + replaceNamespacedParams, } from './qs'; describe('qs (qs.js)', () => { @@ -810,4 +811,64 @@ describe('qs (qs.js)', () => { }); }); }); + + describe('replaceNamespacedParams', () => { + const config = { + namespace: 'template', + defaultParams: { page: 1, page_size: 5, order_by: 'name' }, + integerFields: ['page'], + }; + + test('should update namespaced param', () => { + const query = 'template.name__icontains=workflow&template.page=2'; + const newParams = { + page: 3, + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'template.name__icontains=workflow&template.page=3' + ); + }); + + test('should add new namespaced param', () => { + const query = 'template.name__icontains=workflow&template.page=2'; + const newParams = { + or__type: 'job_template', + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'template.name__icontains=workflow&template.or__type=job_template&template.page=2' + ); + }); + + test('should maintain non-namespaced param', () => { + const query = 'foo=bar&template.page=2&template.name__icontains=workflow'; + const newParams = { + page: 3, + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'foo=bar&template.name__icontains=workflow&template.page=3' + ); + }); + + test('should omit null values', () => { + const query = 'template.name__icontains=workflow&template.page=2'; + const newParams = { + page: 3, + name__icontains: null, + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'template.page=3' + ); + }); + + test.skip('should not alter params of other namespaces', () => { + const query = + 'template.name__icontains=workflow&template.page=2&credential.page=3'; + const newParams = { + page: 3, + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'template.name__icontains=workflow&template.page=3&credential.page=3' + ); + }); + }); }); diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index c3328ce7c7..50be3fc49a 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -4,6 +4,7 @@ import { parseQueryString, replaceParams, encodeNonDefaultQueryString, + replaceNamespacedParams, } from './qs'; import useIsMounted from './useIsMounted'; @@ -111,12 +112,9 @@ export function useDeleteItems( } const params = parseQueryString(qsConfig, location.search); if (params.page > 1 && allItemsSelected) { - const newParams = encodeNonDefaultQueryString( - qsConfig, - replaceParams(params, { - page: params.page - 1, - }) - ); + const newParams = replaceNamespacedParams(qsConfig, location.search, { + page: params.page - 1, + }); history.push(`${location.pathname}?${newParams}`); } else { fetchItems(); From f8374def64d8c5ad9ad344699ffa4b45ddb2567e Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 17 May 2021 14:40:32 -0700 Subject: [PATCH 2/9] 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'; From 25903431bc7994e35e50e52944eb4be10a129b29 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Mon, 17 May 2021 15:25:16 -0700 Subject: [PATCH 3/9] refactor JobOutput to use replaceNamespacedParams util --- .../src/screens/Job/JobOutput/JobOutput.jsx | 47 ++++++++++++------- awx/ui_next/src/util/useRequest.js | 7 +-- 2 files changed, 30 insertions(+), 24 deletions(-) diff --git a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx index 19f1b0f751..e4282e9922 100644 --- a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx +++ b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx @@ -39,12 +39,11 @@ import getRowRangePageSize from './shared/jobOutputUtils'; import { getJobModel, isJobRunning } from '../../../util/jobs'; import useRequest, { useDismissableError } from '../../../util/useRequest'; import { - encodeNonDefaultQueryString, parseQueryString, mergeParams, - replaceParams, removeParams, getQSConfig, + replaceNamespacedParams, } from '../../../util/qs'; import useIsMounted from '../../../util/useIsMounted'; @@ -589,35 +588,47 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { }; const handleSearch = (key, value) => { - let params = parseQueryString(QS_CONFIG, location.search); - params = mergeParams(params, { [key]: value }); - pushHistoryState(params); + const params = parseQueryString(QS_CONFIG, location.search); + const qs = replaceNamespacedParams( + QS_CONFIG, + location.search, + mergeParams(params, { [key]: value }) + ); + pushHistoryState(qs); }; const handleReplaceSearch = (key, value) => { - const oldParams = parseQueryString(QS_CONFIG, location.search); - pushHistoryState(replaceParams(oldParams, { [key]: value })); + const qs = replaceNamespacedParams(QS_CONFIG, location.search, { + [key]: value, + }); + pushHistoryState(qs); }; const handleRemoveSearchTerm = (key, value) => { - let oldParams = parseQueryString(QS_CONFIG, location.search); - if (parseInt(value, 10)) { - oldParams = removeParams(QS_CONFIG, oldParams, { - [key]: parseInt(value, 10), - }); - } - pushHistoryState(removeParams(QS_CONFIG, oldParams, { [key]: value })); + const oldParams = parseQueryString(QS_CONFIG, location.search); + const updatedParams = removeParams(QS_CONFIG, oldParams, { + [key]: value, + }); + const qs = replaceNamespacedParams( + QS_CONFIG, + location.search, + updatedParams + ); + pushHistoryState(qs); }; const handleRemoveAllSearchTerms = () => { const oldParams = parseQueryString(QS_CONFIG, location.search); - pushHistoryState(removeParams(QS_CONFIG, oldParams, { ...oldParams })); + Object.keys(oldParams).forEach(key => { + oldParams[key] = null; + }); + const qs = replaceNamespacedParams(QS_CONFIG, location.search, oldParams); + pushHistoryState(qs); }; - const pushHistoryState = params => { + const pushHistoryState = qs => { const { pathname } = history.location; - const encodedParams = encodeNonDefaultQueryString(QS_CONFIG, params); - history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + history.push(qs ? `${pathname}?${qs}` : pathname); }; const renderSearchComponent = () => ( diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index 50be3fc49a..e9392b3238 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -1,11 +1,6 @@ import { useEffect, useState, useCallback } from 'react'; import { useLocation, useHistory } from 'react-router-dom'; -import { - parseQueryString, - replaceParams, - encodeNonDefaultQueryString, - replaceNamespacedParams, -} from './qs'; +import { parseQueryString, replaceNamespacedParams } from './qs'; import useIsMounted from './useIsMounted'; /* From fd5e22a3f6ecfac0f8d47685503e6f5f87cff895 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Tue, 18 May 2021 12:00:20 -0700 Subject: [PATCH 4/9] fix integer fields in removeParams; maintain page_size/sort --- .../src/components/ListHeader/ListHeader.jsx | 2 ++ .../components/ListHeader/ListHeader.test.jsx | 2 +- awx/ui_next/src/util/qs.js | 17 +++++++--- awx/ui_next/src/util/qs.test.js | 32 +++++++++++++++++++ 4 files changed, 48 insertions(+), 5 deletions(-) diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index 3e200a7c45..d24d3899ba 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -73,6 +73,8 @@ class ListHeader extends React.Component { Object.keys(oldParams).forEach(key => { oldParams[key] = null; }); + delete oldParams.page_size; + delete oldParams.order_by; const qs = replaceNamespacedParams(qsConfig, location.search, oldParams); this.pushHistoryState(qs); } diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx index 76ddd2b8af..f7199fe4d4 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.test.jsx @@ -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(''); + expect(history.location.search).toEqual('?item.page_size=5'); }); test('should test handle search', () => { diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index fd952cba46..a5e0005626 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -163,11 +163,19 @@ export function removeParams(config, oldParams, paramsToRemove) { ...config.defaultParams, }; Object.keys(oldParams).forEach(key => { - const value = removeParam(oldParams[key], paramsToRemove[key]); - if (value == null && Object.prototype.hasOwnProperty.call(updated, key)) { + const valToRemove = paramsToRemove[key]; + const isInt = config.integerFields?.includes(key); + const updatedValue = removeParam( + oldParams[key], + isInt ? parseInt(valToRemove, 10) : valToRemove + ); + if ( + updatedValue == null && + Object.prototype.hasOwnProperty.call(updated, key) + ) { return; } - updated[key] = value; + updated[key] = updatedValue; }); return updated; } @@ -253,7 +261,8 @@ export function replaceParams(oldParams, newParams) { * from other namespaces unaltered * @param {object} qs config object for namespacing params, filtering defaults * @param {string} the url query string to update - * @param {object} namespaced params to add or update + * @param {object} namespaced params to add or update. use null to indicate + * a param should be deleted from the query string * @return {string} url query string */ export function replaceNamespacedParams(config, queryString, newParams) { diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index ee37ce05b6..d1806a2272 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -570,6 +570,38 @@ describe('qs (qs.js)', () => { page_size: 15, }); }); + + test('should remove integer fields when given string value', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['id', 'page', 'page_size'], + }; + const oldParams = { id: 199, foo: 'bar', page: 1, page_size: 15 }; + const toRemove = { id: '199' }; + expect(removeParams(config, oldParams, toRemove)).toEqual({ + foo: 'bar', + id: null, + page: 1, + page_size: 15, + }); + }); + + test('should remove integer fields from array when given string value', () => { + const config = { + namespace: null, + defaultParams: { page: 1, page_size: 15 }, + integerFields: ['id', 'page', 'page_size'], + }; + const oldParams = { id: [199, 200], foo: 'bar', page: 1, page_size: 15 }; + const toRemove = { id: '199' }; + expect(removeParams(config, oldParams, toRemove)).toEqual({ + foo: 'bar', + id: 200, + page: 1, + page_size: 15, + }); + }); }); describe('_stringToObject', () => { From d324c12348572d3aa9045d7feffc88e03597a91d Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Tue, 18 May 2021 12:20:24 -0700 Subject: [PATCH 5/9] add qs test confirming default values are omitted --- awx/ui_next/src/util/qs.test.js | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index d1806a2272..5c0d8ed1b6 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -899,6 +899,17 @@ describe('qs (qs.js)', () => { ); }); + test('should omit default values', () => { + const query = 'template.page=2'; + const newParams = { + page: 3, + page_size: 5, + }; + expect(replaceNamespacedParams(config, query, newParams)).toEqual( + 'template.page=3' + ); + }); + // This fix needed after we're confident refactoring components // to use replaceNamespacedParams provides equivalent functionality test.skip('should not alter params of other namespaces', () => { From 7f6e022852d51108d96fe1900bca1301a50a3eca Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Tue, 18 May 2021 14:15:26 -0700 Subject: [PATCH 6/9] rename replaceNamespacedParams to updateQueryString --- .../src/components/ListHeader/ListHeader.jsx | 16 ++++++---------- .../PaginatedDataList/PaginatedDataList.jsx | 14 +++++++------- .../components/PaginatedTable/HeaderRow.jsx | 10 +++------- .../PaginatedTable/PaginatedTable.jsx | 14 +++++++------- .../InventoryList/useWsInventories.js | 6 +++--- .../src/screens/Job/JobOutput/JobOutput.jsx | 14 +++++--------- awx/ui_next/src/util/qs.js | 4 ++-- awx/ui_next/src/util/qs.test.js | 18 +++++++++--------- awx/ui_next/src/util/useRequest.js | 6 +++--- 9 files changed, 45 insertions(+), 57 deletions(-) diff --git a/awx/ui_next/src/components/ListHeader/ListHeader.jsx b/awx/ui_next/src/components/ListHeader/ListHeader.jsx index d24d3899ba..b974f1d410 100644 --- a/awx/ui_next/src/components/ListHeader/ListHeader.jsx +++ b/awx/ui_next/src/components/ListHeader/ListHeader.jsx @@ -9,7 +9,7 @@ import { parseQueryString, mergeParams, removeParams, - replaceNamespacedParams, + updateQueryString, } from '../../util/qs'; import { QSConfig, SearchColumns, SortColumns } from '../../types'; @@ -38,7 +38,7 @@ class ListHeader extends React.Component { handleSearch(key, value) { const { location, qsConfig } = this.props; const params = parseQueryString(qsConfig, location.search); - const qs = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { ...mergeParams(params, { [key]: value }), page: 1, }); @@ -47,7 +47,7 @@ class ListHeader extends React.Component { handleReplaceSearch(key, value) { const { location, qsConfig } = this.props; - const qs = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { [key]: value, }); this.pushHistoryState(qs); @@ -59,11 +59,7 @@ class ListHeader extends React.Component { const updatedParams = removeParams(qsConfig, oldParams, { [key]: value, }); - const qs = replaceNamespacedParams( - qsConfig, - location.search, - updatedParams - ); + const qs = updateQueryString(qsConfig, location.search, updatedParams); this.pushHistoryState(qs); } @@ -75,13 +71,13 @@ class ListHeader extends React.Component { }); delete oldParams.page_size; delete oldParams.order_by; - const qs = replaceNamespacedParams(qsConfig, location.search, oldParams); + const qs = updateQueryString(qsConfig, location.search, oldParams); this.pushHistoryState(qs); } handleSort(key, order) { const { location, qsConfig } = this.props; - const qs = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { 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 6a30d821b0..74a0596de1 100644 --- a/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx +++ b/awx/ui_next/src/components/PaginatedDataList/PaginatedDataList.jsx @@ -13,7 +13,7 @@ import ContentLoading from '../ContentLoading'; import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; -import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; +import { parseQueryString, updateQueryString } from '../../util/qs'; import { QSConfig, SearchColumns, SortColumns } from '../../types'; @@ -46,22 +46,22 @@ function PaginatedDataList({ }; const handleSetPage = (event, pageNumber) => { - const encodedParams = replaceNamespacedParams(qsConfig, search, { + const qs = updateQueryString(qsConfig, search, { page: pageNumber, }); - pushHistoryState(encodedParams); + pushHistoryState(qs); }; const handleSetPageSize = (event, pageSize, page) => { - const encodedParams = replaceNamespacedParams(qsConfig, search, { + const qs = updateQueryString(qsConfig, search, { page_size: pageSize, page, }); - pushHistoryState(encodedParams); + pushHistoryState(qs); }; - const pushHistoryState = encodedParams => { - history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + const pushHistoryState = qs => { + history.push(qs ? `${pathname}?${qs}` : pathname); }; const searchColumns = toolbarSearchColumns.length diff --git a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx index 368c3eac23..230a61705b 100644 --- a/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx +++ b/awx/ui_next/src/components/PaginatedTable/HeaderRow.jsx @@ -3,7 +3,7 @@ import React from 'react'; import { useLocation, useHistory } from 'react-router-dom'; import { Thead, Tr, Th as PFTh } from '@patternfly/react-table'; import styled from 'styled-components'; -import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; +import { parseQueryString, updateQueryString } from '../../util/qs'; const Th = styled(PFTh)` --pf-c-table--cell--Overflow: initial; @@ -21,15 +21,11 @@ export default function HeaderRow({ const params = parseQueryString(qsConfig, location.search); const onSort = (key, order) => { - const encodedParams = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { order_by: order === 'asc' ? key : `-${key}`, page: null, }); - history.push( - encodedParams - ? `${location.pathname}?${encodedParams}` - : location.pathname - ); + history.push(qs ? `${location.pathname}?${qs}` : location.pathname); }; const sortKey = params.order_by?.replace('-', ''); diff --git a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx index d73082162a..7fd76a207d 100644 --- a/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx +++ b/awx/ui_next/src/components/PaginatedTable/PaginatedTable.jsx @@ -14,7 +14,7 @@ import Pagination from '../Pagination'; import DataListToolbar from '../DataListToolbar'; import LoadingSpinner from '../LoadingSpinner'; -import { parseQueryString, replaceNamespacedParams } from '../../util/qs'; +import { parseQueryString, updateQueryString } from '../../util/qs'; import { QSConfig, SearchColumns } from '../../types'; function PaginatedTable({ @@ -38,23 +38,23 @@ function PaginatedTable({ const { search, pathname } = useLocation(); const history = useHistory(); - const pushHistoryState = encodedParams => { - history.push(encodedParams ? `${pathname}?${encodedParams}` : pathname); + const pushHistoryState = qs => { + history.push(qs ? `${pathname}?${qs}` : pathname); }; const handleSetPage = (event, pageNumber) => { - const encodedParams = replaceNamespacedParams(qsConfig, search, { + const qs = updateQueryString(qsConfig, search, { page: pageNumber, }); - pushHistoryState(encodedParams); + pushHistoryState(qs); }; const handleSetPageSize = (event, pageSize, page) => { - const encodedParams = replaceNamespacedParams(qsConfig, search, { + const qs = updateQueryString(qsConfig, search, { page_size: pageSize, page, }); - pushHistoryState(encodedParams); + pushHistoryState(qs); }; const searchColumns = toolbarSearchColumns.length diff --git a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js index c975100906..b595cdf028 100644 --- a/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js +++ b/awx/ui_next/src/screens/Inventory/InventoryList/useWsInventories.js @@ -1,6 +1,6 @@ import { useState, useEffect } from 'react'; import { useLocation, useHistory } from 'react-router-dom'; -import { parseQueryString, replaceNamespacedParams } from '../../../util/qs'; +import { parseQueryString, updateQueryString } from '../../../util/qs'; import useWebsocket from '../../../util/useWebsocket'; import useThrottle from '../../../util/useThrottle'; @@ -86,10 +86,10 @@ export default function useWsInventories( ) { // We've deleted the last inventory on this page so we'll // try to navigate back to the previous page - const newParams = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { page: params.page - 1, }); - history.push(`${location.pathname}?${newParams}`); + history.push(`${location.pathname}?${qs}`); return; } diff --git a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx index e4282e9922..dfcdc90178 100644 --- a/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx +++ b/awx/ui_next/src/screens/Job/JobOutput/JobOutput.jsx @@ -43,7 +43,7 @@ import { mergeParams, removeParams, getQSConfig, - replaceNamespacedParams, + updateQueryString, } from '../../../util/qs'; import useIsMounted from '../../../util/useIsMounted'; @@ -589,7 +589,7 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { const handleSearch = (key, value) => { const params = parseQueryString(QS_CONFIG, location.search); - const qs = replaceNamespacedParams( + const qs = updateQueryString( QS_CONFIG, location.search, mergeParams(params, { [key]: value }) @@ -598,7 +598,7 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { }; const handleReplaceSearch = (key, value) => { - const qs = replaceNamespacedParams(QS_CONFIG, location.search, { + const qs = updateQueryString(QS_CONFIG, location.search, { [key]: value, }); pushHistoryState(qs); @@ -609,11 +609,7 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { const updatedParams = removeParams(QS_CONFIG, oldParams, { [key]: value, }); - const qs = replaceNamespacedParams( - QS_CONFIG, - location.search, - updatedParams - ); + const qs = updateQueryString(QS_CONFIG, location.search, updatedParams); pushHistoryState(qs); }; @@ -622,7 +618,7 @@ function JobOutput({ job, eventRelatedSearchableKeys, eventSearchableKeys }) { Object.keys(oldParams).forEach(key => { oldParams[key] = null; }); - const qs = replaceNamespacedParams(QS_CONFIG, location.search, oldParams); + const qs = updateQueryString(QS_CONFIG, location.search, oldParams); pushHistoryState(qs); }; diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index a5e0005626..f3eaaa6cd1 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -262,10 +262,10 @@ export function replaceParams(oldParams, newParams) { * @param {object} qs config object for namespacing params, filtering defaults * @param {string} the url query string to update * @param {object} namespaced params to add or update. use null to indicate - * a param should be deleted from the query string + * a param that should be deleted from the query string * @return {string} url query string */ -export function replaceNamespacedParams(config, queryString, newParams) { +export function updateQueryString(config, queryString, newParams) { const oldParams = parseQueryString(config, queryString); const updatedParams = replaceParams(oldParams, newParams); const nonNamespacedParams = parseQueryString({}, queryString); diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index 5c0d8ed1b6..aa90f3ed3b 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -8,7 +8,7 @@ import { _addDefaultsToObject, mergeParams, replaceParams, - replaceNamespacedParams, + updateQueryString, } from './qs'; describe('qs (qs.js)', () => { @@ -851,7 +851,7 @@ describe('qs (qs.js)', () => { }); }); - describe('replaceNamespacedParams', () => { + describe('updateQueryString', () => { const config = { namespace: 'template', defaultParams: { page: 1, page_size: 5, order_by: 'name' }, @@ -863,7 +863,7 @@ describe('qs (qs.js)', () => { const newParams = { page: 3, }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'template.name__icontains=workflow&template.page=3' ); }); @@ -873,7 +873,7 @@ describe('qs (qs.js)', () => { const newParams = { or__type: 'job_template', }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'template.name__icontains=workflow&template.or__type=job_template&template.page=2' ); }); @@ -883,7 +883,7 @@ describe('qs (qs.js)', () => { const newParams = { page: 3, }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'foo=bar&template.name__icontains=workflow&template.page=3' ); }); @@ -894,7 +894,7 @@ describe('qs (qs.js)', () => { page: 3, name__icontains: null, }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'template.page=3' ); }); @@ -905,20 +905,20 @@ describe('qs (qs.js)', () => { page: 3, page_size: 5, }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'template.page=3' ); }); // This fix needed after we're confident refactoring components - // to use replaceNamespacedParams provides equivalent functionality + // to use updateQueryString provides equivalent functionality test.skip('should not alter params of other namespaces', () => { const query = 'template.name__icontains=workflow&template.page=2&credential.page=3'; const newParams = { page: 3, }; - expect(replaceNamespacedParams(config, query, newParams)).toEqual( + expect(updateQueryString(config, query, newParams)).toEqual( 'template.name__icontains=workflow&template.page=3&credential.page=3' ); }); diff --git a/awx/ui_next/src/util/useRequest.js b/awx/ui_next/src/util/useRequest.js index e9392b3238..54ffb9d6dd 100644 --- a/awx/ui_next/src/util/useRequest.js +++ b/awx/ui_next/src/util/useRequest.js @@ -1,6 +1,6 @@ import { useEffect, useState, useCallback } from 'react'; import { useLocation, useHistory } from 'react-router-dom'; -import { parseQueryString, replaceNamespacedParams } from './qs'; +import { parseQueryString, updateQueryString } from './qs'; import useIsMounted from './useIsMounted'; /* @@ -107,10 +107,10 @@ export function useDeleteItems( } const params = parseQueryString(qsConfig, location.search); if (params.page > 1 && allItemsSelected) { - const newParams = replaceNamespacedParams(qsConfig, location.search, { + const qs = updateQueryString(qsConfig, location.search, { page: params.page - 1, }); - history.push(`${location.pathname}?${newParams}`); + history.push(`${location.pathname}?${qs}`); } else { fetchItems(); } From 908263df5033af6f67d245f79e7218eb2e75463d Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Wed, 19 May 2021 13:23:46 -0700 Subject: [PATCH 7/9] Rewrite updateQueryString to preserve namespaces * Refactors ActivityStream to use updateQueryString --- .../screens/ActivityStream/ActivityStream.jsx | 17 ++++----- awx/ui_next/src/util/qs.js | 37 +++++++++++++++---- awx/ui_next/src/util/qs.test.js | 26 +++++++++++-- 3 files changed, 58 insertions(+), 22 deletions(-) diff --git a/awx/ui_next/src/screens/ActivityStream/ActivityStream.jsx b/awx/ui_next/src/screens/ActivityStream/ActivityStream.jsx index b7751118d2..7b1b885dcd 100644 --- a/awx/ui_next/src/screens/ActivityStream/ActivityStream.jsx +++ b/awx/ui_next/src/screens/ActivityStream/ActivityStream.jsx @@ -22,8 +22,7 @@ import useRequest from '../../util/useRequest'; import { getQSConfig, parseQueryString, - replaceParams, - encodeNonDefaultQueryString, + updateQueryString, } from '../../util/qs'; import { ActivityStreamAPI } from '../../api'; @@ -96,16 +95,14 @@ function ActivityStream() { }, [fetchActivityStream]); const pushHistoryState = urlParamsToAdd => { - let searchParams = parseQueryString(QS_CONFIG, location.search); - searchParams = replaceParams(searchParams, { page: 1 }); - const encodedParams = encodeNonDefaultQueryString(QS_CONFIG, searchParams, { + const pageOneQs = updateQueryString(QS_CONFIG, location.search, { + page: 1, + }); + const qs = updateQueryString(null, pageOneQs, { type: urlParamsToAdd.get('type'), }); - history.push( - encodedParams - ? `${location.pathname}?${encodedParams}` - : location.pathname - ); + + history.push(qs ? `${location.pathname}?${qs}` : location.pathname); }; return ( diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index f3eaaa6cd1..5f066cca7e 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -266,12 +266,33 @@ export function replaceParams(oldParams, newParams) { * @return {string} url query string */ export function updateQueryString(config, queryString, newParams) { - const oldParams = parseQueryString(config, queryString); - const updatedParams = replaceParams(oldParams, newParams); - const nonNamespacedParams = parseQueryString({}, queryString); - return encodeNonDefaultQueryString( - config, - updatedParams, - nonNamespacedParams - ); + const allParams = parseFullQueryString(queryString); + const { namespace = null, defaultParams = {} } = config || {}; + Object.keys(newParams).forEach(key => { + const val = newParams[key]; + const fullKey = namespace ? `${namespace}.${key}` : key; + if (val === null || val === defaultParams[key]) { + delete allParams[fullKey]; + } else { + allParams[fullKey] = newParams[key]; + } + }); + return encodeQueryString(allParams); +} + +function parseFullQueryString(queryString) { + const allParams = {}; + queryString + .replace(/^\?/, '') + .split('&') + .map(s => s.split('=')) + .forEach(([rawKey, rawValue]) => { + if (!rawKey) { + return; + } + const key = decodeURIComponent(rawKey); + const value = decodeURIComponent(rawValue); + allParams[key] = mergeParam(allParams[key], value); + }); + return allParams; } diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index aa90f3ed3b..e4a48f1b90 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -858,6 +858,15 @@ describe('qs (qs.js)', () => { integerFields: ['page'], }; + test('should add param to empty query string', () => { + const newParams = { + page: 3, + }; + expect(updateQueryString(config, '', newParams)).toEqual( + 'template.page=3' + ); + }); + test('should update namespaced param', () => { const query = 'template.name__icontains=workflow&template.page=2'; const newParams = { @@ -910,16 +919,25 @@ describe('qs (qs.js)', () => { ); }); - // This fix needed after we're confident refactoring components - // to use updateQueryString provides equivalent functionality - test.skip('should not alter params of other namespaces', () => { + test('should update non-namespaced param', () => { + const query = + 'activity_stream.name__icontains=workflow&activity_stream.page=2'; + const newParams = { + type: 'job', + }; + expect(updateQueryString(null, query, newParams)).toEqual( + 'activity_stream.name__icontains=workflow&activity_stream.page=2&type=job' + ); + }); + + test('should not alter params of other namespaces', () => { const query = 'template.name__icontains=workflow&template.page=2&credential.page=3'; const newParams = { page: 3, }; expect(updateQueryString(config, query, newParams)).toEqual( - 'template.name__icontains=workflow&template.page=3&credential.page=3' + 'credential.page=3&template.name__icontains=workflow&template.page=3' ); }); }); From 8ac3cc1542667dd4dc631800eb1cc41d7170476d Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Wed, 19 May 2021 13:49:57 -0700 Subject: [PATCH 8/9] delete qs utils that are no longer used --- awx/ui_next/src/util/qs.js | 52 --------------- awx/ui_next/src/util/qs.test.js | 114 -------------------------------- 2 files changed, 166 deletions(-) diff --git a/awx/ui_next/src/util/qs.js b/awx/ui_next/src/util/qs.js index 5f066cca7e..1ef26ea8ce 100644 --- a/awx/ui_next/src/util/qs.js +++ b/awx/ui_next/src/util/qs.js @@ -113,44 +113,6 @@ function encodeValue(key, value) { return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`; } -/** - * Convert query param object to url query string, adding namespace and - * removing defaults. Used to put into url bar after ui route - * @param {object} qs config object for namespacing params, filtering defaults - * @param {object} query param object - * @param {object} any non-namespaced params to append - * @return {string} url query string - */ -export const encodeNonDefaultQueryString = ( - config, - params, - nonNamespacedParams = {} -) => { - if (!params) return ''; - const paramsWithoutDefaults = removeParams({}, params, config.defaultParams); - return encodeQueryString({ - ...namespaceParams(config.namespace, paramsWithoutDefaults), - ...nonNamespacedParams, - }); -}; - -/** - * helper function to namespace params object - * @param {string} namespace to append to params - * @param {object} params object to append namespace to - * @return {object} params object with namespaced keys - */ -const namespaceParams = (namespace, params) => { - if (!namespace) return params; - - const namespaced = {}; - Object.keys(params).forEach(key => { - namespaced[`${namespace}.${key}`] = params[key]; - }); - - return namespaced; -}; - /** * 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.) @@ -242,20 +204,6 @@ function dedupeArray(arr) { return deduped; } -/** - * 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} joined namespaced params object - */ -export function replaceParams(oldParams, newParams) { - return { - ...oldParams, - ...newParams, - }; -} - /** * Update namespaced param(s), returning a new query string. Leaves params * from other namespaces unaltered diff --git a/awx/ui_next/src/util/qs.test.js b/awx/ui_next/src/util/qs.test.js index e4a48f1b90..a34543e808 100644 --- a/awx/ui_next/src/util/qs.test.js +++ b/awx/ui_next/src/util/qs.test.js @@ -1,13 +1,11 @@ import { encodeQueryString, - encodeNonDefaultQueryString, parseQueryString, getQSConfig, removeParams, _stringToObject, _addDefaultsToObject, mergeParams, - replaceParams, updateQueryString, } from './qs'; @@ -48,70 +46,6 @@ describe('qs (qs.js)', () => { }); }); - describe('encodeNonDefaultQueryString', () => { - const config = { - namespace: null, - defaultParams: { page: 1, page_size: 5, order_by: 'name' }, - integerFields: ['page'], - }; - - test('should return the expected queryString', () => { - [ - [null, ''], - [{}, ''], - [{ order_by: 'name', page: 1, page_size: 5 }, ''], - [{ order_by: '-name', page: 1, page_size: 5 }, 'order_by=-name'], - [ - { order_by: '-name', page: 3, page_size: 10 }, - 'order_by=-name&page=3&page_size=10', - ], - [ - { order_by: '-name', page: 3, page_size: 10, foo: 'bar' }, - 'foo=bar&order_by=-name&page=3&page_size=10', - ], - ].forEach(([params, expectedQueryString]) => { - const actualQueryString = encodeNonDefaultQueryString(config, params); - - expect(actualQueryString).toEqual(expectedQueryString); - }); - }); - - test('should omit null values', () => { - const vals = { - order_by: 'foo', - page: null, - }; - expect(encodeNonDefaultQueryString(config, vals)).toEqual('order_by=foo'); - }); - - test('should namespace encoded params', () => { - const conf = { - namespace: 'item', - defaultParams: { page: 1 }, - }; - const params = { - page: 1, - foo: 'bar', - }; - expect(encodeNonDefaultQueryString(conf, params)).toEqual('item.foo=bar'); - }); - - test('should handle array values', () => { - const vals = { - foo: ['one', 'two'], - bar: ['alpha', 'beta'], - }; - const conf = { - defaultParams: { - foo: ['one', 'two'], - }, - }; - expect(encodeNonDefaultQueryString(conf, vals)).toEqual( - 'bar=alpha&bar=beta' - ); - }); - }); - describe('getQSConfig', () => { test('should get default QS config object', () => { expect(getQSConfig('organization')).toEqual({ @@ -803,54 +737,6 @@ describe('qs (qs.js)', () => { }); }); - 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', - }); - }); - }); - describe('updateQueryString', () => { const config = { namespace: 'template', From e6cfd726c6bc8e046ce82c931168f869f727f7c1 Mon Sep 17 00:00:00 2001 From: "Keith J. Grant" Date: Thu, 27 May 2021 13:22:47 -0700 Subject: [PATCH 9/9] don't reload host if query string changes --- awx/ui_next/src/screens/Host/Host.jsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/awx/ui_next/src/screens/Host/Host.jsx b/awx/ui_next/src/screens/Host/Host.jsx index dcd1d8c140..9792fd609b 100644 --- a/awx/ui_next/src/screens/Host/Host.jsx +++ b/awx/ui_next/src/screens/Host/Host.jsx @@ -35,7 +35,7 @@ function Host({ setBreadcrumb }) { useEffect(() => { fetchHost(); - }, [fetchHost, location]); + }, [fetchHost, location.pathname]); const tabsArray = [ {