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) + } +}