diff --git a/app/assets/javascripts/vue_merge_request_widget/components/widget/widget.vue b/app/assets/javascripts/vue_merge_request_widget/components/widget/widget.vue index 123fc8d90963c23d6addbd5ac56b6954a4b23f2f..5bfe903af960972b4a987c43c7681955116fda60 100644 --- a/app/assets/javascripts/vue_merge_request_widget/components/widget/widget.vue +++ b/app/assets/javascripts/vue_merge_request_widget/components/widget/widget.vue @@ -16,8 +16,6 @@ import DynamicContent from './dynamic_content.vue'; import StatusIcon from './status_icon.vue'; import ActionButtons from './action_buttons.vue'; -const FETCH_TYPE_COLLAPSED = 'collapsed'; -const FETCH_TYPE_EXPANDED = 'expanded'; const WIDGET_PREFIX = 'Widget'; const MISSING_RESPONSE_HEADERS = 'MR Widget: raesponse object should contain status and headers object. Make sure to include that in your `fetchCollapsedData` and `fetchExpandedData` functions.'; @@ -49,15 +47,6 @@ export default { SafeHtml, }, props: { - /** - * @param {value.collapsed} Object - * @param {value.expanded} Object - */ - value: { - type: Object, - required: false, - default: () => ({}), - }, loadingText: { type: String, required: false, @@ -238,7 +227,7 @@ export default { try { if (this.fetchCollapsedData) { - await this.fetch(this.fetchCollapsedData, FETCH_TYPE_COLLAPSED); + await this.fetch(this.fetchCollapsedData); } } catch { this.summaryError = this.errorText; @@ -271,7 +260,7 @@ export default { this.contentError = null; try { - await this.fetch(this.fetchExpandedData, FETCH_TYPE_EXPANDED); + await this.fetch(this.fetchExpandedData); } catch { this.contentError = this.errorText; @@ -282,7 +271,7 @@ export default { this.isLoadingExpandedContent = false; }, - fetch(handler, dataType) { + fetch(handler) { const requests = this.multiPolling ? handler() : [handler]; const promises = requests.map((request) => { @@ -319,9 +308,7 @@ export default { }); }); - return Promise.all(promises).then((data) => { - this.$emit('input', { ...this.value, [dataType]: this.multiPolling ? data : data[0] }); - }); + return Promise.all(promises); }, }, failedStatusIcon: EXTENSION_ICONS.failed, diff --git a/ee/app/assets/javascripts/vue_merge_request_widget/extensions/security_reports/mr_widget_security_reports.vue b/ee/app/assets/javascripts/vue_merge_request_widget/extensions/security_reports/mr_widget_security_reports.vue index e280bdcc668dcfbbd630a95dcdd61c2e6a10c5fb..34f210d497d22afd42f4aa4dc1eb9fd1e0476efb 100644 --- a/ee/app/assets/javascripts/vue_merge_request_widget/extensions/security_reports/mr_widget_security_reports.vue +++ b/ee/app/assets/javascripts/vue_merge_request_widget/extensions/security_reports/mr_widget_security_reports.vue @@ -60,10 +60,7 @@ export default { isCreatingMergeRequest: false, hasAtLeastOneReportWithMaxNewVulnerabilities: false, modalData: null, - vulnerabilities: { - collapsed: null, - expanded: null, - }, + collapsedData: {}, }; }, @@ -100,6 +97,7 @@ export default { } const issue = finding.issueLinks?.nodes.find((x) => x.linkType === 'CREATED')?.issue; + if (issue) { this.$set(this.modalData.vulnerability, 'hasIssue', true); @@ -125,6 +123,12 @@ export default { }, computed: { + reports() { + return this.endpoints + .map(([, reportType]) => this.collapsedData[reportType]) + .filter((r) => r); + }, + helpPopovers() { return { SAST: { @@ -172,21 +176,17 @@ export default { }, isCollapsible() { - if (!this.vulnerabilities.collapsed) { - return false; - } - return this.vulnerabilitiesCount > 0; }, vulnerabilitiesCount() { - return this.vulnerabilities.collapsed.reduce((counter, current) => { + return this.reports.reduce((counter, current) => { return counter + current.numberOfNewFindings + (current.fixed?.length || 0); }, 0); }, highlights() { - if (!this.vulnerabilities.collapsed) { + if (!this.reports.length) { return {}; } @@ -202,19 +202,13 @@ export default { // { scanner: "DAST", added: [{ id: 15, severity: 'high' }] }, // ... // ] - this.vulnerabilities.collapsed.forEach((report) => - this.highlightsFromReport(report, highlights), - ); + this.reports.forEach((report) => this.highlightsFromReport(report, highlights)); return highlights; }, totalNewVulnerabilities() { - if (!this.vulnerabilities.collapsed) { - return 0; - } - - return this.vulnerabilities.collapsed.reduce((counter, current) => { + return this.reports.reduce((counter, current) => { return counter + (current.numberOfNewFindings || 0); }, 0); }, @@ -308,26 +302,30 @@ export default { this.hasAtLeastOneReportWithMaxNewVulnerabilities = true; } + const report = { + ...props, + ...data, + added, + fixed, + findings: [...added, ...fixed], + numberOfNewFindings: added.length, + numberOfFixedFindings: fixed.length, + qaSelector: this.$options.qaSelectors[reportType], + }; + + this.$set(this.collapsedData, reportType, report); + return { headers, status, - data: { - ...props, - ...data, - added, - fixed, - findings: [...added, ...fixed], - numberOfNewFindings: added.length, - numberOfFixedFindings: fixed.length, - qaSelector: this.$options.qaSelectors[reportType], - }, + data: report, }; }) - .catch(({ headers = {}, status = 500 }) => ({ - headers, - status, - data: { ...props, error: true }, - })); + .catch(({ headers = {}, status = 500 }) => { + const report = { ...props, error: true }; + this.$set(this.collapsedData, reportType, report); + return { headers, status, data: report }; + }); }); }, @@ -396,11 +394,11 @@ export default { }) .then(({ data }) => { const url = getCreatedIssueForVulnerability(data).issue_url; - visitUrl(url); }) .catch(() => { this.isCreatingIssue = false; + this.modalData.error = s__( 'ciReport|There was an error creating the issue. Please try again.', ); @@ -609,7 +607,6 @@ export default { <template> <mr-widget v-if="shouldRenderMrWidget" - v-model="vulnerabilities" :error-text="$options.i18n.error" :fetch-collapsed-data="fetchCollapsedData" :status-icon-name="statusIconName" @@ -662,7 +659,7 @@ export default { :project-full-path="mr.sourceProjectFullPath" /> <mr-widget-row - v-for="report in vulnerabilities.collapsed" + v-for="report in reports" :key="report.reportType" :widget-name="$options.name" :level="2" diff --git a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js index 9343a3a5e906e32b97e253529b67fd974414d1f4..18fdba32f52429275dede3b3bf647928986cce84 100644 --- a/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js +++ b/spec/frontend/vue_merge_request_widget/components/widget/widget_spec.js @@ -121,14 +121,15 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { }); describe('fetch', () => { - it('sets the data.collapsed property after a successfull call - multiPolling: false', async () => { + it('calls fetchCollapsedData properly when multiPolling is false', async () => { const mockData = { headers: {}, status: HTTP_STATUS_OK, data: { vulnerabilities: [] } }; - createComponent({ propsData: { fetchCollapsedData: () => Promise.resolve(mockData) } }); + const fetchCollapsedData = jest.fn().mockResolvedValue(mockData); + createComponent({ propsData: { fetchCollapsedData } }); await waitForPromises(); - expect(wrapper.emitted('input')[0][0]).toEqual({ collapsed: mockData.data, expanded: null }); + expect(fetchCollapsedData).toHaveBeenCalledTimes(1); }); - it('sets the data.collapsed property after a successfull call - multiPolling: true', async () => { + it('calls fetchCollapsedData properly when multiPolling is true', async () => { const mockData1 = { headers: {}, status: HTTP_STATUS_OK, @@ -140,22 +141,22 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { data: { vulnerabilities: [{ vuln: 2 }] }, }; + const fetchCollapsedData = [ + jest.fn().mockResolvedValue(mockData1), + jest.fn().mockResolvedValue(mockData2), + ]; + createComponent({ propsData: { multiPolling: true, - fetchCollapsedData: () => [ - () => Promise.resolve(mockData1), - () => Promise.resolve(mockData2), - ], + fetchCollapsedData: () => fetchCollapsedData, }, }); await waitForPromises(); - expect(wrapper.emitted('input')[0][0]).toEqual({ - collapsed: [mockData1.data, mockData2.data], - expanded: null, - }); + expect(fetchCollapsedData[0]).toHaveBeenCalledTimes(1); + expect(fetchCollapsedData[1]).toHaveBeenCalledTimes(1); }); it('throws an error when the handler does not include headers or status objects', async () => { @@ -328,11 +329,12 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { }; const fetchExpandedData = jest.fn().mockResolvedValue(mockDataExpanded); + const fetchCollapsedData = jest.fn().mockResolvedValue(mockDataCollapsed); await createComponent({ propsData: { isCollapsible: true, - fetchCollapsedData: () => Promise.resolve(mockDataCollapsed), + fetchCollapsedData, fetchExpandedData, }, }); @@ -340,17 +342,8 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { findToggleButton().vm.$emit('click'); await waitForPromises(); - // First fetches the collapsed data - expect(wrapper.emitted('input')[0][0]).toEqual({ - collapsed: mockDataCollapsed.data, - expanded: null, - }); - - // Then fetches the expanded data - expect(wrapper.emitted('input')[1][0]).toEqual({ - collapsed: null, - expanded: mockDataExpanded.data, - }); + expect(fetchCollapsedData).toHaveBeenCalledTimes(1); + expect(fetchExpandedData).toHaveBeenCalledTimes(1); // Triggering a click does not call the expanded data again findToggleButton().vm.$emit('click'); @@ -371,14 +364,7 @@ describe('~/vue_merge_request_widget/components/widget/widget.vue', () => { findToggleButton().vm.$emit('click'); await waitForPromises(); - // First fetches the collapsed data - expect(wrapper.emitted('input')[0][0]).toEqual({ - collapsed: undefined, - expanded: null, - }); - expect(fetchExpandedData).toHaveBeenCalledTimes(1); - expect(wrapper.emitted('input')).toHaveLength(1); // Should not an emit an input call because request failed findToggleButton().vm.$emit('click'); await waitForPromises();