From cc244e5159e3e07dfadd37ee8e995a433ec68e64 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Mon, 21 Sep 2026 02:11:38 -0700 Subject: [PATCH] feat: make every admin panel list page past the first and filter by id: bind query-string ids through models.ParamUUID since gin cannot set a uuid.UUID, page the explorers and secondary lists by an opaque offset cursor with an id tiebreak instead of an id keyset that disagreed with the sort, page the audit log on (created_at, id) with an inclusive YYYY-MM-DD end day and read next_cursor on its page, cast every before-date bound to timestamptz, coalesce nullable audit ip and user agent, and report failed admin queries and 5xx mutations to PostHog or Sentry with method, path, status, code and request id --- admin/src/app/dashboard/AuditPage.tsx | 4 +- admin/src/lib/api/client.ts | 6 + admin/src/lib/api/models/admin.ts | 2 +- admin/src/lib/api/reportFailure.test.ts | 53 ++++ admin/src/lib/api/reportFailure.ts | 31 +++ admin/src/lib/observability.ts | 21 +- admin/src/main.tsx | 5 +- cmd/warmblyctl/status.go | 2 +- cmd/warmblyctl/user.go | 2 +- internal/api/handler/admin.go | 74 ++++- internal/api/handler/admin_mailboxes.go | 5 + internal/api/handler/admin_organization.go | 5 + internal/api/handler/admin_outreach.go | 5 + internal/api/handler/limit_requests.go | 5 + internal/app/admin/service.go | 44 +-- internal/models/admin.go | 68 ++--- internal/models/admin_outreach.go | 8 +- internal/models/discount.go | 2 +- internal/models/param_uuid.go | 19 ++ internal/models/param_uuid_test.go | 70 +++++ internal/repository/admin_paging.go | 22 ++ internal/repository/admin_paging_live_test.go | 261 ++++++++++++++++++ internal/repository/admin_schema_live_test.go | 2 +- internal/repository/pg_admin.go | 190 +++++-------- internal/repository/pg_admin_outreach.go | 14 +- internal/repository/pg_discount.go | 4 +- internal/repository/pg_organization.go | 36 +-- .../repository/placement_queries_live_test.go | 2 +- 28 files changed, 714 insertions(+), 248 deletions(-) create mode 100644 admin/src/lib/api/reportFailure.test.ts create mode 100644 admin/src/lib/api/reportFailure.ts create mode 100644 internal/models/param_uuid.go create mode 100644 internal/models/param_uuid_test.go create mode 100644 internal/repository/admin_paging.go create mode 100644 internal/repository/admin_paging_live_test.go diff --git a/admin/src/app/dashboard/AuditPage.tsx b/admin/src/app/dashboard/AuditPage.tsx index d38c785df..2ad173866 100644 --- a/admin/src/app/dashboard/AuditPage.tsx +++ b/admin/src/app/dashboard/AuditPage.tsx @@ -213,10 +213,10 @@ export default function AuditPage() { size="sm" variant="outline" onClick={() => { - const c = data?.pagination?.cursor; + const c = data?.pagination?.next_cursor; if (c) setCursors((prev) => [...prev, c]); }} - disabled={!data?.pagination?.cursor} + disabled={!data?.pagination?.next_cursor} > Next diff --git a/admin/src/lib/api/client.ts b/admin/src/lib/api/client.ts index cda575d15..819f1442d 100644 --- a/admin/src/lib/api/client.ts +++ b/admin/src/lib/api/client.ts @@ -31,6 +31,10 @@ export class APIError extends Error { // failed page can show exactly what broke instead of a generic message. code?: string; requestId?: string; + // The call that failed, path only: no query string, so no filter value + // reaches an error report. + method?: string; + path?: string; body?: T; constructor(message: string, status: number, body?: T) { super(message); @@ -150,6 +154,8 @@ export async function Request(config: AuthRequestConfig): Promise { function noteFailure(config: AuthRequestConfig, failure: APIError): void { const method = config.method?.toUpperCase() ?? "REQUEST"; const path = config.url?.split("?")[0] ?? ""; + failure.method = method; + failure.path = path; const properties: Record = { method, diff --git a/admin/src/lib/api/models/admin.ts b/admin/src/lib/api/models/admin.ts index cb774a887..b59a52e20 100644 --- a/admin/src/lib/api/models/admin.ts +++ b/admin/src/lib/api/models/admin.ts @@ -135,7 +135,7 @@ export interface AdminAuditLogsResult { // and typing it as an array is what let `data.data.length` crash the page. data: AdminAuditLog[] | null; pagination: { - cursor?: string; + next_cursor?: string | null; has_more?: boolean; }; } diff --git a/admin/src/lib/api/reportFailure.test.ts b/admin/src/lib/api/reportFailure.test.ts new file mode 100644 index 000000000..2182c0d98 --- /dev/null +++ b/admin/src/lib/api/reportFailure.test.ts @@ -0,0 +1,53 @@ +import { beforeEach, describe, expect, it, vi } from "vitest"; + +const captured = vi.hoisted(() => [] as Array<{ error: unknown; properties?: Record }>); +vi.mock("@/lib/observability", () => ({ + captureException: (error: unknown, properties?: Record) => captured.push({ error, properties }), + noteStep: () => {}, +})); + +import { APIError, SessionExpiredError } from "@/lib/api/client"; +import { reportFailure } from "./reportFailure"; + +function apiError(status: number): APIError { + const e = new APIError("invalid query parameters", status, { code: "bad_request", request_id: "req-1" }); + e.method = "GET"; + e.path = "/admin/users"; + return e; +} + +describe("reportFailure", () => { + beforeEach(() => { + captured.length = 0; + }); + + it("reports a 400 on a query with the call that failed", () => { + reportFailure(apiError(400), "query"); + expect(captured).toHaveLength(1); + expect(captured[0].properties).toEqual({ + kind: "query", + status: 400, + method: "GET", + path: "/admin/users", + code: "bad_request", + request_id: "req-1", + }); + }); + + it("reports a 5xx mutation but not a 4xx one", () => { + reportFailure(apiError(422), "mutation"); + reportFailure(apiError(500), "mutation"); + expect(captured.map((c) => (c.error as APIError).status)).toEqual([500]); + }); + + it("skips expected answers, the network and a lapsed session", () => { + for (const status of [0, 401, 403, 404, 429]) reportFailure(apiError(status), "query"); + reportFailure(new SessionExpiredError(), "query"); + expect(captured).toHaveLength(0); + }); + + it("reports an error thrown outside the API client", () => { + reportFailure(new TypeError("x is undefined"), "query"); + expect(captured).toHaveLength(1); + }); +}); diff --git a/admin/src/lib/api/reportFailure.ts b/admin/src/lib/api/reportFailure.ts new file mode 100644 index 000000000..c0a4b033d --- /dev/null +++ b/admin/src/lib/api/reportFailure.ts @@ -0,0 +1,31 @@ +// Reports a failed query or mutation. react-query keeps these as error state +// for the page to render, so without this nothing reaches PostHog or Sentry. + +import { APIError, SessionExpiredError } from "@/lib/api/client"; +import { captureException, type StepProperties } from "@/lib/observability"; + +// Answers that are the expected outcome of a request, not a defect: a signed-out +// or under-permitted operator, a record deleted since the list loaded, a limiter. +const EXPECTED = new Set([401, 403, 404, 429]); + +export function shouldReport(error: unknown, kind: "query" | "mutation"): boolean { + if (error instanceof SessionExpiredError) return false; + if (!(error instanceof APIError)) return true; + // Status 0 is the network, not the panel. + if (error.status === 0 || EXPECTED.has(error.status)) return false; + // A 4xx on a mutation is usually the operator's input, which the form shows. + return kind === "query" || error.status >= 500; +} + +export function reportFailure(error: unknown, kind: "query" | "mutation"): void { + if (!shouldReport(error, kind)) return; + const properties: StepProperties = { kind }; + if (error instanceof APIError) { + properties.status = error.status; + if (error.method) properties.method = error.method; + if (error.path) properties.path = error.path; + if (error.code) properties.code = error.code; + if (error.requestId) properties.request_id = error.requestId; + } + captureException(error, properties); +} diff --git a/admin/src/lib/observability.ts b/admin/src/lib/observability.ts index b1552509a..ca1e3673f 100644 --- a/admin/src/lib/observability.ts +++ b/admin/src/lib/observability.ts @@ -32,7 +32,7 @@ export type Identity = { userId: string; email?: string | null; name?: string | export type StepProperties = Record; type Backend = { - capture: (error: unknown) => void; + capture: (error: unknown, properties?: StepProperties) => void; identify: (identity: Identity) => void; step: (message: string, properties?: StepProperties) => void; }; @@ -42,7 +42,7 @@ const backends: Backend[] = []; // EARLY_LIMIT bounds the pre-load buffers: a render loop that throws every // frame must not grow them without end. const EARLY_LIMIT = 20; -let early: unknown[] = []; +let early: Array<{ error: unknown; properties?: StepProperties }> = []; let earlySteps: Array<{ message: string; properties?: StepProperties }> = []; // identity is remembered rather than forwarded once, because a backend that @@ -74,7 +74,7 @@ export function initErrorReporting(): void { .then((posthog) => settle(posthog && POSTHOG_ERROR_TRACKING ? { - capture: (error) => void posthog.captureException(error), + capture: (error, properties) => void posthog.captureException(error, properties), identify: (next) => identifyPostHog(posthog, next), step: (message, properties) => posthog.addExceptionStep(message, properties), } @@ -102,7 +102,8 @@ export function initErrorReporting(): void { release: SENTRY_RELEASE || undefined, }); settle({ - capture: (error) => void Sentry.captureException(error), + capture: (error, properties) => + void Sentry.captureException(error, properties ? { extra: properties } : undefined), identify: (next) => Sentry.setUser(next ? { id: next.userId, email: next.email ?? undefined, username: next.name ?? undefined } @@ -210,12 +211,12 @@ function identifyPostHog(posthog: import("posthog-js").PostHog, next: Identity): // captureException reports an error the app handled itself. A no-op when no // backend is configured. -export function captureException(error: unknown): void { +export function captureException(error: unknown, properties?: StepProperties): void { // Remembered as well as reported while a backend is still loading, so the // one that has not arrived yet gets it on replay. Only the newly settled // backend replays, so nothing is reported twice. - if (awaiting > 0) remember(error); - for (const backend of backends) backend.capture(error); + if (awaiting > 0) remember(error, properties); + for (const backend of backends) backend.capture(error, properties); } // setErrorIdentity names the operator later events belong to. Pass null on @@ -241,7 +242,7 @@ function settle(backend: Backend | null): void { backends.push(backend); if (identity) backend.identify(identity); for (const step of earlySteps) backend.step(step.message, step.properties); - for (const error of early) backend.capture(error); + for (const entry of early) backend.capture(entry.error, entry.properties); } awaiting--; @@ -269,7 +270,7 @@ function installEarlyHandlers(): void { }; } -function remember(error: unknown): void { +function remember(error: unknown, properties?: StepProperties): void { if (early.length >= EARLY_LIMIT) return; - early.push(error); + early.push({ error, properties }); } diff --git a/admin/src/main.tsx b/admin/src/main.tsx index 0c3b8a0fa..ed5a86bf9 100644 --- a/admin/src/main.tsx +++ b/admin/src/main.tsx @@ -8,13 +8,14 @@ import { RouterProvider, useLocation, } from "react-router-dom"; -import { QueryClient, QueryClientProvider } from "@tanstack/react-query"; +import { MutationCache, QueryCache, QueryClient, QueryClientProvider } from "@tanstack/react-query"; import { ReactQueryDevtools } from "@tanstack/react-query-devtools"; import "@fontsource/inter/400.css"; import "@fontsource/inter/600.css"; import { initErrorReporting } from "@/lib/observability"; +import { reportFailure } from "@/lib/api/reportFailure"; import { Toaster } from "@/components/ui/sonner"; import { AppShell } from "@/components/layout/AppShell"; @@ -65,6 +66,8 @@ import { AdminPerm } from "@/lib/auth/permissions"; // day; we don't want a thundering herd of refetches on focus // - retry: 1 — same reasoning as the dashboard const queryClient = new QueryClient({ + queryCache: new QueryCache({ onError: (error) => reportFailure(error, "query") }), + mutationCache: new MutationCache({ onError: (error) => reportFailure(error, "mutation") }), defaultOptions: { queries: { staleTime: 60_000, diff --git a/cmd/warmblyctl/status.go b/cmd/warmblyctl/status.go index eeadd3cc8..47d677ae2 100644 --- a/cmd/warmblyctl/status.go +++ b/cmd/warmblyctl/status.go @@ -96,7 +96,7 @@ func collectStatus(ctx context.Context, c *conn) (*instanceStatus, error) { } admins := []string{} - result, aerr := c.admins.ListAdmins(ctx, nil, 100) + result, aerr := c.admins.ListAdmins(ctx, 0, 100) if aerr != nil { return nil, fmt.Errorf("listing platform admins: %w", aerr) } diff --git a/cmd/warmblyctl/user.go b/cmd/warmblyctl/user.go index 64ab38919..18b8737f8 100644 --- a/cmd/warmblyctl/user.go +++ b/cmd/warmblyctl/user.go @@ -427,7 +427,7 @@ func runUserRevokeAdmin(ctx context.Context, args []string) error { return nil } - result, lerr := c.admins.ListAdmins(ctx, nil, 100) + result, lerr := c.admins.ListAdmins(ctx, 0, 100) if lerr != nil { return fmt.Errorf("counting the remaining platform admins: %w", lerr) } diff --git a/internal/api/handler/admin.go b/internal/api/handler/admin.go index 8c84f4918..ad31c028e 100644 --- a/internal/api/handler/admin.go +++ b/internal/api/handler/admin.go @@ -23,6 +23,11 @@ func (h *Handler) AdminSearchUsers(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.AdminService.SearchUsers(c.Request.Context(), &search) if xerr != nil { @@ -150,10 +155,13 @@ func (h *Handler) AdminGetUserCampaigns(c *gin.Context) { return } - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.GetUserCampaigns(c.Request.Context(), userID, cursor, limit) + result, xerr := h.AdminService.GetUserCampaigns(c.Request.Context(), userID, offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -170,10 +178,13 @@ func (h *Handler) AdminGetUserEmails(c *gin.Context) { return } - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - emails, pagination, xerr := h.AdminService.GetUserEmails(c.Request.Context(), userID, cursor, limit) + emails, pagination, xerr := h.AdminService.GetUserEmails(c.Request.Context(), userID, offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -232,10 +243,13 @@ func (h *Handler) AdminUpdateUserRateLimits(c *gin.Context) { // AdminListWorkers lists all workers func (h *Handler) AdminListWorkers(c *gin.Context) { - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.ListWorkers(c.Request.Context(), cursor, limit) + result, xerr := h.AdminService.ListWorkers(c.Request.Context(), offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -391,10 +405,13 @@ func (h *Handler) AdminGetWarmupHealthSummary(c *gin.Context) { // AdminGetPoolParticipants gets participants in a warmup pool func (h *Handler) AdminGetPoolParticipants(c *gin.Context) { poolType := c.Param("type") - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.GetPoolParticipants(c.Request.Context(), poolType, cursor, limit) + result, xerr := h.AdminService.GetPoolParticipants(c.Request.Context(), poolType, offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -405,10 +422,13 @@ func (h *Handler) AdminGetPoolParticipants(c *gin.Context) { // AdminListBlockedAccounts lists blocked warmup accounts func (h *Handler) AdminListBlockedAccounts(c *gin.Context) { - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.ListBlockedAccounts(c.Request.Context(), cursor, limit) + result, xerr := h.AdminService.ListBlockedAccounts(c.Request.Context(), offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -474,10 +494,13 @@ func (h *Handler) AdminUnblockAccount(c *gin.Context) { // AdminListAppeals lists warmup appeals func (h *Handler) AdminListAppeals(c *gin.Context) { status := c.Query("status") - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.ListAppeals(c.Request.Context(), status, cursor, limit) + result, xerr := h.AdminService.ListAppeals(c.Request.Context(), status, offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -570,6 +593,11 @@ func (h *Handler) AdminSearchCampaigns(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.AdminService.SearchCampaigns(c.Request.Context(), &search) if xerr != nil { @@ -704,10 +732,13 @@ func (h *Handler) AdminGetUserGrowthStats(c *gin.Context) { // AdminListAdmins lists all admin users func (h *Handler) AdminListAdmins(c *gin.Context) { - cursor := parseCursor(c.Query("cursor")) + offset, ok := offsetCursor(c) + if !ok { + return + } limit := parseLimit(c.Query("limit"), 50) - result, xerr := h.AdminService.ListAdmins(c.Request.Context(), cursor, limit) + result, xerr := h.AdminService.ListAdmins(c.Request.Context(), offset, limit) if xerr != nil { errx.JSON(c, xerr) return @@ -775,6 +806,10 @@ func (h *Handler) AdminSearchAuditLogs(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + if _, _, xerr := paging.DecodeTimeCursor(search.Cursor); xerr != nil { + errx.JSON(c, xerr) + return + } result, xerr := h.AdminService.SearchAuditLogs(c.Request.Context(), &search) if xerr != nil { @@ -793,6 +828,17 @@ func (h *Handler) AdminGetPermissionList(c *gin.Context) { // Helper functions +// offsetCursor decodes an admin list's opaque ?cursor, answering 400 for a +// malformed one rather than quietly serving the first page. +func offsetCursor(c *gin.Context) (int, bool) { + offset, xerr := paging.DecodeOffsetCursor(c.Query("cursor")) + if xerr != nil { + errx.JSON(c, xerr) + return 0, false + } + return offset, true +} + func parseCursor(s string) *uuid.UUID { if s == "" { return nil diff --git a/internal/api/handler/admin_mailboxes.go b/internal/api/handler/admin_mailboxes.go index 399974fee..a1da64242 100644 --- a/internal/api/handler/admin_mailboxes.go +++ b/internal/api/handler/admin_mailboxes.go @@ -17,6 +17,11 @@ func (h *Handler) AdminSearchMailboxes(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.AdminService.SearchMailboxes(c.Request.Context(), &search) if xerr != nil { errx.JSON(c, xerr) diff --git a/internal/api/handler/admin_organization.go b/internal/api/handler/admin_organization.go index c82918c84..3b1d53ad6 100644 --- a/internal/api/handler/admin_organization.go +++ b/internal/api/handler/admin_organization.go @@ -17,6 +17,11 @@ func (h *Handler) AdminListOrganizations(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.OrganizationService.SearchOrganizationsForAdmin(c.Request.Context(), &search) if xerr != nil { diff --git a/internal/api/handler/admin_outreach.go b/internal/api/handler/admin_outreach.go index 2a89132b2..3c11dce13 100644 --- a/internal/api/handler/admin_outreach.go +++ b/internal/api/handler/admin_outreach.go @@ -64,6 +64,11 @@ func (h *Handler) AdminListOutreach(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.AdminOutreachService.Search(c.Request.Context(), &search) if xerr != nil { errx.JSON(c, xerr) diff --git a/internal/api/handler/limit_requests.go b/internal/api/handler/limit_requests.go index 2b148ac9c..64e5dbda6 100644 --- a/internal/api/handler/limit_requests.go +++ b/internal/api/handler/limit_requests.go @@ -86,6 +86,11 @@ func (h *Handler) AdminListLimitRequests(c *gin.Context) { errx.JSON(c, errx.New(errx.BadRequest, "invalid query parameters")) return } + offset, ok := offsetCursor(c) + if !ok { + return + } + search.Offset = offset result, xerr := h.OrganizationService.AdminListLimitRequests(c.Request.Context(), &search) if xerr != nil { errx.JSON(c, xerr) diff --git a/internal/app/admin/service.go b/internal/app/admin/service.go index d817b62ab..de9bda503 100644 --- a/internal/app/admin/service.go +++ b/internal/app/admin/service.go @@ -23,13 +23,13 @@ type AdminService interface { BanUser(ctx context.Context, adminID, userID uuid.UUID, reason string, scope models.BanScope, ipAddress, userAgent string) *errx.Error UnbanUser(ctx context.Context, adminID, userID uuid.UUID, reason string, ipAddress, userAgent string) *errx.Error GetUserBans(ctx context.Context, userID uuid.UUID) ([]models.UserBan, *errx.Error) - GetUserCampaigns(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) (*models.AdminCampaignsResult, *errx.Error) - GetUserEmails(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, *errx.Error) + GetUserCampaigns(ctx context.Context, userID uuid.UUID, offset, limit int) (*models.AdminCampaignsResult, *errx.Error) + GetUserEmails(ctx context.Context, userID uuid.UUID, offset, limit int) ([]models.AdminWorkerEmail, *models.Pagination, *errx.Error) GetUserRateLimits(ctx context.Context, userID uuid.UUID) (*models.AdminUserRateLimits, *errx.Error) UpdateUserRateLimits(ctx context.Context, adminID, userID uuid.UUID, update *models.UpdateUserRateLimitsRequest, ipAddress, userAgent string) (*models.AdminUserRateLimits, *errx.Error) // Worker Management - ListWorkers(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminWorkersResult, *errx.Error) + ListWorkers(ctx context.Context, offset, limit int) (*models.AdminWorkersResult, *errx.Error) GetWorkerDetail(ctx context.Context, workerID uuid.UUID) (*models.AdminWorkerDetail, *errx.Error) UpdateWorker(ctx context.Context, adminID, workerID uuid.UUID, update *models.AdminUpdateWorker, ipAddress, userAgent string) *errx.Error GetWorkerEmails(ctx context.Context, workerID uuid.UUID, beforeAt time.Time, beforeID uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, *errx.Error) @@ -38,13 +38,13 @@ type AdminService interface { // Warmup Management ListWarmupPools(ctx context.Context) ([]models.WarmupPoolInfo, *errx.Error) - GetPoolParticipants(ctx context.Context, poolType string, cursor *uuid.UUID, limit int) (*models.WarmupPoolParticipantsResult, *errx.Error) - ListBlockedAccounts(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminBlockedAccountsResult, *errx.Error) + GetPoolParticipants(ctx context.Context, poolType string, offset, limit int) (*models.WarmupPoolParticipantsResult, *errx.Error) + ListBlockedAccounts(ctx context.Context, offset, limit int) (*models.AdminBlockedAccountsResult, *errx.Error) BlockAccount(ctx context.Context, adminID, accountID uuid.UUID, reason string, ipAddress, userAgent string) *errx.Error UnblockAccount(ctx context.Context, adminID, accountID uuid.UUID, ipAddress, userAgent string) *errx.Error // Appeals - ListAppeals(ctx context.Context, status string, cursor *uuid.UUID, limit int) (*models.WarmupAppealsResult, *errx.Error) + ListAppeals(ctx context.Context, status string, offset, limit int) (*models.WarmupAppealsResult, *errx.Error) GetAppeal(ctx context.Context, appealID uuid.UUID) (*models.WarmupAppeal, *errx.Error) ReviewAppeal(ctx context.Context, adminID, appealID uuid.UUID, approved bool, notes string, ipAddress, userAgent string) *errx.Error @@ -61,7 +61,7 @@ type AdminService interface { GetUserGrowthStats(ctx context.Context, startDate, endDate time.Time) ([]models.UserGrowthStats, *errx.Error) // Admin Management - ListAdmins(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminsResult, *errx.Error) + ListAdmins(ctx context.Context, offset, limit int) (*models.AdminsResult, *errx.Error) GrantAdminPermissions(ctx context.Context, adminID, targetUserID uuid.UUID, permissions models.AdminPermission, ipAddress, userAgent string) *errx.Error RevokeAdminPermissions(ctx context.Context, adminID, targetUserID uuid.UUID, ipAddress, userAgent string) *errx.Error @@ -262,10 +262,10 @@ func (s *adminService) GetUserBans(ctx context.Context, userID uuid.UUID) ([]mod return bans, nil } -func (s *adminService) GetUserCampaigns(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) (*models.AdminCampaignsResult, *errx.Error) { +func (s *adminService) GetUserCampaigns(ctx context.Context, userID uuid.UUID, offset, limit int) (*models.AdminCampaignsResult, *errx.Error) { search := &models.AdminCampaignSearch{ - UserID: &userID, - Cursor: cursor, + UserID: &models.ParamUUID{UUID: userID}, + Offset: offset, Limit: limit, } result, err := s.repo.SearchCampaigns(ctx, search) @@ -276,8 +276,8 @@ func (s *adminService) GetUserCampaigns(ctx context.Context, userID uuid.UUID, c return result, nil } -func (s *adminService) GetUserEmails(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, *errx.Error) { - emails, pagination, err := s.repo.GetUserEmails(ctx, userID, cursor, limit) +func (s *adminService) GetUserEmails(ctx context.Context, userID uuid.UUID, offset, limit int) ([]models.AdminWorkerEmail, *models.Pagination, *errx.Error) { + emails, pagination, err := s.repo.GetUserEmails(ctx, userID, offset, limit) if err != nil { errs.CaptureException(err) return nil, nil, errx.New(errx.Internal, "failed to get user emails") @@ -310,8 +310,8 @@ func (s *adminService) UpdateUserRateLimits(ctx context.Context, adminID, userID // Worker Management -func (s *adminService) ListWorkers(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminWorkersResult, *errx.Error) { - result, err := s.repo.ListWorkers(ctx, cursor, limit) +func (s *adminService) ListWorkers(ctx context.Context, offset, limit int) (*models.AdminWorkersResult, *errx.Error) { + result, err := s.repo.ListWorkers(ctx, offset, limit) if err != nil { errs.CaptureException(err) return nil, errx.New(errx.Internal, "failed to list workers") @@ -380,8 +380,8 @@ func (s *adminService) ListWarmupPools(ctx context.Context) ([]models.WarmupPool return pools, nil } -func (s *adminService) GetPoolParticipants(ctx context.Context, poolType string, cursor *uuid.UUID, limit int) (*models.WarmupPoolParticipantsResult, *errx.Error) { - result, err := s.repo.GetPoolParticipants(ctx, poolType, cursor, limit) +func (s *adminService) GetPoolParticipants(ctx context.Context, poolType string, offset, limit int) (*models.WarmupPoolParticipantsResult, *errx.Error) { + result, err := s.repo.GetPoolParticipants(ctx, poolType, offset, limit) if err != nil { errs.CaptureException(err) return nil, errx.New(errx.Internal, "failed to get pool participants") @@ -389,8 +389,8 @@ func (s *adminService) GetPoolParticipants(ctx context.Context, poolType string, return result, nil } -func (s *adminService) ListBlockedAccounts(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminBlockedAccountsResult, *errx.Error) { - result, err := s.repo.ListBlockedAccounts(ctx, cursor, limit) +func (s *adminService) ListBlockedAccounts(ctx context.Context, offset, limit int) (*models.AdminBlockedAccountsResult, *errx.Error) { + result, err := s.repo.ListBlockedAccounts(ctx, offset, limit) if err != nil { errs.CaptureException(err) return nil, errx.New(errx.Internal, "failed to list blocked accounts") @@ -420,8 +420,8 @@ func (s *adminService) UnblockAccount(ctx context.Context, adminID, accountID uu // Appeals -func (s *adminService) ListAppeals(ctx context.Context, status string, cursor *uuid.UUID, limit int) (*models.WarmupAppealsResult, *errx.Error) { - result, err := s.repo.ListAppeals(ctx, status, cursor, limit) +func (s *adminService) ListAppeals(ctx context.Context, status string, offset, limit int) (*models.WarmupAppealsResult, *errx.Error) { + result, err := s.repo.ListAppeals(ctx, status, offset, limit) if err != nil { errs.CaptureException(err) return nil, errx.New(errx.Internal, "failed to list appeals") @@ -580,8 +580,8 @@ func (s *adminService) GetUserGrowthStats(ctx context.Context, startDate, endDat // Admin Management -func (s *adminService) ListAdmins(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminsResult, *errx.Error) { - result, err := s.repo.ListAdmins(ctx, cursor, limit) +func (s *adminService) ListAdmins(ctx context.Context, offset, limit int) (*models.AdminsResult, *errx.Error) { + result, err := s.repo.ListAdmins(ctx, offset, limit) if err != nil { errs.CaptureException(err) return nil, errx.New(errx.Internal, "failed to list admins") diff --git a/internal/models/admin.go b/internal/models/admin.go index 90c01bd3c..2aa94bcf8 100644 --- a/internal/models/admin.go +++ b/internal/models/admin.go @@ -16,7 +16,7 @@ type AdminUserSearch struct { CreatedWithin int `form:"created_within"` // signup window, in days // Plan / subscription - PlanID *uuid.UUID `form:"plan_id"` // subscribed to THIS plan + PlanID *ParamUUID `form:"plan_id"` // subscribed to THIS plan SubscriptionStatus string `form:"subscription_status"` // trialing, active, past_due, ... IsEnterprise bool `form:"is_enterprise"` HasSubscription bool `form:"has_subscription"` @@ -50,10 +50,10 @@ type AdminUserSearch struct { UpdatedAfter *time.Time `form:"updated_after" time_format:"2006-01-02" time_utc:"true"` UpdatedBefore *time.Time `form:"updated_before" time_format:"2006-01-02" time_utc:"true"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` // created_at, email, name - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` // created_at, email, name + SortDesc bool `form:"sort_desc"` } // AdminUserDetail represents a user with admin-relevant statistics @@ -308,8 +308,8 @@ type AdminCampaignsResult struct { // *time.Time with time_format/time_utc, mirroring AdminOrgSearch. type AdminCampaignSearch struct { Query string `form:"q"` - UserID *uuid.UUID `form:"user_id"` - OrgID *uuid.UUID `form:"org_id"` + UserID *ParamUUID `form:"user_id"` + OrgID *ParamUUID `form:"org_id"` Status string `form:"status"` // draft, active, paused, completed, paused_trial_expired, paused_no_accounts, paused_guardrail, paused_undeliverable // Boolean flags @@ -340,10 +340,10 @@ type AdminCampaignSearch struct { UpdatedAfter *time.Time `form:"updated_after" time_format:"2006-01-02" time_utc:"true"` UpdatedBefore *time.Time `form:"updated_before" time_format:"2006-01-02" time_utc:"true"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` + SortDesc bool `form:"sort_desc"` } // AdminAuditLog represents an audit log entry for admin actions @@ -370,13 +370,13 @@ type AdminAuditLogsResult struct { // AdminAuditLogSearch represents search parameters for audit logs type AdminAuditLogSearch struct { - AdminUserID *uuid.UUID `form:"admin_user_id"` + AdminUserID *ParamUUID `form:"admin_user_id"` Action string `form:"action"` TargetType string `form:"target_type"` - TargetID *uuid.UUID `form:"target_id"` - StartDate *time.Time `form:"start_date"` - EndDate *time.Time `form:"end_date"` - Cursor *uuid.UUID `form:"cursor"` + TargetID *ParamUUID `form:"target_id"` + StartDate *time.Time `form:"start_date" time_format:"2006-01-02" time_utc:"true"` + EndDate *time.Time `form:"end_date" time_format:"2006-01-02" time_utc:"true"` + Cursor string `form:"cursor"` Limit int `form:"limit"` } @@ -440,11 +440,11 @@ type AdminMailboxSearch struct { Provider string `form:"provider"` Warmup string `form:"warmup"` // "on", "off", or "" for any CreatedWithin int `form:"created_within"` // connected window, in days - OrgID *uuid.UUID `form:"org_id"` // browse a single org's mailboxes + OrgID *ParamUUID `form:"org_id"` // browse a single org's mailboxes // Ownership / placement - UserID *uuid.UUID `form:"user_id"` // mailboxes owned by THIS user - WorkerID *uuid.UUID `form:"worker_id"` // mailboxes on THIS worker + UserID *ParamUUID `form:"user_id"` // mailboxes owned by THIS user + WorkerID *ParamUUID `form:"worker_id"` // mailboxes on THIS worker // Classification RiskBand string `form:"risk_band"` // clean, risky, quarantine @@ -472,10 +472,10 @@ type AdminMailboxSearch struct { LastSyncedAfter *time.Time `form:"last_synced_after" time_format:"2006-01-02" time_utc:"true"` LastSyncedBefore *time.Time `form:"last_synced_before" time_format:"2006-01-02" time_utc:"true"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` // email, created_at, last_synced_at, campaign_limit - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` // email, created_at, last_synced_at, campaign_limit + SortDesc bool `form:"sort_desc"` } // DailyEmailStats represents daily email statistics for graphs @@ -654,7 +654,7 @@ type WorkerStats struct { type AdminOrgSearch struct { Query string `form:"q"` Status string `form:"status"` // active, pending_deletion, all - PlanID *uuid.UUID `form:"plan_id"` + PlanID *ParamUUID `form:"plan_id"` PlanVisibility string `form:"plan_visibility"` // public, private, none CreatedWithin int `form:"created_within"` // days; 0 = any HasOverrides bool `form:"has_overrides"` // has organization_limit_overrides @@ -707,10 +707,10 @@ type AdminOrgSearch struct { HasAcquisition bool `form:"has_acquisition"` NoAcquisition bool `form:"no_acquisition"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` // created_at, name, owner_email, member_count, campaign_count, email_account_count - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` // created_at, name, owner_email, member_count, campaign_count, email_account_count + SortDesc bool `form:"sort_desc"` } // AdminOrgListItem is one row in the admin org list. It carries enough @@ -778,8 +778,8 @@ type AdminLimitRequestSearch struct { Status string `form:"status"` // pending, approved, rejected, cancelled, all Field string `form:"field"` // app-validated: max_email_accounts ... daily_campaign_limit - OrgID *uuid.UUID `form:"org_id"` - SubmittedBy *uuid.UUID `form:"submitted_by"` + OrgID *ParamUUID `form:"org_id"` + SubmittedBy *ParamUUID `form:"submitted_by"` // Flags Reviewed bool `form:"reviewed"` // reviewed_at IS NOT NULL @@ -798,10 +798,10 @@ type AdminLimitRequestSearch struct { ReviewedAfter *time.Time `form:"reviewed_after" time_format:"2006-01-02" time_utc:"true"` ReviewedBefore *time.Time `form:"reviewed_before" time_format:"2006-01-02" time_utc:"true"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` // submitted_at, requested, current_effective, reviewed_at, status, field, org_name - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` // submitted_at, requested, current_effective, reviewed_at, status, field, org_name + SortDesc bool `form:"sort_desc"` } // AdminLimitRequestsResult is the paginated admin limit-request response. diff --git a/internal/models/admin_outreach.go b/internal/models/admin_outreach.go index 67017a674..1b7f57444 100644 --- a/internal/models/admin_outreach.go +++ b/internal/models/admin_outreach.go @@ -70,10 +70,10 @@ type AdminOutreachSearch struct { SentAtAfter *time.Time `form:"sent_at_after" time_format:"2006-01-02" time_utc:"true"` SentAtBefore *time.Time `form:"sent_at_before" time_format:"2006-01-02" time_utc:"true"` - Cursor *uuid.UUID `form:"cursor"` - Limit int `form:"limit"` - SortBy string `form:"sort_by"` // created_at, sent_at, status, to_email, subject - SortDesc bool `form:"sort_desc"` + Offset int `form:"-"` // decoded from ?cursor by the handler + Limit int `form:"limit"` + SortBy string `form:"sort_by"` // created_at, sent_at, status, to_email, subject + SortDesc bool `form:"sort_desc"` } // AdminOutreachResult is the paginated response for the admin outreach log. diff --git a/internal/models/discount.go b/internal/models/discount.go index 946f15c7a..b1146f30e 100644 --- a/internal/models/discount.go +++ b/internal/models/discount.go @@ -159,7 +159,7 @@ type AdminDiscountSearch struct { // Plan eligibility PlanScope string `form:"plan_scope"` // all|specific - PlanID *uuid.UUID `form:"plan_id"` + PlanID *ParamUUID `form:"plan_id"` // Flag / relationship existence HasRedemptions bool `form:"has_redemptions"` diff --git a/internal/models/param_uuid.go b/internal/models/param_uuid.go new file mode 100644 index 000000000..4374874f7 --- /dev/null +++ b/internal/models/param_uuid.go @@ -0,0 +1,19 @@ +package models + +import "github.com/google/uuid" + +// ParamUUID is a uuid bound from a query string; gin's form binding cannot set a +// bare uuid.UUID and rejects the whole request instead. +type ParamUUID struct { + uuid.UUID +} + +// UnmarshalParam implements gin's binding.BindUnmarshaler. +func (p *ParamUUID) UnmarshalParam(param string) error { + id, err := uuid.Parse(param) + if err != nil { + return err + } + p.UUID = id + return nil +} diff --git a/internal/models/param_uuid_test.go b/internal/models/param_uuid_test.go new file mode 100644 index 000000000..5699fa319 --- /dev/null +++ b/internal/models/param_uuid_test.go @@ -0,0 +1,70 @@ +package models + +import ( + "net/http/httptest" + "reflect" + "testing" + + "github.com/gin-gonic/gin" + "github.com/google/uuid" +) + +func bindQuery(t *testing.T, rawQuery string, dst any) error { + t.Helper() + gin.SetMode(gin.TestMode) + c, _ := gin.CreateTestContext(httptest.NewRecorder()) + c.Request = httptest.NewRequest("GET", "/?"+rawQuery, nil) + return c.ShouldBindQuery(dst) +} + +func TestParamUUIDBindsFromQuery(t *testing.T) { + id := uuid.New() + var s AdminMailboxSearch + if err := bindQuery(t, "worker_id="+id.String()+"&org_id="+id.String(), &s); err != nil { + t.Fatalf("bind: %v", err) + } + if s.WorkerID == nil || s.WorkerID.UUID != id || s.OrgID == nil || s.OrgID.UUID != id { + t.Fatalf("got worker %v org %v, want %s", s.WorkerID, s.OrgID, id) + } + if s.UserID != nil { + t.Fatalf("absent user_id bound to %v", s.UserID) + } +} + +func TestParamUUIDRejectsGarbage(t *testing.T) { + var s AdminMailboxSearch + if err := bindQuery(t, "worker_id=not-a-uuid", &s); err == nil { + t.Fatal("bound a malformed uuid") + } +} + +// Every admin search struct must bind its ids and cursor together, so a field +// typed as a bare uuid.UUID cannot 400 an explorer again. +func TestAdminSearchStructsBindIDsAndCursor(t *testing.T) { + id := uuid.NewString() + for _, dst := range []any{ + &AdminUserSearch{}, &AdminCampaignSearch{}, &AdminAuditLogSearch{}, &AdminMailboxSearch{}, + &AdminOrgSearch{}, &AdminLimitRequestSearch{}, &AdminOutreachSearch{}, &AdminDiscountSearch{}, + } { + typ := reflect.TypeOf(dst).Elem() + query := "" + for i := 0; i < typ.NumField(); i++ { + f := typ.Field(i) + key := f.Tag.Get("form") + ft := f.Type + if ft.Kind() == reflect.Pointer { + ft = ft.Elem() + } + if ft == reflect.TypeOf(uuid.UUID{}) { + t.Errorf("%s.%s is a uuid.UUID, which gin cannot bind; use ParamUUID", typ.Name(), f.Name) + } + if ft == reflect.TypeOf(ParamUUID{}) { + query += key + "=" + id + "&" + } + } + query += "cursor=o1_NTA" + if err := bindQuery(t, query, dst); err != nil { + t.Errorf("%s: bind %q: %v", typ.Name(), query, err) + } + } +} diff --git a/internal/repository/admin_paging.go b/internal/repository/admin_paging.go new file mode 100644 index 000000000..3535ec3a0 --- /dev/null +++ b/internal/repository/admin_paging.go @@ -0,0 +1,22 @@ +package repository + +import ( + "strconv" + + "github.com/warmbly/warmbly/internal/utils/paging" +) + +// The admin lists page by offset: their sorts are operator-chosen and often +// nullable, so no single keyset fits them. + +// adminLimitOffset is the tail of a paged admin query. limitParam is the +// placeholder carrying limit+1; the offset is an int, so it is rendered inline +// and the count queries can keep dropping only the trailing LIMIT arg. +func adminLimitOffset(limitParam string, offset int) string { + return "LIMIT " + limitParam + " OFFSET " + strconv.Itoa(offset) +} + +// adminNextCursor is the cursor for the page after one that started at offset. +func adminNextCursor(offset, limit int) *string { + return paging.EncodeOffset(offset + limit) +} diff --git a/internal/repository/admin_paging_live_test.go b/internal/repository/admin_paging_live_test.go new file mode 100644 index 000000000..587748f41 --- /dev/null +++ b/internal/repository/admin_paging_live_test.go @@ -0,0 +1,261 @@ +package repository + +import ( + "context" + "sort" + "testing" + "time" + + "github.com/google/uuid" + + "github.com/warmbly/warmbly/internal/models" + "github.com/warmbly/warmbly/internal/utils/paging" +) + +// Issue #639: the admin lists paged on id < cursor while ordering by another +// column, so with random ids page two was an arbitrary slice. Each list is +// walked one row at a time here and must return every row exactly once. +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/?sslmode=disable \ +// go test ./internal/repository/ -run LiveAdminPaging -v + +// walk pages a list to the end, one row per page, the way the panel does. +func walk(t *testing.T, fetch func(cursor string) ([]uuid.UUID, models.Pagination)) []uuid.UUID { + t.Helper() + var seen []uuid.UUID + cursor := "" + for page := 0; page < 20; page++ { + ids, p := fetch(cursor) + seen = append(seen, ids...) + if !p.HasMore { + if p.NextCursor != nil { + t.Fatalf("last page still carries next_cursor %q", *p.NextCursor) + } + return seen + } + if p.NextCursor == nil { + t.Fatal("has_more with no next_cursor") + } + cursor = *p.NextCursor + } + t.Fatal("list never ended") + return nil +} + +func offsetOf(t *testing.T, cursor string) int { + t.Helper() + offset, xerr := paging.DecodeOffsetCursor(cursor) + if xerr != nil { + t.Fatalf("next_cursor %q does not decode as an offset: %v", cursor, xerr) + } + return offset +} + +func sameSet(t *testing.T, what string, got, want []uuid.UUID) { + t.Helper() + g := append([]uuid.UUID(nil), got...) + w := append([]uuid.UUID(nil), want...) + sort.Slice(g, func(i, j int) bool { return g[i].String() < g[j].String() }) + sort.Slice(w, func(i, j int) bool { return w[i].String() < w[j].String() }) + if len(g) != len(w) { + t.Fatalf("%s: walked %d rows %v, want %d %v", what, len(g), g, len(w), w) + } + for i := range g { + if g[i] != w[i] { + t.Fatalf("%s: walked %v, want %v", what, g, w) + } + } +} + +func TestLiveAdminPagingWalksEveryRowOnce(t *testing.T) { + _, pool := liveContactDB(t) + ctx := context.Background() + f := newAdminFixture(t, pool) + admin := NewAdminRepository(pool) + + exec := func(sql string, args ...any) { + t.Helper() + if _, err := pool.Exec(ctx, sql, args...); err != nil { + t.Fatalf("fixture %q: %v", sql[:min(60, len(sql))], err) + } + } + + users := []uuid.UUID{f.user} + mailboxes := []uuid.UUID{f.mailbox} + campaigns := []uuid.UUID{f.campaign} + for i := 0; i < 3; i++ { + u, mb, c := uuid.New(), uuid.New(), uuid.New() + exec(`INSERT INTO users (id, first_name, last_name, email, password_hash) + VALUES ($1, 'Paging', 'Live', $2, 'x')`, u, f.tag+"-u"+u.String()[:6]+"@test.local") + t.Cleanup(func() { _, _ = pool.Exec(context.Background(), `DELETE FROM users WHERE id = $1`, u) }) + exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name, signature_plain, signature_html, provider) + VALUES ($1, $2, $3, $4, 'Paging', '', '', 'smtp_imap')`, mb, f.user, f.org, f.tag+"-mb"+mb.String()[:6]+"@test.local") + exec(`INSERT INTO campaigns (id, user_id, organization_id, name, description, days, status, updated_at, created_at) + VALUES ($1, $2, $3, 'Paging', '', 62, 'active', NOW(), NOW())`, c, f.user, f.org) + users = append(users, u) + mailboxes = append(mailboxes, mb) + campaigns = append(campaigns, c) + } + + // One instant for every audit row, so only the id tiebreak orders them. + at := time.Now().UTC().Truncate(time.Second) + var audits []uuid.UUID + for i := 0; i < 4; i++ { + id := uuid.New() + exec(`INSERT INTO admin_audit_logs (id, admin_user_id, action, target_type, target_id, created_at) + VALUES ($1, $2, 'paging', 'user', $2, $3)`, id, f.user, at) + audits = append(audits, id) + } + + t.Run("users", func(t *testing.T) { + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchUsers(ctx, &models.AdminUserSearch{Query: f.tag, Status: "all", Limit: 1, Offset: offsetOf(t, cursor)}) + if err != nil { + t.Fatalf("SearchUsers: %v", err) + } + var ids []uuid.UUID + for _, u := range res.Data { + ids = append(ids, u.ID) + } + return ids, res.Pagination + }) + sameSet(t, "users", got, users) + }) + + // Every explorer's "before" bound adds a day to a bare parameter, which + // Postgres types as an interval unless the query casts it. + t.Run("before and after date filters", func(t *testing.T) { + today := at.Truncate(24 * time.Hour) + org := &models.ParamUUID{UUID: f.org} + res, err := admin.SearchUsers(ctx, &models.AdminUserSearch{Query: f.tag, Status: "all", CreatedAfter: &today, CreatedBefore: &today, Limit: 50}) + if err != nil { + t.Fatalf("SearchUsers: %v", err) + } + if len(res.Data) != len(users) { + t.Fatalf("users created today: %d, want %d", len(res.Data), len(users)) + } + if _, err := admin.SearchCampaigns(ctx, &models.AdminCampaignSearch{OrgID: org, CreatedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("SearchCampaigns: %v", err) + } + if _, err := admin.SearchMailboxesForAdmin(ctx, &models.AdminMailboxSearch{OrgID: org, CreatedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("SearchMailboxesForAdmin: %v", err) + } + orgs := NewOrganizationRepository(pool) + if _, err := orgs.SearchOrganizationsForAdmin(ctx, &models.AdminOrgSearch{CreatedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("SearchOrganizationsForAdmin: %v", err) + } + if _, err := orgs.ListLimitRequestsForAdmin(ctx, &models.AdminLimitRequestSearch{SubmittedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("ListLimitRequestsForAdmin: %v", err) + } + if _, err := NewAdminOutreachRepository(pool).Search(ctx, &models.AdminOutreachSearch{CreatedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("outreach Search: %v", err) + } + if _, err := NewDiscountCodeRepository(pool).List(ctx, &models.AdminDiscountSearch{CreatedBefore: &today, Limit: 1}); err != nil { + t.Fatalf("discount List: %v", err) + } + }) + + t.Run("users sorted by name", func(t *testing.T) { + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchUsers(ctx, &models.AdminUserSearch{Query: f.tag, Status: "all", SortBy: "name", SortDesc: true, Limit: 1, Offset: offsetOf(t, cursor)}) + if err != nil { + t.Fatalf("SearchUsers: %v", err) + } + var ids []uuid.UUID + for _, u := range res.Data { + ids = append(ids, u.ID) + } + return ids, res.Pagination + }) + sameSet(t, "users by name", got, users) + }) + + t.Run("campaigns", func(t *testing.T) { + org := &models.ParamUUID{UUID: f.org} + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchCampaigns(ctx, &models.AdminCampaignSearch{OrgID: org, SortBy: "name", Limit: 1, Offset: offsetOf(t, cursor)}) + if err != nil { + t.Fatalf("SearchCampaigns: %v", err) + } + var ids []uuid.UUID + for _, c := range res.Data { + ids = append(ids, c.ID) + } + return ids, res.Pagination + }) + sameSet(t, "campaigns", got, campaigns) + }) + + t.Run("mailboxes", func(t *testing.T) { + org := &models.ParamUUID{UUID: f.org} + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchMailboxesForAdmin(ctx, &models.AdminMailboxSearch{OrgID: org, Status: "all", SortBy: "last_synced_at", Limit: 1, Offset: offsetOf(t, cursor)}) + if err != nil { + t.Fatalf("SearchMailboxesForAdmin: %v", err) + } + var ids []uuid.UUID + for _, m := range res.Data { + ids = append(ids, m.ID) + } + return ids, res.Pagination + }) + sameSet(t, "mailboxes", got, mailboxes) + }) + + t.Run("user mailboxes", func(t *testing.T) { + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + emails, p, err := admin.GetUserEmails(ctx, f.user, offsetOf(t, cursor), 1) + if err != nil { + t.Fatalf("GetUserEmails: %v", err) + } + var ids []uuid.UUID + for _, e := range emails { + ids = append(ids, e.ID) + } + return ids, *p + }) + sameSet(t, "user mailboxes", got, mailboxes) + }) + + t.Run("audit log", func(t *testing.T) { + actor := &models.ParamUUID{UUID: f.user} + day := at.Truncate(24 * time.Hour) + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchAuditLogs(ctx, &models.AdminAuditLogSearch{AdminUserID: actor, StartDate: &day, EndDate: &day, Cursor: cursor, Limit: 1}) + if err != nil { + t.Fatalf("SearchAuditLogs: %v", err) + } + var ids []uuid.UUID + for _, l := range res.Data { + ids = append(ids, l.ID) + } + return ids, res.Pagination + }) + // An end date names the whole day, so rows written today are inside it. + sameSet(t, "audit log", got, audits) + }) + + t.Run("organizations, limit requests and outreach page past the first", func(t *testing.T) { + orgs := NewOrganizationRepository(pool) + if _, err := orgs.SearchOrganizationsForAdmin(ctx, &models.AdminOrgSearch{Status: "all", Limit: 1, Offset: 1}); err != nil { + t.Fatalf("SearchOrganizationsForAdmin: %v", err) + } + if _, err := orgs.ListLimitRequestsForAdmin(ctx, &models.AdminLimitRequestSearch{Status: "all", Limit: 1, Offset: 1}); err != nil { + t.Fatalf("ListLimitRequestsForAdmin: %v", err) + } + if _, err := NewAdminOutreachRepository(pool).Search(ctx, &models.AdminOutreachSearch{Limit: 1, Offset: 1}); err != nil { + t.Fatalf("outreach Search: %v", err) + } + for name, call := range map[string]func() error{ + "ListAdmins": func() error { _, err := admin.ListAdmins(ctx, 1, 1); return err }, + "ListWorkers": func() error { _, err := admin.ListWorkers(ctx, 1, 1); return err }, + "GetPoolParticipants": func() error { _, err := admin.GetPoolParticipants(ctx, "free", 1, 1); return err }, + "ListBlockedAccounts": func() error { _, err := admin.ListBlockedAccounts(ctx, 1, 1); return err }, + "ListAppeals": func() error { _, err := admin.ListAppeals(ctx, "", 1, 1); return err }, + } { + if err := call(); err != nil { + t.Fatalf("%s: %v", name, err) + } + } + }) +} diff --git a/internal/repository/admin_schema_live_test.go b/internal/repository/admin_schema_live_test.go index d38c45f83..f4823abf8 100644 --- a/internal/repository/admin_schema_live_test.go +++ b/internal/repository/admin_schema_live_test.go @@ -112,7 +112,7 @@ func TestLiveAdminUserPreviewListsMailboxes(t *testing.T) { // The paginated mailbox list behind the user detail page compared the same // uuid column against a text parameter, and that one surfaced as a 500. - emails, pagination, err := repo.GetUserEmails(context.Background(), f.user, nil, 50) + emails, pagination, err := repo.GetUserEmails(context.Background(), f.user, 0, 50) if err != nil { t.Fatalf("GetUserEmails: %v", err) } diff --git a/internal/repository/pg_admin.go b/internal/repository/pg_admin.go index 059ce9e09..2cb76c275 100644 --- a/internal/repository/pg_admin.go +++ b/internal/repository/pg_admin.go @@ -33,11 +33,11 @@ type AdminRepository interface { BanUser(ctx context.Context, userID, bannedBy uuid.UUID, reason string, scope uint32) error UnbanUser(ctx context.Context, userID, unbannedBy uuid.UUID, reason string) error GetUserBans(ctx context.Context, userID uuid.UUID) ([]models.UserBan, error) - GetUserEmails(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, error) - ListAdmins(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminsResult, error) + GetUserEmails(ctx context.Context, userID uuid.UUID, offset, limit int) ([]models.AdminWorkerEmail, *models.Pagination, error) + ListAdmins(ctx context.Context, offset, limit int) (*models.AdminsResult, error) // Worker Management - ListWorkers(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminWorkersResult, error) + ListWorkers(ctx context.Context, offset, limit int) (*models.AdminWorkersResult, error) GetWorkerDetail(ctx context.Context, workerID uuid.UUID) (*models.AdminWorkerDetail, error) UpdateWorker(ctx context.Context, workerID uuid.UUID, update *models.AdminUpdateWorker) error GetWorkerEmails(ctx context.Context, workerID uuid.UUID, beforeAt time.Time, beforeID uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, error) @@ -46,13 +46,13 @@ type AdminRepository interface { // Warmup Management ListWarmupPools(ctx context.Context) ([]models.WarmupPoolInfo, error) - GetPoolParticipants(ctx context.Context, poolType string, cursor *uuid.UUID, limit int) (*models.WarmupPoolParticipantsResult, error) - ListBlockedAccounts(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminBlockedAccountsResult, error) + GetPoolParticipants(ctx context.Context, poolType string, offset, limit int) (*models.WarmupPoolParticipantsResult, error) + ListBlockedAccounts(ctx context.Context, offset, limit int) (*models.AdminBlockedAccountsResult, error) BlockAccount(ctx context.Context, accountID uuid.UUID, blockedBy uuid.UUID, reason string) error UnblockAccount(ctx context.Context, accountID uuid.UUID) error // Warmup Appeals - ListAppeals(ctx context.Context, status string, cursor *uuid.UUID, limit int) (*models.WarmupAppealsResult, error) + ListAppeals(ctx context.Context, status string, offset, limit int) (*models.WarmupAppealsResult, error) GetAppeal(ctx context.Context, appealID uuid.UUID) (*models.WarmupAppeal, error) ReviewAppeal(ctx context.Context, appealID uuid.UUID, reviewedBy uuid.UUID, approved bool, notes string) error @@ -152,7 +152,7 @@ func (r *adminRepository) SearchUsers(ctx context.Context, search *models.AdminU } addBefore := func(col string, v *time.Time) { if v != nil { - whereClause += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + whereClause += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -161,7 +161,7 @@ func (r *adminRepository) SearchUsers(ctx context.Context, search *models.AdminU // Plan / subscription if search.PlanID != nil { whereClause += ` AND EXISTS (SELECT 1 FROM subscriptions s WHERE s.user_id = u.id AND s.plan_id = $` + itoa(argNum) + `)` - args = append(args, *search.PlanID) + args = append(args, search.PlanID.UUID) argNum++ } if search.SubscriptionStatus != "" { @@ -219,22 +219,18 @@ func (r *adminRepository) SearchUsers(ctx context.Context, search *models.AdminU addAfter("u.updated_at", search.UpdatedAfter) addBefore("u.updated_at", search.UpdatedBefore) - if search.Cursor != nil { - whereClause += ` AND u.id < $` + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset - orderBy := "ORDER BY u.created_at DESC" - if search.SortBy != "" { - switch search.SortBy { - case "email": - orderBy = "ORDER BY u.email" - case "name": - orderBy = "ORDER BY u.first_name, u.last_name" - } + orderBy := "ORDER BY u.created_at DESC, u.id DESC" + if search.SortBy == "email" || search.SortBy == "name" { + dir := " ASC" if search.SortDesc { - orderBy += " DESC" + dir = " DESC" + } + if search.SortBy == "email" { + orderBy = "ORDER BY u.email" + dir + ", u.id" + dir + } else { + orderBy = "ORDER BY u.first_name" + dir + ", u.last_name" + dir + ", u.id" + dir } } @@ -251,7 +247,7 @@ func (r *adminRepository) SearchUsers(ctx context.Context, search *models.AdminU FROM users u ` + whereClause + ` ` + orderBy + ` - LIMIT $` + itoa(argNum) + ` + adminLimitOffset("$"+itoa(argNum), offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -286,8 +282,7 @@ func (r *adminRepository) SearchUsers(ctx context.Context, search *models.AdminU if len(users) > limit { result.Data = users[:limit] - lastID := users[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } // Get total count @@ -604,7 +599,7 @@ func (r *adminRepository) GetUserBans(ctx context.Context, userID uuid.UUID) ([] } // GetUserEmails gets email accounts belonging to a user -func (r *adminRepository) GetUserEmails(ctx context.Context, userID uuid.UUID, cursor *uuid.UUID, limit int) ([]models.AdminWorkerEmail, *models.Pagination, error) { +func (r *adminRepository) GetUserEmails(ctx context.Context, userID uuid.UUID, offset, limit int) ([]models.AdminWorkerEmail, *models.Pagination, error) { if limit <= 0 || limit > 100 { limit = 50 } @@ -612,19 +607,14 @@ func (r *adminRepository) GetUserEmails(ctx context.Context, userID uuid.UUID, c args := []interface{}{userID, limit + 1} // email_accounts.user_id is uuid; casting it to text made every call fail. whereClause := "WHERE ea.user_id = $1" - if cursor != nil { - whereClause += " AND ea.id < $3" - args = append(args, *cursor) - } query := ` SELECT ea.id, ea.email, ea.user_id, ea.organization_id, ea.status, ea.provider, ea.warmup IS NOT NULL, ea.last_synced_at FROM email_accounts ea ` + whereClause + ` - ORDER BY ea.created_at DESC - LIMIT $2 - ` + ORDER BY ea.created_at DESC, ea.id DESC + ` + adminLimitOffset("$2", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -651,24 +641,20 @@ func (r *adminRepository) GetUserEmails(ctx context.Context, userID uuid.UUID, c if len(emails) > limit { emails = emails[:limit] - pagination.NextCursor = paging.UUIDString(emails[limit-1].ID) + pagination.NextCursor = adminNextCursor(offset, limit) } return emails, pagination, nil } // ListAdmins lists all admin users -func (r *adminRepository) ListAdmins(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminsResult, error) { +func (r *adminRepository) ListAdmins(ctx context.Context, offset, limit int) (*models.AdminsResult, error) { if limit <= 0 || limit > 100 { limit = 50 } args := []interface{}{limit + 1} whereClause := "WHERE u.admin_permissions > 0" - if cursor != nil { - whereClause += " AND u.id < $2" - args = append(args, *cursor) - } query := ` SELECT u.id, u.first_name, u.last_name, u.email, u.admin_permissions, @@ -677,9 +663,8 @@ func (r *adminRepository) ListAdmins(ctx context.Context, cursor *uuid.UUID, lim FROM users u LEFT JOIN users gu ON gu.id = u.admin_granted_by ` + whereClause + ` - ORDER BY u.admin_granted_at DESC - LIMIT $1 - ` + ORDER BY u.admin_granted_at DESC NULLS LAST, u.id DESC + ` + adminLimitOffset("$1", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -723,25 +708,19 @@ func (r *adminRepository) ListAdmins(ctx context.Context, cursor *uuid.UUID, lim if len(admins) > limit { result.Data = admins[:limit] - lastID := admins[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } return result, nil } // ListWorkers lists all workers with details -func (r *adminRepository) ListWorkers(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminWorkersResult, error) { +func (r *adminRepository) ListWorkers(ctx context.Context, offset, limit int) (*models.AdminWorkersResult, error) { if limit <= 0 || limit > 100 { limit = 50 } args := []interface{}{limit + 1} - whereClause := "" - if cursor != nil { - whereClause = "WHERE w.id < $2" - args = append(args, *cursor) - } query := ` SELECT w.id, n.name, n.notes, n.address, n.active, @@ -750,10 +729,8 @@ func (r *adminRepository) ListWorkers(ctx context.Context, cursor *uuid.UUID, li (SELECT COUNT(*) FROM email_accounts ea WHERE ea.worker_id = w.id) as connected_emails FROM workers w JOIN fleet_nodes n ON n.id = w.id - ` + whereClause + ` - ORDER BY w.created_at DESC - LIMIT $1 - ` + ORDER BY w.created_at DESC, w.id DESC + ` + adminLimitOffset("$1", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -785,8 +762,7 @@ func (r *adminRepository) ListWorkers(ctx context.Context, cursor *uuid.UUID, li if len(workers) > limit { result.Data = workers[:limit] - lastID := workers[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } return result, nil @@ -1020,17 +996,13 @@ func (r *adminRepository) ListWarmupPools(ctx context.Context) ([]models.WarmupP } // GetPoolParticipants gets participants in a warmup pool -func (r *adminRepository) GetPoolParticipants(ctx context.Context, poolType string, cursor *uuid.UUID, limit int) (*models.WarmupPoolParticipantsResult, error) { +func (r *adminRepository) GetPoolParticipants(ctx context.Context, poolType string, offset, limit int) (*models.WarmupPoolParticipantsResult, error) { if limit <= 0 || limit > 100 { limit = 50 } args := []interface{}{poolType, limit + 1} whereClause := "WHERE wp.pool_type = $1::warmup_pool_type" - if cursor != nil { - whereClause += " AND wpp.email_account_id < $3" - args = append(args, *cursor) - } query := ` SELECT @@ -1044,9 +1016,8 @@ func (r *adminRepository) GetPoolParticipants(ctx context.Context, poolType stri JOIN warmup_pools wp ON wpp.pool_id = wp.id JOIN email_accounts ea ON ea.id = wpp.email_account_id ` + whereClause + ` - ORDER BY wpp.joined_at DESC - LIMIT $2 - ` + ORDER BY wpp.joined_at DESC, wpp.email_account_id DESC + ` + adminLimitOffset("$2", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1076,24 +1047,20 @@ func (r *adminRepository) GetPoolParticipants(ctx context.Context, poolType stri } if len(participants) > limit { result.Data = participants[:limit] - result.Pagination.NextCursor = paging.UUIDString(participants[limit-1].ID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } return result, nil } // ListBlockedAccounts lists blocked warmup accounts -func (r *adminRepository) ListBlockedAccounts(ctx context.Context, cursor *uuid.UUID, limit int) (*models.AdminBlockedAccountsResult, error) { +func (r *adminRepository) ListBlockedAccounts(ctx context.Context, offset, limit int) (*models.AdminBlockedAccountsResult, error) { if limit <= 0 || limit > 100 { limit = 50 } args := []interface{}{limit + 1} whereClause := `WHERE (wpp.health_state IN ('quarantined', 'blocked') OR wpp.blocked_at IS NOT NULL)` - if cursor != nil { - whereClause += " AND wpp.email_account_id < $2" - args = append(args, *cursor) - } query := ` SELECT @@ -1107,9 +1074,8 @@ func (r *adminRepository) ListBlockedAccounts(ctx context.Context, cursor *uuid. JOIN email_accounts ea ON ea.id = wpp.email_account_id JOIN users u ON u.id = ea.user_id::uuid ` + whereClause + ` - ORDER BY blocked_at DESC - LIMIT $1 - ` + ORDER BY blocked_at DESC, wpp.email_account_id DESC + ` + adminLimitOffset("$1", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1147,7 +1113,7 @@ func (r *adminRepository) ListBlockedAccounts(ctx context.Context, cursor *uuid. } if len(accounts) > limit { result.Data = accounts[:limit] - result.Pagination.NextCursor = paging.UUIDString(accounts[limit-1].ID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } return result, nil @@ -1205,7 +1171,7 @@ func (r *adminRepository) UnblockAccount(ctx context.Context, accountID uuid.UUI } // ListAppeals lists warmup appeals -func (r *adminRepository) ListAppeals(ctx context.Context, status string, cursor *uuid.UUID, limit int) (*models.WarmupAppealsResult, error) { +func (r *adminRepository) ListAppeals(ctx context.Context, status string, offset, limit int) (*models.WarmupAppealsResult, error) { if limit <= 0 || limit > 100 { limit = 50 } @@ -1220,11 +1186,6 @@ func (r *adminRepository) ListAppeals(ctx context.Context, status string, cursor argNum++ } - if cursor != nil { - whereClause += " AND wa.id < $" + itoa(argNum) - args = append(args, *cursor) - } - query := ` SELECT wa.id, wa.email_account_id, wa.user_id, wa.reason, wa.status, wa.reviewed_by, wa.reviewed_at, wa.review_notes, wa.created_at, @@ -1232,9 +1193,8 @@ func (r *adminRepository) ListAppeals(ctx context.Context, status string, cursor FROM warmup_appeals wa JOIN users u ON u.id = wa.user_id ` + whereClause + ` - ORDER BY wa.created_at DESC - LIMIT $1 - ` + ORDER BY wa.created_at DESC, wa.id DESC + ` + adminLimitOffset("$1", offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1269,8 +1229,7 @@ func (r *adminRepository) ListAppeals(ctx context.Context, status string, cursor if len(appeals) > limit { result.Data = appeals[:limit] - lastID := appeals[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } return result, nil @@ -1378,13 +1337,13 @@ func (r *adminRepository) SearchCampaigns(ctx context.Context, search *models.Ad if search.UserID != nil { whereClause += " AND c.user_id = $" + itoa(argNum) - args = append(args, *search.UserID) + args = append(args, search.UserID.UUID) argNum++ } if search.OrgID != nil { whereClause += " AND c.organization_id = $" + itoa(argNum) - args = append(args, *search.OrgID) + args = append(args, search.OrgID.UUID) argNum++ } @@ -1435,7 +1394,7 @@ func (r *adminRepository) SearchCampaigns(ctx context.Context, search *models.Ad } addBefore := func(col string, v *time.Time) { if v != nil { - whereClause += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + whereClause += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -1460,11 +1419,7 @@ func (r *adminRepository) SearchCampaigns(ctx context.Context, search *models.Ad addAfter("c.updated_at", search.UpdatedAfter) addBefore("c.updated_at", search.UpdatedBefore) - if search.Cursor != nil { - whereClause += " AND c.id < $" + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset orderCol := "c.created_at" switch search.SortBy { @@ -1507,7 +1462,7 @@ func (r *adminRepository) SearchCampaigns(ctx context.Context, search *models.Ad LEFT JOIN organizations o ON o.id = c.organization_id ` + whereClause + ` ` + orderBy + ` - LIMIT $` + itoa(argNum) + ` + adminLimitOffset("$"+itoa(argNum), offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1554,8 +1509,7 @@ func (r *adminRepository) SearchCampaigns(ctx context.Context, search *models.Ad if len(campaigns) > limit { result.Data = campaigns[:limit] - lastID := campaigns[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } // Total count for the same filter — drop the trailing LIMIT arg. @@ -1643,7 +1597,7 @@ func (r *adminRepository) SearchAuditLogs(ctx context.Context, search *models.Ad if search.AdminUserID != nil { whereClause += " AND al.admin_user_id = $" + itoa(argNum) - args = append(args, *search.AdminUserID) + args = append(args, search.AdminUserID.UUID) argNum++ } @@ -1661,7 +1615,7 @@ func (r *adminRepository) SearchAuditLogs(ctx context.Context, search *models.Ad if search.TargetID != nil { whereClause += " AND al.target_id = $" + itoa(argNum) - args = append(args, *search.TargetID) + args = append(args, search.TargetID.UUID) argNum++ } @@ -1672,27 +1626,30 @@ func (r *adminRepository) SearchAuditLogs(ctx context.Context, search *models.Ad } if search.EndDate != nil { - whereClause += " AND al.created_at <= $" + itoa(argNum) + // The end date names a whole day, so the bound is the next midnight. + whereClause += " AND al.created_at < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *search.EndDate) argNum++ } - if search.Cursor != nil { - whereClause += " AND al.id < $" + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ + // The log only grows, so it pages on its own order rather than an offset + // that shifts under an operator who is watching it fill. + if at, id, xerr := paging.DecodeTimeCursor(search.Cursor); xerr == nil && id != uuid.Nil { + whereClause += " AND (al.created_at, al.id) < ($" + itoa(argNum) + "::timestamptz, $" + itoa(argNum+1) + "::uuid)" + args = append(args, at, id) + argNum += 2 } args = append(args, limit+1) query := ` SELECT al.id, al.admin_user_id, al.action, al.target_type, al.target_id, - al.details, al.ip_address, al.user_agent, al.created_at, + al.details, COALESCE(al.ip_address, ''), COALESCE(al.user_agent, ''), al.created_at, u.id, u.first_name, u.last_name, u.email FROM admin_audit_logs al JOIN users u ON u.id = al.admin_user_id ` + whereClause + ` - ORDER BY al.created_at DESC + ORDER BY al.created_at DESC, al.id DESC LIMIT $` + itoa(argNum) rows, err := r.db.Query(ctx, query, args...) @@ -1733,8 +1690,8 @@ func (r *adminRepository) SearchAuditLogs(ctx context.Context, search *models.Ad if len(logs) > limit { result.Data = logs[:limit] - lastID := logs[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(lastID) + last := logs[limit-1] + result.Pagination.NextCursor = paging.EncodeTime(last.CreatedAt, last.ID) } return result, nil @@ -1964,7 +1921,7 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m } if search.OrgID != nil { where += " AND ea.organization_id = $" + itoa(argNum) - args = append(args, *search.OrgID) + args = append(args, search.OrgID.UUID) argNum++ } @@ -1985,7 +1942,7 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m } addBefore := func(col string, v *time.Time) { if v != nil { - where += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + where += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -1994,12 +1951,12 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m // Ownership / placement if search.UserID != nil { where += " AND ea.user_id = $" + itoa(argNum) - args = append(args, *search.UserID) + args = append(args, search.UserID.UUID) argNum++ } if search.WorkerID != nil { where += " AND ea.worker_id = $" + itoa(argNum) - args = append(args, *search.WorkerID) + args = append(args, search.WorkerID.UUID) argNum++ } @@ -2058,11 +2015,7 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m addAfter("ea.last_synced_at", search.LastSyncedAfter) addBefore("ea.last_synced_at", search.LastSyncedBefore) - if search.Cursor != nil { - where += " AND ea.id < $" + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset orderCol := "ea.id" switch search.SortBy { @@ -2100,7 +2053,7 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m LEFT JOIN organizations o ON o.id = ea.organization_id ` + where + ` ` + orderBy + ` - LIMIT ` + limitParam + ` + adminLimitOffset(limitParam, offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -2131,8 +2084,7 @@ func (r *adminRepository) SearchMailboxesForAdmin(ctx context.Context, search *m } if len(items) > limit { result.Data = items[:limit] - last := items[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(last) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } // Total count for the same filter (drop the trailing LIMIT arg). diff --git a/internal/repository/pg_admin_outreach.go b/internal/repository/pg_admin_outreach.go index 4439d2533..e8f556762 100644 --- a/internal/repository/pg_admin_outreach.go +++ b/internal/repository/pg_admin_outreach.go @@ -8,7 +8,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/warmbly/warmbly/internal/models" - "github.com/warmbly/warmbly/internal/utils/paging" ) // AdminOutreachRepository is the persistence layer for the @@ -112,7 +111,7 @@ func (r *adminOutreachRepository) Search(ctx context.Context, search *models.Adm } addBefore := func(col string, v *time.Time) { if v != nil { - where += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + where += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -127,11 +126,7 @@ func (r *adminOutreachRepository) Search(ctx context.Context, search *models.Adm addAfter("m.sent_at", search.SentAtAfter) addBefore("m.sent_at", search.SentAtBefore) - if search.Cursor != nil { - where += ` AND m.id < $` + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset orderCol := "m.created_at" switch search.SortBy { @@ -163,7 +158,7 @@ func (r *adminOutreachRepository) Search(ctx context.Context, search *models.Adm LEFT JOIN users u ON u.id = m.to_user_id ` + where + ` ` + orderBy + ` - LIMIT $` + itoa(argNum) + ` + adminLimitOffset("$"+itoa(argNum), offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -199,8 +194,7 @@ func (r *adminOutreachRepository) Search(ctx context.Context, search *models.Adm } if len(items) > limit { result.Data = items[:limit] - last := items[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(last) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } countQuery := `SELECT COUNT(*) FROM admin_outreach_messages m JOIN users s ON s.id = m.sent_by LEFT JOIN users u ON u.id = m.to_user_id ` + where diff --git a/internal/repository/pg_discount.go b/internal/repository/pg_discount.go index 11a1bbbdd..795e00c6c 100644 --- a/internal/repository/pg_discount.go +++ b/internal/repository/pg_discount.go @@ -242,7 +242,7 @@ func (r *discountCodeRepository) List(ctx context.Context, search *models.AdminD } if search.PlanID != nil { whereClause += " AND EXISTS (SELECT 1 FROM discount_code_plans dcp WHERE dcp.discount_code_id = dc.id AND dcp.plan_id = $" + itoa(argNum) + ")" - args = append(args, *search.PlanID) + args = append(args, search.PlanID.UUID) argNum++ } if search.HasRedemptions { @@ -274,7 +274,7 @@ func (r *discountCodeRepository) List(ctx context.Context, search *models.AdminD } addBefore := func(col string, v *time.Time) { if v != nil { - whereClause += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + whereClause += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } diff --git a/internal/repository/pg_organization.go b/internal/repository/pg_organization.go index acf6e9816..584a4ff8a 100644 --- a/internal/repository/pg_organization.go +++ b/internal/repository/pg_organization.go @@ -10,7 +10,6 @@ import ( "github.com/jackc/pgx/v5" "github.com/jackc/pgx/v5/pgxpool" "github.com/warmbly/warmbly/internal/models" - "github.com/warmbly/warmbly/internal/utils/paging" ) // OrganizationRepository defines the interface for organization data access @@ -795,7 +794,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context if search.PlanID != nil { where += ` AND s.plan_id = $` + itoa(argNum) - args = append(args, *search.PlanID) + args = append(args, search.PlanID.UUID) argNum++ } switch search.PlanVisibility { @@ -845,7 +844,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context } addBefore := func(col string, v *time.Time) { if v != nil { - where += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + where += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -919,11 +918,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context addAfter("o.updated_at", search.UpdatedAfter) addBefore("o.updated_at", search.UpdatedBefore) - if search.Cursor != nil { - where += ` AND o.id < $` + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset orderCol := "o.created_at" switch search.SortBy { @@ -942,7 +937,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context if search.SortBy != "" && !search.SortDesc { orderDir = "ASC" } - orderBy := "ORDER BY " + orderCol + " " + orderDir + orderBy := "ORDER BY " + orderCol + " " + orderDir + ", o.id DESC" args = append(args, limit+1) @@ -956,7 +951,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context LEFT JOIN plans p ON p.id = s.plan_id` + adminOrgAcquisitionJoin + ` ` + where + ` ` + orderBy + ` - LIMIT $` + itoa(argNum) + ` + adminLimitOffset("$"+itoa(argNum), offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1007,8 +1002,7 @@ func (r *organizationRepository) SearchOrganizationsForAdmin(ctx context.Context } if len(items) > limit { result.Data = items[:limit] - last := items[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(last) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } // Total count for the same filter — drop the trailing LIMIT arg. @@ -1286,12 +1280,12 @@ func (r *organizationRepository) ListLimitRequestsForAdmin(ctx context.Context, } if search.OrgID != nil { where += " AND lr.organization_id = $" + itoa(argNum) - args = append(args, *search.OrgID) + args = append(args, search.OrgID.UUID) argNum++ } if search.SubmittedBy != nil { where += " AND lr.submitted_by = $" + itoa(argNum) - args = append(args, *search.SubmittedBy) + args = append(args, search.SubmittedBy.UUID) argNum++ } if search.Reviewed { @@ -1317,7 +1311,7 @@ func (r *organizationRepository) ListLimitRequestsForAdmin(ctx context.Context, } addBefore := func(col string, v *time.Time) { if v != nil { - where += " AND " + col + " < ($" + itoa(argNum) + " + INTERVAL '1 day')" + where += " AND " + col + " < ($" + itoa(argNum) + "::timestamptz + INTERVAL '1 day')" args = append(args, *v) argNum++ } @@ -1338,12 +1332,7 @@ func (r *organizationRepository) ListLimitRequestsForAdmin(ctx context.Context, addAfter("lr.reviewed_at", search.ReviewedAfter) addBefore("lr.reviewed_at", search.ReviewedBefore) - // Keyset on id (mirrors the org explorer; default sort is submitted_at). - if search.Cursor != nil { - where += " AND lr.id < $" + itoa(argNum) - args = append(args, *search.Cursor) - argNum++ - } + offset := search.Offset orderCol := "lr.submitted_at" switch search.SortBy { @@ -1379,7 +1368,7 @@ func (r *organizationRepository) ListLimitRequestsForAdmin(ctx context.Context, JOIN users u ON u.id = lr.submitted_by ` + where + ` ` + orderBy + ` - LIMIT $` + itoa(argNum) + ` + adminLimitOffset("$"+itoa(argNum), offset) rows, err := r.db.Query(ctx, query, args...) if err != nil { @@ -1411,8 +1400,7 @@ func (r *organizationRepository) ListLimitRequestsForAdmin(ctx context.Context, } if len(items) > limit { result.Data = items[:limit] - last := items[limit-1].ID - result.Pagination.NextCursor = paging.UUIDString(last) + result.Pagination.NextCursor = adminNextCursor(offset, limit) } // Total count for the same filter — drop the trailing LIMIT arg. diff --git a/internal/repository/placement_queries_live_test.go b/internal/repository/placement_queries_live_test.go index d121d484a..ca60c18e5 100644 --- a/internal/repository/placement_queries_live_test.go +++ b/internal/repository/placement_queries_live_test.go @@ -122,7 +122,7 @@ func TestAdminWorkerQueriesLive(t *testing.T) { } admin := NewAdminRepository(pool) - res, err := admin.ListWorkers(ctx, nil, 5) + res, err := admin.ListWorkers(ctx, 0, 5) if err != nil { t.Fatalf("admin ListWorkers: %v", err) }