Commit 71e976c7 authored by Mike Kozono's avatar Mike Kozono

Refactor to avoid respond_to? conditional

A hook is added to `container`s of Repository which is called if
`change_head` is attempted but the specified branch does not exist.
This is just to support the previously existing behavior for
`Project`, in which a validation error is added. Technically this
change removes the validation error from Snippet, but that was never a
defined or necessarily desired behavior before. If someone wishes to
add that in the future, it will be easy; they just need to use the
hook in Snippet.
parent 5aa6b6bd
...@@ -122,4 +122,8 @@ module HasRepository ...@@ -122,4 +122,8 @@ module HasRepository
def after_repository_change_head def after_repository_change_head
reload_default_branch reload_default_branch
end end
def after_change_head_branch_does_not_exist(branch)
# No-op (by default)
end
end end
...@@ -2727,6 +2727,11 @@ class Project < ApplicationRecord ...@@ -2727,6 +2727,11 @@ class Project < ApplicationRecord
self.topics.map(&:name) self.topics.map(&:name)
end end
override :after_change_head_branch_does_not_exist
def after_change_head_branch_does_not_exist(branch)
self.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
end
private private
def save_topics def save_topics
......
...@@ -1116,10 +1116,7 @@ class Repository ...@@ -1116,10 +1116,7 @@ class Repository
copy_gitattributes(branch) copy_gitattributes(branch)
after_change_head after_change_head
else else
# For example, `Wiki` does not have `errors` because it is not an `ActiveModel` container.after_change_head_branch_does_not_exist(branch)
if container.respond_to?(:errors)
container.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
end
false false
end end
......
...@@ -3266,6 +3266,16 @@ RSpec.describe Project, factory_default: :keep do ...@@ -3266,6 +3266,16 @@ RSpec.describe Project, factory_default: :keep do
end end
end end
describe '#after_change_head_branch_does_not_exist' do
let_it_be(:project) { create(:project) }
it 'adds an error to container if branch does not exist' do
expect do
project.after_change_head_branch_does_not_exist('unexisted-branch')
end.to change { project.errors.size }.from(0).to(1)
end
end
describe '#lfs_objects_for_repository_types' do describe '#lfs_objects_for_repository_types' do
let(:project) { create(:project) } let(:project) { create(:project) }
......
...@@ -3274,12 +3274,18 @@ RSpec.describe Repository do ...@@ -3274,12 +3274,18 @@ RSpec.describe Repository do
describe '#change_head' do describe '#change_head' do
let(:branch) { repository.container.default_branch } let(:branch) { repository.container.default_branch }
it 'adds an error to container if branch does not exist' do context 'when the branch exists' do
expect(repository.change_head('unexisted-branch')).to be false it 'returns truthy' do
expect(repository.container.errors.size).to eq(1) expect(repository.change_head(branch)).to be_truthy
end end
it 'calls the before_change_head and after_change_head methods' do it 'does not call container.after_change_head_branch_does_not_exist' do
expect(repository.container).not_to receive(:after_change_head_branch_does_not_exist)
repository.change_head(branch)
end
it 'calls repository hooks' do
expect(repository).to receive(:before_change_head) expect(repository).to receive(:before_change_head)
expect(repository).to receive(:after_change_head) expect(repository).to receive(:after_change_head)
...@@ -3296,4 +3302,26 @@ RSpec.describe Repository do ...@@ -3296,4 +3302,26 @@ RSpec.describe Repository do
repository.change_head(branch) repository.change_head(branch)
end end
end end
context 'when the branch does not exist' do
let(:branch) { 'non-existent-branch' }
it 'returns falsey' do
expect(repository.change_head(branch)).to be_falsey
end
it 'calls container.after_change_head_branch_does_not_exist' do
expect(repository.container).to receive(:after_change_head_branch_does_not_exist).with(branch)
repository.change_head(branch)
end
it 'does not call repository hooks' do
expect(repository).not_to receive(:before_change_head)
expect(repository).not_to receive(:after_change_head)
repository.change_head(branch)
end
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