split qs addParams into mergeParams and replaceParams for clarity

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,
       })

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(
+    this.pushHistoryState(replaceParams(oldParams, { page_size: pageSize }));
-      addParams(qsConfig, oldParams, { page_size: pageSize })
-    );
   }
 
   pushHistoryState(params) {

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
+ * Merge old and new params together, joining values into arrays where necessary
- * @param {object} namespaced params object
+ * @param {object} namespaced params object of old params
- * @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
  * @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(?)
+export function mergeParams(oldParams, newParams) {
-const mergeParams = (oldParams, newParams) => {
+  const merged = {};
-  const merged = arrayToObject(
+  Object.keys(oldParams).forEach(key => {
-    Object.keys(oldParams).map(key => {
+    merged[key] = mergeParam(oldParams[key], newParams[key]);
-      const oldVal = oldParams[key];
+  });
-      const newVal = newParams[key];
-      return [key, mergeParam(oldVal, newVal)];
-    })
-  );
   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);
+    merged = oldVal.concat(newVal);
-    return oldVal;
+  } 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
+ * Join old and new params together, replacing old values with new ones where
- * @param {object} namespaced params object of merged params
+ * 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) =>
+export function replaceParams(oldParams, newParams) {
-  arrayToObject(
+  return {
-    Object.keys(newParams)
+    ...oldParams,
-      .filter(key => Object.keys(mergedParams).indexOf(key) === -1)
+    ...newParams,
-      .map(key => [key, newParams[key]])
+  };
-  );
+}

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',
+      });
+    });
   });
 });