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..1362f927f 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); @@ -148,8 +152,11 @@ export async function Request(config: AuthRequestConfig): Promise { // The path and nothing else about the call travels: no query string, no body, // no header. function noteFailure(config: AuthRequestConfig, failure: APIError): void { - const method = config.method?.toUpperCase() ?? "REQUEST"; + // Axios sends an omitted method as GET. + const method = config.method?.toUpperCase() ?? "GET"; 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..b4beb074d --- /dev/null +++ b/internal/repository/admin_paging_live_test.go @@ -0,0 +1,279 @@ +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("users oldest first", func(t *testing.T) { + var order []uuid.UUID + got := walk(t, func(cursor string) ([]uuid.UUID, models.Pagination) { + res, err := admin.SearchUsers(ctx, &models.AdminUserSearch{Query: f.tag, Status: "all", SortBy: "created_at", Limit: 1, Offset: offsetOf(t, cursor)}) + if err != nil { + t.Fatalf("SearchUsers: %v", err) + } + for _, u := range res.Data { + order = append(order, u.ID) + } + return order[len(order)-len(res.Data):], res.Pagination + }) + sameSet(t, "users oldest first", got, users) + if order[0] != f.user { + t.Fatalf("ascending created_at starts at %s, want the fixture's first user %s", order[0], f.user) + } + }) + + 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..b92ca891c 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,21 @@ 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" + orderBy := "ORDER BY u.created_at DESC, u.id 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" - } + dir := " ASC" if search.SortDesc { - orderBy += " DESC" + dir = " DESC" + } + switch search.SortBy { + case "created_at": + orderBy = "ORDER BY u.created_at" + dir + ", u.id" + dir + case "email": + orderBy = "ORDER BY u.email" + dir + ", u.id" + dir + case "name": + orderBy = "ORDER BY u.first_name" + dir + ", u.last_name" + dir + ", u.id" + dir } } @@ -251,7 +250,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 +285,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 +602,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 +610,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 +644,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 +666,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 +711,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 +732,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 +765,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 +999,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 +1019,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 +1050,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 +1077,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 +1116,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 +1174,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 +1189,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 +1196,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 +1232,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 +1340,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 +1397,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 +1422,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 +1465,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 +1512,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 +1600,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 +1618,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 +1629,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 +1693,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 +1924,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 +1945,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 +1954,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 +2018,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 +2056,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 +2087,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) }