Removing the URL manipulation functions added to 'common_utils.js' in favor of the functions that already existed in 'url_utility.js'. Refactoring 'removeParams' function in 'url_utility.js' to allow url to be passed and to preserve the original host and/or path provided in the url.

2 files changed, 71 insertions, 72 deletions

diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js
index 3a25be766cb..f320f232687 100644
--- a/spec/javascripts/lib/utils/common_utils_spec.js
+++ b/spec/javascripts/lib/utils/common_utils_spec.js
@@ -65,67 +65,6 @@ describe('common_utils', () => {
     });
   });
 
-  describe('setUrlParam', () => {
-    it('should append param when url has no other params', () => {
-      const url = commonUtils.setUrlParam('/feature/home', 'newParam', 'yes');
-      expect(url).toBe('/feature/home?newParam=yes');
-    });
-
-    it('should append param when url has other params', () => {
-      const url = commonUtils.setUrlParam('/feature/home?showAll=true', 'newParam', 'yes');
-      expect(url).toBe('/feature/home?showAll=true&newParam=yes');
-    });
-
-    it('should replace param when url contains the param', () => {
-      const url = commonUtils.setUrlParam('/feature/home?showAll=true&limit=5', 'limit', '100');
-      expect(url).toBe('/feature/home?showAll=true&limit=100');
-    });
-
-    it('should update param and preserve fragment', () => {
-      const url = commonUtils.setUrlParam('/home?q=no&limit=5&showAll=true#H1', 'limit', '100');
-      expect(url).toBe('/home?q=no&limit=100&showAll=true#H1');
-    });
-  });
-
-  describe('removeUrlParam', () => {
-    it('should remove param when url has no other params', () => {
-      const url = commonUtils.removeUrlParam('/feature/home?size=5', 'size');
-      expect(url).toBe('/feature/home');
-    });
-
-    it('should remove param when url has other params', () => {
-      const url = commonUtils.removeUrlParam('/feature/home?q=1&size=5&f=html', 'size');
-      expect(url).toBe('/feature/home?q=1&f=html');
-    });
-
-    it('should remove param and preserve fragment', () => {
-      const url = commonUtils.removeUrlParam('/feature/home?size=5#H2', 'size');
-      expect(url).toBe('/feature/home#H2');
-    });
-
-    it('should not modify url if param does not exist', () => {
-      const url = commonUtils.removeUrlParam('/feature/home?q=1&size=5&f=html', 'locale');
-      expect(url).toBe('/feature/home?q=1&size=5&f=html');
-    });
-  });
-
-  describe('setUrlFragment', () => {
-    it('should set fragment when url has no fragment', () => {
-      const url = commonUtils.setUrlFragment('/home/feature', 'usage');
-      expect(url).toBe('/home/feature#usage');
-    });
-
-    it('should set fragment when url has existing fragment', () => {
-      const url = commonUtils.setUrlFragment('/home/feature#overview', 'usage');
-      expect(url).toBe('/home/feature#usage');
-    });
-
-    it('should set fragment when given fragment includes #', () => {
-      const url = commonUtils.setUrlFragment('/home/feature#overview', '#install');
-      expect(url).toBe('/home/feature#install');
-    });
-  });
-
   describe('handleLocationHash', () => {
     beforeEach(() => {
       spyOn(window.document, 'getElementById').and.callThrough();
diff --git a/spec/javascripts/lib/utils/url_utility_spec.js b/spec/javascripts/lib/utils/url_utility_spec.js
index e4df8441793..fe787baf08e 100644
--- a/spec/javascripts/lib/utils/url_utility_spec.js
+++ b/spec/javascripts/lib/utils/url_utility_spec.js
@@ -1,4 +1,4 @@
-import { webIDEUrl, mergeUrlParams } from '~/lib/utils/url_utility';
+import UrlUtility, * as urlUtils from '~/lib/utils/url_utility';
 
 describe('URL utility', () => {
   describe('webIDEUrl', () => {
@@ -8,7 +8,7 @@ describe('URL utility', () => {
 
     describe('without relative_url_root', () => {
       it('returns IDE path with route', () => {
-        expect(webIDEUrl('/gitlab-org/gitlab-ce/merge_requests/1')).toBe(
+        expect(urlUtils.webIDEUrl('/gitlab-org/gitlab-ce/merge_requests/1')).toBe(
           '/-/ide/project/gitlab-org/gitlab-ce/merge_requests/1',
         );
       });
@@ -20,7 +20,7 @@ describe('URL utility', () => {
       });
 
       it('returns IDE path with route', () => {
-        expect(webIDEUrl('/gitlab/gitlab-org/gitlab-ce/merge_requests/1')).toBe(
+        expect(urlUtils.webIDEUrl('/gitlab/gitlab-org/gitlab-ce/merge_requests/1')).toBe(
           '/gitlab/-/ide/project/gitlab-org/gitlab-ce/merge_requests/1',
         );
       });
@@ -29,23 +29,83 @@ describe('URL utility', () => {
 
   describe('mergeUrlParams', () => {
     it('adds w', () => {
-      expect(mergeUrlParams({ w: 1 }, '#frag')).toBe('?w=1#frag');
-      expect(mergeUrlParams({ w: 1 }, '/path#frag')).toBe('/path?w=1#frag');
-      expect(mergeUrlParams({ w: 1 }, 'https://host/path')).toBe('https://host/path?w=1');
-      expect(mergeUrlParams({ w: 1 }, 'https://host/path#frag')).toBe('https://host/path?w=1#frag');
-      expect(mergeUrlParams({ w: 1 }, 'https://h/p?k1=v1#frag')).toBe('https://h/p?k1=v1&w=1#frag');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, '#frag')).toBe('?w=1#frag');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, '/path#frag')).toBe('/path?w=1#frag');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, 'https://host/path')).toBe('https://host/path?w=1');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, 'https://host/path#frag')).toBe('https://host/path?w=1#frag');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, 'https://h/p?k1=v1#frag')).toBe('https://h/p?k1=v1&w=1#frag');
     });
 
     it('updates w', () => {
-      expect(mergeUrlParams({ w: 1 }, '?k1=v1&w=0#frag')).toBe('?k1=v1&w=1#frag');
+      expect(urlUtils.mergeUrlParams({ w: 1 }, '?k1=v1&w=0#frag')).toBe('?k1=v1&w=1#frag');
     });
 
     it('adds multiple params', () => {
-      expect(mergeUrlParams({ a: 1, b: 2, c: 3 }, '#frag')).toBe('?a=1&b=2&c=3#frag');
+      expect(urlUtils.mergeUrlParams({ a: 1, b: 2, c: 3 }, '#frag')).toBe('?a=1&b=2&c=3#frag');
     });
 
     it('adds and updates encoded params', () => {
-      expect(mergeUrlParams({ a: '&', q: '?' }, '?a=%23#frag')).toBe('?a=%26&q=%3F#frag');
+      expect(urlUtils.mergeUrlParams({ a: '&', q: '?' }, '?a=%23#frag')).toBe('?a=%26&q=%3F#frag');
+    });
+  });
+
+  describe('removeParams', () => {
+    describe('when url is passed', () => {
+      it('removes query param with encoded ampersand', () => {
+        const url = urlUtils.removeParams(['filter'], '/mail?filter=n%3Djoe%26l%3Dhome');
+
+        expect(url).toBe('/mail');
+      });
+
+      it('should remove param when url has no other params', () => {
+        const url = urlUtils.removeParams(['size'], '/feature/home?size=5');
+        expect(url).toBe('/feature/home');
+      });
+
+      it('should remove param when url has other params', () => {
+        const url = urlUtils.removeParams(['size'], '/feature/home?q=1&size=5&f=html');
+        expect(url).toBe('/feature/home?q=1&f=html');
+      });
+
+      it('should remove param and preserve fragment', () => {
+        const url = urlUtils.removeParams(['size'], '/feature/home?size=5#H2');
+        expect(url).toBe('/feature/home#H2');
+      });
+
+      it('should remove multiple params', () => {
+        const url = urlUtils.removeParams(['z', 'a'], '/home?z=11111&l=en_US&a=true#H2');
+        expect(url).toBe('/home?l=en_US#H2');
+      });
+    });
+
+    describe('when no url is passed', () => {
+      it('should remove params from window.location.href', () => {
+        spyOnDependency(UrlUtility, 'windowLocation').and.callFake(() => {
+          const anchor = document.createElement('a');
+          anchor.href = 'https://mysite.com/?zip=11111&locale=en_US&ads=false#privacy';
+          return anchor;
+        });
+
+        const url = urlUtils.removeParams(['locale']);
+        expect(url).toBe('https://mysite.com/?zip=11111&ads=false#privacy');
+      });
+    });
+  });
+
+  describe('setUrlFragment', () => {
+    it('should set fragment when url has no fragment', () => {
+      const url = urlUtils.setUrlFragment('/home/feature', 'usage');
+      expect(url).toBe('/home/feature#usage');
+    });
+
+    it('should set fragment when url has existing fragment', () => {
+      const url = urlUtils.setUrlFragment('/home/feature#overview', 'usage');
+      expect(url).toBe('/home/feature#usage');
+    });
+
+    it('should set fragment when given fragment includes #', () => {
+      const url = urlUtils.setUrlFragment('/home/feature#overview', '#install');
+      expect(url).toBe('/home/feature#install');
     });
   });
 });