Commit c3e01c72 authored by Stan Hu's avatar Stan Hu

Merge branch '296842-review-requests-counter-cache-invalidation' into 'master'

Invalidate reviews counter cache when MR gets closed/merged/reopened

See merge request gitlab-org/gitlab!51055
parents 33944910 6a05c667
...@@ -18,7 +18,7 @@ module MergeRequests ...@@ -18,7 +18,7 @@ module MergeRequests
notification_service.async.close_mr(merge_request, current_user) notification_service.async.close_mr(merge_request, current_user)
todo_service.close_merge_request(merge_request, current_user) todo_service.close_merge_request(merge_request, current_user)
execute_hooks(merge_request, 'close') execute_hooks(merge_request, 'close')
invalidate_cache_counts(merge_request, users: merge_request.assignees) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers)
merge_request.update_project_counter_caches merge_request.update_project_counter_caches
cleanup_environments(merge_request) cleanup_environments(merge_request)
abort_auto_merge(merge_request, 'merge request was closed') abort_auto_merge(merge_request, 'merge request was closed')
......
...@@ -18,7 +18,7 @@ module MergeRequests ...@@ -18,7 +18,7 @@ module MergeRequests
merge_request_activity_counter.track_merge_mr_action(user: current_user) merge_request_activity_counter.track_merge_mr_action(user: current_user)
notification_service.merge_mr(merge_request, current_user) notification_service.merge_mr(merge_request, current_user)
execute_hooks(merge_request, 'merge') execute_hooks(merge_request, 'merge')
invalidate_cache_counts(merge_request, users: merge_request.assignees) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers)
merge_request.update_project_counter_caches merge_request.update_project_counter_caches
delete_non_latest_diffs(merge_request) delete_non_latest_diffs(merge_request)
cancel_review_app_jobs!(merge_request) cancel_review_app_jobs!(merge_request)
......
...@@ -13,7 +13,7 @@ module MergeRequests ...@@ -13,7 +13,7 @@ module MergeRequests
execute_hooks(merge_request, 'reopen') execute_hooks(merge_request, 'reopen')
merge_request.reload_diff(current_user) merge_request.reload_diff(current_user)
merge_request.mark_as_unchecked merge_request.mark_as_unchecked
invalidate_cache_counts(merge_request, users: merge_request.assignees) invalidate_cache_counts(merge_request, users: merge_request.assignees | merge_request.reviewers)
merge_request.update_project_counter_caches merge_request.update_project_counter_caches
merge_request.cache_merge_request_closes_issues!(current_user) merge_request.cache_merge_request_closes_issues!(current_user)
merge_request.cleanup_schedule&.destroy merge_request.cleanup_schedule&.destroy
......
---
title: Invalidate reviews counter cache when MR gets closed/merged/reopened
merge_request: 51055
author:
type: fixed
...@@ -18,6 +18,7 @@ RSpec.describe MergeRequests::CloseService do ...@@ -18,6 +18,7 @@ RSpec.describe MergeRequests::CloseService do
describe '#execute' do describe '#execute' do
it_behaves_like 'cache counters invalidator' it_behaves_like 'cache counters invalidator'
it_behaves_like 'merge request reviewers cache counters invalidator'
context 'valid params' do context 'valid params' do
let(:service) { described_class.new(project, user, {}) } let(:service) { described_class.new(project, user, {}) }
......
...@@ -15,6 +15,7 @@ RSpec.describe MergeRequests::PostMergeService do ...@@ -15,6 +15,7 @@ RSpec.describe MergeRequests::PostMergeService do
describe '#execute' do describe '#execute' do
it_behaves_like 'cache counters invalidator' it_behaves_like 'cache counters invalidator'
it_behaves_like 'merge request reviewers cache counters invalidator'
it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do it 'refreshes the number of open merge requests for a valid MR', :use_clean_rails_memory_store_caching do
# Cache the counter before the MR changed state. # Cache the counter before the MR changed state.
......
...@@ -17,6 +17,7 @@ RSpec.describe MergeRequests::ReopenService do ...@@ -17,6 +17,7 @@ RSpec.describe MergeRequests::ReopenService do
describe '#execute' do describe '#execute' do
it_behaves_like 'cache counters invalidator' it_behaves_like 'cache counters invalidator'
it_behaves_like 'merge request reviewers cache counters invalidator'
context 'valid params' do context 'valid params' do
let(:service) { described_class.new(project, user, {}) } let(:service) { described_class.new(project, user, {}) }
......
...@@ -58,3 +58,18 @@ RSpec.shared_examples 'reviewer_ids filter' do ...@@ -58,3 +58,18 @@ RSpec.shared_examples 'reviewer_ids filter' do
end end
end end
end end
RSpec.shared_examples 'merge request reviewers cache counters invalidator' do
let(:reviewer_1) { create(:user) }
let(:reviewer_2) { create(:user) }
before do
merge_request.update!(reviewers: [reviewer_1, reviewer_2])
end
it 'invalidates counter cache for reviewers' do
expect(merge_request.reviewers).to all(receive(:invalidate_merge_request_cache_counts))
described_class.new(project, user, {}).execute(merge_request)
end
end
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