Merge branch '347237-move-erase-job-log-button' into 'master'

Re-position the job log erase button

See merge request gitlab-org/gitlab!81049

app/assets/javascripts/jobs/components/job_app.vue

@@ -288,7 +288,6 @@ export default {
               'sidebar-collapsed': !isSidebarOpen,
               'has-archived-block': job.archived,
             }"
-            :erase-path="job.erase_path"
             :size="jobLogSize"
             :raw-path="job.raw_path"
             :is-scroll-bottom-disabled="isScrollBottomDisabled"
@@ -325,6 +324,7 @@ export default {
         'right-sidebar-expanded': isSidebarOpen,
         'right-sidebar-collapsed': !isSidebarOpen,
       }"
+      :erase-path="job.erase_path"
       :artifact-help-url="artifactHelpUrl"
       data-testid="job-sidebar"
     />

app/assets/javascripts/jobs/components/job_log_controllers.vue

@@ -5,7 +5,6 @@ import { __, s__, sprintf } from '~/locale';
 
 export default {
   i18n: {
-    eraseLogButtonLabel: s__('Job|Erase job log and artifacts'),
     scrollToBottomButtonLabel: s__('Job|Scroll to bottom'),
     scrollToTopButtonLabel: s__('Job|Scroll to top'),
     showRawButtonLabel: s__('Job|Show complete raw'),
@@ -18,11 +17,6 @@ export default {
     GlTooltip: GlTooltipDirective,
   },
   props: {
-    erasePath: {
-      type: String,
-      required: false,
-      default: null,
-    },
     size: {
       type: Number,
       required: true,
@@ -97,20 +91,6 @@ export default {
         data-testid="job-raw-link-controller"
         icon="doc-text"
       />
-
-      <gl-button
-        v-if="erasePath"
-        v-gl-tooltip.body
-        :title="$options.i18n.eraseLogButtonLabel"
-        :aria-label="$options.i18n.eraseLogButtonLabel"
-        :href="erasePath"
-        :data-confirm="__('Are you sure you want to erase this build?')"
-        class="gl-ml-3"
-        data-testid="job-log-erase-link"
-        data-confirm-btn-variant="danger"
-        data-method="post"
-        icon="remove"
-      />
       <!-- eo links -->
 
       <!-- scroll buttons -->

app/assets/javascripts/jobs/components/job_sidebar_retry_button.vue

 <script>
-import { GlButton, GlLink, GlModalDirective } from '@gitlab/ui';
+import { GlButton, GlModalDirective } from '@gitlab/ui';
 import { mapGetters } from 'vuex';
 import { JOB_SIDEBAR } from '../constants';
 
@@ -10,7 +10,6 @@ export default {
   },
   components: {
     GlButton,
-    GlLink,
   },
   directives: {
     GlModal: GlModalDirective,
@@ -37,9 +36,18 @@ export default {
     :aria-label="$options.i18n.retryLabel"
     category="primary"
     variant="confirm"
-    >{{ $options.i18n.retryLabel }}</gl-button
-  >
-  <gl-link v-else :href="href" class="btn gl-button btn-confirm" data-method="post" rel="nofollow"
-    >{{ $options.i18n.retryLabel }}
-  </gl-link>
+    icon="retry"
+    data-testid="retry-job-button"
+  />
+
+  <gl-button
+    v-else
+    :href="href"
+    :aria-label="$options.i18n.retryLabel"
+    category="primary"
+    variant="confirm"
+    icon="retry"
+    data-method="post"
+    data-testid="retry-job-link"
+  />
 </template>

app/assets/javascripts/jobs/components/sidebar.vue

 <script>
-import { GlButton, GlIcon } from '@gitlab/ui';
+import { GlButton, GlIcon, GlTooltipDirective } from '@gitlab/ui';
 import { isEmpty } from 'lodash';
 import { mapActions, mapGetters, mapState } from 'vuex';
+import { s__ } from '~/locale';
 import TooltipOnTruncate from '~/vue_shared/components/tooltip_on_truncate/tooltip_on_truncate.vue';
 import { JOB_SIDEBAR } from '../constants';
 import ArtifactsBlock from './artifacts_block.vue';
@@ -18,10 +19,17 @@ export const forwardDeploymentFailureModalId = 'forward-deployment-failure';
 export default {
   name: 'JobSidebar',
   i18n: {
+    eraseLogButtonLabel: s__('Job|Erase job log and artifacts'),
+    eraseLogConfirmText: s__('Job|Are you sure you want to erase this job log and artifacts?'),
+    cancelJobButtonLabel: s__('Job|Cancel'),
+    retryJobButtonLabel: s__('Job|Retry'),
     ...JOB_SIDEBAR,
   },
   borderTopClass: ['gl-border-t-solid', 'gl-border-t-1', 'gl-border-t-gray-100'],
   forwardDeploymentFailureModalId,
+  directives: {
+    GlTooltip: GlTooltipDirective,
+  },
   components: {
     ArtifactsBlock,
     CommitBlock,
@@ -41,6 +49,11 @@ export default {
       required: false,
       default: '',
     },
+    erasePath: {
+      type: String,
+      required: false,
+      default: null,
+    },
   },
   computed: {
     ...mapGetters(['hasForwardDeploymentFailure']),
@@ -81,8 +94,24 @@ export default {
             </h4>
           </tooltip-on-truncate>
           <div class="gl-flex-grow-1 gl-flex-shrink-0 gl-text-right">
+            <gl-button
+              v-if="erasePath"
+              v-gl-tooltip.left
+              :title="$options.i18n.eraseLogButtonLabel"
+              :aria-label="$options.i18n.eraseLogButtonLabel"
+              :href="erasePath"
+              :data-confirm="$options.i18n.eraseLogConfirmText"
+              class="gl-mr-2"
+              data-testid="job-log-erase-link"
+              data-confirm-btn-variant="danger"
+              data-method="post"
+              icon="remove"
+            />
             <job-sidebar-retry-button
               v-if="job.retry_path"
+              v-gl-tooltip.left
+              :title="$options.i18n.retryJobButtonLabel"
+              :aria-label="$options.i18n.retryJobButtonLabel"
               :category="retryButtonCategory"
               :href="job.retry_path"
               :modal-id="$options.forwardDeploymentFailureModalId"
@@ -92,12 +121,15 @@ export default {
             />
             <gl-button
               v-if="job.cancel_path"
+              v-gl-tooltip.left
+              :title="$options.i18n.cancelJobButtonLabel"
+              :aria-label="$options.i18n.cancelJobButtonLabel"
               :href="job.cancel_path"
+              icon="cancel"
               data-method="post"
               data-testid="cancel-button"
               rel="nofollow"
-              >{{ $options.i18n.cancel }}
-            </gl-button>
+            />
           </div>
 
           <gl-button

locale/gitlab.pot

@@ -4738,9 +4738,6 @@ msgstr ""
 msgid "Are you sure you want to discard your changes?"
 msgstr ""
 
-msgid "Are you sure you want to erase this build?"
-msgstr ""
-
 msgid "Are you sure you want to import %d repository?"
 msgid_plural "Are you sure you want to import %d repositories?"
 msgstr[0] ""
@@ -21031,9 +21028,15 @@ msgstr ""
 msgid "Jobs|You're about to retry a job that failed because it attempted to deploy code that is older than the latest deployment. Retrying this job could result in overwriting the environment with the older source code."
 msgstr ""
 
+msgid "Job|Are you sure you want to erase this job log and artifacts?"
+msgstr ""
+
 msgid "Job|Browse"
 msgstr ""
 
+msgid "Job|Cancel"
+msgstr ""
+
 msgid "Job|Complete Raw"
 msgstr ""
 
@@ -21061,6 +21064,9 @@ msgstr ""
 msgid "Job|Pipeline"
 msgstr ""
 
+msgid "Job|Retry"
+msgstr ""
+
 msgid "Job|Scroll to bottom"
 msgstr ""
 

spec/features/projects/jobs_spec.rb

@@ -313,7 +313,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
 
       context 'job is cancelable' do
         it 'shows cancel button' do
-          click_link 'Cancel'
+          find('[data-testid="cancel-button"]').click
 
           expect(page.current_path).to eq(job_url)
         end
@@ -1031,7 +1031,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
       end
 
       it 'loads the page and shows all needed controls' do
-        expect(page).to have_content 'Retry'
+        expect(page).to have_selector('[data-testid="retry-button"')
       end
     end
   end
@@ -1049,7 +1049,7 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
 
       it 'shows the right status and buttons' do
         page.within('aside.right-sidebar') do
-          expect(page).to have_content 'Cancel'
+          expect(page).to have_selector('[data-testid="cancel-button"')
         end
       end
     end

spec/frontend/jobs/components/job_log_controllers_spec.js

@@ -8,7 +8,6 @@ describe('Job log controllers', () => {
   afterEach(() => {
     if (wrapper?.destroy) {
       wrapper.destroy();
-      wrapper = null;
     }
   });
 
@@ -34,7 +33,6 @@ describe('Job log controllers', () => {
   const findTruncatedInfo = () => wrapper.find('[data-testid="log-truncated-info"]');
   const findRawLink = () => wrapper.find('[data-testid="raw-link"]');
   const findRawLinkController = () => wrapper.find('[data-testid="job-raw-link-controller"]');
-  const findEraseLink = () => wrapper.find('[data-testid="job-log-erase-link"]');
   const findScrollTop = () => wrapper.find('[data-testid="job-controller-scroll-top"]');
   const findScrollBottom = () => wrapper.find('[data-testid="job-controller-scroll-bottom"]');
 
@@ -76,28 +74,6 @@ describe('Job log controllers', () => {
         expect(findRawLinkController().exists()).toBe(false);
       });
     });
-
-    describe('when is erasable', () => {
-      beforeEach(() => {
-        createWrapper();
-      });
-
-      it('renders erase job link', () => {
-        expect(findEraseLink().exists()).toBe(true);
-      });
-    });
-
-    describe('when it is not erasable', () => {
-      beforeEach(() => {
-        createWrapper({
-          erasePath: null,
-        });
-      });
-
-      it('does not render erase button', () => {
-        expect(findEraseLink().exists()).toBe(false);
-      });
-    });
   });
 
   describe('scroll buttons', () => {

spec/frontend/jobs/components/job_sidebar_retry_button_spec.js

-import { GlButton, GlLink } from '@gitlab/ui';
-import { shallowMount } from '@vue/test-utils';
+import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import JobsSidebarRetryButton from '~/jobs/components/job_sidebar_retry_button.vue';
 import createStore from '~/jobs/store';
 import job from '../mock_data';
@@ -9,12 +8,12 @@ describe('Job Sidebar Retry Button', () => {
   let wrapper;
 
   const forwardDeploymentFailure = 'forward_deployment_failure';
-  const findRetryButton = () => wrapper.find(GlButton);
-  const findRetryLink = () => wrapper.find(GlLink);
+  const findRetryButton = () => wrapper.findByTestId('retry-job-button');
+  const findRetryLink = () => wrapper.findByTestId('retry-job-link');
 
   const createWrapper = ({ props = {} } = {}) => {
     store = createStore();
-    wrapper = shallowMount(JobsSidebarRetryButton, {
+    wrapper = shallowMountExtended(JobsSidebarRetryButton, {
       propsData: {
         href: job.retry_path,
         modalId: 'modal-id',
@@ -27,7 +26,6 @@ describe('Job Sidebar Retry Button', () => {
   afterEach(() => {
     if (wrapper) {
       wrapper.destroy();
-      wrapper = null;
     }
   });
 
@@ -44,7 +42,6 @@ describe('Job Sidebar Retry Button', () => {
 
       expect(findRetryButton().exists()).toBe(buttonExists);
       expect(findRetryLink().exists()).toBe(linkExists);
-      expect(wrapper.text()).toMatch('Retry');
     },
   );
 
@@ -55,6 +52,7 @@ describe('Job Sidebar Retry Button', () => {
       expect(findRetryButton().attributes()).toMatchObject({
         category: 'primary',
         variant: 'confirm',
+        icon: 'retry',
       });
     });
   });
@@ -64,6 +62,7 @@ describe('Job Sidebar Retry Button', () => {
       expect(findRetryLink().attributes()).toMatchObject({
         'data-method': 'post',
         href: job.retry_path,
+        icon: 'retry',
       });
     });
   });

spec/frontend/jobs/components/sidebar_spec.js

@@ -21,25 +21,54 @@ describe('Sidebar details block', () => {
   const findNewIssueButton = () => wrapper.findByTestId('job-new-issue');
   const findRetryButton = () => wrapper.find(JobRetryButton);
   const findTerminalLink = () => wrapper.findByTestId('terminal-link');
+  const findEraseLink = () => wrapper.findByTestId('job-log-erase-link');
 
-  const createWrapper = ({ props = {} } = {}) => {
+  const createWrapper = (props) => {
     store = createStore();
 
     store.state.job = job;
 
     wrapper = extendedWrapper(
       shallowMount(Sidebar, {
+        propsData: {
           ...props,
+        },
+
         store,
       }),
     );
   };
 
   afterEach(() => {
-    if (wrapper) {
     wrapper.destroy();
-      wrapper = null;
-    }
+  });
+
+  describe('when job log is erasable', () => {
+    const path = '/root/ci-project/-/jobs/1447/erase';
+
+    beforeEach(() => {
+      createWrapper({
+        erasePath: path,
+      });
+    });
+
+    it('renders erase job link', () => {
+      expect(findEraseLink().exists()).toBe(true);
+    });
+
+    it('erase job link has correct path', () => {
+      expect(findEraseLink().attributes('href')).toBe(path);
+    });
+  });
+
+  describe('when job log is not erasable', () => {
+    beforeEach(() => {
+      createWrapper();
+    });
+
+    it('does not render erase button', () => {
+      expect(findEraseLink().exists()).toBe(false);
+    });
   });
 
   describe('when there is no retry path retry', () => {
@@ -86,7 +115,7 @@ describe('Sidebar details block', () => {
     });
 
     it('should render link to cancel job', () => {
-      expect(findCancelButton().text()).toMatch('Cancel');
+      expect(findCancelButton().props('icon')).toBe('cancel');
       expect(findCancelButton().attributes('href')).toBe(job.cancel_path);
     });
   });