1 files changed, 148 insertions, 127 deletions

diff --git a/spec/requests/projects/merge_requests_discussions_spec.rb b/spec/requests/projects/merge_requests_discussions_spec.rb
index c761af86c16..9503dafcf2a 100644
--- a/spec/requests/projects/merge_requests_discussions_spec.rb
+++ b/spec/requests/projects/merge_requests_discussions_spec.rb
@@ -16,9 +16,16 @@ RSpec.describe 'merge requests discussions' do
       login_as(user)
     end
 
+    # rubocop:disable RSpec/InstanceVariable
     def send_request
-      get discussions_namespace_project_merge_request_path(namespace_id: project.namespace, project_id: project, id: merge_request.iid)
+      get(
+        discussions_namespace_project_merge_request_path(namespace_id: project.namespace, project_id: project, id: merge_request.iid),
+        headers: { 'If-None-Match' => @etag }
+      )
+
+      @etag = response.etag
     end
+    # rubocop:enable RSpec/InstanceVariable
 
     it 'returns 200' do
       send_request
@@ -63,11 +70,6 @@ RSpec.describe 'merge requests discussions' do
       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
-        send_request
-      end
-
       shared_examples 'cache miss' do
         it 'does not hit a warm cache' do
           expect_next_instance_of(DiscussionSerializer) do |serializer|
@@ -80,176 +82,195 @@ RSpec.describe 'merge requests discussions' do
         end
       end
 
-      it 'gets cached on subsequent requests' do
-        expect_next_instance_of(DiscussionSerializer) do |serializer|
-          expect(serializer).not_to receive(:represent)
-        end
+      shared_examples 'cache hit' do
+        it 'gets cached on subsequent requests' do
+          expect_next_instance_of(DiscussionSerializer) do |serializer|
+            expect(serializer).not_to receive(:represent)
+          end
 
-        send_request
+          send_request
+        end
       end
 
-      context 'when a note in a discussion got updated' do
+      context 'when mr_discussions_http_cache and disabled_mr_discussions_redis_cache are enabled' do
         before do
-          first_note.update!(updated_at: 1.minute.from_now)
+          send_request
         end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
-        end
-      end
+        it_behaves_like 'cache hit'
 
-      context 'when a note in a discussion got its reference state updated' do
-        before do
-          reference.close!
-        end
+        context 'when a note in a discussion got updated' do
+          before do
+            first_note.update!(updated_at: 1.minute.from_now)
+          end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when a note in a discussion got resolved' do
-        before do
-          travel_to(1.minute.from_now) do
-            first_note.resolve!(user)
+        context 'when a note in a discussion got its reference state updated' do
+          before do
+            reference.close!
           end
-        end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when a note is added to a discussion' do
-        let!(:third_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) }
+        context 'when a note in a discussion got resolved' do
+          before do
+            travel_to(1.minute.from_now) do
+              first_note.resolve!(user)
+            end
+          end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note, third_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when a note is removed from a discussion' do
-        before do
-          second_note.destroy!
-        end
+        context 'when a note is added to a discussion' do
+          let!(:third_note) { create(:diff_note_on_merge_request, in_reply_to: first_note, noteable: merge_request, project: project) }
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note, third_note] }
+          end
         end
-      end
 
-      context 'when an emoji is awarded to a note in discussion' do
-        before do
-          travel_to(1.minute.from_now) do
-            create(:award_emoji, awardable: first_note)
+        context 'when a note is removed from a discussion' do
+          before do
+            second_note.destroy!
           end
-        end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note] }
+          end
         end
-      end
 
-      context 'when an award emoji is removed from a note in discussion' do
-        before do
-          travel_to(1.minute.from_now) do
-            award_emoji.destroy!
+        context 'when an emoji is awarded to a note in discussion' do
+          before do
+            travel_to(1.minute.from_now) do
+              create(:award_emoji, awardable: first_note)
+            end
           end
-        end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when the diff note position changes' do
-        before do
-          # This replicates a position change wherein timestamps aren't updated
-          # which is why `Gitlab::Timeless.timeless` is utilized. This is the
-          # same approach being used in Discussions::UpdateDiffPositionService
-          # which is responsible for updating the positions of diff discussions
-          # when MR updates.
-          first_note.position = Gitlab::Diff::Position.new(
-            old_path: first_note.position.old_path,
-            new_path: first_note.position.new_path,
-            old_line: first_note.position.old_line,
-            new_line: first_note.position.new_line + 1,
-            diff_refs: first_note.position.diff_refs
-          )
-
-          Gitlab::Timeless.timeless(first_note, &:save)
-        end
+        context 'when an award emoji is removed from a note in discussion' do
+          before do
+            travel_to(1.minute.from_now) do
+              award_emoji.destroy!
+            end
+          end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when the HEAD diff note position changes' do
-        before do
-          # This replicates a DiffNotePosition change. This is the same approach
-          # being used in Discussions::CaptureDiffNotePositionService which is
-          # responsible for updating/creating DiffNotePosition of a diff discussions
-          # in relation to HEAD diff.
-          new_position = Gitlab::Diff::Position.new(
-            old_path: first_note.position.old_path,
-            new_path: first_note.position.new_path,
-            old_line: first_note.position.old_line,
-            new_line: first_note.position.new_line + 1,
-            diff_refs: first_note.position.diff_refs
-          )
-
-          DiffNotePosition.create_or_update_for(
-            first_note,
-            diff_type: :head,
-            position: new_position,
-            line_code: 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521'
-          )
-        end
+        context 'when the diff note position changes' do
+          before do
+            # This replicates a position change wherein timestamps aren't updated
+            # which is why `Gitlab::Timeless.timeless` is utilized. This is the
+            # same approach being used in Discussions::UpdateDiffPositionService
+            # which is responsible for updating the positions of diff discussions
+            # when MR updates.
+            first_note.position = Gitlab::Diff::Position.new(
+              old_path: first_note.position.old_path,
+              new_path: first_note.position.new_path,
+              old_line: first_note.position.old_line,
+              new_line: first_note.position.new_line + 1,
+              diff_refs: first_note.position.diff_refs
+            )
+
+            Gitlab::Timeless.timeless(first_note, &:save)
+          end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when author detail changes' do
-        before do
-          author.update!(name: "#{author.name} (Updated)")
-        end
+        context 'when the HEAD diff note position changes' do
+          before do
+            # This replicates a DiffNotePosition change. This is the same approach
+            # being used in Discussions::CaptureDiffNotePositionService which is
+            # responsible for updating/creating DiffNotePosition of a diff discussions
+            # in relation to HEAD diff.
+            new_position = Gitlab::Diff::Position.new(
+              old_path: first_note.position.old_path,
+              new_path: first_note.position.new_path,
+              old_line: first_note.position.old_line,
+              new_line: first_note.position.new_line + 1,
+              diff_refs: first_note.position.diff_refs
+            )
+
+            DiffNotePosition.create_or_update_for(
+              first_note,
+              diff_type: :head,
+              position: new_position,
+              line_code: 'bd4b7bfff3a247ccf6e3371c41ec018a55230bcc_534_521'
+            )
+          end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when author status changes' do
-        before do
-          Users::SetStatusService.new(author, message: "updated status").execute
+        context 'when author detail changes' do
+          before do
+            author.update!(name: "#{author.name} (Updated)")
+          end
+
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+        context 'when author status changes' do
+          before do
+            Users::SetStatusService.new(author, message: "updated status").execute
+          end
+
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
-      end
 
-      context 'when author role changes' do
-        before do
-          Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(author_membership)
+        context 'when author role changes' do
+          before do
+            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
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
+        context 'when current_user role changes' do
+          before do
+            Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(project.member(user))
+          end
+
+          it_behaves_like 'cache miss' do
+            let(:changed_notes) { [first_note, second_note] }
+          end
         end
       end
 
-      context 'when current_user role changes' do
+      context 'when disabled_mr_discussions_redis_cache is disabled' do
         before do
-          Members::UpdateService.new(owner, access_level: Gitlab::Access::GUEST).execute(project.member(user))
+          stub_feature_flags(disabled_mr_discussions_redis_cache: false)
+          send_request
         end
 
-        it_behaves_like 'cache miss' do
-          let(:changed_notes) { [first_note, second_note] }
-        end
+        it_behaves_like 'cache hit'
       end
     end
   end