Cursor-based pagination for /notifications
Switches `listForUser` from page-offset to cursor (`before` param,
opaque base64 of `{ts, id}`) ordered by `(created_at DESC, id DESC)`
for stable pagination even with simultaneous fan-out inserts.
Returns `{ rows, nextCursor }` instead of a bare array. Loader surfaces
`?before=<cursor>` on a "Load older" link at the bottom of the list,
shown only while `nextCursor !== null`. Default page size 50, capped
at 100. Malformed cursors fall back to "start from top" rather than
400ing — opaque cursors should not be a client validation surface.
Spec drift: delta spec adds three pagination scenarios (cursor pages
forward, tie-stable on identical `created_at`, malformed-cursor
graceful fallback). Design doc gets a new decision section explaining
the cursor choice over page-offset and why we don't compute totals.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
2892cf9360
commit
b20c8cca39
9 changed files with 206 additions and 35 deletions
|
|
@ -75,12 +75,12 @@ describe.skipIf(!runIntegration)("notifications-fanout integration", () => {
|
|||
const activityId = await makeActivity(owner, "public", "Public Walk");
|
||||
await fanout(activityId);
|
||||
|
||||
expect((await listForUser(a1)).length).toBe(1);
|
||||
expect((await listForUser(a2)).length).toBe(1);
|
||||
expect((await listForUser(p1)).length).toBe(0);
|
||||
expect((await listForUser(p2)).length).toBe(0);
|
||||
expect((await listForUser(a1)).rows.length).toBe(1);
|
||||
expect((await listForUser(a2)).rows.length).toBe(1);
|
||||
expect((await listForUser(p1)).rows.length).toBe(0);
|
||||
expect((await listForUser(p2)).rows.length).toBe(0);
|
||||
|
||||
const a1Rows = await listForUser(a1);
|
||||
const a1Rows = (await listForUser(a1)).rows;
|
||||
expect(a1Rows[0]?.type).toBe("activity_published");
|
||||
expect((a1Rows[0]?.payload as { activityName?: string })?.activityName).toBe("Public Walk");
|
||||
});
|
||||
|
|
@ -95,7 +95,7 @@ describe.skipIf(!runIntegration)("notifications-fanout integration", () => {
|
|||
await fanout(privateAct);
|
||||
await fanout(unlistedAct);
|
||||
|
||||
expect((await listForUser(f)).length).toBe(0);
|
||||
expect((await listForUser(f)).rows.length).toBe(0);
|
||||
});
|
||||
|
||||
it("is idempotent under retry — second fanout doesn't double-insert", async () => {
|
||||
|
|
@ -107,6 +107,6 @@ describe.skipIf(!runIntegration)("notifications-fanout integration", () => {
|
|||
await fanout(activityId);
|
||||
await fanout(activityId);
|
||||
|
||||
expect((await listForUser(f)).length).toBe(1);
|
||||
expect((await listForUser(f)).rows.length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -71,8 +71,9 @@ describe.skipIf(!runIntegration)("notifications.server integration", () => {
|
|||
});
|
||||
expect(second).toBe(false);
|
||||
|
||||
const rows = await listForUser(a);
|
||||
const { rows, nextCursor } = await listForUser(a);
|
||||
expect(rows.length).toBe(1);
|
||||
expect(nextCursor).toBeNull();
|
||||
expect(rows[0]?.payloadVersion).toBe(1);
|
||||
expect(rows[0]?.payload).toMatchObject({ activityId: subject });
|
||||
});
|
||||
|
|
@ -90,7 +91,7 @@ describe.skipIf(!runIntegration)("notifications.server integration", () => {
|
|||
});
|
||||
expect(await countUnread(a)).toBe(1);
|
||||
|
||||
const all = await listForUser(a);
|
||||
const all = (await listForUser(a)).rows;
|
||||
const id = all[0]!.id;
|
||||
|
||||
// Foreign user can't mark a's notification read.
|
||||
|
|
@ -197,13 +198,13 @@ describe.skipIf(!runIntegration)("notifications.server integration", () => {
|
|||
const bRow = (await getDb().select().from(users).where(eq(users.id, b)))[0]!;
|
||||
|
||||
await followUser(a, bRow.username);
|
||||
let bRows = await listForUser(b);
|
||||
let bRows = (await listForUser(b)).rows;
|
||||
expect(bRows.length).toBe(1);
|
||||
expect(bRows[0]?.type).toBe("follow_received");
|
||||
|
||||
// Re-follow is idempotent at the follow layer — and must NOT emit again.
|
||||
await followUser(a, bRow.username);
|
||||
bRows = await listForUser(b);
|
||||
bRows = (await listForUser(b)).rows;
|
||||
expect(bRows.length).toBe(1);
|
||||
});
|
||||
|
||||
|
|
@ -213,7 +214,7 @@ describe.skipIf(!runIntegration)("notifications.server integration", () => {
|
|||
const bRow = (await getDb().select().from(users).where(eq(users.id, b)))[0]!;
|
||||
|
||||
await followUser(a, bRow.username);
|
||||
const bRows = await listForUser(b);
|
||||
const bRows = (await listForUser(b)).rows;
|
||||
expect(bRows.length).toBe(1);
|
||||
expect(bRows[0]?.type).toBe("follow_request_received");
|
||||
|
||||
|
|
@ -221,13 +222,64 @@ describe.skipIf(!runIntegration)("notifications.server integration", () => {
|
|||
expect(reqs.length).toBe(1);
|
||||
|
||||
await approveFollowRequest(b, reqs[0]!.id);
|
||||
const aRows = await listForUser(a);
|
||||
const aRows = (await listForUser(a)).rows;
|
||||
expect(aRows.length).toBe(1);
|
||||
expect(aRows[0]?.type).toBe("follow_request_approved");
|
||||
|
||||
// Idempotent re-approval must not double-emit.
|
||||
await approveFollowRequest(b, reqs[0]!.id);
|
||||
const aRowsAfter = await listForUser(a);
|
||||
const aRowsAfter = (await listForUser(a)).rows;
|
||||
expect(aRowsAfter.length).toBe(1);
|
||||
});
|
||||
|
||||
it("paginates with nextCursor; cursor is stable across pages and ends with null", async () => {
|
||||
const a = await makeUser({ username: `n_pg_a_${Date.now()}` });
|
||||
// Insert 7 notifications, each older than the previous, so we can
|
||||
// page through them with limit: 3. Each row has a unique subject so
|
||||
// the partial-unique constraint doesn't deduplicate them.
|
||||
for (let i = 0; i < 7; i++) {
|
||||
await createNotification({
|
||||
type: "activity_published",
|
||||
recipientUserId: a,
|
||||
actorUserId: null,
|
||||
subjectId: randomUUID(),
|
||||
payload: {
|
||||
activityId: randomUUID(),
|
||||
activityName: `act-${i}`,
|
||||
ownerUsername: "x",
|
||||
ownerDisplayName: null,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
const p1 = await listForUser(a, { limit: 3 });
|
||||
expect(p1.rows.length).toBe(3);
|
||||
expect(p1.nextCursor).not.toBeNull();
|
||||
const p1Names = p1.rows.map((r) => (r.payload as { activityName?: string })?.activityName);
|
||||
|
||||
const p2 = await listForUser(a, { limit: 3, before: p1.nextCursor! });
|
||||
expect(p2.rows.length).toBe(3);
|
||||
expect(p2.nextCursor).not.toBeNull();
|
||||
const p2Names = p2.rows.map((r) => (r.payload as { activityName?: string })?.activityName);
|
||||
// Pages must not overlap.
|
||||
expect(p1Names.some((n) => p2Names.includes(n))).toBe(false);
|
||||
|
||||
const p3 = await listForUser(a, { limit: 3, before: p2.nextCursor! });
|
||||
expect(p3.rows.length).toBe(1);
|
||||
// Only one row left → nextCursor is null (no more pages).
|
||||
expect(p3.nextCursor).toBeNull();
|
||||
});
|
||||
|
||||
it("treats malformed cursor as 'start from top' rather than erroring", async () => {
|
||||
const a = await makeUser({ username: `n_pgbad_a_${Date.now()}` });
|
||||
await createNotification({
|
||||
type: "follow_received",
|
||||
recipientUserId: a,
|
||||
actorUserId: null,
|
||||
subjectId: null,
|
||||
payload: { followerUsername: "x", followerDisplayName: null },
|
||||
});
|
||||
const r = await listForUser(a, { before: "not-a-real-cursor" });
|
||||
expect(r.rows.length).toBe(1);
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -1,5 +1,5 @@
|
|||
import { randomUUID } from "node:crypto";
|
||||
import { and, count, desc, eq, isNull, lt, sql } from "drizzle-orm";
|
||||
import { and, count, desc, eq, isNull, lt, or, sql } from "drizzle-orm";
|
||||
import { getDb } from "./db.ts";
|
||||
import { notifications } from "@trails-cool/db/schema/journal";
|
||||
import type { NotificationType } from "@trails-cool/db/schema/journal";
|
||||
|
|
@ -87,14 +87,86 @@ export interface NotificationRow {
|
|||
createdAt: Date;
|
||||
}
|
||||
|
||||
const PAGE_SIZE = 50;
|
||||
const DEFAULT_PAGE_SIZE = 50;
|
||||
const MAX_PAGE_SIZE = 100;
|
||||
|
||||
export interface ListOptions {
|
||||
/**
|
||||
* Opaque cursor from a previous response. When set, returns rows
|
||||
* strictly older than the cursor's `(createdAt, id)` position, so
|
||||
* pagination is stable even when two rows share `created_at`.
|
||||
*/
|
||||
before?: string;
|
||||
/** Soft cap; clamped to `[1, MAX_PAGE_SIZE]`. Defaults to 50. */
|
||||
limit?: number;
|
||||
}
|
||||
|
||||
export interface ListResult {
|
||||
rows: NotificationRow[];
|
||||
/**
|
||||
* Opaque cursor for the next page, or `null` when the caller has
|
||||
* reached the end. Pass back as `before` on the next request.
|
||||
*/
|
||||
nextCursor: string | null;
|
||||
}
|
||||
|
||||
interface CursorShape {
|
||||
ts: string;
|
||||
id: string;
|
||||
}
|
||||
|
||||
function encodeCursor(c: CursorShape): string {
|
||||
return Buffer.from(JSON.stringify(c), "utf8").toString("base64url");
|
||||
}
|
||||
|
||||
function decodeCursor(s: string): CursorShape | null {
|
||||
try {
|
||||
const obj = JSON.parse(Buffer.from(s, "base64url").toString("utf8")) as {
|
||||
ts?: unknown;
|
||||
id?: unknown;
|
||||
};
|
||||
if (typeof obj.ts !== "string" || typeof obj.id !== "string") return null;
|
||||
if (Number.isNaN(Date.parse(obj.ts))) return null;
|
||||
return { ts: obj.ts, id: obj.id };
|
||||
} catch {
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Cursor-paginated list of `userId`'s notifications, newest first.
|
||||
* Pass the previous response's `nextCursor` as `before` to fetch the
|
||||
* next page; an unknown / malformed cursor is treated as "start from
|
||||
* the top" rather than 400-ing, since the cursor is opaque to clients.
|
||||
*/
|
||||
export async function listForUser(
|
||||
userId: string,
|
||||
opts: { page?: number } = {},
|
||||
): Promise<NotificationRow[]> {
|
||||
opts: ListOptions = {},
|
||||
): Promise<ListResult> {
|
||||
const db = getDb();
|
||||
const page = Math.max(1, opts.page ?? 1);
|
||||
const limit = Math.min(MAX_PAGE_SIZE, Math.max(1, opts.limit ?? DEFAULT_PAGE_SIZE));
|
||||
const cursor = opts.before ? decodeCursor(opts.before) : null;
|
||||
|
||||
const baseWhere = eq(notifications.recipientUserId, userId);
|
||||
const where = cursor
|
||||
? and(
|
||||
baseWhere,
|
||||
// (created_at, id) < (cursor.ts, cursor.id)
|
||||
// Keyset comparison broken into the two-row form so Postgres
|
||||
// uses the (recipient, created_at desc) index for the leading
|
||||
// column.
|
||||
or(
|
||||
lt(notifications.createdAt, new Date(cursor.ts)),
|
||||
and(
|
||||
eq(notifications.createdAt, new Date(cursor.ts)),
|
||||
lt(notifications.id, cursor.id),
|
||||
),
|
||||
),
|
||||
)
|
||||
: baseWhere;
|
||||
|
||||
// Fetch limit + 1 so we can decide whether a `nextCursor` exists
|
||||
// without a separate count query.
|
||||
const rows = await db
|
||||
.select({
|
||||
id: notifications.id,
|
||||
|
|
@ -107,11 +179,18 @@ export async function listForUser(
|
|||
createdAt: notifications.createdAt,
|
||||
})
|
||||
.from(notifications)
|
||||
.where(eq(notifications.recipientUserId, userId))
|
||||
.orderBy(desc(notifications.createdAt))
|
||||
.limit(PAGE_SIZE)
|
||||
.offset((page - 1) * PAGE_SIZE);
|
||||
return rows;
|
||||
.where(where)
|
||||
.orderBy(desc(notifications.createdAt), desc(notifications.id))
|
||||
.limit(limit + 1);
|
||||
|
||||
const hasMore = rows.length > limit;
|
||||
const trimmed = hasMore ? rows.slice(0, limit) : rows;
|
||||
const last = trimmed[trimmed.length - 1];
|
||||
const nextCursor = hasMore && last
|
||||
? encodeCursor({ ts: last.createdAt.toISOString(), id: last.id })
|
||||
: null;
|
||||
|
||||
return { rows: trimmed, nextCursor };
|
||||
}
|
||||
|
||||
export async function countUnread(userId: string): Promise<number> {
|
||||
|
|
|
|||
|
|
@ -14,7 +14,9 @@ export async function loader({ request }: Route.LoaderArgs) {
|
|||
const user = await getSessionUser(request);
|
||||
if (!user) throw redirect("/auth/login");
|
||||
|
||||
const rows = await listForUser(user.id, { page: 1 });
|
||||
const url = new URL(request.url);
|
||||
const before = url.searchParams.get("before") ?? undefined;
|
||||
const { rows, nextCursor } = await listForUser(user.id, { before });
|
||||
|
||||
// Renderer guard: drop activity_published rows whose subject is gone
|
||||
// or no longer public (visibility flipped from public → private/unlisted).
|
||||
|
|
@ -56,6 +58,7 @@ export async function loader({ request }: Route.LoaderArgs) {
|
|||
payload,
|
||||
};
|
||||
}),
|
||||
nextCursor,
|
||||
});
|
||||
}
|
||||
|
||||
|
|
@ -145,7 +148,7 @@ function NotificationItem({ row }: { row: Row }) {
|
|||
}
|
||||
|
||||
export default function Notifications({ loaderData }: Route.ComponentProps) {
|
||||
const { notifications } = loaderData;
|
||||
const { notifications, nextCursor } = loaderData;
|
||||
const { t } = useTranslation("journal");
|
||||
const markAll = useFetcher();
|
||||
|
||||
|
|
@ -171,11 +174,23 @@ export default function Notifications({ loaderData }: Route.ComponentProps) {
|
|||
{notifications.length === 0 ? (
|
||||
<p className="mt-12 text-center text-gray-500">{t("notifications.empty")}</p>
|
||||
) : (
|
||||
<ul className="mt-6 rounded-lg border border-gray-200 bg-white">
|
||||
{notifications.map((n) => (
|
||||
<NotificationItem key={n.id} row={n} />
|
||||
))}
|
||||
</ul>
|
||||
<>
|
||||
<ul className="mt-6 rounded-lg border border-gray-200 bg-white">
|
||||
{notifications.map((n) => (
|
||||
<NotificationItem key={n.id} row={n} />
|
||||
))}
|
||||
</ul>
|
||||
{nextCursor && (
|
||||
<div className="mt-4 text-center">
|
||||
<a
|
||||
href={`/notifications?before=${encodeURIComponent(nextCursor)}`}
|
||||
className="text-sm font-medium text-blue-600 hover:underline"
|
||||
>
|
||||
{t("notifications.loadOlder")}
|
||||
</a>
|
||||
</div>
|
||||
)}
|
||||
</>
|
||||
)}
|
||||
</div>
|
||||
);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue