diff --git a/ee/app/finders/audit_events/base_audit_event_finder.rb b/ee/app/finders/audit_events/base_audit_event_finder.rb index f0a2440ea27540cf475b45b8af8bb0ea02ecefd3..32793b995e4d3bff5c1f40241a015e3382218e3f 100644 --- a/ee/app/finders/audit_events/base_audit_event_finder.rb +++ b/ee/app/finders/audit_events/base_audit_event_finder.rb @@ -40,7 +40,8 @@ def by_author(audit_events) end def sort(audit_events) - audit_events.order_by(params[:sort], use_created_at: true) + use_created_at = params[:pagination] != 'keyset' # Keyset must use ID-only ordering for cursor compatibility + audit_events.order_by(params[:sort], use_created_at: use_created_at) end def valid_author_id? diff --git a/ee/lib/ee/api/groups.rb b/ee/lib/ee/api/groups.rb index 2eb02e99d9f5ad134661d51d50f1c51f53bb396d..9b0a0bfbc0d2815f99ccbb8e3c7afa3b6dc9605a 100644 --- a/ee/lib/ee/api/groups.rb +++ b/ee/lib/ee/api/groups.rb @@ -97,7 +97,7 @@ def check_audit_events_available!(group) def audit_event_finder_params params - .slice(:created_after, :created_before) + .slice(:created_after, :created_before, :pagination) .then { |params| filter_by_author(params) } end diff --git a/ee/lib/ee/api/projects.rb b/ee/lib/ee/api/projects.rb index 2a6ae412ccf44bddbaae6034363bc94aa0419795..3872a4a95b76bd80a492c3c3ae1c4c9a02bbee68 100644 --- a/ee/lib/ee/api/projects.rb +++ b/ee/lib/ee/api/projects.rb @@ -135,7 +135,7 @@ def check_audit_events_available!(project) def audit_event_finder_params params - .slice(:created_after, :created_before) + .slice(:created_after, :created_before, :pagination) .then { |params| filter_by_author(params) } end diff --git a/ee/spec/finders/audit_events/group_audit_event_finder_spec.rb b/ee/spec/finders/audit_events/group_audit_event_finder_spec.rb index 07429369fdfed0305733ac695f66a512e3168ad9..cd4af5703ed61b0abfd9ff3979f611ae719cc6c2 100644 --- a/ee/spec/finders/audit_events/group_audit_event_finder_spec.rb +++ b/ee/spec/finders/audit_events/group_audit_event_finder_spec.rb @@ -145,6 +145,59 @@ end end end + + context 'when using keyset pagination' do + let(:params) { { pagination: 'keyset', sort: 'created_desc' } } + + it 'orders by id only, not created_at' do + result = finder.execute + + expect(result.to_sql).not_to include('created_at') + expect(result.to_sql).to include('ORDER BY') + expect(result.to_sql).to include('"group_audit_events"."id" DESC') + end + + it 'does not raise error when generating keyset cursor' do + result = finder.execute.limit(1) + + expect do + Gitlab::Pagination::Keyset::Paginator.new(scope: result).cursor_for_next_page + end.not_to raise_error + end + + context 'with created_asc sort' do + let(:params) { { pagination: 'keyset', sort: 'created_asc' } } + + it 'orders by id asc only' do + result = finder.execute + + expect(result.to_sql).not_to include('created_at') + expect(result.to_sql).to include('"group_audit_events"."id" ASC') + end + end + end + + context 'when using offset pagination (default)' do + let(:params) { { sort: 'created_desc' } } + + it 'orders by created_at and id' do + result = finder.execute + + expect(result.to_sql).to include('created_at') + expect(result.to_sql).to include('"group_audit_events"."id" DESC') + end + + context 'with created_asc sort' do + let(:params) { { sort: 'created_asc' } } + + it 'orders by created_at asc and id asc' do + result = finder.execute + + expect(result.to_sql).to include('"group_audit_events"."created_at" ASC') + expect(result.to_sql).to include('"group_audit_events"."id" ASC') + end + end + end end describe '#find_by!' do diff --git a/ee/spec/finders/audit_events/project_audit_event_finder_spec.rb b/ee/spec/finders/audit_events/project_audit_event_finder_spec.rb index 2e0bfce94dcc957067f6d2947e38444f81385d33..217908bcef0c4c0aad4ec018edd181d7add4a1a0 100644 --- a/ee/spec/finders/audit_events/project_audit_event_finder_spec.rb +++ b/ee/spec/finders/audit_events/project_audit_event_finder_spec.rb @@ -146,6 +146,59 @@ end end + context 'when using keyset pagination' do + let(:params) { { pagination: 'keyset', sort: 'created_desc' } } + + it 'orders by id only, not created_at' do + result = finder.execute + + expect(result.to_sql).not_to include('created_at') + expect(result.to_sql).to include('ORDER BY') + expect(result.to_sql).to include('"project_audit_events"."id" DESC') + end + + it 'does not raise error when generating keyset cursor' do + result = finder.execute.limit(1) + + expect do + Gitlab::Pagination::Keyset::Paginator.new(scope: result).cursor_for_next_page + end.not_to raise_error + end + + context 'with created_asc sort' do + let(:params) { { pagination: 'keyset', sort: 'created_asc' } } + + it 'orders by id asc only' do + result = finder.execute + + expect(result.to_sql).not_to include('created_at') + expect(result.to_sql).to include('"project_audit_events"."id" ASC') + end + end + end + + context 'when using offset pagination (default)' do + let(:params) { { sort: 'created_desc' } } + + it 'orders by created_at and id' do + result = finder.execute + + expect(result.to_sql).to include('created_at') + expect(result.to_sql).to include('"project_audit_events"."id" DESC') + end + + context 'with created_asc sort' do + let(:params) { { sort: 'created_asc' } } + + it 'orders by created_at asc and id asc' do + result = finder.execute + + expect(result.to_sql).to include('"project_audit_events"."created_at" ASC') + expect(result.to_sql).to include('"project_audit_events"."id" ASC') + end + end + end + describe 'offset optimization' do let_it_be(:pagination_events) do create_list(:audit_events_project_audit_event, 5, project_id: project_1.id, created_at: 3.days.ago)