Commit 865c5617 authored by Jonathan Schafer's avatar Jonathan Schafer

Remove endpoint response code

This also removed unnecessary validation code and code that pulls
the finding from the vulnerability id
parent e52e99bb
...@@ -46,7 +46,6 @@ module Vulnerabilities ...@@ -46,7 +46,6 @@ module Vulnerabilities
def self.find_or_init_for(feedback_params) def self.find_or_init_for(feedback_params)
validate_enums(feedback_params) validate_enums(feedback_params)
validate_finding_uuid(feedback_params[:finding_uuid]) if feedback_params[:finding_uuid]
record = find_or_initialize_by(feedback_params.slice(:category, :feedback_type, :project_fingerprint)) record = find_or_initialize_by(feedback_params.slice(:category, :feedback_type, :project_fingerprint))
record.assign_attributes(feedback_params) record.assign_attributes(feedback_params)
...@@ -66,10 +65,6 @@ module Vulnerabilities ...@@ -66,10 +65,6 @@ module Vulnerabilities
end end
end end
def self.validate_finding_uuid(finding_uuid)
raise ArgumentError.new("No finding with the UUID '#{finding_uuid}'") if Vulnerabilities::Finding.where(uuid: finding_uuid).blank?
end
def self.with_category(category) def self.with_category(category)
where(category: category) where(category: category)
end end
......
...@@ -47,7 +47,6 @@ class Vulnerabilities::FeedbackEntity < Grape::Entity ...@@ -47,7 +47,6 @@ class Vulnerabilities::FeedbackEntity < Grape::Entity
feedback&.pipeline&.ref feedback&.pipeline&.ref
end end
expose :project_fingerprint expose :project_fingerprint
expose :finding_uuid
alias_method :feedback, :object alias_method :feedback, :object
......
...@@ -15,10 +15,6 @@ module VulnerabilityFeedback ...@@ -15,10 +15,6 @@ module VulnerabilityFeedback
dismiss_existing_vulnerability dismiss_existing_vulnerability
end end
if !vulnerability_feedback.vulnerability_data.blank? && vulnerability_feedback.vulnerability_data[:vulnerability_id]
vulnerability_feedback.finding_uuid = Vulnerability.find(vulnerability_feedback.vulnerability_data[:vulnerability_id]).finding.uuid
end
if vulnerability_feedback.persisted? && vulnerability_feedback.valid? if vulnerability_feedback.persisted? && vulnerability_feedback.valid?
success(vulnerability_feedback) success(vulnerability_feedback)
else else
......
---
title: Insert finding_uuid value into vulnerability_feedback when creating records
merge_request: 49408
author:
type: changed
...@@ -36,8 +36,7 @@ ...@@ -36,8 +36,7 @@
}, },
"project_fingerprint": { "type": "string" }, "project_fingerprint": { "type": "string" },
"branch": { "type": ["string", "null"] }, "branch": { "type": ["string", "null"] },
"destroy_vulnerability_feedback_dismissal_path": { "type": "string" }, "destroy_vulnerability_feedback_dismissal_path": { "type": "string" }
"finding_uuid": { "type": ["string", "null"] }
}, },
"additionalProperties": false "additionalProperties": false
} }
...@@ -283,14 +283,26 @@ RSpec.describe Vulnerabilities::Feedback do ...@@ -283,14 +283,26 @@ RSpec.describe Vulnerabilities::Feedback do
subject(:feedback) { described_class.find_or_init_for(feedback_params_with_finding) } subject(:feedback) { described_class.find_or_init_for(feedback_params_with_finding) }
it 'validates and sets finding_uuid' do it 'sets finding_uuid' do
feedback_params[:finding_uuid] = finding.uuid
feedback.save! feedback.save!
expect(feedback.finding_uuid).to eq(finding.uuid) expect(feedback.finding_uuid).to eq(finding.uuid)
end end
end end
context 'when the finding_uuid provided is nil' do
let(:finding) { create(:vulnerabilities_finding) }
let(:feedback_params_with_finding) { feedback_params.merge(finding_uuid: nil) }
subject(:feedback) { described_class.find_or_init_for(feedback_params_with_finding) }
it 'sets finding_uuid as nil' do
feedback.save!
expect(feedback.finding_uuid).to be_nil
end
end
context 'when attempting to save duplicate' do context 'when attempting to save duplicate' do
it 'raises ActiveRecord::RecordInvalid' do it 'raises ActiveRecord::RecordInvalid' do
duplicate = described_class.find_or_init_for(feedback_params) duplicate = described_class.find_or_init_for(feedback_params)
...@@ -315,13 +327,6 @@ RSpec.describe Vulnerabilities::Feedback do ...@@ -315,13 +327,6 @@ RSpec.describe Vulnerabilities::Feedback do
expect { described_class.find_or_init_for(feedback_params) }.to raise_error(ArgumentError, /category/) expect { described_class.find_or_init_for(feedback_params) }.to raise_error(ArgumentError, /category/)
end end
it 'raises ArgumentError when given a bad finding UUID' do
create(:vulnerabilities_finding, uuid: 'bar')
feedback_params[:finding_uuid] = 'foo'
expect { described_class.find_or_init_for(feedback_params) }.to raise_error(ArgumentError, /finding with the UUID/)
end
end end
end end
......
...@@ -167,21 +167,4 @@ RSpec.describe Vulnerabilities::FeedbackEntity do ...@@ -167,21 +167,4 @@ RSpec.describe Vulnerabilities::FeedbackEntity do
expect(subject[:comment_details]).to include(:comment_author) expect(subject[:comment_details]).to include(:comment_author)
end end
end end
context 'when finding_uuid is not present' do
let(:feedback) { build(:vulnerability_feedback, :issue, project: project) }
it 'has a nil finding_uuid' do
expect(subject[:finding_uuid]).to be_nil
end
end
context 'when finding_uuid is present' do
let_it_be(:finding) { create(:vulnerabilities_finding) }
let(:feedback) { create(:vulnerability_feedback, finding_uuid: finding.uuid, project: project) }
it 'exposes finding_uuid' do
expect(subject[:finding_uuid]).to eq(finding.uuid)
end
end
end end
...@@ -172,6 +172,7 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do ...@@ -172,6 +172,7 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
expect(feedback.issue).to be_an(Issue) expect(feedback.issue).to be_an(Issue)
expect(feedback.for_merge_request?).to eq(false) expect(feedback.for_merge_request?).to eq(false)
expect(feedback.merge_request).to be_nil expect(feedback.merge_request).to be_nil
expect(feedback.finding_uuid).to be_nil
end end
it 'updates the feedback when it already exists' do it 'updates the feedback when it already exists' do
...@@ -219,7 +220,6 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do ...@@ -219,7 +220,6 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
it 'creates the feedback' do it 'creates the feedback' do
expect(result[:status]).to eq(:success) expect(result[:status]).to eq(:success)
expect(result[:vulnerability_feedback]).to be_persisted expect(result[:vulnerability_feedback]).to be_persisted
expect(result[:vulnerability_feedback].finding_uuid).to be_nil
end end
end end
...@@ -276,10 +276,6 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do ...@@ -276,10 +276,6 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
expect(result[:vulnerability_feedback]).to be_persisted expect(result[:vulnerability_feedback]).to be_persisted
end end
it 'sets the finding_uuid' do
expect(result[:vulnerability_feedback].finding_uuid).to eq(vulnerability.finding.uuid)
end
context 'when issue link is already created' do context 'when issue link is already created' do
context 'when feedback does not exist' do context 'when feedback does not exist' do
let!(:issue_link) { create(:vulnerabilities_issue_link, :created, vulnerability: vulnerability) } let!(:issue_link) { create(:vulnerabilities_issue_link, :created, vulnerability: vulnerability) }
...@@ -394,6 +390,21 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do ...@@ -394,6 +390,21 @@ RSpec.describe VulnerabilityFeedback::CreateService, '#execute' do
expect(branches.length).to eq 1 expect(branches.length).to eq 1
end end
end end
context 'when finding_uuid is provided' do
let(:vulnerability) { create(:vulnerability, :with_findings, project: project) }
let(:result) do
described_class.new(
project,
user,
feedback_params.merge(finding_uuid: vulnerability.finding.uuid)
).execute
end
it 'sets the finding_uuid' do
expect(result[:vulnerability_feedback].finding_uuid).to eq(vulnerability.finding.uuid)
end
end
end end
context 'when feedback exists' do context 'when feedback exists' do
......
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