Merge branch '336590-remove-cross-table-join' into 'master'

Remove cross-table security_report query

See merge request gitlab-org/gitlab!70031

app/models/ci/job_artifact.rb

@@ -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) }
 

ee/app/finders/ee/group_projects_finder.rb

@@ -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

ee/app/models/ee/project.rb

@@ -145,7 +145,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 }) }

ee/app/models/security/scan.rb

@@ -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

ee/spec/factories/projects.rb

@@ -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

ee/spec/finders/group_projects_finder_spec.rb

@@ -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

ee/spec/models/project_spec.rb

@@ -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))

ee/spec/requests/api/groups_spec.rb

@@ -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