Merge branch '25990-web-terminal-improvements' into 'master'

Improve terminal button on job log page

See merge request gitlab-org/gitlab-ce!21094

6 files changed, 77 insertions, 45 deletions

diff --git a/app/assets/javascripts/jobs/components/sidebar_details_block.vue b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
index d2adf628050..36d4a3e2bc9 100644
--- a/app/assets/javascripts/jobs/components/sidebar_details_block.vue
+++ b/app/assets/javascripts/jobs/components/sidebar_details_block.vue
@@ -1,14 +1,16 @@
 <script>
-import detailRow from './sidebar_detail_row.vue';
-import loadingIcon from '../../vue_shared/components/loading_icon.vue';
-import timeagoMixin from '../../vue_shared/mixins/timeago';
-import { timeIntervalInWords } from '../../lib/utils/datetime_utility';
+import LoadingIcon from '~/vue_shared/components/loading_icon.vue';
+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';
 
 export default {
   name: 'SidebarDetailsBlock',
   components: {
-    detailRow,
-    loadingIcon,
+    DetailRow,
+    LoadingIcon,
+    Icon,
   },
   mixins: [timeagoMixin],
   props: {
@@ -20,16 +22,16 @@ export default {
       type: Boolean,
       required: true,
     },
-    canUserRetry: {
-      type: Boolean,
-      required: false,
-      default: false,
-    },
     runnerHelpUrl: {
       type: String,
       required: false,
       default: '',
     },
+    terminalPath: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   computed: {
     shouldRenderContent() {
@@ -92,7 +94,7 @@ export default {
         {{ job.name }}
       </strong>
       <a
-        v-if="canUserRetry"
+        v-if="job.retry_path"
         :class="retryButtonClass"
         :href="job.retry_path"
         data-method="post"
@@ -100,6 +102,16 @@ export default {
       >
         {{ __('Retry') }}
       </a>
+      <a
+        v-if="terminalPath"
+        :href="terminalPath"
+        class="js-terminal-link pull-right btn btn-primary
+  btn-inverted visible-md-block visible-lg-block"
+        target="_blank"
+      >
+        {{ __('Debug') }}
+        <icon name="external-link" />
+      </a>
       <button
         :aria-label="__('Toggle Sidebar')"
         type="button"
@@ -125,7 +137,7 @@ export default {
           {{ __('New issue') }}
         </a>
         <a
-          v-if="canUserRetry"
+          v-if="job.retry_path"
           :href="job.retry_path"
           class="js-retry-job btn btn-inverted-secondary"
           data-method="post"
diff --git a/app/assets/javascripts/jobs/job_details_bundle.js b/app/assets/javascripts/jobs/job_details_bundle.js
index 0db7b95636c..a84324f14b2 100644
--- a/app/assets/javascripts/jobs/job_details_bundle.js
+++ b/app/assets/javascripts/jobs/job_details_bundle.js
@@ -52,9 +52,9 @@ export default () => {
       return createElement('details-block', {
         props: {
           isLoading: this.mediator.state.isLoading,
-          canUserRetry: !!('canUserRetry' in detailsBlockDataset),
           job: this.mediator.store.state.job,
           runnerHelpUrl: dataset.runnerHelpUrl,
+          terminalPath: detailsBlockDataset.terminalPath,
         },
       });
     },
diff --git a/app/views/projects/jobs/_sidebar.html.haml b/app/views/projects/jobs/_sidebar.html.haml
index 759efd4e9d4..86b2b8bf2f7 100644
--- a/app/views/projects/jobs/_sidebar.html.haml
+++ b/app/views/projects/jobs/_sidebar.html.haml
@@ -1,12 +1,7 @@
 %aside.right-sidebar.right-sidebar-expanded.build-sidebar.js-build-sidebar.js-right-sidebar{ data: { "offset-top" => "101", "spy" => "affix" } }
   .sidebar-container
     .blocks-container
-      - if can?(current_user, :create_build_terminal, @build)
-        .block
-          = link_to terminal_project_job_path(@project, @build), class: 'terminal-button pull-right btn visible-md-block visible-lg-block', title: 'Terminal' do
-            Terminal
-
-      #js-details-block-vue{ data: { can_user_retry: can?(current_user, :update_build, @build) && @build.retryable? } }
+      #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
diff --git a/changelogs/unreleased/25990-web-terminal-improvements.yml b/changelogs/unreleased/25990-web-terminal-improvements.yml
new file mode 100644
index 00000000000..99a4a82ea66
--- /dev/null
+++ b/changelogs/unreleased/25990-web-terminal-improvements.yml
@@ -0,0 +1,5 @@
+---
+title: Make terminal button more visible
+merge_request:
+author:
+type: changed
diff --git a/locale/gitlab.pot b/locale/gitlab.pot
index 92f424ccdf0..e8bf7ae8f0e 100644
--- a/locale/gitlab.pot
+++ b/locale/gitlab.pot
@@ -1993,6 +1993,9 @@ msgstr ""
 msgid "DashboardProjects|Personal"
 msgstr ""
 
+msgid "Debug"
+msgstr ""
+
 msgid "Dec"
 msgstr ""
 
diff --git a/spec/javascripts/jobs/sidebar_details_block_spec.js b/spec/javascripts/jobs/sidebar_details_block_spec.js
index 9c4454252ce..21ef5650b80 100644
--- a/spec/javascripts/jobs/sidebar_details_block_spec.js
+++ b/spec/javascripts/jobs/sidebar_details_block_spec.js
@@ -1,6 +1,7 @@
 import Vue from 'vue';
 import sidebarDetailsBlock from '~/jobs/components/sidebar_details_block.vue';
 import job from './mock_data';
+import mountComponent from '../helpers/vue_mount_component_helper';
 
 describe('Sidebar details block', () => {
   let SidebarComponent;
@@ -20,39 +21,53 @@ describe('Sidebar details block', () => {
 
   describe('when it is loading', () => {
     it('should render a loading spinner', () => {
-      vm = new SidebarComponent({
-        propsData: {
-          job: {},
-          isLoading: true,
-        },
-      }).$mount();
-
+      vm = mountComponent(SidebarComponent, {
+        job: {},
+        isLoading: true,
+      });
       expect(vm.$el.querySelector('.fa-spinner')).toBeDefined();
     });
   });
 
-  describe("when user can't retry", () => {
+  describe('when there is no retry path retry', () => {
     it('should not render a retry button', () => {
-      vm = new SidebarComponent({
-        propsData: {
-          job: {},
-          canUserRetry: false,
-          isLoading: true,
-        },
-      }).$mount();
+      vm = mountComponent(SidebarComponent, {
+        job: {},
+        isLoading: false,
+      });
 
       expect(vm.$el.querySelector('.js-retry-job')).toBeNull();
     });
   });
 
-  beforeEach(() => {
-    vm = new SidebarComponent({
-      propsData: {
+  describe('without terminal path', () => {
+    it('does not render terminal link', () => {
+      vm = mountComponent(SidebarComponent, {
         job,
-        canUserRetry: true,
         isLoading: false,
-      },
-    }).$mount();
+      });
+
+      expect(vm.$el.querySelector('.js-terminal-link')).toBeNull();
+    });
+  });
+
+  describe('with terminal path', () => {
+    it('renders terminal link', () => {
+      vm = mountComponent(SidebarComponent, {
+        job,
+        isLoading: false,
+        terminalPath: 'job/43123/terminal',
+      });
+
+      expect(vm.$el.querySelector('.js-terminal-link')).not.toBeNull();
+    });
+  });
+
+  beforeEach(() => {
+    vm = mountComponent(SidebarComponent, {
+      job,
+      isLoading: false,
+    });
   });
 
   describe('actions', () => {
@@ -102,13 +117,15 @@ describe('Sidebar details block', () => {
     });
 
     it('should render runner ID', () => {
-      expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual('Runner: local ci runner (#1)');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-runner'))).toEqual(
+        'Runner: local ci runner (#1)',
+      );
     });
 
     it('should render timeout information', () => {
-      expect(
-        trimWhitespace(vm.$el.querySelector('.js-job-timeout')),
-      ).toEqual('Timeout: 1m 40s (from runner)');
+      expect(trimWhitespace(vm.$el.querySelector('.js-job-timeout'))).toEqual(
+        'Timeout: 1m 40s (from runner)',
+      );
     });
 
     it('should render coverage', () => {