Commit 2adf2d5c authored by Kerri Miller's avatar Kerri Miller Committed by Thong Kuah

Improve codeowner checks in BlobController

- fixes bug where checks unevenly applied to file create
- strip \n from error messages
- improve tests
parent 488e0370
......@@ -11,19 +11,21 @@ module EE
private
# rubocop:disable Gitlab/ModuleWithInstanceVariables
def validate_codeowner_rules
return unless ::Feature.enabled?(:use_legacy_codeowner_validations)
return if params[:file_path].blank?
return if @file_path.blank?
codeowners_error = codeowners_check_error(project, branch_name, params[:file_path])
codeowners_error = codeowners_check_error(project, branch_name, @file_path)
if codeowners_error.present?
flash.now[:alert] = codeowners_error
flash.now[:alert] = codeowners_error.tr("\n", " ")
view = params[:action] == 'update' ? :edit : :new
render view
end
end
# rubocop:enable Gitlab/ModuleWithInstanceVariables
def codeowners_check_error(project, branch_name, paths)
::Gitlab::CodeOwners::Validator.new(project, branch_name, paths).execute
......
---
title: Fix issue with displaying Code Owner error messages in the UI
merge_request: 34600
author:
type: fixed
......@@ -4,6 +4,7 @@ require 'spec_helper'
RSpec.describe Projects::BlobController do
include ProjectForksHelper
include FakeBlobHelpers
let(:project) { create(:project, :public, :repository) }
......@@ -29,15 +30,34 @@ RSpec.describe Projects::BlobController do
end
shared_examples "renders to the expected_view with an error msg" do
let(:error_msg) { "Example error msg" }
it "renders to the expected_view with an error msg" do
default_params[:file_path] = "CHANGELOG"
let(:error_msg) do
"Pushes to protected branches that contain changes to files that match " \
"patterns defined in `CODEOWNERS` are disabled for this project. " \
"Please submit these changes via a merge request. The following " \
"pattern(s) from `CODEOWNERS` were matched: - docs/ "
end
expect_next_instance_of(Gitlab::CodeOwners::Validator) do |validator|
expect(validator).to receive(:execute).and_return(error_msg)
before do
allow(::ProtectedBranch).to receive(:branch_requires_code_owner_approval?)
.and_return(true)
expect_next_instance_of(Repository) do |repo|
allow(repo).to receive(:code_owners_blob)
.with(ref: "master")
.and_return(
fake_blob(
path: "CODEOWNERS",
data: "*.rb @#{user.username}\ndocs/ @#{user.username}"
)
)
end
stub_licensed_features(code_owner_approval_required: true)
end
it "renders to the edit page with an error msg" do
default_params[:file_path] = "docs/EXAMPLE_FILE"
subject
expect(flash[:alert]).to eq(error_msg)
......@@ -53,7 +73,7 @@ RSpec.describe Projects::BlobController do
project_id: project,
id: 'master',
branch_name: 'master',
file_name: 'CHANGELOG',
file_name: 'docs/EXAMPLE_FILE',
content: 'Added changes',
commit_message: 'Create CHANGELOG'
}
......@@ -68,7 +88,7 @@ RSpec.describe Projects::BlobController do
it 'redirects to blob' do
post :create, params: default_params
expect(response).to be_ok
expect(response).to be_redirect
end
it_behaves_like "file matches a codeowners rule" do
......@@ -91,10 +111,6 @@ RSpec.describe Projects::BlobController do
}
end
def blob_after_edit_path
project_blob_path(project, 'master/CHANGELOG')
end
before do
project.add_maintainer(user)
......
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