Controller logic. Delete before modification. Halfway.

4 files changed, 48 insertions, 18 deletions

diff --git a/app/controllers/projects/pipeline_schedules_controller.rb b/app/controllers/projects/pipeline_schedules_controller.rb
index f0ac0e7098c..fdbf930a5ef 100644
--- a/app/controllers/projects/pipeline_schedules_controller.rb
+++ b/app/controllers/projects/pipeline_schedules_controller.rb
@@ -1,11 +1,11 @@
 class Projects::PipelineSchedulesController < Projects::ApplicationController
-  before_action :schedule, except: [:index, :new, :create]
-
   before_action :authorize_read_pipeline_schedule!
   before_action :authorize_create_pipeline_schedule!, only: [:new, :create]
-  before_action :authorize_update_pipeline_schedule!, except: [:index, :new, :create]
+  before_action :authorize_update_pipeline_schedule!, only: [:edit, :take_ownership, :update]
   before_action :authorize_admin_pipeline_schedule!, only: [:destroy]
 
+  before_action :schedule, only: [:edit, :update, :destroy, :take_ownership]
+
   def index
     @scope = params[:scope]
     @all_schedules = PipelineSchedulesFinder.new(@project).execute
@@ -67,10 +67,6 @@ class Projects::PipelineSchedulesController < Projects::ApplicationController
   def schedule_params
     params.require(:schedule)
       .permit(:description, :cron, :cron_timezone, :ref, :active,
-        variables_attributes: [:id, :key, :value, :_destroy] )
-  end
-
-  def authorize_update_pipeline_schedule!
-    return access_denied! unless can?(current_user, :update_pipeline_schedule, schedule)
+        variables_attributes: [:key, :value] )
   end
 end
diff --git a/app/models/ci/pipeline_schedule.rb b/app/models/ci/pipeline_schedule.rb
index ad9f8b89924..61215b1fb46 100644
--- a/app/models/ci/pipeline_schedule.rb
+++ b/app/models/ci/pipeline_schedule.rb
@@ -9,17 +9,13 @@ module Ci
     belongs_to :owner, class_name: 'User'
     has_one :last_pipeline, -> { order(id: :desc) }, class_name: 'Ci::Pipeline'
     has_many :pipelines
-    has_many :variables, class_name: 'Ci::PipelineScheduleVariable'
+    has_many :variables, :dependent => :destroy, class_name: 'Ci::PipelineScheduleVariable'
 
     validates :cron, unless: :importing?, cron: true, presence: { unless: :importing? }
     validates :cron_timezone, cron_timezone: true, presence: { unless: :importing? }
     validates :ref, presence: { unless: :importing? }
     validates :description, presence: true
-    validates :variables, uniqueness_of_in_memory: {
-                :collection => :variables,
-                :attrs => [:pipeline_schedule_id, :key],
-                :message => ['variables.key', 'keys are duplicated']
-              }
+    validates_associated :variables
 
     before_save :set_next_run_at
 
@@ -28,6 +24,15 @@ module Ci
 
     accepts_nested_attributes_for :variables, allow_destroy: true
 
+    before_validation(on: :update) do
+      # TODO: if validation failed, restore the deleted_obj
+      deleted_obj = Ci::PipelineScheduleVariable.where(pipeline_schedule_id: self).destroy_all
+    end
+
+    after_validation(on: :update) do
+      # TODO: if validation failed, restore the deleted_obj
+    end
+
     def owned_by?(current_user)
       owner == current_user
     end
@@ -64,9 +69,5 @@ module Ci
       Gitlab::Ci::CronParser.new(worker_cron, worker_time_zone)
                             .next_time_from(next_run_at)
     end
-
-    def job_variables
-      variables&.map(&:to_runner_variable) || []
-    end
   end
 end
diff --git a/app/models/ci/pipeline_schedule_variable.rb b/app/models/ci/pipeline_schedule_variable.rb
index 2d681446d00..ee5b8733fac 100644
--- a/app/models/ci/pipeline_schedule_variable.rb
+++ b/app/models/ci/pipeline_schedule_variable.rb
@@ -4,6 +4,7 @@ module Ci
     include HasVariable
 
     belongs_to :pipeline_schedule
+
     validates :key, uniqueness: { scope: :pipeline_schedule_id }
   end
 end
diff --git a/app/views/projects/pipeline_schedules/_form.html.haml b/app/views/projects/pipeline_schedules/_form.html.haml
index 0e600c59ef6..3857a59a043 100644
--- a/app/views/projects/pipeline_schedules/_form.html.haml
+++ b/app/views/projects/pipeline_schedules/_form.html.haml
@@ -22,6 +22,38 @@
       = f.label :ref, _('Target Branch'), class: 'label-light'
       = dropdown_tag(_("Select target branch"), options: { toggle_class: 'btn js-target-branch-dropdown', dropdown_class: 'git-revision-dropdown', title: _("Select target branch"), filter: true, placeholder: s_("OfSearchInADropdown|Filter"), data: { data: @project.repository.branch_names, default_branch: @project.default_branch } } )
       = f.text_field :ref, value: @schedule.ref, id: 'schedule_ref', class: 'hidden', name: 'schedule[ref]', required: true
+  -# TODO: Test code
+  = @schedule.variables.inspect
+  - if @schedule.variables.present?
+    - @schedule.variables.each_with_index do |variable, i|
+      .form-group
+        .col-md-9
+          %label.label-light Key
+          %input.form-control{:name => "schedule[variables_attributes][#{i}][key]", :type => "text", :value => variable.key}/
+          %p.gl-field-error.hide This field is required.
+          %label.label-light Value
+          %input.form-control{:name => "schedule[variables_attributes][#{i}][value]", :type => "text", :value => variable.value}/
+          %p.gl-field-error.hide This field is required.
+    - if @schedule.variables.count == 1
+      - (1..1).each do |i|
+        .form-group
+          .col-md-9
+            %label.label-light Key
+            %input.form-control{:name => "schedule[variables_attributes][#{i}][key]", :type => "text"}/
+            %p.gl-field-error.hide This field is required.
+            %label.label-light Value
+            %input.form-control{:name => "schedule[variables_attributes][#{i}][value]", :type => "text"}/
+            %p.gl-field-error.hide This field is required.
+  - else
+    - (0..0).each do |i|
+      .form-group
+        .col-md-9
+          %label.label-light Key
+          %input.form-control{:name => "schedule[variables_attributes][#{i}][key]", :type => "text"}/
+          %p.gl-field-error.hide This field is required.
+          %label.label-light Value
+          %input.form-control{:name => "schedule[variables_attributes][#{i}][value]", :type => "text"}/
+          %p.gl-field-error.hide This field is required.
   .form-group
     .col-md-9
       %label.label-light