Merge branch 'revert-5d2941d6' into 'master'

Revert "Merge branch 'disable-etag-caching-for-notes' into 'master'"

See merge request gitlab-org/gitlab!54450

app/controllers/concerns/notes_actions.rb

@@ -31,9 +31,9 @@ module NotesActions
     # We know there's more data, so tell the frontend to poll again after 1ms
     set_polling_interval_header(interval: 1) if meta[:more]
 
-    # We might still want to investigate further adjusting ETag caching with paginated notes, but
-    # let's avoid ETag caching for now until we confirm the viability of paginated notes.
-    ::Gitlab::EtagCaching::Middleware.skip!(response)
+    # Only present an ETag for the empty response to ensure pagination works
+    # as expected
+    ::Gitlab::EtagCaching::Middleware.skip!(response) if notes.present?
 
     render json: meta.merge(notes: notes)
   end

spec/controllers/projects/notes_controller_spec.rb

@@ -150,7 +150,7 @@ RSpec.describe Projects::NotesController do
         end
 
         it 'returns an empty page of notes' do
-          expect(Gitlab::EtagCaching::Middleware).to receive(:skip!)
+          expect(Gitlab::EtagCaching::Middleware).not_to receive(:skip!)
 
           request.headers['X-Last-Fetched-At'] = microseconds(Time.zone.now)
 
@@ -169,8 +169,6 @@ RSpec.describe Projects::NotesController do
         end
 
         it 'returns all notes' do
-          expect(Gitlab::EtagCaching::Middleware).to receive(:skip!)
-
           get :index, params: request_params
 
           expect(json_response['notes'].count).to eq((page_1 + page_2 + page_3).size + 1)

spec/requests/projects/noteable_notes_spec.rb

@@ -18,7 +18,9 @@ RSpec.describe 'Project noteable notes' do
       login_as(user)
     end
 
-    it 'does not set a Gitlab::EtagCaching ETag' do
+    it 'does not set a Gitlab::EtagCaching ETag if there is a note' do
+      create(:note_on_merge_request, noteable: merge_request, project: merge_request.project)
+
       get notes_path
 
       expect(response).to have_gitlab_http_status(:ok)
@@ -27,5 +29,12 @@ RSpec.describe 'Project noteable notes' do
       # interfere with notes pagination
       expect(response_etag).not_to eq(stored_etag)
     end
+
+    it 'sets a Gitlab::EtagCaching ETag if there is no note' do
+      get notes_path
+
+      expect(response).to have_gitlab_http_status(:ok)
+      expect(response_etag).to eq(stored_etag)
+    end
   end
 end