Merge branch '336763-remove-ci-join-from-vulnerability-feedback-controller' into 'master'

Replace Vulnerabilities::Feedback.only_valid_feedback without joining ci

See merge request gitlab-org/gitlab!67010

ee/app/controllers/projects/vulnerability_feedback_controller.rb

@@ -17,9 +17,6 @@ class Projects::VulnerabilityFeedbackController < Projects::ApplicationControlle
     # TODO: Move to finder or list service
     @vulnerability_feedback = @project.vulnerability_feedback.with_associations
 
-    # TODO remove once filtered data has been cleaned
-    @vulnerability_feedback = @vulnerability_feedback.only_valid_feedback
-
     if params[:category].present?
       @vulnerability_feedback = @vulnerability_feedback
         .with_category(params[:category])

ee/app/models/vulnerabilities/feedback.rb

@@ -38,13 +38,6 @@ module Vulnerabilities
     after_save :touch_pipeline, if: :for_dismissal?
     after_destroy :touch_pipeline, if: :for_dismissal?
 
-    # TODO remove once filtered data has been cleaned
-    def self.only_valid_feedback
-      pipeline = Ci::Pipeline.arel_table
-      feedback = arel_table
-      joins(:pipeline).where(pipeline[:project_id].eq(feedback[:project_id]))
-    end
-
     def self.find_or_init_for(feedback_params)
       validate_enums(feedback_params)
 

ee/app/serializers/vulnerabilities/feedback_entity.rb

@@ -14,7 +14,8 @@ class Vulnerabilities::FeedbackEntity < Grape::Entity
   end
   expose :finding_uuid
 
-  expose :pipeline, if: -> (feedback, _) { feedback.pipeline.present? } do
+  # TODO: Remove project_id comparison once bad/invalid data has been deleted https://gitlab.com/gitlab-org/gitlab/-/issues/218837
+  expose :pipeline, if: -> (feedback, _) { feedback.pipeline.present? && feedback.pipeline.project_id == feedback.project_id } do
     expose :id do |feedback|
       feedback.pipeline.id
     end

ee/spec/controllers/projects/vulnerability_feedback_controller_spec.rb

@@ -42,8 +42,8 @@ RSpec.describe Projects::VulnerabilityFeedbackController do
         expect(json_response.length).to eq 5
       end
 
-      # TODO remove once filtered data has been cleaned
-      context 'with invalid vulnerability_feedback' do
+      # TODO: Remove once bad/invalid data has been deleted https://gitlab.com/gitlab-org/gitlab/-/issues/218837
+      context 'when the pipeline has been set to another project' do
         let!(:vuln_feedback_in_other_proj) do
           feedback = build(
             :vulnerability_feedback,
@@ -52,14 +52,20 @@ RSpec.describe Projects::VulnerabilityFeedbackController do
             pipeline: create(:ci_pipeline)
           )
 
+          # Simulating vulnerable data that was in the DB before we introduced
+          # the validation preventing
           feedback.save!(validate: false)
+
+          feedback
         end
 
-        it 'ignores feedback in other project' do
+        it 'does not present the pipeline' do
           list_feedbacks
 
           expect(response).to match_response_schema('vulnerability_feedback_list', dir: 'ee')
-          expect(json_response.length).to eq 5
+          expect(json_response.length).to eq 6
+          feedback_with_invalid_pipeline_response = json_response.find { |r| r['id'] == vuln_feedback_in_other_proj.id }
+          expect(feedback_with_invalid_pipeline_response['pipeline']).to be_nil
         end
       end
 

ee/spec/models/vulnerabilities/feedback_spec.rb

@@ -189,26 +189,6 @@ RSpec.describe Vulnerabilities::Feedback do
     end
   end
 
-  # TODO remove once filtered data has been cleaned
-  describe '::only_valid_feedback' do
-    let(:project) { create(:project) }
-    let(:pipeline) { create(:ci_pipeline, project: project) }
-
-    let!(:feedback) { create(:vulnerability_feedback, :dismissal, :sast, project: project, pipeline: pipeline) }
-    let!(:invalid_feedback) do
-      feedback = build(:vulnerability_feedback, project: project, pipeline: create(:ci_pipeline))
-
-      feedback.save(validate: false)
-    end
-
-    it 'filters out invalid feedback' do
-      feedback_records = described_class.only_valid_feedback
-
-      expect(feedback_records.length).to eq 1
-      expect(feedback_records.first).to eq feedback
-    end
-  end
-
   describe '#has_comment?' do
     let_it_be(:project) { create(:project) }