Commit becce866 authored by Patrick Bajao's avatar Patrick Bajao Committed by Vitali Tatarintev

Fix stale MR discussion cache when current user role changes

In https://gitlab.com/gitlab-org/gitlab/-/merge_requests/69642, we
fixed the case when note's author role changes.

It's also possible that the MR discussions cache will be stale if
the current_user role has changed.

For example, current user was a maintainer and viewed the MR page
with discussions. That user will be able to resolve comments. When
that user gets demoted to Guest, for example, they shouldn't be
able to resolve threads anymore.

Currently, that will only affect the notes they authored. For
other notes, the "Resolve" button will still show.

This fix adds the max access of the current user to the cache
context so when it changes, the cache will be invalidated too.
parent d2167efa
......@@ -159,7 +159,9 @@ module IssuableActions
discussions = Discussion.build_collection(notes, issuable)
if issuable.is_a?(MergeRequest) && Feature.enabled?(:merge_request_discussion_cache, issuable.target_project, default_enabled: :yaml)
render_cached(discussions, with: discussion_serializer, context: self)
cache_context = [current_user&.cache_key, project.team.human_max_access(current_user&.id)].join(':')
render_cached(discussions, with: discussion_serializer, cache_context: -> (_) { cache_context }, context: self)
else
render json: discussion_serializer.represent(discussions, context: self)
end
......
......@@ -5,11 +5,13 @@ require 'spec_helper'
RSpec.describe 'merge requests discussions' do
# Further tests can be found at merge_requests_controller_spec.rb
describe 'GET /:namespace/:project/-/merge_requests/:iid/discussions' do
let(:project) { create(:project, :repository) }
let(:user) { project.owner }
let(:project) { create(:project, :repository, :public) }
let(:owner) { project.owner }
let(:user) { create(:user) }
let(:merge_request) { create(:merge_request_with_diffs, target_project: project, source_project: project) }
before do
project.add_maintainer(owner)
project.add_developer(user)
login_as(user)
end
......@@ -232,7 +234,17 @@ RSpec.describe 'merge requests discussions' do
context 'when author role changes' do
before do
Members::UpdateService.new(user, access_level: Gitlab::Access::GUEST).execute(author_membership)
Members::UpdateService.new(owner, 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 current_user role changes' do
before do
Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(project.project_member(user))
end
it_behaves_like 'cache miss' do
......
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