aboutsummaryrefslogtreecommitdiffstats
path: root/tests
diff options
context:
space:
mode:
authorDanilo M. <danix@danix.xyz>2026-08-18 12:35:05 +0200
committerDanilo M. <danix@danix.xyz>2026-08-18 12:41:45 +0200
commit601159309118cf65c73f5f50bb3cf216be9f1cbb (patch)
tree9cbf374538002193dcd373a46f7825a219617efb /tests
parent4583de009571aaa674e7d161d31ec640860787e1 (diff)
downloadqtmaildir-601159309118cf65c73f5f50bb3cf216be9f1cbb.tar.gz
qtmaildir-601159309118cf65c73f5f50bb3cf216be9f1cbb.zip
feat(trash): restore mail from the trash view
Task 6. Delete moved mail into the trash and the only ways back out were a second press of Delete or Ctrl+Z, both of which act on a row the user has to have deleted in this session. Browsing the trash and putting something back needed an action of its own. `restore` is enabled from the QUERY, not from the selection's tags. The trash view is path-based precisely so that mail trashed by another client appears in it, and such a message carries no tag of ours: deciding from `tag:deleted` would disable Restore on exactly the messages that most need it. isShowingTrash() compares the current query against the trash generator's own, for both the per-account and the all-accounts scope, so it follows the account dropdown like every other filter. A message with NO origin tag is the foreign-trashed case, and it is why this is not simply restoreSelected() under a new name. The two callers want opposite things from a missing origin, which `fallbackToInbox` selects. From the trash view the message is demonstrably in the trash and refusing to move it leaves the user looking at mail they cannot get out, so it goes to the inbox and the status bar says so. From a second press of Delete the message is not in the trash at all and merely wears a stale `deleted` tag from an older version or a hand-written notmuch command; moving that to the inbox would relocate mail the user never asked to move, so the tag comes off and the file stays put. The inbox FOLDER is a new optional per-account `inbox` key, defaulting to "Inbox". It is configurable rather than hardcoded because the name is not ours to assume: naming a folder that does not exist CREATES it, beside the real one, and under mbsync's `Create Both` that folder reaches the mail server. That is not hypothetical, it is what a truncated origin folder did to real mail while this branch was being tested. Unlike `trash` the key is optional, since the default is right for any ordinary Maildir and a wrong value here only affects the fallback. Ctrl+R, which was free. The action is only enabled in the trash view, so the key is inert elsewhere rather than doing something surprising. It sits in the Message menu beside Delete and in the thread context menu, greyed outside the trash rather than hidden: an action that vanishes teaches nothing, while a disabled entry with its shortcut beside it says both that it exists and where it applies. **Adding an action is FIVE places, not four.** knownActions(), defaultBindings() and the icon table are each enforced by a test that fails loudly, and being REACHABLE is a fifth that nothing checked: this shipped registered, bound, iconned, correctly enabled, and present in no menu at all, which a green suite reported as complete. Ctrl+R is not a shortcut anyone guesses, so it was effectively invisible. restoreIsReachableWithoutTheKeyboard() closes that, and deliberately excludes the context menu from its menu-bar assertion, since findChildren returns both and one check would otherwise satisfy the other. Four tests, each mutation-checked. Two worth keeping: the hardcoded "Inbox" mutation fails against the fixture's lowercase folders exactly as it would against a Maildir that spells its inbox differently, and the reachability mutation reproduces the keyboard-only state this shipped in. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Diffstat (limited to 'tests')
-rw-r--r--tests/test_mainwindow.cpp211
1 files changed, 211 insertions, 0 deletions
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index a17eba1..cefd686 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -135,6 +135,11 @@ public:
<< "maildir=" << accountMaildir << "\n";
if (!accountTrash.isEmpty())
out << "trash=" << accountTrash << "\n";
+ // The fixture's folders are lowercase, unlike the Maildir
+ // convention Account::inboxFolder() defaults to. Stated rather
+ // than assumed, which is the whole point of the key: naming a
+ // folder that does not exist would CREATE it.
+ out << "inbox=inbox\n";
}
}
file.close();
@@ -372,6 +377,10 @@ private slots:
void deletingAThreadRootTwiceRestoresItRatherThanRedeleting();
void deleteThreadMovesEveryMessageAndRepaintsTheRootCard();
void aFolderNameWithASpaceSurvivesTheRoundTrip();
+ void restoreIsReachableWithoutTheKeyboard();
+ void restoreIsOnlyEnabledInTheTrashView();
+ void restoreReturnsAMessageToItsOriginFolder();
+ void restoreFallsBackToInboxWithoutAnOriginTag();
private:
/// Owns the throwaway lock table init() points every test at. A pointer
@@ -9457,6 +9466,208 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip()
"messages are somewhere mbsync will never sync");
}
+void TestMainWindow::restoreIsReachableWithoutTheKeyboard()
+{
+ // Restore shipped as a keyboard shortcut and nothing else: registered,
+ // iconned, enabled correctly, and present in no menu at all. A user who
+ // does not read the changelog would never learn it exists, and Ctrl+R is
+ // not a guess anyone makes.
+ //
+ // The four places an action must touch are enforced by tests
+ // (knownActions, defaultBindings, the icon table); being REACHABLE is a
+ // fifth that nothing checked, which is why the gap survived a green suite.
+ const Config config;
+ MainWindow window(config);
+
+ auto *restore = window.findChild<QAction *>(QStringLiteral("restore"));
+ QVERIFY(restore);
+
+ const auto menuContains = [](const QMenu *menu, const QAction *action) {
+ return menu && menu->actions().contains(action);
+ };
+
+ // A menu on the MENU BAR, beside Delete whose inverse it is. The context
+ // menu is excluded here so this assertion cannot be satisfied by the one
+ // the next assertion checks: findChildren finds both.
+ auto *context =
+ window.findChild<QMenu *>(QStringLiteral("threadContextMenu"));
+ QVERIFY(context);
+
+ bool inAMenuBarMenu = false;
+ for (const QMenu *menu : window.findChildren<QMenu *>()) {
+ if (menu != context && menuContains(menu, restore)) {
+ inAMenuBarMenu = true;
+ break;
+ }
+ }
+ QVERIFY2(inAMenuBarMenu,
+ "Restore is in no menu-bar menu, so a user browsing the menus "
+ "would never learn it exists");
+
+ // And the thread list's context menu, which is where the other
+ // message-scoped actions are reached by mouse.
+ QVERIFY2(menuContains(context, restore),
+ "Restore is missing from the thread context menu");
+}
+
+void TestMainWindow::restoreIsOnlyEnabledInTheTrashView()
+{
+ // Restore has no meaning outside the trash, and an enabled action that
+ // does nothing is worse than an absent one.
+ //
+ // Enabled from the QUERY rather than from the selection's tags: a message
+ // trashed by another client carries no tag of ours and must still be
+ // restorable, which is the whole reason the trash view is path-based.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("re1@example.org"),
+ QStringLiteral("In the inbox"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/Trash"), QStringLiteral("re2@example.org"),
+ QStringLiteral("In the trash"), QStringLiteral("other@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 11:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"),
+ QStringLiteral("Trash")),
+ qPrintable(backed.error()));
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *queryEdit =
+ window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ auto *restore = window.findChild<QAction *>(QStringLiteral("restore"));
+ QVERIFY(model && queryEdit);
+ QVERIFY2(restore, "there is no restore action");
+
+ // An ordinary view. Both fixture messages carry `inbox`, since the
+ // fixture tags all new mail that way regardless of folder, so this is two
+ // rows rather than one; the count is not what is under test.
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000);
+ QVERIFY2(!restore->isEnabled(),
+ "Restore is enabled in an ordinary view, where it means nothing");
+
+ // The trash view, which is the account's own generated trash query.
+ queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\""));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ QVERIFY2(restore->isEnabled(),
+ "Restore is disabled in the trash view, where it is the point");
+}
+
+void TestMainWindow::restoreReturnsAMessageToItsOriginFolder()
+{
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/inbox"), QStringLiteral("ro1@example.org"),
+ QStringLiteral("Send me back"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"),
+ QStringLiteral("Trash")),
+ qPrintable(backed.error()));
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<ThreadListView *>();
+ auto *queryEdit =
+ window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ QVERIFY(model && view && queryEdit);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString cfg = backed.fixture().configPath();
+ const QString stem = QStringLiteral("ro1.example.org");
+
+ queryEdit->setText(QStringLiteral("tag:inbox"));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("delete"))->trigger();
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem),
+ 15000);
+
+ // Now from the trash view, through Restore rather than through a second
+ // Delete: this is the action the user reaches for when browsing trash.
+ queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\""));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("restore"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem)
+ || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"),
+ stem),
+ 15000);
+ QTRY_VERIFY_WITH_TIMEOUT(
+ notmuchCount(cfg,
+ QStringLiteral("id:ro1@example.org and tag:deleted")) == 0,
+ 15000);
+
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ro1@example.org")), 1);
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:ro1@example.org and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+ QVERIFY(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"),
+ stem));
+}
+
+void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag()
+{
+ // A message trashed by ANOTHER client: it sits in the trash folder and
+ // carries no `deleted-from:` tag, because nothing here put it there. The
+ // real Maildir has such messages, which is why the trash view is path
+ // based rather than tag based.
+ //
+ // Inbox is the documented fallback. Refusing to move it would leave the
+ // user with a message they can see in the trash and cannot get out.
+ WorkerBackedWindow backed;
+ QVERIFY(backed.fixture().addMessage(
+ QStringLiteral("acct/Trash"), QStringLiteral("foreign@example.org"),
+ QStringLiteral("Trashed elsewhere"), QStringLiteral("sender@example.org"),
+ QStringLiteral("Fri, 14 Aug 2026 10:00:00 +0200"),
+ QStringLiteral("Body text.")));
+ QVERIFY2(backed.build(QStringLiteral("acct"), QStringLiteral("acct"),
+ QStringLiteral("Trash")),
+ qPrintable(backed.error()));
+
+ MainWindow window(backed.config());
+ auto *model = window.findChild<ThreadListModel *>();
+ auto *view = window.findChild<ThreadListView *>();
+ auto *queryEdit =
+ window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+ QVERIFY(model && view && queryEdit);
+
+ const QString root = backed.fixture().maildirPath();
+ const QString cfg = backed.fixture().configPath();
+ const QString stem = QStringLiteral("foreign.example.org");
+
+ // The guard this test needs: no origin tag, so the fallback is what is
+ // under test rather than an ordinary restore.
+ QCOMPARE(notmuchCount(cfg, QStringLiteral("id:foreign@example.org and "
+ "tag:\"deleted-from:inbox\"")),
+ 0);
+
+ queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\""));
+ queryEdit->returnPressed();
+ QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
+ view->setCurrentIndex(model->index(0, 0, QModelIndex()));
+ window.findChild<QAction *>(QStringLiteral("restore"))->trigger();
+
+ QTRY_VERIFY_WITH_TIMEOUT(
+ folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem)
+ || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"),
+ stem),
+ 15000);
+ QVERIFY2(!folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"),
+ stem),
+ "the message was copied out of the trash rather than moved");
+}
+
void TestMainWindow::undoMovesTheMessageBack()
{
// Undo is this project's answer to the confirmation dialog it rules out,