Merge branch '34810-vue-pagination' into 'master'

Prevent default click since we change the url manually

Closes #34810

See merge request !12721

4 files changed, 188 insertions, 137 deletions

diff --git a/app/assets/javascripts/vue_shared/components/table_pagination.vue b/app/assets/javascripts/vue_shared/components/table_pagination.vue
index 5e7df22dd83..c9dbc048345 100644
--- a/app/assets/javascripts/vue_shared/components/table_pagination.vue
+++ b/app/assets/javascripts/vue_shared/components/table_pagination.vue
@@ -46,6 +46,8 @@ export default {
   },
   methods: {
     changePage(e) {
+      if (e.target.parentElement.classList.contains('disabled')) return;
+
       const text = e.target.innerText;
       const { totalPages, nextPage, previousPage } = this.pageInfo;
 
@@ -82,7 +84,9 @@ export default {
       const page = this.pageInfo.page;
       const items = [];
 
-      if (page > 1) items.push({ title: FIRST });
+      if (page > 1) {
+        items.push({ title: FIRST, first: true });
+      }
 
       if (page > 1) {
         items.push({ title: PREV, prev: true });
@@ -110,7 +114,9 @@ export default {
         items.push({ title: NEXT, next: true });
       }
 
-      if (total - page >= 1) items.push({ title: LAST, last: true });
+      if (total - page >= 1) {
+        items.push({ title: LAST, last: true });
+      }
 
       return items;
     },
@@ -124,13 +130,15 @@ export default {
         v-for="item in getItems"
         :class="{
           page: item.page,
-          prev: item.prev,
-          next: item.next,
+          'js-previous-button': item.prev,
+          'js-next-button': item.next,
+          'js-last-button': item.last,
+          'js-first-button': item.first,
           separator: item.separator,
           active: item.active,
           disabled: item.disabled
         }">
-        <a @click="changePage($event)">{{item.title}}</a>
+        <a @click.prevent="changePage($event)">{{item.title}}</a>
       </li>
     </ul>
   </div>
diff --git a/changelogs/unreleased/34810-vue-pagination.yml b/changelogs/unreleased/34810-vue-pagination.yml
new file mode 100644
index 00000000000..5cd03518a98
--- /dev/null
+++ b/changelogs/unreleased/34810-vue-pagination.yml
@@ -0,0 +1,4 @@
+---
+title: Prevent disabled pagination button to be clicked
+merge_request:
+author:
diff --git a/spec/javascripts/lib/utils/common_utils_spec.js b/spec/javascripts/lib/utils/common_utils_spec.js
index 52cf217c25f..55037bbbf73 100644
--- a/spec/javascripts/lib/utils/common_utils_spec.js
+++ b/spec/javascripts/lib/utils/common_utils_spec.js
@@ -143,6 +143,7 @@ import '~/lib/utils/common_utils';
 
       it('should return valid parameter', () => {
         const value = gl.utils.getParameterByName('scope');
+        expect(gl.utils.getParameterByName('p')).toEqual('2');
         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 895e1c585b4..b0b78e34e0f 100644
--- a/spec/javascripts/vue_shared/components/table_pagination_spec.js
+++ b/spec/javascripts/vue_shared/components/table_pagination_spec.js
@@ -1,150 +1,188 @@
 import Vue from 'vue';
 import paginationComp from '~/vue_shared/components/table_pagination.vue';
-import '~/lib/utils/common_utils';
 
 describe('Pagination component', () => {
   let component;
   let PaginationComponent;
-
-  const changeChanges = {
-    one: '',
-  };
-
-  const change = (one) => {
-    changeChanges.one = one;
-  };
+  let spy;
+  let mountComponet;
 
   beforeEach(() => {
+    spy = jasmine.createSpy('spy');
     PaginationComponent = Vue.extend(paginationComp);
-  });
-
-  it('should render and start at page 1', () => {
-    component = new PaginationComponent({
-      propsData: {
-        pageInfo: {
-          totalPages: 10,
-          nextPage: 2,
-          previousPage: '',
-        },
-        change,
-      },
-    }).$mount();
 
-    expect(component.$el.classList).toContain('gl-pagination');
-
-    component.changePage({ target: { innerText: '1' } });
-
-    expect(changeChanges.one).toEqual(1);
+    mountComponet = function (props) {
+      return new PaginationComponent({
+        propsData: props,
+      }).$mount();
+    };
   });
 
-  it('should go to the previous page', () => {
-    component = new PaginationComponent({
-      propsData: {
+  describe('render', () => {
+    describe('prev button', () => {
+      it('should be disabled and non clickable', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 2,
+            page: 1,
+            perPage: 20,
+            previousPage: NaN,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        expect(
+          component.$el.querySelector('.js-previous-button').classList.contains('disabled'),
+          ).toEqual(true);
+
+        component.$el.querySelector('.js-previous-button a').click();
+
+        expect(spy).not.toHaveBeenCalled();
+      });
+
+      it('should be enabled and clickable', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 3,
+            page: 2,
+            perPage: 20,
+            previousPage: 1,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        component.$el.querySelector('.js-previous-button a').click();
+
+        expect(spy).toHaveBeenCalledWith(1);
+      });
+    });
+
+    describe('first button', () => {
+      it('should call the change callback with the first page', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 3,
+            page: 2,
+            perPage: 20,
+            previousPage: 1,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        const button = component.$el.querySelector('.js-first-button a');
+
+        expect(button.textContent.trim()).toEqual('« First');
+
+        button.click();
+
+        expect(spy).toHaveBeenCalledWith(1);
+      });
+    });
+
+    describe('last button', () => {
+      it('should call the change callback with the last page', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 3,
+            page: 2,
+            perPage: 20,
+            previousPage: 1,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        const button = component.$el.querySelector('.js-last-button a');
+
+        expect(button.textContent.trim()).toEqual('Last »');
+
+        button.click();
+
+        expect(spy).toHaveBeenCalledWith(5);
+      });
+    });
+
+    describe('next button', () => {
+      it('should be disabled and non clickable', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 5,
+            page: 5,
+            perPage: 20,
+            previousPage: 1,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        expect(
+          component.$el.querySelector('.js-next-button').textContent.trim(),
+        ).toEqual('Next');
+
+        component.$el.querySelector('.js-next-button a').click();
+
+        expect(spy).not.toHaveBeenCalled();
+      });
+
+      it('should be enabled and clickable', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 4,
+            page: 3,
+            perPage: 20,
+            previousPage: 2,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        component.$el.querySelector('.js-next-button a').click();
+
+        expect(spy).toHaveBeenCalledWith(4);
+      });
+    });
+
+    describe('numbered buttons', () => {
+      it('should render 5 pages', () => {
+        component = mountComponet({
+          pageInfo: {
+            nextPage: 4,
+            page: 3,
+            perPage: 20,
+            previousPage: 2,
+            total: 84,
+            totalPages: 5,
+          },
+          change: spy,
+        });
+
+        expect(component.$el.querySelectorAll('.page').length).toEqual(5);
+      });
+    });
+
+    it('should render the spread operator', () => {
+      component = mountComponet({
         pageInfo: {
+          nextPage: 4,
+          page: 3,
+          perPage: 20,
+          previousPage: 2,
+          total: 84,
           totalPages: 10,
-          nextPage: 3,
-          previousPage: 1,
         },
-        change,
-      },
-    }).$mount();
-
-    component.changePage({ target: { innerText: 'Prev' } });
-
-    expect(changeChanges.one).toEqual(1);
-  });
-
-  it('should go to the next page', () => {
-    component = new PaginationComponent({
-      propsData: {
-        pageInfo: {
-          totalPages: 10,
-          nextPage: 5,
-          previousPage: 3,
-        },
-        change,
-      },
-    }).$mount();
-
-    component.changePage({ target: { innerText: 'Next' } });
-
-    expect(changeChanges.one).toEqual(5);
-  });
-
-  it('should go to the last page', () => {
-    component = new PaginationComponent({
-      propsData: {
-        pageInfo: {
-          totalPages: 10,
-          nextPage: 5,
-          previousPage: 3,
-        },
-        change,
-      },
-    }).$mount();
-
-    component.changePage({ target: { innerText: 'Last »' } });
-
-    expect(changeChanges.one).toEqual(10);
-  });
-
-  it('should go to the first page', () => {
-    component = new PaginationComponent({
-      propsData: {
-        pageInfo: {
-          totalPages: 10,
-          nextPage: 5,
-          previousPage: 3,
-        },
-        change,
-      },
-    }).$mount();
-
-    component.changePage({ target: { innerText: '« First' } });
-
-    expect(changeChanges.one).toEqual(1);
-  });
-
-  it('should do nothing', () => {
-    component = new PaginationComponent({
-      propsData: {
-        pageInfo: {
-          totalPages: 10,
-          nextPage: 2,
-          previousPage: '',
-        },
-        change,
-      },
-    }).$mount();
-
-    component.changePage({ target: { innerText: '...' } });
-
-    expect(changeChanges.one).toEqual(1);
-  });
-});
-
-describe('paramHelper', () => {
-  afterEach(() => {
-    window.history.pushState({}, null, '');
-  });
-
-  it('can parse url parameters correctly', () => {
-    window.history.pushState({}, null, '?scope=all&p=2');
-
-    const scope = gl.utils.getParameterByName('scope');
-    const p = gl.utils.getParameterByName('p');
-
-    expect(scope).toEqual('all');
-    expect(p).toEqual('2');
-  });
-
-  it('returns null if param not in url', () => {
-    window.history.pushState({}, null, '?p=2');
-
-    const scope = gl.utils.getParameterByName('scope');
-    const p = gl.utils.getParameterByName('p');
+        change: spy,
+      });
 
-    expect(scope).toEqual(null);
-    expect(p).toEqual('2');
+      expect(component.$el.querySelector('.separator').textContent.trim()).toEqual('...');
+    });
   });
 });