Merge branch '299358-update-compliance-report-and-filter' into 'master'

Update compliance report and violations filter

See merge request gitlab-org/gitlab!78406

ee/app/assets/javascripts/compliance_dashboard/compliance_dashboard_bundle.js

@@ -6,7 +6,6 @@ import { queryToObject } from '~/lib/utils/url_utility';
 import resolvers from './graphql/resolvers';
 import ComplianceDashboard from './components/dashboard.vue';
 import ComplianceReport from './components/report.vue';
-import { parseViolationsQuery } from './utils';
 
 export default () => {
   const el = document.getElementById('js-compliance-report');
@@ -26,9 +25,7 @@ export default () => {
       defaultClient: createDefaultClient(resolvers),
     });
 
-    const defaultQuery = parseViolationsQuery(
-      queryToObject(window.location.search, { gatherArrays: true }),
-    );
+    const defaultQuery = queryToObject(window.location.search, { gatherArrays: true });
 
     return new Vue({
       el,
@@ -36,7 +33,6 @@ export default () => {
       render: (createElement) =>
         createElement(ComplianceReport, {
           props: {
-            emptyStateSvgPath,
             mergeCommitsCsvExportPath,
             groupPath,
             defaultQuery,

ee/app/assets/javascripts/compliance_dashboard/components/report.vue

 <script>
-import { GlAlert, GlLoadingIcon, GlTable } from '@gitlab/ui';
+import { GlAlert, GlLoadingIcon, GlTable, GlLink } from '@gitlab/ui';
 import * as Sentry from '@sentry/browser';
-import { __ } from '~/locale';
+import { s__, __ } from '~/locale';
 import { thWidthClass } from '~/lib/utils/table_utility';
 import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
 import { DRAWER_Z_INDEX } from '~/lib/utils/constants';
 import UrlSync from '~/vue_shared/components/url_sync.vue';
+import { helpPagePath } from '~/helpers/help_page_helper';
 import complianceViolationsQuery from '../graphql/compliance_violations.query.graphql';
 import { mapViolations } from '../graphql/mappers';
-import EmptyState from './empty_state.vue';
 import MergeCommitsExportButton from './merge_requests/merge_commits_export_button.vue';
 import MergeRequestDrawer from './drawer.vue';
 import ViolationReason from './violations/reason.vue';
@@ -17,10 +17,10 @@ import ViolationFilter from './violations/filter.vue';
 export default {
   name: 'ComplianceReport',
   components: {
-    EmptyState,
     GlAlert,
     GlLoadingIcon,
     GlTable,
+    GlLink,
     MergeCommitsExportButton,
     MergeRequestDrawer,
     ViolationReason,
@@ -29,10 +29,6 @@ export default {
     UrlSync,
   },
   props: {
-    emptyStateSvgPath: {
-      type: String,
-      required: true,
-    },
     mergeCommitsCsvExportPath: {
       type: String,
       required: false,
@@ -78,9 +74,6 @@ export default {
     isLoading() {
       return this.$apollo.queries.violations.loading;
     },
-    hasViolations() {
-      return this.violations.length > 0;
-    },
     hasMergeCommitsCsvExportPath() {
       return this.mergeCommitsCsvExportPath !== '';
     },
@@ -92,10 +85,10 @@ export default {
       if (!mergeRequest || (this.showDrawer && id === this.drawerMergeRequest.id)) {
         this.closeDrawer();
       } else {
-        this.openDrawer(id, mergeRequest, project);
+        this.openDrawer(mergeRequest, project);
       }
     },
-    openDrawer(id, mergeRequest, project) {
+    openDrawer(mergeRequest, project) {
       this.showDrawer = true;
       this.drawerMergeRequest = mergeRequest;
       this.drawerProject = project;
@@ -143,16 +136,22 @@ export default {
     subheading: __(
       'The compliance report shows the merge request violations merged in protected environments.',
     ),
-    queryError: __(
-      'Retrieving the compliance report failed. Please refresh the page and try again.',
-    ),
+    queryError: __('Retrieving the compliance report failed. Refresh the page and try again.'),
+    noViolationsFound: s__('ComplianceReport|No violations found'),
+    learnMore: __('Learn more.'),
   },
+  documentationPath: helpPagePath('user/compliance/compliance_report/index.md', {
+    anchor: 'approval-status-and-separation-of-duties',
+  }),
   DRAWER_Z_INDEX,
 };
 </script>
 
 <template>
   <section>
+    <gl-alert v-if="queryError" variant="danger" class="gl-mt-3" :dismissible="false">
+      {{ $options.i18n.queryError }}
+    </gl-alert>
     <header class="gl-mb-6">
       <div class="gl-mt-5 d-flex">
         <h2 class="gl-flex-grow-1 gl-my-0">{{ $options.i18n.heading }}</h2>
@@ -161,46 +160,47 @@ export default {
           :merge-commits-csv-export-path="mergeCommitsCsvExportPath"
         />
       </div>
-      <p class="gl-mt-5">{{ $options.i18n.subheading }}</p>
+      <p class="gl-mt-5" data-testid="subheading">
+        {{ $options.i18n.subheading }}
+        <gl-link :href="$options.documentationPath" target="_blank">{{
+          $options.i18n.learnMore
+        }}</gl-link>
+      </p>
     </header>
-    <gl-loading-icon v-if="isLoading" size="xl" />
-    <gl-alert
-      v-else-if="queryError"
-      variant="danger"
-      :dismissible="false"
-      :title="$options.i18n.queryError"
+    <violation-filter
+      :group-path="groupPath"
+      :default-query="defaultQuery"
+      @filters-changed="updateUrlQuery"
     />
-    <template v-else-if="hasViolations">
-      <violation-filter
-        :group-path="groupPath"
-        :default-query="defaultQuery"
-        @filters-changed="updateUrlQuery"
-      />
-      <gl-table
-        ref="table"
-        :fields="$options.fields"
-        :items="violations"
-        head-variant="white"
-        stacked="lg"
-        select-mode="single"
-        selectable
-        hover
-        selected-variant="primary"
-        thead-class="gl-border-b-solid gl-border-b-1 gl-border-b-gray-100"
-        @row-selected="toggleDrawer"
-      >
-        <template #cell(reason)="{ item: { reason, violatingUser } }">
-          <violation-reason :reason="reason" :user="violatingUser" />
-        </template>
-        <template #cell(mergeRequest)="{ item: { mergeRequest } }">
-          {{ mergeRequest.title }}
-        </template>
-        <template #cell(mergedAt)="{ item: { mergeRequest } }">
-          <time-ago-tooltip :time="mergeRequest.mergedAt" />
-        </template>
-      </gl-table>
-    </template>
-    <empty-state v-else :image-path="emptyStateSvgPath" />
+    <gl-table
+      ref="table"
+      :fields="$options.fields"
+      :items="violations"
+      :busy="isLoading"
+      :empty-text="$options.i18n.noViolationsFound"
+      :selectable="true"
+      show-empty
+      head-variant="white"
+      stacked="lg"
+      select-mode="single"
+      hover
+      selected-variant="primary"
+      thead-class="gl-border-b-solid gl-border-b-1 gl-border-b-gray-100"
+      @row-selected="toggleDrawer"
+    >
+      <template #cell(reason)="{ item: { reason, violatingUser } }">
+        <violation-reason :reason="reason" :user="violatingUser" />
+      </template>
+      <template #cell(mergeRequest)="{ item: { mergeRequest } }">
+        {{ mergeRequest.title }}
+      </template>
+      <template #cell(mergedAt)="{ item: { mergeRequest } }">
+        <time-ago-tooltip :time="mergeRequest.mergedAt" />
+      </template>
+      <template #table-busy>
+        <gl-loading-icon size="lg" color="dark" class="mt-3" />
+      </template>
+    </gl-table>
     <merge-request-drawer
       :show-drawer="showDrawer"
       :merge-request="drawerMergeRequest"

ee/app/assets/javascripts/compliance_dashboard/components/violations/filter.vue

@@ -7,6 +7,7 @@ import { getIdFromGraphQLId } from '~/graphql_shared/utils';
 import { __ } from '~/locale';
 import getGroupProjects from '../../graphql/violation_group_projects.query.graphql';
 import { CURRENT_DATE } from '../../../audit_events/constants';
+import { convertProjectIdsToGraphQl } from '../../utils';
 
 export default {
   components: {
@@ -30,7 +31,7 @@ export default {
   },
   data() {
     return {
-      filterQuery: {},
+      filterQuery: { ...this.defaultQuery },
       defaultProjects: [],
       loadingDefaultProjects: false,
     };
@@ -47,7 +48,8 @@ export default {
   },
   async created() {
     if (this.showProjectFilter && this.defaultQuery.projectIds?.length > 0) {
-      this.defaultProjects = await this.fetchProjects(this.defaultQuery.projectIds);
+      const projectIds = convertProjectIdsToGraphQl(this.defaultQuery.projectIds);
+      this.defaultProjects = await this.fetchProjects(projectIds);
     }
   },
   methods: {

ee/app/assets/javascripts/compliance_dashboard/utils.js

@@ -16,10 +16,8 @@ export const mapDashboardToDrawerData = (mergeRequest) => ({
   },
 });
 
-export const parseViolationsQuery = ({ projectIds = [], ...rest }) => ({
-  projectIds: convertToGraphQLIds(
+export const convertProjectIdsToGraphQl = (projectIds) =>
+  convertToGraphQLIds(
     TYPE_PROJECT,
     projectIds.filter((id) => Boolean(id)),
-  ),
-  ...rest,
-});
+  );

ee/spec/frontend/compliance_dashboard/components/report_spec.js

-import { GlAlert, GlLoadingIcon, GlTable } from '@gitlab/ui';
+import { GlAlert, GlLoadingIcon, GlTable, GlLink } from '@gitlab/ui';
 import { mount, shallowMount } from '@vue/test-utils';
 import VueApollo from 'vue-apollo';
 import Vue, { nextTick } from 'vue';
 import * as Sentry from '@sentry/browser';
+import { extendedWrapper } from 'helpers/vue_test_utils_helper';
 import ComplianceReport from 'ee/compliance_dashboard/components/report.vue';
-import EmptyState from 'ee/compliance_dashboard/components/empty_state.vue';
 import MergeRequestDrawer from 'ee/compliance_dashboard/components/drawer.vue';
 import MergeCommitsExportButton from 'ee/compliance_dashboard/components/merge_requests/merge_commits_export_button.vue';
 import ViolationReason from 'ee/compliance_dashboard/components/violations/reason.vue';
@@ -25,7 +25,6 @@ describe('ComplianceReport component', () => {
   let mockResolver;
 
   const mergeCommitsCsvExportPath = '/csv';
-  const emptyStateSvgPath = 'empty.svg';
   const groupPath = 'group-path';
   const defaultQuery = {
     projectIds: ['gid://gitlab/Project/20'],
@@ -34,11 +33,11 @@ describe('ComplianceReport component', () => {
   };
   const mockGraphQlError = new Error('GraphQL networkError');
 
-  const findLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
+  const findSubheading = () => wrapper.findByTestId('subheading');
   const findErrorMessage = () => wrapper.findComponent(GlAlert);
   const findViolationsTable = () => wrapper.findComponent(GlTable);
+  const findTableLoadingIcon = () => wrapper.findComponent(GlLoadingIcon);
   const findMergeRequestDrawer = () => wrapper.findComponent(MergeRequestDrawer);
-  const findEmptyState = () => wrapper.findComponent(EmptyState);
   const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton);
   const findViolationReason = () => wrapper.findComponent(ViolationReason);
   const findTimeAgoTooltip = () => wrapper.findComponent(TimeAgoTooltip);
@@ -60,20 +59,22 @@ describe('ComplianceReport component', () => {
   }
 
   const createComponent = (mountFn = shallowMount, props = {}) => {
-    return mountFn(ComplianceReport, {
-      apolloProvider: createMockApolloProvider(),
-      propsData: {
-        mergeCommitsCsvExportPath,
-        emptyStateSvgPath,
-        groupPath,
-        defaultQuery,
-        ...props,
-      },
-      stubs: {
-        GlTable: false,
-        ViolationFilter: stubComponent(ViolationFilter),
-      },
-    });
+    return extendedWrapper(
+      mountFn(ComplianceReport, {
+        apolloProvider: createMockApolloProvider(),
+        propsData: {
+          mergeCommitsCsvExportPath,
+          groupPath,
+          defaultQuery,
+          ...props,
+        },
+        stubs: {
+          GlLink,
+          GlTable: false,
+          ViolationFilter: stubComponent(ViolationFilter),
+        },
+      }),
+    );
   };
 
   afterEach(() => {
@@ -81,16 +82,40 @@ describe('ComplianceReport component', () => {
     mockResolver = null;
   });
 
-  describe('loading', () => {
+  describe('default behavior', () => {
     beforeEach(() => {
       wrapper = createComponent();
     });
 
-    it('renders the loading icon', () => {
-      expect(findLoadingIcon().exists()).toBe(true);
+    it('renders the subheading with a help link', () => {
+      const helpLink = findSubheading().find(GlLink);
+
+      expect(findSubheading().text()).toContain(
+        'The compliance report shows the merge request violations merged in protected environments.',
+      );
+      expect(helpLink.text()).toBe('Learn more.');
+      expect(helpLink.attributes('href')).toBe(
+        '/help/user/compliance/compliance_report/index.md#approval-status-and-separation-of-duties',
+      );
+    });
+
+    it('renders the merge commit export button', () => {
+      expect(findMergeCommitsExportButton().exists()).toBe(true);
+    });
+
+    it('does not render an error message', () => {
       expect(findErrorMessage().exists()).toBe(false);
-      expect(findViolationsTable().exists()).toBe(false);
-      expect(findViolationFilter().exists()).toBe(false);
+    });
+  });
+
+  describe('when initializing', () => {
+    beforeEach(() => {
+      wrapper = createComponent(mount);
+    });
+
+    it('renders the table loading icon', () => {
+      expect(findViolationsTable().exists()).toBe(true);
+      expect(findTableLoadingIcon().exists()).toBe(true);
     });
   });
 
@@ -104,10 +129,9 @@ describe('ComplianceReport component', () => {
     it('renders the error message', async () => {
       await waitForPromises();
 
-      expect(findLoadingIcon().exists()).toBe(false);
       expect(findErrorMessage().exists()).toBe(true);
-      expect(findErrorMessage().props('title')).toBe(
-        'Retrieving the compliance report failed. Please refresh the page and try again.',
+      expect(findErrorMessage().text()).toBe(
+        'Retrieving the compliance report failed. Refresh the page and try again.',
       );
       expect(Sentry.captureException.mock.calls[0][0].networkError).toBe(mockGraphQlError);
     });
@@ -121,14 +145,8 @@ describe('ComplianceReport component', () => {
       return waitForPromises();
     });
 
-    it('renders the merge commit export button', () => {
-      expect(findMergeCommitsExportButton().exists()).toBe(true);
-    });
-
-    it('renders the violations table', async () => {
-      expect(findLoadingIcon().exists()).toBe(false);
-      expect(findErrorMessage().exists()).toBe(false);
-      expect(findViolationsTable().exists()).toBe(true);
+    it('does not render the table loading icon', () => {
+      expect(findTableLoadingIcon().exists()).toBe(false);
     });
 
     it('has the correct table headers', () => {
@@ -261,18 +279,13 @@ describe('ComplianceReport component', () => {
           nodes: [],
         },
       });
-      wrapper = createComponent();
+      wrapper = createComponent(mount);
 
       return waitForPromises();
     });
 
-    it('does not render the violations table', () => {
-      expect(findViolationsTable().exists()).toBe(false);
-    });
-
-    it('renders the empty state', () => {
-      expect(findEmptyState().exists()).toBe(true);
-      expect(findEmptyState().props('imagePath')).toBe(emptyStateSvgPath);
+    it('renders the empty table message', () => {
+      expect(findViolationsTable().text()).toContain('No violations found');
     });
   });
 

ee/spec/frontend/compliance_dashboard/components/utils_spec.js

 import * as utils from 'ee/compliance_dashboard/utils';
 
 describe('compliance report utils', () => {
-  describe('parseViolationsQuery', () => {
+  describe('convertProjectIdsToGraphQl', () => {
     it('returns the expected result', () => {
-      const query = {
-        projectIds: ['1', '2'],
-        createdAfter: '2021-12-06',
-        createdBefore: '2022-01-06',
-      };
-
-      expect(utils.parseViolationsQuery(query)).toStrictEqual({
-        projectIds: ['gid://gitlab/Project/1', 'gid://gitlab/Project/2'],
-        createdAfter: query.createdAfter,
-        createdBefore: query.createdBefore,
-      });
+      expect(utils.convertProjectIdsToGraphQl(['1', '2'])).toStrictEqual([
+        'gid://gitlab/Project/1',
+        'gid://gitlab/Project/2',
+      ]);
     });
   });
 });

ee/spec/frontend/compliance_dashboard/components/violations/filter_spec.js

@@ -3,6 +3,7 @@ import Vue from 'vue';
 import { GlDaterangePicker } from '@gitlab/ui';
 import { shallowMountExtended } from 'helpers/vue_test_utils_helper';
 import ViolationFilter from 'ee/compliance_dashboard/components/violations/filter.vue';
+import { convertProjectIdsToGraphQl } from 'ee/compliance_dashboard/utils';
 import ProjectsDropdownFilter from '~/analytics/shared/components/projects_dropdown_filter.vue';
 import { getDateInPast, pikadayToString } from '~/lib/utils/datetime_utility';
 import { CURRENT_DATE } from 'ee/audit_events/constants';
@@ -117,21 +118,31 @@ describe('ViolationFilter component', () => {
       expect(wrapper.emitted('filters-changed')[0]).toStrictEqual([{ ...dateRangeQuery }]);
     });
 
-    it('emits the existing filter query with mutations on each update', async () => {
-      await findProjectsFilter().vm.$emit('selected', []);
+    describe('with a default query', () => {
+      const defaultQuery = { projectIds, createdAfter: '2022-01-01', createdBefore: '2022-01-31' };
 
-      expect(wrapper.emitted('filters-changed')).toHaveLength(1);
-      expect(wrapper.emitted('filters-changed')[0]).toStrictEqual([{ projectIds: [] }]);
+      beforeEach(() => {
+        createComponent({ defaultQuery });
+      });
 
-      await findDatePicker().vm.$emit('input', { startDate, endDate });
+      it('emits the existing filter query with mutations on each update', async () => {
+        await findProjectsFilter().vm.$emit('selected', []);
+
+        expect(wrapper.emitted('filters-changed')).toHaveLength(1);
+        expect(wrapper.emitted('filters-changed')[0]).toStrictEqual([
+          { ...defaultQuery, projectIds: [] },
+        ]);
 
-      expect(wrapper.emitted('filters-changed')).toHaveLength(2);
-      expect(wrapper.emitted('filters-changed')[1]).toStrictEqual([
-        {
-          projectIds: [],
-          ...dateRangeQuery,
-        },
-      ]);
+        await findDatePicker().vm.$emit('input', { startDate, endDate });
+
+        expect(wrapper.emitted('filters-changed')).toHaveLength(2);
+        expect(wrapper.emitted('filters-changed')[1]).toStrictEqual([
+          {
+            projectIds: [],
+            ...dateRangeQuery,
+          },
+        ]);
+      });
     });
   });
 
@@ -139,7 +150,10 @@ describe('ViolationFilter component', () => {
     it('fetches the project details when the default query contains projectIds', () => {
       createComponent({ defaultQuery: { projectIds } });
 
-      expect(groupProjectsSuccess).toHaveBeenCalledWith({ groupPath, projectIds });
+      expect(groupProjectsSuccess).toHaveBeenCalledWith({
+        groupPath,
+        projectIds: convertProjectIdsToGraphQl(projectIds),
+      });
     });
 
     describe('when the defaultProjects are being fetched', () => {

locale/gitlab.pot

@@ -8962,6 +8962,9 @@ msgstr ""
 msgid "ComplianceReport|Less than 2 approvers"
 msgstr ""
 
+msgid "ComplianceReport|No violations found"
+msgstr ""
+
 msgid "Component"
 msgstr ""
 
@@ -30449,7 +30452,7 @@ msgstr ""
 msgid "Resync"
 msgstr ""
 
-msgid "Retrieving the compliance report failed. Please refresh the page and try again."
+msgid "Retrieving the compliance report failed. Refresh the page and try again."
 msgstr ""
 
 msgid "Retry"