DEV: Add support for limit in notifications index w/o recent param (#24423)

Currently to use a limit in the notifications index, you have to also pass recent: true as a param.

This PR:

Adds optional limit param to be used in the notifications query, regardless of the presence of recent
Raises the max limit of the response with recent present from 50 -> 60. It is super weird we have a hard-limit of 50 before with recent param, and 60 without the param.
This commit is contained in:
Mark VanLandingham
2023-11-17 08:22:18 -06:00
committed by GitHub
parent a5ed0ea5d6
commit 7d35e406ba
2 changed files with 32 additions and 4 deletions

View File

@ -90,12 +90,34 @@ RSpec.describe NotificationsController do
describe "when limit params is invalid" do
include_examples "invalid limit params",
"/notifications.json",
described_class::INDEX_LIMIT,
described_class::INDEX_LIMIT + 1,
params: {
recent: true,
}
end
it "respects limit param and properly bumps offset for load_more_notifications URL" do
7.times { notification = Fabricate(:notification, user: user) }
get "/notifications.json", params: { username: user.username, limit: 2 }
expect(response.parsed_body["notifications"].count).to eq(2)
expect(response.parsed_body["load_more_notifications"]).to eq(
"/notifications?limit=2&offset=2&username=#{user.username}",
)
# Same as response above but we need .json added before query params.
get "/notifications.json?limit=2&offset=2&username=#{user.username}"
expect(response.parsed_body["load_more_notifications"]).to eq(
"/notifications?limit=2&offset=4&username=#{user.username}",
)
# We are seeing that the offset is increasing properly and limit is staying the same
get "/notifications.json?limit=2&offset=4&username=#{user.username}"
expect(response.parsed_body["load_more_notifications"]).to eq(
"/notifications?limit=2&offset=6&username=#{user.username}",
)
end
it "get notifications with all filters" do
notification = Fabricate(:notification, user: user)
notification2 = Fabricate(:notification, user: user)