Commit 99f4a779 authored by Alexander Turinske's avatar Alexander Turinske

Clean up code

- do not render columns if conditionally turned off
- do not rely on bootstrap implementation to access table cells
- clean up tests with before each
parent cdd10bf7
...@@ -75,23 +75,12 @@ export default { ...@@ -75,23 +75,12 @@ export default {
shouldShowSelectionSummary() { shouldShowSelectionSummary() {
return this.shouldShowSelection && Boolean(this.numOfSelectedVulnerabilities); return this.shouldShowSelection && Boolean(this.numOfSelectedVulnerabilities);
}, },
checkboxClass() {
return this.shouldShowSelection ? '' : 'gl-display-none';
},
theadClass() { theadClass() {
return this.shouldShowSelectionSummary ? 'below-selection-summary' : ''; return this.shouldShowSelectionSummary ? 'below-selection-summary' : '';
}, },
reportTypeClass() {
return this.shouldShowReportType ? '' : 'gl-display-none';
},
fields() { fields() {
const commonThClass = ['table-th-transparent', 'original-gl-th', 'gl-bg-white!'].join(' '); const commonThClass = ['table-th-transparent', 'original-gl-th', 'gl-bg-white!'].join(' ');
return [ const baseFields = [
{
key: 'checkbox',
class: this.checkboxClass,
thClass: `gl-w-9 ${commonThClass}`,
},
{ {
key: 'state', key: 'state',
label: s__('Vulnerability|Status'), label: s__('Vulnerability|Status'),
...@@ -108,13 +97,23 @@ export default { ...@@ -108,13 +97,23 @@ export default {
thClass: commonThClass, thClass: commonThClass,
tdClass: 'gl-word-break-all', tdClass: 'gl-word-break-all',
}, },
{ ];
if (this.shouldShowSelection) {
baseFields.unshift({
key: 'checkbox',
thClass: `gl-w-9 ${commonThClass}`,
});
}
if (this.shouldShowReportType) {
baseFields.push({
key: 'reportType', key: 'reportType',
class: this.reportTypeClass,
label: s__('Reports|Scanner'), label: s__('Reports|Scanner'),
thClass: commonThClass, thClass: commonThClass,
}, });
]; }
return baseFields;
}, },
}, },
watch: { watch: {
...@@ -194,6 +193,7 @@ export default { ...@@ -194,6 +193,7 @@ export default {
<template #head(checkbox)> <template #head(checkbox)>
<gl-form-checkbox <gl-form-checkbox
class="mr-0 mb-0" class="mr-0 mb-0"
data-testid="vulnerability-checkbox-all"
:checked="hasSelectedAllVulnerabilities" :checked="hasSelectedAllVulnerabilities"
@change="toggleAllVulnerabilities" @change="toggleAllVulnerabilities"
/> />
...@@ -202,6 +202,7 @@ export default { ...@@ -202,6 +202,7 @@ export default {
<template #cell(checkbox)="{ item }"> <template #cell(checkbox)="{ item }">
<gl-form-checkbox <gl-form-checkbox
class="d-inline-block mr-0 mb-0" class="d-inline-block mr-0 mb-0"
data-testid="vulnerability-checkbox"
:checked="isSelected(item)" :checked="isSelected(item)"
@change="toggleVulnerability(item)" @change="toggleVulnerability(item)"
/> />
...@@ -243,7 +244,7 @@ export default { ...@@ -243,7 +244,7 @@ export default {
</template> </template>
<template #cell(reportType)="{ item }"> <template #cell(reportType)="{ item }">
<span class="text-capitalize js-reportType">{{ <span data-testid="vulnerability-reportType" class="text-capitalize js-reportType">{{
useConvertReportType(item.reportType) useConvertReportType(item.reportType)
}}</span> }}</span>
</template> </template>
......
...@@ -46,17 +46,21 @@ describe('security reports utils', () => { ...@@ -46,17 +46,21 @@ describe('security reports utils', () => {
}); });
describe('convertReportType', () => { describe('convertReportType', () => {
it.each([ it.each`
['sast', 'SAST'], reportType | output
['dependency_scanning', 'Dependency Scanning'], ${'sast'} | ${'SAST'}
['CONTAINER_SCANNING', 'Container Scanning'], ${'dependency_scanning'} | ${'Dependency Scanning'}
['CUSTOM_SCANNER', 'Custom scanner'], ${'CONTAINER_SCANNING'} | ${'Container Scanning'}
['mast', 'Mast'], ${'CUSTOM_SCANNER'} | ${'Custom scanner'}
['TAST', 'Tast'], ${'mast'} | ${'Mast'}
[undefined, ''], ${'TAST'} | ${'Tast'}
])('converts the report type "%s" to the human-readable string "%s"', (input, output) => { ${undefined} | ${''}
expect(convertReportType(input)).toEqual(output); `(
}); 'converts the report type "$reportType" to the human-readable string "$output"',
({ reportType, output }) => {
expect(convertReportType(reportType)).toEqual(output);
},
);
}); });
describe('filterByKey', () => { describe('filterByKey', () => {
......
...@@ -32,8 +32,8 @@ describe('Vulnerability list component', () => { ...@@ -32,8 +32,8 @@ describe('Vulnerability list component', () => {
const findCells = label => wrapper.findAll(`.js-${label}`); const findCells = label => wrapper.findAll(`.js-${label}`);
const findRow = (index = 0) => wrapper.findAll('tbody tr').at(index); const findRow = (index = 0) => wrapper.findAll('tbody tr').at(index);
const findSelectionSummary = () => wrapper.find(SelectionSummary); const findSelectionSummary = () => wrapper.find(SelectionSummary);
const findCheckAllCheckboxCell = () => wrapper.find('thead tr th'); const findCheckAllCheckboxCell = () => wrapper.find('[data-testid="vulnerability-checkbox-all"]');
const findFirstCheckboxCell = () => wrapper.find('tbody tr td'); const findFirstCheckboxCell = () => wrapper.find('[data-testid="vulnerability-checkbox"]');
const findLocation = id => wrapper.find(`[data-testid="location-${id}"]`); const findLocation = id => wrapper.find(`[data-testid="location-${id}"]`);
afterEach(() => { afterEach(() => {
...@@ -75,9 +75,7 @@ describe('Vulnerability list component', () => { ...@@ -75,9 +75,7 @@ describe('Vulnerability list component', () => {
}); });
it('should show the selection summary when a checkbox is selected', () => { it('should show the selection summary when a checkbox is selected', () => {
findFirstCheckboxCell() findFirstCheckboxCell().setChecked(true);
.find('input')
.setChecked(true);
return wrapper.vm.$nextTick().then(() => { return wrapper.vm.$nextTick().then(() => {
expect(findSelectionSummary().exists()).toBe(true); expect(findSelectionSummary().exists()).toBe(true);
...@@ -85,9 +83,7 @@ describe('Vulnerability list component', () => { ...@@ -85,9 +83,7 @@ describe('Vulnerability list component', () => {
}); });
it('should sync selected vulnerabilities when the vulnerability list is updated', () => { it('should sync selected vulnerabilities when the vulnerability list is updated', () => {
findFirstCheckboxCell() findFirstCheckboxCell().setChecked(true);
.find('input')
.setChecked(true);
expect(findSelectionSummary().props('selectedVulnerabilities')).toHaveLength(1); expect(findSelectionSummary().props('selectedVulnerabilities')).toHaveLength(1);
wrapper.setProps({ vulnerabilities: [] }); wrapper.setProps({ vulnerabilities: [] });
...@@ -102,24 +98,25 @@ describe('Vulnerability list component', () => { ...@@ -102,24 +98,25 @@ describe('Vulnerability list component', () => {
wrapper = createWrapper({ wrapper = createWrapper({
props: { vulnerabilities, shouldShowSelection: false }, props: { vulnerabilities, shouldShowSelection: false },
}); });
findFirstCheckboxCell()
.find('input')
.setChecked(true);
}); });
it('should not show the checkboxes if shouldShowSelection is passed in', () => { it('should not show the checkboxes if shouldShowSelection is passed in', () => {
expect(findCheckAllCheckboxCell().classes()).toContain('gl-display-none'); expect(findCheckAllCheckboxCell().exists()).toBe(false);
expect(findFirstCheckboxCell().classes()).toContain('gl-display-none'); expect(findFirstCheckboxCell().exists()).toBe(false);
}); });
}); });
describe('when displayed on instance or group level dashboard', () => { describe('when displayed on instance or group level dashboard', () => {
it('should display the vulnerability locations', () => { let newVulnerabilities;
const newVulnerabilities = generateVulnerabilities();
beforeEach(() => {
newVulnerabilities = generateVulnerabilities();
wrapper = createWrapper({ wrapper = createWrapper({
props: { vulnerabilities: newVulnerabilities, shouldShowProjectNamespace: true }, props: { vulnerabilities: newVulnerabilities, shouldShowProjectNamespace: true },
}); });
});
it('should display the vulnerability locations', () => {
expect(findLocation(newVulnerabilities[0].id).text()).toContain( expect(findLocation(newVulnerabilities[0].id).text()).toContain(
'Administrator / Security reports', 'Administrator / Security reports',
); );
...@@ -134,6 +131,11 @@ describe('Vulnerability list component', () => { ...@@ -134,6 +131,11 @@ describe('Vulnerability list component', () => {
); );
}); });
it('should not display the vulnerability report type', () => {
const scannerCell = findRow().find('[data-testid="vulnerability-reportType"');
expect(scannerCell.exists()).toBe(false);
});
it('should not display the vulnerability locations', () => { it('should not display the vulnerability locations', () => {
const vulnerabilityWithoutLocation = [ const vulnerabilityWithoutLocation = [
{ {
...@@ -155,15 +157,6 @@ describe('Vulnerability list component', () => { ...@@ -155,15 +157,6 @@ describe('Vulnerability list component', () => {
); );
expect(findLocation(vulnerabilityWithoutLocation[0].id).findAll('div').length).toBe(2); expect(findLocation(vulnerabilityWithoutLocation[0].id).findAll('div').length).toBe(2);
}); });
it('should not display the vulnerability report type', () => {
const newVulnerabilities = generateVulnerabilities();
wrapper = createWrapper({
props: { vulnerabilities: newVulnerabilities, shouldShowProjectNamespace: true },
});
const scannerCell = findRow().find('[data-label="Scanner"');
expect(scannerCell.classes('gl-display-none')).toBe(true);
});
}); });
describe('when displayed on a project level dashboard', () => { describe('when displayed on a project level dashboard', () => {
...@@ -192,10 +185,10 @@ describe('Vulnerability list component', () => { ...@@ -192,10 +185,10 @@ describe('Vulnerability list component', () => {
it('should display the vulnerability report type', () => { it('should display the vulnerability report type', () => {
const cells = findCells('reportType'); const cells = findCells('reportType');
expect(cells.wrappers[0].text()).toBe('SAST'); expect(cells.at(0).text()).toBe('SAST');
expect(cells.wrappers[1].text()).toBe('Dependency Scanning'); expect(cells.at(1).text()).toBe('Dependency Scanning');
expect(cells.wrappers[2].text()).toBe('Custom scanner without translation'); expect(cells.at(2).text()).toBe('Custom scanner without translation');
expect(cells.wrappers[3].text()).toBe(''); expect(cells.at(3).text()).toBe('');
}); });
}); });
......
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