From c5ae0e7faefa3272b7204ca48259ffc041000b98 Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Tue, 21 Oct 2025 22:55:28 +0530 Subject: [PATCH 1/3] Use correct cursor pattern This commit fixes cursor for keyset pagination in groups and projects audit event finder EE: true Changelog: added --- .../audit_events/base_audit_event_finder.rb | 6 ++- .../group_audit_event_finder_spec.rb | 53 +++++++++++++++++++ .../project_audit_event_finder_spec.rb | 53 +++++++++++++++++++ 3 files changed, 111 insertions(+), 1 deletion(-) 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 f0a2440ea27540..8ccec2313a4a46 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,11 @@ def by_author(audit_events) end def sort(audit_events) - audit_events.order_by(params[:sort], use_created_at: true) + if params[:pagination] == 'keyset' + audit_events.order_by(params[:sort]) + else + audit_events.order_by(params[:sort], use_created_at: true) + end end def valid_author_id? 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 07429369fdfed0..cd4af5703ed61b 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 2e0bfce94dcc95..217908bcef0c4c 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) -- GitLab From 0f5f4843ceb8c21ece837ab001bb5b3287f0e51e Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Wed, 22 Oct 2025 14:23:42 +0530 Subject: [PATCH 2/3] Add comment --- ee/app/finders/audit_events/base_audit_event_finder.rb | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) 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 8ccec2313a4a46..32793b995e4d3b 100644 --- a/ee/app/finders/audit_events/base_audit_event_finder.rb +++ b/ee/app/finders/audit_events/base_audit_event_finder.rb @@ -40,11 +40,8 @@ def by_author(audit_events) end def sort(audit_events) - if params[:pagination] == 'keyset' - audit_events.order_by(params[:sort]) - else - audit_events.order_by(params[:sort], use_created_at: true) - end + 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? -- GitLab From 38a70e6e23ef0fc8f7e5c7dd43852fd1d5a82a8f Mon Sep 17 00:00:00 2001 From: harsimarsandhu Date: Wed, 22 Oct 2025 14:58:49 +0530 Subject: [PATCH 3/3] Add pagination param into finder params --- ee/lib/ee/api/groups.rb | 2 +- ee/lib/ee/api/projects.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/ee/lib/ee/api/groups.rb b/ee/lib/ee/api/groups.rb index 2eb02e99d9f5ad..9b0a0bfbc0d281 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 2a6ae412ccf44b..3872a4a95b76bd 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 -- GitLab