Fix ref reference

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