diff --git a/apps/web/src/components/projects/interactions/task-detail/add-task-link-modal.tsx b/apps/web/src/components/projects/interactions/task-detail/add-task-link-modal.tsx index bf813df24..99b32d755 100644 --- a/apps/web/src/components/projects/interactions/task-detail/add-task-link-modal.tsx +++ b/apps/web/src/components/projects/interactions/task-detail/add-task-link-modal.tsx @@ -1,12 +1,15 @@ +import { keepPreviousData, useInfiniteQuery } from "@tanstack/react-query"; import { Link2, Search, X } from "lucide-react"; import { useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; +import { useDebouncedCallback } from "@/hooks/use-debounced-callback"; import { type DisplayLinkType, type LinkType, listAllTasks, type Task, } from "@/lib/interaction-api"; +import { createLoadMoreScrollHandler } from "@/lib/scroll-pagination"; export interface AddTaskLinkPayload { sourceTaskId: string; @@ -66,22 +69,26 @@ const DISPLAY_TO_CANONICAL: Partial< duplicates: { linkType: "duplicates", otherTaskIsSource: false }, }; -// The task list API caps page_size at 200, so the search box needs to page -// through the full project rather than fetching a single page - otherwise -// tasks past the first 200 are invisible to the search. -const MAX_TASK_PAGES = 25; - -async function fetchAllProjectTasks(projectId: string): Promise { - const all: Task[] = []; - let cursor: string | undefined; - for (let page = 0; page < MAX_TASK_PAGES; page++) { - const result = await listAllTasks(projectId, { pageSize: 200, cursor }); - all.push(...result.items); - const next = result.next_cursor; - if (!next) break; - cursor = next; +const TASK_PAGE_SIZE = 20; +const SEARCH_DEBOUNCE_MS = 300; +// Hiding the current task can leave a page with few rows (and nothing to +// scroll), so keep loading until a few rows are visible. +const MIN_VISIBLE_TASK_ROWS = 5; + +/** The server matches the title and "#", so a full display id such as + * "PRJ-12" is rewritten to "#12" to keep matching what users see in the UI. */ +function toServerSearch( + query: string, + taskIdPrefix: string | undefined, +): string | undefined { + if (!query) return undefined; + if (taskIdPrefix) { + const m = query.match(/^(.+)-(\d+)$/); + if (m && m[1].toLowerCase() === taskIdPrefix.toLowerCase()) { + return `#${m[2]}`; + } } - return all; + return query; } export function AddTaskLinkModal({ @@ -96,34 +103,85 @@ export function AddTaskLinkModal({ const [selectedLinkType, setSelectedLinkType] = useState("blocks"); const [query, setQuery] = useState(""); - const [tasks, setTasks] = useState([]); - const [loading, setLoading] = useState(false); + const [debouncedQuery, setDebouncedQuery] = useState(""); + const applyQuery = useDebouncedCallback( + setDebouncedQuery, + SEARCH_DEBOUNCE_MS, + ); const searchRef = useRef(null); - // Load tasks once when modal opens useEffect(() => { - if (!open) return; - setLoading(true); - fetchAllProjectTasks(projectId) - .then(setTasks) - .catch(() => setTasks([])) - .finally(() => setLoading(false)); - setTimeout(() => searchRef.current?.focus(), 50); - }, [open, projectId]); - - const filteredTasks = useMemo(() => { - const q = query.trim().toLowerCase(); - return tasks.filter((t) => { - if (t.id === currentTaskId) return false; - if (!q) return true; - const prefix = taskIdPrefix - ? `${taskIdPrefix}-${t.task_number}` - : String(t.task_number); - return ( - t.title.toLowerCase().includes(q) || prefix.toLowerCase().includes(q) - ); - }); - }, [tasks, query, currentTaskId, taskIdPrefix]); + if (!open) { + setQuery(""); + setDebouncedQuery(""); + // Supersede any pending debounce so it can't restore the old term. + applyQuery(""); + return; + } + const timer = setTimeout(() => searchRef.current?.focus(), 50); + return () => clearTimeout(timer); + }, [open, applyQuery]); + + // Cursor-paginated, server-side search (matches title and "#"), + // so every task in the project is reachable without loading them all. + const { + data, + isLoading: loading, + isError, + isFetchNextPageError, + isPlaceholderData, + isFetchingNextPage, + hasNextPage, + fetchNextPage, + } = useInfiniteQuery({ + queryKey: ["projects", projectId, "tasks", "link-picker", debouncedQuery], + queryFn: ({ pageParam }: { pageParam: string | undefined }) => + listAllTasks(projectId, { + search: toServerSearch(debouncedQuery, taskIdPrefix), + + pageSize: TASK_PAGE_SIZE, + cursor: pageParam, + }), + initialPageParam: undefined as string | undefined, + getNextPageParam: (lastPage) => lastPage.next_cursor ?? undefined, + enabled: open && !!projectId, + placeholderData: keepPreviousData, + }); + + const filteredTasks = useMemo( + () => + (data?.pages.flatMap((page) => page.items) ?? []).filter( + (t) => t.id !== currentTaskId, + ), + [data, currentTaskId], + ); + + useEffect(() => { + if ( + hasNextPage && + !isFetchingNextPage && + !isFetchNextPageError && + !isPlaceholderData && + !loading && + filteredTasks.length < MIN_VISIBLE_TASK_ROWS + ) { + void fetchNextPage(); + } + }, [ + hasNextPage, + isFetchingNextPage, + isFetchNextPageError, + isPlaceholderData, + loading, + filteredTasks.length, + fetchNextPage, + ]); + + const handleListScroll = createLoadMoreScrollHandler({ + hasMore: !!hasNextPage, + isLoadingMore: isFetchingNextPage, + onLoadMore: () => void fetchNextPage(), + }); function handleSelect(task: Task) { const canonical = DISPLAY_TO_CANONICAL[selectedLinkType]; @@ -200,7 +258,10 @@ export function AddTaskLinkModal({ ref={searchRef} type="text" value={query} - onChange={(e) => setQuery(e.target.value)} + onChange={(e) => { + setQuery(e.target.value); + applyQuery(e.target.value.trim()); + }} placeholder={t("taskDetail.addTaskLinkModal.searchPlaceholder")} className="w-full pl-9 pr-3 py-2.5 rounded-lg border border-border/30 bg-muted/20 text-sm placeholder:text-muted-foreground/50 focus:outline-none focus:ring-2 focus:ring-primary/20 focus:border-primary/40 transition-all duration-150" /> @@ -208,13 +269,21 @@ export function AddTaskLinkModal({ {/* Task list */} -
+
{loading && (
{t("taskDetail.addTaskLinkModal.loadingTasks")}
)} - {!loading && filteredTasks.length === 0 && ( + {!loading && isError && filteredTasks.length === 0 && ( +
+ {t("taskDetail.addTaskLinkModal.loadError")} +
+ )} + {!loading && !isError && filteredTasks.length === 0 && (
{t("taskDetail.addTaskLinkModal.noTasksFound")}
@@ -240,6 +309,11 @@ export function AddTaskLinkModal({ ); })} + {isFetchingNextPage && ( +
+ {t("taskDetail.addTaskLinkModal.loadingTasks")} +
+ )}
diff --git a/apps/web/src/i18n/locales/en/projects.json b/apps/web/src/i18n/locales/en/projects.json index 289c48d69..c8d40cc36 100644 --- a/apps/web/src/i18n/locales/en/projects.json +++ b/apps/web/src/i18n/locales/en/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "Search tasks by title or number...", "loadingTasks": "Loading tasks…", "noTasksFound": "No tasks found", + "loadError": "Couldn't load tasks. Please try again.", "linkTypes": { "blocks": { "label": "Blocks", diff --git a/apps/web/src/i18n/locales/es/projects.json b/apps/web/src/i18n/locales/es/projects.json index 19283b0bd..ecf8ca22d 100644 --- a/apps/web/src/i18n/locales/es/projects.json +++ b/apps/web/src/i18n/locales/es/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "Buscar tareas por título o número...", "loadingTasks": "Cargando tareas…", "noTasksFound": "No se encontraron tareas", + "loadError": "No se pudieron cargar las tareas. Inténtalo de nuevo.", "linkTypes": { "blocks": { "label": "Bloquea", diff --git a/apps/web/src/i18n/locales/fr/projects.json b/apps/web/src/i18n/locales/fr/projects.json index caf82ee00..fa5aa19df 100644 --- a/apps/web/src/i18n/locales/fr/projects.json +++ b/apps/web/src/i18n/locales/fr/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "Rechercher des tâches par titre ou numéro...", "loadingTasks": "Chargement des tâches…", "noTasksFound": "Aucune tâche trouvée", + "loadError": "Impossible de charger les tâches. Veuillez réessayer.", "linkTypes": { "blocks": { "label": "Bloque", diff --git a/apps/web/src/i18n/locales/ja/projects.json b/apps/web/src/i18n/locales/ja/projects.json index 551c7aa8e..aec50a9e4 100644 --- a/apps/web/src/i18n/locales/ja/projects.json +++ b/apps/web/src/i18n/locales/ja/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "タイトルまたは番号でタスクを検索...", "loadingTasks": "タスクを読み込み中…", "noTasksFound": "タスクが見つかりません", + "loadError": "タスクを読み込めませんでした。もう一度お試しください。", "linkTypes": { "blocks": { "label": "ブロックする", diff --git a/apps/web/src/i18n/locales/ko/projects.json b/apps/web/src/i18n/locales/ko/projects.json index d4b393126..3ed1029bd 100644 --- a/apps/web/src/i18n/locales/ko/projects.json +++ b/apps/web/src/i18n/locales/ko/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "제목 또는 번호로 작업 검색...", "loadingTasks": "작업을 불러오는 중…", "noTasksFound": "작업을 찾을 수 없습니다", + "loadError": "작업을 불러오지 못했습니다. 다시 시도해 주세요.", "linkTypes": { "blocks": { "label": "차단함", diff --git a/apps/web/src/i18n/locales/pt-BR/projects.json b/apps/web/src/i18n/locales/pt-BR/projects.json index 3584328af..195dd091e 100644 --- a/apps/web/src/i18n/locales/pt-BR/projects.json +++ b/apps/web/src/i18n/locales/pt-BR/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "Buscar tarefas por título ou número...", "loadingTasks": "Carregando tarefas…", "noTasksFound": "Nenhuma tarefa encontrada", + "loadError": "Não foi possível carregar as tarefas. Tente novamente.", "linkTypes": { "blocks": { "label": "Bloqueia", diff --git a/apps/web/src/i18n/locales/ru/projects.json b/apps/web/src/i18n/locales/ru/projects.json index cf6c4712c..514b4fdd9 100644 --- a/apps/web/src/i18n/locales/ru/projects.json +++ b/apps/web/src/i18n/locales/ru/projects.json @@ -1541,6 +1541,7 @@ "searchPlaceholder": "Поиск задач по названию или номеру...", "loadingTasks": "Загрузка задач…", "noTasksFound": "Задачи не найдены", + "loadError": "Не удалось загрузить задачи. Повторите попытку.", "linkTypes": { "blocks": { "label": "Блокирует", diff --git a/apps/web/src/i18n/locales/vi/projects.json b/apps/web/src/i18n/locales/vi/projects.json index 0297c4770..379a2df44 100644 --- a/apps/web/src/i18n/locales/vi/projects.json +++ b/apps/web/src/i18n/locales/vi/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "Tìm nhiệm vụ theo tiêu đề hoặc số...", "loadingTasks": "Đang tải nhiệm vụ…", "noTasksFound": "Không tìm thấy nhiệm vụ nào", + "loadError": "Không thể tải công việc. Vui lòng thử lại.", "linkTypes": { "blocks": { "label": "Chặn", diff --git a/apps/web/src/i18n/locales/zh-CN/projects.json b/apps/web/src/i18n/locales/zh-CN/projects.json index 629ce4080..99eab1edc 100644 --- a/apps/web/src/i18n/locales/zh-CN/projects.json +++ b/apps/web/src/i18n/locales/zh-CN/projects.json @@ -1523,6 +1523,7 @@ "searchPlaceholder": "按标题或编号搜索任务...", "loadingTasks": "正在加载任务…", "noTasksFound": "未找到任务", + "loadError": "无法加载任务。请重试。", "linkTypes": { "blocks": { "label": "阻塞", diff --git a/apps/web/src/lib/admin-api.test.ts b/apps/web/src/lib/admin-api.test.ts index c43aa41e4..fd5666b5d 100644 --- a/apps/web/src/lib/admin-api.test.ts +++ b/apps/web/src/lib/admin-api.test.ts @@ -32,6 +32,7 @@ import { getGlobalRoles, getMyGlobalPermissions, getUsers, + getUsersByCursor, globalRolesQueryOptions, myPermissionsQueryOptions, resetUserPassword, @@ -39,6 +40,7 @@ import { type User, updateGlobalRole, updateUser, + usersInfiniteQueryOptions, usersQueryOptions, } from "./admin-api"; @@ -213,6 +215,34 @@ describe("admin-api", () => { }); }); + it("sends cursor, search and page_size to getUsersByCursor only when set", async () => { + const page = { items: [], page_size: 20, next_cursor: "c2" }; + mockGet.mockResolvedValue({ + data: { data: page, error_code: null, message: "ok" }, + }); + + await expect(getUsersByCursor(null)).resolves.toEqual(page); + expect(mockGet).toHaveBeenLastCalledWith("/admin/users/cursor", { + params: { page_size: 20 }, + }); + + await getUsersByCursor("c1", 5, { search: "alice" }); + expect(mockGet).toHaveBeenLastCalledWith("/admin/users/cursor", { + params: { page_size: 5, cursor: "c1", search: "alice" }, + }); + }); + + it("usersInfiniteQueryOptions follows next_cursor and stops when null", () => { + const opts = usersInfiniteQueryOptions("al"); + expect(opts.queryKey).toEqual(["admin", "users", "cursor", "al"]); + expect(opts.initialPageParam).toBeNull(); + const next = opts.getNextPageParam as (p: unknown) => unknown; + expect(next({ items: [], page_size: 20, next_cursor: "abc" })).toBe("abc"); + expect( + next({ items: [], page_size: 20, next_cursor: null }), + ).toBeUndefined(); + }); + it("sends search and role to getUsers only when set", async () => { mockGet.mockResolvedValue({ data: { diff --git a/apps/web/src/lib/admin-api.ts b/apps/web/src/lib/admin-api.ts index 66d4c9291..a3b1f7d84 100644 --- a/apps/web/src/lib/admin-api.ts +++ b/apps/web/src/lib/admin-api.ts @@ -129,6 +129,28 @@ export async function getUsers( return data.data; } +export interface CursorUsersResponse { + items: User[]; + page_size: number; + /** Opaque token for the next page; null on the last page. */ + next_cursor: string | null; +} + +export async function getUsersByCursor( + cursor: string | null, + pageSize = 20, + filter: UsersFilter = {}, +): Promise { + const params: Record = { page_size: pageSize }; + if (cursor) params.cursor = cursor; + if (filter.search) params.search = filter.search; + if (filter.role) params.role = filter.role; + const { data } = await apiClient.instance.get< + SuccessEnvelope + >("/admin/users/cursor", { params }); + return data.data; +} + /** Creates a user with the default USER role. To give them another role, call * {@link assignUserGlobalRole} afterwards — assigning a role needs * `global_roles.assign`, so the server no longer accepts `role` here. */ @@ -203,19 +225,15 @@ export function usersQueryOptions( export const ADMIN_USERS_PAGE_SIZE = 20; -/** Infinite-query version of the user list — backs pickers that need to - * page through every user (e.g. the "add team member" dialog), since the - * backend caps page_size at 100 and there's no server-side search to - * narrow the result set. Pages accumulate as the caller scrolls, same - * pattern as the epic picker's infinite query. */ -export const usersInfiniteQueryOptions = () => +/** Cursor-paginated infinite query over the user list — backs pickers that + * page through every user (e.g. the "add team member" dialog). `search` is + * sent to the server (it matches username, full name and email) so users + * beyond the first page are still findable. */ +export const usersInfiniteQueryOptions = (search = "") => infiniteQueryOptions({ - queryKey: ["admin", "users", "all"], - queryFn: ({ pageParam }: { pageParam: number }) => - getUsers(pageParam, ADMIN_USERS_PAGE_SIZE), - initialPageParam: 1, - getNextPageParam: (lastPage) => - lastPage.page * lastPage.page_size < lastPage.total - ? lastPage.page + 1 - : undefined, + queryKey: ["admin", "users", "cursor", search], + queryFn: ({ pageParam }: { pageParam: string | null }) => + getUsersByCursor(pageParam, ADMIN_USERS_PAGE_SIZE, { search }), + initialPageParam: null as string | null, + getNextPageParam: (lastPage) => lastPage.next_cursor ?? undefined, }); diff --git a/apps/web/src/routes/_authenticated/projects/$projectId/team/index.tsx b/apps/web/src/routes/_authenticated/projects/$projectId/team/index.tsx index c4a98be9c..fa253b2fd 100644 --- a/apps/web/src/routes/_authenticated/projects/$projectId/team/index.tsx +++ b/apps/web/src/routes/_authenticated/projects/$projectId/team/index.tsx @@ -1,4 +1,5 @@ import { + keepPreviousData, useInfiniteQuery, useMutation, useQuery, @@ -19,7 +20,7 @@ import { UserRound, Users, } from "lucide-react"; -import { useMemo, useRef, useState } from "react"; +import { useEffect, useMemo, useRef, useState } from "react"; import { useTranslation } from "react-i18next"; import { MembersFilters } from "@/components/projects/team/MembersFilters"; import { HighlightMatch } from "@/components/shared/highlight-match"; @@ -55,6 +56,7 @@ import { } from "@/components/ui/select"; import { Skeleton } from "@/components/ui/skeleton"; import { Textarea } from "@/components/ui/textarea"; +import { useDebouncedCallback } from "@/hooks/use-debounced-callback"; import { usePermissions } from "@/hooks/use-permissions"; import { useProjectPermissions } from "@/hooks/use-project-permissions"; import { type User, usersInfiniteQueryOptions } from "@/lib/admin-api"; @@ -98,6 +100,8 @@ export const Route = createFileRoute( component: TeamPage, }); +const MIN_VISIBLE_USER_ROWS = 5; + function getInitials(name: string): string { return name .split(" ") @@ -170,6 +174,8 @@ function AddMemberDialog({ const [selectedAgent, setSelectedAgent] = useState(null); const [selectedRoleId, setSelectedRoleId] = useState(""); const [userSearch, setUserSearch] = useState(""); + const [debouncedUserSearch, setDebouncedUserSearch] = useState(""); + const applyUserSearch = useDebouncedCallback(setDebouncedUserSearch, 300); const [description, setDescription] = useState(""); const [error, setError] = useState(null); const searchRef = useRef(null); @@ -187,12 +193,15 @@ function AddMemberDialog({ const { data: usersPages, isLoading: isLoadingUsers, + isFetchNextPageError, + isPlaceholderData, fetchNextPage, hasNextPage, isFetchingNextPage, } = useInfiniteQuery({ - ...usersInfiniteQueryOptions(), + ...usersInfiniteQueryOptions(debouncedUserSearch), enabled: open && canReadUsers, + placeholderData: keepPreviousData, }); const usersData = useMemo( @@ -206,18 +215,34 @@ function AddMemberDialog({ onLoadMore: () => void fetchNextPage(), }); - const filteredUsers = useMemo(() => { - const items: User[] = usersData; - const q = userSearch.toLowerCase(); - return items - .filter((u) => !existingMemberIds.has(u.id)) - .filter( - (u) => - !q || - u.username.toLowerCase().includes(q) || - (u.full_name ?? "").toLowerCase().includes(q), - ); - }, [usersData, existingMemberIds, userSearch]); + // Search is done server-side; existing members are only hidden here. + const filteredUsers = useMemo( + () => usersData.filter((u) => !existingMemberIds.has(u.id)), + [usersData, existingMemberIds], + ); + + // Hiding existing members can leave a page with few or no rows (and then + // nothing to scroll), so keep loading until a few rows are visible. + useEffect(() => { + if ( + hasNextPage && + !isFetchingNextPage && + !isFetchNextPageError && + !isPlaceholderData && + !isLoadingUsers && + filteredUsers.length < MIN_VISIBLE_USER_ROWS + ) { + void fetchNextPage(); + } + }, [ + hasNextPage, + isFetchingNextPage, + isFetchNextPageError, + isPlaceholderData, + isLoadingUsers, + filteredUsers.length, + fetchNextPage, + ]); const addMutation = useMutation({ mutationFn: () => { @@ -266,6 +291,9 @@ function AddMemberDialog({ setSelectedAgent(null); setSelectedRoleId(""); setUserSearch(""); + setDebouncedUserSearch(""); + // Supersede any pending debounce so it can't restore the old term. + applyUserSearch(""); setDescription(""); setError(null); onOpenChange(false); @@ -448,7 +476,10 @@ function AddMemberDialog({ className="flex-1 bg-transparent text-sm outline-none placeholder:text-muted-foreground" placeholder={t("team.addMemberDialog.searchPlaceholder")} value={userSearch} - onChange={(e) => setUserSearch(e.target.value)} + onChange={(e) => { + setUserSearch(e.target.value); + applyUserSearch(e.target.value.trim()); + }} autoFocus /> diff --git a/services/api/internal/apierr/codes.go b/services/api/internal/apierr/codes.go index 0ddae44fd..d3fe7eca9 100644 --- a/services/api/internal/apierr/codes.go +++ b/services/api/internal/apierr/codes.go @@ -19,6 +19,8 @@ const ( // CodeUserNotFound represents a user that was not found. CodeUserNotFound Code = "USER_NOT_FOUND" + // CodeUserInvalidCursor indicates a client-supplied pagination cursor failed to decode. + CodeUserInvalidCursor Code = "USER_INVALID_CURSOR" // CodeUsernameTaken represents a username that is already taken. CodeUsernameTaken Code = "USER_USERNAME_TAKEN" // CodeEmailTaken represents an email that is already taken. diff --git a/services/api/internal/domain/user/cursor.go b/services/api/internal/domain/user/cursor.go new file mode 100644 index 000000000..0b16570bb --- /dev/null +++ b/services/api/internal/domain/user/cursor.go @@ -0,0 +1,40 @@ +package userdom + +import ( + "encoding/base64" + "encoding/json" + "fmt" + + "github.com/google/uuid" +) + +// Cursor identifies the last user on a page of the name-sorted user list. +// Only the ID is stored: the repository re-derives the sort keys from that +// row, so the cursor stays valid even if the keys were computed differently +// in Go and SQL, and stays opaque to clients. +type Cursor struct { + ID string `json:"id"` +} + +// EncodeCursor builds an opaque base64 cursor from the last user on a page. +func EncodeCursor(u *User) string { + b, _ := json.Marshal(Cursor{ID: u.ID.String()}) + return base64.URLEncoding.EncodeToString(b) +} + +// DecodeCursor parses a token produced by EncodeCursor. Any failure wraps +// ErrInvalidCursor. +func DecodeCursor(s string) (*Cursor, error) { + b, err := base64.URLEncoding.DecodeString(s) + if err != nil { + return nil, fmt.Errorf("%w: base64: %v", ErrInvalidCursor, err) + } + var c Cursor + if err := json.Unmarshal(b, &c); err != nil { + return nil, fmt.Errorf("%w: json: %v", ErrInvalidCursor, err) + } + if _, err := uuid.Parse(c.ID); err != nil { + return nil, fmt.Errorf("%w: id: %v", ErrInvalidCursor, err) + } + return &c, nil +} diff --git a/services/api/internal/domain/user/cursor_test.go b/services/api/internal/domain/user/cursor_test.go new file mode 100644 index 000000000..f374bc404 --- /dev/null +++ b/services/api/internal/domain/user/cursor_test.go @@ -0,0 +1,27 @@ +package userdom_test + +import ( + "encoding/base64" + "errors" + "testing" + + "github.com/google/uuid" + + userdom "github.com/Paca-AI/api/internal/domain/user" +) + +func TestCursorRoundTrip(t *testing.T) { + u := &userdom.User{ID: uuid.New()} + c, err := userdom.DecodeCursor(userdom.EncodeCursor(u)) + if err != nil || c.ID != u.ID.String() { + t.Fatalf("got %+v, %v", c, err) + } +} + +func TestDecodeCursorInvalid(t *testing.T) { + for _, in := range []string{"!!!", base64.URLEncoding.EncodeToString([]byte("nope")), base64.URLEncoding.EncodeToString([]byte(`{"id":"x"}`))} { + if _, err := userdom.DecodeCursor(in); !errors.Is(err, userdom.ErrInvalidCursor) { + t.Errorf("%q: err = %v, want ErrInvalidCursor", in, err) + } + } +} diff --git a/services/api/internal/domain/user/errors.go b/services/api/internal/domain/user/errors.go index 3ec98f816..21ad33551 100644 --- a/services/api/internal/domain/user/errors.go +++ b/services/api/internal/domain/user/errors.go @@ -13,4 +13,7 @@ var ( // "already used" alike — deliberately not distinguished for callers, so // a caller can't use response differences to enumerate tokens. ErrPasswordSetTokenInvalid = errors.New("user: password set token invalid or expired") + // ErrInvalidCursor is returned when a client-supplied pagination cursor + // fails to decode. + ErrInvalidCursor = errors.New("user: invalid pagination cursor") ) diff --git a/services/api/internal/domain/user/repository.go b/services/api/internal/domain/user/repository.go index becf70443..8c7b69691 100644 --- a/services/api/internal/domain/user/repository.go +++ b/services/api/internal/domain/user/repository.go @@ -29,6 +29,10 @@ type Repository interface { // List returns a page of users matching filter, sorted by name, and the // total count of users matching filter (not just this page). List(ctx context.Context, offset, limit int, filter ListFilter) ([]*User, int64, error) + // ListAfter returns up to limit users matching filter, sorted by name + // (the same order as List), starting after the user identified by + // cursorAfter (nil for the first page), and whether more users follow. + ListAfter(ctx context.Context, limit int, cursorAfter *string, filter ListFilter) ([]*User, bool, error) // CountUsers returns the total count of active, non-system users — the // same count List returns as its total, without paginating any rows. // Used by the home page's workspace stats widget for team-member count. diff --git a/services/api/internal/domain/user/service.go b/services/api/internal/domain/user/service.go index 5aa0b13c6..e83c81d75 100644 --- a/services/api/internal/domain/user/service.go +++ b/services/api/internal/domain/user/service.go @@ -49,6 +49,9 @@ type Service interface { // List returns a page of users matching filter and the total count of // matches. List(ctx context.Context, page, pageSize int, filter ListFilter) ([]*User, int64, error) + // ListAfter returns a keyset-paginated page of users matching filter, + // sorted by name, and whether more users follow. + ListAfter(ctx context.Context, limit int, cursorAfter *string, filter ListFilter) ([]*User, bool, error) // CountUsers returns the total count of users without paginating rows. CountUsers(ctx context.Context) (int64, error) // CountUsersMustChangePassword returns the total count of users who must diff --git a/services/api/internal/repository/postgres/user_repository.go b/services/api/internal/repository/postgres/user_repository.go index c19899775..f84b14aee 100644 --- a/services/api/internal/repository/postgres/user_repository.go +++ b/services/api/internal/repository/postgres/user_repository.go @@ -64,13 +64,12 @@ func NewUserRepository(db *sqlx.DB) *UserRepository { return &UserRepository{db: db} } -// List returns a page of non-deleted, non-system users matching filter, -// ordered by name, plus the count of matches across all pages. Every -// whitespace-separated search word must appear (case-insensitively) in the -// username, full name or email; Role is an exact global role name. The -// built-in agent bot account is excluded because it is an internal system -// identity, not a real user. -func (r *UserRepository) List(ctx context.Context, offset, limit int, filter userdom.ListFilter) ([]*userdom.User, int64, error) { +// userNameSortKey is the primary sort key of every user listing. +const userNameSortKey = `LOWER(COALESCE(NULLIF(users.full_name, ''), users.username))` + +// userListWhere builds the WHERE clause (and its args) shared by List and +// ListAfter. +func userListWhere(filter userdom.ListFilter) (string, []any) { where := `users.deleted_at IS NULL AND users.username != '_paca_agent_bot'` var args []any if filter.Role != "" { @@ -82,6 +81,17 @@ func (r *UserRepository) List(ctx context.Context, offset, limit int, filter use n := len(args) where += fmt.Sprintf(` AND (LOWER(users.username) LIKE LOWER($%[1]d) ESCAPE '\' OR LOWER(users.full_name) LIKE LOWER($%[1]d) ESCAPE '\' OR LOWER(COALESCE(users.email, '')) LIKE LOWER($%[1]d) ESCAPE '\')`, n) } + return where, args +} + +// List returns a page of non-deleted, non-system users matching filter, +// ordered by name, plus the count of matches across all pages. Every +// whitespace-separated search word must appear (case-insensitively) in the +// username, full name or email; Role is an exact global role name. The +// built-in agent bot account is excluded because it is an internal system +// identity, not a real user. +func (r *UserRepository) List(ctx context.Context, offset, limit int, filter userdom.ListFilter) ([]*userdom.User, int64, error) { + where, args := userListWhere(filter) var total int64 if err := r.db.GetContext(ctx, &total, `SELECT COUNT(*) FROM users `+userReadJoin+` WHERE `+where, args...); err != nil { @@ -95,7 +105,7 @@ func (r *UserRepository) List(ctx context.Context, offset, limit int, filter use FROM users `+userReadJoin+` WHERE `+where+fmt.Sprintf(` - ORDER BY LOWER(COALESCE(NULLIF(users.full_name, ''), users.username)), LOWER(users.username), users.id + ORDER BY `+userNameSortKey+`, LOWER(users.username), users.id LIMIT $%d OFFSET $%d`, len(args)-1, len(args)), args...); err != nil { return nil, 0, fmt.Errorf("user repo: list: %w", err) } @@ -107,6 +117,56 @@ func (r *UserRepository) List(ctx context.Context, offset, limit int, filter use return users, total, nil } +// ListAfter is the keyset-paginated counterpart of List: same filter and +// ordering, but it resumes after the user named by cursorAfter instead of +// using an offset, so pages stay stable while users are added or removed. +func (r *UserRepository) ListAfter(ctx context.Context, limit int, cursorAfter *string, filter userdom.ListFilter) ([]*userdom.User, bool, error) { + if limit <= 0 { + limit = 20 + } + where, args := userListWhere(filter) + if cursorAfter != nil { + cur, err := userdom.DecodeCursor(*cursorAfter) + if err != nil { + return nil, false, err + } + // Soft-deleted rows still count: a user removed between page loads + // must not invalidate the cursor. An id that never existed does. + var exists bool + if err := r.db.GetContext(ctx, &exists, `SELECT EXISTS (SELECT 1 FROM users WHERE id = $1)`, cur.ID); err != nil { + return nil, false, fmt.Errorf("user repo: list after: cursor lookup: %w", err) + } + if !exists { + return nil, false, fmt.Errorf("%w: unknown user", userdom.ErrInvalidCursor) + } + args = append(args, cur.ID) + where += fmt.Sprintf(` AND (`+userNameSortKey+`, LOWER(users.username), users.id) > ( + SELECT `+userNameSortKey+`, LOWER(users.username), users.id FROM users WHERE users.id = $%d)`, len(args)) + } + args = append(args, limit+1) + + var rows []userReadRow + if err := r.db.SelectContext(ctx, &rows, ` + SELECT `+userReadCols+` + FROM users + `+userReadJoin+` + WHERE `+where+fmt.Sprintf(` + ORDER BY `+userNameSortKey+`, LOWER(users.username), users.id + LIMIT $%d`, len(args)), args...); err != nil { + return nil, false, fmt.Errorf("user repo: list after: %w", err) + } + + hasMore := len(rows) > limit + if hasMore { + rows = rows[:limit] + } + users := make([]*userdom.User, 0, len(rows)) + for i := range rows { + users = append(users, rowToEntity(&rows[i])) + } + return users, hasMore, nil +} + // escapeLike escapes the LIKE wildcards in s so it matches literally. func escapeLike(s string) string { return strings.NewReplacer(`\`, `\\`, `%`, `\%`, `_`, `\_`).Replace(s) diff --git a/services/api/internal/service/auth/auth_service_test.go b/services/api/internal/service/auth/auth_service_test.go index f94e887ec..c946fb9e6 100644 --- a/services/api/internal/service/auth/auth_service_test.go +++ b/services/api/internal/service/auth/auth_service_test.go @@ -46,6 +46,9 @@ func (r *stubUserRepo) List(_ context.Context, _, _ int, _ userdom.ListFilter) ( return nil, 0, nil } func (r *stubUserRepo) CountUsers(_ context.Context) (int64, error) { return 0, nil } +func (r *stubUserRepo) ListAfter(context.Context, int, *string, userdom.ListFilter) ([]*userdom.User, bool, error) { + return nil, false, nil +} func (r *stubUserRepo) CountUsersMustChangePassword(_ context.Context) (int64, error) { return 0, nil } diff --git a/services/api/internal/service/user/user_service.go b/services/api/internal/service/user/user_service.go index 249a77159..5cbc91f63 100644 --- a/services/api/internal/service/user/user_service.go +++ b/services/api/internal/service/user/user_service.go @@ -115,6 +115,14 @@ func (s *Service) List(ctx context.Context, page, pageSize int, filter userdom.L return s.repo.List(ctx, offset, pageSize, filter) } +// ListAfter returns a cursor-paginated page of users matching filter. +func (s *Service) ListAfter(ctx context.Context, limit int, cursorAfter *string, filter userdom.ListFilter) ([]*userdom.User, bool, error) { + if limit < 1 || limit > 100 { + limit = 20 + } + return s.repo.ListAfter(ctx, limit, cursorAfter, filter) +} + // CountUsers returns the total count of users without paginating rows. func (s *Service) CountUsers(ctx context.Context) (int64, error) { return s.repo.CountUsers(ctx) diff --git a/services/api/internal/service/user/user_service_test.go b/services/api/internal/service/user/user_service_test.go index 30be398b5..971025add 100644 --- a/services/api/internal/service/user/user_service_test.go +++ b/services/api/internal/service/user/user_service_test.go @@ -121,6 +121,9 @@ func (r *stubRepo) List(_ context.Context, _, _ int, _ userdom.ListFilter) ([]*u return nil, 0, nil } func (r *stubRepo) CountUsers(_ context.Context) (int64, error) { return 0, nil } +func (r *stubRepo) ListAfter(context.Context, int, *string, userdom.ListFilter) ([]*userdom.User, bool, error) { + return nil, false, nil +} func (r *stubRepo) CountUsersMustChangePassword(_ context.Context) (int64, error) { return 0, nil } diff --git a/services/api/internal/transport/http/dto/user_dto.go b/services/api/internal/transport/http/dto/user_dto.go index 93183e99b..81d55f74f 100644 --- a/services/api/internal/transport/http/dto/user_dto.go +++ b/services/api/internal/transport/http/dto/user_dto.go @@ -73,6 +73,14 @@ type UserResponse struct { CreatedAt time.Time `json:"created_at"` } +// CursorUsersResponse is a keyset-paginated page of users. NextCursor is nil +// when this is the last page. +type CursorUsersResponse struct { + Items []UserResponse `json:"items"` + PageSize int `json:"page_size"` + NextCursor *string `json:"next_cursor"` +} + // PagedUsersResponse wraps a list of users with pagination metadata. type PagedUsersResponse struct { Items []UserResponse `json:"items"` diff --git a/services/api/internal/transport/http/handler/user_handler.go b/services/api/internal/transport/http/handler/user_handler.go index 8c71cac2d..5be64af9e 100644 --- a/services/api/internal/transport/http/handler/user_handler.go +++ b/services/api/internal/transport/http/handler/user_handler.go @@ -207,6 +207,51 @@ func (h *UserHandler) ListUsers(w http.ResponseWriter, r *http.Request) { }) } +// ListUsersByCursor handles GET /admin/users/cursor — a keyset-paginated +// variant of ListUsers for infinite-scroll pickers. +// +// Supported query params (all optional): +// - page_size=<1-100> defaults to 20 +// - cursor= from the previous page's next_cursor; omit for the first page +// - search= same matching as ListUsers +// - role= same matching as ListUsers +func (h *UserHandler) ListUsersByCursor(w http.ResponseWriter, r *http.Request) { + pageSize, err := parsePageSize(r, 20, 100) + if err != nil { + presenter.Error(w, r, err) + return + } + var cursor *string + if raw := r.URL.Query().Get("cursor"); raw != "" { + cursor = &raw + } + + users, hasMore, err := h.svc.ListAfter(r.Context(), pageSize, cursor, domainuser.ListFilter{ + Search: strings.TrimSpace(r.URL.Query().Get("search")), + Role: strings.TrimSpace(r.URL.Query().Get("role")), + }) + if err != nil { + presenter.Error(w, r, err) + return + } + + items := make([]dto.UserResponse, 0, len(users)) + for _, u := range users { + items = append(items, h.toUserResponse(r.Context(), u)) + } + var nextCursor *string + if hasMore && len(users) > 0 { + s := domainuser.EncodeCursor(users[len(users)-1]) + nextCursor = &s + } + + presenter.OK(w, r, dto.CursorUsersResponse{ + Items: items, + PageSize: pageSize, + NextCursor: nextCursor, + }) +} + // GetUserByID handles GET /admin/users/:userId. func (h *UserHandler) GetUserByID(w http.ResponseWriter, r *http.Request) { id, err := uuid.Parse(chi.URLParam(r, "userId")) diff --git a/services/api/internal/transport/http/handler/user_handler_test.go b/services/api/internal/transport/http/handler/user_handler_test.go index 684bd449e..1ded08180 100644 --- a/services/api/internal/transport/http/handler/user_handler_test.go +++ b/services/api/internal/transport/http/handler/user_handler_test.go @@ -37,6 +37,7 @@ type mockUserSvc struct { setPasswordWithToken func(ctx context.Context, rawToken, newPassword string) error delete func(ctx context.Context, id uuid.UUID) error initiateAvatarUpload func(ctx context.Context, userID uuid.UUID, fileName, contentType string, fileSize int64) (*attachmentdom.UploadSession, error) + listAfter func(ctx context.Context, limit int, cursor *string, filter domainuser.ListFilter) ([]*domainuser.User, bool, error) completeAvatarUpload func(ctx context.Context, userID, fileID uuid.UUID) (*domainuser.User, error) removeAvatar func(ctx context.Context, userID uuid.UUID) (*domainuser.User, error) } @@ -53,6 +54,12 @@ func (m *mockUserSvc) List(ctx context.Context, page, pageSize int, filter domai } return nil, 0, nil } +func (m *mockUserSvc) ListAfter(ctx context.Context, limit int, cursor *string, filter domainuser.ListFilter) ([]*domainuser.User, bool, error) { + if m.listAfter != nil { + return m.listAfter(ctx, limit, cursor, filter) + } + return nil, false, nil +} func (m *mockUserSvc) CountUsers(context.Context) (int64, error) { return 0, nil } @@ -153,6 +160,7 @@ func newUserRouter(svc domainuser.Service) chi.Router { r.Get("/users/me/global-permissions", h.GetMyGlobalPermissions) // admin routes r.Get("/admin/users", h.ListUsers) + r.Get("/admin/users/cursor", h.ListUsersByCursor) r.Post("/admin/users", h.CreateUser) r.Get("/admin/users/{userId}", h.GetUserByID) r.Patch("/admin/users/{userId}", h.AdminUpdateUser) @@ -273,6 +281,68 @@ func TestListUsers_PassesSearchAndRoleFilter(t *testing.T) { } } +func TestListUsersByCursor_NextCursorAndFilter(t *testing.T) { + last := &domainuser.User{ID: uuid.New(), Username: "bob", FullName: "Bob", Role: domainuser.RoleUser} + var gotLimit int + var gotCursor *string + var gotFilter domainuser.ListFilter + svc := &mockUserSvc{ + listAfter: func(_ context.Context, limit int, cursor *string, f domainuser.ListFilter) ([]*domainuser.User, bool, error) { + gotLimit, gotCursor, gotFilter = limit, cursor, f + return []*domainuser.User{last}, true, nil + }, + } + r := newUserRouter(svc) + + w := do(t, r, http.MethodGet, "/admin/users/cursor?page_size=1&cursor=abc&search=%20bo%20", nil) + if w.Code != http.StatusOK { + t.Fatalf("status = %d: %s", w.Code, w.Body.String()) + } + if gotLimit != 1 || gotCursor == nil || *gotCursor != "abc" || gotFilter.Search != "bo" { + t.Fatalf("limit=%d cursor=%v filter=%+v", gotLimit, gotCursor, gotFilter) + } + var env struct { + Data struct { + Items []map[string]any `json:"items"` + NextCursor *string `json:"next_cursor"` + } `json:"data"` + } + if err := json.Unmarshal(w.Body.Bytes(), &env); err != nil { + t.Fatal(err) + } + if len(env.Data.Items) != 1 || env.Data.NextCursor == nil { + t.Fatalf("unexpected body: %s", w.Body.String()) + } + dec, err := domainuser.DecodeCursor(*env.Data.NextCursor) + if err != nil || dec.ID != last.ID.String() { + t.Fatalf("next_cursor = %v (%v), want id %s", *env.Data.NextCursor, err, last.ID) + } +} + +func TestListUsersByCursor_LastPageHasNoCursor(t *testing.T) { + svc := &mockUserSvc{ + listAfter: func(context.Context, int, *string, domainuser.ListFilter) ([]*domainuser.User, bool, error) { + return []*domainuser.User{{ID: uuid.New(), Username: "a", Role: domainuser.RoleUser}}, false, nil + }, + } + w := do(t, newUserRouter(svc), http.MethodGet, "/admin/users/cursor", nil) + if w.Code != http.StatusOK || !strings.Contains(w.Body.String(), `"next_cursor":null`) { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } +} + +func TestListUsersByCursor_InvalidCursor(t *testing.T) { + svc := &mockUserSvc{ + listAfter: func(context.Context, int, *string, domainuser.ListFilter) ([]*domainuser.User, bool, error) { + return nil, false, domainuser.ErrInvalidCursor + }, + } + w := do(t, newUserRouter(svc), http.MethodGet, "/admin/users/cursor?cursor=%21", nil) + if w.Code != http.StatusBadRequest || !strings.Contains(w.Body.String(), "USER_INVALID_CURSOR") { + t.Fatalf("status=%d body=%s", w.Code, w.Body.String()) + } +} + func TestListUsers_ServiceError(t *testing.T) { svc := &mockUserSvc{ list: func(_ context.Context, _, _ int, _ domainuser.ListFilter) ([]*domainuser.User, int64, error) { diff --git a/services/api/internal/transport/http/presenter/response.go b/services/api/internal/transport/http/presenter/response.go index 08b65c659..e54529593 100644 --- a/services/api/internal/transport/http/presenter/response.go +++ b/services/api/internal/transport/http/presenter/response.go @@ -136,6 +136,8 @@ func statusAndCodeFor(err error) (int, apierr.Code) { return http.StatusConflict, apierr.CodeUsernameTaken case errors.Is(err, userdom.ErrEmailTaken): return http.StatusConflict, apierr.CodeEmailTaken + case errors.Is(err, userdom.ErrInvalidCursor): + return http.StatusBadRequest, apierr.CodeUserInvalidCursor case errors.Is(err, userdom.ErrForbidden): return http.StatusForbidden, apierr.CodeForbidden case errors.Is(err, userdom.ErrInvalidCurrentPassword): diff --git a/services/api/internal/transport/http/router/router.go b/services/api/internal/transport/http/router/router.go index 1037cb2c2..776425e8e 100644 --- a/services/api/internal/transport/http/router/router.go +++ b/services/api/internal/transport/http/router/router.go @@ -199,6 +199,7 @@ func New(deps Deps) http.Handler { // global role is a privilege of its own (global_roles.assign) and is // changed solely by PUT /users/{userId}/global-roles below. r.With(require.Global(authz.PermissionUsersRead)).Get("/users", deps.User.ListUsers) + r.With(require.Global(authz.PermissionUsersRead)).Get("/users/cursor", deps.User.ListUsersByCursor) r.With(require.Global(authz.PermissionUsersWrite)).Post("/users", deps.User.CreateUser) r.With(require.Global(authz.PermissionUsersRead)).Get("/users/{userId}", deps.User.GetUserByID) r.With(require.Global(authz.PermissionUsersWrite)).Patch("/users/{userId}", deps.User.AdminUpdateUser) diff --git a/services/api/internal/transport/http/router/router_test.go b/services/api/internal/transport/http/router/router_test.go index 66980344c..441531d98 100644 --- a/services/api/internal/transport/http/router/router_test.go +++ b/services/api/internal/transport/http/router/router_test.go @@ -49,6 +49,9 @@ func (m *mockUserSvc) List(context.Context, int, int, userdom.ListFilter) ([]*us func (m *mockUserSvc) CountUsers(context.Context) (int64, error) { return 0, nil } +func (m *mockUserSvc) ListAfter(context.Context, int, *string, userdom.ListFilter) ([]*userdom.User, bool, error) { + return nil, false, nil +} func (m *mockUserSvc) CountUsersMustChangePassword(context.Context) (int64, error) { return 0, nil } diff --git a/services/api/test/integration/auth_test.go b/services/api/test/integration/auth_test.go index fede29ac9..43a8d14e8 100644 --- a/services/api/test/integration/auth_test.go +++ b/services/api/test/integration/auth_test.go @@ -114,6 +114,10 @@ func (r *fakeUserRepo) List(_ context.Context, offset, limit int, _ userdom.List return all[offset:end], total, nil } +func (r *fakeUserRepo) ListAfter(_ context.Context, _ int, _ *string, _ userdom.ListFilter) ([]*userdom.User, bool, error) { + return nil, false, nil +} + func (r *fakeUserRepo) CountUsers(_ context.Context) (int64, error) { return int64(len(r.byID)), nil }