Skip to content

G
gitlab-ce
Commit 1642b1a5 authored by Mark Florian's avatar Mark Florian Committed by Miguel Rincon
Browse files
Options

Remove core_security_mr_widget_downloads flag 

This removes all the relevant code from the component, and significantly
rewrites the component specs, since they weren't written in a way that
made it easy to simply remove contexts/tests.

The removed code was added in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48769, and the
feature flag was enabled by default in
https://gitlab.com/gitlab-org/gitlab/-/merge_requests/49572.

Addresses https://gitlab.com/gitlab-org/gitlab/-/issues/292266.
parent ca67c9c7
Expand all Show whitespace changes
Inline Side-by-side
Showing with 151 additions and 427 deletions
app/assets/javascripts/vue_shared/security_reports/security_reports_app.vue
View file @ 1642b1a5
<script>
import { mapActions, mapGetters } from 'vuex';
import { GlLink, GlSprintf } from '@gitlab/ui';
import glFeatureFlagsMixin from '~/vue_shared/mixins/gl_feature_flags_mixin';
import ReportSection from '~/reports/components/report_section.vue';
import { LOADING, ERROR, SLOT_SUCCESS, SLOT_LOADING, SLOT_ERROR } from '~/reports/constants';
import { ERROR, SLOT_SUCCESS, SLOT_LOADING, SLOT_ERROR } from '~/reports/constants';
import { s__ } from '~/locale';
import { normalizeHeaders, parseIntPagination } from '~/lib/utils/common_utils';
import createFlash from '~/flash';
import Api from '~/api';
import HelpIcon from './components/help_icon.vue';
import SecurityReportDownloadDropdown from './components/security_report_download_dropdown.vue';
import SecuritySummary from './components/security_summary.vue';
......@@ -24,8 +21,6 @@ import { extractSecurityReportArtifacts } from './utils';
export default {
store,
components: {
GlLink,
GlSprintf,
ReportSection,
HelpIcon,
SecurityReportDownloadDropdown,
......@@ -101,9 +96,6 @@ export default {
),
};
},
skip() {
return !this.canShowDownloads;
},
update(data) {
return extractSecurityReportArtifacts(this.$options.reportTypes, data);
},
......@@ -124,9 +116,6 @@ export default {
},
computed: {
...mapGetters(['groupedSummaryText', 'summaryStatus']),
canShowDownloads() {
return this.glFeatures.coreSecurityMrWidgetDownloads;
},
hasSecurityReports() {
return this.availableSecurityReports.length > 0;
},
......@@ -139,23 +128,6 @@ export default {
isLoadingReportArtifacts() {
return this.$apollo.queries.reportArtifacts.loading;
},
shouldShowDownloadGuidance() {
return !this.canShowDownloads && this.summaryStatus !== LOADING;
},
scansHaveRunMessage() {
return this.canShowDownloads
? this.$options.i18n.scansHaveRun
: this.$options.i18n.scansHaveRunWithDownloadGuidance;
},
},
created() {
if (!this.canShowDownloads) {
this.checkAvailableSecurityReports(this.$options.reportTypes)
.then((availableSecurityReports) => {
this.onCheckingAvailableSecurityReports(Array.from(availableSecurityReports));
})
.catch(this.showError);
}
},
methods: {
...mapActions(MODULE_SAST, {
......@@ -166,36 +138,6 @@ export default {
setSecretDetectionDiffEndpoint: 'setDiffEndpoint',
fetchSecretDetectionDiff: 'fetchDiff',
}),
async checkAvailableSecurityReports(reportTypes) {
const reportTypesSet = new Set(reportTypes);
const availableReportTypes = new Set();
let page = 1;
while (page) {
// eslint-disable-next-line no-await-in-loop
const { data: jobs, headers } = await Api.pipelineJobs(this.projectId, this.pipelineId, {
per_page: 100,
page,
});
jobs.forEach(({ artifacts = [] }) => {
artifacts.forEach(({ file_type }) => {
if (reportTypesSet.has(file_type)) {
availableReportTypes.add(file_type);
}
});
});
// If we've found artifacts for all the report types, stop looking!
if (availableReportTypes.size === reportTypesSet.size) {
return availableReportTypes;
}
page = parseIntPagination(normalizeHeaders(headers)).nextPage;
}
return availableReportTypes;
},
fetchCounts() {
if (!this.glFeatures.coreSecurityMrWidgetCounts) {
return;
......@@ -213,11 +155,6 @@ export default {
this.canShowCounts = true;
}
},
activatePipelinesTab() {
if (window.mrTabs) {
window.mrTabs.tabShown('pipelines');
}
},
onCheckingAvailableSecurityReports(availableSecurityReports) {
this.availableSecurityReports = availableSecurityReports;
this.fetchCounts();
......@@ -236,12 +173,6 @@ export default {
'SecurityReports|Failed to get security report information. Please reload the page or try again later.',
),
scansHaveRun: s__('SecurityReports|Security scans have run'),
scansHaveRunWithDownloadGuidance: s__(
'SecurityReports|Security scans have run. Go to the %{linkStart}pipelines tab%{linkEnd} to download the security reports',
),
downloadFromPipelineTab: s__(
'SecurityReports|Go to the %{linkStart}pipelines tab%{linkEnd} to download the security reports',
),
},
summarySlots: [SLOT_SUCCESS, SLOT_LOADING, SLOT_ERROR],
};
......@@ -265,22 +196,7 @@ export default {
</span>
</template>
<template v-if="shouldShowDownloadGuidance" #sub-heading>
<span class="gl-font-sm">
<gl-sprintf :message="$options.i18n.downloadFromPipelineTab">
<template #link="{ content }">
<gl-link
class="gl-font-sm"
data-testid="show-pipelines"
@click="activatePipelinesTab"
>{{ content }}</gl-link
>
</template>
</gl-sprintf>
</span>
</template>
<template v-if="canShowDownloads" #action-buttons>
<template #action-buttons>
<security-report-download-dropdown
:artifacts="reportArtifacts"
:loading="isLoadingReportArtifacts"
......@@ -298,13 +214,7 @@ export default {
data-testid="security-mr-widget"
>
<template #error>
<gl-sprintf :message="scansHaveRunMessage">
<template #link="{ content }">
<gl-link data-testid="show-pipelines" @click="activatePipelinesTab">{{
content
}}</gl-link>
</template>
</gl-sprintf>
{{ $options.i18n.scansHaveRun }}
<help-icon
:help-path="securityReportsDocsPath"
......@@ -312,7 +222,7 @@ export default {
/>
</template>
<template v-if="canShowDownloads" #action-buttons>
<template #action-buttons>
<security-report-download-dropdown
:artifacts="reportArtifacts"
:loading="isLoadingReportArtifacts"
......
app/controllers/projects/merge_requests_controller.rb
View file @ 1642b1a5
......@@ -38,7 +38,6 @@ class Projects::MergeRequestsController < Projects::MergeRequests::ApplicationCo
push_frontend_feature_flag(:default_merge_ref_for_diffs, @project)
push_frontend_feature_flag(:core_security_mr_widget, @project, default_enabled: true)
push_frontend_feature_flag(:core_security_mr_widget_counts, @project)
push_frontend_feature_flag(:core_security_mr_widget_downloads, @project, default_enabled: true)
push_frontend_feature_flag(:remove_resolve_note, @project, default_enabled: true)
push_frontend_feature_flag(:diffs_gradual_load, @project, default_enabled: true)
push_frontend_feature_flag(:codequality_mr_diff, @project)
......
config/feature_flags/development/core_security_mr_widget_downloads.yml deleted 100644 → 0
View file @ ca67c9c7
---
name: core_security_mr_widget_downloads
introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/48769
rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/273418
milestone: '13.7'
type: development
group: group::static analysis
default_enabled: true
ee/spec/frontend/vue_mr_widget/ee_mr_widget_options_spec.js
View file @ 1642b1a5
import { mount } from '@vue/test-utils';
import MockAdapter from 'axios-mock-adapter';
import { nextTick } from 'vue';
import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo';
import MrWidgetOptions from 'ee/vue_merge_request_widget/mr_widget_options.vue';
import {
sastDiffSuccessMock,
......@@ -11,6 +12,8 @@ import {
coverageFuzzingDiffSuccessMock,
apiFuzzingDiffSuccessMock,
} from 'ee_jest/vue_shared/security_reports/mock_data';
import { securityReportDownloadPathsQueryResponse } from 'jest/vue_shared/security_reports/mock_data';
import createMockApollo from 'helpers/mock_apollo_helper';
import { TEST_HOST } from 'helpers/test_constants';
import { trimText } from 'helpers/text_helper';
import waitForPromises from 'helpers/wait_for_promises';
......@@ -18,12 +21,12 @@ import waitForPromises from 'helpers/wait_for_promises';
import axios from '~/lib/utils/axios_utils';
import { convertObjectPropsToCamelCase } from '~/lib/utils/common_utils';
import { SUCCESS } from '~/vue_merge_request_widget/components/deployment/constants';
import securityReportDownloadPathsQuery from '~/vue_shared/security_reports/queries/security_report_download_paths.query.graphql';
import mockData, {
baseBrowserPerformance,
headBrowserPerformance,
baseLoadPerformance,
headLoadPerformance,
pipelineJobs,
} from './mock_data';
// Force Jest to transpile and cache
......@@ -32,6 +35,8 @@ import _GroupedSecurityReportsApp from 'ee/vue_shared/security_reports/grouped_s
// eslint-disable-next-line no-unused-vars
import _Deployment from '~/vue_merge_request_widget/components/deployment/deployment.vue';
Vue.use(VueApollo);
const SAST_SELECTOR = '.js-sast-widget';
const DAST_SELECTOR = '.js-dast-widget';
const DEPENDENCY_SCANNING_SELECTOR = '.js-dependency-scanning-widget';
......@@ -999,8 +1004,6 @@ describe('ee merge request widget options', () => {
});
describe('CE security report', () => {
const PIPELINE_JOBS_ENDPOINT = `/api/undefined/projects/${mockData.target_project_id}/pipelines/${mockData.pipeline.id}/jobs`;
describe.each`
context | canReadVulnerabilities | hasPipeline | featureFlag | shouldRender
${'user cannot read vulnerabilities'} | ${false} | ${true} | ${true} | ${true}
......@@ -1017,8 +1020,15 @@ describe('ee merge request widget options', () => {
gon.features = { coreSecurityMrWidget: featureFlag };
mock.onGet(PIPELINE_JOBS_ENDPOINT).replyOnce(200, pipelineJobs);
createComponent({ propsData: { mrData: gl.mrWidgetData } });
createComponent({
propsData: { mrData: gl.mrWidgetData },
apolloProvider: createMockApollo([
[
securityReportDownloadPathsQuery,
async () => ({ data: securityReportDownloadPathsQueryResponse }),
],
]),
});
return waitForPromises();
});
......
ee/spec/frontend/vue_mr_widget/mock_data.js
View file @ 1642b1a5
......@@ -133,14 +133,3 @@ export const codequalityParsedIssues = [
];
export { mockStore };
// TODO: Remove as part of https://gitlab.com/gitlab-org/gitlab/-/issues/249544
export const pipelineJobs = [
{
artifacts: [
{
file_type: 'sast',
},
],
},
];
locale/gitlab.pot
View file @ 1642b1a5
......@@ -25168,9 +25168,6 @@ msgstr ""
msgid "SecurityReports|Fuzzing artifacts"
msgstr ""
msgid "SecurityReports|Go to the %{linkStart}pipelines tab%{linkEnd} to download the security reports"
msgstr ""
msgid "SecurityReports|Hide dismissed"
msgstr ""
......@@ -25234,9 +25231,6 @@ msgstr ""
msgid "SecurityReports|Security scans have run"
msgstr ""
msgid "SecurityReports|Security scans have run. Go to the %{linkStart}pipelines tab%{linkEnd} to download the security reports"
msgstr ""
msgid "SecurityReports|Select a project to add by using the project search field above."
msgstr ""
......
spec/frontend/vue_mr_widget/mr_widget_options_spec.js
View file @ 1642b1a5
import { mount } from '@vue/test-utils';
import Vue, { nextTick } from 'vue';
import VueApollo from 'vue-apollo';
import MockAdapter from 'axios-mock-adapter';
import Api from '~/api';
import createMockApollo from 'helpers/mock_apollo_helper';
import { securityReportDownloadPathsQueryResponse } from 'jest/vue_shared/security_reports/mock_data';
import axios from '~/lib/utils/axios_utils';
import MrWidgetOptions from '~/vue_merge_request_widget/mr_widget_options.vue';
import eventHub from '~/vue_merge_request_widget/event_hub';
......@@ -12,11 +14,14 @@ import { stateKey } from '~/vue_merge_request_widget/stores/state_maps';
import mockData from './mock_data';
import { faviconDataUrl, overlayDataUrl } from '../lib/utils/mock_data';
import { SUCCESS } from '~/vue_merge_request_widget/components/deployment/constants';
import securityReportDownloadPathsQuery from '~/vue_shared/security_reports/queries/security_report_download_paths.query.graphql';
jest.mock('~/smart_interval');
jest.mock('~/lib/utils/favicon');
Vue.use(VueApollo);
describe('MrWidgetOptions', () => {
let wrapper;
let mock;
......@@ -41,7 +46,7 @@ describe('MrWidgetOptions', () => {
gon.features = {};
});
const createComponent = (mrData = mockData) => {
const createComponent = (mrData = mockData, options = {}) => {
if (wrapper) {
wrapper.destroy();
}
......@@ -50,6 +55,7 @@ describe('MrWidgetOptions', () => {
propsData: {
mrData: { ...mrData },
},
...options,
});
return axios.waitForAll();
......@@ -815,36 +821,37 @@ describe('MrWidgetOptions', () => {
describe('security widget', () => {
describe.each`
context | hasPipeline | reportType | isFlagEnabled | shouldRender
${'security report and flag enabled'} | ${true} | ${'sast'} | ${true} | ${true}
${'security report and flag disabled'} | ${true} | ${'sast'} | ${false} | ${false}
${'no security report and flag enabled'} | ${true} | ${'foo'} | ${true} | ${false}
${'no pipeline and flag enabled'} | ${false} | ${'sast'} | ${true} | ${false}
`('given $context', ({ hasPipeline, reportType, isFlagEnabled, shouldRender }) => {
context | hasPipeline | isFlagEnabled | shouldRender
${'has pipeline and flag enabled'} | ${true} | ${true} | ${true}
${'has pipeline and flag disabled'} | ${true} | ${false} | ${false}
${'no pipeline and flag enabled'} | ${false} | ${true} | ${false}
`('given $context', ({ hasPipeline, isFlagEnabled, shouldRender }) => {
beforeEach(() => {
gon.features.coreSecurityMrWidget = isFlagEnabled;
if (hasPipeline) {
jest.spyOn(Api, 'pipelineJobs').mockResolvedValue({
data: [{ artifacts: [{ file_type: reportType }] }],
});
}
return createComponent({
const mrData = {
...mockData,
...(hasPipeline ? {} : { pipeline: undefined }),
...(hasPipeline ? {} : { pipeline: null }),
};
// Override top-level mocked requests, which always use a fresh copy of
// mockData, which always includes the full pipeline object.
mock.onGet(mockData.merge_request_widget_path).reply(() => [200, mrData]);
mock.onGet(mockData.merge_request_cached_widget_path).reply(() => [200, mrData]);
return createComponent(mrData, {
apolloProvider: createMockApollo([
[
securityReportDownloadPathsQuery,
async () => ({ data: securityReportDownloadPathsQueryResponse }),
],
]),
});
});
if (shouldRender) {
it('renders', () => {
expect(findSecurityMrWidget().exists()).toBe(true);
it(shouldRender ? 'renders' : 'does not render', () => {
expect(findSecurityMrWidget().exists()).toBe(shouldRender);
});
} else {
it('does not render', () => {
expect(findSecurityMrWidget().exists()).toBe(false);
});
}
});
});
......
spec/frontend/vue_shared/security_reports/mock_data.js
View file @ 1642b1a5
......@@ -322,6 +322,23 @@ export const secretScanningDiffSuccessMock = {
head_report_created_at: '2020-01-10T10:00:00.000Z',
};
export const securityReportDownloadPathsQueryNoArtifactsResponse = {
project: {
mergeRequest: {
headPipeline: {
id: 'gid://gitlab/Ci::Pipeline/176',
jobs: {
nodes: [],
__typename: 'CiJobConnection',
},
__typename: 'Pipeline',
},
__typename: 'MergeRequest',
},
__typename: 'Project',
},
};
export const securityReportDownloadPathsQueryResponse = {
project: {
mergeRequest: {
......
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
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7