Skip to content
Open
Show file tree
Hide file tree
Changes from 2 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
134 changes: 130 additions & 4 deletions convex/users.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,12 +120,63 @@ function makeCtx() {
};
}

function makeListCtx(users: Array<Record<string, unknown>>) {
function makeListCtx(
users: Array<Record<string, unknown>>,
options?: {
publishersByHandle?: Record<string, Record<string, unknown>>;
usersById?: Record<string, Record<string, unknown>>;
},
) {
const take = vi.fn(async (n: number) => users.slice(0, n));
const collect = vi.fn(async () => users);
const order = vi.fn(() => ({ take, collect }));
const query = vi.fn(() => ({ order }));
const get = vi.fn();
const publishersByHandle = options?.publishersByHandle ?? {};
const usersById = options?.usersById ?? {};
const query = vi.fn((table: string) => {
if (table === "users") {
return {
order,
withIndex: (
name: string,
cb?: (q: { eq: (field: string, value: string) => unknown }) => unknown,
) => {
if (name !== "handle") throw new Error(`Unexpected users index ${name}`);
let handle = "";
cb?.({
eq: (field: string, value: string) => {
if (field === "handle") handle = value;
return {};
},
});
return {
unique: vi.fn(async () =>
users.find((user) => typeof user.handle === "string" && user.handle === handle) ?? null,
),
};
},
};
}
if (table === "publishers") {
return {
withIndex: (
name: string,
cb?: (q: { eq: (field: string, value: string) => unknown }) => unknown,
) => {
if (name !== "by_handle") throw new Error(`Unexpected publishers index ${name}`);
let handle = "";
cb?.({
eq: (field: string, value: string) => {
if (field === "handle") handle = value;
return {};
},
});
return { unique: vi.fn(async () => publishersByHandle[handle] ?? null) };
},
};
}
throw new Error(`Unexpected table ${table}`);
});
const get = vi.fn(async (id: string) => usersById[id] ?? null);
return {
ctx: { db: { query, get, normalizeId: vi.fn() } } as never,
take,
Expand Down Expand Up @@ -897,6 +948,81 @@ describe("users.list", () => {
expect(result.items[0]?.handle).toBe("alice");
});

it("includes an exact older handle match outside the bounded scan", async () => {
vi.mocked(requireUser).mockResolvedValue({
userId: "users:admin",
user: { _id: "users:admin", role: "admin" },
} as never);
const users = [
...Array.from({ length: 500 }, (_value, index) => ({
_id: `users:recent-${index}`,
_creationTime: 10_000 - index,
handle: `recent-${index}`,
role: "user",
})),
{ _id: "users:older", _creationTime: 1, handle: "alice", role: "user" },
];
const { ctx, take, collect } = makeListCtx(users);
const listHandler = (
list as unknown as { _handler: (ctx: unknown, args: unknown) => Promise<unknown> }
)._handler;

const result = (await listHandler(ctx, { limit: 50, search: "alice" })) as {
items: Array<Record<string, unknown>>;
total: number;
};

expect(take).toHaveBeenCalledWith(500);
expect(collect).not.toHaveBeenCalled();
expect(result.total).toBe(1);
expect(result.items[0]?._id).toBe("users:older");
});

it("includes an exact personal publisher handle match without a full collect", async () => {
vi.mocked(requireUser).mockResolvedValue({
userId: "users:admin",
user: { _id: "users:admin", role: "admin" },
} as never);
const users = [{ _id: "users:1", _creationTime: 2, handle: "alice", role: "user" }];
const { ctx, take, collect } = makeListCtx(users, {
publishersByHandle: {
lmlukef: {
_id: "publishers:lmlukef",
kind: "user",
handle: "lmlukef",
linkedUserId: "users:owner",
},
},
usersById: {
"users:owner": {
_id: "users:owner",
_creationTime: 1,
handle: "luke",
name: "different-gh-login",
displayName: "Luke",
role: "user",
},
},
});
const listHandler = (
list as unknown as { _handler: (ctx: unknown, args: unknown) => Promise<unknown> }
)._handler;

const result = (await listHandler(ctx, { limit: 50, search: "lmLukeF" })) as {
items: Array<Record<string, unknown>>;
total: number;
};

expect(take).toHaveBeenCalledWith(500);
expect(collect).not.toHaveBeenCalled();
expect(result.total).toBe(1);
expect(result.items[0]).toMatchObject({
_id: "users:owner",
handle: "luke",
displayName: "Luke",
});
});

it("clamps large limit and search scan size", async () => {
vi.mocked(requireUser).mockResolvedValue({
userId: "users:admin",
Expand Down Expand Up @@ -1080,7 +1206,7 @@ describe("users.searchInternal", () => {
);
});

it("clamps limit for empty query and uses non-search path", async () => {
it("still caps empty-query listing and uses non-search path", async () => {
const users = Array.from({ length: 400 }, (_value, index) => ({
_id: `users:${index}`,
_creationTime: 1_000 - index,
Expand Down
22 changes: 13 additions & 9 deletions convex/users.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { getAuthUserId } from "@convex-dev/auth/server";
import { v } from "convex/values";
import { internal } from "./_generated/api";
import type { Doc, Id } from "./_generated/dataModel";
import type { ActionCtx, MutationCtx } from "./_generated/server";
import type { ActionCtx, MutationCtx, QueryCtx } from "./_generated/server";
import { internalAction, internalMutation, internalQuery, mutation, query } from "./functions";
import { assertAdmin, assertModerator, requireUser } from "./lib/access";
import { syncGitHubProfile } from "./lib/githubAccount";
Expand Down Expand Up @@ -367,13 +367,7 @@ function computeUserSearchScanLimit(limit: number) {
}

async function queryUsersForAdminList(
ctx: {
db: {
query: (table: "users") => {
order: (order: "desc") => { take: (n: number) => Promise<Doc<"users">[]> };
};
};
},
ctx: Pick<QueryCtx, "db">,
args: { limit: number; search?: string },
) {
const normalizedSearch = normalizeSearchQuery(args.search);
Expand All @@ -386,7 +380,17 @@ async function queryUsersForAdminList(

const scannedUsers = await orderedUsers.take(computeUserSearchScanLimit(args.limit));
const result = buildUserSearchResults(scannedUsers, normalizedSearch);
Comment on lines 379 to 382
Copy link

Copilot AI Apr 1, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Calling orderedUsers.collect() for every non-empty search performs an unbounded full-table scan of the users table. This will scale linearly with user count and may hit Convex execution/response limits as the dataset grows; consider adding a bounded/paginated scan strategy or a dedicated search/index-based approach instead of collecting all users at once.

Copilot uses AI. Check for mistakes.
return { items: result.items.slice(0, args.limit), total: result.total };
const exactHandleUser = await getUserByHandleOrPersonalPublisher(ctx, args.search);
const includesExactHandleUser = exactHandleUser
? result.items.some((user) => user._id === exactHandleUser._id)
: false;
const items = exactHandleUser
? [exactHandleUser, ...result.items.filter((user) => user._id !== exactHandleUser._id)]
: result.items;
return {
items: items.slice(0, args.limit),
total: result.total + (exactHandleUser && !includesExactHandleUser ? 1 : 0),
};
}

function clampInt(value: number, min: number, max: number) {
Expand Down
Loading