diff options
| -rw-r--r-- | CHANGELOG.md | 4 | ||||
| -rw-r--r-- | src/mainwindow.cpp | 23 | ||||
| -rw-r--r-- | tests/test_mainwindow.cpp | 175 |
3 files changed, 202 insertions, 0 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index e514fc8..dbc06e1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -33,6 +33,10 @@ point at which they are stable. - Open thread, Clear message pane and Clear selection appear in the View menu. All three existed and were reachable only by their shortcuts. +- Restoring from the trash view refreshes the list, so the restored message + leaves it straight away instead of sitting there until the Trash filter is + clicked again. Other views are unaffected: a deleted message's card + deliberately stays where it is. ### Upgrading diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index a1c01c3..58c82ca 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -4982,6 +4982,29 @@ void MainWindow::onMessagesMoved(const QMap<QString, QString> &originByMessageId } } + // A restore out of the TRASH VIEW leaves the row it came from showing a + // message that is no longer there, and only a refresh can say so. + // + // Reported from a hand test: the move was correct and the row sat in the + // list until the Trash filter was clicked again. The trash view is PATH + // based, so a restored message stops matching the query the list was built + // from, which is a state no tag change can express. Nothing else here + // removes a row, deliberately: in an ordinary view a deleted message's + // card should stay put, since one deleted reply does not doom the + // conversation. + // + // refreshCurrentQuery() rather than runCurrentQuery(): it clears nothing, + // so the selection, the expanded threads, the undo stack and the message + // being read all survive. Re-running the query outright would destroy the + // undo entry this function just pushed, which is the one thing a restore + // must leave intact. + // + // Gated on isShowingTrash() and not on the destination: a Delete is a move + // too and reaches this same slot, and refreshing after every delete would + // make a row vanish from under the user in every other view. + if (isShowingTrash()) + refreshCurrentQuery(); + // The undo entries are pushed inside the loop above, one per origin // group, because the placeholder resolves per origin. Nothing is pushed // for a move the undo stack itself started: a MoveCommand is confirmed diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index e982e37..79e137a 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -388,6 +388,9 @@ private slots: void theCleanupQueryFindsStrandedMail(); void theCleanupQueryExcludesMailAlreadyInTrash(); void aMoveThatRelocatesNothingWritesNoTag(); + void restoringFromTheTrashViewRefreshesTheList(); + void theRefreshAfterARestoreLeavesUndoIntact(); + void deletingOutsideTheTrashViewLeavesTheRowInPlace(); private: /// Owns the throwaway lock table init() points every test at. A pointer @@ -10389,4 +10392,176 @@ void TestMainWindow::aMoveThatRelocatesNothingWritesNoTag() QStringLiteral("nomove.example.org"))); } +void TestMainWindow::restoringFromTheTrashViewRefreshesTheList() +{ + // Reported from a hand test: Restore moved the message correctly and the + // row it came from sat in the trash list until the Trash filter was + // clicked again. + // + // The trash view is PATH based, so a restored message no longer matches + // the query the list was built from. That is a state no tag change can + // express: onMessagesMoved() updates tags and the undo stack and never + // removes a row, which is right in an ordinary view (a deleted message's + // card should stay put) and wrong here, where the row is the one thing + // that is now false. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("refr1@example.org"), + QStringLiteral("Restore me"), 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 stem = QStringLiteral("refr1.example.org"); + + // The account's own generated trash query, which is what the Trash filter + // puts in the bar. + 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(); + + // The move really happened, waited on the FILE. Without this the row + // assertion below could pass against a restore that never ran. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/inbox/cur"), stem) + || folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), + stem), + 15000); + + // And the list no longer shows it, without the user touching anything. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); +} + +void TestMainWindow::theRefreshAfterARestoreLeavesUndoIntact() +{ + // The refresh that fixes the stale trash row runs immediately after the + // undo entry is pushed, so it must not be the thing that destroys it. + // runCurrentQuery() clears the undo stack outright, which would make + // Restore the one mutation in the window with no way back; this asserts + // the non-destructive refresh was used and stayed non-destructive. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/Trash"), QStringLiteral("undoref@example.org"), + QStringLiteral("Restore then undo"), + 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 stem = QStringLiteral("undoref.example.org"); + + 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); + // The refresh has run by now, which is what the row count proves. + QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); + + // And the undo entry is still there afterwards. + auto *undo = window.findChild<QAction *>(QStringLiteral("undo")); + QVERIFY(undo); + QVERIFY2(undo->isEnabled(), + "the refresh after a restore cleared the undo stack"); + + undo->trigger(); + + // Back in the trash, asserted on the FILE: the undo has to move it, not + // merely re-tag it. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); +} + +void TestMainWindow::deletingOutsideTheTrashViewLeavesTheRowInPlace() +{ + // The other half of the trash-view refresh, and the reason it is gated. + // + // A Delete is a move too and reaches the same confirmation slot. Refreshing + // on every move would make the row vanish from under the user in every + // ordinary view, which this project has decided against twice: the card + // deliberately stays put, because one deleted message does not doom the + // conversation and a row disappearing mid-gesture loses the user's place. + // + // Nothing asserted this, so a mutation dropping the isShowingTrash() gate + // passed the whole suite. + WorkerBackedWindow backed; + QVERIFY(backed.fixture().addMessage( + QStringLiteral("acct/inbox"), QStringLiteral("stay1@example.org"), + QStringLiteral("Stay on screen"), 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 stem = QStringLiteral("stay1.example.org"); + + // An ORDINARY view that the message STOPS MATCHING once the delete lands. + // Both halves matter and the first draft of this test had only one: a + // `tag:inbox` view looks ordinary but Delete adds `deleted` and the origin + // tag and removes nothing, so the message keeps `inbox` and keeps matching. + // A refresh there is a no-op, and the mutation dropping the + // isShowingTrash() gate passed against it. + // + // A path query on the inbox folder is the honest instrument: the file + // really leaves that folder, so the row survives only because nothing + // refreshed. + queryEdit->setText(QStringLiteral("path:\"acct/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(); + + // The delete really happened, waited on the file rather than on the list. + QTRY_VERIFY_WITH_TIMEOUT( + folderHasMessageFile(root + QStringLiteral("/acct/Trash/cur"), stem), + 15000); + + // A refresh is a queued round trip, so an immediate read would pass against + // one still in flight. Given time to arrive, then asserted not to have + // taken the row away. + QTest::qWait(1500); + QCOMPARE(model->rowCount(QModelIndex()), 1); +} + #include "test_mainwindow.moc" |