Port of feature/intermediate/32568-adding-variables-to-pipelines-schedules to EE

See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12372

Port of feature/intermediate/32568-adding-variables-to-pipelines-schedules to EE
See https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/12372
3c267c74 · Eric Eastwood · c321d953 · 3c267c74 · 3c267c74 · 3c267c74
--- a/locale/en/gitlab.po
+++ b/locale/en/gitlab.po
@@ -17,6 +17,16 @@ msgstr ""
 "Plural-Forms: nplurals=2; plural=n != 1;\n"
 "\n"
+msgid "%d additional commit has been omitted to prevent performance issues."
+msgid_plural "%d additional commits have been omitted to prevent performance issues."
+msgstr[0] ""
+msgstr[1] ""
+msgid "%d commit"
+msgid_plural "%d commits"
+msgstr[0] ""
+msgstr[1] ""
 msgid "%{commit_author_link} committed %{commit_timeago}"
 msgstr ""
@@ -61,9 +71,24 @@ msgstr[1] ""
 msgid "Branch <strong>%{branch_name}</strong> was created. To set up auto deploy, choose a GitLab CI Yaml template and commit your changes. %{link_to_autodeploy_doc}"
 msgstr ""
+msgid "BranchSwitcherPlaceholder|Search branches"
+msgstr ""
+msgid "BranchSwitcherTitle|Switch branch"
+msgstr ""
 msgid "Branches"
 msgstr ""
+msgid "Browse Directory"
+msgstr ""
+msgid "Browse File"
+msgstr ""
+msgid "Browse Files"
+msgstr ""
 msgid "Browse files"
 msgstr ""
@@ -171,6 +196,9 @@ msgstr ""
 msgid "Commits"
 msgstr ""
+msgid "Commits feed"
+msgstr ""
 msgid "Commits|History"
 msgstr ""
@@ -195,6 +223,9 @@ msgstr ""
 msgid "Create New Directory"
 msgstr ""
+msgid "Create a personal access token on your account to pull or push via %{protocol}."
+msgstr ""
 msgid "Create directory"
 msgstr ""
@@ -213,6 +244,9 @@ msgstr ""
 msgid "CreateTag|Tag"
 msgstr ""
+msgid "CreateTokenToCloneLink|create a personal access token"
+msgstr ""
 msgid "Cron Timezone"
 msgstr ""
@@ -323,6 +357,9 @@ msgstr ""
 msgid "Files"
 msgstr ""
+msgid "Filter by commit message"
+msgstr ""
 msgid "Find by path"
 msgstr ""
@@ -547,6 +584,12 @@ msgstr ""
 msgid "PipelineSchedules|Inactive"
 msgstr ""
+msgid "PipelineSchedules|Input variable key"
+msgstr ""
+msgid "PipelineSchedules|Input variable value"
+msgstr ""
 msgid "PipelineSchedules|Next Run"
 msgstr ""
@@ -556,12 +599,18 @@ msgstr ""
 msgid "PipelineSchedules|Provide a short description for this pipeline"
 msgstr ""
+msgid "PipelineSchedules|Remove variable row"
+msgstr ""
 msgid "PipelineSchedules|Take ownership"
 msgstr ""
 msgid "PipelineSchedules|Target"
 msgstr ""
+msgid "PipelineSchedules|Variables"
+msgstr ""
 msgid "PipelineSheduleIntervalPattern|Custom"
 msgstr ""
@@ -688,7 +737,7 @@ msgstr ""
 msgid "Select target branch"
 msgstr ""
-msgid "Set a password on your account to pull or push via %{protocol}"
+msgid "Set a password on your account to pull or push via %{protocol}."
 msgstr ""
 msgid "Set up CI"
@@ -714,10 +763,7 @@ msgstr ""
 msgid "StarProject|Star"
 msgstr ""
-msgid "Start a  %{new_merge_request} with these changes"
+msgid "Start a %{new_merge_request} with these changes"
-msgstr ""
-msgid "Start a <strong>new merge request</strong> with these changes"
 msgstr ""
 msgid "Switch branch/tag"
@@ -948,9 +994,15 @@ msgstr ""
 msgid "Upload file"
 msgstr ""
+msgid "UploadLink|click to upload"
+msgstr ""
 msgid "Use your global notification setting"
 msgstr ""
+msgid "View open merge request"
+msgstr ""
 msgid "VisibilityLevel|Internal"
 msgstr ""

--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -8,8 +8,8 @@ msgid ""
 msgstr ""
 "Project-Id-Version: gitlab 1.0.0\n"
 "Report-Msgid-Bugs-To: \n"
-"POT-Creation-Date: 2017-06-19 15:50-0500\n"
+"POT-Creation-Date: 2017-07-05 14:24-0500\n"
-"PO-Revision-Date: 2017-06-19 15:50-0500\n"
+"PO-Revision-Date: 2017-07-05 14:24-0500\n"
 "Last-Translator: FULL NAME <EMAIL@ADDRESS>\n"
 "Language-Team: LANGUAGE <LL@li.org>\n"
 "Language: \n"
@@ -18,7 +18,7 @@ msgstr ""
 "Content-Transfer-Encoding: 8bit\n"
 "Plural-Forms: nplurals=INTEGER; plural=EXPRESSION;\n"
-msgid "%d additional commit have been omitted to prevent performance issues."
+msgid "%d additional commit has been omitted to prevent performance issues."
 msgid_plural "%d additional commits have been omitted to prevent performance issues."
 msgstr[0] ""
 msgstr[1] ""
@@ -224,6 +224,9 @@ msgstr ""
 msgid "Create New Directory"
 msgstr ""
+msgid "Create a personal access token on your account to pull or push via %{protocol}."
+msgstr ""
 msgid "Create directory"
 msgstr ""
@@ -242,6 +245,9 @@ msgstr ""
 msgid "CreateTag|Tag"
 msgstr ""
+msgid "CreateTokenToCloneLink|create a personal access token"
+msgstr ""
 msgid "Cron Timezone"
 msgstr ""
@@ -579,6 +585,12 @@ msgstr ""
 msgid "PipelineSchedules|Inactive"
 msgstr ""
+msgid "PipelineSchedules|Input variable key"
+msgstr ""
+msgid "PipelineSchedules|Input variable value"
+msgstr ""
 msgid "PipelineSchedules|Next Run"
 msgstr ""
@@ -588,12 +600,18 @@ msgstr ""
 msgid "PipelineSchedules|Provide a short description for this pipeline"
 msgstr ""
+msgid "PipelineSchedules|Remove variable row"
+msgstr ""
 msgid "PipelineSchedules|Take ownership"
 msgstr ""
 msgid "PipelineSchedules|Target"
 msgstr ""
+msgid "PipelineSchedules|Variables"
+msgstr ""
 msgid "PipelineSheduleIntervalPattern|Custom"
 msgstr ""
@@ -720,7 +738,7 @@ msgstr ""
 msgid "Select target branch"
 msgstr ""
-msgid "Set a password on your account to pull or push via %{protocol}"
+msgid "Set a password on your account to pull or push via %{protocol}."
 msgstr ""
 msgid "Set up CI"

--- a/spec/controllers/projects/pipeline_schedules_controller_spec.rb
+++ b/spec/controllers/projects/pipeline_schedules_controller_spec.rb
 require 'spec_helper'
 describe Projects::PipelineSchedulesController do
+  include AccessMatchersForController
  set(:project) { create(:empty_project, :public) }
  let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
@@ -17,6 +19,14 @@
      expect(response).to render_template(:index)
    end
+    it 'avoids N + 1 queries' do
+      control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count
+      create_list(:ci_pipeline_schedule, 2, project: project)
+      expect { visit_pipelines_schedules }.not_to exceed_query_limit(control_count)
+    end
    context 'when the scope is set to active' do
      let(:scope) { 'active' }
@@ -36,104 +46,322 @@ def visit_pipelines_schedules
    end
  end
-  describe 'GET edit' do
+  describe 'GET #new' do
-    let(:user) { create(:user) }
+    set(:user) { create(:user) }
    before do
-      project.add_master(user)
+      project.add_developer(user)
      sign_in(user)
    end
-    it 'loads the pipeline schedule' do
+    it 'initializes a pipeline schedule model' do
-      get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      get :new, namespace_id: project.namespace.to_param, project_id: project
      expect(response).to have_http_status(:ok)
-      expect(assigns(:schedule)).to eq(pipeline_schedule)
+      expect(assigns(:schedule)).to be_a_new(Ci::PipelineSchedule)
    end
  end
-  describe 'DELETE #destroy' do
+  describe 'POST #create' do
-    set(:user) { create(:user) }
+    describe 'functionality' do
+      set(:user) { create(:user) }
-    context 'when a developer makes the request' do
      before do
        project.add_developer(user)
        sign_in(user)
+      end
-        delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      let(:basic_param) do
+        attributes_for(:ci_pipeline_schedule)
      end
-      it 'does not delete the pipeline schedule' do
+      context 'when variables_attributes has one variable' do
-        expect(response).not_to have_http_status(:ok)
+        let(:schedule) do
+          basic_param.merge({
+            variables_attributes: [{ key: 'AAA', value: 'AAA123' }]
+          })
+        end
+        it 'creates a new schedule' do
+          expect { go }
+            .to change { Ci::PipelineSchedule.count }.by(1)
+            .and change { Ci::PipelineScheduleVariable.count }.by(1)
+          expect(response).to have_http_status(:found)
+          Ci::PipelineScheduleVariable.last.tap do |v|
+            expect(v.key).to eq("AAA")
+            expect(v.value).to eq("AAA123")
+          end
+        end
+      end
+      context 'when variables_attributes has two variables and duplicted' do
+        let(:schedule) do
+          basic_param.merge({
+            variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
+          })
+        end
+        it 'returns an error that the keys of variable are duplicated' do
+          expect { go }
+            .to change { Ci::PipelineSchedule.count }.by(0)
+            .and change { Ci::PipelineScheduleVariable.count }.by(0)
+          expect(assigns(:schedule).errors['variables']).not_to be_empty
+        end
      end
    end
-    context 'when a master makes the request' do
+    describe 'security' do
+      let(:schedule) { attributes_for(:ci_pipeline_schedule) }
+      it { expect { go }.to be_allowed_for(:admin) }
+      it { expect { go }.to be_allowed_for(:owner).of(project) }
+      it { expect { go }.to be_allowed_for(:master).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_denied_for(:reporter).of(project) }
+      it { expect { go }.to be_denied_for(:guest).of(project) }
+      it { expect { go }.to be_denied_for(:user) }
+      it { expect { go }.to be_denied_for(:external) }
+      it { expect { go }.to be_denied_for(:visitor) }
+    end
+    def go
+      post :create, namespace_id: project.namespace.to_param, project_id: project, schedule: schedule
+    end
+  end
+  describe 'PUT #update' do
+    describe 'functionality' do
+      set(:user) { create(:user) }
+      let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: user) }
      before do
-        project.add_master(user)
+        project.add_developer(user)
        sign_in(user)
      end
-      it 'destroys the pipeline schedule' do
+      context 'when a pipeline schedule has no variables' do
-        expect do
+        let(:basic_param) do
-          delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+          { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: true }
-        end.to change { project.pipeline_schedules.count }.by(-1)
+        end
-        expect(response).to have_http_status(302)
+        context 'when params include one variable' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ key: 'AAA', value: 'AAA123' }]
+            })
+          end
+          it 'inserts new variable to the pipeline schedule' do
+            expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
+            pipeline_schedule.reload
+            expect(response).to have_http_status(:found)
+            expect(pipeline_schedule.variables.last.key).to eq('AAA')
+            expect(pipeline_schedule.variables.last.value).to eq('AAA123')
+          end
+        end
+        context 'when params include two duplicated variables' do
+          let(:schedule) do
+            basic_param.merge({
+              variables_attributes: [{ key: 'AAA', value: 'AAA123' }, { key: 'AAA', value: 'BBB123' }]
+            })
+          end
+          it 'returns an error that variables are duplciated' do
+            go
+            expect(assigns(:schedule).errors['variables']).not_to be_empty
+          end
+        end
+      end
+      context 'when a pipeline schedule has one variable' do
+        let(:basic_param) do
+          { description: 'updated_desc', cron: '0 1 * * *', cron_timezone: 'UTC', ref: 'patch-x', active: true }
+        end
+        let!(:pipeline_schedule_variable) do
+          create(:ci_pipeline_schedule_variable,
+            key: 'CCC', pipeline_schedule: pipeline_schedule)
+        end
+        context 'when params include one variable' do
+          context 'when adds a new variable' do
+            let(:schedule) do
+              basic_param.merge({
+                variables_attributes: [{ key: 'AAA', value: 'AAA123' }]
+              })
+            end
+            it 'adds the new variable' do
+              expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(1)
+              pipeline_schedule.reload
+              expect(pipeline_schedule.variables.last.key).to eq('AAA')
+            end
+          end
+          context 'when updates a variable' do
+            let(:schedule) do
+              basic_param.merge({
+                variables_attributes: [{ id: pipeline_schedule_variable.id, value: 'new_value' }]
+              })
+            end
+            it 'updates the variable' do
+              expect { go }.not_to change { Ci::PipelineScheduleVariable.count }
+              pipeline_schedule_variable.reload
+              expect(pipeline_schedule_variable.value).to eq('new_value')
+            end
+          end
+          context 'when deletes a variable' do
+            let(:schedule) do
+              basic_param.merge({
+                variables_attributes: [{ id: pipeline_schedule_variable.id, _destroy: true }]
+              })
+            end
+            it 'delete the existsed variable' do
+              expect { go }.to change { Ci::PipelineScheduleVariable.count }.by(-1)
+            end
+          end
+        end
      end
    end
-  end
-  describe 'security' do
+    describe 'security' do
-    include AccessMatchersForController
+      let(:schedule) { { description: 'updated_desc' } }
-    describe 'GET edit' do
      it { expect { go }.to be_allowed_for(:admin) }
      it { expect { go }.to be_allowed_for(:owner).of(project) }
      it { expect { go }.to be_allowed_for(:master).of(project) }
-      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
      it { expect { go }.to be_denied_for(:reporter).of(project) }
      it { expect { go }.to be_denied_for(:guest).of(project) }
      it { expect { go }.to be_denied_for(:user) }
      it { expect { go }.to be_denied_for(:external) }
      it { expect { go }.to be_denied_for(:visitor) }
-      def go
+      context 'when a developer created a pipeline schedule' do
+        let(:developer_1) { create(:user) }
+        let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: developer_1) }
+        before do
+          project.add_developer(developer_1)
+        end
+        it { expect { go }.to be_allowed_for(developer_1) }
+        it { expect { go }.to be_denied_for(:developer).of(project) }
+        it { expect { go }.to be_allowed_for(:master).of(project) }
+      end
+      context 'when a master created a pipeline schedule' do
+        let(:master_1) { create(:user) }
+        let!(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project, owner: master_1) }
+        before do
+          project.add_master(master_1)
+        end
+        it { expect { go }.to be_allowed_for(master_1) }
+        it { expect { go }.to be_allowed_for(:master).of(project) }
+        it { expect { go }.to be_denied_for(:developer).of(project) }
+      end
+    end
+    def go
+      put :update, namespace_id: project.namespace.to_param,
+                   project_id: project, id: pipeline_schedule,
+                   schedule: schedule
+    end
+  end
+  describe 'GET #edit' do
+    describe 'functionality' do
+      let(:user) { create(:user) }
+      before do
+        project.add_master(user)
+        sign_in(user)
+      end
+      it 'loads the pipeline schedule' do
        get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+        expect(response).to have_http_status(:ok)
+        expect(assigns(:schedule)).to eq(pipeline_schedule)
      end
    end
-    describe 'GET take_ownership' do
+    describe 'security' do
      it { expect { go }.to be_allowed_for(:admin) }
      it { expect { go }.to be_allowed_for(:owner).of(project) }
      it { expect { go }.to be_allowed_for(:master).of(project) }
-      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
      it { expect { go }.to be_denied_for(:reporter).of(project) }
      it { expect { go }.to be_denied_for(:guest).of(project) }
      it { expect { go }.to be_denied_for(:user) }
      it { expect { go }.to be_denied_for(:external) }
      it { expect { go }.to be_denied_for(:visitor) }
+    end
-      def go
+    def go
-        post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      get :edit, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
-      end
    end
+  end
-    describe 'PUT update' do
+  describe 'GET #take_ownership' do
+    describe 'security' do
      it { expect { go }.to be_allowed_for(:admin) }
      it { expect { go }.to be_allowed_for(:owner).of(project) }
      it { expect { go }.to be_allowed_for(:master).of(project) }
-      it { expect { go }.to be_allowed_for(:developer).of(project) }
+      it { expect { go }.to be_allowed_for(:developer).of(project).own(pipeline_schedule) }
      it { expect { go }.to be_denied_for(:reporter).of(project) }
      it { expect { go }.to be_denied_for(:guest).of(project) }
      it { expect { go }.to be_denied_for(:user) }
      it { expect { go }.to be_denied_for(:external) }
      it { expect { go }.to be_denied_for(:visitor) }
+    end
-      def go
+    def go
-        put :update, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id,
+      post :take_ownership, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
-                     schedule: { description: 'a' }
+    end
+  end
+  describe 'DELETE #destroy' do
+    set(:user) { create(:user) }
+    context 'when a developer makes the request' do
+      before do
+        project.add_developer(user)
+        sign_in(user)
+        delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+      end
+      it 'does not delete the pipeline schedule' do
+        expect(response).to have_http_status(:not_found)
+      end
+    end
+    context 'when a master makes the request' do
+      before do
+        project.add_master(user)
+        sign_in(user)
+      end
+      it 'destroys the pipeline schedule' do
+        expect do
+          delete :destroy, namespace_id: project.namespace.to_param, project_id: project, id: pipeline_schedule.id
+        end.to change { project.pipeline_schedules.count }.by(-1)
+        expect(response).to have_http_status(302)
      end
    end
  end

--- a/spec/factories/ci/pipeline_schedule_variables.rb
+++ b/spec/factories/ci/pipeline_schedule_variables.rb
+FactoryGirl.define do
+  factory :ci_pipeline_schedule_variable, class: Ci::PipelineScheduleVariable do
+    sequence(:key) { |n| "VARIABLE_#{n}" }
+    value 'VARIABLE_VALUE'
+    pipeline_schedule factory: :ci_pipeline_schedule
+  end
+end
--- a/spec/features/projects/pipeline_schedules_spec.rb
+++ b/spec/features/projects/pipeline_schedules_spec.rb
 require 'spec_helper'
-feature 'Pipeline Schedules', :feature do
+feature 'Pipeline Schedules', :feature, js: true do
  include PipelineSchedulesHelper
  let!(:project) { create(:project) }
@@ -11,27 +11,20 @@
  before do
    project.add_master(user)
    gitlab_sign_in(user)
-    visit_page
  end
  describe 'GET /projects/pipeline_schedules' do
-    let(:visit_page) { visit_pipelines_schedules }
+    before do
+      visit_pipelines_schedules
-    it 'avoids N + 1 queries' do
-      control_count = ActiveRecord::QueryRecorder.new { visit_pipelines_schedules }.count
-      create_list(:ci_pipeline_schedule, 2, project: project)
-      expect { visit_pipelines_schedules }.not_to exceed_query_limit(control_count)
    end
    describe 'The view' do
      it 'displays the required information description' do
        page.within('.pipeline-schedule-table-row') do
          expect(page).to have_content('pipeline schedule')
-          expect(page).to have_content(pipeline_schedule.real_next_run.strftime('%b %d, %Y'))
+          expect(find(".next-run-cell time")['data-original-title'])
+            .to include(pipeline_schedule.real_next_run.strftime('%b %-d, %Y'))
          expect(page).to have_link('master')
          expect(page).to have_link("##{pipeline.id}")
        end
@@ -62,7 +55,7 @@
      it 'deletes the pipeline' do
        click_link 'Delete'
-        expect(page).not_to have_content('pipeline schedule')
+        expect(page).not_to have_css(".pipeline-schedule-table-row")
      end
    end
@@ -78,8 +71,10 @@
    end
  end
-  describe 'POST /projects/pipeline_schedules/new', js: true do
+  describe 'POST /projects/pipeline_schedules/new' do
-    let(:visit_page) { visit_new_pipeline_schedule }
+    before do
+      visit_new_pipeline_schedule
+    end
    it 'sets defaults for timezone and target branch' do
      expect(page).to have_button('master')
@@ -100,8 +95,8 @@
    end
  end
-  describe 'PATCH /projects/pipelines_schedules/:id/edit', js: true do
+  describe 'PATCH /projects/pipelines_schedules/:id/edit' do
-    let(:visit_page) do
+    before do
      edit_pipeline_schedule
    end
@@ -134,6 +129,72 @@
    end
  end
+  context 'when user creates a new pipeline schedule with variables' do
+    background do
+      visit_pipelines_schedules
+      click_link 'New schedule'
+      fill_in_schedule_form
+      all('[name="schedule[variables_attributes][][key]"]')[0].set('AAA')
+      all('[name="schedule[variables_attributes][][value]"]')[0].set('AAA123')
+      all('[name="schedule[variables_attributes][][key]"]')[1].set('BBB')
+      all('[name="schedule[variables_attributes][][value]"]')[1].set('BBB123')
+      save_pipeline_schedule
+    end
+    scenario 'user sees the new variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('AAA')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('AAA123')
+        expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-key-input").value).to eq('BBB')
+        expect(find(".pipeline-variable-row:nth-child(2) .pipeline-variable-value-input").value).to eq('BBB123')
+      end
+    end
+  end
+  context 'when user edits a variable of a pipeline schedule' do
+    background do
+      create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule|
+        create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule)
+      end
+      visit_pipelines_schedules
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      all('[name="schedule[variables_attributes][][key]"]')[0].set('foo')
+      all('[name="schedule[variables_attributes][][value]"]')[0].set('bar')
+      click_button 'Save pipeline schedule'
+    end
+    scenario 'user sees the updated variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('foo')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('bar')
+      end
+    end
+  end
+  context 'when user removes a variable of a pipeline schedule' do
+    background do
+      create(:ci_pipeline_schedule, project: project, owner: user).tap do |pipeline_schedule|
+        create(:ci_pipeline_schedule_variable, key: 'AAA', value: 'AAA123', pipeline_schedule: pipeline_schedule)
+      end
+      visit_pipelines_schedules
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      find('.pipeline-variable-list .pipeline-variable-row-remove-button').click
+      click_button 'Save pipeline schedule'
+    end
+    scenario 'user does not see the removed variable in edit window' do
+      find(".content-list .pipeline-schedule-table-row:nth-child(1) .btn-group a[title='Edit']").click
+      page.within('.pipeline-variable-list') do
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-key-input").value).to eq('')
+        expect(find(".pipeline-variable-row:nth-child(1) .pipeline-variable-value-input").value).to eq('')
+      end
+    end
+  end
  def visit_new_pipeline_schedule
    visit new_namespace_project_pipeline_schedule_path(project.namespace, project, pipeline_schedule)
  end

--- a/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js
+++ b/spec/javascripts/pipeline_schedules/setup_pipeline_variable_list_spec.js
+import {
+  setupPipelineVariableList,
+  insertRow,
+  removeRow,
+} from '~/pipeline_schedules/setup_pipeline_variable_list';
+describe('Pipeline Variable List', () => {
+  let $markup;
+  describe('insertRow', () => {
+    it('should insert another row', () => {
+      $markup = $(`<div>
+        <li class="js-row">
+          <input>
+          <textarea></textarea>
+        </li>
+      </div>`);
+      insertRow($markup.find('.js-row'));
+      expect($markup.find('.js-row').length).toBe(2);
+    });
+    it('should clear `data-is-persisted` on cloned row', () => {
+      $markup = $(`<div>
+        <li class="js-row" data-is-persisted="true"></li>
+      </div>`);
+      insertRow($markup.find('.js-row'));
+      const $lastRow = $markup.find('.js-row').last();
+      expect($lastRow.attr('data-is-persisted')).toBe(undefined);
+    });
+    it('should clear inputs on cloned row', () => {
+      $markup = $(`<div>
+        <li class="js-row">
+          <input value="foo">
+          <textarea>bar</textarea>
+        </li>
+      </div>`);
+      insertRow($markup.find('.js-row'));
+      const $lastRow = $markup.find('.js-row').last();
+      expect($lastRow.find('input').val()).toBe('');
+      expect($lastRow.find('textarea').val()).toBe('');
+    });
+  });
+  describe('removeRow', () => {
+    it('should remove dynamic row', () => {
+      $markup = $(`<div>
+        <li class="js-row">
+          <input>
+          <textarea></textarea>
+        </li>
+      </div>`);
+      removeRow($markup.find('.js-row'));
+      expect($markup.find('.js-row').length).toBe(0);
+    });
+    it('should hide and mark to destroy with already persisted rows', () => {
+      $markup = $(`<div>
+        <li class="js-row" data-is-persisted="true">
+          <input class="js-destroy-input">
+        </li>
+      </div>`);
+      const $row = $markup.find('.js-row');
+      removeRow($row);
+      expect($row.find('.js-destroy-input').val()).toBe('1');
+      expect($markup.find('.js-row').length).toBe(1);
+    });
+  });
+  describe('setupPipelineVariableList', () => {
+    beforeEach(() => {
+      $markup = $(`<form>
+        <li class="js-row">
+          <input class="js-user-input" name="schedule[variables_attributes][][key]">
+          <textarea class="js-user-input" name="schedule[variables_attributes][][value]"></textarea>
+          <button class="js-row-remove-button"></button>
+          <button class="js-row-add-button"></button>
+        </li>
+      </form>`);
+      setupPipelineVariableList($markup);
+    });
+    it('should remove the row when clicking the remove button', () => {
+      $markup.find('.js-row-remove-button').trigger('click');
+      expect($markup.find('.js-row').length).toBe(0);
+    });
+    it('should add another row when editing the last rows key input', () => {
+      const $row = $markup.find('.js-row');
+      $row.find('input.js-user-input')
+        .val('foo')
+        .trigger('input');
+      expect($markup.find('.js-row').length).toBe(2);
+    });
+    it('should add another row when editing the last rows value textarea', () => {
+      const $row = $markup.find('.js-row');
+      $row.find('textarea.js-user-input')
+        .val('foo')
+        .trigger('input');
+      expect($markup.find('.js-row').length).toBe(2);
+    });
+    it('should remove empty row after blurring', () => {
+      const $row = $markup.find('.js-row');
+      $row.find('input.js-user-input')
+        .val('foo')
+        .trigger('input');
+      expect($markup.find('.js-row').length).toBe(2);
+      $row.find('input.js-user-input')
+        .val('')
+        .trigger('input')
+        .trigger('blur');
+      expect($markup.find('.js-row').length).toBe(1);
+    });
+    it('should clear out the `name` attribute on the inputs for the last empty row on form submission (avoid BE validation)', () => {
+      const $row = $markup.find('.js-row');
+      expect($row.find('input').attr('name')).toBe('schedule[variables_attributes][][key]');
+      expect($row.find('textarea').attr('name')).toBe('schedule[variables_attributes][][value]');
+      $markup.filter('form').submit();
+      expect($row.find('input').attr('name')).toBe('');
+      expect($row.find('textarea').attr('name')).toBe('');
+    });
+  });
+});
--- a/spec/lib/gitlab/import_export/all_models.yml
+++ b/spec/lib/gitlab/import_export/all_models.yml
@@ -138,8 +138,11 @@ pipeline_schedules:
 - owner
 - pipelines
 - last_pipeline
+- variables
 pipeline_schedule:
 - pipelines
+pipeline_schedule_variables:
+- pipeline_schedule
 deploy_keys:
 - user
 - deploy_keys_projects

--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -1414,6 +1414,23 @@ def create_mr(build, pipeline, factory: :merge_request, created_at: Time.now)
      it { is_expected.to include(predefined_trigger_variable) }
    end
+    context 'when a job was triggered by a pipeline schedule' do
+      let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
+      let!(:pipeline_schedule_variable) do
+        create(:ci_pipeline_schedule_variable,
+          key: 'SCHEDULE_VARIABLE_KEY',
+          pipeline_schedule: pipeline_schedule)
+      end
+      before do
+        pipeline_schedule.pipelines << pipeline
+        pipeline_schedule.reload
+      end
+      it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) }
+    end
    context 'when yaml_variables are undefined' do
      before do
        build.yaml_variables = nil

--- a/spec/models/ci/pipeline_schedule_spec.rb
+++ b/spec/models/ci/pipeline_schedule_spec.rb
@@ -5,6 +5,7 @@
  it { is_expected.to belong_to(:owner) }
  it { is_expected.to have_many(:pipelines) }
+  it { is_expected.to have_many(:variables) }
  it { is_expected.to respond_to(:ref) }
  it { is_expected.to respond_to(:cron) }
@@ -117,4 +118,20 @@
      end
    end
  end
+  describe '#job_variables' do
+    let!(:pipeline_schedule) { create(:ci_pipeline_schedule) }
+    let!(:pipeline_schedule_variables) do
+      create_list(:ci_pipeline_schedule_variable, 2, pipeline_schedule: pipeline_schedule)
+    end
+    subject { pipeline_schedule.job_variables }
+    before do
+      pipeline_schedule.reload
+    end
+    it { is_expected.to eq(pipeline_schedule_variables.map(&:to_runner_variable)) }
+  end
 end
--- a/spec/models/ci/pipeline_schedule_variable_spec.rb
+++ b/spec/models/ci/pipeline_schedule_variable_spec.rb
+require 'spec_helper'
+describe Ci::PipelineScheduleVariable, models: true do
+  subject { build(:ci_pipeline_schedule_variable) }
+  it { is_expected.to include_module(HasVariable) }
+  it { is_expected.to validate_uniqueness_of(:key).scoped_to(:pipeline_schedule_id) }
+end
--- a/spec/support/matchers/access_matchers_for_controller.rb
+++ b/spec/support/matchers/access_matchers_for_controller.rb
@@ -50,9 +50,24 @@ def description_for(role, type, expected, result)
    "be #{type} for #{role}. Expected: #{expected.join(',')} Got: #{result}"
  end
+  def update_owner(objects, user)
+    return unless objects
+    objects.each do |object|
+      if object.respond_to?(:owner)
+        object.update_attribute(:owner, user)
+      elsif object.respond_to?(:user)
+        object.update_attribute(:user, user)
+      else
+        raise ArgumentError, "cannot own this object #{object}"
+      end
+    end
+  end
  matcher :be_allowed_for do |role|
    match do |action|
-      emulate_user(role, @membership)
+      user = emulate_user(role, @membership)
+      update_owner(@objects, user)
      action.call
      EXPECTED_STATUS_CODE_ALLOWED.include?(response.status)
@@ -62,13 +77,18 @@ def description_for(role, type, expected, result)
      @membership = membership
    end
+    chain :own do |*objects|
+      @objects = objects
+    end
    description { description_for(role, 'allowed', EXPECTED_STATUS_CODE_ALLOWED, response.status) }
    supports_block_expectations
  end
  matcher :be_denied_for do |role|
    match do |action|
-      emulate_user(role, @membership)
+      user = emulate_user(role, @membership)
+      update_owner(@objects, user)
      action.call
      EXPECTED_STATUS_CODE_DENIED.include?(response.status)
@@ -78,6 +98,10 @@ def description_for(role, type, expected, result)
      @membership = membership
    end
+    chain :own do |*objects|
+      @objects = objects
+    end
    description { description_for(role, 'denied', EXPECTED_STATUS_CODE_DENIED, response.status) }
    supports_block_expectations
  end