Commit c90bbb43 authored by Kerri Miller's avatar Kerri Miller

Revert "Revert CODEOWNERS validation of Web requests in diff check"

This reverts commit d193b526.
parent 9089200d
---
title: Revert CODEOWNERS validation of Web requests in diff check
merge_request: 30936
author:
type: fixed
...@@ -12,7 +12,7 @@ module EE ...@@ -12,7 +12,7 @@ module EE
def path_validations def path_validations
validations = [super].flatten validations = [super].flatten
if !updated_from_web? && project.branch_requires_code_owner_approval?(branch_name) if project.branch_requires_code_owner_approval?(branch_name)
validations << validate_code_owners validations << validate_code_owners
end end
...@@ -34,11 +34,13 @@ module EE ...@@ -34,11 +34,13 @@ module EE
matched_rules = loader.entries.collect { |e| "- #{e.pattern}" } matched_rules = loader.entries.collect { |e| "- #{e.pattern}" }
code_owner_path = project.repository.code_owners_blob(ref: branch_name).path || "CODEOWNERS" code_owner_path = project.repository.code_owners_blob(ref: branch_name).path || "CODEOWNERS"
"Pushes to protected branches that contain changes to files that\n" \ msg = "Pushes to protected branches that contain changes to files that\n" \
"match patterns defined in `#{code_owner_path}` are disabled for\n" \ "match patterns defined in `#{code_owner_path}` are disabled for\n" \
"this project. Please submit these changes via a merge request.\n\n" \ "this project. Please submit these changes via a merge request.\n\n" \
"The following pattern(s) from `#{code_owner_path}` were matched:\n" \ "The following pattern(s) from `#{code_owner_path}` were matched:\n" \
"#{matched_rules.join('\n')}\n" "#{matched_rules.join('\n')}\n"
updated_from_web? ? msg.tr("\n", " ") : msg
end end
def validate_path_locks? def validate_path_locks?
......
...@@ -88,6 +88,32 @@ describe Gitlab::Checks::DiffCheck do ...@@ -88,6 +88,32 @@ describe Gitlab::Checks::DiffCheck do
end end
end end
context "and the user is not listed as a code owner" do
before do
stub_feature_flags(sectional_codeowners: false)
end
context "for a non-web-based request" do
it_behaves_like "returns an error message"
it "returns an error message with newline chars" do
expect(validation_result).to include("\n")
end
end
context "for a web-based request" do
before do
expect(subject).to receive(:updated_from_web?).and_return(true)
end
it_behaves_like "returns an error message"
it "returns an error message with newline chars removed" do
expect(validation_result).not_to include("\n")
end
end
end
context "and the user is not listed as a code owner" do context "and the user is not listed as a code owner" do
it_behaves_like "returns an error message" it_behaves_like "returns an error message"
end end
...@@ -152,9 +178,7 @@ describe Gitlab::Checks::DiffCheck do ...@@ -152,9 +178,7 @@ describe Gitlab::Checks::DiffCheck do
end end
context "when the feature is enabled on the project" do context "when the feature is enabled on the project" do
context "updated_from_web? == false" do
before do before do
expect(subject).to receive(:updated_from_web?).and_return(false)
expect(project).to receive(:branch_requires_code_owner_approval?) expect(project).to receive(:branch_requires_code_owner_approval?)
.once.and_return(true) .once.and_return(true)
end end
...@@ -166,18 +190,6 @@ describe Gitlab::Checks::DiffCheck do ...@@ -166,18 +190,6 @@ describe Gitlab::Checks::DiffCheck do
expect(validations.any? { |v| !v.is_a? Proc }).to be_falsy expect(validations.any? { |v| !v.is_a? Proc }).to be_falsy
end end
end end
context "updated_from_web? == true" do
before do
expect(subject).to receive(:updated_from_web?).and_return(true)
expect(project).not_to receive(:branch_requires_code_owner_approval?)
end
it "returns an empty array" do
expect(subject.send(:path_validations)).to eq([])
end
end
end
end end
context 'file name rules' do context 'file name rules' 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