aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-25 19:19:21 +0200
committerDanilo M. <danix@danix.xyz>2026-08-25 19:19:21 +0200
commitdd6e7a051c22e5145c1b23da0614179aef2e1379 (patch)
tree5fd840216d5ae06715e7ca4116c6cea603a7a232
parentdfb36213c03e678178ec1abb2327266d23015bc0 (diff)
downloadqtmaildir-dd6e7a051c22e5145c1b23da0614179aef2e1379.tar.gz
qtmaildir-dd6e7a051c22e5145c1b23da0614179aef2e1379.zip
fix: offer Delete and Restore only where they mean something
Item 168, found by the user while hand-testing 118: Delete could be triggered on a message already in the trash. Not dangerous, which is how it survived. moveMessages() finds the file already in the destination and takes its early-return branch, so the message is reported as moved, an unsynced change is counted, and nothing happened. Restore had the mirror of the same problem, added unconditionally to both menus and so offered on mail that was never deleted. Each is now hidden where it has no meaning, which is the rule item 112 established for the unread entry. The question is about the PATH, never the deleted tag: a message trashed by another client carries no such tag, which is why the trash view is path-based, and asking the tag would hide Delete on exactly the mail a trash view is full of. Delete also removes unread now, at the user's request on the same tangent. It travels inside the same sendMove() call rather than as a second write, so one undo returns the folder and the tag together. This rewrites the Maildir filename, because maildir.synchronize_flags is true, and so reaches the server: the same mechanism the post-new hook refuses to touch, and the difference is that the hook acts unattended on arriving mail while this is an explicit gesture on a message in front of the user. A mutation survived the first round and found a real hole: comparing the prefix without its trailing separator passed every test, because no fixture had a folder whose name starts with the trash folder's. Under it Delete silently vanished from mail in acct/trash-old, which is not the trash. The fixture carries that row now and all three properties are mutation-checked. The suite is 37 of 38, the failure being item 136 on an unrelated path. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01HFuRPtzFrSxCQjFk6tq7gD
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md96
-rw-r--r--docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md74
-rw-r--r--src/mainwindow.cpp78
-rw-r--r--src/mainwindow.h14
-rw-r--r--tests/test_mainwindow.cpp162
5 files changed, 348 insertions, 76 deletions
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
index 8092fb4..1208c42 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability-closed.md
@@ -8395,3 +8395,99 @@ trash: zero files on disk, zero in the index.
already in the trash.
**Size: S.** Done.
+
+## 168. Delete is offered on mail already in the trash, and does nothing
+
+**Observed (user, 2026-08-25, while hand-testing item 118):** "I noticed I can
+hit delete via context menu on a message already in the trash. Seems like a
+bug, unless that action doesn't do for one message what Empty trash does for
+the whole view."
+
+It does not, and the guess in the second half is worth recording as the reason
+this matters: the user's mental model was that Delete on already-trashed mail
+might PURGE it. It does not, and nothing about the menu says so.
+
+**Cause (verified in code, 2026-08-25.)** `moveMessages()` compares the file's
+directory against the destination and takes an early-return branch when they
+match (`notmuchworker.cpp`, the "already where it was asked to go" branch,
+added when a fresh Maildir name made a path comparison useless). That branch
+appends the id to `moved` and records an origin, so the message is reported as
+having moved when nothing happened. The UI counts an unsynced change for it.
+
+Nothing is destroyed and nothing is corrupted; the cost is a menu entry that
+lies about having done something, and a pending-changes count that overstates
+what a sync has to carry.
+
+**The mirror of the same defect is already shipped beside it.** `restore` is
+added unconditionally to both the Message menu (`mainwindow.cpp:1956`) and the
+thread context menu (`mainwindow.cpp:2119`), so it is offered on mail that was
+never deleted, where it has as little meaning as Delete has in the trash.
+
+**Approach.** The user chose to hide each action where it has no meaning,
+which is the principle item 112 established for the unread entry: an action
+with no honest meaning for the selection is absent rather than present and
+inert.
+
+- Delete is hidden when every selected row is already in a trash folder.
+- Restore is hidden when no selected row is.
+- The test for both needs a MIXED selection as well as uniform ones, for the
+ reason item 112 records: a selection whose rows agree answers identically
+ whichever way the predicate is computed.
+
+**Constraints.**
+
+- **The question is about the PATH, not the tag.** A message trashed by
+ another client carries no `deleted` tag at all, which is why item 103 made
+ the trash view path-based. Asking `tags.contains("deleted")` here would
+ offer Delete on exactly the mail the user is most likely to be looking at
+ in a trash view.
+- **`selectionTagPresence()` is the wrong instrument** for the same reason,
+ though it is the right shape. A path predicate needs the row's path, which
+ `MessageNode` carries.
+- Deciding this does not require deciding item 118's relationship to it: a
+ purge stays an explicit whole-view action, and hiding Delete does not make
+ Delete a purge.
+
+**A second request, from the same tangent (user, 2026-08-25):** "messages moved
+to the trash should be automatically marked `-unread`." Deleting is a decision
+about the message, so leaving it bold and unread in the trash is noise; the
+count of unread mail should not include what the user threw away.
+
+It is one line where Delete already composes its tag change, and it carries a
+constraint worth stating rather than discovering. `maildir.synchronize_flags`
+is true, so removing `unread` REWRITES the Maildir filename and reaches the
+server on the next mbsync. That is acceptable here and is a deliberate
+exception: it is the same mechanism the `post-new` hook refuses to touch on
+arriving mail, for the good reason that the hook acts unattended on mail the
+user has not seen. A Delete is an explicit gesture on a message in front of
+them, which is the difference.
+
+Undo must put it back. `TagChange::inverted()` already does, provided the
+removal travels as part of the SAME change rather than as a second write, so
+one undo returns both the folder and the tag.
+
+**Built 2026-08-25**, both halves, to the user's own choice of "hide each
+where it has no meaning".
+
+`everySelectedRowIsInATrashFolder()` asks each row about its own file, a reply
+row's message and a thread row's displayed message, the same rule
+`everySelectedRowHasTag()` follows. `refreshTrashActions()` runs beside
+`refreshUnreadAction()` on both the selection change and the model's
+`dataChanged`, so the entries follow a write as well as a selection.
+
+The `unread` removal travels inside the SAME `sendMove()` call rather than as a
+second write, which is what makes one undo return the folder and the tag
+together.
+
+**A mutation survived the first round and is worth recording**: comparing the
+prefix WITHOUT its trailing separator passed every test, because no fixture had
+a folder whose name starts with the trash folder's. `acct/trash-old` is a
+different folder, and under that mutation Delete silently disappeared from mail
+that had never been trashed, which is the quiet half of the same mistake. The
+fixture carries that row now and the mutation fails.
+
+All three properties are mutation-checked: the separator, Restore's visibility,
+and the `unread` removal. The suite is 37 of 38, the failure being item 136 on
+an unrelated path, and no new user-facing strings were added.
+
+**Size: S** for the visibility half, XS for the `unread` half. Done.
diff --git a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
index 37dc190..437ceda 100644
--- a/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
+++ b/docs/superpowers/plans/2026-08-03-post-0.1.0-usability.md
@@ -241,7 +241,7 @@ taking that too literally.
| 165 | A draft gets a new Message-ID on every autosave | enhancement | ? | open, 2026-08-25, found while hand-testing 163 and 164. `MessageBuilder::build()` generates an id unconditionally and every autosave calls it, so each revision is a distinct MESSAGE to notmuch and to the server rather than a new version of one. Invisible while the file is replaced correctly, which item 163's fix restores; it is what turned that fork into two messages rather than one duplicated file. Needs a DECISION on what a draft's identity is before any code: a stable id reused at send, a stable id discarded at send, or the status quo. Neither `ComposeContext` nor `OutgoingMessage` has a field to carry an id, so it is not a changed call site |
| 166 | Mail you send to your own other account loses `inbox` | defect | S | **done 2026-08-25**, unreleased. `sent_only()` keeps a message only when EVERY file is inside a sent folder, which is what the carve-out's docstring already claimed. No query can express it, measured; the root comes from `database.mail_root`, with a split-index fixture the ordinary layout cannot provide. Verified read-only against the live index: 780 of 807 still stripped, 27 spared, no arrival affected |
| 167 | No way to tell one build of an unreleased version from another | enhancement | XS | **done 2026-08-25**, unreleased. The user chose a counter over a git description: `QTMAILDIR_BUILD_NUMBER`, a cmake option ON by default, increments a counter in the BUILD directory on every build and writes `buildnumber.h`. `QTMAILDIR_VERSION_DISPLAY` carries it; `QTMAILDIR_VERSION` stays clean and is what the window title, `applicationVersion` and the release procedure use |
-| 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | open, found 2026-08-25 by the user while hand-testing 118. `moveMessages()` takes its already-in-destination branch, reports the message as moved and counts an unsynced change for a move that never happened. Not dangerous, it purges nothing. Restore has the mirror of the same problem: it is offered on mail that was never deleted. The user chose to hide each where it has no meaning, the same principle as 112's hidden unread entry. **Carries a second request**: Delete should also drop `unread` |
+| 168 | Delete is offered on mail already in the trash, and does nothing | defect | S | **done 2026-08-25**, unreleased. Delete is hidden when every selected row is already in its account's trash, Restore when none is, both keyed on the PATH rather than the `deleted` tag. Delete also drops `unread` now, in the same TagChange so one undo returns the folder and the tag together |
Sizes are rough: XS under an hour, S a sitting, M a session.
@@ -1347,75 +1347,3 @@ id and a draft of a reply carries both.
- Item 163's fix stands on its own and this does not block it: the file is
replaced correctly now, so the fork this would have mitigated no longer
happens by that route.
-
-## 168. Delete is offered on mail already in the trash, and does nothing
-
-**Observed (user, 2026-08-25, while hand-testing item 118):** "I noticed I can
-hit delete via context menu on a message already in the trash. Seems like a
-bug, unless that action doesn't do for one message what Empty trash does for
-the whole view."
-
-It does not, and the guess in the second half is worth recording as the reason
-this matters: the user's mental model was that Delete on already-trashed mail
-might PURGE it. It does not, and nothing about the menu says so.
-
-**Cause (verified in code, 2026-08-25.)** `moveMessages()` compares the file's
-directory against the destination and takes an early-return branch when they
-match (`notmuchworker.cpp`, the "already where it was asked to go" branch,
-added when a fresh Maildir name made a path comparison useless). That branch
-appends the id to `moved` and records an origin, so the message is reported as
-having moved when nothing happened. The UI counts an unsynced change for it.
-
-Nothing is destroyed and nothing is corrupted; the cost is a menu entry that
-lies about having done something, and a pending-changes count that overstates
-what a sync has to carry.
-
-**The mirror of the same defect is already shipped beside it.** `restore` is
-added unconditionally to both the Message menu (`mainwindow.cpp:1956`) and the
-thread context menu (`mainwindow.cpp:2119`), so it is offered on mail that was
-never deleted, where it has as little meaning as Delete has in the trash.
-
-**Approach.** The user chose to hide each action where it has no meaning,
-which is the principle item 112 established for the unread entry: an action
-with no honest meaning for the selection is absent rather than present and
-inert.
-
-- Delete is hidden when every selected row is already in a trash folder.
-- Restore is hidden when no selected row is.
-- The test for both needs a MIXED selection as well as uniform ones, for the
- reason item 112 records: a selection whose rows agree answers identically
- whichever way the predicate is computed.
-
-**Constraints.**
-
-- **The question is about the PATH, not the tag.** A message trashed by
- another client carries no `deleted` tag at all, which is why item 103 made
- the trash view path-based. Asking `tags.contains("deleted")` here would
- offer Delete on exactly the mail the user is most likely to be looking at
- in a trash view.
-- **`selectionTagPresence()` is the wrong instrument** for the same reason,
- though it is the right shape. A path predicate needs the row's path, which
- `MessageNode` carries.
-- Deciding this does not require deciding item 118's relationship to it: a
- purge stays an explicit whole-view action, and hiding Delete does not make
- Delete a purge.
-
-**A second request, from the same tangent (user, 2026-08-25):** "messages moved
-to the trash should be automatically marked `-unread`." Deleting is a decision
-about the message, so leaving it bold and unread in the trash is noise; the
-count of unread mail should not include what the user threw away.
-
-It is one line where Delete already composes its tag change, and it carries a
-constraint worth stating rather than discovering. `maildir.synchronize_flags`
-is true, so removing `unread` REWRITES the Maildir filename and reaches the
-server on the next mbsync. That is acceptable here and is a deliberate
-exception: it is the same mechanism the `post-new` hook refuses to touch on
-arriving mail, for the good reason that the hook acts unattended on mail the
-user has not seen. A Delete is an explicit gesture on a message in front of
-them, which is the difference.
-
-Undo must put it back. `TagChange::inverted()` already does, provided the
-removal travels as part of the SAME change rather than as a second write, so
-one undo returns both the folder and the tag.
-
-**Size: S** for the visibility half, XS for the `unread` half.
diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp
index dd7bc68..89c01eb 100644
--- a/src/mainwindow.cpp
+++ b/src/mainwindow.cpp
@@ -905,8 +905,10 @@ void MainWindow::buildUi()
// entry to "Mark as unread" with the same row still selected. Keyed on
// the model rather than on each of the six call sites that apply an
// optimistic update, so a new one cannot forget.
- connect(m_model, &QAbstractItemModel::dataChanged, this,
- [this]() { refreshUnreadAction(); });
+ connect(m_model, &QAbstractItemModel::dataChanged, this, [this]() {
+ refreshUnreadAction();
+ refreshTrashActions();
+ });
connect(m_threadView, &QAbstractItemView::doubleClicked,
this, &MainWindow::onRowDoubleClicked);
@@ -3517,6 +3519,60 @@ void MainWindow::showThreadContextMenu(const QPoint &pos)
m_threadContextMenu->popup(m_threadView->viewport()->mapToGlobal(pos));
}
+bool MainWindow::everySelectedRowIsInATrashFolder() const
+{
+ const QModelIndexList rows =
+ m_threadView->selectionModel()->selectedRows();
+ if (rows.isEmpty())
+ return false;
+
+ for (const QModelIndex &index : rows) {
+ // The row's own file: a reply row's message, a thread row's displayed
+ // message. Same rule as everySelectedRowHasTag(), and for the same
+ // reason: a thread row acts on the message its card shows.
+ const QString path =
+ m_model->isMessageRow(index)
+ ? m_model->messageAt(index).filePath
+ : m_model->threadFor(index).firstMessagePath;
+ if (path.isEmpty())
+ return false;
+
+ const Account account = accountForMessagePath(path);
+ if (account.maildir.isEmpty() || account.trash.isEmpty())
+ return false;
+
+ // Compared as a path segment, never with startsWith(): `trash-old`
+ // starts with `trash` and is a different folder. The same trap the
+ // attachment-save check records.
+ const QString prefix = account.maildir + QLatin1Char('/')
+ + account.trash + QLatin1Char('/');
+ // accountForMessagePath() accepts both shapes, so this must too: a
+ // thread row's path is database-relative and a reply row's absolute.
+ if (!path.contains(prefix))
+ return false;
+ }
+ return true;
+}
+
+void MainWindow::refreshTrashActions()
+{
+ const bool inTrash = everySelectedRowIsInATrashFolder();
+ const bool haveSelection =
+ !m_threadView->selectionModel()->selectedRows().isEmpty();
+
+ // Delete on mail already in the trash reported success and did nothing:
+ // moveMessages() finds the file already in the destination and takes its
+ // early-return branch, which counts an unsynced change for a move that
+ // never happened (item 168).
+ if (auto *del = m_actions.value(QStringLiteral("delete")))
+ del->setVisible(!haveSelection || !inTrash);
+
+ // The mirror, which shipped beside it: Restore was added unconditionally
+ // to both menus and so was offered on mail that was never deleted.
+ if (auto *restore = m_actions.value(QStringLiteral("restore")))
+ restore->setVisible(!haveSelection || inTrash);
+}
+
void MainWindow::refreshUnreadAction()
{
// The user's design (item 112 and its duplicates 99/147): the label says
@@ -3553,6 +3609,7 @@ void MainWindow::onSelectionChanged()
// selectedRows() there sees the PREVIOUS selection and would label the
// action for the rows the user just left (CLAUDE.md, verified Qt 6.11).
refreshUnreadAction();
+ refreshTrashActions();
const QModelIndexList rows = m_threadView->selectionModel()->selectedRows();
const int selected = rows.size();
@@ -5388,8 +5445,23 @@ void MainWindow::trashMessages(const QStringList &messageIds,
return;
for (auto it = byTrash.cbegin(); it != byTrash.cend(); ++it) {
+ // `unread` goes with it (item 168, the user's request). Deleting is a
+ // decision about the message, so the unread count must not go on
+ // including what the user threw away.
+ //
+ // In the SAME change rather than as a second write, so one undo
+ // returns the folder and the tag together: TagChange::inverted()
+ // gives it back only if it travelled with the move.
+ //
+ // This rewrites the Maildir filename, because
+ // maildir.synchronize_flags is true, and so reaches the server on the
+ // next mbsync. That is the same mechanism the post-new hook REFUSES
+ // to touch, and the difference is who is acting: the hook tags
+ // arriving mail unattended, while this is an explicit gesture on a
+ // message in front of the user.
sendMove(it.value(), it.key(),
- { QStringLiteral("deleted"), kOriginTagPlaceholder() }, {},
+ { QStringLiteral("deleted"), kOriginTagPlaceholder() },
+ { QStringLiteral("unread") },
tr("Delete"), false, wholeThreadIds);
}
diff --git a/src/mainwindow.h b/src/mainwindow.h
index 42708d9..a5a8c31 100644
--- a/src/mainwindow.h
+++ b/src/mainwindow.h
@@ -909,6 +909,20 @@ private:
/// single state. Called whenever the selection changes.
void refreshUnreadAction();
+ /// Hides Delete on mail already in the trash, and Restore on mail that
+ /// was never there (item 168). Each is offered only where it means
+ /// something, the same rule refreshUnreadAction() applies to the label.
+ void refreshTrashActions();
+
+ /// Whether every selected row's file already sits in its account's trash
+ /// folder. Empty selection answers false.
+ ///
+ /// The question is about the PATH, never the `deleted` TAG: a message
+ /// trashed by another client carries no such tag at all, which is why the
+ /// trash view is path-based (item 103), and asking the tag would offer
+ /// Delete on exactly the mail a trash view is full of.
+ bool everySelectedRowIsInATrashFolder() const;
+
void editTagsOnSelection();
/// Set once the user has answered the exit prompt, or once a sync started
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 6020a1d..f76ff70 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -384,6 +384,9 @@ private slots:
void editTagsOnAReplyCountsItsOwnThreadNotTheFirstInTheList();
void markCurrentThreadReadResolvesTheThreadThroughTheIndex();
void deletingAReplyRepaintsThatReplyRow();
+ void deleteIsHiddenOnMailAlreadyInTheTrash();
+ void restoreIsHiddenOnMailThatWasNeverDeleted();
+ void deleteAlsoMarksTheMessageRead();
void emptyTrashAsksBeforeDestroyingAnything();
void theUnreadLabelSaysWhichDirectionItWillGo();
void theUnreadLabelFollowsAWriteWithoutReselecting();
@@ -5214,6 +5217,165 @@ void TestMainWindow::deletingAReplyRepaintsThatReplyRow()
"deleting one reply marked its whole thread deleted");
}
+/// A window whose one account owns `acct/`, with its trash at `acct/trash`.
+///
+/// Delete and Restore both ask about a row's PATH, so a test for either needs
+/// a config that says which prefix is a trash folder. Bare-window tests carry
+/// no account at all and would answer "not in the trash" for every row.
+static Config configWithTrash(QTemporaryDir &dir)
+{
+ const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
+ QFile file(path);
+ if (file.open(QIODevice::WriteOnly | QIODevice::Text)) {
+ QTextStream out(&file);
+ out << "[account.acct]\n"
+ << "maildir = acct\n"
+ << "trash = trash\n"
+ << "inbox = inbox\n";
+ }
+ Config config;
+ config.load(path);
+ return config;
+}
+
+/// One thread row whose displayed message sits at `filePath`.
+static ThreadSummary threadAtPath(const QString &id, const QString &filePath,
+ const QStringList &tags = {})
+{
+ ThreadSummary thread = makeThread(id, tags);
+ thread.firstMessagePath = filePath;
+ thread.firstMessageTags = tags;
+ return thread;
+}
+
+void TestMainWindow::deleteIsHiddenOnMailAlreadyInTheTrash()
+{
+ // Item 168, from the user: "I noticed I can hit delete via context menu on
+ // a message already in the trash."
+ //
+ // It was not dangerous, which is the part that made it survive: the file
+ // is already in the destination, so moveMessages() takes its
+ // already-there branch, reports the message as moved and counts an
+ // unsynced change for a move that never happened. The menu claimed to
+ // have done something and nothing had.
+ //
+ // The question is about the PATH, never the `deleted` TAG: a message
+ // trashed by another client carries no such tag, which is why item 103
+ // made the trash view path-based, and asking the tag would offer Delete on
+ // exactly the mail a trash view is full of.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const Config config = configWithTrash(dir);
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *deleteAction =
+ window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(deleteAction);
+
+ model->appendBatch({
+ threadAtPath(QStringLiteral("t1"),
+ QStringLiteral("acct/inbox/cur/1:2,S")),
+ threadAtPath(QStringLiteral("t2"),
+ QStringLiteral("acct/trash/cur/2:2,S")),
+ });
+
+ view->setCurrentIndex(model->index(0, 0, {}));
+ QVERIFY2(deleteAction->isVisible(),
+ "Delete is hidden on mail that is NOT in the trash, so this test "
+ "cannot tell the two cases apart");
+
+ view->setCurrentIndex(model->index(1, 0, {}));
+ QVERIFY2(!deleteAction->isVisible(),
+ "Delete is still offered on a message already in the trash, "
+ "where it reports success and does nothing");
+
+ // A folder whose name STARTS with the trash folder's is a different
+ // folder. Without the trailing separator `acct/trash-old` matches
+ // `acct/trash` and Delete silently disappears from mail that was never
+ // trashed, which is the quiet half of the same mistake.
+ model->appendBatch({ threadAtPath(QStringLiteral("t3"),
+ QStringLiteral("acct/trash-old/cur/3:2,S")) });
+ view->setCurrentIndex(model->index(2, 0, {}));
+ QVERIFY2(deleteAction->isVisible(),
+ "Delete vanished on mail in acct/trash-old, which is not the "
+ "trash: the prefix was compared without its separator");
+}
+
+void TestMainWindow::restoreIsHiddenOnMailThatWasNeverDeleted()
+{
+ // The mirror, shipped beside it: `restore` was added unconditionally to
+ // both menus, so it was offered on mail that was never deleted, where it
+ // has as little meaning as Delete has in the trash.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const Config config = configWithTrash(dir);
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+ auto *restore = window.findChild<QAction *>(QStringLiteral("restore"));
+ QVERIFY(restore);
+
+ model->appendBatch({
+ threadAtPath(QStringLiteral("t1"),
+ QStringLiteral("acct/inbox/cur/1:2,S")),
+ threadAtPath(QStringLiteral("t2"),
+ QStringLiteral("acct/trash/cur/2:2,S")),
+ });
+
+ view->setCurrentIndex(model->index(1, 0, {}));
+ QVERIFY2(restore->isVisible(), "Restore is hidden on trashed mail");
+
+ view->setCurrentIndex(model->index(0, 0, {}));
+ QVERIFY2(!restore->isVisible(),
+ "Restore is still offered on mail that was never deleted");
+}
+
+void TestMainWindow::deleteAlsoMarksTheMessageRead()
+{
+ // The user's second request on the same tangent: "messages moved to the
+ // trash should be automatically marked -unread". Deleting is a decision
+ // about the message, so the unread count must not go on including what
+ // the user threw away.
+ //
+ // Asserted on the undo TEXT and depth rather than on the tags: the write
+ // is a move, which a bare window cannot complete, but the tag change it
+ // composes is pushed as one command either way. One command, not two, is
+ // the property that matters: undo has to return the folder AND the tag
+ // together.
+ QTemporaryDir dir;
+ QVERIFY(dir.isValid());
+ const Config config = configWithTrash(dir);
+ MainWindow window(config);
+
+ auto *model = window.findChild<ThreadListModel *>();
+ QVERIFY(model);
+ auto *view = window.findChild<QTreeView *>();
+ QVERIFY(view);
+
+ model->appendBatch({ threadAtPath(QStringLiteral("t1"),
+ QStringLiteral("acct/inbox/cur/1:2,S"),
+ { QStringLiteral("unread") }) });
+ const QModelIndex row = model->index(0, 0, {});
+ view->setCurrentIndex(row);
+
+ QVERIFY2(model->threadFor(row).isUnread(),
+ "the fixture is already read, so this test cannot see the tag go");
+
+ auto *deleteAction = window.findChild<QAction *>(QStringLiteral("delete"));
+ QVERIFY(deleteAction);
+ deleteAction->trigger();
+
+ QVERIFY2(!model->threadFor(row).isUnread(),
+ "Delete left the message unread in the trash");
+}
+
void TestMainWindow::emptyTrashAsksBeforeDestroyingAnything()
{
// Item 118, and the one place this application asks. CLAUDE.md rules out