Move update_project_counter_caches? out of issue and merge request

7 files changed, 14 insertions, 39 deletions

diff --git a/app/models/issue.rb b/app/models/issue.rb
index 3b3c7fb7f8b..b5abc8f57b0 100644
--- a/app/models/issue.rb
+++ b/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
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index dd4e67bc9da..f1a5cc73e83 100644
--- a/app/models/merge_request.rb
+++ b/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
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index 68b49d880f7..90865867ff0 100644
--- a/app/services/issuable_base_service.rb
+++ b/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
diff --git a/app/services/issues/base_service.rb b/app/services/issues/base_service.rb
index 735257c4779..b680eaf5a49 100644
--- a/app/services/issues/base_service.rb
+++ b/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
diff --git a/changelogs/unreleased/39602-move-update-project-counter-caches-out-of-issues-merge-requests.yml b/changelogs/unreleased/39602-move-update-project-counter-caches-out-of-issues-merge-requests.yml
new file mode 100644
index 00000000000..056afe43010
--- /dev/null
+++ b/changelogs/unreleased/39602-move-update-project-counter-caches-out-of-issues-merge-requests.yml
@@ -0,0 +1,5 @@
+---
+title: Move update_project_counter_caches? out of issue and merge request
+merge_request: 15300
+author: George Andrinopoulos
+type: other
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index bb5033c1628..5f901262598 100644
--- a/spec/models/issue_spec.rb
+++ b/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
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index d022dae3476..d250ad50713 100644
--- a/spec/models/merge_request_spec.rb
+++ b/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