mirror of
https://github.com/karatektus/pl_mail.git
synced 2026-10-25 01:54:10 +00:00
Paul LütznerandClaude Opus 5
59cbbc9827
Reported as: deleting a mail that is already in the trash takes the row off the list and leaves the mail in the trash. And, separately: why can spam and trashed mail be snoozed and archived? Both are the same cause. Trashing is "add the Trash label", so trashing something already trashed is a no-op on the server — while the list removed the row anyway, which is why the mail looked deleted twice and was there both times. Archive and Snooze in the bin are the same question from the other side. ## The action set now depends on where the mail sits MailPlacement reads the conversation's own labels rather than the current route, because a thread reached from a search result, a live update or a URL has no list behind it and a route check would go quietly back to offering the wrong controls in exactly those cases. Archive, Delete and Snooze are hidden in trash and spam, on the row, in the thread header and in the overflow menu. It is a service the Twig function delegates to, not a Twig function alone: the purge route refuses anything MailPlacement does not call discarded, and an interface that hides a control while the route behind it stays open is a habit rather than a rule. ## Delete forever Hiding the control left the bin with no way to be emptied, which is the wrong place to stop, so this is the real thing. Most of it turned out to be written and unconnected: ApplyImapFlagsHandler has implemented `'delete' => $imapMessage->delete(expunge: true)` all along, under a comment saying the row was "on its way to having no row at all", and MailChangeRecorder::emailDestroyed() existed with no callers. What was missing was the part that removes the local copy — and one thing that could not have been reused. **LabelChangePropagator cannot carry a purge, and that is the whole design.** Every propagation message names local Message rows and lets the handler look them up. That works while the row survives the operation. A purge deletes it, so a handler would find nothing, log "no messages found" and return — leaving the mail on the server to come back at the next sync as a message the user had explicitly destroyed. Invisible against a fake provider. So PurgeRemoteMessagesMessage carries the ADDRESS instead: IMAP folder and UID, Gmail id, Graph id. Onto the export queue with every other outbound push, so a purge cannot overtake a relabel already queued for the same mail. Note propagator::delete() is NOT dead code and is untouched — it is ComposeController::discard(), where TRASH at Gmail is right rather than a compromise: a draft that was never sent belongs in the bin its own client would put it in. Its docblock now says so, because I misread it as dead on the way past. Gmail gets messages.batchDelete, needing the full mail scope. Without it the API answers insufficientPermissions, which assertSuccess() already classifies as unrecoverable — so it lands in the log once rather than retrying forever. Falling back to TRASH would be the worst available answer: the user is told it is gone, plMail can no longer see it, and Google still has it. The files go too. A deletion that leaves the bytes on disk is a lie in the way that matters most for a mail client. Parts are removed explicitly rather than by adding cascade to the mapping, which would change every other path that touches a message. ## Restore The counterpart the bin never had: mail could be thrown away and not fetched back, except by finding it and attaching Inbox by hand from the label menu. Removes both Trash and Spam, not only the one it was found under — a spam mail somebody then deleted carries both, and one returned to the inbox still marked SPAM gets filed away again by Gmail without plMail doing anything. ## Also here - Pasted markup is reduced to content (CODE-REVIEW U-02): style, class, id, bgcolor, font and every alignment attribute go; lists, links and bold stay. Whitelisted rather than flattened to plain text, which is simpler and throws away what people are usually pasting for. The `<style>` block was the worst of them — it restyled the whole composer, not just the pasted text. - Confirmations go through the app's dialog, not the browser's. Note the row's Delete forever asks in JavaScript rather than through data-turbo-confirm: Turbo only consults that attribute for links and form submissions, so on a fetch-posting button it would have looked like a confirmation and never been read. Verified: 4009 unit tests green, PHPStan clean over src/. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
7 lines
331 B
Twig
7 lines
331 B
Twig
{# The row is gone for good, so it goes from the list the same way a trashed one
|
|
does — the difference is that nothing will bring this one back. Addressed by
|
|
an id captured before the purge, because by now the row it named does not
|
|
exist. #}
|
|
<turbo-stream action="remove" target="thread_{{ targetId }}">
|
|
</turbo-stream>
|