From 4656f8874ed1b7dc8de9e619d1282e86d806e166 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Paul=20L=C3=BCtzner?= Date: Fri, 9 Oct 2026 14:20:26 +0200 Subject: [PATCH] Add a spam button that can also filter the sender or their domain MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A button beside Delete — on a list row, in an open conversation and above a selection. On one conversation it opens a menu: move it to Spam, or move it and create a filter for the sender, or for the sender's whole domain. The domain filter asks once more first, and for a mail provider's domain it sits behind "More options". The filter is an ordinary rule under Settings → Filters, a second press on the same sender makes no second one, and Undo takes back the move and a filter the same click created. The filters match on two new conditions, fromAddress and fromDomain, which compare the whole address or the whole domain. `from` is a substring over address and display name, so a filter for a@x.de built on it also caught ba@x.de. Both are in the filter editor and in JMAP Email/query. Refs #31 Co-Authored-By: Claude Opus 5.5 --- CHANGELOG.md | 16 + .../mail/list_toolbar_controller.js | 11 + .../controllers/mail/spam_menu_controller.js | 220 ++++++++++ .../rules/rule_builder_controller.js | 3 +- docs/CLIENT_DEVELOPMENT.md | 2 + docs/de/CLIENT_DEVELOPMENT.md | 4 +- docs/de/features/filters.md | 4 +- docs/de/features/mail.md | 38 +- docs/features/filters.md | 2 + docs/features/mail.md | 33 ++ src/Controller/Mail/SpamController.php | 161 +++++++ src/Domain/DTO/Mail/SpamSender.php | 33 ++ src/Domain/Filter/FilterAstValidator.php | 13 + src/Domain/Filter/FilterVocabulary.php | 47 ++ src/Jmap/Query/EmailFilterCompiler.php | 44 ++ src/Service/Mail/SpamSenderResolver.php | 104 +++++ src/Service/Mail/StatusUndoService.php | 50 +++ src/Service/Rule/SpamFilterCreator.php | 104 +++++ templates/_partials/_list_toolbar.html.twig | 5 + templates/_partials/_sidebar.html.twig | 6 +- templates/_partials/_spam_menu.html.twig | 65 +++ templates/_partials/_thread_row.html.twig | 15 +- templates/mail/_spam_menu_panel.html.twig | 104 +++++ templates/mail/_thread_content.html.twig | 8 + templates/settings/filters/_editor.html.twig | 2 + .../thread/status/_moved_to.stream.html.twig | 4 +- tests/Controller/Mail/SpamButtonTest.php | 410 ++++++++++++++++++ .../Domain/Filter/EmailFilterCompilerTest.php | 12 + .../Domain/Filter/FilterAstValidatorTest.php | 10 + translations/messages.de.yaml | 18 + translations/messages.en.yaml | 18 + translations/messages.en_PI.yaml | 18 + 32 files changed, 1574 insertions(+), 10 deletions(-) create mode 100644 assets/controllers/mail/spam_menu_controller.js create mode 100644 src/Controller/Mail/SpamController.php create mode 100644 src/Domain/DTO/Mail/SpamSender.php create mode 100644 src/Service/Mail/SpamSenderResolver.php create mode 100644 src/Service/Rule/SpamFilterCreator.php create mode 100644 templates/_partials/_spam_menu.html.twig create mode 100644 templates/mail/_spam_menu_panel.html.twig create mode 100644 tests/Controller/Mail/SpamButtonTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a51a088..bd6f3009 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,22 @@ The published image tags: `latest` follows the most recent release below, ## Unreleased +**Mail can be marked as spam with a button.** It sits beside Delete: among a row's buttons, in an +open conversation's toolbar and above a selection. On one conversation it opens a short menu — move +it to Spam, or move it and also create a filter that sends everything from this sender, or from the +sender's whole domain, to Spam from now on. The domain filter asks once more before it acts, and for +a mail provider's domain such as gmail.com it is behind **More options** first. The filter is an +ordinary one under Settings → Filters. Undo takes back the move and a filter the same click created. +Until now the only ways to say "this is spam" were Move to and dragging onto the Spam row. See +[Mail → Mark as spam](docs/features/mail.md#mark-as-spam). + +**Filters can match a sender exactly.** Two new conditions sit beside "From contains": **From is +exactly**, which matches the sender's whole address, and **Sender's domain is**, which matches +everything after the `@`. "From contains" looks for the text anywhere in the address or the display +name, so a filter for `a@x.de` also caught `ba@x.de`, and anybody could match it by putting that +address in their display name. The spam button's filters use the new conditions. Existing filters +are unchanged. JMAP clients get the same two as `fromAddress` and `fromDomain` in `Email/query`. + **A public demo can see how much it is used.** In demo mode the admin area has a new section, Demo visitors: how many demo sessions were started in the last hour, 24 hours, 7 days and 30 days, how many different visitors started them, the total so far and how many have a mailbox right now. No diff --git a/assets/controllers/mail/list_toolbar_controller.js b/assets/controllers/mail/list_toolbar_controller.js index 12897166..78fd2988 100644 --- a/assets/controllers/mail/list_toolbar_controller.js +++ b/assets/controllers/mail/list_toolbar_controller.js @@ -145,6 +145,17 @@ export default class extends Controller { await this._bulkPost("trash"); } + /** + * The toolbar's spam button: the selection goes to Spam, and that is all. + * + * The same move "Move to → Spam" makes, without opening the picker. No + * filter is offered here, unlike the button on one conversation — a + * selection has no single sender to offer one for. + */ + async spamSelected() { + await this._bulkPost("move-to", { role: "spam" }); + } + async markReadSelected() { await this._bulkPost("read", { read: true }); } diff --git a/assets/controllers/mail/spam_menu_controller.js b/assets/controllers/mail/spam_menu_controller.js new file mode 100644 index 00000000..3e74cf0a --- /dev/null +++ b/assets/controllers/mail/spam_menu_controller.js @@ -0,0 +1,220 @@ +import LabelMenuController from "./label_menu_controller.js"; +import { jsonCsrfHeaders } from "../../csrf.js"; +import { announceWrite } from "../../mail_writes.js"; +import { requestFailed } from "../../request_errors.js"; + +/** + * The spam button's menu: move one conversation to Spam, and optionally make + * the filter that sends its sender, or its sender's whole domain, there too. + * + * Extends the label menu for the reason the move menu does: everything hard + * about that controller is the PANEL — the top layer, fitting inside whatever + * clips, dismissing on scroll — and none of it is about labels. + * + * What is its own is where the panel's contents come from. They depend on who + * the conversation is from, so they are fetched when the button is pressed + * rather than rendered into every row of a list. Fetched on every open, not + * once: it is one small request, and it puts the menu back to its first state + * — a confirmation somebody backed out of is not still showing next time. + * + * Values: + * url — GET, the panel's contents for this conversation + * reportUrl — POST, the action; body `{ filter, scope, value }` where + * filter is `none` | `sender` | `domain` + * closesPane — true in the reading pane, whose conversation has just left + * align — `start` hangs the panel from the button's left edge, `end` + * (the inherited behaviour) from its right + * + * Targets: + * trigger — the button; carries aria-expanded, which a list row also reads + * to keep its hover actions on screen while the menu is open + * + * and, inside the fetched panel, all optional: + * more — "More options", shown in place of the domain row for a mail + * provider's domain + * domain — the "whole domain" row, which asks before it acts + * confirm — the question it asks + */ +export default class extends LabelMenuController { + static targets = ["panel", "trigger", "more", "domain", "confirm"]; + + static values = { + url: String, + reportUrl: String, + closesPane: { type: Boolean, default: false }, + align: { type: String, default: "end" }, + }; + + async toggle(event) { + // Now, not after the fetch: by then the click has already reached the + // row this button sits in, and the row has opened its conversation. + event.stopPropagation(); + + if (false === this.panelTarget.classList.contains("hidden")) { + this._close(); + + return; + } + + if (true === this._loading) { + return; + } + + this._loading = true; + + let response; + + try { + response = await fetch(this.urlValue, { headers: { "X-Requested-With": "XMLHttpRequest" } }); + } catch { + requestFailed(null); + + return; + } finally { + this._loading = false; + } + + if (requestFailed(response)) { + return; + } + + this.panelTarget.innerHTML = await response.text(); + + super.toggle(event); + + this.triggerTarget.setAttribute("aria-expanded", "true"); + } + + /** See the panel in _spam_menu.html.twig: a click in it is not a click on the row. */ + stop(event) { + event.stopPropagation(); + } + + /** A mail provider's domain: the domain row takes the place of the button that revealed it. */ + moreOptions(event) { + event.stopPropagation(); + + this.moreTarget.hidden = true; + this.domainTarget.hidden = false; + + this._place(); + } + + /** First click on "whole domain": ask. The filter is made by the button in the question. */ + askDomain(event) { + event.stopPropagation(); + + this.domainTarget.hidden = true; + this.confirmTarget.hidden = false; + + this._place(); + } + + cancelDomain(event) { + event.stopPropagation(); + + this.confirmTarget.hidden = true; + this.domainTarget.hidden = false; + + this._place(); + } + + async report(event) { + event.stopPropagation(); + + const filter = event.currentTarget.dataset.spamFilter ?? "none"; + const view = this.#view(); + + this._close(); + + let response; + + try { + response = await fetch(this.reportUrlValue, { + method: "POST", + headers: jsonCsrfHeaders(), + body: JSON.stringify({ filter, scope: view.scope, value: view.value }), + }); + } catch { + requestFailed(null); + + return; + } + + if (requestFailed(response)) { + return; + } + + Turbo.renderStreamMessage(await response.text()); + announceWrite(); + + if (true === this.closesPaneValue) { + this.#closePane(); + } + } + + // ── Private ─────────────────────────────────────────────────────────── + + /** + * The inherited placement hangs the panel from the button's right edge, + * which suits a button at the right of its row. The reading pane's button + * is at the pane's left edge, where that put the menu over the sidebar. + * + * Only the top-layer case is corrected here; in place, the stylesheet's + * own `left-0` already says it. + */ + _place() { + super._place(); + + const panel = this.panelTarget; + + if ("start" !== this.alignValue || false === panel.matches(":popover-open")) { + return; + } + + const margin = 8; + const rect = this.triggerTarget.getBoundingClientRect(); + const width = panel.getBoundingClientRect().width; + const left = Math.min( + Math.max(margin, rect.left), + Math.max(margin, window.innerWidth - width - margin), + ); + + panel.style.left = `${Math.round(left)}px`; + } + + _close() { + super._close(); + + if (this.hasTriggerTarget) { + this.triggerTarget.setAttribute("aria-expanded", "false"); + } + } + + /** The label menu ticks what the selection carries. This menu has no ticks. */ + _syncFromSelection() { + } + + /** Which list is on screen, in the list toolbar's own words. See mail--move-menu. */ + #view() { + const toolbar = document.querySelector("[data-controller~='mail--list-toolbar']"); + + return { + scope: toolbar?.getAttribute("data-mail--list-toolbar-view-scope-value") ?? "", + value: toolbar?.getAttribute("data-mail--list-toolbar-view-value-value") ?? "", + }; + } + + /** mail-pane lives on (see _layout/app.html.twig). */ + #closePane() { + const element = document.querySelector('[data-controller~="mail--mail-pane"]'); + + if (!element) { + return; + } + + this.application + .getControllerForElementAndIdentifier(element, "mail--mail-pane") + ?.close(); + } +} diff --git a/assets/controllers/rules/rule_builder_controller.js b/assets/controllers/rules/rule_builder_controller.js index e5b769d6..9858fd36 100644 --- a/assets/controllers/rules/rule_builder_controller.js +++ b/assets/controllers/rules/rule_builder_controller.js @@ -34,7 +34,8 @@ const OPERATORS = [ /** Field kind decides which input is rendered and how the value is coerced. */ const FIELDS = { - from: "text", to: "text", cc: "text", bcc: "text", + from: "text", fromAddress: "text", fromDomain: "text", + to: "text", cc: "text", bcc: "text", subject: "text", body: "text", text: "text", filename: "text", listId: "text", hasLabel: "label", notLabel: "label", diff --git a/docs/CLIENT_DEVELOPMENT.md b/docs/CLIENT_DEVELOPMENT.md index a923ad25..928c8c32 100644 --- a/docs/CLIENT_DEVELOPMENT.md +++ b/docs/CLIENT_DEVELOPMENT.md @@ -805,6 +805,8 @@ returns too many emails and the client cannot tell. | `hasAttachment` | Boolean. | | `text` | **Real full-text search** — Postgres `tsvector` + `websearch_to_tsquery('english')`. Stemmed, ranked, not a substring scan. | | `body` / `subject` / `from` | `ILIKE` substring. `from` covers both address and display name. | +| `fromAddress` | **plMail extension.** The sender's whole address, case-insensitive. Not a substring, and not the display name. | +| `fromDomain` | **plMail extension.** The part of the sender's address after the `@`, whole and case-insensitive; given without the `@`. A subdomain does not match. | | `to` / `cc` / `bcc` | Substring over the serialised JSON address array (matches name or address). | | `filename` | Substring over attachment filenames. Inline parts have null filenames and never match. | | `listId` | Substring over the canonicalised `list-id` header. | diff --git a/docs/de/CLIENT_DEVELOPMENT.md b/docs/de/CLIENT_DEVELOPMENT.md index ee9f775d..51909f83 100644 --- a/docs/de/CLIENT_DEVELOPMENT.md +++ b/docs/de/CLIENT_DEVELOPMENT.md @@ -1,4 +1,4 @@ - + # Einen Client für plMail bauen Alles, was eine Entwicklerin (oder ein Agent) braucht, um einen *neuen* plMail-Client zu schreiben @@ -885,6 +885,8 @@ merken. | `hasAttachment` | Boolescher Wert. | | `text` | **Echte Volltextsuche** — Postgres `tsvector` + `websearch_to_tsquery('english')`. Gestemmt, gewichtet, kein Teilstring-Scan. | | `body` / `subject` / `from` | `ILIKE` auf Teilzeichenketten. `from` deckt Adresse und Anzeigename ab. | +| `fromAddress` | **plMail-Erweiterung.** Die ganze Adresse des Absenders, ohne Beachtung der Groß- und Kleinschreibung. Keine Teilzeichenkette, und nicht der Anzeigename. | +| `fromDomain` | **plMail-Erweiterung.** Der Teil der Absenderadresse hinter dem `@`, vollständig und ohne Beachtung der Groß- und Kleinschreibung; ohne das `@` angegeben. Eine Subdomain trifft nicht. | | `to` / `cc` / `bcc` | Teilzeichenkette über dem serialisierten JSON-Adressarray (trifft Name oder Adresse). | | `filename` | Teilzeichenkette über Dateinamen von Anhängen. Inline-Parts haben leere Dateinamen und treffen nie. | | `listId` | Teilzeichenkette über den kanonisierten `list-id`-Header. | diff --git a/docs/de/features/filters.md b/docs/de/features/filters.md index c7bf03ed..a4e58707 100644 --- a/docs/de/features/filters.md +++ b/docs/de/features/filters.md @@ -1,4 +1,4 @@ - + # Filter @@ -28,6 +28,8 @@ verschachtelt eine weitere Gruppe darin. | Bedingung | Was sie prüft | |---|---| | **Von enthält** | Absenderadresse oder Anzeigename | +| **Von ist genau** | Die ganze Adresse des Absenders, und sonst nichts — kein Teil einer Adresse, und nicht der Anzeigename | +| **Absender-Domain ist** | Alles hinter dem `@` der Absenderadresse, vollständig. Ohne das `@` eingeben. Eine Subdomain ist eine andere Domain | | **An enthält**, **Cc enthält**, **Bcc enthält** | Die jeweilige Adressliste | | **Betreff enthält** | Die Betreffzeile | | **Text enthält** | Den Nachrichtentext | diff --git a/docs/de/features/mail.md b/docs/de/features/mail.md index 1085de19..fbb2dc5a 100644 --- a/docs/de/features/mail.md +++ b/docs/de/features/mail.md @@ -1,4 +1,4 @@ - + # Mail @@ -567,6 +567,42 @@ und als sonst nichts — wo die Nachricht danach landet, ist Sache des Anbieters **Aktualisieren** in der Werkzeugleiste reiht für jedes deiner aktiven Konten eine Synchronisierung ein und dreht sich, bis diese Aufträge abgearbeitet sind. +### Als Spam markieren + +Der Spam-Knopf steht neben **Löschen**: unter den Knöpfen einer Zeile, in der Werkzeugleiste +einer geöffneten Konversation und in der Werkzeugleiste über einer Auswahl. Bei einer einzelnen +Konversation öffnet er ein kurzes Menü: + +| Auswahl | Was sie tut | +|---|---| +| **In den Spam verschieben** | Verschiebt diese Konversation in den Spam-Ordner des Kontos, und sonst nichts | +| **Diesen Absender immer in den Spam** | Dasselbe, und legt einen Filter für die Adresse des Absenders an | +| **Diese ganze Domain immer in den Spam** | Dasselbe, und legt einen Filter für alles hinter dem `@` an | + +Die ersten beiden wirken sofort. Der Domain-Filter fragt noch einmal nach, bevor er etwas tut, +weil er auch Adressen trifft, die dir nie geschrieben haben. Gehört die Domain einem +Mail-Anbieter — gmail.com, gmx.de, outlook.com und ähnliche —, steht diese Auswahl erst im Menü, +wenn du **Weitere Optionen** drückst, und fragt dann trotzdem nach: Ein Filter auf so eine Domain +schickt einen guten Teil jedes Adressbuchs in den Spam. + +Der Absender ist, wer außer dir zuletzt in der Konversation geschrieben hat; deine eigenen +Antworten werden übersprungen. Eine Konversation, die nur deine eigene Mail enthält, bietet das +Verschieben an und keinen Filter. + +Der Filter ist ein gewöhnlicher. Er steht unter **Einstellungen → Filter**, benannt nach dem, +worauf er passt, und lässt sich dort bearbeiten, abschalten oder löschen wie jeder andere — siehe +[Filter](filters.md). Er gilt für Mail, die ab jetzt ankommt, in jedem Konto; den Knopf noch +einmal beim selben Absender zu drücken, legt keinen zweiten an. **Rückgängig** in der Meldung +holt die Konversation zurück und nimmt einen Filter wieder weg, den derselbe Klick angelegt hat. + +Über einer Auswahl verschiebt der Knopf alles Ausgewählte in den Spam und bietet keinen Filter +an: Eine Auswahl hat nicht den einen Absender. Für Mail, die schon im Spam oder im Papierkorb +liegt, wird der Knopf nicht gezeigt. + +Mail in den Spam zu verschieben sagt plMail nichts über die nächste Nachricht. plMail hat keinen +eigenen Spamfilter, der lernt; ob dein Anbieter das Verschieben als Signal wertet, entscheidet +der Anbieter. Ein Filter ist der Weg, es hier dauerhaft zu machen. + ## Zurückstellen Zurückstellen ist Archivieren mit Wecker. Die Konversation verlässt den Posteingang sofort — diff --git a/docs/features/filters.md b/docs/features/filters.md index 12e06b79..e626f8f6 100644 --- a/docs/features/filters.md +++ b/docs/features/filters.md @@ -23,6 +23,8 @@ group; **Add group** nests another group inside it. | Condition | What it tests | |---|---| | **From contains** | Sender address or display name | +| **From is exactly** | The sender's whole address, and nothing else — not part of an address, and not the display name | +| **Sender's domain is** | Everything after the `@` of the sender's address, whole. Enter it without the `@`. A subdomain is a different domain | | **To contains**, **Cc contains**, **Bcc contains** | The corresponding address list | | **Subject contains** | The subject line | | **Body contains** | The message body | diff --git a/docs/features/mail.md b/docs/features/mail.md index 152aa005..36fb0e9f 100644 --- a/docs/features/mail.md +++ b/docs/features/mail.md @@ -516,6 +516,39 @@ and nothing else — where the message lands afterwards is the provider's busine **Refresh** in the toolbar queues a sync for every active account you own and spins until those jobs have drained. +### Mark as spam + +The spam button sits beside **Delete**: among a row's buttons, in the toolbar of an open +conversation, and in the toolbar above a selection. On one conversation it opens a short menu: + +| Choice | What it does | +|---|---| +| **Move to Spam** | Moves this conversation to the account's spam folder, and nothing else | +| **Always move this sender to Spam** | The same, and creates a filter for the sender's address | +| **Always move this whole domain to Spam** | The same, and creates a filter for everything after the `@` | + +The first two act at once. The domain filter asks once more before it does anything, because it +catches addresses that have never written to you. Where the domain belongs to a mail provider — +gmail.com, gmx.de, outlook.com and the like — that choice is not on the menu until you press +**More options**, and then still asks: a filter on such a domain sends a good part of anybody's +address book to spam. + +The sender is whoever else wrote most recently in the conversation; your own replies are skipped. +A conversation that holds only your own mail offers the move and no filter. + +The filter is an ordinary one. It appears under **Settings → Filters**, named after what it +matches, and can be edited, switched off or deleted there like any other — see +[Filters](filters.md). It applies to mail that arrives from now on, in every account; pressing the +button again on the same sender does not create a second one. **Undo** in the toast puts the +conversation back and takes away a filter the same click created. + +Above a selection the button moves everything selected to Spam and offers no filter: a selection +has no one sender. The button is not shown for mail that is already in Spam or in the bin. + +Moving mail to spam tells plMail nothing about the next message. plMail has no spam filter of its +own that learns; whether your provider treats the move as a signal is up to the provider. A filter +is how you make it stick here. + ## Snooze Snoozing is archiving with a timer. The conversation leaves the Inbox now — at the provider too, diff --git a/src/Controller/Mail/SpamController.php b/src/Controller/Mail/SpamController.php new file mode 100644 index 00000000..8a036b92 --- /dev/null +++ b/src/Controller/Mail/SpamController.php @@ -0,0 +1,161 @@ + '\d+'])] +#[IsGranted('ROLE_USER')] +final class SpamController extends AbstractController +{ + use ChecksCsrf; + use RendersTurboStreams; + + private const string NO_FILTER = 'none'; + + public function __construct( + private readonly MessageThreadRepository $threads, + private readonly SpamSenderResolver $senders, + private readonly SpamFilterCreator $filters, + private readonly MoveToService $moveTo, + private readonly StatusUndoService $undo, + private readonly TranslatorInterface $translator, + ) { + } + + /** The menu's contents for this conversation. A fragment, loaded when the button is pressed. */ + #[Route('/spam-menu', name: 'menu', methods: ['GET'])] + public function menu(int $id): Response + { + /** @var User $user */ + $user = $this->getUser(); + + return $this->render('mail/_spam_menu_panel.html.twig', [ + 'sender' => $this->senders->of($this->thread($id), $user), + ]); + } + + #[Route('/spam', name: 'report', methods: ['POST'])] + public function report(Request $request, int $id): Response + { + $this->assertCsrf($request, 'ajax'); + + /** @var User $user */ + $user = $this->getUser(); + $thread = $this->thread($id); + + $body = json_decode($request->getContent(), true); + $body = is_array($body) ? $body : []; + + $filter = (string) ($body['filter'] ?? self::NO_FILTER); + + // Refused before anything moves: a request that asked for a filter and + // got only the move would be half obeyed, with a toast that could not + // say which half. + if (self::NO_FILTER !== $filter && false === in_array($filter, SpamFilterCreator::SCOPES, true)) { + throw new BadRequestHttpException('Unknown filter.'); + } + + $sender = self::NO_FILTER === $filter ? null : $this->senders->of($thread, $user); + + if (self::NO_FILTER !== $filter && null === $sender) { + throw new BadRequestHttpException('This conversation has no sender to filter.'); + } + + $target = $this->moveTo->systemTarget($user, 'spam'); + + if (null === $target) { + throw $this->createAccessDeniedException('No spam label.'); + } + + $scope = (string) ($body['scope'] ?? ''); + $value = (string) ($body['value'] ?? ''); + $move = $this->moveTo->plan($user, $target, $scope, $value); + + // Pressed on mail that is already in Spam, from the Spam list. The + // button is not drawn there; the honest answer to a caller that posts + // anyway is that nothing happened. + if (true === $move->isNoop) { + return $this->renderTurboStream('thread/status/_bulk.stream.html.twig', [ + 'count' => 0, + 'threads' => [], + 'leaves' => false, + ]); + } + + $undoToken = $this->undo->remember($thread->messages); + + $this->moveTo->move([$thread], $move); + + $message = null; + + if (null !== $sender) { + [$rule, $created] = $this->filters->create($user, $sender, $filter); + + // Only a rule this press made goes back with the Undo. + if (true === $created) { + $this->undo->alsoRemove($undoToken, $rule); + } + + $message = $this->translator->trans('toast.spam_filtered', [ + '%sender%' => $this->filters->pattern($sender, $filter), + ]); + } + + return $this->renderTurboStream('thread/status/_moved_to.stream.html.twig', [ + 'count' => 1, + 'threads' => [$thread], + 'label' => $target, + 'staying' => true === $this->moveTo->staysInView($thread, $user, $scope, $value) ? [$thread] : [], + 'undoToken' => $undoToken, + 'message' => $message, + ]); + } + + private function thread(int $id): MessageThread + { + $thread = $this->threads->find($id); + + if (null === $thread) { + throw $this->createNotFoundException(); + } + + $this->denyAccessUnlessGranted(OwnershipVoter::OWN, $thread); + + return $thread; + } +} diff --git a/src/Domain/DTO/Mail/SpamSender.php b/src/Domain/DTO/Mail/SpamSender.php new file mode 100644 index 00000000..deabd905 --- /dev/null +++ b/src/Domain/DTO/Mail/SpamSender.php @@ -0,0 +1,33 @@ +domain; + } +} diff --git a/src/Domain/Filter/FilterAstValidator.php b/src/Domain/Filter/FilterAstValidator.php index 92910ede..95db5ff4 100644 --- a/src/Domain/Filter/FilterAstValidator.php +++ b/src/Domain/Filter/FilterAstValidator.php @@ -140,6 +140,19 @@ final class FilterAstValidator return; } + if (true === in_array($property, FilterVocabulary::SENDER_CONDITIONS, true)) { + if (false === FilterVocabulary::isSenderValue($property, $value)) { + throw new InvalidFilterException(sprintf( + 'fromDomain' === $property + ? '"%s" needs a domain, without the @.' + : '"%s" needs a whole address.', + $property, + )); + } + + return; + } + if (true === in_array($property, FilterVocabulary::INT_CONDITIONS, true)) { if (false === is_int($value)) { throw new InvalidFilterException(sprintf('"%s" must be a whole number.', $property)); diff --git a/src/Domain/Filter/FilterVocabulary.php b/src/Domain/Filter/FilterVocabulary.php index eb2fc059..d752e45e 100644 --- a/src/Domain/Filter/FilterVocabulary.php +++ b/src/Domain/Filter/FilterVocabulary.php @@ -40,6 +40,27 @@ final class FilterVocabulary 'listId', ]; + /** + * Conditions over the sender that match the whole of something, where + * `from` matches any part of it. + * + * fromAddress the sender's address is exactly this one + * fromDomain the part of the sender's address after the `@` is exactly + * this — no `@`, and a subdomain is a different domain + * + * `from` is a substring over address and display name, which is the right + * tool for "anything mentioning Acme" and the wrong one for "this sender": + * `a@x.de` also matches `ba@x.de`, and a display name can be made to + * contain any address at all. A rule that files a sender under Spam needs + * to mean that sender and nobody whose address merely ends the same way. + * + * Case does not matter for either; nothing else is loosened. + */ + public const array SENDER_CONDITIONS = [ + 'fromAddress', + 'fromDomain', + ]; + /** Conditions taking an integer. */ public const array INT_CONDITIONS = [ 'minSize', @@ -81,6 +102,7 @@ final class FilterVocabulary { return array_merge( self::TEXT_CONDITIONS, + self::SENDER_CONDITIONS, self::INT_CONDITIONS, self::DATE_CONDITIONS, self::BOOL_CONDITIONS, @@ -88,6 +110,31 @@ final class FilterVocabulary ); } + /** + * Whether a value is one a sender condition can match on. + * + * One definition for the validator, which refuses to store anything else, + * and the compiler, which JMAP reaches without going through the + * validator. Deliberately loose about what an address looks like — the + * column holds whatever the mail said — and strict only about the shape + * the comparison depends on: an address has something either side of an + * `@`, a domain has no `@` in it, and neither has whitespace. + */ + public static function isSenderValue(string $condition, mixed $value): bool + { + if (false === is_string($value) || '' === $value || 1 === preg_match('/\s/u', $value)) { + return false; + } + + $at = strrpos($value, '@'); + + if ('fromDomain' === $condition) { + return false === $at; + } + + return false !== $at && $at > 0 && $at < strlen($value) - 1; + } + public static function supports(string $condition): bool { return in_array($condition, self::all(), true); diff --git a/src/Jmap/Query/EmailFilterCompiler.php b/src/Jmap/Query/EmailFilterCompiler.php index b5a110da..5232acc1 100644 --- a/src/Jmap/Query/EmailFilterCompiler.php +++ b/src/Jmap/Query/EmailFilterCompiler.php @@ -4,6 +4,7 @@ declare(strict_types=1); namespace App\Jmap\Query; +use App\Domain\Filter\FilterVocabulary; use App\Domain\Enum\Mail\MessageCategory; use App\Domain\Enum\Mail\MessageFlag; use App\Jmap\Protocol\Exception\MethodException; @@ -137,6 +138,10 @@ final class EmailFilterCompiler 'body' => $this->like(['m.body_text'], $value, $parameters), 'subject' => $this->like(['m.subject'], $value, $parameters), 'from' => $this->like(['m.from_address', 'm.from_name'], $value, $parameters), + // plMail extensions: the sender exactly, where `from` is any part + // of address or display name. See FilterVocabulary::SENDER_CONDITIONS. + 'fromAddress' => $this->fromAddress($value, $parameters), + 'fromDomain' => $this->fromDomain($value, $parameters), 'to' => $this->jsonAddressLike('m.to_addresses', $value, $parameters), 'cc' => $this->jsonAddressLike('m.cc_addresses', $value, $parameters), 'bcc' => $this->jsonAddressLike('m.bcc_addresses', $value, $parameters), @@ -446,6 +451,45 @@ final class EmailFilterCompiler return '('.implode(' OR ', $parts).')'; } + /** + * The sender's address, whole and case-insensitively. + * + * Equality on the address column alone — not the display name, which the + * sender chooses and which `from` matches for exactly that reason. + * + * @param array $parameters + */ + private function fromAddress(mixed $value, array &$parameters): string + { + if (false === FilterVocabulary::isSenderValue('fromAddress', $value)) { + throw new MethodException('invalidArguments', '"fromAddress" must be a whole email address.'); + } + + $name = $this->bind(mb_strtolower((string) $value), $parameters); + + return sprintf('LOWER(m.from_address) = :%s', $name); + } + + /** + * Everything after the `@` of the sender's address, whole. + * + * A suffix match anchored on the `@`, which is what makes it the domain + * and not the end of one: `example.com` matches `a@example.com` and + * neither `a@notexample.com` nor `a@mail.example.com`. + * + * @param array $parameters + */ + private function fromDomain(mixed $value, array &$parameters): string + { + if (false === FilterVocabulary::isSenderValue('fromDomain', $value)) { + throw new MethodException('invalidArguments', '"fromDomain" must be a domain, without the @.'); + } + + $name = $this->bind('%@'.$this->escapeLike(mb_strtolower((string) $value)), $parameters); + + return sprintf('LOWER(m.from_address) LIKE :%s', $name); + } + /** * to/cc/bcc are json arrays of {name, address}; matching is a substring * scan over the serialised array, which covers both fields at once. diff --git a/src/Service/Mail/SpamSenderResolver.php b/src/Service/Mail/SpamSenderResolver.php new file mode 100644 index 00000000..8d27a109 --- /dev/null +++ b/src/Service/Mail/SpamSenderResolver.php @@ -0,0 +1,104 @@ +ownAddresses($user); + $newest = null; + + foreach ($thread->messages as $message) { + $address = $this->addressOf($message); + + if (null === $address || true === in_array($address, $own, true)) { + continue; + } + + if (null === $newest || $this->arrivedAt($message) > $this->arrivedAt($newest)) { + $newest = $message; + } + } + + if (null === $newest) { + return null; + } + + $address = (string) $this->addressOf($newest); + $domain = substr($address, (int) strrpos($address, '@') + 1); + + return new SpamSender($address, $domain, null !== $this->presets->findByDomain($domain)); + } + + /** The From address, lower-cased, or null when it is not an address. */ + private function addressOf(Message $message): ?string + { + $address = mb_strtolower(trim((string) $message->fromAddress)); + + if (false === filter_var($address, FILTER_VALIDATE_EMAIL)) { + return null; + } + + return $address; + } + + private function arrivedAt(Message $message): int + { + return ($message->receivedAt ?? $message->sentAt)?->getTimestamp() ?? 0; + } + + /** + * Every address the user sends as: each account's own, and its aliases. + * + * Asked of the repository rather than read off `$user->accounts`: this is + * the list that keeps a filter off the user's own address, and it should + * not depend on whether that collection happens to be loaded and current. + * + * @return list + */ + private function ownAddresses(User $user): array + { + $own = []; + + foreach ($this->accounts->findBy(['usr' => $user]) as $account) { + $own[] = mb_strtolower(trim((string) $account->email)); + + foreach ($account->aliases as $alias) { + $own[] = mb_strtolower(trim($alias->address)); + } + } + + return $own; + } +} diff --git a/src/Service/Mail/StatusUndoService.php b/src/Service/Mail/StatusUndoService.php index 9ccd40b2..2b3600d3 100644 --- a/src/Service/Mail/StatusUndoService.php +++ b/src/Service/Mail/StatusUndoService.php @@ -8,9 +8,11 @@ use App\Domain\Enum\Mail\LabelRole; use App\Entity\Label\Label; use App\Entity\Mail\Account; use App\Entity\Mail\Message; +use App\Entity\Rule\MailRule; use App\Entity\User\User; use App\Repository\Label\LabelRepository; use App\Repository\Mail\MessageRepository; +use App\Repository\Rule\MailRuleRepository; use DateTimeImmutable; use Doctrine\ORM\EntityManagerInterface; use Symfony\Component\HttpFoundation\RequestStack; @@ -77,6 +79,7 @@ final readonly class StatusUndoService private MessageRepository $messages, private LabelRepository $labels, private ThreadStatusUpdater $status, + private MailRuleRepository $rules, ) { } @@ -119,6 +122,31 @@ final readonly class StatusUndoService return $token; } + /** + * Make an Undo of this action take a filter back with it. + * + * For the spam button, whose one click can both move a conversation and + * create the rule that does the same to the sender's next mail. Undoing + * half of that would leave a filter the person has just said they did not + * mean. Called only for a rule the action itself created. + * + * A token that is no longer kept is left alone: there is nothing to attach + * to, and the rule is still listed under Settings → Filters. + */ + public function alsoRemove(string $token, MailRule $rule): void + { + $session = $this->requestStack->getSession(); + $kept = (array) $session->get(self::SESSION_KEY, []); + + if (false === isset($kept[$token]) || null === $rule->id) { + return; + } + + $kept[$token]['rule'] = $rule->id; + + $session->set(self::SESSION_KEY, $kept); + } + /** * Put back what remember() wrote down. * @@ -144,6 +172,8 @@ final readonly class StatusUndoService unset($kept[$token]); $session->set(self::SESSION_KEY, $kept); + $this->removeRule($snapshot['rule'] ?? null, $user); + $messages = []; foreach ($this->messages->findBy(['id' => array_keys($snapshot['messages'])]) as $message) { @@ -174,6 +204,26 @@ final readonly class StatusUndoService // ── Private ─────────────────────────────────────────────────────────────── + /** + * The filter alsoRemove() attached, if there was one and it is still the + * user's. Gone already — deleted by hand in the seconds between — is fine. + */ + private function removeRule(mixed $id, User $user): void + { + if (false === is_int($id)) { + return; + } + + $rule = $this->rules->find($id); + + if (null === $rule || $rule->usr?->id !== $user->id) { + return; + } + + $this->em->remove($rule); + $this->em->flush(); + } + /** * Move every message that is not where it was back to where it was. * diff --git a/src/Service/Rule/SpamFilterCreator.php b/src/Service/Rule/SpamFilterCreator.php new file mode 100644 index 00000000..00476436 --- /dev/null +++ b/src/Service/Rule/SpamFilterCreator.php @@ -0,0 +1,104 @@ +domainPattern() : $sender->address; + } + + /** + * The one condition such a filter has. + * + * @return array + */ + private function condition(SpamSender $sender, string $scope): array + { + return self::DOMAIN === $scope + ? ['fromDomain' => $sender->domain] + : ['fromAddress' => $sender->address]; + } + + /** + * The rule, and whether this call made it. + * + * A second press on mail from the same sender finds the first one's rule + * and makes nothing: two identical filters do nothing one does not, and + * the list under Settings → Filters would grow a row per press. The flag + * is what an Undo reads — it takes back a rule this press made, and leaves + * alone one that was already there. + * + * @return array{0: MailRule, 1: bool} + */ + public function create(User $user, SpamSender $sender, string $scope): array + { + $pattern = $this->pattern($sender, $scope); + $conditions = ['operator' => 'AND', 'conditions' => [$this->condition($sender, $scope)]]; + $actions = [['type' => RuleActionExecutor::MARK_SPAM]]; + + foreach ($this->rules->findForUserOrdered($user) as $existing) { + if ($existing->conditions == $conditions && $existing->actions == $actions && null === $existing->account) { + return [$existing, false]; + } + } + + // Built here from two checked strings, so this cannot fail today. It + // is still the gate every stored condition goes through, because the + // tree feeds a SQL compiler. + $this->validator->validate($conditions); + + $rule = new MailRule(); + $rule->usr = $user; + $rule->name = $this->translator->trans('settings.filters.spam_rule_name', ['%sender%' => $pattern]); + $rule->conditions = $conditions; + $rule->actions = $actions; + $rule->sortOrder = $this->rules->nextSortOrder($user); + + $this->em->persist($rule); + $this->em->flush(); + + return [$rule, true]; + } +} diff --git a/templates/_partials/_list_toolbar.html.twig b/templates/_partials/_list_toolbar.html.twig index 7ec506dc..65220881 100644 --- a/templates/_partials/_list_toolbar.html.twig +++ b/templates/_partials/_list_toolbar.html.twig @@ -222,6 +222,11 @@ The button itself carries `group` and `relative`. > {{ m.tbtn('click->mail--list-toolbar#archiveSelected', 'fa-solid fa-box-archive', 'toolbar.actions.archive'|trans) }} {{ m.tbtn('click->mail--list-toolbar#deleteSelected', 'fa-solid fa-trash', 'toolbar.actions.delete'|trans, 'hover:text-danger') }} + {# Not where the selection is already thrown away. In the bin the move + would pull mail back out of it and into Spam; in Spam it is nothing. #} + {% if list_scope|default('') not in ['spam', 'trash'] %} + {{ m.tbtn('click->mail--list-toolbar#spamSelected', 'fa-solid fa-circle-exclamation', 'toolbar.actions.spam'|trans, 'hover:text-danger') }} + {% endif %} {{ m.tbtn('click->mail--list-toolbar#markReadSelected', 'fa-solid fa-envelope-open', 'toolbar.actions.mark_read'|trans) }} {{ m.tbtn('click->mail--list-toolbar#markUnreadSelected', 'fa-solid fa-envelope', 'toolbar.actions.mark_unread'|trans) }} {# A menu, not a direct action: this used to hardcode "tomorrow 8am" diff --git a/templates/_partials/_sidebar.html.twig b/templates/_partials/_sidebar.html.twig index ac90478e..2d9ac2b1 100644 --- a/templates/_partials/_sidebar.html.twig +++ b/templates/_partials/_sidebar.html.twig @@ -663,9 +663,9 @@ controller instead — the scroll position and which trees are open. {% if _showSpam %} {% set _href = path('app_mail_spam') %}
{# The tier rides on the LINK, not on the pill around it. The pill is what you see — .nav-item draws it — but the row's height comes from diff --git a/templates/_partials/_spam_menu.html.twig b/templates/_partials/_spam_menu.html.twig new file mode 100644 index 00000000..7f771361 --- /dev/null +++ b/templates/_partials/_spam_menu.html.twig @@ -0,0 +1,65 @@ +{# +_spam_menu.html.twig +───────────────────────────────────────────────────────────────────────────── +The spam button, for ONE conversation: in the reading pane's toolbar and among +a list row's hover actions. Pressing it opens a small menu — move this to Spam, +or move it and also filter the sender, or the sender's whole domain. + +The panel is empty here and filled when the button is pressed +(SpamController::menu). What it offers depends on who the conversation is from, +and working that out for every row of a list, to render fifty menus nobody +opens, is the wrong trade. + +The list toolbar does not use this. A selection has no one sender, so its +button moves the selection and offers nothing more (mail--list-toolbar). + +Variables: + threadId int the conversation + variant string `toolbar` (reading pane) or `row` (list hover actions): + which button it is drawn as, and whether the pane showing + the conversation closes afterwards +#} +{% set variant = variant|default('toolbar') %} + +
+ + + + {# A click anywhere in the panel stays in the panel. In a list it is a + descendant of the row, and a row opens its conversation when clicked. + + `visible` for the same reason: a row's actions are `invisible` until the + row is hovered, visibility is inherited, and an open menu must not + depend on where the pointer happens to be. #} + +
diff --git a/templates/_partials/_thread_row.html.twig b/templates/_partials/_thread_row.html.twig index 28cc343d..c5a4403f 100644 --- a/templates/_partials/_thread_row.html.twig +++ b/templates/_partials/_thread_row.html.twig @@ -652,7 +652,7 @@ be a worse regression than the one this feature fixes. them. Stacked there is no hover, so it simply sits there. #} + @xl:group-hover:invisible @xl:group-has-[[aria-expanded=true]]:invisible"> {# The visibility classes go on a wrapper, never on the itself: Font Awesome's own .fa-solid rule sets display, and it lands after Tailwind's utilities, so `hidden` on @@ -689,7 +689,11 @@ be a worse regression than the one this feature fixes. {% endif %} -