diff --git a/ee/app/models/vulnerabilities/state_transition.rb b/ee/app/models/vulnerabilities/state_transition.rb index 7d855583bef421ba43db97a7db7838e35254c16e..5d77c6761e97c746b4c39e3696d6deeef4072d1f 100644 --- a/ee/app/models/vulnerabilities/state_transition.rb +++ b/ee/app/models/vulnerabilities/state_transition.rb @@ -25,11 +25,9 @@ class StateTransition < ApplicationRecord private def to_state_and_from_state_differ - last_state_transition = self.class.where(vulnerability: vulnerability).where.not(id: id).last + return if to_state&.to_sym == :dismissed - return unless to_state == from_state && last_state_transition&.dismissal_reason == dismissal_reason - - errors.add(:to_state, "must not be the same as from_state for the same dismissal_reason") + errors.add(:to_state, "must not be the same as from_state") if to_state == from_state end end end diff --git a/ee/app/services/vulnerabilities/bulk_dismiss_service.rb b/ee/app/services/vulnerabilities/bulk_dismiss_service.rb index 8abfb614b38104596f322cebaa60b3235348dc99..9cb005431b45cef755f20826caf32b17ea205784 100644 --- a/ee/app/services/vulnerabilities/bulk_dismiss_service.rb +++ b/ee/app/services/vulnerabilities/bulk_dismiss_service.rb @@ -17,7 +17,7 @@ def execute ensure_authorized_projects! vulnerability_ids.each_slice(MAX_BATCH).each do |ids| - dismiss(Vulnerability.id_in(ids).without_states(:dismissed)) + dismiss(Vulnerability.id_in(ids)) end refresh_statistics diff --git a/ee/spec/models/vulnerabilities/state_transition_spec.rb b/ee/spec/models/vulnerabilities/state_transition_spec.rb index 8f6055fd680d077dd99fa96dda2a8f64956d593b..24d15b6d58e207b22bf4a085d8e7af01ec579940 100644 --- a/ee/spec/models/vulnerabilities/state_transition_spec.rb +++ b/ee/spec/models/vulnerabilities/state_transition_spec.rb @@ -42,7 +42,7 @@ subject.save! end.to raise_error( ActiveRecord::RecordInvalid, - 'Validation failed: To state must not be the same as from_state for the same dismissal_reason') + 'Validation failed: To state must not be the same as from_state') end context 'when the last record contains a different dismissal_reason' do @@ -51,7 +51,7 @@ end it 'does not fail the validation' do - subject.from_state = subject.to_state + subject.from_state = subject.to_state = :dismissed subject.dismissal_reason = 'acceptable_risk' expect(subject).to be_valid diff --git a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb index 223b71f6c809c2bf03cdf92bfde303e50b24681c..5781186504212d4c6efd4bd163b4ecdede99ecaf 100644 --- a/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/bulk_dismiss_service_spec.rb @@ -136,16 +136,32 @@ let_it_be(:dismissed_vulnerability) { create(:vulnerability, :with_findings, :dismissed, project: project) } let(:vulnerability_ids) { [dismissed_vulnerability.id] } - it 'does not update the vulnerability' do - expect { service.execute }.not_to change { dismissed_vulnerability.reload.dismissed_at } + it 'updates the vulnerability' do + expect { service.execute }.to change { dismissed_vulnerability.reload.dismissed_at } end - it 'does not insert a system note' do - expect { service.execute }.not_to change { Note.count } + it 'inserts a system note' do + expect { service.execute }.to change { Note.count } end - it 'does not insert a state transition' do - expect { service.execute }.not_to change { dismissed_vulnerability.state_transitions.count } + it 'inserts a state transition' do + expect { service.execute }.to change { dismissed_vulnerability.state_transitions.count } + end + + it 'inserts a new vulnerabilities reads record' do + service.execute + + reads = Vulnerabilities::Read.by_vulnerabilities(vulnerability_ids) + expect(reads.pluck(:dismissal_reason)).to match_array([dismissal_reason]) + end + + context 'when called twice with the same arguments' do + it 'creates 2 valid state transitions' do + service.execute + service.execute + + expect(dismissed_vulnerability.reload.state_transitions).to all be_valid + end end end end diff --git a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb index 69900d52806dead31c80c008866bfb7a4bac433d..25891b355addd52036ba7a948b10915ce5480cce 100644 --- a/ee/spec/services/vulnerabilities/dismiss_service_spec.rb +++ b/ee/spec/services/vulnerabilities/dismiss_service_spec.rb @@ -171,7 +171,13 @@ project.add_developer(user) end - it { expect { dismiss_vulnerability }.to raise_error Gitlab::Graphql::Errors::ArgumentError, 'To state must not be the same as from_state for the same dismissal_reason' } + it { expect { dismiss_vulnerability }.not_to raise_error } + + it 'creates a valid state transition' do + dismiss_vulnerability + + expect(vulnerability.reload.latest_state_transition).to be_valid + end context 'with a different dismissal reason' do before do