Move update_assignee_cache_counts to the serviceupdate_assignee_cache_counts_refactoring

12 files changed, 68 insertions, 99 deletions

diff --git a/app/models/issue_assignee.rb b/app/models/issue_assignee.rb
index 0663d3aaef8..06d760b6a89 100644
--- a/app/models/issue_assignee.rb
+++ b/app/models/issue_assignee.rb
@@ -3,11 +3,4 @@ class IssueAssignee < ActiveRecord::Base
 
   belongs_to :issue
   belongs_to :assignee, class_name: "User", foreign_key: :user_id
-
-  after_create :update_assignee_cache_counts
-  after_destroy :update_assignee_cache_counts
-
-  def update_assignee_cache_counts
-    assignee&.update_cache_counts
-  end
 end
diff --git a/app/models/merge_request.rb b/app/models/merge_request.rb
index 59736f70f24..9e5db53b15e 100644
--- a/app/models/merge_request.rb
+++ b/app/models/merge_request.rb
@@ -125,7 +125,6 @@ class MergeRequest < ActiveRecord::Base
   participant :assignee
 
   after_save :keep_around_commit
-  after_save :update_assignee_cache_counts, if: :assignee_id_changed?
 
   def self.reference_prefix
     '!'
@@ -187,13 +186,6 @@ class MergeRequest < ActiveRecord::Base
     work_in_progress?(title) ? title : "WIP: #{title}"
   end
 
-  def update_assignee_cache_counts
-    # make sure we flush the cache for both the old *and* new assignees(if they exist)
-    previous_assignee = User.find_by_id(assignee_id_was) if assignee_id_was
-    previous_assignee&.update_cache_counts
-    assignee&.update_cache_counts
-  end
-
   # Returns a Hash of attributes to be used for Twitter card metadata
   def card_attributes
     {
diff --git a/app/models/user.rb b/app/models/user.rb
index f713a20233c..c7160a6af14 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -929,6 +929,11 @@ class User < ActiveRecord::Base
     assigned_open_issues_count(force: true)
   end
 
+  def invalidate_cache_counts
+    Rails.cache.delete(['users', id, 'assigned_open_merge_requests_count'])
+    Rails.cache.delete(['users', id, 'assigned_open_issues_count'])
+  end
+
   def todos_done_count(force: false)
     Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do
       TodosFinder.new(self, state: :done).execute.count
diff --git a/app/services/issuable_base_service.rb b/app/services/issuable_base_service.rb
index c1e532b504a..370bb43932e 100644
--- a/app/services/issuable_base_service.rb
+++ b/app/services/issuable_base_service.rb
@@ -178,6 +178,7 @@ class IssuableBaseService < BaseService
       after_create(issuable)
       issuable.create_cross_references!(current_user)
       execute_hooks(issuable)
+      issuable.assignees.map(&:invalidate_cache_counts)
     end
 
     issuable
@@ -234,6 +235,11 @@ class IssuableBaseService < BaseService
           old_assignees: old_assignees
         )
 
+        if old_assignees != issuable.assignees
+          assignees = old_assignees + issuable.assignees.to_a
+          assignees.compact.map(&:invalidate_cache_counts)
+        end
+
         after_update(issuable)
         issuable.create_new_cross_references!(current_user)
         execute_hooks(issuable, 'update')
diff --git a/app/services/members/authorized_destroy_service.rb b/app/services/members/authorized_destroy_service.rb
index a85b9465c84..7912cac65d3 100644
--- a/app/services/members/authorized_destroy_service.rb
+++ b/app/services/members/authorized_destroy_service.rb
@@ -43,8 +43,9 @@ module Members
         )
 
         project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil)
-        member.user.update_cache_counts
       end
+
+      member.user.invalidate_cache_counts
     end
   end
 end
diff --git a/spec/features/dashboard/issuables_counter_spec.rb b/spec/features/dashboard/issuables_counter_spec.rb
index 6f7bf0eba6e..354267dbee7 100644
--- a/spec/features/dashboard/issuables_counter_spec.rb
+++ b/spec/features/dashboard/issuables_counter_spec.rb
@@ -19,7 +19,7 @@ describe 'Navigation bar counter', feature: true, caching: true do
 
     issue.assignees = []
 
-    user.update_cache_counts
+    user.invalidate_cache_counts
 
     Timecop.travel(3.minutes.from_now) do
       visit issues_path
@@ -35,6 +35,8 @@ describe 'Navigation bar counter', feature: true, caching: true do
 
     merge_request.update(assignee: nil)
 
+    user.invalidate_cache_counts
+
     Timecop.travel(3.minutes.from_now) do
       visit merge_requests_path
 
diff --git a/spec/models/issue_spec.rb b/spec/models/issue_spec.rb
index 725f5c2311f..bb4e70db2e9 100644
--- a/spec/models/issue_spec.rb
+++ b/spec/models/issue_spec.rb
@@ -38,46 +38,6 @@ describe Issue, models: true do
     end
   end
 
-  describe "before_save" do
-    describe "#update_cache_counts when an issue is reassigned" do
-      let(:issue) { create(:issue) }
-      let(:assignee) { create(:user) }
-
-      context "when previous assignee exists" do
-        before do
-          issue.project.team << [assignee, :developer]
-          issue.assignees << assignee
-        end
-
-        it "updates cache counts for new assignee" do
-          user = create(:user)
-
-          expect(user).to receive(:update_cache_counts)
-
-          issue.assignees << user
-        end
-
-        it "updates cache counts for previous assignee" do
-          issue.assignees.first
-
-          expect_any_instance_of(User).to receive(:update_cache_counts)
-
-          issue.assignees.destroy_all
-        end
-      end
-
-      context "when previous assignee does not exist" do
-        it "updates cache count for the new assignee" do
-          issue.assignees = []
-
-          expect_any_instance_of(User).to receive(:update_cache_counts)
-
-          issue.assignees << assignee
-        end
-      end
-    end
-  end
-
   describe '#card_attributes' do
     it 'includes the author name' do
       allow(subject).to receive(:author).and_return(double(name: 'Robert'))
diff --git a/spec/models/merge_request_spec.rb b/spec/models/merge_request_spec.rb
index ef349530761..e8124c4cbe9 100644
--- a/spec/models/merge_request_spec.rb
+++ b/spec/models/merge_request_spec.rb
@@ -87,48 +87,6 @@ describe MergeRequest, models: true do
     end
   end
 
-  describe "before_save" do
-    describe "#update_cache_counts when a merge request is reassigned" do
-      let(:project) { create :project }
-      let(:merge_request) { create(:merge_request, source_project: project, target_project: project) }
-      let(:assignee) { create :user }
-
-      context "when previous assignee exists" do
-        before do
-          project.team << [assignee, :developer]
-          merge_request.update(assignee: assignee)
-        end
-
-        it "updates cache counts for new assignee" do
-          user = create(:user)
-
-          expect(user).to receive(:update_cache_counts)
-
-          merge_request.update(assignee: user)
-        end
-
-        it "updates cache counts for previous assignee" do
-          old_assignee = merge_request.assignee
-          allow(User).to receive(:find_by_id).with(old_assignee.id).and_return(old_assignee)
-
-          expect(old_assignee).to receive(:update_cache_counts)
-
-          merge_request.update(assignee: nil)
-        end
-      end
-
-      context "when previous assignee does not exist" do
-        it "updates cache count for the new assignee" do
-          merge_request.update(assignee: nil)
-
-          expect_any_instance_of(User).to receive(:update_cache_counts)
-
-          merge_request.update(assignee: assignee)
-        end
-      end
-    end
-  end
-
   describe '#card_attributes' do
     it 'includes the author name' do
       allow(subject).to receive(:author).and_return(double(name: 'Robert'))
diff --git a/spec/services/issues/create_service_spec.rb b/spec/services/issues/create_service_spec.rb
index 01edc46496d..dab1a3469f7 100644
--- a/spec/services/issues/create_service_spec.rb
+++ b/spec/services/issues/create_service_spec.rb
@@ -118,6 +118,22 @@ describe Issues::CreateService, services: true do
         end
       end
 
+      context 'when assignee is set' do
+        let(:opts) do
+          { title: 'Title',
+            description: 'Description',
+            assignees: [assignee] }
+        end
+
+        it 'invalidates open issues counter for assignees when issue is assigned' do
+          project.team << [assignee, :master]
+
+          described_class.new(project, user, opts).execute
+
+          expect(assignee.assigned_open_issues_count).to eq 1
+        end
+      end
+
       it 'executes issue hooks when issue is not confidential' do
         opts = { title: 'Title', description: 'Description', confidential: false }
 
diff --git a/spec/services/issues/update_service_spec.rb b/spec/services/issues/update_service_spec.rb
index 1954d8739f6..5184c1d5f19 100644
--- a/spec/services/issues/update_service_spec.rb
+++ b/spec/services/issues/update_service_spec.rb
@@ -59,6 +59,13 @@ describe Issues::UpdateService, services: true do
         expect(issue.due_date).to eq Date.tomorrow
       end
 
+      it 'updates open issue counter for assignees when issue is reassigned' do
+        update_issue(assignee_ids: [user2.id])
+
+        expect(user3.assigned_open_issues_count).to eq 0
+        expect(user2.assigned_open_issues_count).to eq 1
+      end
+
       it 'sorts issues as specified by parameters' do
         issue1 = create(:issue, project: project, assignees: [user3])
         issue2 = create(:issue, project: project, assignees: [user3])
diff --git a/spec/services/merge_requests/create_service_spec.rb b/spec/services/merge_requests/create_service_spec.rb
index ace82380cc9..41752f1a01a 100644
--- a/spec/services/merge_requests/create_service_spec.rb
+++ b/spec/services/merge_requests/create_service_spec.rb
@@ -144,6 +144,26 @@ describe MergeRequests::CreateService, services: true do
           expect(merge_request.assignee).to eq(assignee)
         end
 
+        context 'when assignee is set' do
+          let(:opts) do
+            {
+              title: 'Title',
+              description: 'Description',
+              assignee_id: assignee.id,
+              source_branch: 'feature',
+              target_branch: 'master'
+            }
+          end
+
+          it 'invalidates open merge request counter for assignees when merge request is assigned' do
+            project.team << [assignee, :master]
+
+            described_class.new(project, user, opts).execute
+
+            expect(assignee.assigned_open_merge_requests_count).to eq 1
+          end
+        end
+
         context "when issuable feature is private" do
           before do
             project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
diff --git a/spec/services/merge_requests/update_service_spec.rb b/spec/services/merge_requests/update_service_spec.rb
index 07f5440cc36..2c8fbb46e75 100644
--- a/spec/services/merge_requests/update_service_spec.rb
+++ b/spec/services/merge_requests/update_service_spec.rb
@@ -299,6 +299,15 @@ describe MergeRequests::UpdateService, services: true do
         end
       end
 
+      context 'when the assignee changes' do
+        it 'updates open merge request counter for assignees when merge request is reassigned' do
+          update_merge_request(assignee_id: user2.id)
+
+          expect(user3.assigned_open_merge_requests_count).to eq 0
+          expect(user2.assigned_open_merge_requests_count).to eq 1
+        end
+      end
+
       context 'when the target branch change' do
         before do
           update_merge_request({ target_branch: 'target' })