Merge remote-tracking branch 'upstream/master' into ce-to-ee-2018-11-02

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"
       />
     </div>
+
+    <table-pagination
+      v-if="shouldRenderPagination"
+      :change="onChangePage"
+      :page-info="state.pageInfo"
+    />
   </div>
 </template>

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;
 

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) {

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) {

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

app/controllers/projects/merge_requests_controller.rb

@@ -86,13 +86,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

app/models/ci/build.rb

@@ -263,8 +263,7 @@ module Ci
     end
 
     def schedulable?
-      Feature.enabled?('ci_enable_scheduled_build', default_enabled: true) &&
-        self.when == 'delayed' && options[:start_in].present?
+      self.when == 'delayed' && options[:start_in].present?
     end
 
     def options_scheduled_at

app/models/concerns/token_authenticatable.rb

@@ -10,6 +10,7 @@ module TokenAuthenticatable
 
     def add_authentication_token_field(token_field, options = {})
       @token_fields = [] unless @token_fields
+      unique = options.fetch(:unique, true)
 
       if @token_fields.include?(token_field)
         raise ArgumentError.new("#{token_field} already configured via add_authentication_token_field")
@@ -25,8 +26,10 @@ module TokenAuthenticatable
                    TokenAuthenticatableStrategies::Insecure.new(self, token_field, options)
                  end
 
-      define_singleton_method("find_by_#{token_field}") do |token|
-        strategy.find_token_authenticatable(token)
+      if unique
+        define_singleton_method("find_by_#{token_field}") do |token|
+          strategy.find_token_authenticatable(token)
+        end
       end
 
       define_method(token_field) do

app/models/concerns/token_authenticatable_strategies/base.rb

@@ -43,10 +43,14 @@ module TokenAuthenticatableStrategies
       set_token(instance, new_token)
     end
 
+    def unique
+      @options.fetch(:unique, true)
+    end
+
     def generate_available_token
       loop do
         token = generate_token
-        break token unless find_token_authenticatable(token, true)
+        break token unless unique && find_token_authenticatable(token, true)
       end
     end
 

app/serializers/build_action_entity.rb

@@ -12,7 +12,8 @@ class BuildActionEntity < Grape::Entity
   end
 
   expose :playable?, as: :playable
-  expose :scheduled_at, if: -> (build) { build.scheduled? }
+  expose :scheduled?, as: :scheduled
+  expose :scheduled_at, if: -> (*) { scheduled? }
 
   expose :unschedule_path, if: -> (build) { build.scheduled? } do |build|
     unschedule_project_job_path(build.project, build)
@@ -25,4 +26,8 @@ class BuildActionEntity < Grape::Entity
   def playable?
     build.playable? && can?(request.current_user, :update_build, build)
   end
+
+  def scheduled?
+    build.scheduled?
+  end
 end

app/serializers/job_entity.rb

@@ -33,6 +33,7 @@ class JobEntity < Grape::Entity
   end
 
   expose :playable?, as: :playable
+  expose :scheduled?, as: :scheduled
   expose :scheduled_at, if: -> (*) { scheduled? }
   expose :created_at
   expose :updated_at

changelogs/unreleased/28249-add-pagination.yml

+---
+title: Adds pagination to pipelines table in merge request page
+merge_request:
+author:
+type: performance

changelogs/unreleased/add-scheduled-flag-to-job-entity.yml

+---
+title: Add scheduled flag to job entity
+merge_request: 22710
+author:
+type: other

changelogs/unreleased/remove-ci_enable_scheduled_build-feature-flag.yml

+---
+title: Remove `ci_enable_scheduled_build` feature flag
+merge_request: 22742
+author:
+type: other

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

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
 

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', () => {

spec/models/ci/build_spec.rb

@@ -217,14 +217,6 @@ describe Ci::Build do
       let(:build) { create(:ci_build, :created, :schedulable, project: project) }
 
       it { expect(subject).to be_truthy }
-
-      context 'when feature flag is diabled' do
-        before do
-          stub_feature_flags(ci_enable_scheduled_build: false)
-        end
-
-        it { expect(subject).to be_falsy }
-      end
     end
 
     context 'when build is not schedulable' do
@@ -328,10 +320,6 @@ describe Ci::Build do
   describe '#enqueue_scheduled' do
     subject { build.enqueue_scheduled }
 
-    before do
-      stub_feature_flags(ci_enable_scheduled_build: true)
-    end
-
     context 'when build is scheduled and the right time has not come yet' do
       let(:build) { create(:ci_build, :scheduled, pipeline: pipeline) }
 

spec/serializers/build_action_entity_spec.rb

@@ -26,6 +26,10 @@ describe BuildActionEntity do
     context 'when job is scheduled' do
       let(:job) { create(:ci_build, :scheduled) }
 
+      it 'returns scheduled' do
+        expect(subject[:scheduled]).to be_truthy
+      end
+
       it 'returns scheduled_at' do
         expect(subject[:scheduled_at]).to eq(job.scheduled_at)
       end

spec/serializers/job_entity_spec.rb

@@ -117,6 +117,7 @@ describe JobEntity do
     end
 
     it 'contains scheduled_at' do
+      expect(subject[:scheduled]).to be_truthy
       expect(subject[:scheduled_at]).to eq(job.scheduled_at)
     end
   end

spec/services/ci/process_build_service_spec.rb

@@ -98,47 +98,19 @@ describe Ci::ProcessBuildService, '#execute' do
 
     let(:build) { create(:ci_build, :created, :schedulable, user: user, project: project) }
 
-    context 'when ci_enable_scheduled_build is enabled' do
-      before do
-        stub_feature_flags(ci_enable_scheduled_build: true)
-      end
-
-      context 'when current status is success' do
-        let(:current_status) { 'success' }
-
-        it 'changes the build status' do
-          expect { subject }.to change { build.status }.to('scheduled')
-        end
-      end
-
-      context 'when current status is failed' do
-        let(:current_status) { 'failed' }
+    context 'when current status is success' do
+      let(:current_status) { 'success' }
 
-        it 'does not change the build status' do
-          expect { subject }.to change { build.status }.to('skipped')
-        end
+      it 'changes the build status' do
+        expect { subject }.to change { build.status }.to('scheduled')
       end
     end
 
-    context 'when ci_enable_scheduled_build is disabled' do
-      before do
-        stub_feature_flags(ci_enable_scheduled_build: false)
-      end
-
-      context 'when current status is success' do
-        let(:current_status) { 'success' }
-
-        it 'changes the build status' do
-          expect { subject }.to change { build.status }.to('manual')
-        end
-      end
-
-      context 'when current status is failed' do
-        let(:current_status) { 'failed' }
+    context 'when current status is failed' do
+      let(:current_status) { 'failed' }
 
-        it 'does not change the build status' do
-          expect { subject }.to change { build.status }.to('skipped')
-        end
+      it 'does not change the build status' do
+        expect { subject }.to change { build.status }.to('skipped')
       end
     end
   end

spec/services/ci/run_scheduled_build_service_spec.rb

@@ -7,10 +7,6 @@ describe Ci::RunScheduledBuildService do
 
   subject { described_class.new(project, user).execute(build) }
 
-  before do
-    stub_feature_flags(ci_enable_scheduled_build: true)
-  end
-
   context 'when user can update build' do
     before do
       project.add_developer(user)