Commit 6237d305 authored by Avielle Wolfe's avatar Avielle Wolfe

Prefer prefix :for for feedback type enum

It avoids confusion with ActiveRecord's use of `type` for associations.
parent 90970775
......@@ -14,15 +14,15 @@ module Vulnerabilities
attr_accessor :vulnerability_data
enum feedback_type: { dismissal: 0, issue: 1, merge_request: 2 }, _suffix: :type
enum feedback_type: { dismissal: 0, issue: 1, merge_request: 2 }, _prefix: :for
enum category: { sast: 0, dependency_scanning: 1, container_scanning: 2, dast: 3 }
validates :project, presence: true
validates :author, presence: true
validates :comment_timestamp, :comment_author, presence: true, if: :comment?
validates :issue, presence: true, if: :issue_type?
validates :merge_request, presence: true, if: :merge_request_type?
validates :vulnerability_data, presence: true, unless: :dismissal_type?
validates :issue, presence: true, if: :for_issue?
validates :merge_request, presence: true, if: :for_merge_request?
validates :vulnerability_data, presence: true, unless: :for_dismissal?
validates :feedback_type, presence: true
validates :category, presence: true
validates :project_fingerprint, presence: true, uniqueness: { scope: [:project_id, :category, :feedback_type] }
......
......@@ -4,9 +4,9 @@ module Vulnerabilities
class FeedbackPolicy < BasePolicy
delegate { @subject.project }
condition(:issue) { @subject.issue_type? }
condition(:merge_request) { @subject.merge_request_type? }
condition(:dismissal) { @subject.dismissal_type? }
condition(:issue) { @subject.for_issue? }
condition(:merge_request) { @subject.for_merge_request? }
condition(:dismissal) { @subject.for_dismissal? }
rule { issue & ~can?(:create_issue) }.prevent :create_vulnerability_feedback
......
......@@ -7,11 +7,11 @@ module VulnerabilityFeedback
raise Gitlab::Access::AccessDeniedError unless can?(current_user, :create_vulnerability_feedback, vulnerability_feedback)
if vulnerability_feedback.issue_type? &&
if vulnerability_feedback.for_issue? &&
!vulnerability_feedback.vulnerability_data.blank?
create_issue_for(vulnerability_feedback)
elsif vulnerability_feedback.merge_request_type? &&
elsif vulnerability_feedback.for_merge_request? &&
!vulnerability_feedback.vulnerability_data.blank?
create_merge_request_for(vulnerability_feedback)
......
......@@ -7,7 +7,7 @@ describe Vulnerabilities::Feedback do
is_expected.to(
define_enum_for(:feedback_type)
.with_values(dismissal: 0, issue: 1, merge_request: 2)
.with_suffix(:type)
.with_prefix(:for)
)
}
it { is_expected.to define_enum_for(:category) }
......
......@@ -50,10 +50,10 @@ describe VulnerabilityFeedback::CreateService, '#execute' do
expect(feedback.pipeline_id).to eq(pipeline.id)
expect(feedback.category).to eq('sast')
expect(feedback.project_fingerprint).to eq('418291a26024a1445b23fe64de9380cdcdfd1fa8')
expect(feedback.dismissal_type?).to eq(true)
expect(feedback.issue_type?).to eq(false)
expect(feedback.for_dismissal?).to eq(true)
expect(feedback.for_issue?).to eq(false)
expect(feedback.issue).to be_nil
expect(feedback.merge_request_type?).to eq(false)
expect(feedback.for_merge_request?).to eq(false)
expect(feedback.merge_request).to be_nil
end
......@@ -101,10 +101,10 @@ describe VulnerabilityFeedback::CreateService, '#execute' do
expect(feedback.pipeline_id).to eq(pipeline.id)
expect(feedback.category).to eq('sast')
expect(feedback.project_fingerprint).to eq('418291a26024a1445b23fe64de9380cdcdfd1fa8')
expect(feedback.dismissal_type?).to eq(false)
expect(feedback.issue_type?).to eq(true)
expect(feedback.for_dismissal?).to eq(false)
expect(feedback.for_issue?).to eq(true)
expect(feedback.issue).to be_an(Issue)
expect(feedback.merge_request_type?).to eq(false)
expect(feedback.for_merge_request?).to eq(false)
expect(feedback.merge_request).to be_nil
end
......@@ -176,10 +176,10 @@ describe VulnerabilityFeedback::CreateService, '#execute' do
expect(feedback.pipeline_id).to eq(pipeline.id)
expect(feedback.category).to eq('dependency_scanning')
expect(feedback.project_fingerprint).to eq('418291a26024a1445b23fe64de9380cdcdfd1fa8')
expect(feedback.dismissal_type?).to eq(false)
expect(feedback.issue_type?).to eq(false)
expect(feedback.for_dismissal?).to eq(false)
expect(feedback.for_issue?).to eq(false)
expect(feedback.issue).to be_nil
expect(feedback.merge_request_type?).to eq(true)
expect(feedback.for_merge_request?).to eq(true)
expect(feedback.merge_request).to be_an(MergeRequest)
end
......
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