improve qs test coverage, minor cleanup

awx/ui_next/src/util/qs.js

@@ -28,7 +28,6 @@ export function getQSConfig(
  * @param {string} url query string
  * @return {object} query param object
  */
-// TODO: rename to parseNamespacedQueryString?
 export function parseQueryString(config, queryString) {
   if (!queryString) {
     return config.defaultParams;
@@ -51,18 +50,19 @@ export const encodeQueryString = params => {
     .sort()
     .filter(key => params[key] !== null)
     .map(key => [key, params[key]])
-    .map(([key, value]) => {
+    .map(([key, value]) => encodeValue(key, value))
-      // if value is array, should return more than one key value pair
-      if (Array.isArray(value)) {
-        return value
-          .map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`)
-          .join('&');
-      }
-      return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`;
-    })
     .join('&');
 };
 
+function encodeValue(key, value) {
+  if (Array.isArray(value)) {
+    return value
+      .map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`)
+      .join('&');
+  }
+  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
@@ -93,22 +93,15 @@ export const encodeNonDefaultQueryString = (config, params) => {
     .map(key => {
       return [key, namespacedParams[key]];
     })
-    .map(([key, value]) => {
+    .map(([key, value]) => encodeValue(key, value))
-      // if value is array, should return more than one key value pair
-      if (Array.isArray(value)) {
-        return value
-          .map(val => `${encodeURIComponent(key)}=${encodeURIComponent(val)}`)
-          .join('&');
-      }
-      return `${encodeURIComponent(key)}=${encodeURIComponent(value)}`;
-    })
     .join('&');
 };
 
 /**
- * Merges existing params of search string with new ones and returns the updated list of params
+ * 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 search
+ * @param {object} object with params from existing query
  * @param {object} object with new params to add
  * @return {object} query param object
  */
@@ -181,7 +174,7 @@ const stringToObject = (config, qs) => {
     .split('&')
     .map(s => s.split('='))
     .forEach(([nsKey, rawValue]) => {
-      if (!nsKey || !nsKey.startsWith(`${config.namespace}.`)) {
+      if (!nsKey || !namespaceMatches(config.namespace, nsKey)) {
         return;
       }
       const key = decodeURIComponent(nsKey.substr(config.namespace.length + 1));
@@ -198,14 +191,6 @@ const stringToObject = (config, qs) => {
 };
 export { stringToObject as _stringToObject };
 
-function addDefaultsToObject(config, params) {
-  return {
-    ...config.defaultParams,
-    ...params,
-  };
-}
-export { addDefaultsToObject as _addDefaultsToObject };
-
 function parseValue(config, key, rawValue) {
   if (config.integerFields && config.integerFields.some(v => v === key)) {
     return parseInt(rawValue, 10);
@@ -214,6 +199,14 @@ function parseValue(config, key, rawValue) {
   return decodeURIComponent(rawValue);
 }
 
+function addDefaultsToObject(config, params) {
+  return {
+    ...config.defaultParams,
+    ...params,
+  };
+}
+export { addDefaultsToObject as _addDefaultsToObject };
+
 /**
  * helper function used to convert from
  * Object.entries format ([ [ key, value ], ... ]) to object
@@ -288,7 +281,7 @@ const paramValueIsEqual = (one, two) => {
   let isEqual = false;
 
   if (Array.isArray(one) && Array.isArray(two)) {
-    isEqual = one.filter(val => two.indexOf(val) > -1).length === 0;
+    isEqual = one.filter(val => two.indexOf(val) > -1).length === one.length;
   } else if (
     (typeof one === 'string' && typeof two === 'string') ||
     (typeof one === 'number' && typeof two === 'number')

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

@@ -5,7 +5,6 @@ import {
   getQSConfig,
   addParams,
   removeParams,
-  _paramStringToArray,
   _stringToObject,
   _addDefaultsToObject,
 } from './qs';
@@ -38,6 +37,13 @@ describe('qs (qs.js)', () => {
       };
       expect(encodeQueryString(vals)).toEqual('order_by=name');
     });
+
+    test('should encode array params', () => {
+      const vals = {
+        foo: ['one', 'two', 'three'],
+      };
+      expect(encodeQueryString(vals)).toEqual('foo=one&foo=two&foo=three');
+    });
   });
 
   describe('encodeNonDefaultQueryString', () => {
@@ -75,6 +81,18 @@ describe('qs (qs.js)', () => {
       };
       expect(encodeNonDefaultQueryString(config, vals)).toEqual('order_by=foo');
     });
+
+    test('should compare array values', () => {
+      const vals = {
+        foo: ['one', 'two'],
+      };
+      const conf = {
+        defaultParams: {
+          foo: ['one', 'two'],
+        }
+      };
+      expect(encodeNonDefaultQueryString(conf, vals)).toEqual('');
+    })
   });
 
   describe('getQSConfig', () => {
@@ -241,6 +259,16 @@ describe('qs (qs.js)', () => {
         page_size: 15,
       });
     });
+
+    test('should parse long arrays', () => {
+      const config = {
+        namespace: 'item',
+      };
+      const query = '?item.baz=one&item.baz=two&item.baz=three';
+      expect(parseQueryString(config, query)).toEqual({
+        baz: ['one', 'two', 'three'],
+      });
+    });
   });
 
   describe('addParams', () => {
@@ -306,6 +334,22 @@ describe('qs (qs.js)', () => {
       });
     });
 
+    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',
@@ -569,7 +613,7 @@ describe('qs (qs.js)', () => {
   });
 
   describe('_stringToObject', () => {
-    it('should convert to object', () => {
+    test('should convert to object', () => {
       const config = { namespace: 'unit' };
       expect(_stringToObject(config, '?unit.foo=bar&unit.baz=bam')).toEqual({
         foo: 'bar',
@@ -577,14 +621,14 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    it('should convert duplicated keys to array', () => {
+    test('should convert duplicated keys to array', () => {
       const config = { namespace: 'unit' };
       expect(_stringToObject(config, '?unit.foo=bar&unit.foo=bam')).toEqual({
         foo: ['bar', 'bam'],
       });
     });
 
-    it('should omit keys from other namespaces', () => {
+    test('should omit keys from other namespaces', () => {
       const config = { namespace: 'unit' };
       expect(
         _stringToObject(config, '?unit.foo=bar&other.bar=bam&one=two')
@@ -593,7 +637,7 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    it('should convert numbers to correct type', () => {
+    test('should convert numbers to correct type', () => {
       const config = {
         namespace: 'unit',
         integerFields: ['page'],
@@ -605,7 +649,7 @@ describe('qs (qs.js)', () => {
   });
 
   describe('_addDefaultsToObject', () => {
-    it('should add missing default values', () => {
+    test('should add missing default values', () => {
       const config = {
         defaultParams: { page: 1, page_size: 5, order_by: 'name' },
       }
@@ -616,7 +660,7 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    it('should not override existing params', () => {
+    test('should not override existing params', () => {
       const config = {
         defaultParams: { page: 1, page_size: 5, order_by: 'name' },
       }
@@ -631,7 +675,7 @@ describe('qs (qs.js)', () => {
       });
     });
 
-    it('should handle missing defaultParams', () => {
+    test('should handle missing defaultParams', () => {
       const params = {
         page: 2,
         order_by: 'date_created',