Skip to content

Commit a8a6242

Browse files
BunsDevclaude
andcommitted
fix: address PR review — reason guard, one-shot fetch, index scan
- Reject empty reason strings in setSoftDeleted calls + re-add .catch() for error feedback (both reported-skills and skill-tools sections) - Replace useQuery with ConvexHttpClient.query() on /users public browse page per CLAUDE.md policy - Add by_active_handle compound index on users table to avoid full table scan in queryUsersForPublicList Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent 39c0fa2 commit a8a6242

File tree

4 files changed

+37
-21
lines changed

4 files changed

+37
-21
lines changed

convex/schema.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,8 @@ const users = defineTable({
4040
})
4141
.index("email", ["email"])
4242
.index("phone", ["phone"])
43-
.index("handle", ["handle"]);
43+
.index("handle", ["handle"])
44+
.index("by_active_handle", ["deletedAt", "deactivatedAt", "handle"]);
4445

4546
const publishers = defineTable({
4647
kind: v.union(v.literal("user"), v.literal("org")),

convex/users.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -443,10 +443,12 @@ async function queryUsersForPublicList(
443443
const scanLimit = normalizedSearch
444444
? computeUserSearchScanLimit(args.limit)
445445
: clampInt(args.limit * 6, args.limit, MAX_USER_SEARCH_SCAN);
446-
const scannedUsers = await ctx.db.query("users").order("desc").take(scanLimit);
447-
const activeUsers = scannedUsers.filter(
448-
(user) => !user.deletedAt && !user.deactivatedAt && Boolean(user.handle),
449-
);
446+
const scannedUsers = await ctx.db
447+
.query("users")
448+
.withIndex("by_active_handle", (q) => q.eq("deletedAt", undefined).eq("deactivatedAt", undefined))
449+
.order("desc")
450+
.take(scanLimit);
451+
const activeUsers = scannedUsers.filter((user) => Boolean(user.handle));
450452
const result = buildUserSearchResults(activeUsers, normalizedSearch);
451453
return {
452454
items: result.items.slice(0, args.limit),

src/routes/management.tsx

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -331,12 +331,12 @@ function Management() {
331331
const reason = window.prompt(
332332
skill.softDeletedAt ? "Restore reason:" : "Hide reason:",
333333
);
334-
if (reason === null) return;
334+
if (!reason?.trim()) return;
335335
void setSoftDeleted({
336336
skillId: skill._id,
337337
deleted: !skill.softDeletedAt,
338-
reason,
339-
});
338+
reason: reason.trim(),
339+
}).catch((error) => window.alert(formatMutationError(error)));
340340
}}
341341
>
342342
{skill.softDeletedAt ? "Restore" : "Hide"}
@@ -600,12 +600,12 @@ function Management() {
600600
const reason = window.prompt(
601601
skill.softDeletedAt ? "Restore reason:" : "Hide reason:",
602602
);
603-
if (reason === null) return;
603+
if (!reason?.trim()) return;
604604
void setSoftDeleted({
605605
skillId: skill._id,
606606
deleted: !skill.softDeletedAt,
607-
reason,
608-
});
607+
reason: reason.trim(),
608+
}).catch((error) => window.alert(formatMutationError(error)));
609609
}}
610610
>
611611
{skill.softDeletedAt ? "Restore" : "Hide"}

src/routes/users/index.tsx

Lines changed: 23 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,17 @@
11
import { createFileRoute } from "@tanstack/react-router";
2-
import { useQuery } from "convex/react";
32
import { Search } from "lucide-react";
4-
import { useEffect, useState } from "react";
3+
import { useCallback, useEffect, useState } from "react";
54
import { api } from "../../../convex/_generated/api";
65
import { UserListItem } from "../../components/UserListItem";
6+
import { convexHttp } from "../../convex/client";
77
import type { PublicUser } from "../../lib/publicUser";
88

99
type UserSearchState = {
1010
q?: string;
1111
};
1212

13+
type UsersLoaderResult = { items: PublicUser[]; total: number };
14+
1315
export const Route = createFileRoute("/users/")({
1416
validateSearch: (search): UserSearchState => ({
1517
q: typeof search.q === "string" && search.q.trim() ? search.q.trim() : undefined,
@@ -21,15 +23,26 @@ function UsersIndex() {
2123
const search = Route.useSearch();
2224
const navigate = Route.useNavigate();
2325
const [query, setQuery] = useState(search.q ?? "");
26+
const [result, setResult] = useState<UsersLoaderResult | undefined>(undefined);
27+
const [loading, setLoading] = useState(true);
28+
29+
const fetchUsers = useCallback(async (q?: string) => {
30+
setLoading(true);
31+
try {
32+
const data = await convexHttp.query(api.users.listPublic, {
33+
limit: 48,
34+
search: q,
35+
});
36+
setResult(data as UsersLoaderResult);
37+
} finally {
38+
setLoading(false);
39+
}
40+
}, []);
2441

2542
useEffect(() => {
2643
setQuery(search.q ?? "");
27-
}, [search.q]);
28-
29-
const result = useQuery(api.users.listPublic, {
30-
limit: 48,
31-
search: search.q,
32-
}) as { items: PublicUser[]; total: number } | undefined;
44+
void fetchUsers(search.q);
45+
}, [search.q, fetchUsers]);
3346

3447
const users = result?.items ?? [];
3548

@@ -66,11 +79,11 @@ function UsersIndex() {
6679
<div className="browse-results">
6780
<div className="browse-results-toolbar">
6881
<span className="browse-results-count">
69-
{result === undefined ? "Loading users..." : `${users.length} users`}
82+
{loading ? "Loading users..." : `${users.length} users`}
7083
</span>
7184
</div>
7285

73-
{result === undefined ? (
86+
{loading ? (
7487
<div className="card">
7588
<div className="loading-indicator">Loading users...</div>
7689
</div>

0 commit comments

Comments
 (0)