Merge branch 'pb-refactor-trigger-block-debt' into 'master'

Refactor trigger block and spec

See merge request gitlab-org/gitlab!53151

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

 <script>
 import { GlAlert, GlBadge, GlLink } from '@gitlab/ui';
-import { s__ } from '../../locale';
+import { s__ } from '~/locale';
 /**
  * Renders Stuck Runners block for job's view.
  */

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

 <script>
-import { GlButton } from '@gitlab/ui';
+import { GlButton, GlTable } from '@gitlab/ui';
 import { __ } from '~/locale';
 
-const HIDDEN_VALUE = '••••••';
+const DEFAULT_TD_CLASSES = 'gl-w-half gl-font-sm! gl-border-gray-200!';
+const DEFAULT_TH_CLASSES =
+  'gl-bg-transparent! gl-border-b-solid! gl-border-b-gray-200! gl-border-b-1!';
 
 export default {
+  fields: [
+    {
+      key: 'key',
+      label: __('Key'),
+      tdAttr: { 'data-testid': 'trigger-build-key' },
+      tdClass: DEFAULT_TD_CLASSES,
+      thClass: DEFAULT_TH_CLASSES,
+    },
+    {
+      key: 'value',
+      label: __('Value'),
+      tdAttr: { 'data-testid': 'trigger-build-value' },
+      tdClass: DEFAULT_TD_CLASSES,
+      thClass: DEFAULT_TH_CLASSES,
+    },
+  ],
   components: {
     GlButton,
+    GlTable,
   },
   props: {
     trigger: {
@@ -21,7 +40,7 @@ export default {
   },
   computed: {
     hasVariables() {
-      return this.trigger.variables && this.trigger.variables.length > 0;
+      return this.trigger.variables.length > 0;
     },
     getToggleButtonText() {
       return this.showVariableValues ? __('Hide values') : __('Reveal values');
@@ -35,45 +54,41 @@ export default {
       this.showVariableValues = !this.showVariableValues;
     },
     getDisplayValue(value) {
-      return this.showVariableValues ? value : HIDDEN_VALUE;
+      return this.showVariableValues ? value : '••••••';
     },
   },
 };
 </script>
 
 <template>
-  <div class="build-widget block">
+  <div class="block">
     <p
       v-if="trigger.short_token"
-      class="js-short-token"
       :class="{ 'gl-mb-2': hasVariables, 'gl-mb-0': !hasVariables }"
+      data-testid="trigger-short-token"
     >
-      <span class="font-weight-bold">{{ __('Trigger token:') }}</span> {{ trigger.short_token }}
+      <span class="gl-font-weight-bold">{{ __('Trigger token:') }}</span> {{ trigger.short_token }}
     </p>
 
     <template v-if="hasVariables">
-      <p class="trigger-variables-btn-container d-flex">
-        <span class="font-weight-bold">{{ __('Trigger variables:') }}</span>
+      <p class="gl-display-flex gl-justify-content-space-between gl-align-items-center">
+        <span class="gl-font-weight-bold">{{ __('Trigger variables:') }}</span>
 
         <gl-button
           v-if="hasValues"
-          class="group js-reveal-variables trigger-variables-btn"
+          class="gl-mt-2"
           size="small"
+          data-testid="trigger-reveal-values-button"
           @click="toggleValues"
           >{{ getToggleButtonText }}</gl-button
         >
       </p>
 
-      <table class="js-build-variables trigger-build-variables">
-        <tr v-for="(variable, index) in trigger.variables" :key="`${variable.key}-${index}`">
-          <td class="js-build-variable trigger-build-variable trigger-variables-table-cell">
-            {{ variable.key }}
-          </td>
-          <td class="js-build-value trigger-build-value trigger-variables-table-cell">
-            {{ getDisplayValue(variable.value) }}
-          </td>
-        </tr>
-      </table>
+      <gl-table :items="trigger.variables" :fields="$options.fields" small bordered>
+        <template #cell(value)="data">
+          {{ getDisplayValue(data.value) }}
+        </template>
+      </gl-table>
     </template>
   </div>
 </template>

spec/features/projects/jobs_spec.rb

@@ -533,10 +533,10 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
             expect(page).to have_content('Trigger token')
             expect(page).to have_content('Trigger variables')
 
-            expect(page).not_to have_css('.js-reveal-variables')
+            expect(page).not_to have_selector('[data-testid="trigger-reveal-values-button"]')
 
-            expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
-            expect(page).to have_selector('.js-build-value', text: '••••••')
+            expect(page).to have_selector('[data-testid="trigger-build-key"]', text: 'TRIGGER_KEY_1')
+            expect(page).to have_selector('[data-testid="trigger-build-value"]', text: '••••••')
           end
         end
 
@@ -571,17 +571,17 @@ RSpec.describe 'Jobs', :clean_gitlab_redis_shared_state do
             expect(page).to have_content('Trigger token')
             expect(page).to have_content('Trigger variables')
 
-            expect(page).to have_css('.js-reveal-variables')
+            expect(page).to have_selector('[data-testid="trigger-reveal-values-button"]')
 
-            expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
-            expect(page).to have_selector('.js-build-value', text: '••••••')
+            expect(page).to have_selector('[data-testid="trigger-build-key"]', text: 'TRIGGER_KEY_1')
+            expect(page).to have_selector('[data-testid="trigger-build-value"]', text: '••••••')
           end
 
           it 'reveals values on button click', :js do
             click_button 'Reveal values'
 
-            expect(page).to have_selector('.js-build-variable', text: 'TRIGGER_KEY_1')
-            expect(page).to have_selector('.js-build-value', text: 'TRIGGER_VALUE_1')
+            expect(page).to have_selector('[data-testid="trigger-build-key"]', text: 'TRIGGER_KEY_1')
+            expect(page).to have_selector('[data-testid="trigger-build-value"]', text: 'TRIGGER_VALUE_1')
           end
         end
 

spec/frontend/jobs/components/trigger_block_spec.js

-import Vue from 'vue';
-import mountComponent from 'helpers/vue_mount_component_helper';
-import component from '~/jobs/components/trigger_block.vue';
+import { mount } from '@vue/test-utils';
+import { GlButton, GlTable } from '@gitlab/ui';
+import TriggerBlock from '~/jobs/components/trigger_block.vue';
 
 describe('Trigger block', () => {
-  const Component = Vue.extend(component);
-  let vm;
+  let wrapper;
+
+  const findRevealButton = () => wrapper.find(GlButton);
+  const findVariableTable = () => wrapper.find(GlTable);
+  const findShortToken = () => wrapper.find('[data-testid="trigger-short-token"]');
+  const findVariableValue = (index) =>
+    wrapper.findAll('[data-testid="trigger-build-value"]').at(index);
+  const findVariableKey = (index) => wrapper.findAll('[data-testid="trigger-build-key"]').at(index);
+
+  const createComponent = (props) => {
+    wrapper = mount(TriggerBlock, {
+      propsData: {
+        ...props,
+      },
+    });
+  };
 
   afterEach(() => {
-    vm.$destroy();
+    wrapper.destroy();
+    wrapper = null;
   });
 
-  describe('with short token', () => {
+  describe('with short token and no variables', () => {
     it('renders short token', () => {
-      vm = mountComponent(Component, {
+      createComponent({
         trigger: {
           short_token: '0a666b2',
+          variables: [],
         },
       });
 
-      expect(vm.$el.querySelector('.js-short-token').textContent).toContain('0a666b2');
+      expect(findShortToken().text()).toContain('0a666b2');
     });
   });
 
-  describe('without short token', () => {
+  describe('without variables or short token', () => {
+    beforeEach(() => {
+      createComponent({ trigger: { variables: [] } });
+    });
+
     it('does not render short token', () => {
-      vm = mountComponent(Component, { trigger: {} });
+      expect(findShortToken().exists()).toBe(false);
+    });
 
-      expect(vm.$el.querySelector('.js-short-token')).toBeNull();
+    it('does not render variables', () => {
+      expect(findRevealButton().exists()).toBe(false);
+      expect(findVariableTable().exists()).toBe(false);
     });
   });
 
   describe('with variables', () => {
     describe('hide/reveal variables', () => {
-      it('should toggle variables on click', (done) => {
-        vm = mountComponent(Component, {
+      it('should toggle variables on click', async () => {
+        const hiddenValue = '••••••';
+        const gcsVar = { key: 'UPLOAD_TO_GCS', value: 'false', public: false };
+        const s3Var = { key: 'UPLOAD_TO_S3', value: 'true', public: false };
+
+        createComponent({
           trigger: {
-            short_token: 'bd7e',
-            variables: [
-              { key: 'UPLOAD_TO_GCS', value: 'false', public: false },
-              { key: 'UPLOAD_TO_S3', value: 'true', public: false },
-            ],
+            variables: [gcsVar, s3Var],
           },
         });
 
-        vm.$el.querySelector('.js-reveal-variables').click();
-
-        vm.$nextTick()
-          .then(() => {
-            expect(vm.$el.querySelector('.js-build-variables')).not.toBeNull();
-            expect(vm.$el.querySelector('.js-reveal-variables').textContent.trim()).toEqual(
-              'Hide values',
-            );
-
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain(
-              'UPLOAD_TO_GCS',
-            );
+        expect(findRevealButton().text()).toBe('Reveal values');
 
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('false');
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain(
-              'UPLOAD_TO_S3',
-            );
+        expect(findVariableValue(0).text()).toBe(hiddenValue);
+        expect(findVariableValue(1).text()).toBe(hiddenValue);
 
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain('true');
+        expect(findVariableKey(0).text()).toBe(gcsVar.key);
+        expect(findVariableKey(1).text()).toBe(s3Var.key);
 
-            vm.$el.querySelector('.js-reveal-variables').click();
-          })
-          .then(vm.$nextTick)
-          .then(() => {
-            expect(vm.$el.querySelector('.js-reveal-variables').textContent.trim()).toEqual(
-              'Reveal values',
-            );
+        await findRevealButton().trigger('click');
 
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain(
-              'UPLOAD_TO_GCS',
-            );
+        expect(findRevealButton().text()).toBe('Hide values');
 
-            expect(vm.$el.querySelector('.js-build-value').textContent).toContain('••••••');
-
-            expect(vm.$el.querySelector('.js-build-variables').textContent).toContain(
-              'UPLOAD_TO_S3',
-            );
-
-            expect(vm.$el.querySelector('.js-build-value').textContent).toContain('••••••');
-          })
-          .then(done)
-          .catch(done.fail);
+        expect(findVariableValue(0).text()).toBe(gcsVar.value);
+        expect(findVariableValue(1).text()).toBe(s3Var.value);
       });
     });
   });
-
-  describe('without variables', () => {
-    it('does not render variables', () => {
-      vm = mountComponent(Component, { trigger: {} });
-
-      expect(vm.$el.querySelector('.js-reveal-variables')).toBeNull();
-      expect(vm.$el.querySelector('.js-build-variables')).toBeNull();
-    });
-  });
 });