Commit f4cc5194 authored by Adam Cohen's avatar Adam Cohen Committed by Terri Chu

Remove cross-table security_report query

parent 9cb5a4eb
......@@ -185,7 +185,6 @@ module Ci
scope :order_expired_desc, -> { order(expire_at: :desc) }
scope :with_destroy_preloads, -> { includes(project: [:route, :statistics]) }
scope :scoped_project, -> { where('ci_job_artifacts.project_id = projects.id') }
scope :for_project, ->(project) { where(project_id: project) }
scope :created_in_time_range, ->(from: nil, to: nil) { where(created_at: from..to) }
......
......@@ -14,12 +14,12 @@ module EE
override :filter_projects
def filter_projects(collection)
collection = super(collection)
by_security_reports_presence(collection)
by_security_scans_presence(collection)
end
def by_security_reports_presence(collection)
def by_security_scans_presence(collection)
if params[:with_security_reports] && group.licensed_feature_available?(:security_dashboard)
collection.with_security_reports
collection.with_security_scans
else
collection
end
......
......@@ -138,7 +138,7 @@ module EE
scope :with_repositories_enabled, -> { joins(:project_feature).where(project_features: { repository_access_level: ::ProjectFeature::ENABLED }) }
scope :with_security_reports_stored, -> { where('EXISTS (?)', ::Vulnerabilities::Finding.scoped_project.select(1)) }
scope :with_security_reports, -> { where('EXISTS (?)', ::Ci::JobArtifact.security_reports.scoped_project.select(1)) }
scope :with_security_scans, -> { where('EXISTS (?)', Security::Scan.scoped_project.select(1)) }
scope :with_github_integration_pipeline_events, -> { joins(:github_integration).merge(::Integrations::Github.pipeline_hooks) }
scope :with_active_prometheus_integration, -> { joins(:prometheus_integration).merge(::Integrations::Prometheus.active) }
scope :with_enabled_incident_sla, -> { joins(:incident_management_setting).where(project_incident_management_settings: { sla_timer: true }) }
......
......@@ -29,6 +29,8 @@ module Security
scope :by_scan_types, -> (scan_types) { where(scan_type: scan_types) }
scope :scoped_project, -> { where('security_scans.project_id = projects.id') }
scope :has_dismissal_feedback, -> do
# The `category` enum on `vulnerability_feedback` table starts from 0 but the `scan_type` enum
# on `security_scans` from 1. For this reason, we have to decrease the value of `scan_type` by one
......
......@@ -39,6 +39,12 @@ FactoryBot.modify do
end
end
trait :with_security_scans do
after(:create) do |project|
create_list(:security_scan, 2, project: project)
end
end
trait :with_compliance_framework do
association :compliance_framework_setting, factory: :compliance_framework_project_setting
end
......
......@@ -29,14 +29,10 @@ RSpec.describe GroupProjectsFinder do
end
end
describe "group's projects with security reports" do
describe "group's projects with security scans" do
let(:params) { { with_security_reports: true } }
let(:project_with_reports) { create(:project, :public, group: group) }
let!(:project_without_reports) { create(:project, :public, group: group) }
before do
create(:ee_ci_job_artifact, :sast, project: project_with_reports)
end
let!(:project_with_security_scans) { create(:project, :with_security_scans, :public, group: group) }
let!(:project_without_security_scans) { create(:project, :public, group: group) }
context 'when security dashboard is enabled for a group' do
let(:group) { create(:group_with_plan, plan: :ultimate_plan) } # overriding group from 'GroupProjectsFinder context'
......@@ -46,14 +42,14 @@ RSpec.describe GroupProjectsFinder do
enable_namespace_license_check!
end
it { is_expected.to contain_exactly(project_with_reports) }
it { is_expected.to contain_exactly(project_with_security_scans) }
end
context 'when security dashboard is disabled for a group' do
let(:project_with_reports) { create(:project, :public, group: group) }
let(:project_with_security_scans) { create(:project, :public, group: group) }
# using `include` since other projects may be added to this group from different contexts
it { is_expected.to include(project_with_reports, project_without_reports) }
it { is_expected.to include(project_with_security_scans, project_without_security_scans) }
end
end
end
......@@ -311,6 +311,18 @@ RSpec.describe Project do
end
end
describe '.with_security_scans' do
it 'returns the correct project' do
project_without_security_scans = create(:project)
project_with_security_scans = create(:project, :with_security_scans)
expect(described_class.with_security_scans)
.to include(project_with_security_scans)
expect(described_class.with_security_scans)
.not_to include(project_without_security_scans)
end
end
describe '.with_github_integration_pipeline_events' do
it 'returns the correct project' do
project_with_github_integration_pipeline_events = create(:project, github_integration: create(:github_integration))
......
......@@ -603,12 +603,8 @@ RSpec.describe API::Groups do
describe "GET /groups/:id/projects" do
context "when authenticated as user" do
let(:project_with_reports) { create(:project, :public, group: group) }
let!(:project_without_reports) { create(:project, :public, group: group) }
before do
create(:ee_ci_job_artifact, :sast, project: project_with_reports)
end
let!(:project_with_security_scans) { create(:project, :with_security_scans, :public, group: group) }
let!(:project_without_security_scans) { create(:project, :public, group: group) }
subject { get api("/groups/#{group.id}/projects", user), params: { with_security_reports: true } }
......@@ -620,19 +616,19 @@ RSpec.describe API::Groups do
enable_namespace_license_check!
end
it "returns only projects with security reports" do
it "returns only projects with security scans" do
subject
expect(json_response.map { |p| p['id'] }).to contain_exactly(project_with_reports.id)
expect(json_response.map { |p| p['id'] }).to contain_exactly(project_with_security_scans.id)
end
end
context 'when security dashboard is disabled for a group' do
it "returns all projects regardless of the security reports" do
it "returns all projects regardless of the security scans" do
subject
# using `include` since other projects may be added to this group from different contexts
expect(json_response.map { |p| p['id'] }).to include(project_with_reports.id, project_without_reports.id)
expect(json_response.map { |p| p['id'] }).to include(project_with_security_scans.id, project_without_security_scans.id)
end
end
end
......
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