Commit fcb32a9e authored by Jiaan Louw's avatar Jiaan Louw Committed by Savas Vedova

Apply reviewer feedback

- Create a merge request violation severity component
- Update specs
parent eeceb35f
...@@ -7,6 +7,7 @@ import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue'; ...@@ -7,6 +7,7 @@ import TimeAgoTooltip from '~/vue_shared/components/time_ago_tooltip.vue';
import { DRAWER_Z_INDEX } from '~/lib/utils/constants'; import { DRAWER_Z_INDEX } from '~/lib/utils/constants';
import UrlSync from '~/vue_shared/components/url_sync.vue'; import UrlSync from '~/vue_shared/components/url_sync.vue';
import { helpPagePath } from '~/helpers/help_page_helper'; import { helpPagePath } from '~/helpers/help_page_helper';
import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
import complianceViolationsQuery from '../graphql/compliance_violations.query.graphql'; import complianceViolationsQuery from '../graphql/compliance_violations.query.graphql';
import { mapViolations } from '../graphql/mappers'; import { mapViolations } from '../graphql/mappers';
import { parseViolationsQueryFilter } from '../utils'; import { parseViolationsQueryFilter } from '../utils';
...@@ -25,6 +26,7 @@ export default { ...@@ -25,6 +26,7 @@ export default {
MergeCommitsExportButton, MergeCommitsExportButton,
MergeRequestDrawer, MergeRequestDrawer,
ViolationReason, ViolationReason,
SeverityBadge,
TimeAgoTooltip, TimeAgoTooltip,
ViolationFilter, ViolationFilter,
UrlSync, UrlSync,
...@@ -190,6 +192,9 @@ export default { ...@@ -190,6 +192,9 @@ export default {
thead-class="gl-border-b-solid gl-border-b-1 gl-border-b-gray-100" thead-class="gl-border-b-solid gl-border-b-1 gl-border-b-gray-100"
@row-selected="toggleDrawer" @row-selected="toggleDrawer"
> >
<template #cell(severity)="{ item: { severity } }">
<severity-badge :severity="severity" />
</template>
<template #cell(reason)="{ item: { reason, violatingUser } }"> <template #cell(reason)="{ item: { reason, violatingUser } }">
<violation-reason :reason="reason" :user="violatingUser" /> <violation-reason :reason="reason" :user="violatingUser" />
</template> </template>
......
...@@ -29,7 +29,7 @@ export default { ...@@ -29,7 +29,7 @@ export default {
</script> </script>
<template> <template>
<div class="gl-display-flex gl-align-items-center"> <div class="gl-display-inline-flex gl-align-items-center">
<span class="gl-mr-2">{{ violationMessage }}</span> <span class="gl-mr-2">{{ violationMessage }}</span>
<user-avatar v-if="user" :user="user" /> <user-avatar v-if="user" :user="user" />
</div> </div>
......
...@@ -29,3 +29,10 @@ export const MERGE_REQUEST_VIOLATION_MESSAGES = { ...@@ -29,3 +29,10 @@ export const MERGE_REQUEST_VIOLATION_MESSAGES = {
[VIOLATION_TYPE_APPROVED_BY_COMMITTER]: s__('ComplianceReport|Approved by committer'), [VIOLATION_TYPE_APPROVED_BY_COMMITTER]: s__('ComplianceReport|Approved by committer'),
[VIOLATION_TYPE_APPROVED_BY_INSUFFICIENT_USERS]: s__('ComplianceReport|Less than 2 approvers'), [VIOLATION_TYPE_APPROVED_BY_INSUFFICIENT_USERS]: s__('ComplianceReport|Less than 2 approvers'),
}; };
export const MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS = {
1: 'high',
2: 'medium',
3: 'low',
4: 'info',
};
import { MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS } from '../constants';
export const mapViolations = (nodes = []) => { export const mapViolations = (nodes = []) => {
return nodes.map((node) => ({ return nodes.map((node) => ({
...node, ...node,
...@@ -11,5 +13,6 @@ export const mapViolations = (nodes = []) => { ...@@ -11,5 +13,6 @@ export const mapViolations = (nodes = []) => {
...node.project, ...node.project,
complianceFramework: node.project?.complianceFrameworks?.nodes[0] || null, complianceFramework: node.project?.complianceFrameworks?.nodes[0] || null,
}, },
severity: MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS[node.severity],
})); }));
}; };
...@@ -9,6 +9,7 @@ import MergeRequestDrawer from 'ee/compliance_dashboard/components/drawer.vue'; ...@@ -9,6 +9,7 @@ import MergeRequestDrawer from 'ee/compliance_dashboard/components/drawer.vue';
import MergeCommitsExportButton from 'ee/compliance_dashboard/components/merge_requests/merge_commits_export_button.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'; import ViolationReason from 'ee/compliance_dashboard/components/violations/reason.vue';
import ViolationFilter from 'ee/compliance_dashboard/components/violations/filter.vue'; import ViolationFilter from 'ee/compliance_dashboard/components/violations/filter.vue';
import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
import resolvers from 'ee/compliance_dashboard/graphql/resolvers'; import resolvers from 'ee/compliance_dashboard/graphql/resolvers';
import { mapViolations } from 'ee/compliance_dashboard/graphql/mappers'; import { mapViolations } from 'ee/compliance_dashboard/graphql/mappers';
import { stripTypenames } from 'helpers/graphql_helpers'; import { stripTypenames } from 'helpers/graphql_helpers';
...@@ -43,6 +44,7 @@ describe('ComplianceReport component', () => { ...@@ -43,6 +44,7 @@ describe('ComplianceReport component', () => {
const findMergeRequestDrawer = () => wrapper.findComponent(MergeRequestDrawer); const findMergeRequestDrawer = () => wrapper.findComponent(MergeRequestDrawer);
const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton); const findMergeCommitsExportButton = () => wrapper.findComponent(MergeCommitsExportButton);
const findViolationReason = () => wrapper.findComponent(ViolationReason); const findViolationReason = () => wrapper.findComponent(ViolationReason);
const findSeverityBadge = () => wrapper.findComponent(SeverityBadge);
const findTimeAgoTooltip = () => wrapper.findComponent(TimeAgoTooltip); const findTimeAgoTooltip = () => wrapper.findComponent(TimeAgoTooltip);
const findViolationFilter = () => wrapper.findComponent(ViolationFilter); const findViolationFilter = () => wrapper.findComponent(ViolationFilter);
const findUrlSync = () => wrapper.findComponent(UrlSync); const findUrlSync = () => wrapper.findComponent(UrlSync);
...@@ -176,19 +178,23 @@ describe('ComplianceReport component', () => { ...@@ -176,19 +178,23 @@ describe('ComplianceReport component', () => {
expect(headerTexts).toStrictEqual(['Severity', 'Violation', 'Merge request', 'Date merged']); expect(headerTexts).toStrictEqual(['Severity', 'Violation', 'Merge request', 'Date merged']);
}); });
// Note: This should be refactored as each table component is created
// Severity: https://gitlab.com/gitlab-org/gitlab/-/issues/342900
it('has the correct first row data', () => { it('has the correct first row data', () => {
const headerTexts = findTablesFirstRowData().wrappers.map((d) => d.text()); const headerTexts = findTablesFirstRowData().wrappers.map((d) => d.text());
expect(headerTexts).toEqual([ expect(headerTexts).toEqual([
'1', 'High',
'Approved by committer', 'Approved by committer',
'Officiis architecto voluptas ut sit qui qui quisquam sequi consectetur porro.', 'Officiis architecto voluptas ut sit qui qui quisquam sequi consectetur porro.',
'in 1 year', 'in 1 year',
]); ]);
}); });
it('renders the violation severity badge', () => {
const { severity } = mapViolations(mockResolver().mergeRequestViolations.nodes)[0];
expect(findSeverityBadge().props()).toStrictEqual({ severity });
});
it('renders the violation reason', () => { it('renders the violation reason', () => {
const { const {
violatingUser: { __typename, ...user }, violatingUser: { __typename, ...user },
......
import { mapViolations } from 'ee/compliance_dashboard/graphql/mappers';
import resolvers from 'ee/compliance_dashboard/graphql/resolvers';
import { MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS } from 'ee/compliance_dashboard/constants';
describe('mapViolations', () => {
const mockViolations = resolvers.Query.group().mergeRequestViolations.nodes;
const severityLevels = Object.keys(MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS).map(Number);
it.each(severityLevels)(
'maps to the expected severity level when the violation severity number is %s',
(severity) => {
const { severity: severityLevel } = mapViolations([{ ...mockViolations[0], severity }])[0];
expect(severityLevel).toBe(MERGE_REQUEST_VIOLATION_SEVERITY_LEVELS[severity]);
},
);
});
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment