Merge branch 'follow-up-move-statuses-to-new-component' into 'master'

Follow up: Move compliance dashboard statuses to new component

See merge request gitlab-org/gitlab!37230

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/grid.vue

 <script>
 import { GlTooltipDirective } from '@gitlab/ui';
-import { isEmpty } from 'lodash';
 
 import { sprintf, __ } from '~/locale';
 import timeagoMixin from '~/vue_shared/mixins/timeago';
 
-import ApprovalStatus from './approval_status.vue';
 import Approvers from './approvers.vue';
 import BranchDetails from './branch_details.vue';
-import MergeRequest from './merge_request.vue';
-import PipelineStatus from './pipeline_status.vue';
 import GridColumnHeading from '../shared/grid_column_heading.vue';
+import MergeRequest from './merge_request.vue';
 import Pagination from '../shared/pagination.vue';
+import Status from './status.vue';
 
 export default {
   directives: {
     GlTooltip: GlTooltipDirective,
   },
   components: {
-    ApprovalStatus,
     Approvers,
     BranchDetails,
     GridColumnHeading,
     MergeRequest,
-    PipelineStatus,
     Pagination,
+    Status,
   },
   mixins: [timeagoMixin],
   props: {
@@ -50,9 +47,6 @@ export default {
     timeTooltip(mergedAt) {
       return this.tooltipTitle(mergedAt);
     },
-    hasStatus(status) {
-      return !isEmpty(status);
-    },
     hasBranchDetails(mergeRequest) {
       return mergeRequest.target_branch && mergeRequest.source_branch;
     },
@@ -86,24 +80,15 @@ export default {
           :merge-request="mergeRequest"
         />
 
-        <div
-          :key="key(mergeRequest.id, $options.keyTypes.approvalStatus)"
-          class="gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-        >
-          <approval-status
-            v-if="hasStatus(mergeRequest.approval_status)"
-            :status="mergeRequest.approval_status"
-          />
-        </div>
-        <div
-          :key="key(mergeRequest.id, $options.keyTypes.pipeline)"
-          class="dashboard-pipeline gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-        >
-          <pipeline-status
-            v-if="hasStatus(mergeRequest.pipeline_status)"
-            :status="mergeRequest.pipeline_status"
-          />
-        </div>
+        <status
+          :key="key(mergeRequest.id, 'approval')"
+          :status="{ type: 'approval', data: mergeRequest.approval_status }"
+        />
+
+        <status
+          :key="key(mergeRequest.id, 'pipeline')"
+          :status="{ type: 'pipeline', data: mergeRequest.pipeline_status }"
+        />
 
         <div
           :key="key(mergeRequest.id, $options.keyTypes.updates)"

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/status.vue

+<script>
+import { isEmpty } from 'lodash';
+
+import Approval from './statuses/approval.vue';
+import Pipeline from './statuses/pipeline.vue';
+
+export default {
+  components: {
+    Approval,
+    Pipeline,
+  },
+  props: {
+    status: {
+      type: Object,
+      required: true,
+    },
+  },
+  computed: {
+    hasData() {
+      return !isEmpty(this.status.data);
+    },
+  },
+};
+</script>
+
+<template>
+  <div
+    class="gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
+  >
+    <component :is="status.type" v-if="hasData" :status="status.data" />
+  </div>
+</template>

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/approval_status.vue → ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/statuses/approval.vue

@@ -28,13 +28,11 @@ export default {
       return `status_${this.status}`;
     },
     group() {
-      const { status } = this;
-
-      if (status === 'warning') {
+      if (this.status === 'warning') {
         return APPROVAL_WARNING_ICON;
       }
 
-      return status;
+      return this.status;
     },
   },
   tooltips: {

ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/pipeline_status.vue → ee/app/assets/javascripts/compliance_dashboard/components/merge_requests/statuses/pipeline.vue

 <script>
-import { GlTooltipDirective } from '@gitlab/ui';
+import { GlLink, GlTooltipDirective } from '@gitlab/ui';
 
 import { sprintf, s__ } from '~/locale';
 import CiIcon from '~/vue_shared/components/ci_icon.vue';
@@ -10,6 +10,7 @@ export default {
   },
   components: {
     CiIcon,
+    GlLink,
   },
   props: {
     status: {
@@ -31,7 +32,7 @@ export default {
 </script>
 
 <template>
-  <a :href="pipelineCiStatus.details_path">
+  <gl-link :href="pipelineCiStatus.details_path">
     <ci-icon v-gl-tooltip.left="pipelineTitle" class="gl-display-flex" :status="pipelineCiStatus" />
-  </a>
+  </gl-link>
 </template>

ee/spec/frontend/compliance_dashboard/components/merge_requests/__snapshots__/grid_spec.js.snap

@@ -30,17 +30,13 @@ exports[`MergeRequestsGrid component when intialized matches the snapshot 1`] =
       Merge request 0
     </div>
      
-    <div
-      class="gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-    >
-      <!---->
-    </div>
+    <status-stub
+      status="[object Object]"
+    />
      
-    <div
-      class="dashboard-pipeline gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-    >
-      <!---->
-    </div>
+    <status-stub
+      status="[object Object]"
+    />
      
     <div
       class="gl-text-right gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5 gl-relative"
@@ -65,17 +61,13 @@ exports[`MergeRequestsGrid component when intialized matches the snapshot 1`] =
       Merge request 1
     </div>
      
-    <div
-      class="gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-    >
-      <!---->
-    </div>
+    <status-stub
+      status="[object Object]"
+    />
      
-    <div
-      class="dashboard-pipeline gl-display-flex gl-align-items-center gl-justify-content-center gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5"
-    >
-      <!---->
-    </div>
+    <status-stub
+      status="[object Object]"
+    />
      
     <div
       class="gl-text-right gl-border-b-solid gl-border-b-1 gl-border-b-gray-100 gl-p-5 gl-relative"

ee/spec/frontend/compliance_dashboard/components/merge_requests/grid_spec.js

 import { shallowMount } from '@vue/test-utils';
 
-import MergeRequestsGrid from 'ee/compliance_dashboard/components/merge_requests/grid.vue';
-import ApprovalStatus from 'ee/compliance_dashboard/components/merge_requests/approval_status.vue';
-import BranchDetails from 'ee/compliance_dashboard/components/merge_requests/branch_details.vue';
-import PipelineStatus from 'ee/compliance_dashboard/components/merge_requests/pipeline_status.vue';
 import Approvers from 'ee/compliance_dashboard/components/merge_requests/approvers.vue';
-import { createMergeRequests, createPipelineStatus } from '../../mock_data';
+import BranchDetails from 'ee/compliance_dashboard/components/merge_requests/branch_details.vue';
+import MergeRequestsGrid from 'ee/compliance_dashboard/components/merge_requests/grid.vue';
+import Status from 'ee/compliance_dashboard/components/merge_requests/status.vue';
+
+import { createMergeRequests } from '../../mock_data';
 
 describe('MergeRequestsGrid component', () => {
   let wrapper;
 
   const findMergeRequests = () => wrapper.findAll('[data-testid="merge-request"]');
   const findTime = () => wrapper.find('time');
-  const findApprovalStatus = () => wrapper.find(ApprovalStatus);
-  const findPipelineStatus = () => wrapper.find(PipelineStatus);
+  const findStatuses = () => wrapper.findAll(Status);
   const findApprovers = () => wrapper.find(Approvers);
   const findBranchDetails = () => wrapper.find(BranchDetails);
 
-  const createComponent = (mergeRequestProps = {}) => {
+  const createComponent = (mergeRequests = {}) => {
     return shallowMount(MergeRequestsGrid, {
       propsData: {
-        mergeRequests: createMergeRequests({ count: 2, props: mergeRequestProps }),
+        mergeRequests,
         isLastPage: false,
       },
       stubs: {
@@ -38,7 +37,7 @@ describe('MergeRequestsGrid component', () => {
 
   describe('when intialized', () => {
     beforeEach(() => {
-      wrapper = createComponent();
+      wrapper = createComponent(createMergeRequests({ count: 2, props: {} }));
     });
 
     it('matches the snapshot', () => {
@@ -46,28 +45,28 @@ describe('MergeRequestsGrid component', () => {
     });
 
     it('renders a list of merge requests', () => {
-      expect(findMergeRequests().length).toBe(2);
+      expect(findMergeRequests()).toHaveLength(2);
     });
 
-    describe('approval status', () => {
-      it('does not render if there is no approval status', () => {
-        expect(findApprovalStatus().exists()).toBe(false);
-      });
+    it('passes the correct props to the statuses', () => {
+      const mergeRequest = createMergeRequests({ count: 1 });
+      wrapper = createComponent(mergeRequest);
 
-      it('renders if there is an approval status', () => {
-        wrapper = createComponent({ approval_status: 'success' });
-        expect(findApprovalStatus().exists()).toBe(true);
-      });
-    });
+      findStatuses().wrappers.forEach(status => {
+        const { type, data } = status.props('status');
 
-    describe('pipeline status', () => {
-      it('does not render if there is no pipeline', () => {
-        expect(findPipelineStatus().exists()).toBe(false);
-      });
+        switch (type) {
+          case 'pipeline':
+            expect(data).toEqual(mergeRequest[0].pipeline_status);
+            break;
+
+          case 'approval':
+            expect(data).toEqual(mergeRequest[0].approval_status);
+            break;
 
-      it('renders if there is a pipeline', () => {
-        wrapper = createComponent({ pipeline_status: createPipelineStatus('success') });
-        expect(findPipelineStatus().exists()).toBe(true);
+          default:
+            throw new Error('Unknown status type');
+        }
       });
     });
 
@@ -77,7 +76,12 @@ describe('MergeRequestsGrid component', () => {
       });
 
       it('renders if there are branch details', () => {
-        wrapper = createComponent({ target_branch: 'master', source_branch: 'feature' });
+        wrapper = createComponent(
+          createMergeRequests({
+            count: 2,
+            props: { target_branch: 'master', source_branch: 'feature' },
+          }),
+        );
         expect(findBranchDetails().exists()).toBe(true);
       });
     });

ee/spec/frontend/compliance_dashboard/components/merge_requests/status_spec.js

+import { shallowMount } from '@vue/test-utils';
+
+import Status from 'ee/compliance_dashboard/components/merge_requests/status.vue';
+import Approval from 'ee/compliance_dashboard/components/merge_requests/statuses/approval.vue';
+import Pipeline from 'ee/compliance_dashboard/components/merge_requests/statuses/pipeline.vue';
+
+describe('Status component', () => {
+  let wrapper;
+
+  const createComponent = status => {
+    return shallowMount(Status, {
+      propsData: { status },
+    });
+  };
+
+  const checkStatusComponentExists = (status, exists) => {
+    switch (status.type) {
+      case 'approval':
+        return expect(wrapper.find(Approval).exists()).toBe(exists);
+      case 'pipeline':
+        return expect(wrapper.find(Pipeline).exists()).toBe(exists);
+      default:
+        throw new Error(`Unknown status type: ${status.type}`);
+    }
+  };
+
+  afterEach(() => {
+    wrapper.destroy();
+  });
+
+  describe('rendering', () => {
+    it.each`
+      type          | data
+      ${'approval'} | ${null}
+      ${'approval'} | ${''}
+      ${'pipeline'} | ${{}}
+    `('does not render if given the status $value', status => {
+      wrapper = createComponent(status);
+
+      checkStatusComponentExists(status, false);
+    });
+
+    it.each`
+      type          | data
+      ${'approval'} | ${'success'}
+      ${'pipeline'} | ${{ group: 'warning' }}
+    `('renders if given the status $value', status => {
+      wrapper = createComponent(status);
+
+      checkStatusComponentExists(status, true);
+    });
+  });
+});

ee/spec/frontend/compliance_dashboard/components/merge_requests/approval_status_spec.js → ee/spec/frontend/compliance_dashboard/components/merge_requests/statuses/approval_spec.js

 import { shallowMount } from '@vue/test-utils';
 import { GlLink } from '@gitlab/ui';
 
-import ApprovalStatus from 'ee/compliance_dashboard/components/merge_requests/approval_status.vue';
+import Approval from 'ee/compliance_dashboard/components/merge_requests/statuses/approval.vue';
 
 describe('ApprovalStatus component', () => {
   let wrapper;
@@ -10,7 +10,7 @@ describe('ApprovalStatus component', () => {
   const findLink = () => wrapper.find(GlLink);
 
   const createComponent = status => {
-    return shallowMount(ApprovalStatus, {
+    return shallowMount(Approval, {
       propsData: { status },
       stubs: {
         CiIcon: {

ee/spec/frontend/compliance_dashboard/components/merge_requests/pipeline_status_spec.js → ee/spec/frontend/compliance_dashboard/components/merge_requests/statuses/pipeline_spec.js

 import { shallowMount } from '@vue/test-utils';
+import { GlLink } from '@gitlab/ui';
 
-import PipelineStatus from 'ee/compliance_dashboard/components/merge_requests/pipeline_status.vue';
-import { createPipelineStatus } from '../../mock_data';
+import Pipeline from 'ee/compliance_dashboard/components/merge_requests/statuses/pipeline.vue';
+import { createPipelineStatus } from '../../../mock_data';
 
-describe('PipelineStatus component', () => {
+describe('Pipeline component', () => {
   let wrapper;
 
   const findCiIcon = () => wrapper.find('.ci-icon');
-  const findCiLink = () => wrapper.find('a');
+  const findCiLink = () => wrapper.find(GlLink);
 
   const createComponent = status => {
-    return shallowMount(PipelineStatus, {
+    return shallowMount(Pipeline, {
       propsData: { status },
       stubs: {
         CiIcon: {

ee/spec/frontend/compliance_dashboard/mock_data.js

@@ -13,6 +13,18 @@ const createUser = id => ({
   web_url: `http://localhost:3000/user-${id}`,
 });
 
+export const createPipelineStatus = status => ({
+  details_path: '/h5bp/html5-boilerplate/pipelines/58',
+  favicon: '',
+  group: status,
+  has_details: true,
+  icon: `status_${status}`,
+  illustration: null,
+  label: status,
+  text: status,
+  tooltip: status,
+});
+
 export const createMergeRequest = ({ id = 1, props } = {}) => {
   const mergeRequest = {
     id,
@@ -22,25 +34,14 @@ export const createMergeRequest = ({ id = 1, props } = {}) => {
     milestone: null,
     path: `/h5bp/html5-boilerplate/-/merge_requests/${id}`,
     title: `Merge request ${id}`,
+    author: createUser(id),
+    pipeline_status: createPipelineStatus('success'),
+    approval_status: 'success',
   };
 
-  mergeRequest.author = createUser(id);
-
   return { ...mergeRequest, ...props };
 };
 
-export const createPipelineStatus = status => ({
-  details_path: '/h5bp/html5-boilerplate/pipelines/58',
-  favicon: '',
-  group: status,
-  has_details: true,
-  icon: `status_${status}`,
-  illustration: null,
-  label: status,
-  text: status,
-  tooltip: status,
-});
-
 export const createApprovers = count => {
   return Array(count)
     .fill()