diff --git a/ee/app/models/ee/ci/pipeline.rb b/ee/app/models/ee/ci/pipeline.rb index ad0fe9872daaa227051213a3b51f8677cf461792..c5965817dede33bad4ce27cd43415ee5efe6cb5e 100644 --- a/ee/app/models/ee/ci/pipeline.rb +++ b/ee/app/models/ee/ci/pipeline.rb @@ -130,7 +130,7 @@ def batch_lookup_report_artifact_for_file_type(file_type) end def any_report_artifact_for_type(file_type) - report_artifact_for_file_type(file_type) || legacy_report_artifact_for_file_type(file_type) + report_artifact_for_file_type(file_type) end def report_artifact_for_file_type(file_type) @@ -139,24 +139,6 @@ def report_artifact_for_file_type(file_type) job_artifacts.where(file_type: ::Ci::JobArtifact.file_types[file_type]).last end - def legacy_report_artifact_for_file_type(file_type) - return unless available_licensed_report_type?(file_type) - - legacy_names = LEGACY_REPORT_FORMATS[file_type] - return unless legacy_names - - builds.success.latest.where(name: legacy_names[:names]).each do |build| - legacy_names[:files].each do |file_name| - next unless build.has_artifact?(file_name) - - return OpenStruct.new(build: build, path: file_name) - end - end - - # In case there is no artifact return nil - nil - end - def expose_license_management_data? any_report_artifact_for_type(:license_management) end diff --git a/ee/app/presenters/ee/ci/pipeline_presenter.rb b/ee/app/presenters/ee/ci/pipeline_presenter.rb index 65200801cc223da9f9278d92b8a19fe4a8ef733a..ade2b8a6724e4a76cf03e8d1ae9c5b51edbe771d 100644 --- a/ee/app/presenters/ee/ci/pipeline_presenter.rb +++ b/ee/app/presenters/ee/ci/pipeline_presenter.rb @@ -27,20 +27,12 @@ def expose_security_dashboard? def downloadable_path_for_report_type(file_type) if (job_artifact = batch_lookup_report_artifact_for_file_type(file_type)) && can?(current_user, :read_build, job_artifact.job) - return download_project_job_artifacts_path( + download_project_job_artifacts_path( job_artifact.project, job_artifact.job, file_type: file_type, proxy: true) end - - if (build_artifact = legacy_report_artifact_for_file_type(file_type)) && - can?(current_user, :read_build, build_artifact.build) - return raw_project_job_artifacts_path( - build_artifact.build.project, - build_artifact.build, - path: build_artifact.path) - end end end end diff --git a/ee/changelogs/unreleased/sh-remove-legacy-security-reports-from-mr-widget.yml b/ee/changelogs/unreleased/sh-remove-legacy-security-reports-from-mr-widget.yml new file mode 100644 index 0000000000000000000000000000000000000000..fc44bcd24ceccc715bd0da4efd3343ac744925d5 --- /dev/null +++ b/ee/changelogs/unreleased/sh-remove-legacy-security-reports-from-mr-widget.yml @@ -0,0 +1,5 @@ +--- +title: Remove support for checking legacy security reports +merge_request: 14291 +author: +type: performance diff --git a/ee/spec/controllers/projects/pipelines_controller_spec.rb b/ee/spec/controllers/projects/pipelines_controller_spec.rb index 7778f5f6faad0dd91c4b18b3be13a34c302e3048..c411be534e07ee9c084462d643efce520ce49fe6 100644 --- a/ee/spec/controllers/projects/pipelines_controller_spec.rb +++ b/ee/spec/controllers/projects/pipelines_controller_spec.rb @@ -200,7 +200,7 @@ def get_pipeline_json(pipeline) describe 'GET security' do context 'with a sast artifact' do before do - create(:ee_ci_build, :legacy_sast, pipeline: pipeline) + create(:ee_ci_build, :sast, pipeline: pipeline) end context 'with feature enabled' do diff --git a/ee/spec/features/projects/pipelines/pipeline_spec.rb b/ee/spec/features/projects/pipelines/pipeline_spec.rb index aaca39182dfe0057094858177daf8214297ad05e..cfb536d846d8ae2ed14524386bd2f9fb94789335 100644 --- a/ee/spec/features/projects/pipelines/pipeline_spec.rb +++ b/ee/spec/features/projects/pipelines/pipeline_spec.rb @@ -98,7 +98,7 @@ context 'with a sast artifact' do before do - create(:ee_ci_build, :legacy_sast, pipeline: pipeline) + create(:ee_ci_build, :sast, pipeline: pipeline) visit security_project_pipeline_path(project, pipeline) end @@ -135,7 +135,7 @@ context 'with a license management artifact' do before do - create(:ee_ci_build, :legacy_license_management, pipeline: pipeline) + create(:ee_ci_build, :license_management, pipeline: pipeline) visit licenses_project_pipeline_path(project, pipeline) end @@ -143,7 +143,7 @@ it 'shows jobs tab pane as active' do expect(page).to have_content('Licenses') expect(page).to have_css('#js-tab-licenses') - expect(find('.js-licenses-counter')).to have_content('0') + expect(find('.js-licenses-counter')).to have_content('4') end it 'shows security report section' do diff --git a/ee/spec/models/ci/pipeline_spec.rb b/ee/spec/models/ci/pipeline_spec.rb index d2877ee22562ce7e0850354044f6603f67450e8e..193315424e9456866a08161084aae300c6677d91 100644 --- a/ee/spec/models/ci/pipeline_spec.rb +++ b/ee/spec/models/ci/pipeline_spec.rb @@ -26,26 +26,6 @@ end end - describe '#with_legacy_security_reports scope' do - let(:pipeline_1) { create(:ci_pipeline_without_jobs, project: project) } - let(:pipeline_2) { create(:ci_pipeline_without_jobs, project: project) } - let(:pipeline_3) { create(:ci_pipeline_without_jobs, project: project) } - let(:pipeline_4) { create(:ci_pipeline_without_jobs, project: project) } - let(:pipeline_5) { create(:ci_pipeline_without_jobs, project: project) } - - before do - create(:ee_ci_build, :legacy_sast, pipeline: pipeline_1) - create(:ee_ci_build, :legacy_dependency_scanning, pipeline: pipeline_2) - create(:ee_ci_build, :legacy_container_scanning, pipeline: pipeline_3) - create(:ee_ci_build, :legacy_dast, pipeline: pipeline_4) - create(:ee_ci_build, :success, :artifacts, name: 'foobar', pipeline: pipeline_5) - end - - it "returns pipeline with security reports" do - expect(described_class.with_legacy_security_reports).to contain_exactly(pipeline_1, pipeline_2, pipeline_3, pipeline_4) - end - end - describe '#with_vulnerabilities scope' do let!(:pipeline_1) { create(:ci_pipeline_without_jobs, project: project) } let!(:pipeline_2) { create(:ci_pipeline_without_jobs, project: project) } @@ -146,45 +126,6 @@ end end - describe '#legacy_report_artifact_for_file_type' do - let(:build_name) { ::EE::Ci::Pipeline::LEGACY_REPORT_FORMATS[file_type][:names].first } - let(:artifact_path) { ::EE::Ci::Pipeline::LEGACY_REPORT_FORMATS[file_type][:files].first } - - let!(:build) do - create( - :ci_build, - :success, - :artifacts, - name: build_name, - pipeline: pipeline, - options: { - artifacts: { - paths: [artifact_path] - } - } - ) - end - - subject { pipeline.legacy_report_artifact_for_file_type(file_type) } - - described_class::LEGACY_REPORT_FORMATS.each do |file_type, _| - licensed_features = described_class::REPORT_LICENSED_FEATURES[file_type] - - context "for file_type: #{file_type}" do - let(:file_type) { file_type } - let(:expected) { OpenStruct.new(build: build, path: artifact_path) } - - if licensed_features.nil? - it_behaves_like 'unlicensed report type' - elsif licensed_features.size == 1 - it_behaves_like 'licensed report type', licensed_features.first - else - it_behaves_like 'multi-licensed report type', licensed_features - end - end - end - end - describe '#security_reports' do subject { pipeline.security_reports } diff --git a/ee/spec/serializers/merge_request_widget_entity_spec.rb b/ee/spec/serializers/merge_request_widget_entity_spec.rb index efe4d5b6fc0023b9f9e1140126142b3762c2499e..360244a4791efe45e3588b844cee45f9693e0a4d 100644 --- a/ee/spec/serializers/merge_request_widget_entity_spec.rb +++ b/ee/spec/serializers/merge_request_widget_entity_spec.rb @@ -97,16 +97,6 @@ def create_all_artifacts end end - context "with legacy report artifacts" do - before do - create(:ee_ci_build, :"legacy_#{artifact_type}", pipeline: pipeline) - end - - it "has data entry" do - expect(subject.as_json).to include(json_entry) - end - end - context "without artifacts" do it "does not have data entry" do expect(subject.as_json).not_to include(json_entry) @@ -167,26 +157,11 @@ def create_all_artifacts end end - context 'when legacy artifact is defined' do - before do - create(:ee_ci_build, :legacy_license_management, pipeline: pipeline) - end - - it 'is included, if license manage management features are on' do - expect(subject.as_json).to include(:license_management) - expect(subject.as_json[:license_management]).to include(:head_path) - expect(subject.as_json[:license_management]).to include(:base_path) - expect(subject.as_json[:license_management]).to include(:managed_licenses_path) - expect(subject.as_json[:license_management]).to include(:can_manage_licenses) - expect(subject.as_json[:license_management]).to include(:license_management_full_report_path) - end - end - describe '#managed_licenses_path' do let(:managed_licenses_path) { expose_path(api_v4_projects_managed_licenses_path(id: project.id)) } before do - create(:ee_ci_build, :legacy_license_management, pipeline: pipeline) + create(:ee_ci_build, :license_management, pipeline: pipeline) end it 'is a path for target project' do