CloudToys/puyopuyotetr.is
1
0
Fork 0
Code

Fix pagination parameters in GET /api/v2_alpha/notificatins (#31509)

Browse Source
This commit is contained in:
Claire 2024-08-20 15:54:08 +02:00 committed by GitHub
parent fa2e7b1708
commit 711e1fce0a
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
2 changed files with 27 additions and 9 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

8
app/models/notification.rb
View File

@ -180,8 +180,8 @@ class Notification < ApplicationRecord
# Notifications that have no `group_key` each count as a separate group.
def paginate_groups_by_max_id(limit, max_id: nil, since_id: nil)
query = reorder(id: :desc)
query = query.where(id: ...max_id) if max_id.present?
query = query.where(id: (since_id + 1)...) if since_id.present?
query = query.where(id: ...(max_id.to_i)) if max_id.present?
query = query.where(id: (since_id.to_i + 1)...) if since_id.present?
query.paginate_groups(limit, :desc)
end
@ -190,8 +190,8 @@ class Notification < ApplicationRecord
# Results will be in ascending order by id.
def paginate_groups_by_min_id(limit, max_id: nil, min_id: nil)
query = reorder(id: :asc)
query = query.where(id: (min_id + 1)...) if min_id.present?
query = query.where(id: ...max_id) if max_id.present?
query = query.where(id: (min_id.to_i + 1)...) if min_id.present?
query = query.where(id: ...(max_id.to_i)) if max_id.present?
query.paginate_groups(limit, :asc)
end

28
spec/requests/api/v2_alpha/notifications_spec.rb
View File

@ -97,8 +97,7 @@ RSpec.describe 'Notifications' do
before do
first_status = PostStatusService.new.call(user.account, text: 'Test')
ReblogService.new.call(bob.account, first_status)
mentioning_status = PostStatusService.new.call(bob.account, text: 'Hello @alice')
mentioning_status.mentions.first
PostStatusService.new.call(bob.account, text: 'Hello @alice')
FavouriteService.new.call(bob.account, first_status)
FavouriteService.new.call(tom.account, first_status)
FollowService.new.call(bob.account, user.account)
@ -141,18 +140,37 @@ RSpec.describe 'Notifications' do
context 'with limit param' do
let(:params) { { limit: 3 } }
let(:notifications) { user.account.notifications.reorder(id: :desc) }
it 'returns the requested number of notifications paginated', :aggregate_failures do
subject
notifications = user.account.notifications
expect(body_as_json[:notification_groups].size)
.to eq(params[:limit])
expect(response)
.to include_pagination_headers(
prev: api_v2_alpha_notifications_url(limit: params[:limit], min_id: notifications.last.id),
prev: api_v2_alpha_notifications_url(limit: params[:limit], min_id: notifications.first.id),
# TODO: one downside of the current approach is that we return the first ID matching the group,
# not the last that has been skipped, so pagination is very likely to give overlap
next: api_v2_alpha_notifications_url(limit: params[:limit], max_id: notifications[3].id)
)
end
end
context 'with since_id param' do
let(:params) { { since_id: notifications[2].id } }
let(:notifications) { user.account.notifications.reorder(id: :desc) }
it 'returns the requested number of notifications paginated', :aggregate_failures do
subject
expect(body_as_json[:notification_groups].size)
.to eq(2)
expect(response)
.to include_pagination_headers(
prev: api_v2_alpha_notifications_url(limit: params[:limit], min_id: notifications.first.id),
# TODO: one downside of the current approach is that we return the first ID matching the group,
# not the last that has been skipped, so pagination is very likely to give overlap
next: api_v2_alpha_notifications_url(limit: params[:limit], max_id: notifications[1].id)