qs cleanup, more tests

awx/ui_next/src/util/qs.js

@@ -64,8 +64,8 @@ function encodeValue(key, value) {
 }
 
 /**
- * Convert query param object to url query string, adding namespace and removing defaults
- * Used to put into url bar after ui route
+ * 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
  * @return {string} url query string
@@ -106,6 +106,16 @@ export const encodeNonDefaultQueryString = (config, params) => {
  * @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(
@@ -117,7 +127,7 @@ export function addParams(config, oldParams, paramsToAdd) {
     namespacedOldParams,
     namespacedDefaultParams
   );
-  const namespacedMergedParams = getMergedParams(
+  const namespacedMergedParams = mergeParams(
     namespacedOldParamsNotDefaults,
     namespacedParamsToAdd
   );
@@ -179,13 +189,7 @@ const stringToObject = (config, qs) => {
       }
       const key = decodeURIComponent(nsKey.substr(config.namespace.length + 1));
       const value = parseValue(config, key, rawValue);
-      if (!params[key]) {
-        params[key] = value;
-      } else if (Array.isArray(params[key])) {
-        params[key].push(value);
-      } else {
-        params[key] = [params[key], value];
-      }
+      params[key] = mergeParam(params[key], value);
     });
   return params;
 };
@@ -324,21 +328,37 @@ const getNonDefaultParams = (params, defaults) =>
  * @param {object} namespaced params object of new params
  * @return {object} merged namespaced params object
  */
-const getMergedParams = (oldParams, newParams) =>
-  arrayToObject(
+// TODO: BUG? newParam that doesn't exist in oldParams isn't added(?)
+const mergeParams = (oldParams, newParams) => {
+  const merged = arrayToObject(
     Object.keys(oldParams).map(key => {
-      let oldVal = oldParams[key];
+      const oldVal = oldParams[key];
       const newVal = newParams[key];
-      if (newVal) {
-        if (Array.isArray(oldVal)) {
-          oldVal.push(newVal);
-        } else {
-          oldVal = [oldVal, newVal];
-        }
-      }
-      return [key, oldVal];
+      return [key, mergeParam(oldVal, newVal)];
     })
   );
+  Object.keys(newParams).forEach(key => {
+    if (!merged[key]) {
+      merged[key] = newParams[key];
+    }
+  });
+  return merged;
+}
+export { mergeParams as _mergeParams };
+
+function mergeParam(oldVal, newVal) {
+  if (!newVal) {
+    return oldVal;
+  }
+  if (!oldVal) {
+    return newVal;
+  }
+  if (Array.isArray(oldVal)) {
+    oldVal.push(newVal);
+    return oldVal;
+  }
+  return [oldVal, newVal];
+}
 
 /**
  * helper function to get new params that are not in merged params

awx/ui_next/src/util/qs.test.js

@@ -7,6 +7,7 @@ import {
   removeParams,
   _stringToObject,
   _addDefaultsToObject,
+  _mergeParams,
 } from './qs';
 
 describe('qs (qs.js)', () => {
@@ -53,7 +54,7 @@ describe('qs (qs.js)', () => {
       integerFields: ['page'],
     };
 
-    test('encodeNonDefaultQueryString returns the expected queryString', () => {
+    test('should return the expected queryString', () => {
       [
         [null, ''],
         [{}, ''],
@@ -74,7 +75,7 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    test('encodeNonDefaultQueryString omits null values', () => {
+    test('should omit null values', () => {
       const vals = {
         order_by: 'foo',
         page: null,
@@ -82,17 +83,32 @@ describe('qs (qs.js)', () => {
       expect(encodeNonDefaultQueryString(config, vals)).toEqual('order_by=foo');
     });
 
-    test('should compare array values', () => {
+    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('');
-    })
+      expect(encodeNonDefaultQueryString(conf, vals)).toEqual(
+        'bar=alpha&bar=beta'
+      );
+    });
   });
 
   describe('getQSConfig', () => {
@@ -303,7 +319,7 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    test('should replace query params that are defaults', () => {
+    test('should replace query params that have defaults', () => {
       const config = {
         namespace: null,
         defaultParams: { page: 1, page_size: 15 },
@@ -318,6 +334,21 @@ describe('qs (qs.js)', () => {
       });
     });
 
+    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,
@@ -652,7 +683,7 @@ describe('qs (qs.js)', () => {
     test('should add missing default values', () => {
       const config = {
         defaultParams: { page: 1, page_size: 5, order_by: 'name' },
-      }
+      };
       expect(_addDefaultsToObject(config, {})).toEqual({
         page: 1,
         page_size: 5,
@@ -663,11 +694,11 @@ describe('qs (qs.js)', () => {
     test('should not override existing params', () => {
       const config = {
         defaultParams: { page: 1, page_size: 5, order_by: 'name' },
-      }
+      };
       const params = {
         page: 2,
         order_by: 'date_created',
-      }
+      };
       expect(_addDefaultsToObject(config, params)).toEqual({
         page: 2,
         page_size: 5,
@@ -679,11 +710,52 @@ describe('qs (qs.js)', () => {
       const params = {
         page: 2,
         order_by: 'date_created',
-      }
+      };
       expect(_addDefaultsToObject({}, params)).toEqual({
         page: 2,
         order_by: 'date_created',
       });
-    })
-  })
+    });
+  });
+
+  describe('mergeParams', () => {
+    it('should merge param into an array', () => {
+      const oldParams = {
+        foo: 'one',
+      };
+      const newParams = {
+        foo: 'two',
+      };
+      expect(_mergeParams(oldParams, newParams)).toEqual({
+        foo: ['one', 'two'],
+      });
+    });
+
+    it('should retain unaltered params', () => {
+      const oldParams = {
+        foo: 'one',
+        bar: 'baz'
+      };
+      const newParams = {
+        foo: 'two',
+      };
+      expect(_mergeParams(oldParams, newParams)).toEqual({
+        foo: ['one', 'two'],
+        bar: 'baz',
+      });
+    });
+
+    it('should merge objects', () => {
+      const oldParams = {
+        one: 'one',
+      };
+      const newParams = {
+        two: 'two',
+      };
+      expect(_mergeParams(oldParams, newParams)).toEqual({
+        one: 'one',
+        two: 'two',
+      });
+    });
+  });
 });