From 73f6bff2ed659098350f20301a3204920e9d615f Mon Sep 17 00:00:00 2001 From: SUMAN JANA Date: Fri, 11 Sep 2026 07:19:25 +0000 Subject: [PATCH 1/4] feat: stop a browser signed in as someone else from being offered Accept on an invite it cannot accept, comparing the signed-in address with the invited one on the invite page and offering Switch account instead, and naming both addresses in the backend's 403 so the cause is visible --- internal/app/organization/service.go | 7 +++--- web/src/app/invite/page.tsx | 34 ++++++++++++++++++++++++++- web/src/lib/api/hooks/auth/useUser.ts | 5 +++- 3 files changed, 41 insertions(+), 5 deletions(-) diff --git a/internal/app/organization/service.go b/internal/app/organization/service.go index 50590f1b..a539550f 100644 --- a/internal/app/organization/service.go +++ b/internal/app/organization/service.go @@ -641,9 +641,10 @@ func (s *organizationService) GetInvitationToken(ctx context.Context, orgID, inv // acceptResolved performs the actual join given an already-loaded invitation. func (s *organizationService) acceptResolved(ctx context.Context, inv *models.OrganizationInvitation, userID uuid.UUID, email string) (*models.OrganizationMember, *errx.Error) { - // Verify email matches - if strings.ToLower(email) != strings.ToLower(inv.Email) { - return nil, errx.New(errx.Forbidden, "email does not match invitation") + // Verify email matches. Name both addresses: the usual cause is a browser + // already signed in as someone else, and a bare "does not match" hides it. + if !strings.EqualFold(email, inv.Email) { + return nil, errx.New(errx.Forbidden, "this invitation is for "+inv.Email+", but you are signed in as "+email+"; sign out and use the invited address") } // Check if invitation is expired diff --git a/web/src/app/invite/page.tsx b/web/src/app/invite/page.tsx index 56f9a9fb..8760ffd7 100644 --- a/web/src/app/invite/page.tsx +++ b/web/src/app/invite/page.tsx @@ -14,6 +14,8 @@ import useAcceptInvitation from "@/lib/api/hooks/app/organizations/useAcceptInvi import useOrganizations from "@/lib/api/hooks/app/organizations/useOrganizations"; import useSwitchOrganization from "@/lib/api/hooks/app/organizations/useSwitchOrganization"; import useAuthConfig from "@/lib/api/hooks/auth/useAuthConfig"; +import useUser from "@/lib/api/hooks/auth/useUser"; +import useLogout from "@/lib/api/hooks/auth/useLogout"; import { useAppStore } from "@/stores"; import { Logo } from "@/components/svg"; import type { AppError } from "@/lib/api/client/normalizeError"; @@ -32,6 +34,11 @@ export default function InviteAcceptPage() { const setOrganizations = useAppStore((s) => s.setOrganizations); const setCurrentOrganization = useAppStore((s) => s.setCurrentOrganization); const { config: authConfig } = useAuthConfig(); + // Who this browser is signed in as. The backend only lets the invited + // address accept, so a session for anyone else must switch first — + // otherwise Accept is a guaranteed 403. + const me = useUser(loggedIn); + const logout = useLogout(); const nextPath = `/invite?token=${encodeURIComponent(token ?? "")}`; // The invited address has to travel too: the backend only accepts a signup @@ -44,6 +51,14 @@ export default function InviteAcceptPage() { (invitedEmail ? `&email=${encodeURIComponent(invitedEmail)}` : "") + `&next=${encodeURIComponent(nextPath)}`; const signupClosed = authConfig.registration === "true"; + const signedInEmail = me.data?.email ?? ""; + const wrongAccount = + loggedIn && !!signedInEmail && !!invitedEmail && signedInEmail.toLowerCase() !== invitedEmail.toLowerCase(); + + async function onSwitchAccount() { + await logout.mutateAsync(); + navigate(`/auth/login?next=${encodeURIComponent(nextPath)}`, { replace: true }); + } async function onAccept() { if (!token) return; @@ -132,7 +147,24 @@ export default function InviteAcceptPage() { )} - {loggedIn ? ( + {wrongAccount ? ( +
+

+ You're signed in as {signedInEmail}, but this + invitation is for {invitedEmail}. Sign out, then sign in + or create an account with the invited address. +

+ +
+ ) : loggedIn ? ( + )} + + Manage contacts + + ); } diff --git a/web/src/components/app/unibox/ConversationItem.tsx b/web/src/components/app/unibox/ConversationItem.tsx index 983e7e36..408af0be 100644 --- a/web/src/components/app/unibox/ConversationItem.tsx +++ b/web/src/components/app/unibox/ConversationItem.tsx @@ -9,6 +9,7 @@ import type UniboxEmail from "@/lib/api/models/app/unibox/UniboxEmail"; import { useAppStore } from "@/stores"; import { useResourceViewers } from "@/hooks/PresenceProvider"; import { cn } from "@/lib/utils"; +import { nameFromAddr } from "@/lib/helper/emailAddress"; function relative(d: Date): string { const diff = Date.now() - d.getTime(); @@ -24,9 +25,7 @@ function relative(d: Date): string { function fromName(s: string): string { if (!s) return "Unknown sender"; - const m = s.match(/^"?([^"<]+)"?\s*<.+>$/); - if (m) return m[1].trim(); - return s.replace(/<.+>/, "").trim() || s; + return nameFromAddr(s); } function initials(s: string): string { diff --git a/web/src/components/app/unibox/InsertBookingLink.tsx b/web/src/components/app/unibox/InsertBookingLink.tsx index 7082fc5a..ce3c8730 100644 --- a/web/src/components/app/unibox/InsertBookingLink.tsx +++ b/web/src/components/app/unibox/InsertBookingLink.tsx @@ -7,12 +7,7 @@ import { CalendarPlusIcon } from "lucide-react"; import toast from "react-hot-toast"; import useIntegrationConnections from "@/lib/api/hooks/app/integrations/useIntegrationConnections"; import { bookingURL, prefilledBookingURL } from "@/lib/api/models/app/integrations/Integration"; - -function bareEmail(s: string): string { - const m = s.match(/<([^>]+)>/); - if (m) return m[1].trim(); - return s.trim(); -} +import { bareEmail } from "@/lib/helper/emailAddress"; export default function InsertBookingLink({ email, diff --git a/web/src/components/app/unibox/MessageBubble.tsx b/web/src/components/app/unibox/MessageBubble.tsx index ad7244e3..9a3f3149 100644 --- a/web/src/components/app/unibox/MessageBubble.tsx +++ b/web/src/components/app/unibox/MessageBubble.tsx @@ -18,6 +18,7 @@ import { AlertCircleIcon, CornerUpLeftIcon, ForwardIcon, Loader2Icon } from "luc import EmailBody from "./EmailBody"; import useUniboxEmail from "@/lib/api/hooks/app/unibox/useUniboxEmail"; import type UniboxEmail from "@/lib/api/models/app/unibox/UniboxEmail"; +import { nameFromAddr, wrappedEmail } from "@/lib/helper/emailAddress"; interface MessageBubbleProps { email: UniboxEmail; @@ -27,17 +28,8 @@ interface MessageBubbleProps { onForward?: () => void; } -function fromName(s: string): string { - const m = s.match(/^"?([^"<]+)"?\s*<.+>$/); - if (m) return m[1].trim(); - return s.replace(/<.+>/, "").trim() || s; -} - -function fromAddr(s: string): string | null { - const m = s.match(/<([^>]+)>/); - if (m) return m[1].trim(); - return null; -} +const fromName = nameFromAddr; +const fromAddr = wrappedEmail; function initials(s: string): string { const name = fromName(s); diff --git a/web/src/components/app/unibox/ReplyComposer.tsx b/web/src/components/app/unibox/ReplyComposer.tsx index 8270eb19..46b7d4e1 100644 --- a/web/src/components/app/unibox/ReplyComposer.tsx +++ b/web/src/components/app/unibox/ReplyComposer.tsx @@ -53,6 +53,7 @@ import { } from "@/components/ui/popover-menu"; import { cn } from "@/lib/utils"; import { plainToHtml } from "@/lib/email/body"; +import { bareEmail, nameFromAddr } from "@/lib/helper/emailAddress"; export type ReplyMode = "reply" | "forward"; @@ -138,17 +139,6 @@ function looksLikeEmail(s: string): boolean { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(s.trim()); } -function nameFromAddr(s: string): string { - const m = s.match(/^"?([^"<]+)"?\s*<.+>$/); - if (m) return m[1].trim(); - return s.replace(/<.+>/, "").trim() || s; -} - -function bareEmail(s: string): string { - const m = s.match(/<([^>]+)>/); - if (m) return m[1].trim(); - return s.trim(); -} // Derive composer defaults from the message the user explicitly chose // to reply to (or forward). Reply takes the message's "from" as the diff --git a/web/src/components/app/unibox/ThreadView.tsx b/web/src/components/app/unibox/ThreadView.tsx index 641928b3..d69dbb6d 100644 --- a/web/src/components/app/unibox/ThreadView.tsx +++ b/web/src/components/app/unibox/ThreadView.tsx @@ -41,6 +41,8 @@ import { CategoryChip } from "@/components/app/contacts/CategoryPicker"; import { SectionBar } from "@/components/layout/Page"; import useThread from "@/lib/api/hooks/app/unibox/useThread"; import useMarkSeen from "@/lib/api/hooks/app/unibox/useMarkSeen"; +import useMoveFolder from "@/lib/api/hooks/app/unibox/useMoveFolder"; +import { bareEmail, nameFromAddr, wrappedEmail } from "@/lib/helper/emailAddress"; import useThreadLabels from "@/lib/api/hooks/app/unibox/useThreadLabels"; import useThreadScheduled from "@/lib/api/hooks/app/unibox/useThreadScheduled"; import cancelScheduled from "@/lib/api/client/app/unibox/cancelScheduled"; @@ -263,6 +265,27 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { markSeenMutate({ ids: unseenIds, threadId }); }, [threadId, q.data, markSeenMutate]); + // Header actions. Each one closes the thread: the effect above would + // otherwise re-mark an "unread" thread as seen on the next refetch, and a + // trashed/archived thread has left the list the reader is looking at. + const moveFolder = useMoveFolder(); + const setSelectedThreadId = useAppStore((s) => s.setSelectedThreadId); + const threadIds = () => (q.data?.data ?? []).map((m) => m.id); + const markUnread = () => { + markSeenMutate({ ids: threadIds(), seen: false }); + setSelectedThreadId(null); + }; + const fileThread = (folder: "trash" | "archive") => { + toast + .promise(moveFolder.mutateAsync({ ids: threadIds(), folder }), { + loading: folder === "trash" ? "Deleting…" : "Archiving…", + success: folder === "trash" ? "Moved to Trash" : "Archived", + error: folder === "trash" ? "Couldn't delete" : "Couldn't archive", + }) + .then(() => setSelectedThreadId(null)) + .catch(() => undefined); + }; + const snooze = useMutation({ mutationFn: (until: Date) => snoozeThread({ thread_id: threadId, snoozed_until: until.toISOString() }), @@ -340,15 +363,20 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { // our own mailbox. Addresses arrive as "Name " or bare "addr"; reduce // to the bare address so the comparison + the CRM panel lookup both work. const mailboxEmail = mailbox?.email?.toLowerCase(); - const bareAddr = (s: string) => { - const m = s.match(/<([^>]+)>/); - return (m ? m[1] : s).trim(); - }; - const contactEmail = + const contactFrom = messages - .map((m) => bareAddr(m.from)) - .find((e) => e && e.toLowerCase() !== mailboxEmail) ?? - bareAddr(messages[0]?.from ?? ""); + .map((m) => m.from) + .find((f) => { + const e = bareEmail(f); + return e && e.toLowerCase() !== mailboxEmail; + }) ?? (messages[0]?.from ?? ""); + const contactEmail = bareEmail(contactFrom); + // Display name from the From header, so an "Add as contact" from the + // panel does not create a nameless row. Empty when the header is bare. + const contactName = + wrappedEmail(contactFrom) && nameFromAddr(contactFrom) !== contactEmail + ? nameFromAddr(contactFrom) + : ""; const submitCustomSnooze = () => { if (!customValue) return; @@ -505,15 +533,18 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { } + onClick={markUnread} /> } + onClick={() => fileThread("archive")} /> } + onClick={() => fileThread("trash")} /> @@ -529,15 +560,20 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { } + onSelect={markUnread} > Mark as unread - }> + } + onSelect={() => fileThread("archive")} + > Archive thread } + onSelect={() => fileThread("trash")} > Delete thread @@ -640,6 +676,7 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { {crmOpen && ( setCrmOpen(false)} /> diff --git a/web/src/components/app/unibox/compose/ComposeHistoryPanel.tsx b/web/src/components/app/unibox/compose/ComposeHistoryPanel.tsx index 513dbf94..a98e7f3a 100644 --- a/web/src/components/app/unibox/compose/ComposeHistoryPanel.tsx +++ b/web/src/components/app/unibox/compose/ComposeHistoryPanel.tsx @@ -17,6 +17,7 @@ import type { UniboxListRow } from "@/lib/api/client/app/unibox/searchIncoming"; import { SearchInput } from "@/components/ui/field"; import { useAppStore } from "@/stores"; import { cn } from "@/lib/utils"; +import { bareEmail } from "@/lib/helper/emailAddress"; type HistoryTab = "all" | "sent"; @@ -29,12 +30,6 @@ interface ComposeHistoryPanelProps { affinityLine?: string; } -function bareEmail(s: string): string { - const m = s.match(/<([^>]+)>/); - if (m) return m[1].trim(); - return s.trim(); -} - function formatWhen(iso: string): string { const d = new Date(iso); const now = new Date(); diff --git a/web/src/components/app/unibox/compose/ComposeWindow.tsx b/web/src/components/app/unibox/compose/ComposeWindow.tsx index 48de188b..06bcda2f 100644 --- a/web/src/components/app/unibox/compose/ComposeWindow.tsx +++ b/web/src/components/app/unibox/compose/ComposeWindow.tsx @@ -66,6 +66,7 @@ import type { AppError } from "@/lib/api/client/normalizeError"; import buildError from "@/lib/helper/buildError"; import { cn } from "@/lib/utils"; import { plainToHtml } from "@/lib/email/body"; +import { bareEmail } from "@/lib/helper/emailAddress"; const MAX_BODY_LEN = 4000; const MAX_SCHEDULE_MS = 29 * 24 * 60 * 60 * 1000; @@ -74,12 +75,6 @@ function looksLikeEmail(s: string): boolean { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(s.trim()); } -function bareEmail(s: string): string { - const m = s.match(/<([^>]+)>/); - if (m) return m[1].trim(); - return s.trim(); -} - function offsetHours(h: number): Date { const d = new Date(); d.setHours(d.getHours() + h); diff --git a/web/src/lib/api/client/app/unibox/moveFolder.ts b/web/src/lib/api/client/app/unibox/moveFolder.ts new file mode 100644 index 00000000..9f31940d --- /dev/null +++ b/web/src/lib/api/client/app/unibox/moveFolder.ts @@ -0,0 +1,13 @@ +import Request from "../../Request"; + +// PATCH /unibox/folder re-files messages into one canonical folder. Delete in +// the thread header is folder "trash", Archive is "archive". Store-side only: +// the provider copy stays put. +export default async function moveFolder(data: { ids: string[]; folder: "trash" | "archive" | "inbox" }): Promise { + return await Request({ + method: "PATCH", + url: `/unibox/folder`, + data: { email_ids: data.ids, folder: data.folder }, + authorization: true, + }) +} diff --git a/web/src/lib/api/hooks/app/unibox/useMoveFolder.ts b/web/src/lib/api/hooks/app/unibox/useMoveFolder.ts new file mode 100644 index 00000000..ddb22de3 --- /dev/null +++ b/web/src/lib/api/hooks/app/unibox/useMoveFolder.ts @@ -0,0 +1,15 @@ +import { useMutation, useQueryClient } from "@tanstack/react-query"; +import moveFolder from "@/lib/api/client/app/unibox/moveFolder"; + +export default function useMoveFolder() { + const queryClient = useQueryClient(); + + return useMutation({ + mutationFn: (data: { ids: string[]; folder: "trash" | "archive" | "inbox" }) => moveFolder(data), + onSuccess: () => { + queryClient.invalidateQueries({ + queryKey: ["unibox"], + }) + } + }) +} diff --git a/web/src/lib/helper/emailAddress.test.ts b/web/src/lib/helper/emailAddress.test.ts new file mode 100644 index 00000000..752effe6 --- /dev/null +++ b/web/src/lib/helper/emailAddress.test.ts @@ -0,0 +1,28 @@ +import { describe, expect, it } from "vitest"; +import { bareEmail, nameFromAddr, wrappedEmail } from "./emailAddress"; + +// The three shapes the API actually stores (see the header comment), pinned +// so the reply composer's seeded To passes its own validator for each. +describe("emailAddress", () => { + it("parses the IMAP sync's parenthesised form", () => { + const s = "Centous Support (support@centous.com)"; + expect(bareEmail(s)).toBe("support@centous.com"); + expect(nameFromAddr(s)).toBe("Centous Support"); + }); + + it("parses the RFC angle-bracket form, quoted or not", () => { + expect(bareEmail('"Jane Doe" ')).toBe("jane@x.com"); + expect(nameFromAddr('"Jane Doe" ')).toBe("Jane Doe"); + expect(nameFromAddr("Jane Doe ")).toBe("Jane Doe"); + }); + + it("passes a bare address through and names it by itself", () => { + expect(wrappedEmail("jane@x.com")).toBeNull(); + expect(bareEmail(" jane@x.com ")).toBe("jane@x.com"); + expect(nameFromAddr("jane@x.com")).toBe("jane@x.com"); + }); + + it("falls back to the address when the name is empty", () => { + expect(nameFromAddr(" (noreply-dmarc-support@google.com)")).toBe("noreply-dmarc-support@google.com"); + }); +}); diff --git a/web/src/lib/helper/emailAddress.ts b/web/src/lib/helper/emailAddress.ts new file mode 100644 index 00000000..dca82ec9 --- /dev/null +++ b/web/src/lib/helper/emailAddress.ts @@ -0,0 +1,24 @@ +// One parser for the header-style addresses the API hands the UI. They come in +// three shapes: RFC "Name " (Gmail/Graph sync), "Name (addr)" (the IMAP +// sync, internal/client/smtpimap/imap/address.go), or a bare "addr". Every +// component used to carry its own angle-bracket-only copy, so an IMAP sender +// seeded the reply composer with "Name (addr)" and Send stayed disabled. +const WRAPPED = /[<(]\s*([^<>()\s]+@[^<>()\s]+)\s*[>)]\s*$/; + +// The address inside the brackets, or null when there are none. +export function wrappedEmail(s: string): string | null { + const m = s.match(WRAPPED); + return m ? m[1] : null; +} + +// The bare address: the bracketed one when present, else the trimmed input. +export function bareEmail(s: string): string { + return wrappedEmail(s) ?? s.trim(); +} + +// The display name in front of the brackets; the address when there is none. +export function nameFromAddr(s: string): string { + const m = s.match(WRAPPED); + if (!m) return s.trim(); + return s.slice(0, m.index).replace(/"/g, "").trim() || m[1]; +} From da4b89da0ba641cd502be9e896dd4bece5eced24 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Fri, 11 Sep 2026 03:23:19 -0700 Subject: [PATCH 3/4] feat: split a unibox message's provider placement into its own provider_folder column (migration 000146) so Archive and Delete in the thread header survive the next sync without the sync losing the ability to follow a real provider move, and narrow PATCH /unibox/folder to inbox/archive/trash behind the unibox feature gate with an audit entry so the move reaches every teammate's list live --- docs/content/docs/api/endpoints.mdx | 3 + docs/public/openapi.json | 110 ++++++++++ internal/api/handler/unibox.go | 16 +- internal/app/consumer/event_update_email.go | 22 +- internal/app/unibox/seen.go | 12 +- internal/errx/common.go | 3 + .../000146_unibox_provider_folder.down.sql | 8 + .../000146_unibox_provider_folder.up.sql | 35 ++++ internal/models/unibox.go | 76 +++++-- internal/models/unibox_folder_sync_test.go | 78 +++++++ internal/repository/pg_unibox.go | 22 +- .../unibox_move_folder_live_test.go | 198 ++++++++++++++++++ 12 files changed, 539 insertions(+), 44 deletions(-) create mode 100644 internal/infrastructure/db/migrations/000146_unibox_provider_folder.down.sql create mode 100644 internal/infrastructure/db/migrations/000146_unibox_provider_folder.up.sql create mode 100644 internal/models/unibox_folder_sync_test.go create mode 100644 internal/repository/unibox_move_folder_live_test.go diff --git a/docs/content/docs/api/endpoints.mdx b/docs/content/docs/api/endpoints.mdx index 4e3c51fb..e0ec97c6 100644 --- a/docs/content/docs/api/endpoints.mdx +++ b/docs/content/docs/api/endpoints.mdx @@ -185,6 +185,7 @@ A source's `column_mapping` is validated when the source is written, not on its | GET | `/unibox/thread` | `READ_UNIBOX` | | GET | `/unibox/:id` | `READ_UNIBOX` | | PATCH | `/unibox/seen` | `WRITE_UNIBOX` | +| PATCH | `/unibox/folder` | `WRITE_UNIBOX` | | POST | `/unibox/reply` | `WRITE_UNIBOX` | | POST | `/unibox/reply/draft` | `READ_UNIBOX` | | GET | `/unibox/compose/candidates` | `READ_UNIBOX` | @@ -197,6 +198,8 @@ A source's `column_mapping` is validated when the source is written, not on its | POST | `/unibox/agent-drafts/:id/approve` | `WRITE_UNIBOX` | | POST | `/unibox/agent-drafts/:id/discard` | `WRITE_UNIBOX` | +`PATCH /unibox/folder` re-files up to 500 messages into `inbox`, `archive` or `trash` at once. The move is Warmbly's own: the copy at the mail provider stays where it is, and the next sync will not undo it, because the provider's placement is tracked separately and followed only when the provider itself moves the message. `sent`, `drafts` and `spam` are placements a provider reaches rather than somewhere a person files mail, so they are rejected with a `400`. See [filing a conversation](/guides/unibox/#filing-a-conversation). + `POST /unibox/reply/draft` returns an AI-drafted reply (it never sends) grounded in the thread, the contact, and your [voice profile](/guides/unibox/#ai-reply-drafts). It charges AI credits; see [AI credits](/guides/ai-credits/). `POST /unibox/compose/draft` returns a grounded AI draft for a new email (it never sends): the recipient's contact record, correspondence history, and the workspace voice profile feed the prompt, and the response carries either `text` or a clarifying `question` plus a `grounding` report. Charges AI credits like the reply draft; see [AI credits](/guides/ai-credits/). diff --git a/docs/public/openapi.json b/docs/public/openapi.json index a30a61ec..1ed541bd 100644 --- a/docs/public/openapi.json +++ b/docs/public/openapi.json @@ -8215,6 +8215,88 @@ } } }, + "/unibox/folder": { + "patch": { + "operationId": "unibox_move_folder", + "summary": "Move messages between folders", + "description": "Re-files a batch of messages into Inbox, Archive or Trash, org-wide. Up to 500 ids per call.\n\nThis is a move in Warmbly only. The copy at the mail provider stays where it is, and a later sync will not undo the move: Warmbly tracks the provider's own placement separately and follows it only when the provider itself moves the message. `sent`, `drafts` and `spam` are placements the provider reaches, so they are rejected here with a `400`.", + "tags": [ + "unibox" + ], + "security": [ + { + "bearerAuth": [] + } + ], + "parameters": [ + { + "$ref": "#/components/parameters/IdempotencyKey" + } + ], + "requestBody": { + "required": true, + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UniboxMoveFolderRequest" + } + } + } + }, + "responses": { + "200": { + "description": "Echoes the request back.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/UniboxMoveFolderRequest" + } + } + } + }, + "400": { + "description": "Invalid body, more than 500 ids, a folder outside inbox/archive/trash, or no organization selected.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "401": { + "description": "Missing or invalid credentials.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "403": { + "description": "Missing scope/permission, or organization lacks unified-inbox access.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + }, + "429": { + "description": "Rate limited.", + "content": { + "application/json": { + "schema": { + "$ref": "#/components/schemas/Error" + } + } + } + } + } + } + }, "/unibox/reply": { "post": { "operationId": "unibox_reply", @@ -23777,6 +23859,34 @@ } } }, + "UniboxMoveFolderRequest": { + "type": "object", + "description": "Also the echoed response body.", + "required": [ + "email_ids", + "folder" + ], + "properties": { + "email_ids": { + "type": "array", + "items": { + "type": "string", + "format": "uuid" + }, + "maxItems": 500, + "description": "Message UUIDs to move (max 500)." + }, + "folder": { + "type": "string", + "enum": [ + "inbox", + "archive", + "trash" + ], + "description": "Destination folder. Anything else is a `400`." + } + } + }, "UniboxReplyRequest": { "type": "object", "required": [ diff --git a/internal/api/handler/unibox.go b/internal/api/handler/unibox.go index a5180c0c..5c649a33 100644 --- a/internal/api/handler/unibox.go +++ b/internal/api/handler/unibox.go @@ -387,9 +387,15 @@ func (h *Handler) UniboxMarkSeen(c *gin.Context) { c.JSON(http.StatusOK, resp) } -// UniboxMoveFolder re-files messages (Delete = trash, Archive = archive). +// UniboxMoveFolder re-files messages (Archive = archive, Delete = trash, +// Move to inbox = inbox). Org-scoped like /seen: the inbox is shared, so any +// member with unibox access may file it. Naturally idempotent, so no +// Idempotency-Key: the body names the destination, not a delta. // PATCH /unibox/folder func (h *Handler) UniboxMoveFolder(c *gin.Context) { + if !h.gateUnibox(c) { + return + } orgID := middleware.GetOrganizationID(c) if orgID == nil { errx.Handle(c, errx.ErrUser) @@ -408,6 +414,14 @@ func (h *Handler) UniboxMoveFolder(c *gin.Context) { return } + // Audited so the spine broadcasts it: a teammate looking at the same list + // has to lose the thread too, and there is no sync event behind this one. + h.auditOrg(c, models.AuditActionUpdate, models.AuditEntityUnibox, nil, nil, map[string]string{ + "action": "move_folder", + "folder": data.Folder, + "messages": strconv.Itoa(len(data.EmailIDs)), + }) + c.JSON(http.StatusOK, resp) } diff --git a/internal/app/consumer/event_update_email.go b/internal/app/consumer/event_update_email.go index 0e49522f..23e98b28 100644 --- a/internal/app/consumer/event_update_email.go +++ b/internal/app/consumer/event_update_email.go @@ -35,15 +35,14 @@ func (s *JobsService) HandleUpdateEmail(ctx context.Context, e *models.JobEventE if e.FolderPath != "" && email.FolderPath != e.FolderPath { updateData.FolderPath = &e.FolderPath } - // A folder move follows the provider. Events from workers predating the - // field carry "", which keeps the stored value. Delete/Archive in the - // thread header only re-file the row here, so a later flag change on the - // provider (still reporting inbox) must not pull the message back out. - localMove := (email.Folder == models.FolderTrash || email.Folder == models.FolderArchive) && - e.Folder == models.FolderInbox - followProvider := models.ValidFolder(e.Folder) && !localMove - if followProvider && email.Folder != e.Folder { - updateData.Folder = &e.Folder + // The store follows the provider only when the provider itself moved the + // message; a scan that keeps naming the same folder leaves local filing be. + folder, provider, providerMoved := models.ResolveFolderSync(email.Folder, email.ProviderFolder, e.Folder) + if providerMoved { + updateData.ProviderFolder = &provider + if folder != email.Folder { + updateData.Folder = &folder + } } if err := s.UniboxRepository.UpdateEntry(ctx, e.UserID, e.EmailID, e.ID, &updateData); err != nil { @@ -54,8 +53,9 @@ func (s *JobsService) HandleUpdateEmail(ctx context.Context, e *models.JobEventE email.UID = e.UID email.Mailbox = e.Mailbox email.ModSeq = e.ModSeq - if followProvider { - email.Folder = e.Folder + if providerMoved { + email.Folder = folder + email.ProviderFolder = provider } s.publishEmailUpdated(ctx, e.UserID, email) return nil diff --git a/internal/app/unibox/seen.go b/internal/app/unibox/seen.go index 2194d8c9..bc816eea 100644 --- a/internal/app/unibox/seen.go +++ b/internal/app/unibox/seen.go @@ -47,15 +47,17 @@ func (s *uniboxService) MarkSeenBulk(ctx context.Context, orgID uuid.UUID, data return data, nil } -// MoveFolderBulk backs Delete (trash) and Archive in the thread header. -// ponytail: store-side only; the provider copy stays where it is. A -// provider-side move needs a worker event per client (IMAP/Gmail/Graph). +// MoveFolderBulk backs Archive, Delete and Move to inbox in the thread header. +// Store-side only: the provider copy stays where it is, and provider_folder is +// left alone so the sync can still tell a real provider move from a flag scan. func (s *uniboxService) MoveFolderBulk(ctx context.Context, orgID uuid.UUID, data *models.MoveFolder) (*models.MoveFolder, *errx.Error) { if len(data.EmailIDs) > 500 { return nil, errx.ErrSeenMax } - if !models.ValidFolder(data.Folder) { - return nil, errx.ErrUniboxFolder + // Only the three a user can file into. sent/drafts/spam are verdicts the + // provider reaches, and accepting them here would let a caller forge one. + if !models.FilableFolder(data.Folder) { + return nil, errx.ErrUniboxFilableFolder } if err := s.uniboxRepository.MoveToFolderBulk(ctx, orgID, data.EmailIDs, data.Folder); err != nil { errs.CaptureException(err) diff --git a/internal/errx/common.go b/internal/errx/common.go index 063c141e..bc1081d4 100644 --- a/internal/errx/common.go +++ b/internal/errx/common.go @@ -185,6 +185,9 @@ var ( // Folder scoping (unibox sidebar). ErrUniboxFolder = New(BadRequest, "Folder must be one of inbox, sent, drafts, archive, spam, trash.") ErrSeenFolderAndIDs = New(BadRequest, "Provide either email_ids or folder, not both.") + // Filing a message is narrower than scoping a list: the other three are + // verdicts the provider reaches, not somewhere a user puts mail. + ErrUniboxFilableFolder = New(BadRequest, "Folder must be one of inbox, archive, trash.") // Servers ErrIPAddr = New(BadRequest, "Invalid IP Address.") diff --git a/internal/infrastructure/db/migrations/000146_unibox_provider_folder.down.sql b/internal/infrastructure/db/migrations/000146_unibox_provider_folder.down.sql new file mode 100644 index 00000000..419584cc --- /dev/null +++ b/internal/infrastructure/db/migrations/000146_unibox_provider_folder.down.sql @@ -0,0 +1,8 @@ +-- Back to one folder column. Any message the user filed in Warmbly rather +-- than at the provider keeps that placement until the next provider move +-- overwrites it, which is the pre-split behaviour. +ALTER TABLE public.unibox_emails + DROP CONSTRAINT IF EXISTS unibox_emails_provider_folder_check; + +ALTER TABLE public.unibox_emails + DROP COLUMN IF EXISTS provider_folder; diff --git a/internal/infrastructure/db/migrations/000146_unibox_provider_folder.up.sql b/internal/infrastructure/db/migrations/000146_unibox_provider_folder.up.sql new file mode 100644 index 00000000..930013ec --- /dev/null +++ b/internal/infrastructure/db/migrations/000146_unibox_provider_folder.up.sql @@ -0,0 +1,35 @@ +-- Where the PROVIDER has the message, tracked apart from where Warmbly shows +-- it, so the two can disagree. +-- +-- Until now unibox_emails.folder was both at once: the sync wrote the +-- provider's placement into it and every read treated it as the placement. +-- That is fine while the provider is the only thing that files mail, and it +-- stops being fine the moment the thread header can Archive or Delete a +-- conversation. Those actions move the message here and not at the provider, +-- so the next flag scan of the provider's INBOX reports inbox again and pulls +-- it straight back out. +-- +-- Suppressing "provider says inbox" outright would be worse: it is also what +-- an ordinary un-archive at the provider looks like, and refusing it would +-- strand the message here forever. With both values stored, the rule is +-- exact. The store follows the provider only when the PROVIDER's folder +-- actually changed from the one last observed; a scan that keeps reporting +-- the same folder changes nothing, and a local filing survives it. +-- +-- Cost note: unibox_emails holds every synced message, so the backfill below +-- is the expensive part of this migration. The backend applies migrations at +-- boot inside one transaction and blocks until they finish, so deploy this in +-- a window rather than alongside traffic. ADD COLUMN with a constant DEFAULT +-- is metadata-only on PG 11+ and is not itself a rewrite. +ALTER TABLE public.unibox_emails + ADD COLUMN provider_folder text NOT NULL DEFAULT ''; + +ALTER TABLE public.unibox_emails + ADD CONSTRAINT unibox_emails_provider_folder_check + CHECK (provider_folder IN ('', 'inbox', 'sent', 'drafts', 'archive', 'spam', 'trash')); + +-- Every existing row was filed by the provider and by nothing else, so the +-- two values start out equal and no historical message reads as locally +-- filed. '' stays reachable for a row written by a consumer that predates +-- this column; the handler treats it as "never observed" and adopts. +UPDATE public.unibox_emails SET provider_folder = folder; diff --git a/internal/models/unibox.go b/internal/models/unibox.go index 7f574269..1de913ed 100644 --- a/internal/models/unibox.go +++ b/internal/models/unibox.go @@ -105,26 +105,31 @@ type EmailMessageStoreData struct { // Folder is the canonical folder (see the Folder* constants) the message // was in at sync time. Empty on events from workers predating the field; // the consumer normalizes before storing. - Folder string `json:"folder,omitempty"` - ThreadID string `json:"thread_id"` - MessageID string `json:"message_id"` - GmailID string `json:"gmail_id"` - ParentID string `json:"parent_id"` - UID uint32 `json:"uid"` - ModSeq uint64 `json:"mod_seq"` - Flags []string `json:"flags"` - BCC []string `json:"bcc"` - CC []string `json:"cc"` - FromAddr []string `json:"from_addr"` - InReplyTo []string `json:"in_reply_to"` - ReplyTo []string `json:"reply_to"` - ToAddr []string `json:"to_addr"` - Subject string `json:"subject"` - Size int64 `json:"size"` - InternalDate time.Time `json:"internal_date"` - SentDate time.Time `json:"sent_date"` - Snippet string `json:"snippet"` - Seen bool `json:"seen"` + Folder string `json:"folder,omitempty"` + // ProviderFolder is where the PROVIDER last reported the message, which + // Folder stops tracking once the user files the message in Warmbly. The + // two are compared to tell a real provider move from a flag scan that + // keeps naming the folder the provider still has it in. + ProviderFolder string `json:"provider_folder,omitempty"` + ThreadID string `json:"thread_id"` + MessageID string `json:"message_id"` + GmailID string `json:"gmail_id"` + ParentID string `json:"parent_id"` + UID uint32 `json:"uid"` + ModSeq uint64 `json:"mod_seq"` + Flags []string `json:"flags"` + BCC []string `json:"bcc"` + CC []string `json:"cc"` + FromAddr []string `json:"from_addr"` + InReplyTo []string `json:"in_reply_to"` + ReplyTo []string `json:"reply_to"` + ToAddr []string `json:"to_addr"` + Subject string `json:"subject"` + Size int64 `json:"size"` + InternalDate time.Time `json:"internal_date"` + SentDate time.Time `json:"sent_date"` + Snippet string `json:"snippet"` + Seen bool `json:"seen"` // BodyText is a bounded plain-text rendering of the message, carried on the // new-email event so the consumer can make the message findable by what it // says. The full body goes to object storage, never here. @@ -212,6 +217,32 @@ func ValidFolder(f string) bool { return false } +// ResolveFolderSync decides what one sync event does to a message's placement. +// +// reported is compared against the folder the PROVIDER was last seen to have +// the message in, never against the stored folder. A message the user filed in +// Warmbly still turns up in the provider's inbox on every scan, and comparing +// against the stored folder would read each of those as a move back and undo +// them. An empty storedProvider is a row written before the column existed +// (migration 000146), so it adopts. An invalid reported folder is a worker +// predating the field and changes nothing. +func ResolveFolderSync(storedFolder, storedProvider, reported string) (folder, provider string, changed bool) { + if !ValidFolder(reported) || reported == storedProvider { + return storedFolder, storedProvider, false + } + return reported, reported, true +} + +// FilableFolder reports whether f is a folder a user may move mail INTO from +// the unibox. sent/drafts/spam are provider verdicts, never a user's choice. +func FilableFolder(f string) bool { + switch f { + case FolderInbox, FolderArchive, FolderTrash: + return true + } + return false +} + // NormalizeFolder resolves the folder to persist for a message: the worker's // value when it sent a valid one, otherwise a flag-derived fallback so events // from workers predating the folder field still file spam and drafts sanely. @@ -297,8 +328,9 @@ type MarkSeen struct { Seen bool `json:"seen"` } -// MoveFolder re-files messages into one canonical folder (Delete = trash, -// Archive = archive). Store-side only: the provider copy is not moved. +// MoveFolder re-files messages into one canonical folder (Archive = archive, +// Delete = trash, Move to inbox = inbox). Store-side only: the provider copy +// is not moved, so the message stays where it is in the user's mail client. type MoveFolder struct { EmailIDs []uuid.UUID `json:"email_ids"` Folder string `json:"folder"` diff --git a/internal/models/unibox_folder_sync_test.go b/internal/models/unibox_folder_sync_test.go new file mode 100644 index 00000000..38374771 --- /dev/null +++ b/internal/models/unibox_folder_sync_test.go @@ -0,0 +1,78 @@ +package models + +import "testing" + +// The rule that makes Archive/Delete in the thread header survive a sync. +// Every case below is a real event shape the consumer sees; the third is the +// one the feature exists for, and the fourth is the one a naive guard breaks. +func TestResolveFolderSync(t *testing.T) { + cases := []struct { + name string + storedFolder string + storedProvider string + reported string + wantFolder string + wantProvider string + wantChanged bool + }{ + { + name: "a worker predating the folder field changes nothing", + storedFolder: FolderInbox, storedProvider: FolderInbox, reported: "", + wantFolder: FolderInbox, wantProvider: FolderInbox, wantChanged: false, + }, + { + name: "an unknown folder changes nothing", + storedFolder: FolderInbox, storedProvider: FolderInbox, reported: "starred", + wantFolder: FolderInbox, wantProvider: FolderInbox, wantChanged: false, + }, + { + name: "a flag scan does not undo a message filed in Warmbly", + storedFolder: FolderTrash, storedProvider: FolderInbox, reported: FolderInbox, + wantFolder: FolderTrash, wantProvider: FolderInbox, wantChanged: false, + }, + { + name: "the provider moving it out of the inbox still wins", + storedFolder: FolderArchive, storedProvider: FolderInbox, reported: FolderSpam, + wantFolder: FolderSpam, wantProvider: FolderSpam, wantChanged: true, + }, + { + name: "un-archiving at the provider reaches a locally archived message", + storedFolder: FolderArchive, storedProvider: FolderArchive, reported: FolderInbox, + wantFolder: FolderInbox, wantProvider: FolderInbox, wantChanged: true, + }, + { + name: "an ordinary provider move is followed", + storedFolder: FolderInbox, storedProvider: FolderInbox, reported: FolderTrash, + wantFolder: FolderTrash, wantProvider: FolderTrash, wantChanged: true, + }, + { + name: "a row written before provider_folder existed adopts", + storedFolder: FolderInbox, storedProvider: "", reported: FolderInbox, + wantFolder: FolderInbox, wantProvider: FolderInbox, wantChanged: true, + }, + } + + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + folder, provider, changed := ResolveFolderSync(tc.storedFolder, tc.storedProvider, tc.reported) + if folder != tc.wantFolder || provider != tc.wantProvider || changed != tc.wantChanged { + t.Fatalf("ResolveFolderSync(%q, %q, %q) = (%q, %q, %v), want (%q, %q, %v)", + tc.storedFolder, tc.storedProvider, tc.reported, + folder, provider, changed, tc.wantFolder, tc.wantProvider, tc.wantChanged) + } + }) + } +} + +func TestFilableFolderRefusesProviderVerdicts(t *testing.T) { + for _, f := range []string{FolderInbox, FolderArchive, FolderTrash} { + if !FilableFolder(f) { + t.Errorf("FilableFolder(%q) = false, want true", f) + } + } + for _, f := range []string{FolderSent, FolderDrafts, FolderSpam, "", "Inbox"} { + if FilableFolder(f) { + t.Errorf("FilableFolder(%q) = true, want false", f) + } + } +} diff --git a/internal/repository/pg_unibox.go b/internal/repository/pg_unibox.go index 734f90d4..1e5468db 100644 --- a/internal/repository/pg_unibox.go +++ b/internal/repository/pg_unibox.go @@ -22,6 +22,9 @@ type UpdateUniboxEntry struct { // FolderPath is the source folder's name, the folder's identity. FolderPath *string `json:"folder_path"` Folder *string `json:"folder"` + // ProviderFolder is the provider's own placement. It moves on every real + // provider move; Folder only follows when the message was not filed here. + ProviderFolder *string `json:"provider_folder"` } type UniboxRepository interface { @@ -114,7 +117,7 @@ var mailFieldsFull = []string{ "gmail_id", "parent_id", "uid", "mod_seq", "flags", "bcc", "cc", "from_addr", "in_reply_to", "reply_to", "to_addr", "subject", "size", "internal_date", "sent_date", - "snippet", "seen", "updated_at", "created_at", "folder", + "snippet", "seen", "updated_at", "created_at", "folder", "provider_folder", } var mailFieldsPreview = []string{ @@ -129,13 +132,15 @@ func (r *uniboxRepository) CreateEntry(ctx context.Context, userID uuid.UUID, e gmail_id, parent_id, uid, mod_seq, flags, bcc, cc, from_addr, in_reply_to, reply_to, to_addr, subject, size, internal_date, sent_date, - snippet, seen, created_at, updated_at, body_text, folder + snippet, seen, created_at, updated_at, body_text, folder, + provider_folder ) VALUES ( $1, $2, $3, $4, $5, $6, $7, $8, $9, $10, $11, $12, $13, $14, $15, $16, $17, $18, $19, $20, $21, $22, - $23, $24, $25, $26, $27, $28 + $23, $24, $25, $26, $27, $28, + $28 ) ON CONFLICT (id) DO NOTHING ` @@ -149,6 +154,8 @@ func (r *uniboxRepository) CreateEntry(ctx context.Context, userID uuid.UUID, e textArray(e.InReplyTo), textArray(e.ReplyTo), textArray(e.ToAddr), e.Subject, e.Size, e.InternalDate, e.SentDate, e.Snippet, e.Seen, e.CreatedAt, e.UpdatedAt, e.BodyText, + // $28 is both columns: a message starts out where the provider put it, + // and only diverges once someone files it in Warmbly. models.NormalizeFolder(e.Folder, e.Flags), ) return err @@ -198,6 +205,11 @@ func (r *uniboxRepository) UpdateEntry(ctx context.Context, userID, emailID, id args = append(args, *e.Folder) argPos++ } + if e.ProviderFolder != nil { + setClauses = append(setClauses, fmt.Sprintf("provider_folder = $%d", argPos)) + args = append(args, *e.ProviderFolder) + argPos++ + } if argPos == 3 { return nil // nothing to update @@ -254,7 +266,7 @@ func (r *uniboxRepository) GetByID(ctx context.Context, userID, id uuid.UUID) (* &e.GmailID, &e.ParentID, &e.UID, &e.ModSeq, &e.Flags, &e.BCC, &e.CC, &e.FromAddr, &e.InReplyTo, &e.ReplyTo, &e.ToAddr, &e.Subject, &e.Size, &e.InternalDate, &e.SentDate, - &e.Snippet, &e.Seen, &e.UpdatedAt, &e.CreatedAt, &e.Folder, + &e.Snippet, &e.Seen, &e.UpdatedAt, &e.CreatedAt, &e.Folder, &e.ProviderFolder, ) if err != nil { if err == pgx.ErrNoRows { @@ -293,7 +305,7 @@ func (r *uniboxRepository) GetByIDForOrg(ctx context.Context, orgID, id uuid.UUI &e.GmailID, &e.ParentID, &e.UID, &e.ModSeq, &e.Flags, &e.BCC, &e.CC, &e.FromAddr, &e.InReplyTo, &e.ReplyTo, &e.ToAddr, &e.Subject, &e.Size, &e.InternalDate, &e.SentDate, - &e.Snippet, &e.Seen, &e.UpdatedAt, &e.CreatedAt, &e.Folder, + &e.Snippet, &e.Seen, &e.UpdatedAt, &e.CreatedAt, &e.Folder, &e.ProviderFolder, ) if err != nil { if err == pgx.ErrNoRows { diff --git a/internal/repository/unibox_move_folder_live_test.go b/internal/repository/unibox_move_folder_live_test.go new file mode 100644 index 00000000..caf8a942 --- /dev/null +++ b/internal/repository/unibox_move_folder_live_test.go @@ -0,0 +1,198 @@ +package repository + +import ( + "context" + "os" + "testing" + "time" + + "github.com/google/uuid" + "github.com/jackc/pgx/v5/pgxpool" + + "github.com/warmbly/warmbly/internal/infrastructure/db" + "github.com/warmbly/warmbly/internal/models" +) + +// Filing a conversation in the thread header moves it here and not at the +// provider, which only works because unibox_emails keeps the two placements in +// separate columns (migration 000146). models.ResolveFolderSync covers the +// decision; these run the statements it feeds against a real schema, because +// the interesting parts are things Go cannot check: that the insert seeds both +// columns from one bound value, that the full-row scan still lines up after a +// column was appended, and that the move leaves provider_folder alone. +// +// WARMBLY_TEST_DB=postgres://warmbly:warmbly@localhost:15432/warmbly_dev?sslmode=disable \ +// go test ./internal/repository/ -run LiveUniboxMoveFolder -v + +type uniboxFolderFixture struct { + org uuid.UUID + user uuid.UUID + mailbox uuid.UUID +} + +func newUniboxFolderFixture(t *testing.T, pool *pgxpool.Pool) *uniboxFolderFixture { + t.Helper() + ctx := context.Background() + f := &uniboxFolderFixture{org: uuid.New(), user: uuid.New(), mailbox: uuid.New()} + tag := "pr435-" + f.org.String()[:8] + + 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) + } + } + exec(`INSERT INTO users (id, first_name, last_name, email, password_hash) + VALUES ($1, 'Folder', 'Live', $2, 'x')`, f.user, tag+"@test.local") + exec(`INSERT INTO organizations (id, name, slug, owner_user_id) + VALUES ($1, 'PR 435', $2, $3)`, f.org, tag, f.user) + exec(`INSERT INTO organization_members (organization_id, user_id, role, accepted_at) + VALUES ($1, $2, 'owner', NOW())`, f.org, f.user) + exec(`INSERT INTO email_accounts (id, user_id, organization_id, email, name, signature_plain, signature_html, provider) + VALUES ($1, $2, $3, $4, 'Folder', '', '', 'smtp_imap')`, f.mailbox, f.user, f.org, tag+"-mb@test.local") + + t.Cleanup(func() { + c := context.Background() + _, _ = pool.Exec(c, `DELETE FROM unibox_emails WHERE email_id = $1`, f.mailbox) + _, _ = pool.Exec(c, `DELETE FROM email_accounts WHERE id = $1`, f.mailbox) + _, _ = pool.Exec(c, `DELETE FROM organization_members WHERE organization_id = $1`, f.org) + _, _ = pool.Exec(c, `DELETE FROM organizations WHERE id = $1`, f.org) + _, _ = pool.Exec(c, `DELETE FROM users WHERE id = $1`, f.user) + }) + return f +} + +func liveUniboxFolderDB(t *testing.T) *db.DB { + t.Helper() + dsn := os.Getenv("WARMBLY_TEST_DB") + if dsn == "" { + t.Skip("WARMBLY_TEST_DB not set") + } + handle, err := db.New(context.Background(), dsn) + if err != nil { + t.Fatalf("connect: %v", err) + } + t.Cleanup(func() { handle.Pool.Close() }) + return handle +} + +func (f *uniboxFolderFixture) message(t *testing.T, repo UniboxRepository, folder string) uuid.UUID { + t.Helper() + id := uuid.New() + now := time.Now().UTC() + err := repo.CreateEntry(context.Background(), f.user, &models.EmailMessageStoreData{ + ID: id, EmailID: f.mailbox, Folder: folder, + ThreadID: "thread-" + id.String(), MessageID: "<" + id.String() + "@test.local>", + FromAddr: []string{"Centous Support (support@centous.com)"}, + ToAddr: []string{"me@test.local"}, + Subject: "Filing", Snippet: "Filing", + InternalDate: now, SentDate: now, CreatedAt: now, UpdatedAt: now, + Seen: true, + }) + if err != nil { + t.Fatalf("CreateEntry: %v", err) + } + return id +} + +// A new message is in one place, so both columns start there. Getting this +// wrong would make every row read as locally filed from the moment it arrives. +func TestLiveUniboxMoveFolderSeedsBothColumnsOnInsert(t *testing.T) { + handle := liveUniboxFolderDB(t) + f := newUniboxFolderFixture(t, handle.Pool) + repo := NewUniboxRepository(handle) + + id := f.message(t, repo, models.FolderInbox) + got, err := repo.GetByID(context.Background(), f.user, id) + if err != nil { + t.Fatalf("GetByID: %v", err) + } + if got.Folder != models.FolderInbox || got.ProviderFolder != models.FolderInbox { + t.Fatalf("folder=%q provider_folder=%q, want both %q", got.Folder, got.ProviderFolder, models.FolderInbox) + } +} + +// The move writes folder and nothing else. provider_folder staying put is what +// lets the next sync tell this apart from the provider moving the message. +func TestLiveUniboxMoveFolderLeavesTheProviderPlacementAlone(t *testing.T) { + handle := liveUniboxFolderDB(t) + f := newUniboxFolderFixture(t, handle.Pool) + repo := NewUniboxRepository(handle) + ctx := context.Background() + + id := f.message(t, repo, models.FolderInbox) + if err := repo.MoveToFolderBulk(ctx, f.org, []uuid.UUID{id}, models.FolderTrash); err != nil { + t.Fatalf("MoveToFolderBulk: %v", err) + } + + got, err := repo.GetByID(ctx, f.user, id) + if err != nil { + t.Fatalf("GetByID: %v", err) + } + if got.Folder != models.FolderTrash { + t.Fatalf("folder = %q, want %q", got.Folder, models.FolderTrash) + } + if got.ProviderFolder != models.FolderInbox { + t.Fatalf("provider_folder = %q, want it untouched at %q", got.ProviderFolder, models.FolderInbox) + } + + // And the message really has left every default view. + res, err := repo.Search(ctx, f.org, f.user, &models.MailSearchParams{}) + if err != nil { + t.Fatalf("Search: %v", err) + } + for _, row := range res.Data { + if row.ID == id { + t.Fatal("a trashed message is still in the unscoped list") + } + } +} + +// Another organization's ids are not this organization's to file. +func TestLiveUniboxMoveFolderIsOrgScoped(t *testing.T) { + handle := liveUniboxFolderDB(t) + mine := newUniboxFolderFixture(t, handle.Pool) + theirs := newUniboxFolderFixture(t, handle.Pool) + repo := NewUniboxRepository(handle) + ctx := context.Background() + + id := theirs.message(t, repo, models.FolderInbox) + if err := repo.MoveToFolderBulk(ctx, mine.org, []uuid.UUID{id}, models.FolderTrash); err != nil { + t.Fatalf("MoveToFolderBulk: %v", err) + } + + got, err := repo.GetByID(ctx, theirs.user, id) + if err != nil { + t.Fatalf("GetByID: %v", err) + } + if got.Folder != models.FolderInbox { + t.Fatalf("folder = %q, want another org's message left at %q", got.Folder, models.FolderInbox) + } +} + +// The thread lookup behind the "label email" automation step reads the address +// out of the raw header, and the IMAP sync writes those as "Name (addr)". +func TestLiveUniboxLatestThreadIDMatchesTheParenthesisedForm(t *testing.T) { + handle := liveUniboxFolderDB(t) + f := newUniboxFolderFixture(t, handle.Pool) + repo := NewUniboxRepository(handle) + ctx := context.Background() + + id := f.message(t, repo, models.FolderInbox) + threadID, err := repo.LatestThreadIDForContact(ctx, f.user, "support@centous.com") + if err != nil { + t.Fatalf("LatestThreadIDForContact: %v", err) + } + if threadID != "thread-"+id.String() { + t.Fatalf("thread = %q, want %q", threadID, "thread-"+id.String()) + } + + // Still an exact match, never a substring one. + other, err := repo.LatestThreadIDForContact(ctx, f.user, "upport@centous.com") + if err != nil { + t.Fatalf("LatestThreadIDForContact: %v", err) + } + if other != "" { + t.Fatalf("thread = %q, want no match for a partial address", other) + } +} From 9f61d070d424f86888a26670af0d266b76c1d2b9 Mon Sep 17 00:00:00 2001 From: Matthew Meszaros Date: Fri, 11 Sep 2026 03:23:23 -0700 Subject: [PATCH 4/4] feat: give the unibox thread header a way back out of a filing mistake, with Undo on the Archive and Delete toasts, Move to inbox while reading the Trash or Archive folder, and the actions disabled while one is in flight, plus stop the invite page offering Accept before it knows which account the browser is signed in as --- docs/content/docs/guides/unibox.mdx | 22 ++- web/src/app/invite/page.tsx | 12 +- .../components/app/unibox/ReplyComposer.tsx | 1 - web/src/components/app/unibox/ThreadView.tsx | 158 +++++++++++++----- .../lib/api/client/app/unibox/moveFolder.ts | 12 +- .../lib/api/hooks/app/unibox/useMoveFolder.ts | 12 +- web/src/lib/helper/emailAddress.test.ts | 12 ++ 7 files changed, 174 insertions(+), 55 deletions(-) diff --git a/docs/content/docs/guides/unibox.mdx b/docs/content/docs/guides/unibox.mdx index 5ff25488..8a52d7f1 100644 --- a/docs/content/docs/guides/unibox.mdx +++ b/docs/content/docs/guides/unibox.mdx @@ -14,11 +14,11 @@ The rail opens with the standard mail folders, so direction is visually clear in | Inbox | Inbound mail, plus anything filed in a custom folder at the provider | | Drafts | Messages sitting in a mailbox's drafts folder | | Sent | Outbound mail, campaign and manual | -| Archive | Archived at the provider (Gmail's All Mail, the Archive folder elsewhere) | +| Archive | Archived (Gmail's All Mail, the Archive folder elsewhere) | | Spam | Junked at the provider | -| Trash | Deleted at the provider | +| Trash | Deleted | -Each message's folder follows the provider: IMAP special-use folder attributes, Gmail labels, and Outlook well-known folders all map to the same six. Moves at the provider (junking a message, clearing it out of spam) follow on the next sync. The active folder is highlighted with a grey row and bold label, unread counts sit on the right, and each folder's three-dot menu offers **Mark all as read**. +Each message starts in the folder the provider has it in: IMAP special-use folder attributes, Gmail labels, and Outlook well-known folders all map to the same six. Moves at the provider (junking a message, clearing it out of spam) follow on the next sync. You can also file a conversation yourself, which moves it here without moving it at the provider; see [filing a conversation](#filing-a-conversation). The active folder is highlighted with a grey row and bold label, unread counts sit on the right, and each folder's three-dot menu offers **Mark all as read**. Spam and Trash stay out of every other view: the **All** scope, the metric strip, and the unread badge only count the folders you actually work. @@ -77,6 +77,22 @@ Categories label conversations. Tags label the mailboxes themselves (grouping ac A conversation is unread when any message inside is unseen, marked by a bright left edge bar and bolder text. Opening it marks its messages seen and updates the counts. Read state syncs both ways with the mailbox provider. +## Filing a conversation + +The thread header carries three filing actions, on the row above the message on a wide screen and behind the three-dot menu on a narrow one: + +| Action | Does | +| --- | --- | +| Mark as unread | Puts every message in the conversation back to unread and closes it | +| Archive | Moves the conversation to Archive, so it leaves Inbox | +| Delete | Moves the conversation to Trash, so it leaves every view except Trash | + +Archive and Delete both offer **Undo** on the confirmation toast. Open the Trash or Archive folder and the same header offers **Move to inbox**, so nothing filed by accident is stuck. + + +Archive and Delete are Warmbly's own filing. The message keeps its place in Gmail, Outlook, or whatever mail client the mailbox belongs to, and deleting a conversation here never deletes mail there. The next sync will not undo your filing either: Warmbly records where the provider has each message separately from where you filed it, and follows the provider only when the provider itself moves the message. So junking a message in Gmail still reaches Warmbly, and an ordinary sync pass does not. + + ## Replying Reply, forward, or hover any single message to reply to it specifically. The composer only appears when you ask for it. Replies go from the mailbox that owns the thread, so conversations stay on one account. You can apply a saved **template** or **Insert booking link**. diff --git a/web/src/app/invite/page.tsx b/web/src/app/invite/page.tsx index 8760ffd7..6efcdadf 100644 --- a/web/src/app/invite/page.tsx +++ b/web/src/app/invite/page.tsx @@ -54,6 +54,10 @@ export default function InviteAcceptPage() { const signedInEmail = me.data?.email ?? ""; const wrongAccount = loggedIn && !!signedInEmail && !!invitedEmail && signedInEmail.toLowerCase() !== invitedEmail.toLowerCase(); + // Accept is a guaranteed 403 for the wrong session, so it must not be + // clickable before we know which session this is. `me` never resolves when + // it was never asked (no token), hence the loggedIn half. + const identityPending = loggedIn && (me.isPending || me.isFetching); async function onSwitchAccount() { await logout.mutateAsync(); @@ -168,11 +172,13 @@ export default function InviteAcceptPage() { ) : (
diff --git a/web/src/components/app/unibox/ReplyComposer.tsx b/web/src/components/app/unibox/ReplyComposer.tsx index 46b7d4e1..39cd0d53 100644 --- a/web/src/components/app/unibox/ReplyComposer.tsx +++ b/web/src/components/app/unibox/ReplyComposer.tsx @@ -139,7 +139,6 @@ function looksLikeEmail(s: string): boolean { return /^[^\s@]+@[^\s@]+\.[^\s@]+$/.test(s.trim()); } - // Derive composer defaults from the message the user explicitly chose // to reply to (or forward). Reply takes the message's "from" as the // new "to". Forward leaves "to" empty so the user picks the new diff --git a/web/src/components/app/unibox/ThreadView.tsx b/web/src/components/app/unibox/ThreadView.tsx index d69dbb6d..3565aaa4 100644 --- a/web/src/components/app/unibox/ThreadView.tsx +++ b/web/src/components/app/unibox/ThreadView.tsx @@ -6,6 +6,7 @@ // path with a native datetime input. import React from "react"; +import { useParams } from "react-router-dom"; import { AnimatePresence, motion } from "framer-motion"; import { useQueryClient, useMutation } from "@tanstack/react-query"; import toast from "react-hot-toast"; @@ -17,6 +18,7 @@ import { ClockIcon, CornerUpLeftIcon, ForwardIcon, + InboxIcon, Loader2Icon, MailCheckIcon, MoonIcon, @@ -42,6 +44,7 @@ import { SectionBar } from "@/components/layout/Page"; import useThread from "@/lib/api/hooks/app/unibox/useThread"; import useMarkSeen from "@/lib/api/hooks/app/unibox/useMarkSeen"; import useMoveFolder from "@/lib/api/hooks/app/unibox/useMoveFolder"; +import moveFolderRequest, { type FilableFolder } from "@/lib/api/client/app/unibox/moveFolder"; import { bareEmail, nameFromAddr, wrappedEmail } from "@/lib/helper/emailAddress"; import useThreadLabels from "@/lib/api/hooks/app/unibox/useThreadLabels"; import useThreadScheduled from "@/lib/api/hooks/app/unibox/useThreadScheduled"; @@ -86,6 +89,14 @@ function toUniboxEmail(m: UniboxThreadMessage): UniboxEmail { }; } +// Filing copy, per destination. "Deleted" is deliberately not said anywhere: +// the message is moved to Trash here and still sits in the mail client. +const FILE_COPY: Record = { + archive: { loading: "Archiving…", done: "Archived", failed: "Couldn't archive" }, + trash: { loading: "Moving to Trash…", done: "Moved to Trash", failed: "Couldn't move to Trash" }, + inbox: { loading: "Moving to Inbox…", done: "Moved to Inbox", failed: "Couldn't move to Inbox" }, +}; + const SNOOZE_PRESETS: { label: string; until: () => Date }[] = [ { label: "In 1 hour", until: () => offsetHours(1) }, { label: "In 3 hours", until: () => offsetHours(3) }, @@ -267,25 +278,66 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { // Header actions. Each one closes the thread: the effect above would // otherwise re-mark an "unread" thread as seen on the next refetch, and a - // trashed/archived thread has left the list the reader is looking at. + // filed thread has left the list the reader is looking at. const moveFolder = useMoveFolder(); const setSelectedThreadId = useAppStore((s) => s.setSelectedThreadId); const threadIds = () => (q.data?.data ?? []).map((m) => m.id); const markUnread = () => { - markSeenMutate({ ids: threadIds(), seen: false }); + markSeenMutate({ ids: threadIds(), seen: false, threadId }); setSelectedThreadId(null); }; - const fileThread = (folder: "trash" | "archive") => { - toast - .promise(moveFolder.mutateAsync({ ids: threadIds(), folder }), { - loading: folder === "trash" ? "Deleting…" : "Archiving…", - success: folder === "trash" ? "Moved to Trash" : "Archived", - error: folder === "trash" ? "Couldn't delete" : "Couldn't archive", - }) - .then(() => setSelectedThreadId(null)) - .catch(() => undefined); + + // One click and the conversation is gone from the list, so the way back + // belongs on screen; the Trash scope's Move to inbox is the slow path. This + // pane has already closed by the time Undo is clicked, so it calls the + // endpoint directly: react-query drops an unmounted observer's callbacks, + // and the invalidation is the whole point. + const offerUndo = (message: string, ids: string[]) => { + toast((t) => ( + + {message} + + + )); }; + // Filing is store-side: the message keeps its place at the provider, and + // the sync knows not to undo this (migration 000146). + const fileThread = async (folder: FilableFolder) => { + const ids = threadIds(); + if (ids.length === 0 || moveFolder.isPending) return; + const copy = FILE_COPY[folder]; + const pending = toast.loading(copy.loading); + try { + await moveFolder.mutateAsync({ ids, folder }); + setSelectedThreadId(null); + toast.dismiss(pending); + if (folder === "inbox") toast.success(copy.done); + else offerUndo(copy.done, ids); + } catch { + toast.dismiss(pending); + toast.error(copy.failed); + } + }; + + // Restoring is only offered where the user can see what they are restoring. + const { scope: urlScope } = useParams<{ scope?: string }>(); + const filed = urlScope === "trash" || urlScope === "archive"; + const snooze = useMutation({ mutationFn: (until: Date) => snoozeThread({ thread_id: threadId, snoozed_until: until.toISOString() }), @@ -360,8 +412,8 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { const mailbox = accounts.find((a) => a.id === messages[0]?.account_id); // The external party of the thread = the first message address that isn't - // our own mailbox. Addresses arrive as "Name " or bare "addr"; reduce - // to the bare address so the comparison + the CRM panel lookup both work. + // our own mailbox. Headers arrive in all three shapes lib/helper/emailAddress + // parses; reduce to the bare address so the comparison + the lookup work. const mailboxEmail = mailbox?.email?.toLowerCase(); const contactFrom = messages @@ -535,17 +587,30 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { icon={} onClick={markUnread} /> - } - onClick={() => fileThread("archive")} - /> - } - onClick={() => fileThread("trash")} - /> + {filed ? ( + } + disabled={moveFolder.isPending} + onClick={() => fileThread("inbox")} + /> + ) : ( + } + disabled={moveFolder.isPending} + onClick={() => fileThread("archive")} + /> + )} + {urlScope !== "trash" && ( + } + disabled={moveFolder.isPending} + onClick={() => fileThread("trash")} + /> + )}
@@ -564,19 +629,33 @@ export function ThreadView({ threadId, emailId }: ThreadViewProps) { > Mark as unread - } - onSelect={() => fileThread("archive")} - > - Archive thread - - } - onSelect={() => fileThread("trash")} - > - Delete thread - + {filed ? ( + } + disabled={moveFolder.isPending} + onSelect={() => fileThread("inbox")} + > + Move to inbox + + ) : ( + } + disabled={moveFolder.isPending} + onSelect={() => fileThread("archive")} + > + Archive thread + + )} + {urlScope !== "trash" && ( + } + disabled={moveFolder.isPending} + onSelect={() => fileThread("trash")} + > + Delete thread + + )} @@ -689,11 +768,13 @@ function IconAction({ label, icon, danger, + disabled, onClick, }: { label: string; icon: React.ReactNode; danger?: boolean; + disabled?: boolean; onClick?: () => void; }) { return ( @@ -702,9 +783,10 @@ function IconAction({