Commit 77648998 authored by Mark Florian's avatar Mark Florian

Address review feedback and pipeline failures

- Fix ESLint errors
- Update gitlab.pot translations
- Use optional chaining
- Make template prettier
- Extract `find` helper in specs
- [Use refs][1] instead of legacy `js-` classes for component testing

And last but not least: import used `expand-button` component.

It turns out the original creation of this component wasn't perfect,
done in 18d13bf7, as it didn't bring
over the `expand-button` component from the parent modal from which it
was extracted.

Still, this went unnoticed for around 10 months, so I'm not convinced
`instances` are often - or ever - present in vulnerabilities/findings.

The snapshot test has been updated to include it as well.

[1]: https://docs.gitlab.com/ce/development/testing_guide/frontend_testing.html#how-to-query-dom-elements
parent f19d356d
import Vue from 'vue'; import Vue from 'vue';
import getFileLocation from 'ee/vue_shared/security_reports/store/utils/get_file_location';
import { s__, __ } from '~/locale'; import { s__, __ } from '~/locale';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
import * as types from './mutation_types'; import * as types from './mutation_types';
...@@ -78,8 +77,8 @@ export default { ...@@ -78,8 +77,8 @@ export default {
Vue.set(state.modal, 'title', vulnerability.name); Vue.set(state.modal, 'title', vulnerability.name);
Vue.set(state.modal.project, 'value', vulnerability.project && vulnerability.project.full_name); Vue.set(state.modal.project, 'value', vulnerability.project?.full_name);
Vue.set(state.modal.project, 'url', vulnerability.project && vulnerability.project.full_path); Vue.set(state.modal.project, 'url', vulnerability.project?.full_path);
Vue.set(state.modal, 'vulnerability', vulnerability); Vue.set(state.modal, 'vulnerability', vulnerability);
Vue.set( Vue.set(
......
import { __, s__ } from '~/locale';
export default () => ({ export default () => ({
isLoadingVulnerabilities: true, isLoadingVulnerabilities: true,
errorLoadingVulnerabilities: false, errorLoadingVulnerabilities: false,
......
<script> <script>
import { GlFriendlyWrap } from '@gitlab/ui'; import { GlFriendlyWrap } from '@gitlab/ui';
import { __, s__ } from '~/locale';
import SafeLink from 'ee/vue_shared/components/safe_link.vue'; import SafeLink from 'ee/vue_shared/components/safe_link.vue';
import Icon from '~/vue_shared/components/icon.vue'; import Icon from '~/vue_shared/components/icon.vue';
import ExpandButton from '~/vue_shared/components/expand_button.vue';
import SeverityBadge from './severity_badge.vue'; import SeverityBadge from './severity_badge.vue';
import getFileLocation from '../store/utils/get_file_location'; import getFileLocation from '../store/utils/get_file_location';
import VulnerabilityDetail from './vulnerability_detail.vue'; import VulnerabilityDetail from './vulnerability_detail.vue';
...@@ -10,6 +10,7 @@ import VulnerabilityDetail from './vulnerability_detail.vue'; ...@@ -10,6 +10,7 @@ import VulnerabilityDetail from './vulnerability_detail.vue';
export default { export default {
name: 'VulnerabilityDetails', name: 'VulnerabilityDetails',
components: { components: {
ExpandButton,
GlFriendlyWrap, GlFriendlyWrap,
Icon, Icon,
SafeLink, SafeLink,
...@@ -22,14 +23,6 @@ export default { ...@@ -22,14 +23,6 @@ export default {
required: true, required: true,
}, },
}, },
methods: {
hasMoreValues(index, values) {
return index < values.length - 1;
},
asNonEmptyListOrNull(list) {
return list?.length > 0 ? list : null;
},
},
computed: { computed: {
url() { url() {
return getFileLocation(this.vulnerability.location); return getFileLocation(this.vulnerability.location);
...@@ -75,6 +68,14 @@ export default { ...@@ -75,6 +68,14 @@ export default {
return this.asNonEmptyListOrNull(this.vulnerability.instances); return this.asNonEmptyListOrNull(this.vulnerability.instances);
}, },
}, },
methods: {
hasMoreValues(index, values) {
return index < values.length - 1;
},
asNonEmptyListOrNull(list) {
return list?.length > 0 ? list : null;
},
},
}; };
</script> </script>
...@@ -88,13 +89,13 @@ export default { ...@@ -88,13 +89,13 @@ export default {
</vulnerability-detail> </vulnerability-detail>
<vulnerability-detail v-if="vulnerability.project" :label="s__('Vulnerability|Project')"> <vulnerability-detail v-if="vulnerability.project" :label="s__('Vulnerability|Project')">
<safe-link class="js-link-project" :href="vulnerability.project.full_path" target="_blank"> <safe-link ref="projectLink" :href="vulnerability.project.full_path" target="_blank">
<gl-friendly-wrap :text="vulnerability.project.full_name" /> <gl-friendly-wrap :text="vulnerability.project.full_name" />
</safe-link> </safe-link>
</vulnerability-detail> </vulnerability-detail>
<vulnerability-detail v-if="url" :label="__('URL')"> <vulnerability-detail v-if="url" :label="__('URL')">
<safe-link class="js-link-url" :href="url" target="_blank"> <safe-link ref="urlLink" :href="url" target="_blank">
<gl-friendly-wrap :text="url" /> <gl-friendly-wrap :text="url" />
</safe-link> </safe-link>
</vulnerability-detail> </vulnerability-detail>
...@@ -102,7 +103,7 @@ export default { ...@@ -102,7 +103,7 @@ export default {
<vulnerability-detail v-if="file" :label="s__('Vulnerability|File')"> <vulnerability-detail v-if="file" :label="s__('Vulnerability|File')">
<safe-link <safe-link
v-if="vulnerability.blob_path" v-if="vulnerability.blob_path"
class="js-link-file" ref="fileLink"
:href="vulnerability.blob_path" :href="vulnerability.blob_path"
target="_blank" target="_blank"
> >
...@@ -115,7 +116,7 @@ export default { ...@@ -115,7 +116,7 @@ export default {
<span v-for="(identifier, i) in identifiers" :key="i"> <span v-for="(identifier, i) in identifiers" :key="i">
<safe-link <safe-link
v-if="identifier.url" v-if="identifier.url"
class="js-link-identifiers" ref="identifiersLink"
:href="identifier.url" :href="identifier.url"
target="_blank" target="_blank"
rel="noopener noreferrer" rel="noopener noreferrer"
...@@ -156,7 +157,7 @@ export default { ...@@ -156,7 +157,7 @@ export default {
<vulnerability-detail v-if="links" :label="s__('Vulnerability|Links')"> <vulnerability-detail v-if="links" :label="s__('Vulnerability|Links')">
<span v-for="(link, i) in links" :key="i"> <span v-for="(link, i) in links" :key="i">
<safe-link class="js-link-links" :href="link.url" target="_blank" rel="noopener noreferrer"> <safe-link ref="linksLink" :href="link.url" target="_blank" rel="noopener noreferrer">
{{ link.value || link.url }} {{ link.value || link.url }}
</safe-link> </safe-link>
<span v-if="hasMoreValues(i, links)">,&nbsp;</span> <span v-if="hasMoreValues(i, links)">,&nbsp;</span>
...@@ -185,12 +186,12 @@ export default { ...@@ -185,12 +186,12 @@ export default {
</safe-link> </safe-link>
</div> </div>
<expand-button v-if="instance.evidence"> <expand-button v-if="instance.evidence">
<template #expanded>
<pre <pre
slot="expanded"
class="block report-block-dast-code prepend-top-10 report-block-issue-code" class="block report-block-dast-code prepend-top-10 report-block-issue-code"
> v-text="instance.evidence"
{{ instance.evidence }}</pre ></pre>
> </template>
</expand-button> </expand-button>
</div> </div>
</li> </li>
......
import Vue from 'vue'; import Vue from 'vue';
import * as types from './mutation_types'; import * as types from './mutation_types';
import { findIssueIndex, parseDiff } from './utils'; import { findIssueIndex, parseDiff } from './utils';
import getFileLocation from './utils/get_file_location';
import { visitUrl } from '~/lib/utils/url_utility'; import { visitUrl } from '~/lib/utils/url_utility';
export default { export default {
......
import { __, s__ } from '~/locale';
export default () => ({ export default () => ({
blobPath: { blobPath: {
head: null, head: null,
......
...@@ -17,7 +17,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -17,7 +17,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
label="Project" label="Project"
> >
<safe-link-stub <safe-link-stub
class="js-link-project"
href="/gitlab-org/gitlab-ui" href="/gitlab-org/gitlab-ui"
target="_blank" target="_blank"
> >
...@@ -34,7 +33,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -34,7 +33,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
label="File" label="File"
> >
<safe-link-stub <safe-link-stub
class="js-link-file"
href="/gitlab-org/gitlab-ui/blob/ad137f0a8ac59af961afe47d04e5cc062c6864a9/yarn.lock" href="/gitlab-org/gitlab-ui/blob/ad137f0a8ac59af961afe47d04e5cc062c6864a9/yarn.lock"
target="_blank" target="_blank"
> >
...@@ -50,7 +48,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -50,7 +48,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
> >
<span> <span>
<safe-link-stub <safe-link-stub
class="js-link-identifiers"
href="https://deps.sec.gitlab.com/packages/npm/serialize-javascript/versions/1.7.0/advisories" href="https://deps.sec.gitlab.com/packages/npm/serialize-javascript/versions/1.7.0/advisories"
rel="noopener noreferrer" rel="noopener noreferrer"
target="_blank" target="_blank"
...@@ -66,7 +63,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -66,7 +63,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
</span> </span>
<span> <span>
<safe-link-stub <safe-link-stub
class="js-link-identifiers"
href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16769" href="https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2019-16769"
rel="noopener noreferrer" rel="noopener noreferrer"
target="_blank" target="_blank"
...@@ -111,7 +107,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -111,7 +107,6 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
> >
<span> <span>
<safe-link-stub <safe-link-stub
class="js-link-links"
href="https://nvd.nist.gov/vuln/detail/CVE-2019-16769" href="https://nvd.nist.gov/vuln/detail/CVE-2019-16769"
rel="noopener noreferrer" rel="noopener noreferrer"
target="_blank" target="_blank"
...@@ -125,6 +120,58 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = ` ...@@ -125,6 +120,58 @@ exports[`VulnerabilityDetails component pin test renders correctly 1`] = `
</span> </span>
</vulnerability-detail-stub> </vulnerability-detail-stub>
<!----> <vulnerability-detail-stub
label="Instances"
>
<div
class="info-well"
>
<ul
class="report-block-list"
>
<li
class="report-block-list-issue"
>
<div
class="report-block-list-icon append-right-5 failed"
>
<icon-stub
name="status_failed_borderless"
size="32"
/>
</div>
<div
class="report-block-list-issue-description prepend-top-5 append-bottom-5"
>
<div
class="report-block-list-issue-description-text"
>
POST
</div>
<div
class="report-block-list-issue-description-link"
>
<safe-link-stub
class="break-link"
href="/bar"
rel="noopener noreferrer nofollow"
target="_blank"
>
/bar
</safe-link-stub>
</div>
<expand-button-stub />
</div>
</li>
</ul>
</div>
</vulnerability-detail-stub>
</div> </div>
`; `;
import { mount, shallowMount } from '@vue/test-utils'; import { mount, shallowMount } from '@vue/test-utils';
import createState from 'ee/vue_shared/security_reports/store/state';
import VulnerabilityDetails from 'ee/vue_shared/security_reports/components/vulnerability_details.vue'; import VulnerabilityDetails from 'ee/vue_shared/security_reports/components/vulnerability_details.vue';
import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue'; import SeverityBadge from 'ee/vue_shared/security_reports/components/severity_badge.vue';
import SafeLink from 'ee/vue_shared/components/safe_link.vue'; import SafeLink from 'ee/vue_shared/components/safe_link.vue';
...@@ -26,6 +25,8 @@ describe('VulnerabilityDetails component', () => { ...@@ -26,6 +25,8 @@ describe('VulnerabilityDetails component', () => {
expect(link.text()).toBe(text); expect(link.text()).toBe(text);
}; };
const findLink = name => wrapper.find({ ref: `${name}Link` });
afterEach(() => { afterEach(() => {
wrapper.destroy(); wrapper.destroy();
}); });
...@@ -42,7 +43,7 @@ describe('VulnerabilityDetails component', () => { ...@@ -42,7 +43,7 @@ describe('VulnerabilityDetails component', () => {
const vulnerability = makeVulnerability(); const vulnerability = makeVulnerability();
componentFactory(vulnerability); componentFactory(vulnerability);
expectSafeLink(wrapper.find('.js-link-project'), { expectSafeLink(findLink('project'), {
href: vulnerability.project.full_path, href: vulnerability.project.full_path,
text: vulnerability.project.full_name, text: vulnerability.project.full_name,
}); });
...@@ -58,12 +59,7 @@ describe('VulnerabilityDetails component', () => { ...@@ -58,12 +59,7 @@ describe('VulnerabilityDetails component', () => {
componentFactory(vulnerability); componentFactory(vulnerability);
const link = wrapper expect(findLink('file').html()).toMatch('/<wbr>some/<wbr>file/<wbr>path');
.findAll(SafeLink)
.filter(wrapper => wrapper.classes('js-link-file'))
.at(0);
expect(link.html()).toMatch('/<wbr>some/<wbr>file/<wbr>path');
}); });
it('escapes wrapped file paths', () => { it('escapes wrapped file paths', () => {
...@@ -76,12 +72,9 @@ describe('VulnerabilityDetails component', () => { ...@@ -76,12 +72,9 @@ describe('VulnerabilityDetails component', () => {
componentFactory(vulnerability); componentFactory(vulnerability);
const link = wrapper expect(findLink('file').html()).toMatch(
.findAll(SafeLink) '/<wbr>unsafe/<wbr>path&lt;script&gt;&lt;/<wbr>script&gt;',
.filter(wrapper => wrapper.classes('js-link-file')) );
.at(0);
expect(link.html()).toMatch('/<wbr>unsafe/<wbr>path&lt;script&gt;&lt;/<wbr>script&gt;');
}); });
describe('does not render XSS links', () => { describe('does not render XSS links', () => {
...@@ -103,21 +96,21 @@ describe('VulnerabilityDetails component', () => { ...@@ -103,21 +96,21 @@ describe('VulnerabilityDetails component', () => {
}); });
it('for the link field', () => { it('for the link field', () => {
expectSafeLink(wrapper.find('.js-link-links'), { expectSafeLink(findLink('links'), {
href: badUrl, href: badUrl,
text: badUrl, text: badUrl,
}); });
}); });
it('for the identifiers field', () => { it('for the identifiers field', () => {
expectSafeLink(wrapper.find('.js-link-identifiers'), { expectSafeLink(findLink('identifiers'), {
href: badUrl, href: badUrl,
text: 'BAD_URL', text: 'BAD_URL',
}); });
}); });
it('for the file field', () => { it('for the file field', () => {
expectSafeLink(wrapper.find('.js-link-file'), { expectSafeLink(findLink('file'), {
href: badUrl, href: badUrl,
text: 'badFile.lock', text: 'badFile.lock',
}); });
...@@ -166,7 +159,18 @@ describe('VulnerabilityDetails component', () => { ...@@ -166,7 +159,18 @@ describe('VulnerabilityDetails component', () => {
}; };
it('renders correctly', () => { it('renders correctly', () => {
factory(makeVulnerability()); factory(
makeVulnerability({
instances: [
{
method: 'POST',
evidence: 'foo',
uri: '/bar',
},
],
}),
);
expect(wrapper.element).toMatchSnapshot(); expect(wrapper.element).toMatchSnapshot();
}); });
}); });
......
...@@ -21817,6 +21817,9 @@ msgstr "" ...@@ -21817,6 +21817,9 @@ msgstr ""
msgid "Vulnerability|Links" msgid "Vulnerability|Links"
msgstr "" msgstr ""
msgid "Vulnerability|Method"
msgstr ""
msgid "Vulnerability|Namespace" msgid "Vulnerability|Namespace"
msgstr "" msgstr ""
...@@ -22935,9 +22938,6 @@ msgstr "" ...@@ -22935,9 +22938,6 @@ msgstr ""
msgid "ciReport|Base pipeline codequality artifact not found" msgid "ciReport|Base pipeline codequality artifact not found"
msgstr "" msgstr ""
msgid "ciReport|Class"
msgstr ""
msgid "ciReport|Code quality" msgid "ciReport|Code quality"
msgstr "" msgstr ""
...@@ -22968,9 +22968,6 @@ msgstr "" ...@@ -22968,9 +22968,6 @@ msgstr ""
msgid "ciReport|Dependency scanning" msgid "ciReport|Dependency scanning"
msgstr "" msgstr ""
msgid "ciReport|Description"
msgstr ""
msgid "ciReport|Download patch to resolve" msgid "ciReport|Download patch to resolve"
msgstr "" msgstr ""
...@@ -22983,42 +22980,21 @@ msgstr "" ...@@ -22983,42 +22980,21 @@ msgstr ""
msgid "ciReport|Failed to load %{reportName} report" msgid "ciReport|Failed to load %{reportName} report"
msgstr "" msgstr ""
msgid "ciReport|File"
msgstr ""
msgid "ciReport|Fixed:" msgid "ciReport|Fixed:"
msgstr "" msgstr ""
msgid "ciReport|Identifiers"
msgstr ""
msgid "ciReport|Image"
msgstr ""
msgid "ciReport|Instances"
msgstr ""
msgid "ciReport|Investigate this vulnerability by creating an issue" msgid "ciReport|Investigate this vulnerability by creating an issue"
msgstr "" msgstr ""
msgid "ciReport|Learn more about interacting with security reports" msgid "ciReport|Learn more about interacting with security reports"
msgstr "" msgstr ""
msgid "ciReport|Links"
msgstr ""
msgid "ciReport|Loading %{reportName} report" msgid "ciReport|Loading %{reportName} report"
msgstr "" msgstr ""
msgid "ciReport|Manage licenses" msgid "ciReport|Manage licenses"
msgstr "" msgstr ""
msgid "ciReport|Method"
msgstr ""
msgid "ciReport|Namespace"
msgstr ""
msgid "ciReport|No changes to code quality" msgid "ciReport|No changes to code quality"
msgstr "" msgstr ""
...@@ -23040,9 +23016,6 @@ msgstr "" ...@@ -23040,9 +23016,6 @@ msgstr ""
msgid "ciReport|Security scanning failed loading any results" msgid "ciReport|Security scanning failed loading any results"
msgstr "" msgstr ""
msgid "ciReport|Severity"
msgstr ""
msgid "ciReport|Solution" msgid "ciReport|Solution"
msgstr "" msgstr ""
......
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