From 87444fb6eda755a0340ab2e38ad83bb8edc67f6f Mon Sep 17 00:00:00 2001 From: Filipa Lacerda Date: Tue, 30 Oct 2018 15:56:30 +0000 Subject: [PATCH] Adds pagination to pipelines table in merge request page --- .../commit/pipelines/pipelines_table.vue | 19 +++++++++- .../pipelines/components/pipelines.vue | 38 ------------------- .../javascripts/pipelines/mixins/pipelines.js | 38 +++++++++++++++++++ .../mixins/ci_pagination_api_mixin.js | 9 ++++- app/controllers/projects/commit_controller.rb | 3 +- .../projects/merge_requests_controller.rb | 3 +- .../unreleased/28249-add-pagination.yml | 5 +++ .../projects/commit_controller_spec.rb | 1 + .../merge_requests_controller_spec.rb | 1 + .../commit/pipelines/pipelines_spec.js | 23 +++++++++++ 10 files changed, 98 insertions(+), 42 deletions(-) create mode 100644 changelogs/unreleased/28249-add-pagination.yml diff --git a/app/assets/javascripts/commit/pipelines/pipelines_table.vue b/app/assets/javascripts/commit/pipelines/pipelines_table.vue index a2aa3d197e3..82532539c9c 100644 --- a/app/assets/javascripts/commit/pipelines/pipelines_table.vue +++ b/app/assets/javascripts/commit/pipelines/pipelines_table.vue @@ -2,9 +2,15 @@ import PipelinesService from '../../pipelines/services/pipelines_service'; import PipelineStore from '../../pipelines/stores/pipelines_store'; import pipelinesMixin from '../../pipelines/mixins/pipelines'; +import TablePagination from '../../vue_shared/components/table_pagination.vue'; +import { getParameterByName } from '../../lib/utils/common_utils'; +import CIPaginationMixin from '../../vue_shared/mixins/ci_pagination_api_mixin'; export default { - mixins: [pipelinesMixin], + components: { + TablePagination, + }, + mixins: [pipelinesMixin, CIPaginationMixin], props: { endpoint: { type: String, @@ -35,6 +41,8 @@ export default { return { store, state: store.state, + page: getParameterByName('page') || '1', + requestData: {}, }; }, @@ -48,11 +56,14 @@ export default { }, created() { this.service = new PipelinesService(this.endpoint); + this.requestData = { page: this.page }; }, methods: { successCallback(resp) { // depending of the endpoint the response can either bring a `pipelines` key or not. const pipelines = resp.data.pipelines || resp.data; + + this.store.storePagination(resp.headers); this.setCommonData(pipelines); const updatePipelinesEvent = new CustomEvent('update-pipelines-count', { @@ -97,5 +108,11 @@ export default { :view-type="viewType" /> + + diff --git a/app/assets/javascripts/pipelines/components/pipelines.vue b/app/assets/javascripts/pipelines/components/pipelines.vue index ea526cf1309..fcd8a54c9c1 100644 --- a/app/assets/javascripts/pipelines/components/pipelines.vue +++ b/app/assets/javascripts/pipelines/components/pipelines.vue @@ -155,14 +155,6 @@ export default { ); }, - shouldRenderPagination() { - return ( - !this.isLoading && - this.state.pipelines.length && - this.state.pageInfo.total > this.state.pageInfo.perPage - ); - }, - emptyTabMessage() { const { scopes } = this.$options; const possibleScopes = [scopes.pending, scopes.running, scopes.finished]; @@ -232,36 +224,6 @@ export default { this.setCommonData(resp.data.pipelines); } }, - /** - * Handles URL and query parameter changes. - * When the user uses the pagination or the tabs, - * - update URL - * - Make API request to the server with new parameters - * - Update the polling function - * - Update the internal state - */ - updateContent(parameters) { - this.updateInternalState(parameters); - - // fetch new data - return this.service - .getPipelines(this.requestData) - .then(response => { - this.isLoading = false; - this.successCallback(response); - - // restart polling - this.poll.restart({ data: this.requestData }); - }) - .catch(() => { - this.isLoading = false; - this.errorCallback(); - - // restart polling - this.poll.restart({ data: this.requestData }); - }); - }, - handleResetRunnersCache(endpoint) { this.isResetCacheButtonLoading = true; diff --git a/app/assets/javascripts/pipelines/mixins/pipelines.js b/app/assets/javascripts/pipelines/mixins/pipelines.js index 8929b397f6c..85781f548c6 100644 --- a/app/assets/javascripts/pipelines/mixins/pipelines.js +++ b/app/assets/javascripts/pipelines/mixins/pipelines.js @@ -23,6 +23,15 @@ export default { hasMadeRequest: false, }; }, + computed: { + shouldRenderPagination() { + return ( + !this.isLoading && + this.state.pipelines.length && + this.state.pageInfo.total > this.state.pageInfo.perPage + ); + }, + }, beforeMount() { this.poll = new Poll({ resource: this.service, @@ -65,6 +74,35 @@ export default { this.poll.stop(); }, methods: { + /** + * Handles URL and query parameter changes. + * When the user uses the pagination or the tabs, + * - update URL + * - Make API request to the server with new parameters + * - Update the polling function + * - Update the internal state + */ + updateContent(parameters) { + this.updateInternalState(parameters); + + // fetch new data + return this.service + .getPipelines(this.requestData) + .then(response => { + this.isLoading = false; + this.successCallback(response); + + // restart polling + this.poll.restart({ data: this.requestData }); + }) + .catch(() => { + this.isLoading = false; + this.errorCallback(); + + // restart polling + this.poll.restart({ data: this.requestData }); + }); + }, updateTable() { // Cancel ongoing request if (this.isMakingRequest) { diff --git a/app/assets/javascripts/vue_shared/mixins/ci_pagination_api_mixin.js b/app/assets/javascripts/vue_shared/mixins/ci_pagination_api_mixin.js index 67a1632269e..f9e3f3df0cc 100644 --- a/app/assets/javascripts/vue_shared/mixins/ci_pagination_api_mixin.js +++ b/app/assets/javascripts/vue_shared/mixins/ci_pagination_api_mixin.js @@ -14,7 +14,14 @@ export default { onChangePage(page) { /* URLS parameters are strings, we need to parse to match types */ - this.updateContent({ scope: this.scope, page: Number(page).toString() }); + const params = { + page: Number(page).toString(), + }; + + if (this.scope) { + params.scope = this.scope; + } + this.updateContent(params); }, updateInternalState(parameters) { diff --git a/app/controllers/projects/commit_controller.rb b/app/controllers/projects/commit_controller.rb index 00b63f55710..32fc5140366 100644 --- a/app/controllers/projects/commit_controller.rb +++ b/app/controllers/projects/commit_controller.rb @@ -43,7 +43,7 @@ class Projects::CommitController < Projects::ApplicationController # rubocop: disable CodeReuse/ActiveRecord def pipelines @pipelines = @commit.pipelines.order(id: :desc) - @pipelines = @pipelines.where(ref: params[:ref]) if params[:ref] + @pipelines = @pipelines.where(ref: params[:ref]).page(params[:page]).per(30) if params[:ref] respond_to do |format| format.html @@ -53,6 +53,7 @@ class Projects::CommitController < Projects::ApplicationController render json: { pipelines: PipelineSerializer .new(project: @project, current_user: @current_user) + .with_pagination(request, response) .represent(@pipelines), count: { all: @pipelines.count diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 27b83da4f54..4bdb857b2d9 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -84,13 +84,14 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo end def pipelines - @pipelines = @merge_request.all_pipelines + @pipelines = @merge_request.all_pipelines.page(params[:page]).per(30) Gitlab::PollingInterval.set_header(response, interval: 10_000) render json: { pipelines: PipelineSerializer .new(project: @project, current_user: @current_user) + .with_pagination(request, response) .represent(@pipelines), count: { all: @pipelines.count diff --git a/changelogs/unreleased/28249-add-pagination.yml b/changelogs/unreleased/28249-add-pagination.yml new file mode 100644 index 00000000000..df15094405a --- /dev/null +++ b/changelogs/unreleased/28249-add-pagination.yml @@ -0,0 +1,5 @@ +--- +title: Adds pagination to pipelines table in merge request page +merge_request: +author: +type: performance diff --git a/spec/controllers/projects/commit_controller_spec.rb b/spec/controllers/projects/commit_controller_spec.rb index 9e149bc4c3c..e34fdee62d6 100644 --- a/spec/controllers/projects/commit_controller_spec.rb +++ b/spec/controllers/projects/commit_controller_spec.rb @@ -356,6 +356,7 @@ describe Projects::CommitController do expect(response).to be_ok expect(JSON.parse(response.body)['pipelines']).not_to be_empty expect(JSON.parse(response.body)['count']['all']).to eq 1 + expect(response).to include_pagination_headers end end end diff --git a/spec/controllers/projects/merge_requests_controller_spec.rb b/spec/controllers/projects/merge_requests_controller_spec.rb index 7b0459e0325..d08cbfb134c 100644 --- a/spec/controllers/projects/merge_requests_controller_spec.rb +++ b/spec/controllers/projects/merge_requests_controller_spec.rb @@ -563,6 +563,7 @@ describe Projects::MergeRequestsController do it 'responds with serialized pipelines' do expect(json_response['pipelines']).not_to be_empty expect(json_response['count']['all']).to eq 1 + expect(response).to include_pagination_headers end end diff --git a/spec/javascripts/commit/pipelines/pipelines_spec.js b/spec/javascripts/commit/pipelines/pipelines_spec.js index b797cc44ae7..04c8ab44405 100644 --- a/spec/javascripts/commit/pipelines/pipelines_spec.js +++ b/spec/javascripts/commit/pipelines/pipelines_spec.js @@ -72,6 +72,29 @@ describe('Pipelines table in Commits and Merge requests', function() { done(); }, 0); }); + + describe('with pagination', () => { + it('should make an API request when using pagination', done => { + setTimeout(() => { + spyOn(vm, 'updateContent'); + + vm.store.state.pageInfo = { + page: 1, + total: 10, + perPage: 2, + nextPage: 2, + totalPages: 5, + }; + + vm.$nextTick(() => { + vm.$el.querySelector('.js-next-button a').click(); + + expect(vm.updateContent).toHaveBeenCalledWith({ page: '2' }); + done(); + }); + }); + }); + }); }); describe('pipeline badge counts', () => {