Merge branch '337723-can-t-edit-file-via-single-file-editor-when-mr-diffs-is-cached' into 'master'

Fix single file editor rendering when MR diffs is cached

See merge request gitlab-org/gitlab!67658

app/controllers/projects/merge_requests/diffs_controller.rb

@@ -27,10 +27,10 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     diff_options_hash[:paths] = params[:paths] if params[:paths]
 
     diffs = @compare.diffs_in_batch(params[:page], params[:per_page], diff_options: diff_options_hash)
-    positions = @merge_request.note_positions_for_paths(diffs.diff_file_paths, current_user)
+    unfoldable_positions = @merge_request.note_positions_for_paths(diffs.diff_file_paths, current_user).unfoldable
     environment = @merge_request.environments_for(current_user, latest: true).last
 
-    diffs.unfold_diff_files(positions.unfoldable)
+    diffs.unfold_diff_files(unfoldable_positions)
     diffs.write_cache
 
     options = {
@@ -43,10 +43,24 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic
     }
 
     if diff_options_hash[:paths].blank? && Feature.enabled?(:diffs_batch_render_cached, project, default_enabled: :yaml)
+      # NOTE: Any variables that would affect the resulting json needs to be added to the cache_context to avoid stale cache issues.
+      cache_context = [
+        current_user&.cache_key,
+        environment&.cache_key,
+        unfoldable_positions.map(&:to_h),
+        diff_view,
+        params[:w],
+        params[:expanded],
+        params[:page],
+        params[:per_page],
+        options[:merge_ref_head_diff],
+        options[:allow_tree_conflicts]
+      ]
+
       render_cached(
         diffs,
         with: PaginatedDiffSerializer.new(current_user: current_user),
-        cache_context: -> (_) { [diff_view, params[:w], params[:expanded], params[:per_page], params[:page]] },
+        cache_context: -> (_) { [Digest::SHA256.hexdigest(cache_context.to_s)] },
         **options
       )
     else

spec/requests/projects/merge_requests/diffs_spec.rb

@@ -76,6 +76,78 @@ RSpec.describe 'Merge Requests Diffs' do
           subject
         end
 
+        context 'with the different user' do
+          let(:another_user) { create(:user) }
+
+          before do
+            project.add_maintainer(another_user)
+            sign_in(another_user)
+          end
+
+          it_behaves_like 'serializes diffs with expected arguments' do
+            let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
+            let(:expected_options) { collection_arguments(total_pages: 20) }
+          end
+        end
+
+        context 'with a new unfoldable diff position' do
+          let(:unfoldable_position) do
+            create(:diff_position)
+          end
+
+          before do
+            expect_next_instance_of(Gitlab::Diff::PositionCollection) do |instance|
+              expect(instance)
+                .to receive(:unfoldable)
+                .and_return([unfoldable_position])
+            end
+          end
+
+          it_behaves_like 'serializes diffs with expected arguments' do
+            let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
+            let(:expected_options) { collection_arguments(total_pages: 20) }
+          end
+        end
+
+        context 'with a new environment' do
+          let(:environment) do
+            create(:environment, :available, project: project)
+          end
+
+          let!(:deployment) do
+            create(:deployment, :success, environment: environment, ref: merge_request.source_branch)
+          end
+
+          it_behaves_like 'serializes diffs with expected arguments' do
+            let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
+            let(:expected_options) { collection_arguments(total_pages: 20).merge(environment: environment) }
+          end
+        end
+
+        context 'with disabled display_merge_conflicts_in_diff feature' do
+          before do
+            stub_feature_flags(display_merge_conflicts_in_diff: false)
+          end
+
+          it_behaves_like 'serializes diffs with expected arguments' do
+            let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
+            let(:expected_options) { collection_arguments(total_pages: 20).merge(allow_tree_conflicts: false) }
+          end
+        end
+
+        context 'with diff_head option' do
+          subject { go(page: 0, per_page: 5, diff_head: true) }
+
+          before do
+            merge_request.create_merge_head_diff!
+          end
+
+          it_behaves_like 'serializes diffs with expected arguments' do
+            let(:collection) { Gitlab::Diff::FileCollection::MergeRequestDiffBatch }
+            let(:expected_options) { collection_arguments(total_pages: 20).merge(merge_ref_head_diff: true) }
+          end
+        end
+
         context 'with the different pagination option' do
           subject { go(page: 5, per_page: 5) }