diff --git a/assets/controllers/admin/report_selection_controller.js b/assets/controllers/admin/report_selection_controller.js new file mode 100644 index 00000000..4002e0f8 --- /dev/null +++ b/assets/controllers/admin/report_selection_controller.js @@ -0,0 +1,123 @@ +import { Controller } from "@hotwired/stimulus"; + +/** + * Choose which reports to take, and keep the copy/download text in step. + * + * WHY IT DOES NOT COPY ANYTHING ITSELF. It writes the chosen lines into the + * `sink` element, which is also ui--clipboard's `source` target on the same + * wrapper — so copying stays entirely that controller's job, including the + * execCommand path a self-hosted plMail on plain HTTP depends on. A second copy + * implementation here would be a second one to keep working. + * + * Download is its own, because it has no such prior art: a Blob and an , which is the whole of it. + * + * Everything starts CHECKED. The common case is still "take the lot", and a + * panel that opens with nothing selected asks every reader to do work before + * they can do the thing the card is for. Selecting is for the reader who wants + * one group out of five. + * + * Targets: + * row — a report's checkbox. `data-line` carries the text it stands for. + * group — a group's checkbox. `data-group` matches the rows' own. + * all — the select-everything checkbox. + * sink — hidden; receives the selected lines, one per line. + * count — optional; its text becomes "n selected". + * action — buttons disabled while nothing is selected. + */ +export default class extends Controller { + static targets = ["row", "group", "all", "sink", "count", "action"]; + + static values = { + // "%count% selected" — translated at the call site. + countLabel: { type: String, default: "%count% selected" }, + filename: { type: String, default: "category-reports.txt" }, + }; + + connect() { + this.#sync(); + } + + /** A single report was toggled. */ + toggleRow() { + this.#sync(); + } + + /** A group heading was toggled: its rows follow it. */ + toggleGroup(event) { + const group = event.target.dataset.group; + + for (const row of this.rowTargets) { + if (row.dataset.group === group) { + row.checked = event.target.checked; + } + } + + this.#sync(); + } + + toggleAll(event) { + for (const row of this.rowTargets) { + row.checked = event.target.checked; + } + + this.#sync(); + } + + download() { + const text = this.sinkTarget.textContent; + + if ("" === text) { + return; + } + + // Blob rather than a data: URI — a hundred reports is comfortably past + // the length some browsers will follow in an href. + const url = URL.createObjectURL(new Blob([text + "\n"], { type: "text/plain" })); + const link = document.createElement("a"); + + link.href = url; + link.download = this.filenameValue; + document.body.appendChild(link); + link.click(); + link.remove(); + + // Not revoked immediately: Firefox has cancelled the download of a URL + // revoked in the same tick. + setTimeout(() => URL.revokeObjectURL(url), 10_000); + } + + /** + * The chosen lines into the sink, and every indicator into agreement. + * + * Group and select-all boxes are DERIVED here rather than tracked, so a row + * unticked by hand shows up immediately as a half-ticked group above it. + */ + #sync() { + const chosen = this.rowTargets.filter((row) => row.checked); + + this.sinkTarget.textContent = chosen.map((row) => row.dataset.line).join("\n"); + + for (const group of this.groupTargets) { + const rows = this.rowTargets.filter((row) => row.dataset.group === group.dataset.group); + const ticked = rows.filter((row) => row.checked).length; + + group.checked = 0 < ticked && ticked === rows.length; + group.indeterminate = 0 < ticked && ticked < rows.length; + } + + if (true === this.hasAllTarget) { + this.allTarget.checked = 0 < chosen.length && chosen.length === this.rowTargets.length; + this.allTarget.indeterminate = + 0 < chosen.length && chosen.length < this.rowTargets.length; + } + + if (true === this.hasCountTarget) { + this.countTarget.textContent = this.countLabelValue.replace("%count%", chosen.length); + } + + for (const action of this.actionTargets) { + action.disabled = 0 === chosen.length; + } + } +} diff --git a/assets/controllers/mail/category_report_controller.js b/assets/controllers/mail/category_report_controller.js index dcdea954..359a050b 100644 --- a/assets/controllers/mail/category_report_controller.js +++ b/assets/controllers/mail/category_report_controller.js @@ -20,6 +20,7 @@ export default class extends Controller { token: String, sentLabel: { type: String, default: "Reported" }, failedLabel: { type: String, default: "Could not report" }, + staleLabel: { type: String, default: "Already there — reload" }, }; async send() { @@ -41,6 +42,16 @@ export default class extends Controller { }), }); + // 409 means the message is already in the tab that was chosen — + // this page was drawn before something moved it. Its own sentence, + // because "could not report" would send somebody looking for a + // fault when the answer is that they got what they wanted. + if (409 === response.status) { + this.#settle(this.staleLabelValue, "fa-rotate", false); + + return; + } + if (false === response.ok) { this.#settle(this.failedLabelValue, "fa-triangle-exclamation", true); diff --git a/migrations/Version20260904170000.php b/migrations/Version20260904170000.php new file mode 100644 index 00000000..a6bcc205 --- /dev/null +++ b/migrations/Version20260904170000.php @@ -0,0 +1,68 @@ +addSql(<<<'SQL' + ALTER TABLE category_report + ADD filed_message VARCHAR(16) DEFAULT NULL, + ADD pinned BOOLEAN DEFAULT FALSE NOT NULL, + ADD override_provider BOOLEAN DEFAULT FALSE NOT NULL, + ADD ai_asked BOOLEAN DEFAULT FALSE NOT NULL, + ADD has_plain_text BOOLEAN DEFAULT FALSE NOT NULL, + ADD list_id VARCHAR(255) DEFAULT NULL + SQL); + } + + public function down(Schema $schema): void + { + $this->addSql(<<<'SQL' + ALTER TABLE category_report + DROP filed_message, + DROP pinned, + DROP override_provider, + DROP ai_asked, + DROP has_plain_text, + DROP list_id + SQL); + } +} diff --git a/src/Controller/Mail/CategoryReportController.php b/src/Controller/Mail/CategoryReportController.php index 72bea6aa..7223ac9a 100644 --- a/src/Controller/Mail/CategoryReportController.php +++ b/src/Controller/Mail/CategoryReportController.php @@ -70,7 +70,28 @@ final class CategoryReportController extends AbstractController return new Response(status: Response::HTTP_BAD_REQUEST); } - $em->persist($this->recorder->record($user, $message, $shouldBe)); + $report = $this->recorder->record($user, $message, $shouldBe); + + // ALREADY THERE, so there is nothing to report. + // + // The page offers every tab EXCEPT the one the conversation is in, so + // a reader can only ever choose a different one — but the report is + // written against the category as it stands when the button is pressed, + // and a re-sort or a re-classification between those two moments moves + // it. What lands then is a row saying a message should be where it + // already is: true, and carrying nothing. + // + // Cheap to guard and worth guarding, because a handful of these in a + // list of twenty is enough to make somebody distrust the whole list. + // + // 409 rather than 400: the request was well formed and the reader did + // nothing wrong. Their page is behind, which is what the card says + // back to them. + if ($report->filed === $shouldBe) { + return new Response(status: Response::HTTP_CONFLICT); + } + + $em->persist($report); $em->flush(); // 204 and no redirect: this is pressed inside a popover on a thread the diff --git a/src/Entity/Monitoring/CategoryReport.php b/src/Entity/Monitoring/CategoryReport.php index 6335a69c..8f6a1041 100644 --- a/src/Entity/Monitoring/CategoryReport.php +++ b/src/Entity/Monitoring/CategoryReport.php @@ -66,10 +66,33 @@ class CategoryReport #[ORM\Column] public int $messageId; - /** Where it actually was when somebody objected. */ + /** + * Where it actually was when somebody objected — the THREAD's category. + * + * The mailbox lists threads and the tabs are a filter over `thread.category`, + * so this is the only column that answers "which tab was it in". This read + * `message.category` first, and the very first report this feature received + * said `filed:updates` about a conversation its owner was looking at in + * Primary. True, and about a column nobody can see. + */ #[ORM\Column(length: 16, enumType: MessageCategory::class)] public MessageCategory $filed; + /** + * The reported message's own category, when it disagrees with its thread. + * + * Null when they agree, which is the ordinary case. When they do not, the + * conversation is being placed by a sibling message or by a pin rather than + * by anything on the message in front of the reader — and that is a + * different problem from a misclassification, with a different fix. + */ + #[ORM\Column(length: 16, nullable: true, enumType: MessageCategory::class)] + public ?MessageCategory $filedMessage = null; + + /** Somebody put this conversation where it is by hand. Nothing else applies. */ + #[ORM\Column] + public bool $pinned = false; + /** Where the reader says it belongs. The whole point of the button. */ #[ORM\Column(length: 16, enumType: MessageCategory::class)] public MessageCategory $shouldBe; @@ -92,10 +115,54 @@ class CategoryReport #[ORM\Column(length: 16)] public string $source; + /** + * Whether that reader had asked for the provider's labels to be overruled. + * + * `source` alone does not say it, and without it a line reading + * `gmail:updates ... by:assistant` is unreadable: it does not say whether + * Gmail was consulted and lost, or never consulted at all. + */ + #[ORM\Column] + public bool $overrideProvider = false; + + /** + * Whether the model was ever actually asked. + * + * `model:-` covers two completely different failures — never asked, and + * asked and returned nothing — and only one of them is a prompt problem. + */ + #[ORM\Column] + public bool $aiAsked = false; + + /** + * Whether the message had a plain-text part for the classifier to read. + * + * The single most decisive input that is not visible anywhere else. Most + * marketing mail is HTML-only, the classifier is handed `(no plain text + * part)`, and it is then deciding on headers and a subject alone. A report + * about a mail the model never really saw is not evidence about the prompt. + */ + #[ORM\Column] + public bool $hasPlainText = false; + /** The bulk headers the message carries, names only. Never their values. */ #[ORM\Column(length: 255)] public string $bulkHeaders = ''; + /** + * List-Id's VALUE, which is the exception to the names-only rule above. + * + * The others are kept as names because their values are tracking links — + * per-recipient, useless twice, and the most identifying thing in the + * message. List-Id is the opposite: a stable name for the mailing itself + * (``), identical for every recipient, and the exact + * string a rule would be written against. Reporting that a shop's offers + * are misfiled without it leaves whoever reads this guessing at the + * identifier they would need. + */ + #[ORM\Column(length: 255, nullable: true)] + public ?string $listId = null; + #[ORM\Column(length: 320)] public string $fromAddress = ''; @@ -123,17 +190,30 @@ class CategoryReport */ public function asLine(): string { + // Where it sat, and — only when they disagree — what the reported + // message itself said and whether a pin is holding it there. + $notes = array_filter([ + null !== $this->filedMessage ? 'msg:' . $this->filedMessage->value : null, + $this->pinned ? 'pinned' : null, + ]); + return sprintf( - "%s | filed:%s should:%s | gmail:%s rules:%s%s model:%s | by:%s | headers:%s | from:%s <%s> | %s", + "%s | filed:%s%s should:%s | gmail:%s rules:%s%s model:%s | by:%s%s | headers:%s%s | body:%s | from:%s <%s> | %s", $this->createdAt->format('Y-m-d'), $this->filed->value, + [] === $notes ? '' : '(' . implode(',', $notes) . ')', $this->shouldBe->value, null === $this->gmail ? '-' : $this->gmail->value, null === $this->rules ? '-' : $this->rules->value, null !== $this->rulesSignal ? '(' . $this->rulesSignal . ')' : '', - null === $this->model ? '-' : $this->model->value, + // Three states, not two: never asked is not the same failure as + // asked and got nothing, and only the second is about the prompt. + null !== $this->model ? $this->model->value : ($this->aiAsked ? 'noanswer' : 'notasked'), $this->source, + $this->overrideProvider ? ' override:yes' : '', '' === $this->bulkHeaders ? 'none' : $this->bulkHeaders, + null !== $this->listId ? ' list-id:' . $this->listId : '', + $this->hasPlainText ? 'text' : 'none', $this->fromName, $this->fromAddress, $this->subject, diff --git a/src/Service/Mail/CategoryReportRecorder.php b/src/Service/Mail/CategoryReportRecorder.php index a347d8f2..9441eabc 100644 --- a/src/Service/Mail/CategoryReportRecorder.php +++ b/src/Service/Mail/CategoryReportRecorder.php @@ -60,25 +60,69 @@ final readonly class CategoryReportRecorder $report = new CategoryReport(); - $report->usr = $user; - $report->messageId = (int) $message->id; - $report->filed = $message->category ?? MessageCategory::Primary; - $report->shouldBe = $shouldBe; - $report->gmail = null === $message->gmailLabelIds + $thread = $message->thread; + + // THE THREAD'S CATEGORY, because that is the one the tabs filter on and + // therefore the only one that answers "which tab was it in". The + // message's own is an input to it — resolved most-recent-wins across + // the conversation, overridden outright by a pin — and the two are free + // to differ on any thread with more than one message. Recording the + // message's made this feature's first real report read `filed:updates` + // about a conversation its owner was reading in Primary. + $filed = $thread->category ?? $message->category ?? MessageCategory::Primary; + + $report->usr = $user; + $report->messageId = (int) $message->id; + $report->filed = $filed; + + // Only when they disagree. Then the placement is coming from a sibling + // message or a pin rather than from the message in front of the reader, + // which is a different problem with a different fix. + $report->filedMessage = $message->category !== $filed ? $message->category : null; + $report->pinned = null !== $thread?->categoryPinnedAt; + $report->shouldBe = $shouldBe; + $report->gmail = null === $message->gmailLabelIds ? null : MessageCategory::fromGmailLabels($message->gmailLabelIds); - $report->rules = $rules['category']; - $report->rulesSignal = $rules['signal']; - $report->model = $message->aiCategory; - $report->source = $user->categorySorting->source; - $report->bulkHeaders = self::bulkHeaders($message); - $report->fromAddress = mb_substr((string) $message->fromAddress, 0, 320); - $report->fromName = mb_substr((string) $message->fromName, 0, 255); - $report->subject = (string) $message->subject; + $report->rules = $rules['category']; + $report->rulesSignal = $rules['signal']; + $report->model = $message->aiCategory; + + // Asked and answered / asked and got nothing / never asked. `model:-` + // could not tell the last two apart, and only one of them is evidence + // about the prompt. + $report->aiAsked = null !== $message->aiCategorisedAt; + $report->source = $user->categorySorting->source; + $report->overrideProvider = $user->categorySorting->overrideProvider; + + // What the classifier could actually read. HTML-only mail is handed + // `(no plain text part)` and decided on headers and a subject alone — + // worth knowing before treating a report as evidence about a prompt. + $report->hasPlainText = '' !== trim((string) $message->bodyText); + $report->bulkHeaders = self::bulkHeaders($message); + $report->listId = self::listId($message); + $report->fromAddress = mb_substr((string) $message->fromAddress, 0, 320); + $report->fromName = mb_substr((string) $message->fromName, 0, 255); + $report->subject = (string) $message->subject; return $report; } + /** + * List-Id's value — the one bulk header whose value is worth keeping. + * + * The others are per-recipient tracking links. This is a stable name for + * the mailing itself, identical for everyone who receives it, and the exact + * string somebody would write a rule against. + */ + private static function listId(Message $message): ?string + { + $headers = array_change_key_case($message->headers ?? [], CASE_LOWER); + $listId = trim((string) ($headers['list-id'] ?? '')); + + return '' === $listId ? null : mb_substr($listId, 0, 255); + } + /** Names only. The values are tracking links and say nothing extra. */ private static function bulkHeaders(Message $message): string { diff --git a/src/Twig/CategoryReportExtension.php b/src/Twig/CategoryReportExtension.php index af1a7a6e..4e81b373 100644 --- a/src/Twig/CategoryReportExtension.php +++ b/src/Twig/CategoryReportExtension.php @@ -19,9 +19,15 @@ use Twig\TwigFunction; * connection test, which is exactly when somebody is fiddling with the model * and most likely to want to see what it has been getting wrong. * - * Flattened to lines here rather than handed over as entities, because the - * PRODUCT of this feature is text somebody pastes somewhere else. The template - * renders what it is given and owns no format. + * Grouped by the PROBLEM here rather than handed over as entities, because + * that is the unit somebody acts on. Twenty reports are rarely twenty problems: + * they are "shop mail keeps landing in Primary" four times and "a person keeps + * landing in Promotions" once, and the fix for one is not the fix for the other. + * Grouping by `filed → should` puts each of those together and lets whoever is + * changing a rule take the group that is about their rule and leave the rest. + * + * The line itself is still built on the entity — it is the PRODUCT, and a format + * assembled here would be a format that changes with the panel. */ final class CategoryReportExtension extends AbstractExtension { @@ -36,14 +42,61 @@ final class CategoryReportExtension extends AbstractExtension ]; } - /** @return array{lines: list, count: int} */ + /** + * @return array{ + * count: int, + * lines: list, + * groups: list, + * }>, + * } + */ private function reports(): array { - $rows = $this->reports->recent(); + $rows = $this->reports->recent(); + $groups = []; + + foreach ($rows as $report) { + $key = $report->filed->value . '>' . $report->shouldBe->value; + + if (false === isset($groups[$key])) { + $groups[$key] = [ + 'key' => $key, + 'filed' => $report->filed->value, + 'shouldBe' => $report->shouldBe->value, + 'count' => 0, + 'rows' => [], + ]; + } + + ++$groups[$key]['count']; + + $groups[$key]['rows'][] = [ + 'id' => (int) $report->id, + 'line' => $report->asLine(), + 'date' => $report->createdAt->format('Y-m-d'), + // One string: the table has a column for "who", and a name + // without its address is not enough to write a rule against + // while an address without its name is unreadable. + 'from' => '' === $report->fromName + ? $report->fromAddress + : $report->fromName . ' <' . $report->fromAddress . '>', + 'subject' => $report->subject, + ]; + } + + // Biggest problem first. A group of six is a rule worth changing; a + // group of one is somebody's odd mail, and it belongs below. + usort($groups, static fn (array $a, array $b): int => $b['count'] <=> $a['count']); return [ - 'lines' => array_map(static fn (CategoryReport $r): string => $r->asLine(), $rows), - 'count' => count($rows), + 'count' => count($rows), + 'lines' => array_map(static fn (CategoryReport $r): string => $r->asLine(), $rows), + 'groups' => $groups, ]; } } diff --git a/src/Twig/MessageCategoryExtension.php b/src/Twig/MessageCategoryExtension.php index ea20f95c..a0acacb5 100644 --- a/src/Twig/MessageCategoryExtension.php +++ b/src/Twig/MessageCategoryExtension.php @@ -72,6 +72,7 @@ final class MessageCategoryExtension extends AbstractExtension /** * @return array{ * filed: string|null, + * filedMessage: string|null, * effective: array{category: string, reason: string, signal: string|null}, * gmail: string|null, * rules: array{category: string, reason: string, signal: string|null}, @@ -98,18 +99,38 @@ final class MessageCategoryExtension extends AbstractExtension $sorting = $user->categorySorting; return [ - // WHERE IT ACTUALLY IS, read straight off the row rather than - // worked out again — and this row is why the panel was wrong. + // WHERE IT ACTUALLY IS — off the THREAD, which is the row the + // tabs are a filter over. // - // Everything else here is a RECOMPUTATION, which is the right shape - // for "what would each of them say"; it is the wrong shape for - // "where is this mail". The two came apart the moment sorting - // became a setting: the effective line was computed with the - // shipped defaults and announced "Updates — Gmail said so" about a - // message sitting in Primary, because the reader had asked for the - // model and for Gmail to be overruled. A panel that explains a - // mailbox it is not looking at is worse than one that says nothing. - 'filed' => $message->category?->value, + // Two mistakes have been made here and this is the second one. The + // first was recomputing it: everything else in this array is a + // recomputation, which is the right shape for "what would each of + // them say" and the wrong shape for "where is this mail", and the + // panel announced "Updates — Gmail said so" about a message sitting + // in Primary. So it was read off the row instead. + // + // It was read off the WRONG row. The mailbox lists threads and + // filters on `thread.category`; `message.category` is an input to + // that, resolved most-recent-wins across the conversation and + // overridden entirely by a pin. On any thread with more than one + // message the two are free to differ, and when they do it is the + // thread that decides which tab the reader is looking at. A report + // filed from this panel said `filed:updates` about a conversation + // sitting in Primary, which is a true statement about a column + // nobody can see. + // + // Falls back to the message for a message not yet threaded. + 'filed' => ($message->thread->category ?? $message->category)?->value, + + // The message's own row, and only when it disagrees with the + // thread. Not decoration: a disagreement means the conversation is + // being decided by a sibling message or a pin rather than by + // anything on this one, which is the difference between a rule to + // fix and nothing to fix at all. + 'filedMessage' => null !== $message->thread?->category + && $message->thread->category !== $message->category + ? $message->category?->value + : null, // The same cascade the reader's own settings produce, so the reason // beside the category above is the reason for THAT category. diff --git a/templates/admin/ai/_frame.html.twig b/templates/admin/ai/_frame.html.twig index 5a85cec6..dfcaca89 100644 --- a/templates/admin/ai/_frame.html.twig +++ b/templates/admin/ai/_frame.html.twig @@ -328,60 +328,4 @@ poll would empty a field mid-word. - {# What the sorting has been getting wrong, in the words of the people - reading it. Here rather than in a nav entry of its own because this is - where somebody ACTS on it: the prompt and the model are a card or two - above, and a report is only worth keeping if the thing it is evidence - about is within reach. - - The product is the TEXT, not the table. Whoever reads this is going to - paste it somewhere — into a prompt, an issue, a message to whoever is - changing a rule — so the copy button takes the whole block and the rows - are there to be skimmed before deciding whether it is worth pasting. #} - {% set reports = category_reports() %} - {% if reports.count > 0 %} -
-
-
-

{{ 'admin.ai.reports.heading'|trans }}

-

- {{ 'admin.ai.reports.subheading'|trans({'%count%': reports.count}) }} -

-
- -
- - - - - {# Clearing is how a list of grievances stops being a list of - the same grievance twenty times. These are somebody's - observations about their own mail and nothing regenerates - them, so it asks first. #} -
- - -
-
-
- -
- 
{{ reports.lines|join("\n") }}
-
-
- {% endif %} diff --git a/templates/admin/insight_reports/_frame.html.twig b/templates/admin/insight_reports/_frame.html.twig index cb72a058..51fc1d9d 100644 --- a/templates/admin/insight_reports/_frame.html.twig +++ b/templates/admin/insight_reports/_frame.html.twig @@ -260,4 +260,156 @@ list back, so nothing here navigates the page. {% endif %} + {# What the SORTING has been getting wrong, beside what the insights have. + Two different reports about two different features, and they live + together because of where somebody looks for them. + + This started in Admin → AI, on the reasoning that a report should sit + beside the thing it is evidence about — the prompt and the model are + configured there. That lost to the plainer fact that this section is + called "Reported mail": somebody who has just pressed a button labelled + Report goes looking for what it did under the heading that matches, finds + an empty list, and concludes the button is broken. Which is exactly what + happened. + + The product is the TEXT, not the table. Whoever reads this is going to + paste it somewhere — into a prompt, an issue, a message to whoever is + changing a rule — so the copy button takes the whole block and the rows + are there to be skimmed before deciding whether it is worth pasting. #} + {% set reports = category_reports() %} + {% if reports.count > 0 %} + {# GROUPED BY THE PROBLEM, not listed by date. Twenty reports are rarely + twenty problems — they are "this shop keeps landing in Primary" six + times and one odd one — and the person reading this is deciding + whether to change a rule, which is a decision about a group. The + selection exists so they can take the group that is about their rule + and leave the four that are not. + + Copying is ui--clipboard's, on the same wrapper: the selection + controller only keeps the hidden block in step with the ticks, so the + execCommand path that a plain-HTTP installation needs stays in one + place rather than being written twice. #} +
+ + {# The selected lines, and the only thing either button acts on. #} + + +
+
+

{{ 'admin.ai.reports.heading'|trans }}

+

+ {{ 'admin.ai.reports.subheading'|trans({'%count%': reports.count}) }} +

+
+ +
+ + + + + + + {# Clearing is how a list of grievances stops being a list of + the same grievance twenty times. These are somebody's + observations about their own mail and nothing regenerates + them, so it asks first. #} +
+ + +
+
+
+ + + +
+ {% for group in reports.groups %} +
+ + + {% for row in group.rows %} + + {% endfor %} +
+ {% endfor %} +
+
+ {% endif %} diff --git a/templates/mail/_message_details.html.twig b/templates/mail/_message_details.html.twig index 4e8ce154..3f8f3c88 100644 --- a/templates/mail/_message_details.html.twig +++ b/templates/mail/_message_details.html.twig @@ -228,12 +228,17 @@ mailed-by and signed-by were silently missing from every IMAP message. #}
{{ 'message.details.category'|trans }}
- {# WHERE IT ACTUALLY IS, off the row. The three lines below - are recomputations — "what would each of them say" — and - this one is the answer to a different question entirely. - Reading it off the row is also the only way the panel can - be honest when they disagree, which they do whenever a - re-file is still queued. #} + {# WHERE IT ACTUALLY IS, off the THREAD's row — the one the + tabs are a filter over. The three lines below are + recomputations, "what would each of them say"; this one + answers a different question entirely, and reading it off + a row is the only way the panel can be honest when they + disagree, which they do whenever a re-file is queued. + + It read the MESSAGE's row until it put a report's name to + the problem: a conversation sitting in Primary, described + here as Updates, because a sibling message carried the + thread and this panel was reading a column no tab shows. #} {{ ('tabs.' ~ (category.filed ?? category.effective.category))|trans }} {%- if category.pinnedAt %} @@ -255,6 +260,20 @@ mailed-by and signed-by were silently missing from every IMAP message. {% if not category.pinnedAt and category.filed and category.filed != category.effective.category %} {{ 'message.details.categorised.awaiting_resort'|trans }} {% endif %} + + {# THE CONVERSATION IS NOT WHERE THIS MESSAGE WOULD PUT IT. + A thread takes one category for all of it, most recent + wins, so a reply can carry the whole conversation out of + the tab an earlier message belongs in. Without this the + panel reads as simply wrong: every line agrees on Updates + and the mail is plainly sitting in Primary. #} + {% if category.filedMessage %} + + {{ 'message.details.categorised.thread_differs'|trans({ + '%category%': ('tabs.' ~ category.filedMessage)|trans, + }) }} + + {% endif %}
{# @@ -356,7 +375,8 @@ mailed-by and signed-by were silently missing from every IMAP message. data-mail--category-report-url-value="{{ path('app_mail_category_report', {'id': message.id}) }}" data-mail--category-report-token-value="{{ csrf_token('category_report') }}" data-mail--category-report-sent-label-value="{{ 'message.details.report.sent'|trans }}" - data-mail--category-report-failed-label-value="{{ 'message.details.report.failed'|trans }}"> + data-mail--category-report-failed-label-value="{{ 'message.details.report.failed'|trans }}" + data-mail--category-report-stale-label-value="{{ 'message.details.report.stale'|trans }}">
{{ 'message.details.report.label'|trans }} diff --git a/tests/Service/Mail/CategoryReportRecorderTest.php b/tests/Service/Mail/CategoryReportRecorderTest.php new file mode 100644 index 00000000..4a897c38 --- /dev/null +++ b/tests/Service/Mail/CategoryReportRecorderTest.php @@ -0,0 +1,82 @@ +record(MessageCategory::Primary, MessageCategory::Updates); + + self::assertSame(MessageCategory::Primary, $report->filed); + self::assertSame(MessageCategory::Updates, $report->filedMessage); + self::assertStringContainsString('filed:primary(msg:updates)', $report->asLine()); + } + + /** The ordinary case: nothing to say about a disagreement there isn't. */ + public function testMessageCategoryIsOmittedWhenItAgrees(): void + { + $report = $this->record(MessageCategory::Updates, MessageCategory::Updates); + + self::assertNull($report->filedMessage); + self::assertStringContainsString('filed:updates should:', $report->asLine()); + } + + /** + * Never asked and asked-for-nothing are different failures, and the line + * has to tell them apart — only one of them is about the prompt. + */ + public function testTheLineSaysWhenTheModelWasNeverAsked(): void + { + self::assertStringContainsString( + 'model:notasked', + $this->record(MessageCategory::Primary, MessageCategory::Primary)->asLine(), + ); + } + + private function record(MessageCategory $thread, MessageCategory $message): CategoryReport + { + $contacts = $this->createStub(ContactRepository::class); + $contacts->method('isCorrespondent')->willReturn(false); + + $recorder = new CategoryReportRecorder(new MessageCategorizer(), $contacts); + + $mail = new Message(); + $mail->fromAddress = 'noreply@shop.test'; + $mail->fromName = 'Shop'; + $mail->subject = 'Your order'; + $mail->category = $message; + $mail->thread = new MessageThread(); + $mail->thread->category = $thread; + + return $recorder->record(new User(), $mail, MessageCategory::Promotions); + } +} diff --git a/translations/messages.de.yaml b/translations/messages.de.yaml index 332bf1d3..a3861215 100644 --- a/translations/messages.de.yaml +++ b/translations/messages.de.yaml @@ -91,9 +91,13 @@ admin: seconds: '%count% s' reports: clear: 'Meldungen leeren' - copy: 'Alles kopieren' + clear_confirm: 'Wirklich alle Meldungen löschen? Die kommen nicht wieder — es ist das, was jemand an der eigenen Mail aufgefallen ist.' + copy: 'Auswahl kopieren' + download: Herunterladen heading: 'Als falsch einsortiert gemeldet' - subheading: '%count% Nachrichten, die jemand im falschen Tab gefunden hat. Kopier die Liste, um herauszufinden, was sich ändern muss — eine Regel, die Anweisung oder das Modell.' + select_all: 'Alle auswählen' + selected: '%count% ausgewählt' + subheading: '%count% Nachrichten, die jemand im falschen Tab gefunden hat, gruppiert nach dem Problem. Nimm dir die Gruppe, um die es dir geht — eine Regel, die Anweisung oder das Modell.' embedding_preset: heading: 'Gemessene Voreinstellungen — ein Klick setzt Modell, Anweisung und Schwelle gemeinsam:' all-minilm: 'Klein und schnell: 45 MB und 384 Dimensionen, für Maschinen mit wenig Speicher. Vor allem Englisch — auf deutscher Mail also das schwächste hier. Braucht keine Anweisung.' @@ -418,8 +422,8 @@ admin: lead: '%count% Meldung(en) hast du als erledigt markiert. Was noch wartet, bleibt liegen.' delete: 'Diese Meldung löschen' delete_confirm: 'Diese Meldung löschen? Die Kopie der Mail verschwindet mit ihr.' - empty: 'Es wurde noch nichts gemeldet.' - empty_hint: 'Hier landen Meldungen, wenn jemand plMail sagt, dass eine Mail eine Info hätte liefern sollen und es nicht getan hat.' + empty: 'Keine Info-Meldungen.' + empty_hint: 'Die landen hier, wenn jemand plMail sagt, dass eine Mail eine Info hätte liefern sollen und es nicht getan hat. Mail, die als falsch einsortiert gemeldet wurde, steht darunter.' export: heading: 'Für die Arbeit außerhalb exportieren' intro: 'Eine JSON-Datei mit allen Kopien: Absender, Betreff, Eingangszeitpunkt, Text und das, was die meldende Person erwartet hat. Daraus schreibst du eine neue Auswertung.' @@ -1926,6 +1930,7 @@ message: pinned: 'du hast sie hierher verschoben' promotion: 'ein Massenmail-Header: ' social: 'die Absenderdomain ' + thread_differs: 'Diese Nachricht allein wäre %category% — eine Unterhaltung übernimmt die Kategorie ihrer neuesten Nachricht.' update: 'ein Hinweis auf automatischen Versand: ' category: Kategorie category_ai: 'das Modell' @@ -1939,6 +1944,7 @@ message: failed: 'Konnte nicht gemeldet werden' label: 'Falscher Tab? Gehört nach' send: Melden + stale: 'Liegt schon da — neu laden' sent: Gemeldet date: Datum from: von diff --git a/translations/messages.en.yaml b/translations/messages.en.yaml index 5db92c47..470779df 100644 --- a/translations/messages.en.yaml +++ b/translations/messages.en.yaml @@ -91,9 +91,13 @@ admin: seconds: '%count% s' reports: clear: 'Clear the reports' - copy: 'Copy all' + clear_confirm: 'Delete every report? Nothing regenerates these — they are what somebody noticed about their own mail.' + copy: 'Copy selected' + download: Download heading: 'Reported as wrongly sorted' - subheading: '%count% messages somebody said were in the wrong tab. Copy the list to work out what needs changing — a rule, the prompt, or the model.' + select_all: 'Select all' + selected: '%count% selected' + subheading: '%count% messages somebody said were in the wrong tab, grouped by the problem they describe. Take the group that is about your rule — a rule, the prompt, or the model.' embedding_preset: heading: 'Measured presets — one click sets the model, its instruction and its threshold together:' all-minilm: 'Small and fast: 45 MB and 384 dimensions, for a machine that cannot spare the memory. English-first, so expect it to be the weakest of these on German mail. Needs no instruction.' @@ -411,8 +415,8 @@ admin: lead: '%count% report(s) are marked handled. Deleting them leaves anything still waiting alone.' delete: 'Delete this report' delete_confirm: 'Delete this report? The copy of the mail it carries goes with it.' - empty: 'Nothing has been reported.' - empty_hint: 'Reports land here when someone tells plMail that a mail should have produced an insight and did not.' + empty: 'No insight reports.' + empty_hint: 'These land here when someone tells plMail that a mail should have produced an insight and did not. Mail reported as sorted into the wrong tab appears below.' export: heading: 'Export for offline work' intro: 'One JSON file holding every snapshot: sender, subject, when the mail arrived, the text, and what the person reporting it expected to see. That is what a new extractor gets written from.' @@ -1938,6 +1942,12 @@ message: pinned: 'you moved it here' promotion: 'a bulk-mail header: ' social: 'the sender domain ' + # A conversation takes ONE category, from its newest message, so a + # reply can carry the whole thread out of the tab this message + # belongs in. Said outright, because without it the panel reads as + # simply wrong: every line agreeing on one category while the mail + # sits plainly in another. + thread_differs: 'This message on its own would be %category% — a conversation takes its category from its newest message.' update: 'an automated-mail signal: ' category: category # What a language model made of it — asked about every message now, and @@ -1956,6 +1966,7 @@ message: failed: 'Could not report' label: 'Wrong tab? Should be' send: Report + stale: 'Already there — reload' sent: Reported date: date from: from diff --git a/translations/messages.en_PI.yaml b/translations/messages.en_PI.yaml index 32af57b2..a078d77c 100644 --- a/translations/messages.en_PI.yaml +++ b/translations/messages.en_PI.yaml @@ -91,9 +91,13 @@ admin: seconds: '%count% s' reports: clear: "Empty th' reports" - copy: "Copy the lot" + clear_confirm: "Scuttle every report? Naught brings 'em back — 'tis what somebody spied in their own mail." + copy: "Copy what be picked" + download: "Haul it down" heading: "Reported as wrongly stowed" - subheading: "%count% messages somebody found in th' wrong tab. Copy th' list to work out what wants changin' — a rule, th' orders, or th' oracle." + select_all: "Pick the lot" + selected: "%count% picked" + subheading: "%count% messages somebody found in th' wrong tab, sorted by th' trouble they tell of. Take th' bundle that be about yer rule — a rule, th' orders, or th' oracle." embedding_preset: heading: "Measured settin's — one click sets model, orders an' threshold as one:" all-minilm: "Small an' swift: 45 MB an' 384 dimensions, for a vessel short on memory. English first, so the weakest here on German mail. Wants no orders." @@ -290,7 +294,7 @@ admin: delete: 'Sink this report' delete_confirm: 'Sink this report? Th'' copy o'' th'' letter goes down with it.' empty: 'Naught has been reported.' - empty_hint: 'Reports wash up here when a hand tells plMail a letter should have yielded a findin'' an'' did not.' + empty_hint: 'These wash up here when a hand tells plMail a letter should have yielded a findin'' an'' did not. Post reported as wrongly stowed shows below.' export: heading: 'Haul it ashore' intro: 'One JSON chest holdin'' every copy: who sent it, th'' heading, when it made port, th'' text, an'' what th'' hand reportin'' it expected to see. That be what a new spyglass gets built from.' @@ -1621,6 +1625,7 @@ message: pinned: 'ye hauled it here yerself' promotion: 'a bulk-cargo header: ' social: 'the sender port ' + thread_differs: "This message alone would be %category% — a yarn takes th' category o' its newest message." update: 'a sign of an unmanned sender: ' category: rank category_ai: 'the oracle' @@ -1634,6 +1639,7 @@ message: failed: "Could nay be reported" label: "Wrong tab? Belongs in" send: Report her + stale: "Already stowed there — reload" sent: Reported date: date from: from