Merge branch '262112_synch_dismissal_information_always' into 'master'

Synch dismissal information between vulnerabilities and findings

See merge request gitlab-org/gitlab!46141

ee/app/models/vulnerabilities/feedback.rb

@@ -90,5 +90,13 @@ module Vulnerabilities
     def touch_pipeline
       pipeline&.touch
     end
+
+    def finding
+      Finding.find_by(
+        project_id: project_id,
+        report_type: category,
+        project_fingerprint: project_fingerprint
+      )
+    end
   end
 end

ee/app/services/vulnerabilities/confirm_service.rb

@@ -7,9 +7,13 @@ module Vulnerabilities
     def execute
       raise Gitlab::Access::AccessDeniedError unless authorized?
 
-      @vulnerability.tap do |vulnerability|
-        update_with_note(vulnerability, state: Vulnerability.states[:confirmed], confirmed_by: @user, confirmed_at: Time.current)
+      @vulnerability.transaction do
+        DestroyDismissalFeedbackService.new(@user, @vulnerability).execute
+
+        update_with_note(@vulnerability, state: Vulnerability.states[:confirmed], confirmed_by: @user, confirmed_at: Time.current)
       end
+
+      @vulnerability
     end
   end
 end

ee/app/services/vulnerabilities/destroy_dismissal_feedback_service.rb

+# frozen_string_literal: true
+
+require_dependency 'vulnerabilities/base_service'
+
+# This service class removes all the dismissal feedback
+# associated with a vulnerability through it's findings.
+module Vulnerabilities
+  class DestroyDismissalFeedbackService < BaseService
+    def execute
+      @vulnerability.dismissed_findings.each do |finding|
+        unless destroy_feedback_for(finding)
+          handle_finding_revert_error(finding)
+          raise ActiveRecord::Rollback
+        end
+      end
+    end
+
+    private
+
+    def destroy_feedback_for(finding)
+      VulnerabilityFeedback::DestroyService
+        .new(@project, @user, finding.dismissal_feedback, revert_vulnerability_state: false)
+        .execute
+    end
+
+    def handle_finding_revert_error(finding)
+      @vulnerability.errors.add(
+        :base,
+        :finding_revert_to_detected_error,
+        message: _("failed to revert associated finding(id=%{finding_id}) to detected") %
+          {
+            finding_id: finding.id
+          })
+    end
+  end
+end

ee/app/services/vulnerabilities/dismiss_service.rb

@@ -6,20 +6,23 @@ module Vulnerabilities
   class DismissService < BaseService
     FindingsDismissResult = Struct.new(:ok?, :finding, :message)
 
-    def initialize(current_user, vulnerability, comment = nil)
+    def initialize(current_user, vulnerability, comment = nil, dismiss_findings: true)
       super(current_user, vulnerability)
       @comment = comment
+      @dismiss_findings = dismiss_findings
     end
 
     def execute
       raise Gitlab::Access::AccessDeniedError unless authorized?
 
       @vulnerability.transaction do
-        result = dismiss_findings
+        if dismiss_findings
+          result = dismiss_vulnerability_findings
 
-        unless result.ok?
-          handle_finding_dismissal_error(result.finding, result.message)
-          raise ActiveRecord::Rollback
+          unless result.ok?
+            handle_finding_dismissal_error(result.finding, result.message)
+            raise ActiveRecord::Rollback
+          end
         end
 
         update_with_note(@vulnerability, state: Vulnerability.states[:dismissed], dismissed_by: @user, dismissed_at: Time.current)
@@ -30,6 +33,8 @@ module Vulnerabilities
 
     private
 
+    attr_reader :dismiss_findings
+
     def feedback_service_for(finding)
       VulnerabilityFeedback::CreateService.new(@project, @user, feedback_params_for(finding))
     end
@@ -40,11 +45,12 @@ module Vulnerabilities
         feedback_type: 'dismissal',
         project_fingerprint: finding.project_fingerprint,
         comment: @comment,
-        pipeline: @project.latest_pipeline_with_security_reports(only_successful: true)
+        pipeline: @project.latest_pipeline_with_security_reports(only_successful: true),
+        dismiss_vulnerability: false
       }
     end
 
-    def dismiss_findings
+    def dismiss_vulnerability_findings
       @vulnerability.findings.each do |finding|
         result = feedback_service_for(finding).execute
 

ee/app/services/vulnerabilities/resolve_service.rb

@@ -7,9 +7,13 @@ module Vulnerabilities
     def execute
       raise Gitlab::Access::AccessDeniedError unless authorized?
 
-      @vulnerability.tap do |vulnerability|
-        update_with_note(vulnerability, state: Vulnerability.states[:resolved], resolved_by: @user, resolved_at: Time.current)
+      @vulnerability.transaction do
+        DestroyDismissalFeedbackService.new(@user, @vulnerability).execute
+
+        update_with_note(@vulnerability, state: Vulnerability.states[:resolved], resolved_by: @user, resolved_at: Time.current)
       end
+
+      @vulnerability
     end
   end
 end

ee/app/services/vulnerabilities/revert_to_detected_service.rb

@@ -10,46 +10,12 @@ module Vulnerabilities
       raise Gitlab::Access::AccessDeniedError unless authorized?
 
       @vulnerability.transaction do
-        revert_result = revert_findings_to_detected_state
-        raise ActiveRecord::Rollback unless revert_result
+        DestroyDismissalFeedbackService.new(@user, @vulnerability).execute
 
         update_with_note(@vulnerability, state: Vulnerability.states[:detected], **REVERT_PARAMS)
       end
 
       @vulnerability
     end
-
-    private
-
-    def destroy_feedback_for(finding)
-      VulnerabilityFeedback::DestroyService
-        .new(@project, @user, finding.dismissal_feedback)
-        .execute
-    end
-
-    def revert_findings_to_detected_state
-      @vulnerability
-        .dismissed_findings
-        .each do |finding|
-          result = destroy_feedback_for(finding)
-
-          unless result
-            handle_finding_revert_error(finding)
-            return false
-          end
-        end
-
-      true
-    end
-
-    def handle_finding_revert_error(finding)
-      @vulnerability.errors.add(
-        :base,
-        :finding_revert_to_detected_error,
-        message: _("failed to revert associated finding(id=%{finding_id}) to detected") %
-          {
-            finding_id: finding.id
-          })
-    end
   end
 end

ee/app/services/vulnerability_feedback/create_service.rb

@@ -2,21 +2,17 @@
 
 module VulnerabilityFeedback
   class CreateService < ::BaseService
-    def execute
-      vulnerability_feedback = @project.vulnerability_feedback.find_or_init_for(create_params)
+    include Gitlab::Utils::StrongMemoize
 
+    def execute
       raise Gitlab::Access::AccessDeniedError unless can?(current_user, :create_vulnerability_feedback, vulnerability_feedback)
 
-      if vulnerability_feedback.for_issue? &&
-          !vulnerability_feedback.vulnerability_data.blank?
-
-        create_issue_for(vulnerability_feedback)
-      elsif vulnerability_feedback.for_merge_request? &&
-          !vulnerability_feedback.vulnerability_data.blank?
-
-        create_merge_request_for(vulnerability_feedback)
+      if vulnerability_feedback.for_issue? && !vulnerability_feedback.vulnerability_data.blank?
+        create_issue
+      elsif vulnerability_feedback.for_merge_request? && !vulnerability_feedback.vulnerability_data.blank?
+        create_merge_request
       else
-        vulnerability_feedback.save
+        dismiss_existing_vulnerability
       end
 
       if vulnerability_feedback.persisted? && vulnerability_feedback.valid?
@@ -34,9 +30,15 @@ module VulnerabilityFeedback
 
     private
 
+    attr_reader :params
+
+    def vulnerability_feedback
+      @vulnerability_feedback ||= @project.vulnerability_feedback.find_or_init_for(create_params)
+    end
+
     def create_params
       @params[:author] = @current_user
-      @params.merge(comment_params)
+      @params.merge(comment_params).except(:dismiss_vulnerability)
     end
 
     def comment_params
@@ -52,7 +54,7 @@ module VulnerabilityFeedback
       super().merge(vulnerability_feedback: vulnerability_feedback)
     end
 
-    def create_issue_for(vulnerability_feedback)
+    def create_issue
       # Wrap Feedback and Issue creation in the same transaction
       ActiveRecord::Base.transaction do
         result = Issues::CreateFromVulnerabilityDataService
@@ -80,7 +82,7 @@ module VulnerabilityFeedback
       end
     end
 
-    def create_merge_request_for(vulnerability_feedback)
+    def create_merge_request
       result = MergeRequests::CreateFromVulnerabilityDataService
         .new(@project, @current_user, vulnerability_feedback.vulnerability_data)
         .execute
@@ -115,5 +117,26 @@ module VulnerabilityFeedback
         .new(current_user, vulnerability, issue, link_type: Vulnerabilities::IssueLink.link_types[:created])
         .execute
     end
+
+    def dismiss_existing_vulnerability
+      ActiveRecord::Base.transaction do
+        if dismiss_vulnerability? && existing_vulnerability
+          Vulnerabilities::DismissService.new(current_user,
+                                              existing_vulnerability,
+                                              params[:comment],
+                                              dismiss_findings: false).execute
+        end
+
+        raise ActiveRecord::Rollback unless vulnerability_feedback.save
+      end
+    end
+
+    def existing_vulnerability
+      strong_memoize(:existing_vulnerability) { vulnerability_feedback.finding&.vulnerability }
+    end
+
+    def dismiss_vulnerability?
+      params[:dismiss_vulnerability] != false && can?(current_user, :admin_vulnerability, project)
+    end
   end
 end

ee/app/services/vulnerability_feedback/destroy_service.rb

@@ -2,15 +2,34 @@
 
 module VulnerabilityFeedback
   class DestroyService < ::BaseService
-    def initialize(project, user, vulnerability_feedback)
-      @project, @current_user, @vulnerability_feedback = project, user, vulnerability_feedback
+    include Gitlab::Utils::StrongMemoize
+
+    def initialize(project, user, vulnerability_feedback, revert_vulnerability_state: true)
+      @project, @current_user, @vulnerability_feedback, @revert_vulnerability_state = project, user, vulnerability_feedback, revert_vulnerability_state
     end
 
     def execute
       # TODO: Add system note when destroying a dismissal feedback
-      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_vulnerability_feedback, @vulnerability_feedback)
+      raise Gitlab::Access::AccessDeniedError unless can?(current_user, :destroy_vulnerability_feedback, vulnerability_feedback)
+
+      revert_vulnerability if revert_vulnerability_state?
+      vulnerability_feedback.destroy
+    end
+
+    private
+
+    attr_reader :vulnerability_feedback, :revert_vulnerability_state
+
+    def revert_vulnerability
+      Vulnerabilities::RevertToDetectedService.new(current_user, existing_vulnerability).execute
+    end
+
+    def revert_vulnerability_state?
+      revert_vulnerability_state && existing_vulnerability
+    end
 
-      @vulnerability_feedback.destroy
+    def existing_vulnerability
+      strong_memoize(:existing_vulnerability) { vulnerability_feedback.finding&.vulnerability }
     end
   end
 end

ee/changelogs/unreleased/262112_synch_dismissal_information_always.yml

+---
+title: Synchronize dismissal information between `finding` and `vulnerability` entries
+merge_request: 46141
+author:
+type: fixed

ee/spec/models/vulnerabilities/feedback_spec.rb

@@ -235,4 +235,25 @@ RSpec.describe Vulnerabilities::Feedback do
 
     it { is_expected.to eq({ project_id: project.id, category: category, project_fingerprint: project_fingerprint }) }
   end
+
+  describe '#finding' do
+    let_it_be(:feedback) { create(:vulnerability_feedback) }
+
+    subject { feedback.finding }
+
+    context 'when the is no finding persisted' do
+      it { is_expected.to be_nil }
+    end
+
+    context 'when there is a persisted finding' do
+      let!(:finding) do
+        create(:vulnerabilities_finding,
+               project: feedback.project,
+               report_type: feedback.category,
+               project_fingerprint: feedback.project_fingerprint)
+      end
+
+      it { is_expected.to eq(finding) }
+    end
+  end
 end

ee/spec/services/vulnerabilities/confirm_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ConfirmService do
     end
 
     it_behaves_like 'calls vulnerability statistics utility services in order'
+    it_behaves_like 'removes dismissal feedback from associated findings'
 
     it 'confirms a vulnerability' do
       Timecop.freeze do

ee/spec/services/vulnerabilities/destroy_dismissal_feedback_service_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+RSpec.describe Vulnerabilities::DestroyDismissalFeedbackService do
+  let_it_be(:user) { create(:user) }
+  let_it_be(:project) { create(:project) }
+  let_it_be(:vulnerability) { create(:vulnerability, project: project) }
+
+  before(:all) do
+    finding_1 = create(:vulnerabilities_finding, project: project)
+    finding_2 = create(:vulnerabilities_finding, project: project)
+
+    create(:vulnerability_feedback, project: project, category: finding_1.report_type, project_fingerprint: finding_1.project_fingerprint)
+    create(:vulnerability_feedback, project: project, category: finding_2.report_type, project_fingerprint: finding_2.project_fingerprint)
+    create(:vulnerability_feedback)
+
+    vulnerability.findings << finding_1
+    vulnerability.findings << finding_2
+  end
+
+  describe '#execute' do
+    subject(:destroy_feedback) { described_class.new(user, vulnerability).execute }
+
+    context 'without necessary permissions' do
+      it 'raises `Gitlab::Access::AccessDeniedError` error' do
+        expect { destroy_feedback }.to raise_error(Gitlab::Access::AccessDeniedError)
+                                   .and not_change { Vulnerabilities::Feedback.count }
+      end
+    end
+
+    context 'with necessary permissions' do
+      before do
+        project.add_developer(user)
+      end
+
+      it 'destroys the feedback records associated with the findings of the given vulnerability' do
+        expect { destroy_feedback }.to change { Vulnerabilities::Feedback.count }.from(3).to(1)
+      end
+    end
+  end
+end

ee/spec/services/vulnerabilities/dismiss_service_spec.rb

@@ -14,7 +14,8 @@ RSpec.describe Vulnerabilities::DismissService do
   let!(:pipeline) { create(:ci_pipeline, :success, project: project) }
   let!(:build) { create(:ee_ci_build, :sast, pipeline: pipeline) }
   let(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
-  let(:service) { described_class.new(user, vulnerability) }
+  let(:dismiss_findings) { true }
+  let(:service) { described_class.new(user, vulnerability, dismiss_findings: dismiss_findings) }
 
   subject(:dismiss_vulnerability) { service.execute }
 
@@ -25,13 +26,29 @@ RSpec.describe Vulnerabilities::DismissService do
 
     it_behaves_like 'calls vulnerability statistics utility services in order'
 
-    it 'dismisses a vulnerability and its associated findings' do
-      Timecop.freeze do
-        dismiss_vulnerability
+    context 'when the `dismiss_findings` argument is false' do
+      let(:dismiss_findings) { false }
 
-        expect(vulnerability.reload).to(
-          have_attributes(state: 'dismissed', dismissed_by: user, dismissed_at: be_like_time(Time.current)))
-        expect(vulnerability.findings).to all have_vulnerability_dismissal_feedback
+      it 'dismisses only vulnerability' do
+        Timecop.freeze do
+          dismiss_vulnerability
+
+          expect(vulnerability.reload).to(
+            have_attributes(state: 'dismissed', dismissed_by: user, dismissed_at: be_like_time(Time.current)))
+          expect(vulnerability.findings).not_to include have_vulnerability_dismissal_feedback
+        end
+      end
+    end
+
+    context 'when the `dismiss_findings` argument is not false' do
+      it 'dismisses a vulnerability and its associated findings' do
+        Timecop.freeze do
+          dismiss_vulnerability
+
+          expect(vulnerability.reload).to(
+            have_attributes(state: 'dismissed', dismissed_by: user, dismissed_at: be_like_time(Time.current)))
+          expect(vulnerability.findings).to all have_vulnerability_dismissal_feedback
+        end
       end
     end
 
@@ -62,7 +79,7 @@ RSpec.describe Vulnerabilities::DismissService do
 
     context 'when there is a finding dismissal error' do
       before do
-        allow(service).to receive(:dismiss_findings).and_return(
+        allow(service).to receive(:dismiss_vulnerability_findings).and_return(
           described_class::FindingsDismissResult.new(false, broken_finding, 'something went wrong'))
       end
 

ee/spec/services/vulnerabilities/resolve_service_spec.rb

@@ -22,6 +22,7 @@ RSpec.describe Vulnerabilities::ResolveService do
     end
 
     it_behaves_like 'calls vulnerability statistics utility services in order'
+    it_behaves_like 'removes dismissal feedback from associated findings'
 
     it 'resolves a vulnerability' do
       Timecop.freeze do

ee/spec/services/vulnerabilities/revert_to_detected_service_spec.rb

@@ -23,7 +23,6 @@ RSpec.describe Vulnerabilities::RevertToDetectedService do
 
         expect(vulnerability.reload).to(
           have_attributes(state: 'detected', dismissed_by: nil, dismissed_at: nil, resolved_by: nil, resolved_at: nil, confirmed_by: nil, confirmed_at: nil))
-        expect(vulnerability.findings).to all not_have_vulnerability_dismissal_feedback
       end
     end
 
@@ -34,6 +33,7 @@ RSpec.describe Vulnerabilities::RevertToDetectedService do
     end
 
     it_behaves_like 'calls vulnerability statistics utility services in order'
+    it_behaves_like 'removes dismissal feedback from associated findings'
   end
 
   context 'with an authorized user with proper permissions' do
@@ -59,23 +59,6 @@ RSpec.describe Vulnerabilities::RevertToDetectedService do
       include_examples 'reverts vulnerability'
     end
 
-    context 'when there is an error' do
-      let(:broken_finding) { vulnerability.findings.first }
-
-      let!(:dismissal_feedback) do
-        create(:vulnerability_feedback, :dismissal, project: broken_finding.project, project_fingerprint: broken_finding.project_fingerprint)
-      end
-
-      before do
-        allow(service).to receive(:destroy_feedback_for).and_return(false)
-      end
-
-      it 'responds with error' do
-        expect(revert_vulnerability_to_detected.errors.messages).to eq(
-          base: ["failed to revert associated finding(id=#{broken_finding.id}) to detected"])
-      end
-    end
-
     context 'when security dashboard feature is disabled' do
       before do
         stub_licensed_features(security_dashboard: false)

ee/spec/services/vulnerability_feedback/create_service_spec.rb

@@ -7,20 +7,24 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
   let(:project) { create(:project, :public, :repository, namespace: group) }
   let(:user)    { create(:user) }
   let(:pipeline) { create(:ci_pipeline, project: project) }
+  let(:dismiss_vulnerability) { true }
 
   before do
     group.add_developer(user)
   end
 
   context 'when params are valid' do
+    let(:category) { 'sast' }
+    let(:project_fingerprint) { '418291a26024a1445b23fe64de9380cdcdfd1fa8' }
     let(:feedback_params) do
       {
-        feedback_type: 'dismissal', pipeline_id: pipeline.id, category: 'sast',
-        project_fingerprint: '418291a26024a1445b23fe64de9380cdcdfd1fa8',
+        feedback_type: 'dismissal', pipeline_id: pipeline.id, category: category,
+        project_fingerprint: project_fingerprint,
         comment: 'a dismissal comment',
+        dismiss_vulnerability: dismiss_vulnerability,
         vulnerability_data: {
           blob_path: '/path/to/blob',
-          category: 'sast',
+          category: category,
           priority: 'Low', line: '41',
           file: 'subdir/src/main/java/com/gitlab/security_products/tests/App.java',
           cve: '818bf5dacb291e15d9e6dc3c5ac32178:PREDICTABLE_RANDOM',
@@ -42,6 +46,13 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
 
     context 'when feedback_type is dismissal' do
       let(:result) { described_class.new(project, user, feedback_params).execute }
+      let!(:finding) do
+        create(:vulnerabilities_finding,
+               :detected,
+               project: project,
+               report_type: category,
+               project_fingerprint: project_fingerprint)
+      end
 
       it 'creates the feedback with the given params' do
         expect(result[:status]).to eq(:success)
@@ -51,8 +62,8 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
         expect(feedback.author).to eq(user)
         expect(feedback.feedback_type).to eq('dismissal')
         expect(feedback.pipeline_id).to eq(pipeline.id)
-        expect(feedback.category).to eq('sast')
-        expect(feedback.project_fingerprint).to eq('418291a26024a1445b23fe64de9380cdcdfd1fa8')
+        expect(feedback.category).to eq(category)
+        expect(feedback.project_fingerprint).to eq(project_fingerprint)
         expect(feedback.for_dismissal?).to eq(true)
         expect(feedback.for_issue?).to eq(false)
         expect(feedback.issue).to be_nil
@@ -87,6 +98,44 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
           expect(feedback.comment_timestamp).to be_nil
         end
       end
+
+      context 'when the `dismiss_vulnerability` argument is true' do
+        context 'when the security_dashboard is not enabled' do
+          it 'does not dismiss the existing vulnerability' do
+            expect { result }.not_to change { finding.vulnerability.reload.state }.from('detected')
+          end
+        end
+
+        context 'when the security_dashboard is enabled' do
+          before do
+            stub_licensed_features(security_dashboard: true)
+          end
+
+          it 'dismisses the existing vulnerability' do
+            expect { result }.to change { finding.vulnerability.reload.state }.from('detected').to('dismissed')
+          end
+        end
+      end
+
+      context 'when the `dismiss_vulnerability` argument is false' do
+        let(:dismiss_vulnerability) { false }
+
+        context 'when the security_dashboard is not enabled' do
+          it 'does not dismiss the existing vulnerability' do
+            expect { result }.not_to change { finding.vulnerability.reload.state }.from('detected')
+          end
+        end
+
+        context 'when the security_dashboard is enabled' do
+          before do
+            stub_licensed_features(security_dashboard: true)
+          end
+
+          it 'dismisses the existing vulnerability' do
+            expect { result }.not_to change { finding.vulnerability.reload.state }.from('detected')
+          end
+        end
+      end
     end
 
     context 'when feedback_type is issue' do
@@ -145,7 +194,7 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
           feedback_params.deep_merge(
             feedback_type: 'issue',
             vulnerability_data: { vulnerability_id: vulnerability_id }
-          )
+          ).except(:dismiss_vulnerability)
         end
 
         subject(:result) { described_class.new(project, user, feedback_params_with_vulnerability_id).execute }

ee/spec/services/vulnerability_feedback/destroy_service_spec.rb

@@ -3,32 +3,67 @@
 require 'spec_helper'
 
 RSpec.describe VulnerabilityFeedback::DestroyService, '#execute' do
-  let(:group)   { create(:group) }
-  let(:project) { create(:project, :public, :repository, namespace: group) }
+  let(:project) { create(:project, :public, :repository) }
   let(:user)    { create(:user) }
   let(:vulnerability_feedback) { create(:vulnerability_feedback, feedback_type, project: project)}
+  let(:revert_vulnerability_state) { true }
+  let(:service_object) { described_class.new(project, user, vulnerability_feedback, revert_vulnerability_state: revert_vulnerability_state) }
 
   before do
-    group.add_developer(user)
+    project.add_developer(user)
+    stub_licensed_features(security_dashboard: true)
   end
 
-  subject { described_class.new(project, user, vulnerability_feedback).execute }
+  subject(:destroy_feedback) { service_object.execute }
 
   context 'when feedback_type is dismissal' do
     let(:feedback_type) { :dismissal }
 
-    it 'destroys the feedback' do
-      subject
+    context 'when the user is authorized' do
+      context 'when the `revert_vulnerability_state` argument is set as true' do
+        context 'when the finding is not associated with a vulnerability' do
+          it 'destroys the feedback' do
+            expect { destroy_feedback }.to change { vulnerability_feedback.destroyed? }.to(true)
+          end
+        end
 
-      expect { vulnerability_feedback.reload }.to raise_error ActiveRecord::RecordNotFound
+        context 'when the finding is associated with a vulnerability' do
+          let(:finding) { create(:vulnerabilities_finding, :dismissed, project: project) }
+          let(:vulnerability_feedback) { finding.dismissal_feedback }
+
+          it 'changes the state of the vulnerability to `detected`' do
+            expect { destroy_feedback }.to change { finding.vulnerability.reload.state }.from('dismissed').to('detected')
+          end
+        end
+      end
+
+      context 'when the `revert_vulnerability_state` argument is set as false' do
+        let(:revert_vulnerability_state) { false }
+
+        context 'when the finding is not associated with a vulnerability' do
+          it 'destroys the feedback' do
+            expect { destroy_feedback }.to change { vulnerability_feedback.destroyed? }.to(true)
+          end
+        end
+
+        context 'when the finding is associated with a vulnerability' do
+          let(:finding) { create(:vulnerabilities_finding, :dismissed, project: project) }
+          let(:vulnerability_feedback) { finding.dismissal_feedback }
+
+          it 'does not change the state of the vulnerability to `detected`' do
+            expect { destroy_feedback }.not_to change { finding.vulnerability.reload.state }
+          end
+        end
+      end
     end
 
     context 'when user is not authorized' do
-      let(:unauthorized_user) { create(:user) }
+      before do
+        project.add_guest(user)
+      end
 
       it 'raise error if permission is denied' do
-        expect { described_class.new(project, unauthorized_user, vulnerability_feedback).execute }
-          .to raise_error(Gitlab::Access::AccessDeniedError)
+        expect { destroy_feedback }.to raise_error(Gitlab::Access::AccessDeniedError)
       end
     end
   end
@@ -37,8 +72,7 @@ RSpec.describe VulnerabilityFeedback::DestroyService, '#execute' do
     let(:feedback_type) { :issue }
 
     it 'raise error as this type of feedback can not be destroyed' do
-      expect { described_class.new(project, user, vulnerability_feedback).execute }
-        .to raise_error(Gitlab::Access::AccessDeniedError)
+      expect { destroy_feedback }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 
@@ -46,8 +80,7 @@ RSpec.describe VulnerabilityFeedback::DestroyService, '#execute' do
     let(:feedback_type) { :merge_request }
 
     it 'raise error as this type of feedback can not be destroyed' do
-      expect { described_class.new(project, user, vulnerability_feedback).execute }
-        .to raise_error(Gitlab::Access::AccessDeniedError)
+      expect { destroy_feedback }.to raise_error(Gitlab::Access::AccessDeniedError)
     end
   end
 end

ee/spec/support/shared_examples/services/vulnerabilities/removes_dismissal_feedback_from_associated_findings_shared_example.rb

+# frozen_string_literal: true
+
+RSpec.shared_examples 'removes dismissal feedback from associated findings' do
+  let(:finding) { create(:vulnerabilities_finding, vulnerability: vulnerability, project: vulnerability.project) }
+
+  before do
+    create(:vulnerability_feedback,
+           :dismissal,
+           project: finding.project,
+           category: finding.report_type,
+           project_fingerprint: finding.project_fingerprint)
+  end
+
+  context 'when there is no error' do
+    it 'removes dismissal feedback from associated findings' do
+      expect { subject }.to change { Vulnerabilities::Feedback.count }.by(-1)
+    end
+  end
+
+  context 'when there is an error' do
+    before do
+      allow_next_instance_of(VulnerabilityFeedback::DestroyService) do |destroy_service_object|
+        allow(destroy_service_object).to receive(:execute).and_return(false)
+      end
+    end
+
+    it 'does not remove any feedback' do
+      expect { subject }.not_to change { Vulnerabilities::Feedback.count }
+    end
+
+    it 'responds with error' do
+      expect(subject.errors.messages).to eq(
+        base: ["failed to revert associated finding(id=#{finding.id}) to detected"])
+    end
+  end
+end