Cherrypick from !13792fix/sm/37559-for-9-5-new

4 files changed, 78 insertions, 15 deletions

diff --git a/app/presenters/ci/build_presenter.rb b/app/presenters/ci/build_presenter.rb
index c495c3f39bb..255475e1fe6 100644
--- a/app/presenters/ci/build_presenter.rb
+++ b/app/presenters/ci/build_presenter.rb
@@ -17,5 +17,16 @@ module Ci
         "Job is redundant and is auto-canceled by Pipeline ##{auto_canceled_by_id}"
       end
     end
+
+    def trigger_variables
+      return [] unless trigger_request
+
+      @trigger_variables ||=
+        if pipeline.variables.any?
+          pipeline.variables.map(&:to_runner_variable)
+        else
+          trigger_request.user_variables
+        end
+    end
   end
 end
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index f5d5bc7eda9..43e23bb2200 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -46,14 +46,14 @@
             %span.build-light-text Token:
             #{@build.trigger_request.trigger.short_token}
 
-          - if @build.trigger_request.variables
+          - if @build.trigger_variables.any?
             %p
               %button.btn.group.btn-group-justified.reveal-variables Reveal Variables
 
             %dl.js-build-variables.trigger-build-variables.hide
-              - @build.trigger_request.variables.each do |key, value|
-                %dt.js-build-variable.trigger-build-variable= key
-                %dd.js-build-value.trigger-build-value= value
+              - @build.trigger_variables.each do |trigger_variable|
+                %dt.js-build-variable.trigger-build-variable= trigger_variable[:key]
+                %dd.js-build-value.trigger-build-value= trigger_variable[:value]
 
       %div{ class: (@build.pipeline.stages_count > 1 ? "block" : "block-last") }
         %p
diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb
index 037ac00d39f..3b5c6966287 100644
--- a/spec/features/projects/jobs_spec.rb
+++ b/spec/features/projects/jobs_spec.rb
@@ -292,26 +292,44 @@ feature 'Jobs' do
     end
 
     feature 'Variables' do
-      let(:trigger_request) { create(:ci_trigger_request_with_variables) }
+      let(:trigger_request) { create(:ci_trigger_request) }
 
       let(:job) do
         create :ci_build, pipeline: pipeline, trigger_request: trigger_request
       end
 
-      before do
-        visit project_job_path(project, job)
+      shared_examples 'expected variables behavior' do
+        it 'shows variable key and value after click', js: true do
+          expect(page).to have_css('.reveal-variables')
+          expect(page).not_to have_css('.js-build-variable')
+          expect(page).not_to have_css('.js-build-value')
+
+          click_button 'Reveal Variables'
+
+          expect(page).not_to have_css('.reveal-variables')
+          expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
+          expect(page).to have_selector('.js-build-value', text: 'TRIGGER_VALUE_1')
+        end
       end
 
-      it 'shows variable key and value after click', js: true do
-        expect(page).to have_css('.reveal-variables')
-        expect(page).not_to have_css('.js-build-variable')
-        expect(page).not_to have_css('.js-build-value')
+      context 'when variables are stored in trigger_request' do
+        before do
+          trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } )
 
-        click_button 'Reveal Variables'
+          visit project_job_path(project, job)
+        end
+
+        it_behaves_like 'expected variables behavior'
+      end
+
+      context 'when variables are stored in pipeline_variables' do
+        before do
+          create(:ci_pipeline_variable, pipeline: pipeline, key: 'TRIGGER_KEY_1', value: 'TRIGGER_VALUE_1')
+
+          visit project_job_path(project, job)
+        end
 
-        expect(page).not_to have_css('.reveal-variables')
-        expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
-        expect(page).to have_selector('.js-build-value', text: 'TRIGGER_VALUE_1')
+        it_behaves_like 'expected variables behavior'
       end
     end
 
diff --git a/spec/presenters/ci/build_presenter_spec.rb b/spec/presenters/ci/build_presenter_spec.rb
index a7a34ecac72..1a8001be6ab 100644
--- a/spec/presenters/ci/build_presenter_spec.rb
+++ b/spec/presenters/ci/build_presenter_spec.rb
@@ -100,4 +100,38 @@ describe Ci::BuildPresenter do
       end
     end
   end
+
+  describe '#trigger_variables' do
+    let(:build) { create(:ci_build, pipeline: pipeline, trigger_request: trigger_request) }
+    let(:trigger) { create(:ci_trigger, project: project) }
+    let(:trigger_request) { create(:ci_trigger_request, pipeline: pipeline, trigger: trigger) }
+
+    context 'when variable is stored in ci_pipeline_variables' do
+      let!(:pipeline_variable) { create(:ci_pipeline_variable, pipeline: pipeline) }
+
+      context 'when pipeline is triggered by trigger API' do
+        it 'returns variables' do
+          expect(presenter.trigger_variables).to eq([pipeline_variable.to_runner_variable])
+        end
+      end
+
+      context 'when pipeline is not triggered by trigger API' do
+        let(:build) { create(:ci_build, pipeline: pipeline) }
+
+        it 'does not return variables' do
+          expect(presenter.trigger_variables).to eq([])
+        end
+      end
+    end
+
+    context 'when variable is stored in ci_trigger_requests.variables' do
+      before do
+        trigger_request.update_attribute(:variables, { 'TRIGGER_KEY_1' => 'TRIGGER_VALUE_1' } )
+      end
+
+      it 'returns variables' do
+        expect(presenter.trigger_variables).to eq(trigger_request.user_variables)
+      end
+    end
+  end
 end