Merge branch...

Merge branch 'dosuken123/gitlab-ce-2989-run-cicd-pipelines-on-a-schedule-idea1-basic-backend-implementation-with-quick-ui' into 'master'

Add "engineering" UI for Pipeline Schedule

See merge request !10533

app/controllers/projects/triggers_controller.rb

@@ -11,7 +11,7 @@ class Projects::TriggersController < Projects::ApplicationController
   end
 
   def create
-    @trigger = project.triggers.create(create_params.merge(owner: current_user))
+    @trigger = project.triggers.create(trigger_params.merge(owner: current_user))
 
     if @trigger.valid?
       flash[:notice] = 'Trigger was created successfully.'
@@ -36,7 +36,7 @@ class Projects::TriggersController < Projects::ApplicationController
   end
 
   def update
-    if trigger.update(update_params)
+    if trigger.update(trigger_params)
       redirect_to namespace_project_settings_ci_cd_path(@project.namespace, @project), notice: 'Trigger was successfully updated.'
     else
       render action: "edit"
@@ -67,11 +67,10 @@ class Projects::TriggersController < Projects::ApplicationController
     @trigger ||= project.triggers.find(params[:id]) || render_404
   end
 
-  def create_params
-    params.require(:trigger).permit(:description)
-  end
-
-  def update_params
-    params.require(:trigger).permit(:description)
+  def trigger_params
+    params.require(:trigger).permit(
+      :description,
+      trigger_schedule_attributes: [:id, :active, :cron, :cron_timezone, :ref]
+    )
   end
 end

app/models/ci/trigger.rb

@@ -14,6 +14,8 @@ module Ci
 
     before_validation :set_default_values
 
+    accepts_nested_attributes_for :trigger_schedule
+
     def set_default_values
       self.token = SecureRandom.hex(15) if self.token.blank?
     end
@@ -37,5 +39,9 @@ module Ci
     def can_access_project?
       self.owner_id.blank? || Ability.allowed?(self.owner, :create_build, project)
     end
+
+    def trigger_schedule
+      super || build_trigger_schedule(project: project)
+    end
   end
 end

app/models/ci/trigger_schedule.rb

@@ -8,15 +8,19 @@ module Ci
     belongs_to :project
     belongs_to :trigger
 
-    delegate :ref, to: :trigger
-
     validates :trigger, presence: { unless: :importing? }
-    validates :cron, cron: true, presence: { unless: :importing? }
-    validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? }
-    validates :ref, presence: { unless: :importing? }
+    validates :cron, unless: :importing_or_inactive?, cron: true, presence: { unless: :importing_or_inactive? }
+    validates :cron_timezone, cron_timezone: true, presence: { unless: :importing_or_inactive? }
+    validates :ref, presence: { unless: :importing_or_inactive? }
 
     before_save :set_next_run_at
 
+    scope :active, -> { where(active: true) }
+
+    def importing_or_inactive?
+      importing? || !active?
+    end
+
     def set_next_run_at
       self.next_run_at = Gitlab::Ci::CronParser.new(cron, cron_timezone).next_time_from(Time.now)
     end
@@ -26,5 +30,12 @@ module Ci
     rescue ActiveRecord::RecordInvalid
       update_attribute(:next_run_at, nil) # update without validation
     end
+
+    def real_next_run(
+        worker_cron: Settings.cron_jobs['trigger_schedule_worker']['cron'],
+        worker_time_zone: Time.zone.name)
+      Gitlab::Ci::CronParser.new(worker_cron, worker_time_zone)
+                            .next_time_from(next_run_at)
+    end
   end
 end

app/views/projects/triggers/_form.html.haml

@@ -8,4 +8,26 @@
   .form-group
     = f.label :key, "Description", class: "label-light"
     = f.text_field :description, class: "form-control", required: true, title: 'Trigger description is required.', placeholder: "Trigger description"
+    - if @trigger.persisted?
+      %hr
+      = f.fields_for :trigger_schedule do |schedule_fields|
+        = schedule_fields.hidden_field :id
+        .form-group
+          .checkbox
+            = schedule_fields.label :active do
+              = schedule_fields.check_box :active
+              %strong Schedule trigger (experimental)
+            .help-block
+              If checked, this trigger will be executed periodically according to cron and timezone.
+              = link_to icon('question-circle'), help_page_path('ci/triggers', anchor: 'schedule')
+        .form-group
+          = schedule_fields.label :cron, "Cron", class: "label-light"
+          = schedule_fields.text_field :cron, class: "form-control", title: 'Cron specification is required.', placeholder: "0 1 * * *"
+        .form-group
+          = schedule_fields.label :cron, "Timezone", class: "label-light"
+          = schedule_fields.text_field :cron_timezone, class: "form-control", title: 'Timezone is required.', placeholder: "UTC"
+        .form-group
+          = schedule_fields.label :ref, "Branch or tag", class: "label-light"
+          = schedule_fields.text_field :ref, class: "form-control", title: 'Branch or tag is required.', placeholder: "master"
+          .help-block Existing branch name, tag
   = f.submit btn_text, class: "btn btn-save"

app/views/projects/triggers/_index.html.haml

@@ -22,6 +22,8 @@
                 %th
                   %strong Last used
                 %th
+                  %strong Next run at
+                %th
               = render partial: 'projects/triggers/trigger', collection: @triggers, as: :trigger
         - else
           %p.settings-message.text-center.append-bottom-default

app/views/projects/triggers/_trigger.html.haml

@@ -29,6 +29,12 @@
     - else
       Never
 
+  %td
+    - if trigger.trigger_schedule&.active?
+      = trigger.trigger_schedule.real_next_run
+    - else
+      Never
+
   %td.text-right.trigger-actions
     - take_ownership_confirmation = "By taking ownership you will bind this trigger to your user account. With this the trigger will have access to all your projects as if it was you. Are you sure?"
     - revoke_trigger_confirmation = "By revoking a trigger you will break any processes making use of it. Are you sure?"

app/workers/trigger_schedule_worker.rb

@@ -3,7 +3,7 @@ class TriggerScheduleWorker
   include CronjobQueue
 
   def perform
-    Ci::TriggerSchedule.where("next_run_at < ?", Time.now).find_each do |trigger_schedule|
+    Ci::TriggerSchedule.active.where("next_run_at < ?", Time.now).find_each do |trigger_schedule|
       begin
         Ci::CreateTriggerRequestService.new.execute(trigger_schedule.project,
                                                     trigger_schedule.trigger,

changelogs/unreleased/add-ui-for-trigger-schedule.yml

+---
+title: Add UI for Trigger Schedule
+merge_request: 10533
+author: dosuken123

db/migrate/20170407114956_add_ref_to_ci_trigger_schedule.rb

+class AddRefToCiTriggerSchedule < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_trigger_schedules, :ref, :string
+  end
+end

db/migrate/20170407122426_add_active_to_ci_trigger_schedule.rb

+class AddActiveToCiTriggerSchedule < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  def change
+    add_column :ci_trigger_schedules, :active, :boolean
+  end
+end

db/migrate/20170407140450_add_index_to_next_run_at_and_active.rb

+# See http://doc.gitlab.com/ce/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddIndexToNextRunAtAndActive < ActiveRecord::Migration
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  disable_ddl_transaction!
+
+  def up
+    add_concurrent_index :ci_trigger_schedules, [:active, :next_run_at]
+  end
+
+  def down
+    remove_concurrent_index :ci_trigger_schedules, [:active, :next_run_at]
+  end
+end

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20170406115029) do
+ActiveRecord::Schema.define(version: 20170407140450) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -311,8 +311,11 @@ ActiveRecord::Schema.define(version: 20170406115029) do
     t.string "cron"
     t.string "cron_timezone"
     t.datetime "next_run_at"
+    t.string "ref"
+    t.boolean "active"
   end
 
+  add_index "ci_trigger_schedules", ["active", "next_run_at"], name: "index_ci_trigger_schedules_on_active_and_next_run_at", using: :btree
   add_index "ci_trigger_schedules", ["next_run_at"], name: "index_ci_trigger_schedules_on_next_run_at", using: :btree
   add_index "ci_trigger_schedules", ["project_id"], name: "index_ci_trigger_schedules_on_project_id", using: :btree
 

spec/factories/ci/trigger_schedules.rb

@@ -3,9 +3,11 @@ FactoryGirl.define do
     trigger factory: :ci_trigger_for_trigger_schedule
     cron '0 1 * * *'
     cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE
+    ref 'master'
+    active true
 
     after(:build) do |trigger_schedule, evaluator|
-      trigger_schedule.update!(project: trigger_schedule.trigger.project)
+      trigger_schedule.project ||= trigger_schedule.trigger.project
     end
 
     trait :nightly do

spec/features/triggers_spec.rb

@@ -77,6 +77,59 @@ feature 'Triggers', feature: true, js: true do
       expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
       expect(page.find('.triggers-list')).to have_content new_trigger_title
     end
+
+    context 'scheduled triggers' do
+      let!(:trigger) do
+        create(:ci_trigger, owner: user, project: @project, description: trigger_title)
+      end
+
+      context 'enabling schedule' do
+        before do
+          visit edit_namespace_project_trigger_path(@project.namespace, @project, trigger)
+        end
+
+        scenario 'do fill form with valid data and save' do
+          find('#trigger_trigger_schedule_attributes_active').click
+          fill_in 'trigger_trigger_schedule_attributes_cron', with: '1 * * * *'
+          fill_in 'trigger_trigger_schedule_attributes_cron_timezone', with: 'UTC'
+          fill_in 'trigger_trigger_schedule_attributes_ref', with: 'master'
+          click_button 'Save trigger'
+
+          expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
+        end
+
+        scenario 'do not fill form with valid data and save' do
+          find('#trigger_trigger_schedule_attributes_active').click
+          click_button 'Save trigger'
+
+          expect(page).to have_content 'The form contains the following errors'
+        end
+      end
+
+      context 'disabling schedule' do
+        before do
+          trigger.create_trigger_schedule(
+            project: trigger.project,
+            active: true,
+            ref: 'master',
+            cron: '1 * * * *',
+            cron_timezone: 'UTC')
+
+          visit edit_namespace_project_trigger_path(@project.namespace, @project, trigger)
+        end
+
+        scenario 'disable and save form' do
+          find('#trigger_trigger_schedule_attributes_active').click
+          click_button 'Save trigger'
+          expect(page.find('.flash-notice')).to have_content 'Trigger was successfully updated.'
+
+          visit edit_namespace_project_trigger_path(@project.namespace, @project, trigger)
+          checkbox = find_field('trigger_trigger_schedule_attributes_active')
+
+          expect(checkbox).not_to be_checked
+        end
+      end
+    end
   end
 
   describe 'trigger "Take ownership" workflow' do

spec/lib/gitlab/import_export/safe_model_attributes.yml

@@ -253,6 +253,8 @@ Ci::TriggerSchedule:
 - cron
 - cron_timezone
 - next_run_at
+- ref
+- active
 DeployKey:
 - id
 - user_id

spec/workers/trigger_schedule_worker_spec.rb

@@ -8,24 +8,39 @@ describe TriggerScheduleWorker do
   end
 
   context 'when there is a scheduled trigger within next_run_at' do
+    let(:next_run_at) { 2.days.ago }
+
+    let!(:trigger_schedule) do
+      create(:ci_trigger_schedule, :nightly)
+    end
+
     before do
-      trigger_schedule = create(:ci_trigger_schedule, :nightly)
-      time_future = Time.now + 10.days
-      allow(Time).to receive(:now).and_return(time_future)
-      @next_time = Gitlab::Ci::CronParser.new(trigger_schedule.cron, trigger_schedule.cron_timezone).next_time_from(time_future)
+      trigger_schedule.update_column(:next_run_at, next_run_at)
     end
 
     it 'creates a new trigger request' do
-      expect { worker.perform }.to change { Ci::TriggerRequest.count }.by(1)
+      expect { worker.perform }.to change { Ci::TriggerRequest.count }
     end
 
     it 'creates a new pipeline' do
-      expect { worker.perform }.to change { Ci::Pipeline.count }.by(1)
+      expect { worker.perform }.to change { Ci::Pipeline.count }
       expect(Ci::Pipeline.last).to be_pending
     end
 
     it 'updates next_run_at' do
-      expect { worker.perform }.to change { Ci::TriggerSchedule.last.next_run_at }.to(@next_time)
+      worker.perform
+
+      expect(trigger_schedule.reload.next_run_at).not_to eq(next_run_at)
+    end
+
+    context 'inactive schedule' do
+      before do
+        trigger_schedule.update(active: false)
+      end
+
+      it 'does not create a new trigger' do
+        expect { worker.perform }.not_to change { Ci::TriggerRequest.count }
+      end
     end
   end
 
@@ -43,8 +58,8 @@ describe TriggerScheduleWorker do
 
   context 'when next_run_at is nil' do
     before do
-      trigger_schedule = create(:ci_trigger_schedule, :nightly)
-      trigger_schedule.update_attribute(:next_run_at, nil)
+      schedule = create(:ci_trigger_schedule, :nightly)
+      schedule.update_column(:next_run_at, nil)
     end
 
     it 'does not create a new pipeline' do