From 32ccde8ccdafa5920c5677c6caa7b22337e7cb44 Mon Sep 17 00:00:00 2001
From: Filipa Lacerda <filipa@gitlab.com>
Date: Tue, 25 Sep 2018 08:07:47 +0000
Subject: [PATCH] Use Vue component for job artifacts, Commit and Trigger
 Variables

---
 app/assets/javascripts/build_variables.js     |  10 -
 .../jobs/components/artifacts_block.vue       |  68 +++---
 .../jobs/components/commit_block.vue          |  72 +++----
 .../jobs/components/sidebar_details_block.vue | 195 +++++++++++-------
 .../jobs/components/trigger_block.vue         |  28 +--
 .../javascripts/jobs/job_details_mediator.js  |   2 -
 app/views/projects/jobs/_sidebar.html.haml    |  57 -----
 .../unreleased/50904-move-job-page-vue.yml    |   5 +
 .../testing/request_inspector_middleware.rb   |   6 +-
 locale/gitlab.pot                             |   2 +-
 .../user_downloads_artifacts_spec.rb          |  14 +-
 spec/features/projects/jobs_spec.rb           |  60 ++++--
 .../jobs/components/artifacts_block_spec.js   |  66 +++---
 .../jobs/components/commit_block_spec.js      |  41 ++--
 ...er_value_spec.js => trigger_block_spec.js} |  25 ++-
 .../projects/jobs/show.html.haml_spec.rb      |  36 ----
 16 files changed, 318 insertions(+), 369 deletions(-)
 delete mode 100644 app/assets/javascripts/build_variables.js
 create mode 100644 changelogs/unreleased/50904-move-job-page-vue.yml
 rename spec/javascripts/jobs/components/{trigger_value_spec.js => trigger_block_spec.js} (75%)

diff --git a/app/assets/javascripts/build_variables.js b/app/assets/javascripts/build_variables.js
deleted file mode 100644
index d398e4a4c83..00000000000
--- a/app/assets/javascripts/build_variables.js
+++ /dev/null
@@ -1,10 +0,0 @@
-import $ from 'jquery';
-
-export default function handleRevealVariables() {
-  $('.js-reveal-variables')
-    .off('click')
-    .on('click', function click() {
-      $('.js-build-variables').toggle();
-      $(this).hide();
-    });
-}
diff --git a/app/assets/javascripts/jobs/components/artifacts_block.vue b/app/assets/javascripts/jobs/components/artifacts_block.vue
index 525c5eec91a..d5866f9b9f1 100644
--- a/app/assets/javascripts/jobs/components/artifacts_block.vue
+++ b/app/assets/javascripts/jobs/components/artifacts_block.vue
@@ -1,40 +1,27 @@
 <script>
-  import TimeagoTooltiop from '~/vue_shared/components/time_ago_tooltip.vue';
+  import TimeagoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
+  import timeagoMixin from '~/vue_shared/mixins/timeago';
 
   export default {
     components: {
-      TimeagoTooltiop,
+      TimeagoTooltip,
     },
+    mixins: [
+      timeagoMixin,
+    ],
     props: {
-      // @build.artifacts_expired?
-      haveArtifactsExpired: {
-        type: Boolean,
+      artifact: {
+        type: Object,
         required: true,
       },
-      // @build.has_expiring_artifacts?
-      willArtifactsExpire: {
-        type: Boolean,
-        required: true,
+    },
+    computed: {
+      isExpired() {
+        return this.artifact.expired;
       },
-      expireAt: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      keepArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      downloadArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
-      },
-      browseArtifactsPath: {
-        type: String,
-        required: false,
-        default: null,
+      // Only when the key is `false` we can render this block
+      willExpire() {
+        return this.artifact.expired === false;
       },
     },
   };
@@ -46,21 +33,22 @@
     </div>
 
     <p
-      v-if="haveArtifactsExpired"
+      v-if="isExpired"
       class="js-artifacts-removed build-detail-row"
     >
       {{ s__('Job|The artifacts were removed') }}
     </p>
+
     <p
-      v-else-if="willArtifactsExpire"
+      v-else-if="willExpire"
       class="js-artifacts-will-be-removed build-detail-row"
     >
-      {{ s__('Job|The artifacts will be removed') }}
+      {{ s__('Job|The artifacts will be removed in') }}
     </p>
 
-    <timeago-tooltiop
-      v-if="expireAt"
-      :time="expireAt"
+    <timeago-tooltip
+      v-if="artifact.expire_at"
+      :time="artifact.expire_at"
     />
 
     <div
@@ -68,8 +56,8 @@
       role="group"
     >
       <a
-        v-if="keepArtifactsPath"
-        :href="keepArtifactsPath"
+        v-if="artifact.keep_path"
+        :href="artifact.keep_path"
         class="js-keep-artifacts btn btn-sm btn-default"
         data-method="post"
       >
@@ -77,8 +65,8 @@
       </a>
 
       <a
-        v-if="downloadArtifactsPath"
-        :href="downloadArtifactsPath"
+        v-if="artifact.download_path"
+        :href="artifact.download_path"
         class="js-download-artifacts btn btn-sm btn-default"
         download
         rel="nofollow"
@@ -87,8 +75,8 @@
       </a>
 
       <a
-        v-if="browseArtifactsPath"
-        :href="browseArtifactsPath"
+        v-if="artifact.browse_path"
+        :href="artifact.browse_path"
         class="js-browse-artifacts btn btn-sm btn-default"
       >
         {{ s__('Job|Browse') }}
diff --git a/app/assets/javascripts/jobs/components/commit_block.vue b/app/assets/javascripts/jobs/components/commit_block.vue
index 7f485295513..39a4ff159e2 100644
--- a/app/assets/javascripts/jobs/components/commit_block.vue
+++ b/app/assets/javascripts/jobs/components/commit_block.vue
@@ -1,64 +1,56 @@
 <script>
-import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
+  import ClipboardButton from '~/vue_shared/components/clipboard_button.vue';
 
-export default {
-  components: {
-    ClipboardButton,
-  },
-  props: {
-    pipelineShortSha: {
-      type: String,
-      required: true,
+  export default {
+    components: {
+      ClipboardButton,
     },
-    pipelineShaPath: {
-      type: String,
-      required: true,
+    props: {
+      commit: {
+        type: Object,
+        required: true,
+      },
+      mergeRequest: {
+        type: Object,
+        required: false,
+        default: null,
+      },
+      isLastBlock: {
+        type: Boolean,
+        required: true,
+      },
     },
-    mergeRequestReference: {
-      type: String,
-      required: false,
-      default: null,
-    },
-    mergeRequestPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
-    gitCommitTitlte: {
-      type: String,
-      required: true,
-    },
-  },
-};
+  };
 </script>
 <template>
-  <div class="block">
+  <div
+    :class="{
+      'block-last': isLastBlock,
+      block: !isLastBlock
+  }">
     <p>
       {{ __('Commit') }}
 
       <a
-        :href="pipelineShaPath"
+        :href="commit.commit_path"
         class="js-commit-sha commit-sha link-commit"
-      >
-        {{ pipelineShortSha }}
-      </a>
+      >{{ commit.short_id }}</a>
 
       <clipboard-button
-        :text="pipelineShortSha"
+        :text="commit.short_id"
         :title="__('Copy commit SHA to clipboard')"
+        css-class="btn btn-clipboard btn-transparent"
       />
 
       <a
-        v-if="mergeRequestPath && mergeRequestReference"
-        :href="mergeRequestPath"
+        v-if="mergeRequest"
+        :href="mergeRequest.path"
         class="js-link-commit link-commit"
-      >
-        {{ mergeRequestReference }}
-      </a>
+      >{{ mergeRequest.iid }}</a>
     </p>
 
     <p class="build-light-text append-bottom-0">
-      {{ gitCommitTitlte }}
+      {{ commit.title }}
     </p>
   </div>
 </template>
diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index 80c2a5fb48b..a591fcfb482 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -1,89 +1,113 @@
 <script>
-import timeagoMixin from '~/vue_shared/mixins/timeago';
-import { timeIntervalInWords } from '~/lib/utils/datetime_utility';
-import Icon from '~/vue_shared/components/icon.vue';
-import DetailRow from './sidebar_detail_row.vue';
+  import _ from 'underscore';
+  import timeagoMixin from '~/vue_shared/mixins/timeago';
+  import { timeIntervalInWords } from '~/lib/utils/datetime_utility';
+  import Icon from '~/vue_shared/components/icon.vue';
+  import DetailRow from './sidebar_detail_row.vue';
+  import ArtifactsBlock from './artifacts_block.vue';
+  import TriggerBlock from './trigger_block.vue';
+  import CommitBlock from './commit_block.vue';
 
-export default {
-  name: 'SidebarDetailsBlock',
-  components: {
-    DetailRow,
-    Icon,
-  },
-  mixins: [timeagoMixin],
-  props: {
-    job: {
-      type: Object,
-      required: true,
+  export default {
+    name: 'SidebarDetailsBlock',
+    components: {
+      ArtifactsBlock,
+      CommitBlock,
+      DetailRow,
+      Icon,
+      TriggerBlock,
     },
-    isLoading: {
-      type: Boolean,
-      required: true,
+    mixins: [timeagoMixin],
+    props: {
+      job: {
+        type: Object,
+        required: true,
+      },
+      isLoading: {
+        type: Boolean,
+        required: true,
+      },
+      runnerHelpUrl: {
+        type: String,
+        required: false,
+        default: '',
+      },
+      terminalPath: {
+        type: String,
+        required: false,
+        default: null,
+      },
     },
-    runnerHelpUrl: {
-      type: String,
-      required: false,
-      default: '',
-    },
-    terminalPath: {
-      type: String,
-      required: false,
-      default: null,
-    },
-  },
-  computed: {
-    shouldRenderContent() {
-      return !this.isLoading && Object.keys(this.job).length > 0;
-    },
-    coverage() {
-      return `${this.job.coverage}%`;
-    },
-    duration() {
-      return timeIntervalInWords(this.job.duration);
-    },
-    queued() {
-      return timeIntervalInWords(this.job.queued);
-    },
-    runnerId() {
-      return `${this.job.runner.description} (#${this.job.runner.id})`;
-    },
-    retryButtonClass() {
-      let className =
-        'js-retry-button float-right btn btn-retry d-none d-md-block d-lg-block d-xl-block';
-      className +=
-        this.job.status && this.job.recoverable ? ' btn-primary' : ' btn-inverted-secondary';
-      return className;
-    },
-    hasTimeout() {
-      return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
-    },
-    timeout() {
-      if (this.job.metadata == null) {
-        return '';
-      }
+    computed: {
+      shouldRenderContent() {
+        return !this.isLoading && Object.keys(this.job).length > 0;
+      },
+      coverage() {
+        return `${this.job.coverage}%`;
+      },
+      duration() {
+        return timeIntervalInWords(this.job.duration);
+      },
+      queued() {
+        return timeIntervalInWords(this.job.queued);
+      },
+      runnerId() {
+        return `${this.job.runner.description} (#${this.job.runner.id})`;
+      },
+      retryButtonClass() {
+        let className =
+          'js-retry-button float-right btn btn-retry d-none d-md-block d-lg-block d-xl-block';
+        className +=
+          this.job.status && this.job.recoverable ? ' btn-primary' : ' btn-inverted-secondary';
+        return className;
+      },
+      hasTimeout() {
+        return this.job.metadata != null && this.job.metadata.timeout_human_readable !== null;
+      },
+      timeout() {
+        if (this.job.metadata == null) {
+          return '';
+        }
 
-      let t = this.job.metadata.timeout_human_readable;
-      if (this.job.metadata.timeout_source !== '') {
-        t += ` (from ${this.job.metadata.timeout_source})`;
-      }
+        let t = this.job.metadata.timeout_human_readable;
+        if (this.job.metadata.timeout_source !== '') {
+          t += ` (from ${this.job.metadata.timeout_source})`;
+        }
 
-      return t;
+        return t;
+      },
+      renderBlock() {
+        return (
+          this.job.merge_request ||
+          this.job.duration ||
+          this.job.finished_data ||
+          this.job.erased_at ||
+          this.job.queued ||
+          this.job.runner ||
+          this.job.coverage ||
+          this.job.tags.length ||
+          this.job.cancel_path
+        );
+      },
+      hasArtifact() {
+        return !_.isEmpty(this.job.artifact);
+      },
+      hasTriggers() {
+        return !_.isEmpty(this.job.trigger);
+      },
+      hasStages() {
+        return (
+          this.job &&
+          this.job.pipeline &&
+          this.job.pipeline.stages &&
+          this.job.pipeline.stages.length > 0
+        ) || false;
+      },
+      commit() {
+        return this.job.pipeline.commit || {};
+      },
     },
-    renderBlock() {
-      return (
-        this.job.merge_request ||
-        this.job.duration ||
-        this.job.finished_data ||
-        this.job.erased_at ||
-        this.job.queued ||
-        this.job.runner ||
-        this.job.coverage ||
-        this.job.tags.length ||
-        this.job.cancel_path
-      );
-    },
-  },
-};
+  };
 </script>
 <template>
   <div>
@@ -229,6 +253,19 @@ export default {
           </a>
         </div>
       </div>
+      <artifacts-block
+        v-if="hasArtifact"
+        :artifact="job.artifact"
+      />
+      <trigger-block
+        v-if="hasTriggers"
+        :trigger="job.trigger"
+      />
+      <commit-block
+        :is-last-block="hasStages"
+        :commit="commit"
+        :merge-request="job.merge_request"
+      />
     </template>
     <gl-loading-icon
       v-if="isLoading"
diff --git a/app/assets/javascripts/jobs/components/trigger_block.vue b/app/assets/javascripts/jobs/components/trigger_block.vue
index 8a88e5da6aa..d7b3c4fcb5b 100644
--- a/app/assets/javascripts/jobs/components/trigger_block.vue
+++ b/app/assets/javascripts/jobs/components/trigger_block.vue
@@ -1,16 +1,9 @@
 <script>
   export default {
     props: {
-      shortToken: {
-        type: String,
-        required: false,
-        default: null,
-      },
-
-      variables: {
+      trigger: {
         type: Object,
-        required: false,
-        default: () => ({}),
+        required: true,
       },
     },
     data() {
@@ -20,7 +13,7 @@
     },
     computed: {
       hasVariables() {
-        return Object.keys(this.variables).length > 0;
+        return this.trigger.variables && this.trigger.variables.length > 0;
       },
     },
     methods: {
@@ -38,17 +31,18 @@
     </h4>
 
     <p
-      v-if="shortToken"
+      v-if="trigger.short_token"
       class="js-short-token"
     >
       <span class="build-light-text">
         {{ __('Token') }}
       </span>
-      {{ shortToken }}
+      {{ trigger.short_token }}
     </p>
 
     <p v-if="hasVariables">
       <button
+        v-if="!areVariablesVisible"
         type="button"
         class="btn btn-default group js-reveal-variables"
         @click="revealVariables"
@@ -63,20 +57,20 @@
       class="js-build-variables trigger-build-variables"
     >
       <template
-        v-for="(value, key) in variables"
+        v-for="variable in trigger.variables"
       >
         <dt
-          :key="`${key}-variable`"
+          :key="`${variable.key}-variable`"
           class="js-build-variable trigger-build-variable"
         >
-          {{ key }}
+          {{ variable.key }}
         </dt>
 
         <dd
-          :key="`${key}-value`"
+          :key="`${variable.key}-value`"
           class="js-build-value trigger-build-value"
         >
-          {{ value }}
+          {{ variable.value }}
         </dd>
       </template>
     </dl>
diff --git a/app/assets/javascripts/jobs/job_details_mediator.js b/app/assets/javascripts/jobs/job_details_mediator.js
index 89019da9d1e..073e518baa0 100644
--- a/app/assets/javascripts/jobs/job_details_mediator.js
+++ b/app/assets/javascripts/jobs/job_details_mediator.js
@@ -4,7 +4,6 @@ import Poll from '../lib/utils/poll';
 import JobStore from './stores/job_store';
 import JobService from './services/job_service';
 import Job from '../job';
-import handleRevealVariables from '../build_variables';
 
 export default class JobMediator {
   constructor(options = {}) {
@@ -20,7 +19,6 @@ export default class JobMediator {
 
   initBuildClass() {
     this.build = new Job();
-    handleRevealVariables();
   }
 
   fetchJob() {
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index acc1e17b811..66a3b8b8fd1 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -3,63 +3,6 @@
     .blocks-container
       #js-details-block-vue{ data: { terminal_path: can?(current_user, :create_build_terminal, @build) && @build.has_terminal? ? terminal_project_job_path(@project, @build) : nil } }
 
-      - if can?(current_user, :read_build, @project) && (@build.artifacts? || @build.artifacts_expired?)
-        .block
-          .title
-            Job artifacts
-          - if @build.artifacts_expired?
-            %p.build-detail-row
-              The artifacts were removed
-              #{time_ago_with_tooltip(@build.artifacts_expire_at)}
-          - elsif @build.has_expiring_artifacts?
-            %p.build-detail-row
-              The artifacts will be removed
-              #{time_ago_with_tooltip(@build.artifacts_expire_at)}
-
-          - if @build.artifacts?
-            .btn-group.d-flex{ role: :group }
-              - if @build.has_expiring_artifacts? && can?(current_user, :update_build, @build)
-                = link_to keep_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default', method: :post do
-                  Keep
-
-              = link_to download_project_job_artifacts_path(@project, @build), rel: 'nofollow', download: '', class: 'btn btn-sm btn-default' do
-                Download
-
-              - if @build.browsable_artifacts?
-                = link_to browse_project_job_artifacts_path(@project, @build), class: 'btn btn-sm btn-default' do
-                  Browse
-
-      - if @build.trigger_request
-        .build-widget.block
-          %h4.title
-            Trigger
-
-          - if @build.trigger_request&.trigger&.short_token
-            %p
-              %span.build-light-text Token:
-              #{@build.trigger_request.trigger.short_token}
-
-          - if @build.trigger_variables.any?
-            %p
-              %button.btn.group.js-reveal-variables Reveal Variables
-
-            %dl.js-build-variables.trigger-build-variables.hide
-              - @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
-          Commit
-          = link_to @build.pipeline.short_sha, project_commit_path(@project, @build.pipeline.sha), class: 'commit-sha link-commit'
-          = clipboard_button(text: @build.pipeline.short_sha, title: "Copy commit SHA to clipboard")
-          - if @build.merge_request
-            in
-            = link_to "#{@build.merge_request.to_reference}", merge_request_path(@build.merge_request), class: 'link-commit'
-
-        %p.build-light-text.append-bottom-0
-          #{@build.pipeline.git_commit_title}
-
       - if @build.pipeline.stages_count > 1
         .block-last.dropdown.build-dropdown
           %div
diff --git a/changelogs/unreleased/50904-move-job-page-vue.yml b/changelogs/unreleased/50904-move-job-page-vue.yml
new file mode 100644
index 00000000000..e907c6301ec
--- /dev/null
+++ b/changelogs/unreleased/50904-move-job-page-vue.yml
@@ -0,0 +1,5 @@
+---
+title: Use Vue components and new API to render Artifacts, Trigger Variables and Commit blocks on Job page
+merge_request: 21777
+author:
+type: other
diff --git a/lib/gitlab/testing/request_inspector_middleware.rb b/lib/gitlab/testing/request_inspector_middleware.rb
index e387667480d..c251e78f5c5 100644
--- a/lib/gitlab/testing/request_inspector_middleware.rb
+++ b/lib/gitlab/testing/request_inspector_middleware.rb
@@ -35,11 +35,15 @@ module Gitlab
         request_headers = env_http_headers(env)
         status, headers, body = @app.call(env)
 
+        full_body = ''
+        body.each { |b| full_body << b }
+
         request = OpenStruct.new(
           url: url,
           status_code: status,
           request_headers: request_headers,
-          response_headers: headers
+          response_headers: headers,
+          body: full_body
         )
         log_request request
 
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 5663adb0eb3..771d1612676 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -3365,7 +3365,7 @@ msgstr ""
 msgid "Job|The artifacts were removed"
 msgstr ""
 
-msgid "Job|The artifacts will be removed"
+msgid "Job|The artifacts will be removed in"
 msgstr ""
 
 msgid "Job|This job is stuck, because the project doesn't have any runners online assigned to it."
diff --git a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
index 67ed2f18d76..554f0b49052 100644
--- a/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
+++ b/spec/features/projects/artifacts/user_downloads_artifacts_spec.rb
@@ -20,23 +20,13 @@ describe "User downloads artifacts" do
     end
 
     context "via job id" do
-      set(:url) { download_project_job_artifacts_path(project, job) }
+      let(:url) { download_project_job_artifacts_path(project, job) }
 
       it_behaves_like "downloading"
     end
 
     context "via branch name and job name" do
-      set(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.ref}/download", job: job.name) }
-
-      it_behaves_like "downloading"
-    end
-
-    context "via clicking the `Download` button" do
-      set(:url) { project_job_path(project, job) }
-
-      before do
-        click_link("Download")
-      end
+      let(:url) { latest_succeeded_project_artifacts_path(project, "#{pipeline.ref}/download", job: job.name) }
 
       it_behaves_like "downloading"
     end
diff --git a/spec/features/projects/jobs_spec.rb b/spec/features/projects/jobs_spec.rb
index 83293c0ca7d..d0bf4975b81 100644
--- a/spec/features/projects/jobs_spec.rb
+++ b/spec/features/projects/jobs_spec.rb
@@ -5,7 +5,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
   let(:user) { create(:user) }
   let(:user_access_level) { :developer }
   let(:project) { create(:project, :repository) }
-  let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:pipeline) { create(:ci_pipeline, project: project, sha: project.commit('HEAD').sha) }
 
   let(:job) { create(:ci_build, :trace_live, pipeline: pipeline) }
   let(:job2) { create(:ci_build) }
@@ -20,7 +20,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
   end
 
   describe "GET /:project/jobs" do
-    let!(:job) { create(:ci_build,  pipeline: pipeline) }
+    let!(:job) { create(:ci_build, pipeline: pipeline) }
 
     context "Pending scope" do
       before do
@@ -115,22 +115,28 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
     context "Job from project" do
       let(:job) { create(:ci_build, :success, :trace_live, pipeline: pipeline) }
 
-      before do
-        visit project_job_path(project, job)
-      end
-
       it 'shows status name', :js do
+        visit project_job_path(project, job)
+
+        wait_for_requests
+
         expect(page).to have_css('.ci-status.ci-success', text: 'passed')
       end
 
-      it 'shows commit`s data' do
-        expect(page.status_code).to eq(200)
+      it 'shows commit`s data', :js do
+        requests = inspect_requests() do
+          visit project_job_path(project, job)
+        end
+
+        wait_for_requests
+        expect(requests.first.status_code).to eq(200)
         expect(page).to have_content pipeline.sha[0..7]
-        expect(page).to have_content pipeline.git_commit_message
-        expect(page).to have_content pipeline.git_author_name
+        expect(page).to have_content pipeline.commit.title
       end
 
       it 'shows active job' do
+        visit project_job_path(project, job)
+
         expect(page).to have_selector('.build-job.active')
       end
     end
@@ -199,7 +205,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
       it { expect(page.status_code).to eq(404) }
     end
 
-    context "Download artifacts" do
+    context "Download artifacts", :js do
       before do
         job.update(legacy_artifacts_file: artifacts_file)
         visit project_job_path(project, job)
@@ -208,9 +214,22 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
       it 'has button to download artifacts' do
         expect(page).to have_content 'Download'
       end
+
+      it 'downloads the zip file when user clicks the download button' do
+        requests = inspect_requests() do
+          click_link 'Download'
+        end
+
+        artifact_request = requests.find { |req| req.url.match(%r{artifacts/download}) }
+
+        expect(artifact_request.response_headers["Content-Disposition"]).to eq(%Q{attachment; filename="#{job.artifacts_file.filename}"})
+        expect(artifact_request.response_headers['Content-Transfer-Encoding']).to eq("binary")
+        expect(artifact_request.response_headers['Content-Type']).to eq("image/gif")
+        expect(artifact_request.body).to eq(job.artifacts_file.file.read.b)
+      end
     end
 
-    context 'Artifacts expire date' do
+    context 'Artifacts expire date', :js do
       before do
         job.update(legacy_artifacts_file: artifacts_file,
                    artifacts_expire_at: expire_at)
@@ -231,12 +250,12 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
 
         context 'when user has ability to update job' do
           it 'keeps artifacts when keep button is clicked' do
-            expect(page).to have_content 'The artifacts will be removed'
+            expect(page).to have_content 'The artifacts will be removed in'
 
             click_link 'Keep'
 
             expect(page).to have_no_link 'Keep'
-            expect(page).to have_no_content 'The artifacts will be removed'
+            expect(page).to have_no_content 'The artifacts will be removed in'
           end
         end
 
@@ -314,6 +333,7 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
 
       shared_examples 'expected variables behavior' do
         it 'shows variable key and value after click', :js do
+          expect(page).to have_content('Token')
           expect(page).to have_css('.js-reveal-variables')
           expect(page).not_to have_css('.js-build-variable')
           expect(page).not_to have_css('.js-build-value')
@@ -542,20 +562,26 @@ describe 'Jobs', :clean_gitlab_redis_shared_state do
     end
   end
 
-  describe "GET /:project/jobs/:id/download" do
+  describe "GET /:project/jobs/:id/download", :js do
     before do
       job.update(legacy_artifacts_file: artifacts_file)
       visit project_job_path(project, job)
+
       click_link 'Download'
     end
 
     context "Build from other project" do
       before do
         job2.update(legacy_artifacts_file: artifacts_file)
-        visit download_project_job_artifacts_path(project, job2)
       end
 
-      it { expect(page.status_code).to eq(404) }
+      it do
+        requests = inspect_requests() do
+          visit download_project_job_artifacts_path(project, job2)
+        end
+
+        expect(requests.first.status_code).to eq(404)
+      end
     end
   end
 
diff --git a/spec/javascripts/jobs/components/artifacts_block_spec.js b/spec/javascripts/jobs/components/artifacts_block_spec.js
index a06d287b3fa..2fa7ff653fe 100644
--- a/spec/javascripts/jobs/components/artifacts_block_spec.js
+++ b/spec/javascripts/jobs/components/artifacts_block_spec.js
@@ -11,6 +11,19 @@ describe('Artifacts block', () => {
   const timeago = getTimeago();
   const formatedDate = timeago.format(expireAt);
 
+  const expiredArtifact = {
+    expire_at: expireAt,
+    expired: true,
+  };
+
+  const nonExpiredArtifact = {
+    download_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/download',
+    browse_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/browse',
+    keep_path: '/gitlab-org/gitlab-ce/-/jobs/98314558/artifacts/keep',
+    expire_at: expireAt,
+    expired: false,
+  };
+
   afterEach(() => {
     vm.$destroy();
   });
@@ -18,100 +31,87 @@ describe('Artifacts block', () => {
   describe('with expired artifacts', () => {
     it('renders expired artifact date and info', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-artifacts-removed')).not.toBeNull();
       expect(vm.$el.querySelector('.js-artifacts-will-be-removed')).toBeNull();
       expect(vm.$el.textContent).toContain(formatedDate);
+      expect(vm.$el.querySelector('.js-artifacts-removed').textContent.trim()).toEqual(
+        'The artifacts were removed',
+      );
     });
   });
 
   describe('with artifacts that will expire', () => {
     it('renders will expire artifact date and info', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: false,
-        willArtifactsExpire: true,
-        expireAt,
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-artifacts-removed')).toBeNull();
       expect(vm.$el.querySelector('.js-artifacts-will-be-removed')).not.toBeNull();
       expect(vm.$el.textContent).toContain(formatedDate);
+      expect(vm.$el.querySelector('.js-artifacts-will-be-removed').textContent.trim()).toEqual(
+        'The artifacts will be removed in',
+      );
     });
   });
 
-  describe('when the user can keep the artifacts', () => {
+  describe('with keep path', () => {
     it('renders the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        keepArtifactsPath: '/keep',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-keep-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not keep the artifacts', () => {
+  describe('without keep path', () => {
     it('does not render the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-keep-artifacts')).toBeNull();
     });
   });
 
-  describe('when the user can download the artifacts', () => {
+  describe('with download path', () => {
     it('renders the download button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        downloadArtifactsPath: '/download',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-download-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not download the artifacts', () => {
+  describe('without download path', () => {
     it('does not render the keep button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-download-artifacts')).toBeNull();
     });
   });
 
-  describe('when the user can browse the artifacts', () => {
+  describe('with browse path', () => {
     it('does not render the browse button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
-        browseArtifactsPath: '/browse',
+        artifact: nonExpiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-browse-artifacts')).not.toBeNull();
     });
   });
 
-  describe('when the user can not browse the artifacts', () => {
+  describe('without browse path', () => {
     it('does not render the browse button', () => {
       vm = mountComponent(Component, {
-        haveArtifactsExpired: true,
-        willArtifactsExpire: false,
-        expireAt,
+        artifact: expiredArtifact,
       });
 
       expect(vm.$el.querySelector('.js-browse-artifacts')).toBeNull();
diff --git a/spec/javascripts/jobs/components/commit_block_spec.js b/spec/javascripts/jobs/components/commit_block_spec.js
index e21fa9c2874..61ee993f46a 100644
--- a/spec/javascripts/jobs/components/commit_block_spec.js
+++ b/spec/javascripts/jobs/components/commit_block_spec.js
@@ -7,11 +7,16 @@ describe('Commit block', () => {
   let vm;
 
   const props = {
-    pipelineShortSha: '1f0fb84f',
-    pipelineShaPath: 'commit/1f0fb84fb6770d74d97eee58118fd3909cd4f48c',
-    mergeRequestReference: '!21244',
-    mergeRequestPath: 'merge_requests/21244',
-    gitCommitTitlte: 'Regenerate pot files',
+    commit: {
+      short_id: '1f0fb84f',
+      commit_path: 'commit/1f0fb84fb6770d74d97eee58118fd3909cd4f48c',
+      title: 'Update README.md',
+    },
+    mergeRequest: {
+      iid: '!21244',
+      path: 'merge_requests/21244',
+    },
+    isLastBlock: true,
   };
 
   afterEach(() => {
@@ -26,12 +31,18 @@ describe('Commit block', () => {
     });
 
     it('renders pipeline short sha link', () => {
-      expect(vm.$el.querySelector('.js-commit-sha').getAttribute('href')).toEqual(props.pipelineShaPath);
-      expect(vm.$el.querySelector('.js-commit-sha').textContent.trim()).toEqual(props.pipelineShortSha);
+      expect(vm.$el.querySelector('.js-commit-sha').getAttribute('href')).toEqual(
+        props.commit.commit_path,
+      );
+      expect(vm.$el.querySelector('.js-commit-sha').textContent.trim()).toEqual(
+        props.commit.short_id,
+      );
     });
 
     it('renders clipboard button', () => {
-      expect(vm.$el.querySelector('button').getAttribute('data-clipboard-text')).toEqual(props.pipelineShortSha);
+      expect(vm.$el.querySelector('button').getAttribute('data-clipboard-text')).toEqual(
+        props.commit.short_id,
+      );
     });
   });
 
@@ -41,17 +52,19 @@ describe('Commit block', () => {
         ...props,
       });
 
-      expect(vm.$el.querySelector('.js-link-commit').getAttribute('href')).toEqual(props.mergeRequestPath);
-      expect(vm.$el.querySelector('.js-link-commit').textContent.trim()).toEqual(props.mergeRequestReference);
-
+      expect(vm.$el.querySelector('.js-link-commit').getAttribute('href')).toEqual(
+        props.mergeRequest.path,
+      );
+      expect(vm.$el.querySelector('.js-link-commit').textContent.trim()).toEqual(
+        props.mergeRequest.iid,
+      );
     });
   });
 
   describe('without merge request', () => {
     it('does not render merge request', () => {
       const copyProps = Object.assign({}, props);
-      delete copyProps.mergeRequestPath;
-      delete copyProps.mergeRequestReference;
+      delete copyProps.mergeRequest;
 
       vm = mountComponent(Component, {
         ...copyProps,
@@ -67,7 +80,7 @@ describe('Commit block', () => {
         ...props,
       });
 
-      expect(vm.$el.textContent).toContain(props.gitCommitTitlte);
+      expect(vm.$el.textContent).toContain(props.commit.title);
     });
   });
 });
diff --git a/spec/javascripts/jobs/components/trigger_value_spec.js b/spec/javascripts/jobs/components/trigger_block_spec.js
similarity index 75%
rename from spec/javascripts/jobs/components/trigger_value_spec.js
rename to spec/javascripts/jobs/components/trigger_block_spec.js
index 3d41a3cfac1..e1b9898393e 100644
--- a/spec/javascripts/jobs/components/trigger_value_spec.js
+++ b/spec/javascripts/jobs/components/trigger_block_spec.js
@@ -13,7 +13,9 @@ describe('Trigger block', () => {
   describe('with short token', () => {
     it('renders short token', () => {
       vm = mountComponent(Component, {
-        shortToken: '0a666b2',
+        trigger: {
+          short_token: '0a666b2',
+        },
       });
 
       expect(vm.$el.querySelector('.js-short-token').textContent).toContain('0a666b2');
@@ -22,7 +24,7 @@ describe('Trigger block', () => {
 
   describe('without short token', () => {
     it('does not render short token', () => {
-      vm = mountComponent(Component, {});
+      vm = mountComponent(Component, { trigger: {} });
 
       expect(vm.$el.querySelector('.js-short-token')).toBeNull();
     });
@@ -32,9 +34,12 @@ describe('Trigger block', () => {
     describe('reveal variables', () => {
       it('reveals variables on click', done => {
         vm = mountComponent(Component, {
-          variables: {
-            key: 'value',
-            variable: 'foo',
+          trigger: {
+            short_token: 'bd7e',
+            variables: [
+              { key: 'UPLOAD_TO_GCS', value: 'false', public: false },
+              { key: 'UPLOAD_TO_S3', value: 'true', public: false },
+            ],
           },
         });
 
@@ -44,10 +49,10 @@ describe('Trigger block', () => {
           .$nextTick()
           .then(() => {
             expect(vm.$el.querySelector('.js-build-variables')).not.toBeNull();
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('key');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('value');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('variable');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('foo');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('UPLOAD_TO_GCS');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('false');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('UPLOAD_TO_S3');
+            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('true');
           })
           .then(done)
           .catch(done.fail);
@@ -57,7 +62,7 @@ describe('Trigger block', () => {
 
   describe('without variables', () => {
     it('does not render variables', () => {
-      vm = mountComponent(Component);
+      vm = mountComponent(Component, { trigger: {} });
 
       expect(vm.$el.querySelector('.js-reveal-variables')).toBeNull();
       expect(vm.$el.querySelector('.js-build-variables')).toBeNull();
diff --git a/spec/views/projects/jobs/show.html.haml_spec.rb b/spec/views/projects/jobs/show.html.haml_spec.rb
index c93152b88e3..496646dc623 100644
--- a/spec/views/projects/jobs/show.html.haml_spec.rb
+++ b/spec/views/projects/jobs/show.html.haml_spec.rb
@@ -188,40 +188,4 @@ describe 'projects/jobs/show' do
       expect(rendered).not_to have_link('New issue')
     end
   end
-
-  context 'when incomplete trigger_request is used' do
-    before do
-      build.trigger_request = FactoryBot.build(:ci_trigger_request, trigger: nil)
-    end
-
-    it 'test should not render token block' do
-      render
-
-      expect(rendered).not_to have_content('Token')
-    end
-  end
-
-  context 'when complete trigger_request is used' do
-    before do
-      build.trigger_request = FactoryBot.build(:ci_trigger_request)
-    end
-
-    it 'should render token' do
-      render
-
-      expect(rendered).to have_content('Token')
-      expect(rendered).to have_content(build.trigger_request.trigger.short_token)
-    end
-  end
-
-  describe 'commit title in sidebar' do
-    let(:commit_title) { project.commit.title }
-
-    it 'shows commit title and not show commit message' do
-      render
-
-      expect(rendered).to have_css('p.build-light-text.append-bottom-0',
-        text: /\A\n#{Regexp.escape(commit_title)}\n\Z/)
-    end
-  end
 end