diff options
author | Fatih Acet <acetfatih@gmail.com> | 2017-04-03 11:07:28 +0000 |
---|---|---|
committer | DJ Mountney <david@twkie.net> | 2017-04-03 14:07:00 -0700 |
commit | 11a784791f55637ad0d978c885a3a044cdfb14a1 (patch) | |
tree | 1532be8880fa5478fc0c3bed330ce8cfbfd6bf4a | |
parent | 1ee3f14a9781a0d57fe6226cd40178d650f6ab27 (diff) | |
download | gitlab-ce-11a784791f55637ad0d978c885a3a044cdfb14a1.tar.gz |
Merge branch '30264-fix-vue-pagination' into 'master'
Fixes method not replacing URL parameters correctly
Closes #30264
See merge request !10351
5 files changed, 79 insertions, 10 deletions
diff --git a/app/assets/javascripts/lib/utils/common_utils.js b/app/assets/javascripts/lib/utils/common_utils.js index a1423b6fda5..6e1590e3b0b 100644 --- a/app/assets/javascripts/lib/utils/common_utils.js +++ b/app/assets/javascripts/lib/utils/common_utils.js @@ -247,7 +247,7 @@ }); /** - * Updates the search parameter of a URL given the parameter and values provided. + * Updates the search parameter of a URL given the parameter and value provided. * * If no search params are present we'll add it. * If param for page is already present, we'll update it @@ -262,17 +262,24 @@ let search; const locationSearch = window.location.search; - if (locationSearch.length === 0) { - search = `?${param}=${value}`; - } + if (locationSearch.length) { + const parameters = locationSearch.substring(1, locationSearch.length) + .split('&') + .reduce((acc, element) => { + const val = element.split('='); + acc[val[0]] = decodeURIComponent(val[1]); + return acc; + }, {}); - if (locationSearch.indexOf(param) !== -1) { - const regex = new RegExp(param + '=\\d'); - search = locationSearch.replace(regex, `${param}=${value}`); - } + parameters[param] = value; - if (locationSearch.length && locationSearch.indexOf(param) === -1) { - search = `${locationSearch}&${param}=${value}`; + const toString = Object.keys(parameters) + .map(val => `${val}=${encodeURIComponent(parameters[val])}`) + .join('&'); + + search = `?${toString}`; + } else { + search = `?${param}=${value}`; } return search; diff --git a/changelogs/unreleased/30264-fix-vue-pagination.yml b/changelogs/unreleased/30264-fix-vue-pagination.yml new file mode 100644 index 00000000000..d5846e52bcf --- /dev/null +++ b/changelogs/unreleased/30264-fix-vue-pagination.yml @@ -0,0 +1,5 @@ +--- +title: Fixes method not replacing URL parameters correctly and breaking pipelines + pagination +merge_request: +author: diff --git a/spec/javascripts/environments/environment_spec.js b/spec/javascripts/environments/environment_spec.js index edd0cad32d0..eb8e5e50434 100644 --- a/spec/javascripts/environments/environment_spec.js +++ b/spec/javascripts/environments/environment_spec.js @@ -91,6 +91,10 @@ describe('Environment', () => { }); describe('pagination', () => { + afterEach(() => { + window.history.pushState({}, null, ''); + }); + it('should render pagination', (done) => { setTimeout(() => { expect( diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js index f4d3e77e515..7c7b334c5d8 100644 --- a/spec/javascripts/lib/utils/common_utils_spec.js +++ b/spec/javascripts/lib/utils/common_utils_spec.js @@ -46,6 +46,10 @@ require('~/lib/utils/common_utils'); spyOn(window.document, 'getElementById').and.callThrough(); }); + afterEach(() => { + window.history.pushState({}, null, ''); + }); + function expectGetElementIdToHaveBeenCalledWith(elementId) { expect(window.document.getElementById).toHaveBeenCalledWith(elementId); } @@ -75,11 +79,56 @@ require('~/lib/utils/common_utils'); }); }); + describe('gl.utils.setParamInURL', () => { + afterEach(() => { + window.history.pushState({}, null, ''); + }); + + it('should return the parameter', () => { + window.history.replaceState({}, null, ''); + + expect(gl.utils.setParamInURL('page', 156)).toBe('?page=156'); + expect(gl.utils.setParamInURL('page', '156')).toBe('?page=156'); + }); + + it('should update the existing parameter when its a number', () => { + window.history.pushState({}, null, '?page=15'); + + expect(gl.utils.setParamInURL('page', 16)).toBe('?page=16'); + expect(gl.utils.setParamInURL('page', '16')).toBe('?page=16'); + expect(gl.utils.setParamInURL('page', true)).toBe('?page=true'); + }); + + it('should update the existing parameter when its a string', () => { + window.history.pushState({}, null, '?scope=all'); + + expect(gl.utils.setParamInURL('scope', 'finished')).toBe('?scope=finished'); + }); + + it('should update the existing parameter when more than one parameter exists', () => { + window.history.pushState({}, null, '?scope=all&page=15'); + + expect(gl.utils.setParamInURL('scope', 'finished')).toBe('?scope=finished&page=15'); + }); + + it('should add a new parameter to the end of the existing ones', () => { + window.history.pushState({}, null, '?scope=all'); + + expect(gl.utils.setParamInURL('page', 16)).toBe('?scope=all&page=16'); + expect(gl.utils.setParamInURL('page', '16')).toBe('?scope=all&page=16'); + expect(gl.utils.setParamInURL('page', true)).toBe('?scope=all&page=true'); + }); + }); + describe('gl.utils.getParameterByName', () => { beforeEach(() => { window.history.pushState({}, null, '?scope=all&p=2'); }); + afterEach(() => { + window.history.replaceState({}, null, null); + }); + it('should return valid parameter', () => { const value = gl.utils.getParameterByName('scope'); expect(value).toBe('all'); diff --git a/spec/javascripts/vue_shared/components/table_pagination_spec.js b/spec/javascripts/vue_shared/components/table_pagination_spec.js index 9cb067921a7..1e3e60f249a 100644 --- a/spec/javascripts/vue_shared/components/table_pagination_spec.js +++ b/spec/javascripts/vue_shared/components/table_pagination_spec.js @@ -136,6 +136,10 @@ describe('Pagination component', () => { }); describe('paramHelper', () => { + afterEach(() => { + window.history.pushState({}, null, ''); + }); + it('can parse url parameters correctly', () => { window.history.pushState({}, null, '?scope=all&p=2'); |