diff options
| -rw-r--r-- | app/controllers/projects/triggers_controller.rb | 5 | ||||
| -rw-r--r-- | app/models/ci/trigger.rb | 4 | ||||
| -rw-r--r-- | app/models/ci/trigger_schedule.rb | 17 | ||||
| -rw-r--r-- | app/views/projects/triggers/_form.html.haml | 53 | ||||
| -rw-r--r-- | app/views/projects/triggers/_trigger.html.haml | 4 | ||||
| -rw-r--r-- | app/workers/trigger_schedule_worker.rb | 2 | ||||
| -rw-r--r-- | changelogs/unreleased/add-ui-for-trigger-schedule.yml | 4 | ||||
| -rw-r--r-- | db/migrate/20170407114956_add_ref_to_ci_trigger_schedule.rb | 31 | ||||
| -rw-r--r-- | db/migrate/20170407122426_add_active_to_ci_trigger_schedule.rb | 31 | ||||
| -rw-r--r-- | spec/factories/ci/trigger_schedules.rb | 4 |
10 files changed, 119 insertions, 36 deletions
diff --git a/app/controllers/projects/triggers_controller.rb b/app/controllers/projects/triggers_controller.rb index c1fc40f75d7..afa56de920b 100644 --- a/app/controllers/projects/triggers_controller.rb +++ b/app/controllers/projects/triggers_controller.rb @@ -16,7 +16,6 @@ class Projects::TriggersController < Projects::ApplicationController if @trigger.valid? flash[:notice] = 'Trigger was created successfully.' else - puts "@trigger.errors: #{@trigger.errors.inspect}" flash[:alert] = 'You could not create a new trigger.' end @@ -70,8 +69,8 @@ class Projects::TriggersController < Projects::ApplicationController def trigger_params params.require(:trigger).permit( - :description, :ref, - trigger_schedule_attributes: [:cron, :cron_timezone, :_destroy] + :description, + trigger_schedule_attributes: [:id, :active, :cron, :cron_timezone, :ref] ) end end diff --git a/app/models/ci/trigger.rb b/app/models/ci/trigger.rb index 3358f901542..d01fad44fda 100644 --- a/app/models/ci/trigger.rb +++ b/app/models/ci/trigger.rb @@ -8,7 +8,7 @@ module Ci belongs_to :owner, class_name: "User" has_many :trigger_requests, dependent: :destroy - has_one :trigger_schedule, dependent: :destroy, inverse_of: :trigger + has_one :trigger_schedule, dependent: :destroy validates :token, presence: true, uniqueness: true @@ -41,7 +41,7 @@ module Ci end def trigger_schedule - super || build_trigger_schedule + super || build_trigger_schedule(project: project) end end end diff --git a/app/models/ci/trigger_schedule.rb b/app/models/ci/trigger_schedule.rb index 72716eb416b..c9a16643f32 100644 --- a/app/models/ci/trigger_schedule.rb +++ b/app/models/ci/trigger_schedule.rb @@ -6,20 +6,19 @@ module Ci acts_as_paranoid belongs_to :project - belongs_to :trigger, inverse_of: :trigger_schedule - - delegate :ref, to: :trigger, allow_nil: true + belongs_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_create :set_project before_save :set_next_run_at - def set_project - self.project = trigger.project + scope :active, -> { where(active: true) } + + def importing_or_inactive? + importing? || !active? end def set_next_run_at diff --git a/app/views/projects/triggers/_form.html.haml b/app/views/projects/triggers/_form.html.haml index c56ee3a1130..410caeeecd4 100644 --- a/app/views/projects/triggers/_form.html.haml +++ b/app/views/projects/triggers/_form.html.haml @@ -6,24 +6,39 @@ %label.label-light Token %p.form-control-static= @trigger.token .form-group - = f.label :key, "Description (For extenral trigger and scheduled trigger)", class: "label-light" + = f.label :key, "Description", class: "label-light" = f.text_field :description, class: "form-control", required: true, title: 'Trigger description is required.', placeholder: "Trigger description" - = f.fields_for :trigger_schedule do |schedule_fields| - .form-group - = schedule_fields.label :cron, "Cron (For scheduled trigger)", class: "label-light" - = schedule_fields.text_field :cron, class: "form-control", title: 'Trigger Schedule cron is required.', placeholder: "0 1 * * *" - .form-group - = schedule_fields.label :cron_timezone, "Cron timezone (For scheduled trigger)", class: "label-light" - = schedule_fields.text_field :cron_timezone, class: "form-control", title: 'Trigger Schedule cron_timezone is required.', placeholder: "UTC" - .form-group - = f.label :ref, "Ref (For scheduled trigger)", class: "label-light" - = f.text_field :ref, class: "form-control", title: 'Trigger Schedule Ref is required.', placeholder: "master" - .form-group - .checkbox - = schedule_fields.label :_destroy do - = schedule_fields.check_box :_destroy, { checked: (@trigger.trigger_schedule.id.present?) }, 0, 1 - %strong Register as scheduled trigger - .help-block - If checked, this trigger will be executed periodically according to `cron`, `cron_timezone` and `ref` - = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'visibility-of-pipelines') + - 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 + .help-block + If checked, this trigger will be executed periodically according to `cron`, `cron_timezone` and `ref` + = link_to icon('question-circle'), help_page_path('user/project/pipelines/settings', anchor: 'visibility-of-pipelines') + .form-group + = schedule_fields.label :cron, "Cron", class: "label-light" + = schedule_fields.text_field :cron, class: "form-control", title: 'Trigger Schedule cron 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: 'Trigger Schedule cron_timezone is required.', placeholder: "UTC" + .form-group + - schedule_ref = @trigger.trigger_schedule.ref || @project.default_branch + = schedule_fields.label :ref, "Branch or tag", class: "label-light" + = hidden_field_tag 'trigger[trigger_schedule_attributes][ref]', schedule_ref + = dropdown_tag(schedule_ref, + options: { toggle_class: 'js-branch-select wide', + filter: true, dropdown_class: "dropdown-menu-selectable", placeholder: "Search branches", + data: { selected: schedule_ref, field_name: 'trigger[trigger_schedule_attributes][ref]' } }) + .help-block Existing branch name, tag = f.submit btn_text, class: "btn btn-save" + + +:javascript + var availableRefs = #{@project.repository.ref_names.to_json}; + + new NewBranchForm($('.js-new-pipeline-form'), availableRefs) diff --git a/app/views/projects/triggers/_trigger.html.haml b/app/views/projects/triggers/_trigger.html.haml index 3ed1fc9ac23..71fb37928fe 100644 --- a/app/views/projects/triggers/_trigger.html.haml +++ b/app/views/projects/triggers/_trigger.html.haml @@ -30,10 +30,10 @@ Never %td - - if trigger.trigger_schedule.persisted? + - if trigger.trigger_schedule.present? && trigger.trigger_schedule.active? = trigger.trigger_schedule.real_next_run - else - None (External trigger) + None %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?" diff --git a/app/workers/trigger_schedule_worker.rb b/app/workers/trigger_schedule_worker.rb index 440c579b99d..9c1baf7e6c5 100644 --- a/app/workers/trigger_schedule_worker.rb +++ b/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, diff --git a/changelogs/unreleased/add-ui-for-trigger-schedule.yml b/changelogs/unreleased/add-ui-for-trigger-schedule.yml new file mode 100644 index 00000000000..e60a0975744 --- /dev/null +++ b/changelogs/unreleased/add-ui-for-trigger-schedule.yml @@ -0,0 +1,4 @@ +--- +title: Add UI for Trigger Schedule +merge_request: +author: diff --git a/db/migrate/20170407114956_add_ref_to_ci_trigger_schedule.rb b/db/migrate/20170407114956_add_ref_to_ci_trigger_schedule.rb new file mode 100644 index 00000000000..9bf82e49aec --- /dev/null +++ b/db/migrate/20170407114956_add_ref_to_ci_trigger_schedule.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddRefToCiTriggerSchedule < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :ci_trigger_schedules, :ref, :string + end +end diff --git a/db/migrate/20170407122426_add_active_to_ci_trigger_schedule.rb b/db/migrate/20170407122426_add_active_to_ci_trigger_schedule.rb new file mode 100644 index 00000000000..1da31c66fa6 --- /dev/null +++ b/db/migrate/20170407122426_add_active_to_ci_trigger_schedule.rb @@ -0,0 +1,31 @@ +# See http://doc.gitlab.com/ce/development/migration_style_guide.html +# for more information on how to write migrations for GitLab. + +class AddActiveToCiTriggerSchedule < ActiveRecord::Migration + include Gitlab::Database::MigrationHelpers + + # Set this constant to true if this migration requires downtime. + DOWNTIME = false + + # When a migration requires downtime you **must** uncomment the following + # constant and define a short and easy to understand explanation as to why the + # migration requires downtime. + # DOWNTIME_REASON = '' + + # When using the methods "add_concurrent_index", "remove_concurrent_index" or + # "add_column_with_default" you must disable the use of transactions + # as these methods can not run in an existing transaction. + # When using "add_concurrent_index" or "remove_concurrent_index" methods make sure + # that either of them is the _only_ method called in the migration, + # any other changes should go in a separate migration. + # This ensures that upon failure _only_ the index creation or removing fails + # and can be retried or reverted easily. + # + # To disable transactions uncomment the following line and remove these + # comments: + # disable_ddl_transaction! + + def change + add_column :ci_trigger_schedules, :active, :boolean + end +end diff --git a/spec/factories/ci/trigger_schedules.rb b/spec/factories/ci/trigger_schedules.rb index f20799245e0..315bce16995 100644 --- a/spec/factories/ci/trigger_schedules.rb +++ b/spec/factories/ci/trigger_schedules.rb @@ -4,6 +4,10 @@ FactoryGirl.define do cron '0 1 * * *' cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE + after(:build) do |trigger_schedule, evaluator| + trigger_schedule.update!(project: trigger_schedule.trigger.project) + end + trait :nightly do cron '0 1 * * *' cron_timezone Gitlab::Ci::CronParser::VALID_SYNTAX_SAMPLE_TIME_ZONE |