Commit 5d2941d6 authored by Sean McGivern's avatar Sean McGivern

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

Disable ETag caching on notes due to issues with pagination

See merge request gitlab-org/gitlab!52765
parents 1fc783a6 27fdce1a
...@@ -31,9 +31,9 @@ module NotesActions ...@@ -31,9 +31,9 @@ module NotesActions
# We know there's more data, so tell the frontend to poll again after 1ms # We know there's more data, so tell the frontend to poll again after 1ms
set_polling_interval_header(interval: 1) if meta[:more] set_polling_interval_header(interval: 1) if meta[:more]
# Only present an ETag for the empty response to ensure pagination works # We might still want to investigate further adjusting ETag caching with paginated notes, but
# as expected # let's avoid ETag caching for now until we confirm the viability of paginated notes.
::Gitlab::EtagCaching::Middleware.skip!(response) if notes.present? ::Gitlab::EtagCaching::Middleware.skip!(response)
render json: meta.merge(notes: notes) render json: meta.merge(notes: notes)
end end
......
...@@ -150,7 +150,7 @@ RSpec.describe Projects::NotesController do ...@@ -150,7 +150,7 @@ RSpec.describe Projects::NotesController do
end end
it 'returns an empty page of notes' do it 'returns an empty page of notes' do
expect(Gitlab::EtagCaching::Middleware).not_to receive(:skip!) expect(Gitlab::EtagCaching::Middleware).to receive(:skip!)
request.headers['X-Last-Fetched-At'] = microseconds(Time.zone.now) request.headers['X-Last-Fetched-At'] = microseconds(Time.zone.now)
...@@ -169,6 +169,8 @@ RSpec.describe Projects::NotesController do ...@@ -169,6 +169,8 @@ RSpec.describe Projects::NotesController do
end end
it 'returns all notes' do it 'returns all notes' do
expect(Gitlab::EtagCaching::Middleware).to receive(:skip!)
get :index, params: request_params get :index, params: request_params
expect(json_response['notes'].count).to eq((page_1 + page_2 + page_3).size + 1) expect(json_response['notes'].count).to eq((page_1 + page_2 + page_3).size + 1)
......
...@@ -18,9 +18,7 @@ RSpec.describe 'Project noteable notes' do ...@@ -18,9 +18,7 @@ RSpec.describe 'Project noteable notes' do
login_as(user) login_as(user)
end end
it 'does not set a Gitlab::EtagCaching ETag if there is a note' do it 'does not set a Gitlab::EtagCaching ETag' do
create(:note_on_merge_request, noteable: merge_request, project: merge_request.project)
get notes_path get notes_path
expect(response).to have_gitlab_http_status(:ok) expect(response).to have_gitlab_http_status(:ok)
...@@ -29,12 +27,5 @@ RSpec.describe 'Project noteable notes' do ...@@ -29,12 +27,5 @@ RSpec.describe 'Project noteable notes' do
# interfere with notes pagination # interfere with notes pagination
expect(response_etag).not_to eq(stored_etag) expect(response_etag).not_to eq(stored_etag)
end 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
end end
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