Merge branch '39602-move-update-project-counter-caches-out-of-issues-merge-requests' into 'master'

Move update_project_counter_caches? out of issue and merge request

Closes #39602

See merge request gitlab-org/gitlab-ce!15300

app/models/issue.rb

@@ -262,10 +262,6 @@ class Issue < ActiveRecord::Base
     true
   end
 
-  def update_project_counter_caches?
-    state_changed? || confidential_changed?
-  end
-
   def update_project_counter_caches
     Projects::OpenIssuesCountService.new(project).refresh_cache
   end

app/models/merge_request.rb

@@ -958,10 +958,6 @@ class MergeRequest < ActiveRecord::Base
     true
   end
 
-  def update_project_counter_caches?
-    state_changed?
-  end
-
   def update_project_counter_caches
     Projects::OpenMergeRequestsCountService.new(target_project).refresh_cache
   end

app/services/issuable_base_service.rb

@@ -187,7 +187,7 @@ class IssuableBaseService < BaseService
 
       # We have to perform this check before saving the issuable as Rails resets
       # the changed fields upon calling #save.
-      update_project_counters = issuable.project && issuable.update_project_counter_caches?
+      update_project_counters = issuable.project && update_project_counter_caches?(issuable)
 
       if issuable.with_transaction_returning_status { issuable.save }
         # We do not touch as it will affect a update on updated_at field
@@ -288,4 +288,8 @@ class IssuableBaseService < BaseService
   # override if needed
   def execute_hooks(issuable, action = 'open', params = {})
   end
+
+  def update_project_counter_caches?(issuable)
+    issuable.state_changed?
+  end
 end

app/services/issues/base_service.rb

@@ -45,5 +45,9 @@ module Issues
         params.delete(:assignee_ids)
       end
     end
+
+    def update_project_counter_caches?(issue)
+      super || issue.confidential_changed?
+    end
   end
 end

changelogs/unreleased/39602-move-update-project-counter-caches-out-of-issues-merge-requests.yml

+---
+title: Move update_project_counter_caches? out of issue and merge request
+merge_request: 15300
+author: George Andrinopoulos
+type: other

spec/models/issue_spec.rb

@@ -765,22 +765,4 @@ describe Issue do
       expect(described_class.public_only).to eq([public_issue])
     end
   end
-
-  describe '#update_project_counter_caches?' do
-    it 'returns true when the state changes' do
-      subject.state = 'closed'
-
-      expect(subject.update_project_counter_caches?).to eq(true)
-    end
-
-    it 'returns true when the confidential flag changes' do
-      subject.confidential = true
-
-      expect(subject.update_project_counter_caches?).to eq(true)
-    end
-
-    it 'returns false when the state or confidential flag did not change' do
-      expect(subject.update_project_counter_caches?).to eq(false)
-    end
-  end
 end

spec/models/merge_request_spec.rb

@@ -1772,16 +1772,4 @@ describe MergeRequest do
         .to change { project.open_merge_requests_count }.from(1).to(0)
     end
   end
-
-  describe '#update_project_counter_caches?' do
-    it 'returns true when the state changes' do
-      subject.state = 'closed'
-
-      expect(subject.update_project_counter_caches?).to eq(true)
-    end
-
-    it 'returns false when the state did not change' do
-      expect(subject.update_project_counter_caches?).to eq(false)
-    end
-  end
 end