Commit 59117bea authored by Mehmet Emin INAC's avatar Mehmet Emin INAC

Rename approval rule `security` enum as `vulnerability`

This change will make it easier to understand what the enum stands for.
parent dabad744
...@@ -55,12 +55,12 @@ class ApprovalMergeRequestRule < ApplicationRecord ...@@ -55,12 +55,12 @@ class ApprovalMergeRequestRule < ApplicationRecord
alias_method :code_owner, :code_owner? alias_method :code_owner, :code_owner?
enum report_type: { enum report_type: {
security: 1, vulnerability: 1,
license_scanning: 2 license_scanning: 2
} }
scope :security_report, -> { report_approver.where(report_type: :security) } scope :vulnerability_report, -> { report_approver.vulnerability }
scope :license_compliance, -> { report_approver.where(report_type: :license_scanning) } scope :license_compliance, -> { report_approver.license_scanning }
scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) } scope :with_head_pipeline, -> { includes(merge_request: [:head_pipeline]) }
scope :open_merge_requests, -> { merge(MergeRequest.opened) } scope :open_merge_requests, -> { merge(MergeRequest.opened) }
scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline } scope :for_checks_that_can_be_refreshed, -> { license_compliance.open_merge_requests.with_head_pipeline }
......
...@@ -5,10 +5,10 @@ module ApprovalRuleLike ...@@ -5,10 +5,10 @@ module ApprovalRuleLike
DEFAULT_NAME = 'Default' DEFAULT_NAME = 'Default'
DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check' DEFAULT_NAME_FOR_LICENSE_REPORT = 'License-Check'
DEFAULT_NAME_FOR_SECURITY_REPORT = 'Vulnerability-Check' DEFAULT_NAME_FOR_VULNERABILITY_REPORT = 'Vulnerability-Check'
REPORT_TYPES_BY_DEFAULT_NAME = { REPORT_TYPES_BY_DEFAULT_NAME = {
DEFAULT_NAME_FOR_LICENSE_REPORT => :license_scanning, DEFAULT_NAME_FOR_LICENSE_REPORT => :license_scanning,
DEFAULT_NAME_FOR_SECURITY_REPORT => :security DEFAULT_NAME_FOR_VULNERABILITY_REPORT => :vulnerability
}.freeze }.freeze
APPROVALS_REQUIRED_MAX = 100 APPROVALS_REQUIRED_MAX = 100
ALL_MEMBERS = 'All Members' ALL_MEMBERS = 'All Members'
......
...@@ -42,7 +42,7 @@ module Security ...@@ -42,7 +42,7 @@ module Security
# If we don't have reports, then we should wait until pipeline stops. # If we don't have reports, then we should wait until pipeline stops.
return if reports.empty? && !pipeline.complete? return if reports.empty? && !pipeline.complete?
remove_required_approvals_for(ApprovalMergeRequestRule.security_report, sync_required_merge_requests) remove_required_approvals_for(ApprovalMergeRequestRule.vulnerability_report, sync_required_merge_requests)
end end
def reports def reports
......
...@@ -21,7 +21,7 @@ FactoryBot.define do ...@@ -21,7 +21,7 @@ FactoryBot.define do
factory :report_approver_rule, parent: :approval_merge_request_rule do factory :report_approver_rule, parent: :approval_merge_request_rule do
merge_request merge_request
rule_type { :report_approver } rule_type { :report_approver }
report_type { :security } report_type { :vulnerability }
sequence(:name) { |n| "*-#{n}.js" } sequence(:name) { |n| "*-#{n}.js" }
trait :requires_approval do trait :requires_approval do
...@@ -48,13 +48,13 @@ FactoryBot.define do ...@@ -48,13 +48,13 @@ FactoryBot.define do
approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) } approvals_required { rand(1..ApprovalProjectRule::APPROVALS_REQUIRED_MAX) }
end end
trait :security_report do trait :vulnerability_report do
rule_type { :report_approver } rule_type { :report_approver }
name { ApprovalRuleLike::DEFAULT_NAME_FOR_SECURITY_REPORT } name { ApprovalRuleLike::DEFAULT_NAME_FOR_VULNERABILITY_REPORT }
end end
trait :security do trait :vulnerability do
security_report vulnerability_report
end end
trait :license_scanning do trait :license_scanning do
......
...@@ -136,9 +136,9 @@ RSpec.describe ApprovalMergeRequestRule do ...@@ -136,9 +136,9 @@ RSpec.describe ApprovalMergeRequestRule do
end end
end end
describe '.security_report' do describe '.vulnerability_report' do
it 'returns the correct rules' do it 'returns the correct rules' do
expect(described_class.security_report) expect(described_class.vulnerability_report)
.to contain_exactly(report_approver_rule) .to contain_exactly(report_approver_rule)
end end
end end
......
...@@ -21,7 +21,7 @@ RSpec.describe ApprovalProjectRule do ...@@ -21,7 +21,7 @@ RSpec.describe ApprovalProjectRule do
describe '.regular' do describe '.regular' do
it 'returns non-report_approver records' do it 'returns non-report_approver records' do
rules = create_list(:approval_project_rule, 2) rules = create_list(:approval_project_rule, 2)
create(:approval_project_rule, :security_report) create(:approval_project_rule, :vulnerability_report)
expect(described_class.regular).to contain_exactly(*rules) expect(described_class.regular).to contain_exactly(*rules)
end end
...@@ -31,7 +31,7 @@ RSpec.describe ApprovalProjectRule do ...@@ -31,7 +31,7 @@ RSpec.describe ApprovalProjectRule do
it 'returns regular or any-approver rules' do it 'returns regular or any-approver rules' do
any_approver_rule = create(:approval_project_rule, rule_type: :any_approver) any_approver_rule = create(:approval_project_rule, rule_type: :any_approver)
regular_rule = create(:approval_project_rule) regular_rule = create(:approval_project_rule)
create(:approval_project_rule, :security_report) create(:approval_project_rule, :vulnerability_report)
expect(described_class.regular_or_any_approver).to( expect(described_class.regular_or_any_approver).to(
contain_exactly(any_approver_rule, regular_rule) contain_exactly(any_approver_rule, regular_rule)
...@@ -48,14 +48,14 @@ RSpec.describe ApprovalProjectRule do ...@@ -48,14 +48,14 @@ RSpec.describe ApprovalProjectRule do
end end
describe '#regular?' do describe '#regular?' do
let(:security_approver_rule) { build(:approval_project_rule, :security_report) } let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
it 'returns true for regular rules' do it 'returns true for regular rules' do
expect(subject.regular?).to eq(true) expect(subject.regular?).to eq(true)
end end
it 'returns false for report_approver rules' do it 'returns false for report_approver rules' do
expect(security_approver_rule.regular?). to eq(false) expect(vulnerability_approver_rule.regular?). to eq(false)
end end
end end
...@@ -66,14 +66,14 @@ RSpec.describe ApprovalProjectRule do ...@@ -66,14 +66,14 @@ RSpec.describe ApprovalProjectRule do
end end
describe '#report_approver?' do describe '#report_approver?' do
let(:security_approver_rule) { build(:approval_project_rule, :security_report) } let(:vulnerability_approver_rule) { build(:approval_project_rule, :vulnerability_report) }
it 'returns false for regular rules' do it 'returns false for regular rules' do
expect(subject.report_approver?).to eq(false) expect(subject.report_approver?).to eq(false)
end end
it 'returns true for report_approver rules' do it 'returns true for report_approver rules' do
expect(security_approver_rule.report_approver?). to eq(true) expect(vulnerability_approver_rule.report_approver?). to eq(true)
end end
end end
...@@ -82,8 +82,8 @@ RSpec.describe ApprovalProjectRule do ...@@ -82,8 +82,8 @@ RSpec.describe ApprovalProjectRule do
expect(build(:approval_project_rule).rule_type).to eq('regular') expect(build(:approval_project_rule).rule_type).to eq('regular')
end end
it 'returns the report_approver type for security report approvers rules' do it 'returns the report_approver type for vulnerability report approvers rules' do
expect(build(:approval_project_rule, :security_report).rule_type).to eq('report_approver') expect(build(:approval_project_rule, :vulnerability_report).rule_type).to eq('report_approver')
end end
end end
...@@ -114,7 +114,7 @@ RSpec.describe ApprovalProjectRule do ...@@ -114,7 +114,7 @@ RSpec.describe ApprovalProjectRule do
describe "validation" do describe "validation" do
let(:project_approval_rule) { create(:approval_project_rule) } let(:project_approval_rule) { create(:approval_project_rule) }
let(:license_compliance_rule) { create(:approval_project_rule, :license_scanning) } let(:license_compliance_rule) { create(:approval_project_rule, :license_scanning) }
let(:vulnerability_check_rule) { create(:approval_project_rule, :security) } let(:vulnerability_check_rule) { create(:approval_project_rule, :vulnerability) }
context "when creating a new rule" do context "when creating a new rule" do
specify { expect(project_approval_rule).to be_valid } specify { expect(project_approval_rule).to be_valid }
...@@ -166,7 +166,7 @@ RSpec.describe ApprovalProjectRule do ...@@ -166,7 +166,7 @@ RSpec.describe ApprovalProjectRule do
let_it_be(:new_user) { create(:user, name: 'Spiderman') } let_it_be(:new_user) { create(:user, name: 'Spiderman') }
let_it_be(:new_group) { create(:group, name: 'Avengers') } let_it_be(:new_group) { create(:group, name: 'Avengers') }
let_it_be(:rule, reload: true) { create(:approval_project_rule, name: 'Security', users: [user], groups: [group]) } let_it_be(:rule, reload: true) { create(:approval_project_rule, name: 'Vulnerability', users: [user], groups: [group]) }
shared_examples 'auditable' do shared_examples 'auditable' do
context 'when audit event queue is active' do context 'when audit event queue is active' do
...@@ -196,28 +196,28 @@ RSpec.describe ApprovalProjectRule do ...@@ -196,28 +196,28 @@ RSpec.describe ApprovalProjectRule do
describe '#audit_add users after :add' do describe '#audit_add users after :add' do
let(:action!) { rule.update(users: [user, new_user]) } let(:action!) { rule.update(users: [user, new_user]) }
let(:message) { 'Added User Spiderman to approval group on Security rule' } let(:message) { 'Added User Spiderman to approval group on Vulnerability rule' }
it_behaves_like 'auditable' it_behaves_like 'auditable'
end end
describe '#audit_remove users after :remove' do describe '#audit_remove users after :remove' do
let(:action!) { rule.update(users: []) } let(:action!) { rule.update(users: []) }
let(:message) { 'Removed User Batman from approval group on Security rule' } let(:message) { 'Removed User Batman from approval group on Vulnerability rule' }
it_behaves_like 'auditable' it_behaves_like 'auditable'
end end
describe '#audit_add groups after :add' do describe '#audit_add groups after :add' do
let(:action!) { rule.update(groups: [group, new_group]) } let(:action!) { rule.update(groups: [group, new_group]) }
let(:message) { 'Added Group Avengers to approval group on Security rule' } let(:message) { 'Added Group Avengers to approval group on Vulnerability rule' }
it_behaves_like 'auditable' it_behaves_like 'auditable'
end end
describe '#audit_remove groups after :remove' do describe '#audit_remove groups after :remove' do
let(:action!) { rule.update(groups: []) } let(:action!) { rule.update(groups: []) }
let(:message) { 'Removed Group Justice League from approval group on Security rule' } let(:message) { 'Removed Group Justice League from approval group on Vulnerability rule' }
it_behaves_like 'auditable' it_behaves_like 'auditable'
end end
......
...@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalRules do ...@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalRules do
context 'when the request is correct' do context 'when the request is correct' do
let!(:rule) do let!(:rule) do
rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7) rule = create(:approval_project_rule, name: 'vulnerability', project: project, approvals_required: 7)
rule.users << approver rule.users << approver
rule rule
end end
...@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalRules do ...@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalRules do
rule = json.first rule = json.first
expect(rule['approvals_required']).to eq(7) expect(rule['approvals_required']).to eq(7)
expect(rule['name']).to eq('security') expect(rule['name']).to eq('vulnerability')
end end
context 'private group filtering' do context 'private group filtering' do
...@@ -73,7 +73,7 @@ RSpec.describe API::ProjectApprovalRules do ...@@ -73,7 +73,7 @@ RSpec.describe API::ProjectApprovalRules do
context 'report_approver rules' do context 'report_approver rules' do
let!(:report_approver_rule) do let!(:report_approver_rule) do
create(:approval_project_rule, :security_report, project: project) create(:approval_project_rule, :vulnerability_report, project: project)
end end
it 'includes report_approver rules' do it 'includes report_approver rules' do
......
...@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalSettings do ...@@ -16,7 +16,7 @@ RSpec.describe API::ProjectApprovalSettings do
context 'when the request is correct' do context 'when the request is correct' do
let!(:rule) do let!(:rule) do
rule = create(:approval_project_rule, name: 'security', project: project, approvals_required: 7) rule = create(:approval_project_rule, name: 'vulnerability', project: project, approvals_required: 7)
rule.users << approver rule.users << approver
rule rule
end end
...@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalSettings do ...@@ -40,7 +40,7 @@ RSpec.describe API::ProjectApprovalSettings do
rule = json['rules'].first rule = json['rules'].first
expect(rule['approvals_required']).to eq(7) expect(rule['approvals_required']).to eq(7)
expect(rule['name']).to eq('security') expect(rule['name']).to eq('vulnerability')
end end
context 'when target_branch is specified' do context 'when target_branch is specified' do
...@@ -103,7 +103,7 @@ RSpec.describe API::ProjectApprovalSettings do ...@@ -103,7 +103,7 @@ RSpec.describe API::ProjectApprovalSettings do
context 'report_approver rules' do context 'report_approver rules' do
let!(:report_approver_rule) do let!(:report_approver_rule) do
create(:approval_project_rule, :security_report, project: project) create(:approval_project_rule, :vulnerability_report, project: project)
end end
it 'includes report_approver rules' do it 'includes report_approver rules' do
......
...@@ -12,35 +12,35 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -12,35 +12,35 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
stub_licensed_features(report_approver_rules: true) stub_licensed_features(report_approver_rules: true)
end end
context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_SECURITY_REPORT}` approval rule" do context "when a project has a single `#{ApprovalProjectRule::DEFAULT_NAME_FOR_VULNERABILITY_REPORT}` approval rule" do
let!(:security_approval_project_rule) { create(:approval_project_rule, :security_report, project: merge_request.target_project, approvals_required: 2) } let!(:vulnerability_approval_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project, approvals_required: 2) }
context 'when report_approver_rules are enabled' do context 'when report_approver_rules are enabled' do
let!(:regular_approval_project_rule) { create(:approval_project_rule, project: merge_request.target_project) } let!(:regular_approval_project_rule) { create(:approval_project_rule, project: merge_request.target_project) }
it 'creates rule for report approvers' do it 'creates rule for report approvers' do
expect { service.execute } expect { service.execute }
.to change { merge_request.approval_rules.security_report.count }.from(0).to(1) .to change { merge_request.approval_rules.vulnerability_report.count }.from(0).to(1)
rule = merge_request.approval_rules.security_report.first rule = merge_request.approval_rules.vulnerability_report.first
expect(rule).to be_report_approver expect(rule).to be_report_approver
expect(rule.report_type).to eq 'security' expect(rule.report_type).to eq 'vulnerability'
expect(rule.name).to eq(security_approval_project_rule.name) expect(rule.name).to eq(vulnerability_approval_project_rule.name)
expect(rule.approval_project_rule).to eq(security_approval_project_rule) expect(rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
end end
it 'updates previous rules if defined' do it 'updates previous rules if defined' do
mr_rule = create(:report_approver_rule, merge_request: merge_request, approvals_required: 0) mr_rule = create(:report_approver_rule, merge_request: merge_request, approvals_required: 0)
expect { service.execute } expect { service.execute }
.not_to change { merge_request.approval_rules.security_report.count } .not_to change { merge_request.approval_rules.vulnerability_report.count }
expect(mr_rule.reload).to be_report_approver expect(mr_rule.reload).to be_report_approver
expect(mr_rule.report_type).to eq 'security' expect(mr_rule.report_type).to eq 'vulnerability'
expect(mr_rule.name).to eq(security_approval_project_rule.name) expect(mr_rule.name).to eq(vulnerability_approval_project_rule.name)
expect(mr_rule.approvals_required).to eq security_approval_project_rule.approvals_required expect(mr_rule.approvals_required).to eq vulnerability_approval_project_rule.approvals_required
expect(mr_rule.approval_project_rule).to eq(security_approval_project_rule) expect(mr_rule.approval_project_rule).to eq(vulnerability_approval_project_rule)
end end
end end
end end
...@@ -76,11 +76,11 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -76,11 +76,11 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
end end
context "when a project has multiple report approval rules" do context "when a project has multiple report approval rules" do
let!(:vulnerability_project_rule) { create(:approval_project_rule, :security_report, project: merge_request.target_project) } let!(:vulnerability_project_rule) { create(:approval_project_rule, :vulnerability_report, project: merge_request.target_project) }
let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) } let!(:license_compliance_project_rule) { create(:approval_project_rule, :license_scanning, project: merge_request.target_project) }
context "when none of the rules have been synchronized to the merge request yet" do context "when none of the rules have been synchronized to the merge request yet" do
let(:vulnerability_check_rule) { merge_request.reload.approval_rules.security.last } let(:vulnerability_check_rule) { merge_request.reload.approval_rules.vulnerability_report.last }
let(:license_check_rule) { merge_request.reload.approval_rules.find_by(name: ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT) } let(:license_check_rule) { merge_request.reload.approval_rules.find_by(name: ApprovalProjectRule::DEFAULT_NAME_FOR_LICENSE_REPORT) }
before do before do
...@@ -95,7 +95,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -95,7 +95,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
specify { expect(merge_request.reload.approval_rules.count).to be(2) } specify { expect(merge_request.reload.approval_rules.count).to be(2) }
specify { expect(vulnerability_check_rule).to be_report_approver } specify { expect(vulnerability_check_rule).to be_report_approver }
specify { expect(vulnerability_check_rule.approvals_required).to eql(vulnerability_project_rule.approvals_required) } specify { expect(vulnerability_check_rule.approvals_required).to eql(vulnerability_project_rule.approvals_required) }
specify { expect(vulnerability_check_rule).to be_security } specify { expect(vulnerability_check_rule).to be_vulnerability }
specify { expect(vulnerability_check_rule.name).to eq(vulnerability_project_rule.name) } specify { expect(vulnerability_check_rule.name).to eq(vulnerability_project_rule.name) }
specify { expect(vulnerability_check_rule.approval_project_rule).to eq(vulnerability_project_rule) } specify { expect(vulnerability_check_rule.approval_project_rule).to eq(vulnerability_project_rule) }
specify { expect(license_check_rule).to be_report_approver } specify { expect(license_check_rule).to be_report_approver }
...@@ -113,7 +113,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do ...@@ -113,7 +113,7 @@ RSpec.describe MergeRequests::SyncReportApproverApprovalRules do
end end
specify { expect(merge_request.reload.approval_rules.count).to be(2) } specify { expect(merge_request.reload.approval_rules.count).to be(2) }
specify { expect(merge_request.reload.approval_rules.security_report.count).to be(1) } specify { expect(merge_request.reload.approval_rules.vulnerability_report.count).to be(1) }
specify { expect(merge_request.reload.approval_rules.where(report_type: :license_scanning)).to match_array([previous_rule]) } specify { expect(merge_request.reload.approval_rules.where(report_type: :license_scanning)).to match_array([previous_rule]) }
end end
end 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