Move update_assignee_cache_counts to the service
This commit is contained in:
parent
92bf7dfcb0
commit
e2a3a5095a
12 changed files with 68 additions and 99 deletions
|
@ -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
|
||||
|
|
|
@ -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
|
||||
{
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -178,6 +178,7 @@ class IssuableBaseService < BaseService
|
|||
after_create(issuable)
|
||||
issuable.create_cross_references!(current_user)
|
||||
execute_hooks(issuable)
|
||||
issuable.assignees.each(&: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.each(&:invalidate_cache_counts)
|
||||
end
|
||||
|
||||
after_update(issuable)
|
||||
issuable.create_new_cross_references!(current_user)
|
||||
execute_hooks(issuable, 'update')
|
||||
|
|
|
@ -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
|
||||
|
|
|
@ -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
|
||||
|
||||
|
|
|
@ -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'))
|
||||
|
|
|
@ -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'))
|
||||
|
|
|
@ -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 }
|
||||
|
||||
|
|
|
@ -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])
|
||||
|
|
|
@ -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,
|
||||
|
|
|
@ -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' })
|
||||
|
|
Loading…
Reference in a new issue