Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 9 additions & 0 deletions drizzle/0016_doubts_composite_indexes.sql
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
-- Composite indexes for the doubts feed (issue #319).
-- Support the common filter + recency-ordering access pattern:
-- WHERE classroomId = ? [AND type = ? | AND isSolved = ?] ORDER BY createdAt DESC
-- createdAt is the trailing column so the index covers both the filter and the
-- ORDER BY (no separate sort step). Idempotent so it is safe to re-run via the
-- manual migrate runner.
CREATE INDEX IF NOT EXISTS "doubts_classroomId_createdAt_idx" ON "doubts" USING btree ("classroomId", "createdAt");--> statement-breakpoint
Comment thread
madsysharma marked this conversation as resolved.
CREATE INDEX IF NOT EXISTS "doubts_classroomId_type_createdAt_idx" ON "doubts" USING btree ("classroomId", "type", "createdAt");--> statement-breakpoint
CREATE INDEX IF NOT EXISTS "doubts_classroomId_isSolved_createdAt_idx" ON "doubts" USING btree ("classroomId", "isSolved", "createdAt");
5 changes: 3 additions & 2 deletions drizzle/meta/_journal.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
{ "idx": 13, "version": "7", "when": 1781300000000, "tag": "0013_identity_system_update", "breakpoints": true },
{ "idx": 14, "version": "7", "when": 1781340000000, "tag": "0014_practice_attempts", "breakpoints": true },
{ "idx": 15, "version": "7", "when": 1781354492608, "tag": "0015_add_onboarding_fields", "breakpoints": true },
{ "idx": 16, "version": "7", "when": 1781400000000, "tag": "0016_video_jobs", "breakpoints": true }
{ "idx": 16, "version": "7", "when": 1781400000000, "tag": "0016_doubts_composite_indexes", "breakpoints": true }
{ "idx": 17, "version": "7", "when": 1781400000000, "tag": "0017_video_jobs", "breakpoints": true }
]
}
}
4 changes: 2 additions & 2 deletions src/__tests__/api/teacher-insights.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,7 @@ describe('Teacher Insights API Endpoint', () => {
const json = await res.json();

expect(res.status).toBe(400);
expect(json.error).toBe('classroomId is required');
expect(json.error).toBe('Invalid classroom ID');
});

it('returns 403 when the user is not the teacher of the classroom', async () => {
Expand All @@ -79,7 +79,7 @@ describe('Teacher Insights API Endpoint', () => {
const json = await res.json();

expect(res.status).toBe(403);
expect(json.error).toBe('Forbidden: not the teacher of this classroom');
expect(json.error).toBe('Access denied to this classroom');
});

it('returns classroom-scoped insights for the teacher', async () => {
Expand Down
20 changes: 13 additions & 7 deletions src/__tests__/inngest/digest-functions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,15 @@ function makeStep() {
};
}

// Inngest's createFunction returns a function object whose handler is stored on
// `.fn`; invoke that directly to unit-test the handler with a mock step.
function runHandler(
fn: unknown,
step: ReturnType<typeof makeStep>,
): Promise<unknown> {
return (fn as { fn: (ctx: { step: unknown }) => Promise<unknown> }).fn({ step });
}

// ── Tests ─────────────────────────────────────────────────────────────────────

describe("sendDailyDigest — per-user step isolation", () => {
Expand Down Expand Up @@ -115,8 +124,7 @@ describe("sendDailyDigest — per-user step isolation", () => {
const { sendDailyDigest } = await import("@/inngest/functions");

const step = makeStep();
// @ts-expect-error — internal test invocation bypasses Inngest runtime types
await expect(sendDailyDigest({ step })).rejects.toThrow("SMTP timeout");
await expect(runHandler(sendDailyDigest, step)).rejects.toThrow("SMTP timeout");

// Alice's row MUST have been deleted (email succeeded).
expect(dbMock.delete).toHaveBeenCalledTimes(1);
Expand Down Expand Up @@ -146,19 +154,17 @@ describe("sendDailyDigest — per-user step isolation", () => {

const deleteChain = { where: jest.fn().mockResolvedValue(undefined) };
(dbMock.delete as jest.Mock).mockReturnValue(deleteChain);
mockSendDigestEmail.mockResolvedValue(undefined);
mockSendDigestEmail.mockResolvedValue({ success: true });

const { sendDailyDigest } = await import("@/inngest/functions");

// First run — completes successfully.
const step = makeStep();
// @ts-expect-error
await sendDailyDigest({ step });
await runHandler(sendDailyDigest, step);
expect(mockSendDigestEmail).toHaveBeenCalledTimes(1);

// Simulate Inngest retry: same step shim (memoised results) → per-user step is a no-op.
// @ts-expect-error
await sendDailyDigest({ step });
await runHandler(sendDailyDigest, step);
// sendDigestEmail must NOT be called again.
expect(mockSendDigestEmail).toHaveBeenCalledTimes(1);
});
Expand Down
5 changes: 2 additions & 3 deletions src/__tests__/lib/anonymity.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,9 @@ describe("anonymity: fail closed in production", () => {
afterEach(() => {
if (origEnv === undefined) delete mutableEnv.NODE_ENV;
else mutableEnv.NODE_ENV = origEnv;
if (origSalt === undefined) delete process.env.ANON_HANDLE_SALT;
else process.env.ANON_HANDLE_SALT = origSalt;
if (origSalt === undefined) delete mutableEnv.ANON_HANDLE_SALT;
else mutableEnv.ANON_HANDLE_SALT = origSalt;
});


it("throws when ANON_HANDLE_SALT is missing in production", () => {
delete process.env.ANON_HANDLE_SALT;
Expand Down
46 changes: 46 additions & 0 deletions src/__tests__/lib/pagination.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
import { encodeCursor, decodeCursor } from "@/lib/pagination";

describe("pagination cursor", () => {
it("round-trips a (createdAt, id) key", () => {
const createdAt = new Date("2026-06-01T12:34:56.000Z");
const cursor = encodeCursor(createdAt, 42);
const decoded = decodeCursor(cursor);
expect(decoded).not.toBeNull();
expect(decoded!.id).toBe(42);
expect(decoded!.createdAt.toISOString()).toBe(createdAt.toISOString());
});

it("accepts a Date, ISO string, or epoch ms as input", () => {
const iso = "2026-01-15T08:00:00.000Z";
const fromDate = decodeCursor(encodeCursor(new Date(iso), 1));
const fromString = decodeCursor(encodeCursor(iso, 1));
const fromEpoch = decodeCursor(encodeCursor(new Date(iso).getTime(), 1));
expect(fromDate!.createdAt.toISOString()).toBe(iso);
expect(fromString!.createdAt.toISOString()).toBe(iso);
expect(fromEpoch!.createdAt.toISOString()).toBe(iso);
});

it("produces an opaque base64 string (no raw delimiter leakage)", () => {
const cursor = encodeCursor(new Date("2026-06-01T00:00:00.000Z"), 7);
expect(cursor).toMatch(/^[A-Za-z0-9+/]+=*$/);
expect(cursor).not.toContain("|");
});

it("returns null for null/empty/garbage input instead of throwing", () => {
expect(decodeCursor(null)).toBeNull();
expect(decodeCursor(undefined)).toBeNull();
expect(decodeCursor("")).toBeNull();
expect(decodeCursor("not-base64-!!!")).toBeNull();
expect(decodeCursor(Buffer.from("garbage-no-separator").toString("base64"))).toBeNull();
});

it("rejects a cursor with a non-numeric id", () => {
const bad = Buffer.from("2026-06-01T00:00:00.000Z|abc").toString("base64");
expect(decodeCursor(bad)).toBeNull();
});

it("rejects a cursor with an invalid date", () => {
const bad = Buffer.from("not-a-date|5").toString("base64");
expect(decodeCursor(bad)).toBeNull();
});
});
9 changes: 6 additions & 3 deletions src/app/api/doubts/[id]/accept/route.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,10 @@ let mockReply: {

let mockUpdatedDoubt: { id: number } | null = { id: 1 };

// jest.mock factories are hoisted, and the hoist guard only permits references to
// out-of-scope variables whose names match /^mock/i: hence these names.
const mockInngestSend = jest.fn();
let mockSelectCallCount = 0;

// ── Mocks ─────────────────────────────────────────────────────────────────────
jest.mock("@clerk/nextjs/server", () => ({
Expand Down Expand Up @@ -97,7 +100,7 @@ async function callPost(replyId = 42) {
const { POST } = await import("./route");
return POST(makeRequest(replyId), {
params: Promise.resolve({ id: "1" }),
} as any);
} as unknown as { params: Promise<{ id: string }> });
}

// ── Tests ─────────────────────────────────────────────────────────────────────
Expand Down Expand Up @@ -164,7 +167,7 @@ describe("POST /api/doubts/[id]/accept — idempotency (issue #687)", () => {
it("returns 500 with a generic message and does not leak error details", async () => {
// Make the DB throw to exercise the catch block
const { db } = await import("@/configs/db");
jest.mocked(db.select).mockImplementationOnce(() => {
(db.select as jest.Mock).mockImplementationOnce(() => {
throw new Error("relation \"doubts\" does not exist");
});

Expand All @@ -177,4 +180,4 @@ describe("POST /api/doubts/[id]/accept — idempotency (issue #687)", () => {
expect(JSON.stringify(body)).not.toContain("relation");
expect(JSON.stringify(body)).not.toContain("does not exist");
});
});
});
131 changes: 90 additions & 41 deletions src/app/api/doubts/route.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import {
isNull,
or,
not,
lt,
sql,
SQL,
ilike,
Expand All @@ -38,6 +39,7 @@ import { buildRankOrder } from "@/lib/search";
import { canTeach } from "@/lib/auth/membership-guard";
import { currentUser } from "@clerk/nextjs/server";
import { parsePositiveInt } from "@/lib/utils";
import { decodeCursor, encodeCursor } from "@/lib/pagination";
import { toPublicDoubt } from "@/lib/anonymity";

export async function GET(req: Request) {
Expand Down Expand Up @@ -144,6 +146,19 @@ export async function GET(req: Request) {
: 0;
const page = Math.floor(offset / limit) + 1;

// Cursor-based pagination (issue #319): opt-in via the `cursor` query param
// (present even when empty, for the first page). Cursor mode uses pure
// (createdAt, id) keyset ordering end-to-end, so `nextCursor` is only ever
// issued from an already-cursor-ordered response: never from a pinned-first
// offset page, which could otherwise skip newer unpinned doubts. Only honored
// for the default recency sort with no search. The keyset predicate is applied
// once a cursor token is actually present.
const cursorRequested = searchParams.has("cursor");
const decodedCursor = decodeCursor(searchParams.get("cursor"));
const cursorCreatedAt = decodedCursor?.createdAt ?? null;
const cursorId = decodedCursor?.id ?? null;
const useCursor = cursorRequested && sort === "newest" && !search;

if (tag && tag !== "All") {
const normalizedTag = tag.trim().replace(/\s+/g, " ").toLowerCase();
conditions.push(
Expand All @@ -167,6 +182,20 @@ export async function GET(req: Request) {
Number,
);

// EXISTS subqueries (issue #319): fold per-user like/bookmark status into the
// main query instead of two extra full-scan round-trips. Anonymous viewers
// resolve to false.
const hasLikedSql = (
email
? sql<boolean>`EXISTS (SELECT 1 FROM ${likesTable} WHERE ${likesTable.doubtId} = ${doubtsTable.id} AND ${likesTable.userEmail} = ${email})`
: sql<boolean>`false`
).mapWith(Boolean);
const hasBookmarkedSql = (
email
? sql<boolean>`EXISTS (SELECT 1 FROM ${bookmarksTable} WHERE ${bookmarksTable.doubtId} = ${doubtsTable.id} AND ${bookmarksTable.userEmail} = ${email})`
: sql<boolean>`false`
).mapWith(Boolean);

const [totalCountRow] = await db
.select({ count: count() })
.from(doubtsTable)
Expand All @@ -177,53 +206,64 @@ export async function GET(req: Request) {
.select({
...getTableColumns(doubtsTable),
replyCount: replyCountSql,
hasLiked: hasLikedSql,
hasBookmarked: hasBookmarkedSql,
})
.from(doubtsTable);

const orderByFields: SQL[] = [desc(doubtsTable.isPinned)];

if (search) {
const rankOrder = buildRankOrder(search);
if (rankOrder) orderByFields.push(rankOrder);
}

if (sort === "popular") {
orderByFields.push(desc(doubtsTable.likes));
} else if (sort === "most-replied") {
orderByFields.push(desc(replyCountSql));
}
orderByFields.push(desc(doubtsTable.createdAt));

let doubts = await query
.where(and(...conditions))
.orderBy(...orderByFields)
.limit(limit)
.offset(offset);
// Cursor mode orders purely by (createdAt, id) for a stable keyset; offset mode
// keeps pinned-first plus rank/popular/most-replied ordering.
const orderByFields: SQL[] = useCursor
? [desc(doubtsTable.createdAt), desc(doubtsTable.id)]
: [desc(doubtsTable.isPinned)];

if (email && doubts.length > 0) {
const userLikes = await db
.select({ doubtId: likesTable.doubtId })
.from(likesTable)
.where(eq(likesTable.userEmail, email));
if (!useCursor) {
if (search) {
const rankOrder = buildRankOrder(search);
if (rankOrder) orderByFields.push(rankOrder);
}

const likedIds = new Set(userLikes.map((l) => l.doubtId));
doubts = doubts.map((doubt) => ({
...doubt,
hasLiked: likedIds.has(doubt.id),
}));
if (sort === "popular") {
orderByFields.push(desc(doubtsTable.likes));
} else if (sort === "most-replied") {
orderByFields.push(desc(replyCountSql));
}
orderByFields.push(desc(doubtsTable.createdAt));
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

if (doubts.length > 0 && email) {
const userBookmarks = await db
.select({ doubtId: bookmarksTable.doubtId })
.from(bookmarksTable)
.where(eq(bookmarksTable.userEmail, email));

const bookmarkedIds = new Set(userBookmarks.map((b) => b.doubtId));
doubts = doubts.map((doubt) => ({
...doubt,
hasBookmarked: bookmarkedIds.has(doubt.id),
}));
// Apply the keyset predicate only once a cursor token is present (the first
// cursor-mode page has none and just returns the newest rows in keyset order).
const cursorKeyset =
cursorCreatedAt !== null && cursorId !== null
? or(
lt(doubtsTable.createdAt, cursorCreatedAt),
and(
eq(doubtsTable.createdAt, cursorCreatedAt),
lt(doubtsTable.id, cursorId),
),
)
: undefined;

// Cursor mode over-fetches one sentinel row so `hasMore` is exact (avoids a
// false positive + dead cursor when the remaining rows are exactly `limit`).
let doubts = useCursor
? await query
.where(cursorKeyset ? and(...conditions, cursorKeyset) : and(...conditions))
.orderBy(...orderByFields)
.limit(limit + 1)
: await query
.where(and(...conditions))
.orderBy(...orderByFields)
.limit(limit)
.offset(offset);

// hasLiked / hasBookmarked are now resolved inline via EXISTS subqueries above.

const hasMore = useCursor
? doubts.length > limit
: offset + doubts.length < totalCount;
if (useCursor && hasMore) {
doubts = doubts.slice(0, limit); // drop the sentinel before enrichment
}

if (doubts.length > 0) {
Expand Down Expand Up @@ -256,7 +296,15 @@ export async function GET(req: Request) {
}));
}

const hasMore = offset + doubts.length < totalCount;
// Only emit a cursor from an already-cursor-ordered response (see useCursor),
// so clients never resume keyset paging from a pinned-first offset page.
const nextCursor =
useCursor && hasMore
? encodeCursor(
doubts[doubts.length - 1].createdAt,
doubts[doubts.length - 1].id,
)
: null;

// Strip author identifiers (userEmail), the internal embedding vector and
// soft-delete marker before returning. Only the anonymized handle and a
Expand All @@ -269,6 +317,7 @@ export async function GET(req: Request) {
totalCount,
page,
limit,
nextCursor,
});
} catch (error) {
const { status, body } = buildErrorResponse(error);
Expand Down
16 changes: 16 additions & 0 deletions src/configs/schema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,22 @@ export const doubtsTable = pgTable("doubts", {
table.userEmail,
table.classroomId,
),
// Composite indexes supporting the feed's filter + recency ordering
// (WHERE classroomId = ? [AND type/isSolved] ORDER BY createdAt DESC).
classroomCreatedAtIndex: index("doubts_classroomId_createdAt_idx").on(
table.classroomId,
table.createdAt,
),
classroomTypeIndex: index("doubts_classroomId_type_createdAt_idx").on(
table.classroomId,
table.type,
table.createdAt,
),
classroomSolvedIndex: index("doubts_classroomId_isSolved_createdAt_idx").on(
table.classroomId,
table.isSolved,
table.createdAt,
),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
userEmailFk: foreignKey({
columns: [table.userEmail],
foreignColumns: [usersTable.email],
Expand Down
Loading
Loading