Commit 03b4f3cb authored by Savas Vedova's avatar Savas Vedova

Address code review changes

- Update specs
- Improve fileUrl function
parent 40c31a56
...@@ -249,9 +249,14 @@ export default { ...@@ -249,9 +249,14 @@ export default {
return identifiers?.length - 1; return identifiers?.length - 1;
}, },
fileUrl(vulnerability) { fileUrl(vulnerability) {
const { startLine: start, endLine: end } = vulnerability.location; const { startLine: start, endLine: end, blobPath } = vulnerability.location;
const lineNumber = end > start ? `${start}-${end}` : start; const lineNumber = end > start ? `${start}-${end}` : start;
return (vulnerability.location.blobPath || '') + (lineNumber ? `#L${lineNumber}` : '');
if (!blobPath) {
return '';
}
return `${blobPath}${lineNumber ? `#L${lineNumber}` : ''}`;
}, },
primaryIdentifier(identifiers) { primaryIdentifier(identifiers) {
return getPrimaryIdentifier(identifiers, 'externalType'); return getPrimaryIdentifier(identifiers, 'externalType');
...@@ -427,9 +432,10 @@ export default { ...@@ -427,9 +432,10 @@ export default {
{{ item.project.nameWithNamespace }} {{ item.project.nameWithNamespace }}
</div> </div>
<div v-if="shouldShowVulnerabilityPath(item)"> <div v-if="shouldShowVulnerabilityPath(item)">
<gl-link :href="fileUrl(item)"> <gl-link v-if="item.location.blobPath" :href="fileUrl(item)">
<gl-truncate :text="createLocationString(item.location)" position="middle" /> <gl-truncate :text="createLocationString(item.location)" position="middle" />
</gl-link> </gl-link>
<gl-truncate v-else :text="createLocationString(item.location)" position="middle" />
</div> </div>
</div> </div>
</template> </template>
......
...@@ -46,6 +46,7 @@ describe('Vulnerability list component', () => { ...@@ -46,6 +46,7 @@ describe('Vulnerability list component', () => {
); );
}; };
const locationText = ({ file, startLine }) => `${file}:${startLine}`;
const findTable = () => wrapper.findComponent(GlTable); const findTable = () => wrapper.findComponent(GlTable);
const findSortableColumn = () => wrapper.find('[aria-sort="descending"]'); const findSortableColumn = () => wrapper.find('[aria-sort="descending"]');
const findCell = (label) => wrapper.find(`.js-${label}`); const findCell = (label) => wrapper.find(`.js-${label}`);
...@@ -62,6 +63,7 @@ describe('Vulnerability list component', () => { ...@@ -62,6 +63,7 @@ describe('Vulnerability list component', () => {
findRow(row).findComponent(VulnerabilityCommentIcon); findRow(row).findComponent(VulnerabilityCommentIcon);
const findDataCell = (label) => wrapper.findByTestId(label); const findDataCell = (label) => wrapper.findByTestId(label);
const findDataCells = (label) => wrapper.findAll(`[data-testid="${label}"]`); const findDataCells = (label) => wrapper.findAll(`[data-testid="${label}"]`);
const findLocationCell = (id) => wrapper.findByTestId(`location-${id}`);
const findLocationTextWrapper = (cell) => cell.find(GlTruncate); const findLocationTextWrapper = (cell) => cell.find(GlTruncate);
const findFiltersProducedNoResults = () => wrapper.findComponent(FiltersProducedNoResults); const findFiltersProducedNoResults = () => wrapper.findComponent(FiltersProducedNoResults);
const findDashboardHasNoVulnerabilities = () => const findDashboardHasNoVulnerabilities = () =>
...@@ -232,7 +234,7 @@ describe('Vulnerability list component', () => { ...@@ -232,7 +234,7 @@ describe('Vulnerability list component', () => {
it('should display the vulnerability locations for images', () => { it('should display the vulnerability locations for images', () => {
const { id, project, location } = newVulnerabilities[0]; const { id, project, location } = newVulnerabilities[0];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).toContain(project.nameWithNamespace); expect(cell.text()).toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: location.image, text: location.image,
...@@ -242,17 +244,17 @@ describe('Vulnerability list component', () => { ...@@ -242,17 +244,17 @@ describe('Vulnerability list component', () => {
it('should display the vulnerability locations for code', () => { it('should display the vulnerability locations for code', () => {
const { id, project, location } = newVulnerabilities[1]; const { id, project, location } = newVulnerabilities[1];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).toContain(project.nameWithNamespace); expect(cell.text()).toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: `${location.file}:${location.startLine}`, text: locationText(location),
position: 'middle', position: 'middle',
}); });
}); });
it('should display the vulnerability locations for code with no line data', () => { it('should display the vulnerability locations for code with no line data', () => {
const { id, project, location } = newVulnerabilities[2]; const { id, project, location } = newVulnerabilities[2];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).toContain(project.nameWithNamespace); expect(cell.text()).toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: location.file, text: location.file,
...@@ -262,14 +264,14 @@ describe('Vulnerability list component', () => { ...@@ -262,14 +264,14 @@ describe('Vulnerability list component', () => {
it('should not display the vulnerability locations for vulnerabilities without a location', () => { it('should not display the vulnerability locations for vulnerabilities without a location', () => {
const { id, project } = newVulnerabilities[4]; const { id, project } = newVulnerabilities[4];
const cellText = findDataCell(`location-${id}`).text(); const cellText = findLocationCell(id).text();
expect(cellText).toEqual(project.nameWithNamespace); expect(cellText).toEqual(project.nameWithNamespace);
expect(cellText).not.toContain(':'); expect(cellText).not.toContain(':');
}); });
it('should display the vulnerability locations for path', () => { it('should display the vulnerability locations for path', () => {
const { id, project, location } = newVulnerabilities[5]; const { id, project, location } = newVulnerabilities[5];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).toContain(project.nameWithNamespace); expect(cell.text()).toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: location.path, text: location.path,
...@@ -293,7 +295,7 @@ describe('Vulnerability list component', () => { ...@@ -293,7 +295,7 @@ describe('Vulnerability list component', () => {
it('should not display the vulnerability group/project locations for images', () => { it('should not display the vulnerability group/project locations for images', () => {
const { id, project, location } = newVulnerabilities[0]; const { id, project, location } = newVulnerabilities[0];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).not.toContain(project.nameWithNamespace); expect(cell.text()).not.toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: location.image, text: location.image,
...@@ -310,23 +312,29 @@ describe('Vulnerability list component', () => { ...@@ -310,23 +312,29 @@ describe('Vulnerability list component', () => {
it('should display the vulnerability locations for code', () => { it('should display the vulnerability locations for code', () => {
const { id, project, location } = newVulnerabilities[1]; const { id, project, location } = newVulnerabilities[1];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).not.toContain(project.nameWithNamespace); expect(cell.text()).not.toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: `${location.file}:${location.startLine}`, text: locationText(location),
position: 'middle', position: 'middle',
}); });
}); });
it('should make the file path linkable', () => { it('should make the file path linkable', () => {
const { id, location } = newVulnerabilities[1]; const { id, location } = newVulnerabilities[1];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.find('a').attributes('href')).toBe(`${location.blobPath}#L${location.startLine}`); expect(cell.find('a').attributes('href')).toBe(`${location.blobPath}#L${location.startLine}`);
}); });
it('should not make the file path linkable if blobPath is missing', () => {
const { id } = newVulnerabilities[0];
const cell = findLocationCell(id);
expect(cell.find('a').exists()).toBe(false);
});
it('should not display the vulnerability group/project locations for code with no line data', () => { it('should not display the vulnerability group/project locations for code with no line data', () => {
const { id, project, location } = newVulnerabilities[2]; const { id, project, location } = newVulnerabilities[2];
const cell = findDataCell(`location-${id}`); const cell = findLocationCell(id);
expect(cell.text()).not.toContain(project.nameWithNamespace); expect(cell.text()).not.toContain(project.nameWithNamespace);
expect(findLocationTextWrapper(cell).props()).toEqual({ expect(findLocationTextWrapper(cell).props()).toEqual({
text: location.file, text: location.file,
......
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