Commit 8a00b1c4 authored by Patrick Bajao's avatar Patrick Bajao

Fix stale MR discussion cache when author role changes

Currently, when MR discussion is cached and the author's role has
changed (e.g. Developer to Guest), the role pill and some actions
will show as enabled (e.g. being able to resolve a discussion
but it'll not actually work).

This fix adds the max access of author to the cache key so when it
changes, the cache will be invalidated too.
parent 81c42be4
......@@ -583,6 +583,7 @@ class Note < ApplicationRecord
def post_processed_cache_key
cache_key_items = [cache_key, author&.cache_key]
cache_key_items << project.team.human_max_access(author&.id) if author.present?
cache_key_items << Digest::SHA1.hexdigest(redacted_note_html) if redacted_note_html.present?
cache_key_items.join(':')
......
......@@ -1573,7 +1573,7 @@ RSpec.describe Note do
let(:note) { build(:note) }
it 'returns cache key and author cache key by default' do
expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}")
expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{note.project.team.human_max_access(note.author_id)}")
end
context 'when note has no author' do
......@@ -1592,7 +1592,7 @@ RSpec.describe Note do
end
it 'returns cache key with redacted_note_html sha' do
expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{Digest::SHA1.hexdigest(redacted_note_html)}")
expect(note.post_processed_cache_key).to eq("#{note.cache_key}:#{note.author.cache_key}:#{note.project.team.human_max_access(note.author_id)}:#{Digest::SHA1.hexdigest(redacted_note_html)}")
end
end
end
......
......@@ -59,6 +59,7 @@ RSpec.describe 'merge requests discussions' do
let!(:first_note) { create(:diff_note_on_merge_request, author: author, noteable: merge_request, project: project, note: "reference: #{reference.to_reference}") }
let!(:second_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) }
let!(:award_emoji) { create(:award_emoji, awardable: first_note) }
let!(:author_membership) { project.add_maintainer(author) }
before do
# Make a request to cache the discussions
......@@ -229,6 +230,16 @@ RSpec.describe 'merge requests discussions' do
end
end
context 'when author role changes' do
before do
Members::UpdateService.new(user, access_level: Gitlab::Access::GUEST).execute(author_membership)
end
it_behaves_like 'cache miss' do
let(:changed_notes) { [first_note, second_note] }
end
end
context 'when merge_request_discussion_cache is disabled' do
before do
stub_feature_flags(merge_request_discussion_cache: false)
......
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