Commit a7d02e19 authored by Robert Hunt's avatar Robert Hunt

Initial review MR feedback

- Updated approval status to be easier to read
- Updated tests to check each status for the correct values
parent 281eaa64
...@@ -4,6 +4,8 @@ import { GlTooltipDirective } from '@gitlab/ui'; ...@@ -4,6 +4,8 @@ import { GlTooltipDirective } from '@gitlab/ui';
import { s__ } from '~/locale'; import { s__ } from '~/locale';
import CiIcon from '~/vue_shared/components/ci_icon.vue'; import CiIcon from '~/vue_shared/components/ci_icon.vue';
const APPROVAL_WARNING_ICON = 'success-with-warnings';
export default { export default {
directives: { directives: {
GlTooltip: GlTooltipDirective, GlTooltip: GlTooltipDirective,
...@@ -21,22 +23,17 @@ export default { ...@@ -21,22 +23,17 @@ export default {
tooltip() { tooltip() {
return this.$options.tooltips[this.status]; return this.$options.tooltips[this.status];
}, },
iconName() { icon() {
return `status_${this.status}`; return `status_${this.status}`;
}, },
iconStatus() { group() {
const { status, iconName: icon } = this; const { status } = this;
let group = status;
// Need to set this to be the group for warnings so the correct icon color fill is used if (status === 'warning') {
if (group === 'warning') { return APPROVAL_WARNING_ICON;
group = 'success-with-warnings';
} }
return { return status;
group,
icon,
};
}, },
}, },
tooltips: { tooltips: {
...@@ -51,6 +48,6 @@ export default { ...@@ -51,6 +48,6 @@ export default {
<a <a
href="https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties" href="https://docs.gitlab.com/ee/user/compliance/compliance_dashboard/#approval-status-and-separation-of-duties"
> >
<ci-icon v-gl-tooltip.left="tooltip" class="gl-display-flex" :status="iconStatus" /> <ci-icon v-gl-tooltip.left="tooltip" class="gl-display-flex" :status="{ icon, group }" />
</a> </a>
</template> </template>
...@@ -41,29 +41,26 @@ describe('ApprovalStatus component', () => { ...@@ -41,29 +41,26 @@ describe('ApprovalStatus component', () => {
expect(findIcon().text()).toEqual(`status_${approvalStatus}`); expect(findIcon().text()).toEqual(`status_${approvalStatus}`);
}); });
it.each` describe.each`
status | tooltip status | icon | group | tooltip
${'success'} | ${'Adheres to separation of duties'} ${'success'} | ${'status_success'} | ${'success'} | ${'Adheres to separation of duties'}
${'warning'} | ${'At least one rule does not adhere to separation of duties'} ${'warning'} | ${'status_warning'} | ${'success-with-warnings'} | ${'At least one rule does not adhere to separation of duties'}
${'failed'} | ${'Fails to adhere to separation of duties'} ${'failed'} | ${'status_failed'} | ${'failed'} | ${'Fails to adhere to separation of duties'}
`('shows the correct tooltip for $status', ({ status, tooltip }) => { `('returns the correct values for $status', ({ status, icon, group, tooltip }) => {
beforeEach(() => {
wrapper = createComponent(status); wrapper = createComponent(status);
expect(wrapper.vm.tooltip).toEqual(tooltip);
});
}); });
describe('with a warning approval status', () => { it('returns the correct icon', () => {
const approvalStatus = 'warning'; expect(wrapper.vm.icon).toEqual(icon);
});
beforeEach(() => { it('returns the correct group', () => {
wrapper = createComponent(approvalStatus); expect(wrapper.vm.group).toEqual(group);
}); });
it('returns the correct status object`', () => { it('returns the correct tooltip', () => {
expect(wrapper.vm.iconStatus).toEqual({ expect(wrapper.vm.tooltip).toEqual(tooltip);
group: 'success-with-warnings',
icon: 'status_warning',
}); });
}); });
}); });
......
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