Skip to content

fix(sync): a draft synced from Outlook or Gmail opens in the editor, and one sent elsewhere stops being a draft - #164

Merged
karatektus merged 14 commits into
mainfrom
fix/synced-draft-stays-editable
Oct 15, 2026
Merged

karatektus merged 14 commits into
mainfrom
fix/synced-draft-stays-editable

Conversation

@karatektus

@karatektus karatektus commented Oct 15, 2026 •

Copy link
Copy Markdown
Owner

Closes #135

Takes over #123 by @Flouss, whose fork was archived, so that branch can no longer be pushed to. All their commits are kept. Squash-merge with Co-authored-by: Flouss.

Changes on top of #123

  • Migration Version20261015130000 for drafts stored before the upgrade, which fix(sync): a draft synced from Outlook or Gmail opens in the editor, and one sent elsewhere stops being a draft #123 left at a 404. It clears sent_at only on rows flagged \Draft that the provider still files as drafts: Gmail DRAFT without SENT, Graph's Drafts label without Sent. A stored draft that was sent elsewhere before the upgrade keeps its sent_at, so it can't become sendable a second time. IMAP rows aren't touched (their flag is Draft, which isDraft() doesn't read as a draft). Threads aren't re-ranked. down() is a no-op.
    SyncedDraftSentAtMigrationTest runs it against rows: six cases, each "cleared" with a "kept" partner. Mutation-checked: without the narrowing, the three kept cases fail.
  • Merged main. GraphDraftSentElsewhereTest now builds GraphApiSyncer with the two arguments main added since (ThreadLabelSynchronizer, SyncNotifier). Without them PHPStan and the test failed.
  • CHANGELOG: says what the migration does, and credits this PR.
  • On top of fix(push): the installed app says New mail only when mail arrived #162 (now merged): its guard records a Graph re-report only when the folder changed. A draft filed in Sent Items before settling existed still wears \Draft, and its re-report is where it settles. That is now counted as a change, so it's recorded for JMAP clients and the open list refreshes. Tested in GraphDraftSentElsewhereTest::testADraftAlreadyFiledInSentIsSettledAndRecordedOnItsReReport (fails without the change).

How it was checked

  • PHPStan: no errors.
  • PHPUnit, 1,754 tests (ComposeSafety, Domain, Entity/Mail, Messaging handlers, Gmail, Graph, Imap, Service/Mail, Migrations, Documentation, Translation): green.

Left as is

  • A draft sent from Gmail, or moved into Sent Items under immutable ids, is settled with the time it was noticed (now), not the provider's send time. That only affects the sort time of a draft stored after this change.

#123's own description:

Problem

A draft that arrives by sync (Outlook or Gmail) is listed in its conversation, but clicking it gives a 404: ComposeController::assertEditableDraft (the guard that only lets an unsent draft open) refuses anything with a sentAt, and every sync builder sets sentAt on a draft:

  • Graph: sentAt = sentDateTime ?? receivedDateTime, and Outlook gives a draft a receivedDateTime but no sentDateTime.
  • Gmail: sentAt = receivedAt from internalDate.
  • IMAP: sentAt = receivedAt from the Date header.

Change

  • The builders leave sentAt null for a draft, the same state a draft written in plMail has (receivedAt is unchanged). Documented on Message::$sentAt. The IMAP builder decides from the stored flag via isDraft(); webklex strips the backslash, so a real server's \Draft is stored as Draft and is not a draft to the app yet. That part does nothing until the flags are canonicalised at ingest, and is deliberately kept consistent with isDraft() instead.
  • ReadReceiptPolicy::decide recognised our own mail by sentAt; it checks the draft flag as well now.

sentAt was also quietly protecting the other direction, which this PR has to handle: sync stores a draft once and never rebuilds a stored row, so a draft sent from another client stayed \Draft with no sentAt and would now open in the editor, be sendable a second time, and discarding it would delete the sent copy at the provider. Message::settleAsSent() is the one place that says what "was sent" means for a stored draft (flag off, keep an existing sentAt else now, thread ranked by it, never backwards). It is called when

  • Graph files a stored message in Sent Items (same id, immutable ids),
  • the Graph batch handler re-points a stored message whose fetched resource says isDraft: false (new id, no immutable ids; an absent key is not an answer),
  • Gmail enrichment re-reads a stored draft whose labels have SENT and no DRAFT.

Trash and Drafts do not settle.

RemoteFlagState::storedFlags() rebuilt Gmail's and Graph's flag mirror from read and starred alone, which dropped \Draft the first time a stored draft was re-read, although the refresh's own comment says it survives. It now carries the flag over. This touches the shared flag refresh, so it deserves a look.

Notes

  • Visible side effect (in the changelog): a conversation holding a synced draft is no longer moved up the list by the draft's date, the same as for a draft written in plMail.
  • Drafts already stored are not migrated; their sent_at has to be cleared once (a single UPDATE on the affected rows).
  • The send-elsewhere behaviour rests on two assumptions about the providers that the tests encode but that I have not observed against live Outlook or Gmail: Graph either keeps the id or re-fetches the sent copy with isDraft: false, and Gmail keeps the message id with DRAFT replaced by SENT.

Tests

Builder tests for all three providers, the sent-elsewhere paths (Graph same id, Graph new id, Gmail) each with a companion that must stay a draft, the settle method, the flag mirror, the editor opening a synced draft and still refusing a received mail, and the read-receipt guard. Each behaviour was broken on purpose and the test written for it failed. Full suite green except IntegrationUrlValidatorTest (container cannot resolve localhost.); phpstan clean on the changed files.

🤖 Generated with Claude Code

🤖 Generated with Claude Code

Flouss and others added 9 commits October 14, 2026 21:53
…s in the editor

The editor, the send queue and the thread read "sentAt is set" as "has gone
out". The Graph, Gmail and IMAP builders stamped every synced message with
sentAt (Outlook: sentDateTime ?? receivedDateTime, which a draft has), so a
draft written in another client was listed in the thread and then refused by
the editor with a 404 (ComposeController::assertEditableDraft).

The builders now leave sentAt null for a draft, the same state a draft written
here has. receivedAt is unchanged. IMAP compares the flag canonically, since
webklex reports `Draft` from some servers and `\Draft` from others.

ReadReceiptPolicy::decide used "has sentAt" to recognise our own mail; a draft
no longer has one, so it checks the draft flag as well.

Visible change: MessageThreader skips an unsent draft when moving a thread's
lastMessageAt, so a synced draft no longer floats its thread up the list.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…raft() reads it

webklex strips the backslash from a server's flags, so a real `\Draft` is
stored as `Draft` and is not a draft to Message::isDraft(). Matching it
canonically gave a row that was neither a draft nor sent. sentAt now follows
the stored flag exactly, and the changelog no longer claims IMAP drafts.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
Sync stores a draft once and never rebuilds a stored row, so sending it
elsewhere left it flagged \Draft with no sentAt. Now that a synced draft has no
sentAt the editor opens it (it used to 404, which was an accidental guard), so
the stale row could be opened, sent a second time, and discarded, which deletes
the sent copy at the provider.

Message::settleAsSent() is the one place that says what "was sent" means for a
stored draft: flag off, sentAt set, thread ranked by it. It is called when
  - Graph files a stored message in Sent Items (same id, immutable ids),
  - the Graph batch handler re-points a stored message whose fetched resource
    says isDraft = false (new id, no immutable ids),
  - Gmail enrichment re-reads a stored draft whose labels no longer include
    DRAFT (before the flag refresh).
Trash and Drafts do not settle: a draft thrown away or put back is unsent.

RemoteFlagState::storedFlags() rebuilt Gmail's and Graph's flag mirror from the
read and starred facts alone, which dropped \Draft the first time a stored draft
was re-read, despite the refresh's own comment saying it survives. It now
carries the flag over.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…sent

Also drops three needless nullsafe calls from the settle test (phpstan).

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
…w what counts as sent

Review of the settle change:
- A draft stored before synced drafts were kept unsent already has a sentAt.
  settleAsSent() ranked the thread by "now" instead, so the mass re-read an
  upgrade causes would float every old conversation. It keeps its sentAt and
  ranks by it.
- Gmail settled on "DRAFT label absent"; Graph only on Sent Items. Gmail now
  needs SENT present and DRAFT absent.
- The Graph handler settled on an absent isDraft; only an explicit false counts.
- Comments that overstated (JMAP announcement of the move, "a delta never
  changes draft state") corrected.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
The builders now leave a synced draft's sentAt null, but drafts already
stored kept the receivedAt they were given as sentAt, so they still
gave a 404 in the editor. The migration clears it only for drafts the
provider still files as drafts (Gmail DRAFT without SENT, Graph Drafts
without Sent): a stored draft that was sent elsewhere before the
upgrade keeps its sentAt, so it cannot be sent a second time.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@karatektus karatektus added this to the v0.3.6 milestone Oct 15, 2026
@karatektus karatektus added the bug Something that worked, or should work, does not label Oct 15, 2026
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
karatektus and others added 4 commits October 15, 2026 15:12
With #162's guard, a re-report that moves nothing records nothing. A
draft filed in Sent Items before settling existed still wears \Draft
beside Sent, and its re-report is where it settles; that changes its
keywords and the list's draft marker, so it now counts as a change.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…gration

Doctrine prepares a migration's statement, and a ? is a placeholder to
it: the jsonb ? operator reached Postgres as $1 and app-init failed on
every upgrade. The test executed the SQL raw, which let it through; it
now prepares it the way Doctrine Migrations does, and fails on the old
statement.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@karatektus
karatektus merged commit ebd2570 into main Oct 15, 2026
10 checks passed
@karatektus
karatektus deleted the fix/synced-draft-stays-editable branch October 15, 2026 18:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area: gmail Concerns gmail area: imap Concerns imap area: mail Concerns mail area: outlook Concerns outlook bug Something that worked, or should work, does not

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A draft synced from Outlook or Gmail does not open in the editor, and one sent elsewhere stays a draft

2 participants