Merge branch 'boards-querystring' into 'master'

don't add empty query params to boards

See merge request gitlab-org/gitlab-ee!4441

app/assets/javascripts/boards/components/modal/index.vue

 <script>
   /* global ListIssue */
-  import queryData from '~/boards/utils/query_data';
+  import { urlParamsToObject } from '~/lib/utils/common_utils';
   import loadingIcon from '~/vue_shared/components/loading_icon.vue';
   import ModalHeader from './header.vue';
   import ModalList from './list.vue';
@@ -109,13 +109,11 @@
       loadIssues(clearIssues = false) {
         if (!this.showAddIssuesModal) return false;
 
-        return gl.boardService
-          .getBacklog(
-            queryData(this.filter.path, {
-              page: this.page,
-              per: this.perPage,
-            }),
-          )
+        return gl.boardService.getBacklog({
+          ...urlParamsToObject(this.filter.path),
+          page: this.page,
+          per: this.perPage,
+        })
           .then(res => res.data)
           .then(data => {
             if (clearIssues) {

app/assets/javascripts/boards/models/list.js

@@ -4,7 +4,7 @@
 import { __ } from '~/locale';
 import ListLabel from '~/vue_shared/models/label';
 import ListAssignee from '~/vue_shared/models/assignee';
-import queryData from '../utils/query_data';
+import { urlParamsToObject } from '~/lib/utils/common_utils';
 import ListMilestone from './milestone';
 
 const PER_PAGE = 20;
@@ -121,7 +121,10 @@ class List {
   }
 
   getIssues(emptyIssues = true) {
-    const data = queryData(gl.issueBoards.BoardsStore.filter.path, { page: this.page });
+    const data = {
+      ...urlParamsToObject(gl.issueBoards.BoardsStore.filter.path),
+      page: this.page,
+    };
 
     if (this.label && data.label_name) {
       data.label_name = data.label_name.filter(label => label !== this.label.title);

app/assets/javascripts/boards/utils/query_data.js

-export default (path, extraData) => path.split('&').reduce((dataParam, filterParam) => {
-  if (filterParam === '') return dataParam;
-
-  const data = dataParam;
-  const paramSplit = filterParam.split('=');
-  const paramKeyNormalized = paramSplit[0].replace('[]', '');
-  const isArray = paramSplit[0].indexOf('[]');
-  const value = decodeURIComponent(paramSplit[1].replace(/\+/g, ' '));
-
-  if (isArray !== -1) {
-    if (!data[paramKeyNormalized]) {
-      data[paramKeyNormalized] = [];
-    }
-
-    data[paramKeyNormalized].push(value);
-  } else {
-    data[paramKeyNormalized] = value;
-  }
-
-  return data;
-}, extraData);

app/assets/javascripts/lib/utils/common_utils.js

@@ -132,16 +132,43 @@ export const parseUrlPathname = url => {
   return parsedUrl.pathname.charAt(0) === '/' ? parsedUrl.pathname : `/${parsedUrl.pathname}`;
 };
 
-// We can trust that each param has one & since values containing & will be encoded
-// Remove the first character of search as it is always ?
-export const getUrlParamsArray = () =>
-  window.location.search
-    .slice(1)
-    .split('&')
-    .map(param => {
-      const split = param.split('=');
-      return [decodeURI(split[0]), split[1]].join('=');
-    });
+const splitPath = (path = '') => path
+  .replace(/^\?/, '')
+  .split('&');
+
+export const urlParamsToArray = (path = '') => splitPath(path)
+  .filter(param => param.length > 0)
+  .map(param => {
+    const split = param.split('=');
+    return [decodeURI(split[0]), split[1]].join('=');
+  });
+
+export const getUrlParamsArray = () => urlParamsToArray(window.location.search);
+
+export const urlParamsToObject = (path = '') => splitPath(path)
+  .reduce((dataParam, filterParam) => {
+    if (filterParam === '') {
+      return dataParam;
+    }
+
+    const data = dataParam;
+    let [key, value] = filterParam.split('=');
+    const isArray = key.includes('[]');
+    key = key.replace('[]', '');
+    value = decodeURIComponent(value.replace(/\+/g, ' '));
+
+    if (isArray) {
+      if (!data[key]) {
+        data[key] = [];
+      }
+
+      data[key].push(value);
+    } else {
+      data[key] = value;
+    }
+
+    return data;
+  }, {});
 
 export const isMetaKey = e => e.metaKey || e.ctrlKey || e.altKey || e.shiftKey;
 

ee/app/assets/javascripts/boards/stores/boards_store_ee.js

@@ -38,6 +38,9 @@ class BoardsStoreEE {
     };
 
     this.store.updateFiltersUrl = (replaceState = false) => {
+      if (!this.store.filter.path) {
+        return;
+      }
       if (replaceState) {
         window.history.replaceState(null, null, `?${this.store.filter.path}`);
       } else {

ee/changelogs/unreleased/boards-querystring.yml

+---
+title: don't add empty query params to boards
+merge_request: 4441
+author:
+type: fixed

spec/javascripts/boards/mock_data.js

@@ -34,7 +34,7 @@ export const listObjDuplicate = {
 
 export const BoardsMockData = {
   GET: {
-    '/test/-/boards/1/lists/300/issues?id=300&page=1&=': {
+    '/test/-/boards/1/lists/300/issues?id=300&page=1': {
       issues: [
         {
           title: 'Testing',

spec/javascripts/boards/utils/query_data_spec.js

-import queryData from '~/boards/utils/query_data';
-
-describe('queryData', () => {
-  it('parses path for label with trailing +', () => {
-    expect(
-      queryData('label_name[]=label%2B', {}),
-    ).toEqual({
-      label_name: ['label+'],
-    });
-  });
-
-  it('parses path for milestone with trailing +', () => {
-    expect(
-      queryData('milestone_title=A%2B', {}),
-    ).toEqual({
-      milestone_title: 'A+',
-    });
-  });
-
-  it('parses path for search terms with spaces', () => {
-    expect(
-      queryData('search=two+words', {}),
-    ).toEqual({
-      search: 'two words',
-    });
-  });
-});

spec/javascripts/lib/utils/common_utils_spec.js

@@ -29,24 +29,46 @@ describe('common_utils', () => {
     });
   });
 
-  describe('getUrlParamsArray', () => {
-    it('should return params array', () => {
-      expect(commonUtils.getUrlParamsArray() instanceof Array).toBe(true);
+  describe('urlParamsToArray', () => {
+    it('returns empty array for empty querystring', () => {
+      expect(commonUtils.urlParamsToArray('')).toEqual([]);
+    });
+
+    it('should decode params', () => {
+      expect(
+        commonUtils.urlParamsToArray('?label_name%5B%5D=test')[0],
+      ).toBe('label_name[]=test');
     });
 
     it('should remove the question mark from the search params', () => {
-      const paramsArray = commonUtils.getUrlParamsArray();
+      const paramsArray = commonUtils.urlParamsToArray('?test=thing');
       expect(paramsArray[0][0] !== '?').toBe(true);
     });
+  });
 
-    it('should decode params', () => {
-      window.history.pushState('', '', '?label_name%5B%5D=test');
+  describe('urlParamsToObject', () => {
+    it('parses path for label with trailing +', () => {
+      expect(
+        commonUtils.urlParamsToObject('label_name[]=label%2B', {}),
+      ).toEqual({
+        label_name: ['label+'],
+      });
+    });
 
+    it('parses path for milestone with trailing +', () => {
       expect(
-        commonUtils.getUrlParamsArray()[0],
-      ).toBe('label_name[]=test');
+        commonUtils.urlParamsToObject('milestone_title=A%2B', {}),
+      ).toEqual({
+        milestone_title: 'A+',
+      });
+    });
 
-      window.history.pushState('', '', '?');
+    it('parses path for search terms with spaces', () => {
+      expect(
+        commonUtils.urlParamsToObject('search=two+words', {}),
+      ).toEqual({
+        search: 'two words',
+      });
     });
   });