Merge branch 'mk/refactor-change-head-project-errors' into 'master'

Refactor to avoid respond_to conditional

See merge request gitlab-org/gitlab!70004

app/models/concerns/has_repository.rb

@@ -122,4 +122,8 @@ module HasRepository
   def after_repository_change_head
     reload_default_branch
   end
+
+  def after_change_head_branch_does_not_exist(branch)
+    # No-op (by default)
+  end
 end

app/models/project.rb

@@ -2739,6 +2739,11 @@ class Project < ApplicationRecord
     self.topics.map(&:name)
   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
 
   def save_topics

app/models/repository.rb

@@ -1123,10 +1123,7 @@ class Repository
       copy_gitattributes(branch)
       after_change_head
     else
-      # For example, `Wiki` does not have `errors` because it is not an `ActiveModel`
-      if container.respond_to?(:errors)
-        container.errors.add(:base, _("Could not change HEAD: branch '%{branch}' does not exist") % { branch: branch })
-      end
+      container.after_change_head_branch_does_not_exist(branch)
 
       false
     end

spec/models/project_spec.rb

@@ -3310,6 +3310,16 @@ RSpec.describe Project, factory_default: :keep do
     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
     let(:project) { create(:project) }
 

spec/models/repository_spec.rb

@@ -3283,26 +3283,54 @@ RSpec.describe Repository do
   describe '#change_head' do
     let(:branch) { repository.container.default_branch }
 
-    it 'adds an error to container if branch does not exist' do
-      expect(repository.change_head('unexisted-branch')).to be false
-      expect(repository.container.errors.size).to eq(1)
-    end
+    context 'when the branch exists' do
+      it 'returns truthy' do
+        expect(repository.change_head(branch)).to be_truthy
+      end
 
-    it 'calls the before_change_head and after_change_head methods' do
-      expect(repository).to receive(:before_change_head)
-      expect(repository).to receive(:after_change_head)
+      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
+        repository.change_head(branch)
+      end
+
+      it 'calls repository hooks' do
+        expect(repository).to receive(:before_change_head)
+        expect(repository).to receive(:after_change_head)
+
+        repository.change_head(branch)
+      end
+
+      it 'copies the gitattributes' do
+        expect(repository).to receive(:copy_gitattributes).with(branch)
+        repository.change_head(branch)
+      end
 
-    it 'copies the gitattributes' do
-      expect(repository).to receive(:copy_gitattributes).with(branch)
-      repository.change_head(branch)
+      it 'reloads the default branch' do
+        expect(repository.container).to receive(:reload_default_branch)
+        repository.change_head(branch)
+      end
     end
 
-    it 'reloads the default branch' do
-      expect(repository.container).to receive(:reload_default_branch)
-      repository.change_head(branch)
+    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