Commit 42699b0a authored by Sean McGivern's avatar Sean McGivern

Merge branch 'update_assignee_cache_counts_refactoring' into 'master'

Move update_assignee_cache_counts to the service(port from CE)

See merge request !1887
parents df2d3773 78ef2360
...@@ -4,18 +4,11 @@ class IssueAssignee < ActiveRecord::Base ...@@ -4,18 +4,11 @@ class IssueAssignee < ActiveRecord::Base
belongs_to :issue belongs_to :issue
belongs_to :assignee, class_name: "User", foreign_key: :user_id belongs_to :assignee, class_name: "User", foreign_key: :user_id
after_create :update_assignee_cache_counts
after_destroy :update_assignee_cache_counts
# EE-specific # EE-specific
after_create :update_elasticsearch_index after_create :update_elasticsearch_index
after_destroy :update_elasticsearch_index after_destroy :update_elasticsearch_index
# EE-specific # EE-specific
def update_assignee_cache_counts
assignee&.update_cache_counts
end
def update_elasticsearch_index def update_elasticsearch_index
if current_application_settings.elasticsearch_indexing? if current_application_settings.elasticsearch_indexing?
ElasticIndexerWorker.perform_async( ElasticIndexerWorker.perform_async(
......
...@@ -129,7 +129,6 @@ class MergeRequest < ActiveRecord::Base ...@@ -129,7 +129,6 @@ class MergeRequest < ActiveRecord::Base
participant :assignee participant :assignee
after_save :keep_around_commit after_save :keep_around_commit
after_save :update_assignee_cache_counts, if: :assignee_id_changed?
def self.reference_prefix def self.reference_prefix
'!' '!'
...@@ -191,13 +190,6 @@ class MergeRequest < ActiveRecord::Base ...@@ -191,13 +190,6 @@ class MergeRequest < ActiveRecord::Base
work_in_progress?(title) ? title : "WIP: #{title}" work_in_progress?(title) ? title : "WIP: #{title}"
end 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 # Returns a Hash of attributes to be used for Twitter card metadata
def card_attributes def card_attributes
{ {
......
...@@ -950,6 +950,11 @@ class User < ActiveRecord::Base ...@@ -950,6 +950,11 @@ class User < ActiveRecord::Base
assigned_open_issues_count(force: true) assigned_open_issues_count(force: true)
end 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) def todos_done_count(force: false)
Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do Rails.cache.fetch(['users', id, 'todos_done_count'], force: force) do
TodosFinder.new(self, state: :done).execute.count TodosFinder.new(self, state: :done).execute.count
......
...@@ -178,6 +178,7 @@ class IssuableBaseService < BaseService ...@@ -178,6 +178,7 @@ class IssuableBaseService < BaseService
after_create(issuable) after_create(issuable)
issuable.create_cross_references!(current_user) issuable.create_cross_references!(current_user)
execute_hooks(issuable) execute_hooks(issuable)
issuable.assignees.each(&:invalidate_cache_counts)
end end
issuable issuable
...@@ -234,6 +235,12 @@ class IssuableBaseService < BaseService ...@@ -234,6 +235,12 @@ class IssuableBaseService < BaseService
old_assignees: old_assignees old_assignees: old_assignees
) )
if old_assignees != issuable.assignees
new_assignees = issuable.assignees.to_a
affected_assignees = (old_assignees + new_assignees) - (old_assignees & new_assignees)
affected_assignees.compact.each(&:invalidate_cache_counts)
end
after_update(issuable) after_update(issuable)
issuable.create_new_cross_references!(current_user) issuable.create_new_cross_references!(current_user)
execute_hooks(issuable, 'update') execute_hooks(issuable, 'update')
......
...@@ -43,8 +43,9 @@ module Members ...@@ -43,8 +43,9 @@ module Members
) )
project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil) project.merge_requests.opened.assigned_to(member.user).update_all(assignee_id: nil)
member.user.update_cache_counts
end end
member.user.invalidate_cache_counts
end end
end end
end end
...@@ -19,7 +19,7 @@ describe 'Navigation bar counter', feature: true, caching: true do ...@@ -19,7 +19,7 @@ describe 'Navigation bar counter', feature: true, caching: true do
issue.assignees = [] issue.assignees = []
user.update_cache_counts user.invalidate_cache_counts
Timecop.travel(3.minutes.from_now) do Timecop.travel(3.minutes.from_now) do
visit issues_path visit issues_path
...@@ -35,6 +35,8 @@ describe 'Navigation bar counter', feature: true, caching: true do ...@@ -35,6 +35,8 @@ describe 'Navigation bar counter', feature: true, caching: true do
merge_request.update(assignee: nil) merge_request.update(assignee: nil)
user.invalidate_cache_counts
Timecop.travel(3.minutes.from_now) do Timecop.travel(3.minutes.from_now) do
visit merge_requests_path visit merge_requests_path
......
...@@ -38,46 +38,6 @@ describe Issue, models: true do ...@@ -38,46 +38,6 @@ describe Issue, models: true do
end end
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 describe '#card_attributes' do
it 'includes the author name' do it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert')) allow(subject).to receive(:author).and_return(double(name: 'Robert'))
......
...@@ -88,48 +88,6 @@ describe MergeRequest, models: true do ...@@ -88,48 +88,6 @@ describe MergeRequest, models: true do
end end
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 describe '#card_attributes' do
it 'includes the author name' do it 'includes the author name' do
allow(subject).to receive(:author).and_return(double(name: 'Robert')) allow(subject).to receive(:author).and_return(double(name: 'Robert'))
......
...@@ -118,6 +118,22 @@ describe Issues::CreateService, services: true do ...@@ -118,6 +118,22 @@ describe Issues::CreateService, services: true do
end end
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 it 'executes issue hooks when issue is not confidential' do
opts = { title: 'Title', description: 'Description', confidential: false } opts = { title: 'Title', description: 'Description', confidential: false }
......
...@@ -59,6 +59,13 @@ describe Issues::UpdateService, services: true do ...@@ -59,6 +59,13 @@ describe Issues::UpdateService, services: true do
expect(issue.due_date).to eq Date.tomorrow expect(issue.due_date).to eq Date.tomorrow
end 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 it 'sorts issues as specified by parameters' do
issue1 = create(:issue, project: project, assignees: [user3]) issue1 = create(:issue, project: project, assignees: [user3])
issue2 = create(:issue, project: project, assignees: [user3]) issue2 = create(:issue, project: project, assignees: [user3])
......
...@@ -144,6 +144,26 @@ describe MergeRequests::CreateService, services: true do ...@@ -144,6 +144,26 @@ describe MergeRequests::CreateService, services: true do
expect(merge_request.assignee).to eq(assignee) expect(merge_request.assignee).to eq(assignee)
end 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 context "when issuable feature is private" do
before do before do
project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE, project.project_feature.update(issues_access_level: ProjectFeature::PRIVATE,
......
...@@ -297,6 +297,15 @@ describe MergeRequests::UpdateService, services: true do ...@@ -297,6 +297,15 @@ describe MergeRequests::UpdateService, services: true do
end end
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 context 'when the target branch change' do
before do before do
update_merge_request({ target_branch: 'target' }) update_merge_request({ target_branch: 'target' })
......
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