From 830aa264f81af1a92dec9ee95bc24a9b25dee53d Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Wed, 19 Aug 2026 10:03:04 +0200 Subject: i18n: translate the trash strings, and document the trash key The three new strings from the cleanup action, translated into Italian. lrelease reports 383 finished and 0 unfinished; an unfinished string is silently dropped and ships as English inside an otherwise Italian UI. The changelog gains an Upgrading section for the mandatory `trash` key, the new optional `inbox` key and the `Del` binding, and states the consequence that cost real mail on this branch: a folder name that does not match the server is created rather than reported, mbsync adopts it, and under Create Both it propagates to the server where other clients see it. CLAUDE.md is corrected on two counts. Adding an action is five places, not four; the fifth is a menu, and nothing enforced it until this branch added everyActionIsReachableFromAMenu(). And the trash design is recorded: why the origin lives in a tag, why those tags are joined by a tab rather than a space, and why Restore resolves against the database rather than the model. Also repairs a race in deletingTwiceLeavesNoOriginTagBehind(). Its guard ran a query through the bar in the gap between the file rename and the tag writes, and a query bar run in that gap returns zero rows forever, since QTRY_VERIFY re-reads rowCount() and never re-runs the query. Measured 3 failures in 12 runs, each burning a full 15s timeout; 0 in 8 after asking the database directly, with the runtime down from 45s to 0.3s. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 49 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 49 insertions(+) (limited to 'CHANGELOG.md') diff --git a/CHANGELOG.md b/CHANGELOG.md index 75d2de1..e514fc8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,55 @@ point at which they are stable. ## [Unreleased] +### Added + +- Delete now moves mail into the account's trash folder instead of only + tagging it. A **Trash** filter sits beside Unread, Inbox, Important and + Sent, and composes with the account selector like the others. +- **Restore from trash** (`Ctrl+R`), enabled while the trash view is showing. + A message this application deleted returns to the folder it came from; one + trashed by another client returns to the inbox. +- **Find stranded deleted mail** (`Ctrl+Alt+T`), in the Message menu. It lists + mail tagged `deleted` that never moved anywhere. Run it whenever you like; + it reports and moves nothing on its own. +- An optional per-account `inbox` key, naming the inbox folder a restore falls + back to when a message carries no record of where it came from. It defaults + to `Inbox`, so an account whose inbox is named that needs nothing. +- `Del` now deletes, alongside `Ctrl+D`. It still edits text in the query bar + and in any other text field, so nothing is lost where the key already had a + job. + +### Changed + +- Open thread, Clear message pane and Clear selection appear in the View menu. + All three existed and were reachable only by their shortcuts. + +### Upgrading + +**Every account now needs a `trash` key** in `qtmaildir.conf`, naming its +trash folder relative to `maildir`: + + [account.work] + maildir = work + trash = Trash + +The folder must be one your `mbsync` configuration actually syncs, or the move +will never reach the server. Accounts without the key still load and still +read mail, but Delete cannot work on them and a warning says so at startup. + +**Name the folder exactly as it exists on the server.** A trash or inbox name +that does not match creates that folder rather than reporting an error, and +under mbsync's `Create Both` the wrongly named folder then propagates to the +mail server, where other clients will see it. + +**Mail deleted by earlier versions is not migrated.** It carries the `deleted` +tag and sits wherever it always was. Use **Find stranded deleted mail** to +review it, and Delete on what should really go. + +Note that Delete's reversibility depends on your provider: a trash folder the +provider purges on a timer will eventually remove the mail for good. + + ## [0.25.0] - 2026-08-17 Acting on a row now means the message that row displays, not the whole -- cgit v1.2.3 From 2e0db925d5ca7100d8405ffc352ae435cdbdb73d Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Wed, 19 Aug 2026 10:27:02 +0200 Subject: fix(trash): refresh the list when a restore empties a row 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 stops matching the query the list was built from. That is a state no tag change can express, and nothing in onMessagesMoved() removes a row, deliberately: in an ordinary view a deleted message's card should stay put, since one deleted message does not doom the conversation. refreshCurrentQuery(), not runCurrentQuery(). The refresh runs immediately after the undo entry is pushed, and re-running the query outright clears the undo stack, which would make Restore the one mutation in the window with no way back. Gated on isShowingTrash() rather than on the destination, because a Delete is a move too and reaches the same slot. Three tests, each catching a different mutation: the row leaves, undo survives the refresh and still moves the file back, and a delete outside the trash view leaves its row alone. The third one was wrong on its first draft and passed against the mutation it existed to catch. It used a `tag:inbox` view, which looks ordinary but which a deleted message keeps matching, since Delete adds `deleted` and the origin tag and removes nothing. A path query on the inbox folder is the honest instrument: the file really leaves, so the row survives only because nothing refreshed. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 4 ++ src/mainwindow.cpp | 23 ++++++ tests/test_mainwindow.cpp | 175 ++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 202 insertions(+) (limited to 'CHANGELOG.md') 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 &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(); + auto *view = window.findChild(); + auto *queryEdit = + window.findChild(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(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(); + auto *view = window.findChild(); + auto *queryEdit = + window.findChild(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(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(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(); + auto *view = window.findChild(); + auto *queryEdit = + window.findChild(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(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" -- cgit v1.2.3