From 533c9243bede053f4ea7e1f20de13e72c2b939b8 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sun, 13 Sep 2026 20:17:43 +0200 Subject: refactor: rename the origin tag to moved-from: with overwrite semantics --- src/config.h | 2 +- src/mainwindow.cpp | 32 +++++++++---------- src/mainwindow.h | 12 +++---- src/notmuchworker.cpp | 26 ++++++++++++++- src/notmuchworker.h | 2 +- src/tagdialog.cpp | 2 +- src/types.h | 5 +++ tests/test_mainwindow.cpp | 70 ++++++++++++++++++++--------------------- tests/test_notmuchworker.cpp | 75 +++++++++++++++++++++++++++++++++++++++++--- tests/test_tagdialog.cpp | 4 +-- 10 files changed, 163 insertions(+), 67 deletions(-) diff --git a/src/config.h b/src/config.h index aa3c332..5a43d8c 100644 --- a/src/config.h +++ b/src/config.h @@ -109,7 +109,7 @@ struct Account /// The account's inbox folder, relative to maildir. Optional. /// /// Only Restore reads it, as the destination for a message that carries no - /// `deleted-from:` origin, which is what mail trashed by another client + /// `moved-from:` origin, which is what mail trashed by another client /// looks like. Defaults to "Inbox", the Maildir convention and mbsync's /// own default. /// diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 2cba6b1..6c66953 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -5969,7 +5969,7 @@ MainWindow::TagPresence MainWindow::selectionTagPresence( // three-message thread left the replies undeleted, so the union // carried no `deleted`, so a second press read the row as // not-deleted and deleted it AGAIN, trash-to-trash, ending with - // `deleted-from:inbox` and `deleted-from:Trash` at once and no way + // `moved-from:inbox` and `moved-from:Trash` at once and no way // back. Item 177 removes the case rather than the symptom: a // three-message row is a conversation now and is asked about its // conversation, above. @@ -5979,7 +5979,7 @@ MainWindow::TagPresence MainWindow::selectionTagPresence( // messageById() and NOT summary.firstMessageTags, which is the // value the QUERY delivered and is not refreshed by an optimistic // update: applyMessageTagChange() writes the row's node, so after - // a delete the node reads `deleted, deleted-from:inbox` while the + // a delete the node reads `deleted, moved-from:inbox` while the // summary still reads `inbox, unread`. Measured, and preferring // the summary left this defect exactly as it was. tags = own.messageId.isEmpty() ? summary.firstMessageTags @@ -6349,7 +6349,7 @@ void MainWindow::trashMessages(const QStringList &messageIds, // view after being thrown away: measured 2026-08-26 on the user's own // mail, where it was the only message ever deleted from an inbox and // therefore the only one that could show it. Restore does not depend - // on it surviving, since `deleted-from:` carries the origin. + // on it surviving, since `moved-from:` carries the origin. sendMove(it.value(), it.key(), { QStringLiteral("deleted"), kOriginTagPlaceholder() }, { QStringLiteral("unread"), QStringLiteral("inbox") }, @@ -6362,7 +6362,7 @@ void MainWindow::trashMessages(const QStringList &messageIds, QString MainWindow::originTagFor(const QString &dbRelativeFolder) const { - // `acct/inbox` becomes `deleted-from:inbox`. The tag stores the folder + // `acct/inbox` becomes `moved-from:inbox`. The tag stores the folder // relative to the ACCOUNT, never to the database: the account prefix is // recomposed from the message's own path when it is read back, so storing // it would duplicate it and would go stale the day the user renames a @@ -6371,7 +6371,7 @@ QString MainWindow::originTagFor(const QString &dbRelativeFolder) const // Shared by the two sites that need the tag, rather than derived twice. // They disagreed once already: onMessagesMoved() resolved a placeholder // from the folder the worker reported, which on a RESTORE is the trash - // rather than the origin, so the restore stripped `deleted-from:Trash` + // rather than the origin, so the restore stripped `moved-from:Trash` // and left the real tag in place. const Account account = accountForMessagePath(dbRelativeFolder + QLatin1Char('/')); @@ -6382,7 +6382,7 @@ QString MainWindow::originTagFor(const QString &dbRelativeFolder) const } if (accountRelative.isEmpty()) return QString(); - return QStringLiteral("deleted-from:%1").arg(accountRelative); + return QString(kOriginTagPrefix) + accountRelative; } void MainWindow::trashThreads(const QStringList &threadIds) @@ -6476,9 +6476,9 @@ void MainWindow::onThreadMessagesResolved(const QStringList &messageIds, return; // Restore, resolved per message: each one goes back to the folder its own - // `deleted-from:` tag names, so a thread whose messages were deleted from + // `moved-from:` tag names, so a thread whose messages were deleted from // different folders reassembles correctly rather than collapsing into one. - const QString prefix = QStringLiteral("deleted-from:"); + const QString prefix = QString::fromLatin1(kOriginTagPrefix); QHash byOrigin; QStringList unknown; for (int i = 0; i < messageIds.size(); ++i) { @@ -6592,7 +6592,7 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, if (messageIds.size() != paths.size() || messageIds.size() != tags.size()) return; - const QString prefix = QStringLiteral("deleted-from:"); + const QString prefix = QString::fromLatin1(kOriginTagPrefix); QHash byOrigin; QHash byInbox; QStringList stranded; @@ -6652,7 +6652,7 @@ void MainWindow::restoreResolvedMessages(const QStringList &messageIds, // and is out of scope here. // // The destination FOLDER, taken from the key rather than from - // `origin` above: that is the finished TAG, `deleted-from:Inbox`, + // `origin` above: that is the finished TAG, `moved-from:Inbox`, // which never equals `Inbox` however the account spells it. The // comparison was therefore always false and the `inbox` tag never came // back, so a restored message sat in the inbox folder invisible to the @@ -6900,7 +6900,7 @@ void MainWindow::restoreSelected(bool fallbackToInbox) // Where each message came from, read back off its own tag. This is what // the tag exists for: the file has moved, so nothing on disk and nothing // in notmuch still records the original folder. - const QString prefix = QStringLiteral("deleted-from:"); + const QString prefix = QString::fromLatin1(kOriginTagPrefix); QHash byOrigin; QStringList unknown; for (const QString &messageId : scope.messageIds) { @@ -6986,8 +6986,8 @@ void MainWindow::restoreSelected(bool fallbackToInbox) // onMessagesMoved() resolves the placeholder from the origin the // WORKER reports, which is where the message is coming FROM. On a // delete that is the inbox and correct; on a restore it is the trash, - // so the placeholder resolved to `deleted-from:Trash` and asked to - // remove a tag that never existed, while the real `deleted-from:inbox` + // so the placeholder resolved to `moved-from:Trash` and asked to + // remove a tag that never existed, while the real `moved-from:inbox` // was never named. The message came home still claiming to have been // deleted from somewhere, which then made Restore offer to move a // message that was already back. @@ -7102,7 +7102,7 @@ void MainWindow::sendMove(const QStringList &messageIds, // account before the first confirmation arrives both name `acct/Trash`, // so the second insert overwrote the first and the second confirmation // took an empty PendingMove. That file landed in the trash carrying - // neither `deleted` nor `deleted-from:`, which makes it unrestorable and + // neither `deleted` nor `moved-from:`, which makes it unrestorable and // invisible to a `tag:deleted` query. The worker handles one move at a // time on its own thread and emits in the order it was asked, so a plain // FIFO matches confirmations to requests without needing a key at all. @@ -7279,7 +7279,7 @@ void MainWindow::onMessagesMoved(const QMap &originByMessageId return; // The origin differs per message, so the tags do too: two messages deleted - // from different folders get different `deleted-from:` tags out of one + // from different folders get different `moved-from:` tags out of one // gesture. Grouped by the resolved tag list so identical ones still travel // as a single write. QHash byOrigin; @@ -7325,7 +7325,7 @@ void MainWindow::onMessagesMoved(const QMap &originByMessageId // It used to be handed pending.add straight, which still holds the // unresolved placeholder: undo then asked to remove a tag by that // literal name, which no message carries, so the removal was a silent - // no-op and `deleted-from:inbox` survived the undo. The file came home + // no-op and `moved-from:inbox` survived the undo. The file came home // still claiming to have been deleted from somewhere. Same defect as // the one the second-Delete path had, reached through Ctrl+Z instead. // diff --git a/src/mainwindow.h b/src/mainwindow.h index 55d67ae..97d90e5 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -1187,7 +1187,7 @@ private: /// failed, which is the half-done state item 103 exists to remove. /// /// `add` may contain the placeholder kOriginTagPlaceholder, which - /// onMessagesMoved() replaces with `deleted-from:` per message. + /// onMessagesMoved() replaces with `moved-from:` per message. /// The origin is not known until the worker reports it, and it differs per /// message in a multi-row selection. /// `fromUndo` marks a move the undo stack itself started, which must NOT @@ -1241,7 +1241,7 @@ private: const QString &requestTag); /// The inverse: moves each selected row's message back to the folder its - /// `deleted-from:` tag names, stripping both tags. + /// `moved-from:` tag names, stripping both tags. /// /// `fallbackToInbox` decides what happens to a message with NO origin tag, /// and the two callers want opposite things. From the trash view the @@ -1320,7 +1320,7 @@ private: /// in it, and such a message carries no tag of ours. bool isShowingTrash() const; - /// The `deleted-from:` tag naming `dbRelativeFolder`, or empty when no + /// The `moved-from:` tag naming `dbRelativeFolder`, or empty when no /// account owns it. /// /// One rule for both sites that need the tag: the delete that writes it @@ -1381,7 +1381,7 @@ private: /// account before the first confirmation arrives name the same folder, so /// a keyed map dropped the first entry and left the second confirmation /// with nothing to apply. That file reached the trash carrying neither - /// `deleted` nor `deleted-from:`, unrestorable and invisible to a + /// `deleted` nor `moved-from:`, unrestorable and invisible to a /// `tag:deleted` query. The worker moves one batch at a time and emits in /// request order, so position alone matches a confirmation to its request. struct PendingMove { @@ -1477,7 +1477,7 @@ private: friend class MessageTagCommand; friend class MoveCommand; - /// Stands in for `deleted-from:` between asking for a move and + /// Stands in for `moved-from:` between asking for a move and /// learning where each message actually came from. Not a tag anyone ever /// sees: onMessagesMoved() substitutes the real one per message before /// anything is written. @@ -2061,7 +2061,7 @@ private: /// Undo entry for a message MOVE, which is a file rename plus a tag change. /// /// The destination is CARRIED rather than derived, and that is the whole -/// reason `deleted-from:` exists at all. A Maildir filename does not record +/// reason `moved-from:` exists at all. A Maildir filename does not record /// where a message came from, and once the file has moved notmuch cannot /// answer either, so an undo that recomputed the origin would have nothing to /// recompute it from. Each message carries its own, since one selection can diff --git a/src/notmuchworker.cpp b/src/notmuchworker.cpp index bc3ae65..2672f6a 100644 --- a/src/notmuchworker.cpp +++ b/src/notmuchworker.cpp @@ -36,6 +36,7 @@ #include #include +#include #include "maildirname.h" #include "mimeparser.h" @@ -1122,17 +1123,40 @@ void NotmuchWorker::applyTags(const TagChange &change) // write, so the only way to know is to look first. const QStringList before = tagsOf(message.get()); + // One origin tag ever: writing a `moved-from:` tag strips any OTHER + // tag with that prefix the message still carries, so a message that + // travelled inbox -> spam -> trash ends with exactly one origin and + // Restore has one answer. Without this the reader's first-match + // break() picks silently. + QStringList strippedOrigins; + const bool writingOrigin = + std::any_of(change.added.cbegin(), change.added.cend(), + [](const QString &t) { + return t.startsWith(QLatin1String(kOriginTagPrefix)); + }); + if (writingOrigin) { + for (const QString &tag : std::as_const(before)) { + if (tag.startsWith(QLatin1String(kOriginTagPrefix)) + && !change.added.contains(tag)) { + strippedOrigins.append(tag); + } + } + } + bool moves = false; for (const QString &tag : change.removed) moves = moves || before.contains(tag); for (const QString &tag : change.added) moves = moves || !before.contains(tag); + moves = moves || !strippedOrigins.isEmpty(); if (moves) changedIds.append(id); notmuch_message_freeze(message.get()); for (const QString &tag : change.removed) notmuch_message_remove_tag(message.get(), tag.toUtf8().constData()); + for (const QString &tag : strippedOrigins) + notmuch_message_remove_tag(message.get(), tag.toUtf8().constData()); for (const QString &tag : change.added) notmuch_message_add_tag(message.get(), tag.toUtf8().constData()); notmuch_message_thaw(message.get()); @@ -1689,7 +1713,7 @@ void NotmuchWorker::resolveQuery(const QString &query, QDir(dbRoot).relativeFilePath(QString::fromUtf8(rawName))); // Joined by a TAB, not a space. A notmuch tag may absolutely contain // a space: a Maildir folder named "Inbox/SlackBuilds users" produces - // `deleted-from:Inbox/SlackBuilds users`, and splitting that on spaces + // `moved-from:Inbox/SlackBuilds users`, and splitting that on spaces // truncated the folder to "Inbox/SlackBuilds". Restore then moved the // messages into a folder of that name, CREATING it, so four real // messages ended up in a directory mbsync does not sync and the user diff --git a/src/notmuchworker.h b/src/notmuchworker.h index 32eab58..b4e9707 100644 --- a/src/notmuchworker.h +++ b/src/notmuchworker.h @@ -398,7 +398,7 @@ signals: /// order. `requestTag` is echoed back so a caller can tell which request /// this answers. /// `tags` carries each message's tags joined by a space, in the same - /// order. Needed because Restore reads a message's `deleted-from:` tag to + /// order. Needed because Restore reads a message's `moved-from:` tag to /// decide where to send it, and an unexpanded thread's messages have no /// node in the model to read tags from. void threadMessagesResolved(const QStringList &messageIds, diff --git a/src/tagdialog.cpp b/src/tagdialog.cpp index fb2c629..1810e63 100644 --- a/src/tagdialog.cpp +++ b/src/tagdialog.cpp @@ -276,7 +276,7 @@ void TagDialog::accept() // troublesome tag being CREATED; a tag that already exists is a fact, and // refusing to remove it because it breaks a rule leaves the user with a // tag they can see, cannot type, and cannot get rid of. That happened with - // `deleted-from:Inbox/SlackBuilds users`: an origin tag naming a Maildir + // `moved-from:Inbox/SlackBuilds users`: an origin tag naming a Maildir // folder whose name contains a space, rejected by the space rule, so the // one dialog that could have cleared it refused the only text that names // it. Whether such a tag SHOULD exist is a separate question from whether diff --git a/src/types.h b/src/types.h index fb7901c..6fd54a2 100644 --- a/src/types.h +++ b/src/types.h @@ -23,6 +23,11 @@ #include #include +/// Prefix of the origin tag a move writes. One origin tag per message, +/// overwritten on each move, so a reader cannot be handed two and forced to +/// pick one silently. Not translated, not user-facing. +inline constexpr auto kOriginTagPrefix = "moved-from:"; + struct ThreadSummary { QString threadId; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 9a38a95..a92b3d1 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -11981,7 +11981,7 @@ void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() // would report the tag whether or not the write ever landed. // Re-queried by id and asserted on the TAG LIST the database returns. // - // Not with `tag:"deleted-from:inbox"` in the query: notmuch's parser does + // Not with `tag:"moved-from:inbox"` in the query: notmuch's parser does // not match a quoted tag containing a colon that way, so such a query // returns nothing against a perfectly tagged message and reads as the // feature being broken. Asking for the message and inspecting its tags @@ -11998,7 +11998,7 @@ void TestMainWindow::deleteRecordsWhereTheMessageCameFrom() QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000); const QStringList tags = model->threadAt(0).tags; tagged = tags.contains(QStringLiteral("deleted")) - && tags.contains(QStringLiteral("deleted-from:inbox")); + && tags.contains(QStringLiteral("moved-from:inbox")); if (!tagged) QTest::qWait(200); } @@ -12017,8 +12017,8 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() // onMessagesMoved() resolved the origin placeholder from the folder the // WORKER reported, which is where the message came FROM. On a delete that // is the inbox and correct. On a restore it is the TRASH, so the restore - // asked to remove `deleted-from:Trash`, a tag that had never been written, - // while the real `deleted-from:inbox` was never named and stayed on the + // asked to remove `moved-from:Trash`, a tag that had never been written, + // while the real `moved-from:inbox` was never named and stayed on the // message. It came home still claiming to have been deleted from // somewhere, which makes Restore offer to move a message already at home. // @@ -12066,7 +12066,7 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(backed.fixture().configPath(), QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); // Second press on the same message, which restores it. @@ -12096,11 +12096,11 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() == 0, 15000); - // BOTH tags gone, asked of the database. `deleted-from:` left behind is + // BOTH tags gone, asked of the database. `moved-from:` left behind is // the defect this covers, and it survived a green suite before. // The origin tag specifically, asserted on its OWN query. // - // A combined `tag:deleted or tag:"deleted-from:inbox"` query is NOT + // A combined `tag:deleted or tag:"moved-from:inbox"` query is NOT // equivalent and passed against the bug: `deleted` is removed correctly // and promptly, so the disjunction went to zero on that term alone while // the origin tag was still on the message. Split, so the assertion can @@ -12120,18 +12120,18 @@ void TestMainWindow::deletingTwiceLeavesNoOriginTagBehind() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org")), 1); // The origin tag is gone. This is the defect: it used to survive the - // restore, because the placeholder resolved to `deleted-from:Trash`, the + // restore, because the placeholder resolved to `moved-from:Trash`, the // folder the message was coming FROM, and stripped a tag that had never // been written. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); // And no tag naming the trash was invented in its place. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:twice@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); // `deleted` itself, so a fix that dropped this one instead cannot hide. @@ -12151,7 +12151,7 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() // placeholder for the tags it wrote to the database, but handed the undo // command the raw list. Undo then asked to remove a tag by the // placeholder's literal name, which no message carries, so the removal - // was a silent no-op and `deleted-from:inbox` survived. The message came + // was a silent no-op and `moved-from:inbox` survived. The message came // home still claiming to have been deleted from somewhere, which makes // Restore offer to move a message that is already at home. // @@ -12193,7 +12193,7 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() // about it being REMOVED rather than never having existed. QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); window.findChild(QStringLiteral("undo"))->trigger(); @@ -12217,11 +12217,11 @@ void TestMainWindow::undoOfADeleteRemovesTheOriginTagToo() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org")), 1); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:undotag@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); } @@ -12237,8 +12237,8 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() // from the mismatch: the toggle asked a thread ROW about its thread's // tags, which notmuch gives as a UNION, so deleting the root left the // union carrying no `deleted` and a second press ran Delete AGAIN, - // trash-to-trash, producing `deleted-from:inbox` and - // `deleted-from:Trash` at once with no way back. Item 177 dissolves the + // trash-to-trash, producing `moved-from:inbox` and + // `moved-from:Trash` at once with no way back. Item 177 dissolves the // mismatch rather than patching it: the row and the write now agree about // what they are for. The trash-to-trash assertions stay, because they are // what proves a delete cannot run twice on one message. @@ -12286,7 +12286,7 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() QTRY_VERIFY_WITH_TIMEOUT(folderHasMessageFile(trash, stem), 15000); QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:inbox\"")) == 1, + "tag:\"moved-from:inbox\"")) == 1, 15000); // There is no second press to make any more, and that is the point. @@ -12330,12 +12330,12 @@ void TestMainWindow::deletingALoneMessageRemovesItFromTheInboxAndUndoReturnsIt() // query bar passes against any state of the database. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org")), 1); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); // The tag a re-delete would invent. Its presence is the signature of a // trash-to-trash move rather than a variation on the origin-tag defects. QCOMPARE(notmuchCount(cfg, QStringLiteral("id:tlone@example.org and " - "tag:\"deleted-from:Trash\"")), + "tag:\"moved-from:Trash\"")), 0); QVERIFY2(!folderHasMessageFile(trash, stem), "the message was left in the trash"); @@ -12349,7 +12349,7 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() // when Delete became a move, so a whole conversation stayed in the inbox // wearing a `deleted` chip, which is the half-deleted state item 103 // existed to remove. It moves every message now, each carrying its own - // `deleted-from:` origin so a thread spanning folders reassembles. + // `moved-from:` origin so a thread spanning folders reassembles. // // And the ROOT card did not repaint until it was clicked, while its // replies did. A thread-scoped move updated each message's node; @@ -12418,7 +12418,7 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() // Each with its own origin, which is what makes the move reversible. QCOMPARE(notmuchCount(cfg, thread + QStringLiteral(" and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 3); // The ROOT CARD's own state, which is what the user watches. Read from the @@ -12441,7 +12441,7 @@ void TestMainWindow::deleteThreadMovesEveryMessageAndRepaintsTheRootCard() QCOMPARE(notmuchCount(cfg, thread), 3); QCOMPARE(notmuchCount(cfg, thread + QStringLiteral(" and " - "tag:\"deleted-from:inbox\"")), + "tag:\"moved-from:inbox\"")), 0); QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt0.example.org"))); QVERIFY(!folderHasMessageFile(trash, QStringLiteral("dt1.example.org"))); @@ -12452,8 +12452,8 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() { // A notmuch tag MAY contain a space, and a Maildir folder name may too. // The worker reported each message's tags as one space-joined string, so - // `deleted-from:Inbox/SlackBuilds users` was split back into - // "deleted-from:Inbox/SlackBuilds" and "users", and Restore moved the + // `moved-from:Inbox/SlackBuilds users` was split back into + // "moved-from:Inbox/SlackBuilds" and "users", and Restore moved the // messages to the truncated folder, CREATING it. On the user's real // Maildir that put four messages into a directory mbsync does not sync, // beside the real folder of 808, and they read as missing. @@ -12505,7 +12505,7 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() // The origin tag carries the WHOLE folder name, space included. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds users\"")), 2); @@ -12521,13 +12521,13 @@ void TestMainWindow::aFolderNameWithASpaceSurvivesTheRoundTrip() // could see, could not type, and could not remove. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds users\"")), 0); // Nor a truncated one, which is what a space-split would have written. QCOMPARE(notmuchCount(cfg, thread - + QStringLiteral(" and tag:\"deleted-from:" + + QStringLiteral(" and tag:\"moved-from:" "Inbox/SlackBuilds\"")), 0); @@ -12754,7 +12754,7 @@ void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() // removes the race rather than papering over it with a longer timeout. QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, QStringLiteral("id:ro1@example.org and " - "tag:\"deleted-from:inbox\"")) == 0, + "tag:\"moved-from:inbox\"")) == 0, 15000); QTRY_VERIFY_WITH_TIMEOUT( notmuchCount(cfg, @@ -12784,7 +12784,7 @@ void TestMainWindow::restoreReturnsAMessageToItsOriginFolder() 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 + // carries no `moved-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. // @@ -12814,7 +12814,7 @@ void TestMainWindow::restoreFallsBackToInboxWithoutAnOriginTag() // 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\"")), + "tag:\"moved-from:inbox\"")), 0); queryEdit->setText(QStringLiteral("path:\"acct/Trash/**\"")); @@ -12882,10 +12882,10 @@ void TestMainWindow::undoMovesTheMessageBack() QVERIFY2(!folderHasMessageFile(trash, stem), "undo restored the file and left a copy in the trash"); - // Both tags gone, asked of the database. `deleted-from:` left behind would + // Both tags gone, asked of the database. `moved-from:` left behind would // make Restore offer to move a message that is already home. queryEdit->setText(QStringLiteral( - "id:del3@example.org and (tag:deleted or tag:\"deleted-from:inbox\")")); + "id:del3@example.org and (tag:deleted or tag:\"moved-from:inbox\")")); queryEdit->returnPressed(); QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 0, 15000); // The guard the assertion above needs: a query that matches nothing @@ -13101,7 +13101,7 @@ void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags() // Deletes in one account before the first confirmation arrived both named // `acct/Trash`: the second insert overwrote the first and the second // confirmation took an empty entry. That file reached the trash carrying - // neither `deleted` nor `deleted-from:`, which makes it unrestorable by + // neither `deleted` nor `moved-from:`, which makes it unrestorable by // Restore and invisible to a `tag:deleted` query. WorkerBackedWindow backed; QVERIFY(backed.fixture().addMessage( @@ -13156,7 +13156,7 @@ void TestMainWindow::twoDeletesToOneTrashBothGetTheirTags() // the defect was a write that never happened, and the model would have // shown the optimistic state either way. queryEdit->setText(QStringLiteral( - "tag:deleted and tag:\"deleted-from:inbox\" and " + "tag:deleted and tag:\"moved-from:inbox\" and " "(id:two1@example.org or id:two2@example.org)")); queryEdit->returnPressed(); QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 2, 15000); @@ -13385,7 +13385,7 @@ void TestMainWindow::aMoveThatRelocatesNothingWritesNoTag() QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " "tag:deleted")), 0); QCOMPARE(notmuchCount(cfg, QStringLiteral("id:nomove@example.org and " - "tag:\"deleted-from:inbox\"")), 0); + "tag:\"moved-from:inbox\"")), 0); // And the file never left. QVERIFY(folderHasMessageFile(root + QStringLiteral("/acct/inbox/new"), diff --git a/tests/test_notmuchworker.cpp b/tests/test_notmuchworker.cpp index 970f51d..2877894 100644 --- a/tests/test_notmuchworker.cpp +++ b/tests/test_notmuchworker.cpp @@ -54,6 +54,7 @@ private slots: void applyTagsWithNoIdsDoesNothing(); void applyTagsReportsOnlyTheMessagesItChanged(); void applyTagsThatChangeNothingEmitNothing(); + void applyTagsKeepsOnlyTheNewestOriginTag(); void queryStillWorksAfterWrite(); void aThreadCarriesItsCardMessagesOwnTags(); @@ -142,8 +143,10 @@ private: QString fileOf(const QString &messageId, const QString &configPath = QString()); - /// Tags of one message, read back through a fresh worker query. - QStringList tagsOf(const QString &messageId); + /// Tags of one message, read back through a fresh worker query. Defaults to + /// the shared fixture; a test with its own fixture passes its own path. + QStringList tagsOf(const QString &messageId, + const QString &configPath = QString()); QVector messagesOfThread(const QString &threadId, const QString &matchQuery = QString(), bool matchedOnly = false); @@ -401,9 +404,11 @@ QVector TestNotmuchWorker::messagesOfThread(const QString &threadId, return loaded.first().at(0).value>(); } -QStringList TestNotmuchWorker::tagsOf(const QString &messageId) +QStringList TestNotmuchWorker::tagsOf(const QString &messageId, + const QString &configPath) { - NotmuchWorker worker(m_fixture.configPath()); + NotmuchWorker worker(configPath.isEmpty() ? m_fixture.configPath() + : configPath); QSignalSpy loaded(&worker, &NotmuchWorker::threadLoaded); worker.loadThread(QStringLiteral("{id:%1}").arg(messageId), QString(), 1); if (loaded.isEmpty()) @@ -1117,6 +1122,68 @@ void TestNotmuchWorker::applyTagsThatChangeNothingEmitNothing() QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); } +void TestNotmuchWorker::applyTagsKeepsOnlyTheNewestOriginTag() +{ + // A message that travelled inbox -> spam -> trash must hold exactly ONE + // `moved-from:` tag, the newest, or Restore's first-match scan picks an + // origin arbitrarily. Its own fixture, so the tags this test leaves behind + // cannot move the shared one's thread counts. + NotmuchFixture fixture; + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("origin1@example.org"), + QStringLiteral("Travelled"), + QStringLiteral("Erin "), + QStringLiteral("Sun, 7 Jun 2026 10:00:00 +0000"), + QStringLiteral("body"), false)); + QVERIFY(fixture.addMessage(QStringLiteral("inbox"), + QStringLiteral("origin2@example.org"), + QStringLiteral("Fresh"), + QStringLiteral("Erin "), + QStringLiteral("Sun, 7 Jun 2026 11:00:00 +0000"), + QStringLiteral("body"), false)); + QVERIFY(fixture.index()); + + NotmuchWorker worker(fixture.configPath()); + QSignalSpy errors(&worker, &NotmuchWorker::errorOccurred); + + // An earlier move left an origin behind. + worker.applyTags(TagChange{ { QStringLiteral("origin1@example.org") }, + { QStringLiteral("moved-from:inbox") }, + {}, + QStringLiteral("Earlier move") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + QVERIFY(tagsOf(QStringLiteral("origin1@example.org"), fixture.configPath()) + .contains(QStringLiteral("moved-from:inbox"))); + + // The move under test: a new origin lands while the old one is still there. + worker.applyTags(TagChange{ + { QStringLiteral("origin1@example.org"), + QStringLiteral("origin2@example.org") }, + { QStringLiteral("moved-from:Spam"), QStringLiteral("spam") }, + { QStringLiteral("inbox"), QStringLiteral("unread") }, + QStringLiteral("Mark spam") }); + QVERIFY2(errors.isEmpty(), qPrintable(errors.value(0).value(0).toString())); + + const QStringList travelled = + tagsOf(QStringLiteral("origin1@example.org"), fixture.configPath()); + QVERIFY(!travelled.contains(QStringLiteral("moved-from:inbox"))); + QVERIFY(travelled.contains(QStringLiteral("moved-from:Spam"))); + int origins = 0; + for (const QString &tag : travelled) { + if (tag.startsWith(QStringLiteral("moved-from:"))) + ++origins; + } + QCOMPARE(origins, 1); + + // The message that never carried an origin keeps exactly the new one, and + // the move still removed what a move removes. + const QStringList fresh = + tagsOf(QStringLiteral("origin2@example.org"), fixture.configPath()); + QVERIFY(fresh.contains(QStringLiteral("moved-from:Spam"))); + QVERIFY(fresh.contains(QStringLiteral("spam"))); + QVERIFY(!fresh.contains(QStringLiteral("inbox"))); +} + void TestNotmuchWorker::queryStillWorksAfterWrite() { // applyTags closes the read-only handle to take the write lock. The same diff --git a/tests/test_tagdialog.cpp b/tests/test_tagdialog.cpp index 9b12109..93a7229 100644 --- a/tests/test_tagdialog.cpp +++ b/tests/test_tagdialog.cpp @@ -176,14 +176,14 @@ void TestTagDialog::aTagWithASpaceCanStillBeRemoved() // they can see and cannot get rid of. // // Reached by a real Maildir: a folder named "Inbox/SlackBuilds users" - // produced `deleted-from:Inbox/SlackBuilds users`, and the one dialog that + // produced `moved-from:Inbox/SlackBuilds users`, and the one dialog that // could have cleared it refused the only text that names it. // // Only the TYPED route was blocked. Unchecking appends to the removal list // after validation has run, so it worked throughout; that asymmetry is why // both routes are asserted here rather than just the one that failed. const QString spaced = - QStringLiteral("deleted-from:Inbox/SlackBuilds users"); + QStringLiteral("moved-from:Inbox/SlackBuilds users"); QHash current; current.insert(spaced, 1); -- cgit v1.2.3