From 06f01073696199da980a2101bc3eb77922131a8a Mon Sep 17 00:00:00 2001
From: Shinya Maeda <shinya@gitlab.com>
Date: Wed, 21 Jun 2017 18:25:01 +0900
Subject: [PATCH] pipeline_schedule_variables model/db

---
 app/models/ci/build.rb                        | 14 ++++----
 app/models/ci/pipeline_schedule_variable.rb   |  1 -
 ...8_create_ci_pipeline_schedule_variables.rb |  2 +-
 spec/models/ci/build_spec.rb                  | 35 ++++++++-----------
 .../ci/pipeline_schedule_variable_spec.rb     |  2 +-
 5 files changed, 24 insertions(+), 30 deletions(-)

diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb
index 6c5b27a7ac5..6878f055667 100644
--- a/app/models/ci/build.rb
+++ b/app/models/ci/build.rb
@@ -176,12 +176,9 @@ module Ci
     #   * Lowercased
     #   * Anything not matching [a-z0-9-] is replaced with a -
     #   * Maximum length is 63 bytes
-    #   * First/Last Character is not a hyphen
     def ref_slug
-      ref.to_s
-          .downcase
-          .gsub(/[^a-z0-9]/, '-')[0..62]
-          .gsub(/(\A-+|-+\z)/, '')
+      slugified = ref.to_s.downcase
+      slugified.gsub(/[^a-z0-9]/, '-')[0..62]
     end
 
     # Variables whose value does not depend on environment
@@ -195,8 +192,11 @@ module Ci
       variables += yaml_variables
       variables += user_variables
       variables += project.secret_variables_for(ref).map(&:to_runner_variable)
-      variables += trigger_request.user_variables if trigger_request
-      variables += pipeline.pipeline_schedule.job_variables if pipeline.pipeline_schedule
+      if trigger_request
+        variables += trigger_request.user_variables
+      elsif pipeline.pipeline_schedule
+        variables += pipeline.pipeline_schedule.variables.map(&:to_runner_variable)
+      end
       variables
     end
 
diff --git a/app/models/ci/pipeline_schedule_variable.rb b/app/models/ci/pipeline_schedule_variable.rb
index ee5b8733fac..2d681446d00 100644
--- a/app/models/ci/pipeline_schedule_variable.rb
+++ b/app/models/ci/pipeline_schedule_variable.rb
@@ -4,7 +4,6 @@ module Ci
     include HasVariable
 
     belongs_to :pipeline_schedule
-
     validates :key, uniqueness: { scope: :pipeline_schedule_id }
   end
 end
diff --git a/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb b/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb
index 92833765a82..7d2d313a91a 100644
--- a/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb
+++ b/db/migrate/20170620064728_create_ci_pipeline_schedule_variables.rb
@@ -10,7 +10,7 @@ class CreateCiPipelineScheduleVariables < ActiveRecord::Migration
       t.string :encrypted_value_iv
       t.integer :pipeline_schedule_id, null: false
 
-      t.timestamps_with_timezone null: true
+      t.timestamps null: false
     end
 
     add_index :ci_pipeline_schedule_variables,
diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb
index 211356f415a..c6a7791d64b 100644
--- a/spec/models/ci/build_spec.rb
+++ b/spec/models/ci/build_spec.rb
@@ -998,17 +998,13 @@ describe Ci::Build, :models do
 
   describe '#ref_slug' do
     {
-      'master'                => 'master',
-      '1-foo'                 => '1-foo',
-      'fix/1-foo'             => 'fix-1-foo',
-      'fix-1-foo'             => 'fix-1-foo',
-      'a' * 63                => 'a' * 63,
-      'a' * 64                => 'a' * 63,
-      'FOO'                   => 'foo',
-      '-' + 'a' * 61 + '-'    => 'a' * 61,
-      '-' + 'a' * 62 + '-'    => 'a' * 62,
-      '-' + 'a' * 63 + '-'    => 'a' * 62,
-      'a' * 62 + ' '          => 'a' * 62
+      'master'    => 'master',
+      '1-foo'     => '1-foo',
+      'fix/1-foo' => 'fix-1-foo',
+      'fix-1-foo' => 'fix-1-foo',
+      'a' * 63    => 'a' * 63,
+      'a' * 64    => 'a' * 63,
+      'FOO'       => 'foo'
     }.each do |ref, slug|
       it "transforms #{ref} to #{slug}" do
         build.ref = ref
@@ -1373,21 +1369,20 @@ describe Ci::Build, :models do
       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)
+    context 'when build was triggered by scheduled pipeline' do
+      let(:secret_variable) do
+        { key: 'SECRET_KEY', value: 'secret_value', public: false }
       end
 
+      let(:pipeline_schedule) { create(:ci_pipeline_schedule, project: project) }
+
       before do
         pipeline_schedule.pipelines << pipeline
-        pipeline_schedule.reload
+        create(:ci_pipeline_schedule_variable,
+               secret_variable.slice(:key, :value).merge(pipeline_schedule: pipeline_schedule))
       end
 
-      it { is_expected.to include(pipeline_schedule_variable.to_runner_variable) }
+      it { is_expected.to include(secret_variable) }
     end
 
     context 'when yaml_variables are undefined' do
diff --git a/spec/models/ci/pipeline_schedule_variable_spec.rb b/spec/models/ci/pipeline_schedule_variable_spec.rb
index 9c0b0153e03..eb67792bf2f 100644
--- a/spec/models/ci/pipeline_schedule_variable_spec.rb
+++ b/spec/models/ci/pipeline_schedule_variable_spec.rb
@@ -3,6 +3,6 @@ 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 be_kind_of(HasVariable) }
   it { is_expected.to validate_uniqueness_of(:key).scoped_to(:pipeline_schedule_id) }
 end