From 0c5eea8f0d0ccc5b8eb6220814c9e212d6c1ccc2 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 19:19:58 +0200 Subject: feat(queries): save a query from the UI, and split the buttons off the query row Second half of item 23, on top of the storage change. A query can now be kept without hand-editing a file, and the row of buttons no longer grows without bound. Ctrl+S opens a dialog on whatever is in the query bar, taking a name, an optional account scope and whether the query is pinned. It preselects the account already chosen in the dropdown, since that is the scope the user is looking at, and it says so when a name is about to replace an existing query rather than refusing the name: overwriting a saved query on purpose is a normal edit, and the only thing worth preventing is doing it without noticing. Saving over an entry keeps the stored entry's unknown fields rather than the dialog's fresh value, so a field written by a later build survives being edited here. The saved queries move to a row of their own beneath the query bar, pinned ones as buttons and the rest behind a More queries menu that only exists when something is in it. The ponytail note that stood in the query row predicted exactly this: an unbounded list of buttons sharing the row squeezed the field. Sent moves down with them and is still not a saved query, for the reason already recorded there. A saved query's account scope goes through the account DROPDOWN rather than being baked into the query text. runQuery() already wraps the query in the selected account's path, so pre-scoping here would apply it twice, and setting the dropdown also shows the user which scope they are in. An unscoped query clears the selection rather than inheriting whatever the last one left, which is the same defect the rules preview had. Seven tests, three mutations. Ignoring the pinned flag fails two of them, pre-scoping the text instead of setting the dropdown fails two, and letting an unscoped query inherit the previous account fails one. The menu-absence test initially passed against no implementation at all, since it only asserted a widget was missing; it now proves the row was populated first, which is the guard that class of test needs. Two existing invariants caught real omissions rather than needing adjustment: every registered action must appear in KeyMap::knownActions(), which is what gives it a configurable binding, and every action needs its own icon. Co-Authored-By: Claude Opus 5 --- README.md | 58 ++++++++++++++++++++++++++++++++++++++++++++-------------- 1 file changed, 44 insertions(+), 14 deletions(-) (limited to 'README.md') diff --git a/README.md b/README.md index 0921791..81ec7d8 100644 --- a/README.md +++ b/README.md @@ -106,7 +106,7 @@ identity. ; point: once you zoom with Ctrl+wheel or Ctrl+/Ctrl-, that is remembered ; separately and this value no longer applies. ; message_zoom = 1.0 -; Optional. Which [queries] entry to open at startup, by name. Defaults to +; Optional. Which saved query to open at startup, by name. Defaults to ; Unread. Falls back to the first saved query if no query by this name ; exists, and warns if you named one explicitly. ; startup_query = Unread @@ -191,11 +191,6 @@ shopping = #3366cc ; also colours shopping/amazon, shopping/nike, ... shopping/amazon = #ff9900 ; ... unless the exact tag overrides it work = #cc4444 -[queries] -Inbox = tag:inbox -Unread = tag:unread -Important = tag:flagged - [keys] Ctrl+E = archive Ctrl+D = delete @@ -203,14 +198,47 @@ j = next_thread k = prev_thread ``` -Saved-query buttons appear in alphabetical order rather than file order: -QSettings returns keys sorted, and preserving file order would mean -hand-rolling an INI parser. Which query opens at startup is therefore a -separate setting, `[general] startup_query`, rather than "the first one". +### Saved queries + +Saved queries live in `~/.config/qtmaildir/queries.json`, not in the config +file. They are written by the **Save query** action (`Ctrl+S`), which names the +query in the bar, optionally scopes it to one account, and chooses whether it +gets a button: + +```json +{ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Unread", "query": "tag:unread", "pinned": true }, + { "name": "Billing", "query": "from:billing", "account": "work" } + ] +} +``` -The button text is the key you write here, so these names are yours to -choose. `Important = tag:flagged` and `Flagged = tag:flagged` run the same -query and differ only in what the button says. +The order in the file is the order the buttons appear in, so rearranging them +is a matter of moving lines. `pinned` decides between a button and the **More +queries** menu, which keeps the row usable once you have more than a handful. +`account` names an `[account.]` section and scopes the query to it, the +same as choosing that account in the dropdown; leave it out for a query that +spans every account. + +The name is what the button says, so `Important` and `Flagged` can run the same +query and differ only in the label. + +**Upgrading from 0.17.0 or earlier.** Saved queries used to live in a +`[queries]` section of `qtmaildir.conf`. The first launch after upgrading reads +that section, writes `queries.json` from it, and marks every entry pinned so +your buttons stay where they were. Your config file is not modified: the old +`[queries]` section is left exactly as it is, ignored from then on, and you can +delete it by hand whenever you like. The reason it is not removed for you is +that rewriting the file would drop your comments and reorder your keys. + +One behaviour changes with the move. Buttons used to appear in alphabetical +order, because the INI backend returns keys sorted and preserving file order +would have meant hand-rolling a parser. They now follow the file. If +`startup_query` names a query that does not exist, the fallback is likewise the +first query in the file rather than the alphabetically first one. ### Sent mail @@ -253,7 +281,8 @@ behaves like any other query, threads and all. ## The query bar The bar at the top takes a notmuch query and shows the matching threads. -Saved queries from `[queries]` sit beside it as buttons. +Saved queries sit on their own row beneath it: the pinned ones as buttons, the +rest behind **More queries**. `Ctrl+S` keeps the current query as a new one. Completion helps with the syntax rather than replacing it. `Ctrl+Space` opens the popup, and ordinary typing keeps it up to date. Candidates carry a @@ -478,6 +507,7 @@ Defaults, all rebindable through `[keys]`: | `Ctrl+I` | `flag` | Mark important (adds `flagged`) | | `Ctrl+L` | `focus_query` | Focus and select the query bar | | `Ctrl+Space` | `complete_query` | Focus the query bar and offer completions | +| `Ctrl+S` | `save_query` | Keep the current query as a saved query | | `Ctrl+H` | `toggle_html` | Switch the thread between HTML and plain text | | `Ctrl+M` | `load_remote` | Load remote images for the current thread | | `Ctrl+T` | `edit_tags` | Add or remove any tag on the selected threads | -- cgit v1.2.3 From 97c81f8cad571ce9ce724ddab8269e911df05a7c Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 19:52:13 +0200 Subject: feat(queries): make Sent a saved query rather than a fixed button The user asked whether the default queries could be unified with Sent. The answer runs the other way: Sent joins the saved queries rather than the saved queries becoming hardcoded. Inbox, Unread and Important are complete strings that depend on nothing and can never go stale, so generating them would buy nothing and would cost the four things the file just gained: reordering, unpinning, renaming and deleting. Hardcoding them would also make them undeletable, which is a regression for anyone who does not want one of them. Sent is different only in that its query CANNOT be stored: it is composed from every account's `sent` key, so a stored copy goes stale the moment a folder is renamed. That is a property of Sent, not of "default queries". Storing the GENERATOR rather than its output keeps both halves: `"generated": "sent"` still resolves from the accounts at click time, and the entry is an ordinary row that can be reordered, renamed, unpinned or removed. The row now follows one rule instead of carrying one member the user did not own. Two properties had to travel with the entry. The composed query, resolved through Config::resolvedQuery() so what lands in the bar is what actually ran; and FLAT mode, since a sent view lists messages and a threaded one folds every reply back into the conversation the user sent one message into. The sent generator implies flat rather than trusting the file to say so, because a hand-edited row would otherwise produce a threaded sent view. An unknown generator is reported but the row is KEPT: a later build may know it, and dropping it here would delete it from the file on the next save, which is the same data loss the unknown-field handling exists to prevent. A generator whose accounts configure nothing is skipped entirely, exactly as the hardcoded button was hidden rather than offering one that finds nothing. Eight new tests. The four pre-existing Sent tests reach this through migration and were left alone, which is what proves the migrated path still behaves; the new ones cover a STORED file, which is the path every launch after the first takes. Mutations: a generator resolving to nothing fails three, ignoring flat fails two, and not skipping an empty generator fails one. A rename test guards the property the change exists for, since anything keyed on the literal name "Sent" would break it. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 12 +++- README.md | 28 +++++++-- src/config.cpp | 66 +++++++++++++++++++- src/config.h | 21 +++++++ src/mainwindow.cpp | 50 ++++++++------- tests/test_config.cpp | 156 ++++++++++++++++++++++++++++++++++++++++++++++ tests/test_mainwindow.cpp | 111 +++++++++++++++++++++++++++++++++ 7 files changed, 413 insertions(+), 31 deletions(-) (limited to 'README.md') diff --git a/CHANGELOG.md b/CHANGELOG.md index 428e3e5..b384eda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,6 +22,11 @@ point at which they are stable. **order**, a `pinned` flag and an optional account scope. The order in the file is the order the buttons appear in, so rearranging them is a matter of moving lines. +- **Sent is a saved query now**, carrying `"generated": "sent"` instead of a + stored query. It is still composed from your accounts' `sent` keys every time + you click it, so correcting a folder name still updates it with no edit, but + it can now be reordered, renamed, unpinned or deleted like any other entry + rather than being a fixed button you did not own. ### Changed @@ -30,13 +35,18 @@ point at which they are stable. query field is no longer squeezed by a long list of buttons. - Saved-query buttons follow the file's order instead of appearing alphabetically. +- The Sent button is no longer hardcoded beside the saved queries, so the whole + row now follows one rule instead of having one member that behaved + differently from its neighbours. ### Upgrading Saved queries move out of the `[queries]` section of `qtmaildir.conf` and into `~/.config/qtmaildir/queries.json`. **The first launch migrates them for you**: the section is read, the JSON file is written from it, and every entry is -marked pinned so your buttons stay where they were. +marked pinned so your buttons stay where they were. Sent is appended as a +`generated` entry, where its button already sat, provided an account configures +a sent folder. Your config file is left byte-for-byte alone. The old `[queries]` section stays in it, ignored from then on, and can be deleted by hand whenever you like. It is diff --git a/README.md b/README.md index 81ec7d8..749d89d 100644 --- a/README.md +++ b/README.md @@ -223,16 +223,36 @@ queries** menu, which keeps the row usable once you have more than a handful. same as choosing that account in the dropdown; leave it out for a query that spans every account. +**Sent is an entry like any other**, and the one that carries `generated` +instead of `query`: + +```json +{ "name": "Sent", "generated": "sent", "pinned": true } +``` + +A generated query is composed from your accounts every time you click it, +rather than stored. That is why Sent has no `query` of its own: it is built +from every account's `sent` key, so adding an account or correcting a folder +name updates the button with no edit here. A stored copy of the same string +would quietly go on naming the old folder. + +Being an ordinary entry, it can be reordered, renamed, unpinned or deleted like +the rest. Renaming it to `Posta inviata` changes only the label. `sent` is the +only generator today, and it is skipped entirely when no account configures a +sent folder, rather than offering a button that finds nothing. + The name is what the button says, so `Important` and `Flagged` can run the same query and differ only in the label. **Upgrading from 0.17.0 or earlier.** Saved queries used to live in a `[queries]` section of `qtmaildir.conf`. The first launch after upgrading reads that section, writes `queries.json` from it, and marks every entry pinned so -your buttons stay where they were. Your config file is not modified: the old -`[queries]` section is left exactly as it is, ignored from then on, and you can -delete it by hand whenever you like. The reason it is not removed for you is -that rewriting the file would drop your comments and reorder your keys. +your buttons stay where they were. Sent is added as a `generated` entry at the +end, where its button already sat, provided an account configures a sent +folder. Your config file is not modified: the old `[queries]` section is left +exactly as it is, ignored from then on, and you can delete it by hand whenever +you like. The reason it is not removed for you is that rewriting the file would +drop your comments and reorder your keys. One behaviour changes with the move. Buttons used to appear in alphabetical order, because the INI backend returns keys sorted and preserving file order diff --git a/src/config.cpp b/src/config.cpp index 8cd5e56..9edba50 100644 --- a/src/config.cpp +++ b/src/config.cpp @@ -51,6 +51,14 @@ constexpr int kMaxToolbarIconSize = 64; /// two-repo change and no hook stops tagging if it is half-deployed. constexpr int kQueriesFormatVersion = 1; +/// Generators a saved query may name in its `generated` field. +/// +/// A closed set, checked on load so a typo is reported rather than producing a +/// button that silently finds nothing. Adding one here needs no format bump: +/// an older build keeps the row and reports it, which is why an unknown +/// generator is a problem rather than a reason to drop the entry. +const QStringList kQueryGenerators = { QStringLiteral("sent") }; + } // namespace QString Account::scopedQuery(const QString &query) const @@ -457,9 +465,26 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) } settings.endGroup(); + // Sent was a hardcoded button beside the saved queries and becomes an + // ordinary row here, so it can be reordered, renamed, unpinned or + // removed like any other. It stays GENERATED, so it still follows the + // accounts. Appended last, where the button already sat. + // + // Only when an account actually configures a sent folder: the button + // was hidden entirely otherwise, and migrating a row that always finds + // nothing would be worse than what it replaces. + if (!allSentQuery().isEmpty()) { + SavedQuery sent; + sent.name = QStringLiteral("Sent"); + sent.generated = QStringLiteral("sent"); + sent.pinned = true; + sent.flat = true; + m_savedQueries.append(sent); + } + // Order is alphabetical here because childKeys() is genuinely all the // INI knows. The user reorders once and it sticks from then on. - if (!names.isEmpty() && !saveSavedQueries()) { + if (!m_savedQueries.isEmpty() && !saveSavedQueries()) { addProblem(QStringLiteral("Could not write saved queries to %1.") .arg(m_queriesPath)); } @@ -516,6 +541,26 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) query.query = object.value(QStringLiteral("query")).toString(); query.pinned = object.value(QStringLiteral("pinned")).toBool(false); query.account = object.value(QStringLiteral("account")).toString(); + query.generated = object.value(QStringLiteral("generated")).toString(); + // A generator carries its own view mode, so "sent" is flat whether or + // not the file says so. Storing it as a plain field would let a + // hand-edited or migrated-from-elsewhere row produce a THREADED sent + // view, which folds every reply back into the conversation the user + // sent one message into. The file may still set it for an ordinary + // query. + query.flat = object.value(QStringLiteral("flat")).toBool(false) + || query.generated == QStringLiteral("sent"); + + if (query.isGenerated() + && !kQueryGenerators.contains(query.generated)) { + // Reported but KEPT. A later build may know this generator, and + // dropping the row here would delete it from the file on the next + // save, which is the same data loss the unknown-field handling + // exists to prevent. + addProblem(QStringLiteral("Saved query '%1' uses an unknown " + "generator '%2' and will find nothing.") + .arg(query.name, query.generated)); + } if (query.name.isEmpty()) { addProblem(QStringLiteral("A saved query in %1 has no name and was " @@ -526,7 +571,8 @@ void Config::loadSavedQueries(const QString &configPath, QSettings &settings) for (auto it = object.begin(); it != object.end(); ++it) { static const QStringList known = { QStringLiteral("name"), QStringLiteral("query"), - QStringLiteral("pinned"), QStringLiteral("account") + QStringLiteral("pinned"), QStringLiteral("account"), + QStringLiteral("generated"), QStringLiteral("flat") }; if (!known.contains(it.key())) query.unknown.insert(it.key(), it.value()); @@ -550,6 +596,10 @@ bool Config::saveSavedQueries() const object.insert(QStringLiteral("pinned"), true); if (!query.account.isEmpty()) object.insert(QStringLiteral("account"), query.account); + if (query.isGenerated()) + object.insert(QStringLiteral("generated"), query.generated); + if (query.flat) + object.insert(QStringLiteral("flat"), true); for (auto it = query.unknown.begin(); it != query.unknown.end(); ++it) object.insert(it.key(), it.value()); array.append(object); @@ -572,6 +622,18 @@ bool Config::saveSavedQueries() const QString Config::resolvedQuery(const SavedQuery &query) const { + // Composed from the accounts every time it is asked for, which is the + // point: the answer follows the config rather than a copy of it taken when + // the entry was written. + if (query.isGenerated()) { + if (query.generated == QStringLiteral("sent")) + return allSentQuery(); + // An unknown generator was reported on load. Empty rather than the + // bare stored query, which for a generated entry is empty anyway and + // would otherwise run as "match everything". + return QString(); + } + if (query.account.isEmpty()) return query.query; diff --git a/src/config.h b/src/config.h index 4211b0c..b9ee9d6 100644 --- a/src/config.h +++ b/src/config.h @@ -122,6 +122,27 @@ struct SavedQuery /// user edits it. Resolve through Config::resolvedQuery(). QString account; + /// Names a builtin that COMPOSES this query from the accounts at run time, + /// rather than storing it. Empty for an ordinary query. + /// + /// "sent" is the only one today. Its query is built from every account's + /// `sent` key, so adding an account or correcting a folder name is a config + /// edit and nothing else; a stored copy of the same string would go stale + /// silently. That property is why Sent used to be hardcoded beside the + /// saved queries instead of living with them, which left one button on the + /// row that could not be reordered, renamed, unpinned or removed. + /// + /// Storing the GENERATOR rather than its output keeps both: the query stays + /// live, and the entry is an ordinary row the user owns. + QString generated; + + /// Lists messages rather than threads. Set for the sent view, where a + /// thread would fold every reply back into the conversation the user sent + /// one message into. + bool flat = false; + + bool isGenerated() const { return !generated.isEmpty(); } + /// Keys this build does not understand, preserved verbatim so a file /// written by a later version survives a save from this one. QJsonObject unknown; diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index f06b308..62f4751 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1673,40 +1673,34 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) auto *box = new QHBoxLayout(row); box->setContentsMargins(0, 0, 0, 0); + // Sent is an ordinary row here, not a hardcoded button beside the others. + // It is still GENERATED, so its query is composed from the accounts' `sent` + // keys at click time and correcting a folder name stays a config edit and + // nothing else; what changed is that the entry can now be reordered, + // renamed, unpinned or removed like every other, instead of being the one + // control on the row the user did not own. QList unpinned; for (const SavedQuery &saved : m_config.savedQueries()) { + // A generator whose accounts configure nothing produces a button that + // always finds nothing. Skipped entirely, which is what the hardcoded + // Sent button did and is worth keeping. + if (saved.isGenerated() && m_config.resolvedQuery(saved).isEmpty()) + continue; + if (!saved.pinned) { unpinned.append(saved); continue; } auto *button = new QPushButton(saved.name, row); + // The generated entries keep a stable object name so a test can find + // the sent button without depending on what the user renamed it to. + if (saved.generated == QStringLiteral("sent")) + button->setObjectName(QStringLiteral("sentButton")); connect(button, &QPushButton::clicked, this, [this, saved]() { runSavedQuery(saved); }); box->addWidget(button); } - // Sent sits with the saved queries and is not one: its query is COMPOSED - // from the accounts' `sent` keys at click time, so adding an account or - // correcting a folder name is a config edit and nothing else. A stored - // entry holding the same string would go stale silently, and could not - // narrow to the selected account the way this does through - // runCurrentQuery()'s existing scope wrap. - // - // Hidden entirely when no account configures a sent folder, rather than - // offering a button that always finds nothing. - if (!m_config.allSentQuery().isEmpty()) { - auto *sentButton = new QPushButton(tr("Sent"), row); - sentButton->setObjectName(QStringLiteral("sentButton")); - connect(sentButton, &QPushButton::clicked, this, [this]() { - m_queryEdit->setText(m_config.allSentQuery()); - // Flat for this query only. runCurrentQuery() clears it again for - // anything else, including the same query typed by hand, so the - // flag cannot outlive the button that set it. - runQuery(FlatResult::Yes); - }); - box->addWidget(sentButton); - } - // Everything above is left-aligned; the stretch here pushes what follows // to the right edge. The buttons are the row's content and read as a set, // while the overflow menu is a control over that set, so it sits apart @@ -1751,8 +1745,16 @@ void MainWindow::runSavedQuery(const SavedQuery &saved) if (index >= 0) m_accountBox->setCurrentIndex(index); - m_queryEdit->setText(saved.query); - runCurrentQuery(); + // A generated entry has no stored query: the text is composed from the + // accounts now, so what lands in the bar is what actually ran and the user + // can see and edit it. + m_queryEdit->setText(saved.isGenerated() ? m_config.resolvedQuery(saved) + : saved.query); + + // Flat for this query only. runQuery() sets the mode on EVERY run, so the + // flag cannot outlive the entry that asked for it, including for the same + // query typed by hand afterwards. + runQuery(saved.flat ? FlatResult::Yes : FlatResult::No); } void MainWindow::saveCurrentQuery() diff --git a/tests/test_config.cpp b/tests/test_config.cpp index 8024728..dfe463b 100644 --- a/tests/test_config.cpp +++ b/tests/test_config.cpp @@ -62,6 +62,11 @@ private slots: void futureVersionIsRefusedAndReported(); void startupQueryFallsBackToDocumentOrder(); void scopedSavedQueryParenthesisesADisjunction(); + void aGeneratedQueryResolvesFromTheAccounts(); + void aGeneratedQueryTracksAConfigChange(); + void anUnknownGeneratorResolvesToNothingAndReports(); + void migrationAddsSentWhenAnAccountHasOne(); + void migrationAddsNoSentWithoutTheKey(); void generalSectionKeysAreActuallyRead(); void messageZoomDefaultsAndValidates(); void messageZoomOutOfRangeIsReported(); @@ -1416,5 +1421,156 @@ void TestConfig::scopedSavedQueryParenthesisesADisjunction() QCOMPARE(config.resolvedQuery(orphan), QStringLiteral("tag:inbox")); } +// --------------------------------------------------------------------------- +// Generated saved queries +// --------------------------------------------------------------------------- + +static QString twoAccountsWithSent() +{ + return QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent\n" + "\n" + "[account.personal]\n" + "name=Test User\n" + "address=me@example.net\n" + "maildir=personal\n" + "sent=[Provider]/Posta inviata\n" + ); +} + +void TestConfig::aGeneratedQueryResolvesFromTheAccounts() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })")); + + Config config; + config.load(path); + + const SavedQuery sent = config.savedQueries().at(0); + QVERIFY(sent.isGenerated()); + // The stored query is empty; the text comes from the accounts. + QVERIFY(sent.query.isEmpty()); + QCOMPARE(config.resolvedQuery(sent), config.allSentQuery()); + QVERIFY(config.resolvedQuery(sent).contains( + QStringLiteral("path:\"work-mail/Sent/**\""))); + // The quotes matter: "[" and "]" are Xapian syntax and an unquoted term + // is parsed rather than matched. + QVERIFY(config.resolvedQuery(sent).contains( + QStringLiteral("path:\"personal/[Provider]/Posta inviata/**\""))); + + // Flat, not threaded: a sent view lists messages, and that property has to + // travel with the entry or it is lost the moment Sent is a stored row. + QVERIFY(sent.flat); +} + +/// The whole reason Sent is generated rather than stored. A stored copy would +/// keep naming an account that has been renamed or a folder that has moved. +void TestConfig::aGeneratedQueryTracksAConfigChange() +{ + QTemporaryDir dir; + const QString queries = QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "Sent", "generated": "sent" } ] + })"); + + const QString before = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, queries); + Config first; + first.load(before); + const QString firstResolved = first.resolvedQuery(first.savedQueries().at(0)); + + // The user corrects a folder name. Nothing in queries.json changes. + QTemporaryDir second; + const QString after = writeIni(second, QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent Items\n" + )); + writeQueries(second, queries); + Config later; + later.load(after); + const QString laterResolved = later.resolvedQuery(later.savedQueries().at(0)); + + QVERIFY(firstResolved != laterResolved); + QVERIFY(laterResolved.contains(QStringLiteral("Sent Items"))); +} + +void TestConfig::anUnknownGeneratorResolvesToNothingAndReports() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent()); + writeQueries(dir, QStringLiteral(R"({ + "version": 1, + "queries": [ { "name": "Future", "generated": "not_a_generator" } ] + })")); + + Config config; + config.load(path); + + // Kept rather than dropped: a later build may know this generator, and + // silently deleting the row on save would lose it. + QCOMPARE(config.savedQueries().size(), 1); + QVERIFY(config.resolvedQuery(config.savedQueries().at(0)).isEmpty()); + QVERIFY2(!config.problems().isEmpty(), + "an unknown generator must be reported, not silently inert"); +} + +void TestConfig::migrationAddsSentWhenAnAccountHasOne() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, twoAccountsWithSent() + + QStringLiteral( + "\n[queries]\n" + "Inbox=tag:inbox\n" + )); + + Config config; + config.load(path); + + const QList queries = config.savedQueries(); + QCOMPARE(queries.size(), 2); + // Last, where the button already sat: after the saved queries. + QCOMPARE(queries.at(1).name, QStringLiteral("Sent")); + QVERIFY(queries.at(1).isGenerated()); + QVERIFY(queries.at(1).pinned); + QVERIFY(queries.at(1).flat); +} + +/// Today the button is hidden entirely when no account configures a sent +/// folder, rather than offering one that always finds nothing. The migration +/// must not invent a row that would do exactly that. +void TestConfig::migrationAddsNoSentWithoutTheKey() +{ + QTemporaryDir dir; + const QString path = writeIni(dir, QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "\n" + "[queries]\n" + "Inbox=tag:inbox\n" + )); + + Config config; + config.load(path); + + const QList queries = config.savedQueries(); + QCOMPARE(queries.size(), 1); + QCOMPARE(queries.at(0).name, QStringLiteral("Inbox")); +} + QTEST_MAIN(TestConfig) #include "test_config.moc" diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 42d7d78..883e9a7 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -200,6 +200,9 @@ private slots: void thereIsASaveButtonBesideTheQueryBar(); void theMenuIsRightAlignedAwayFromTheButtons(); void theRowSurvivesWithNothingButUnpinnedQueries(); + void aStoredGeneratedQueryRunsFlatAndComposed(); + void aRenamedSentEntryKeepsWorking(); + void aGeneratedQueryWithNothingToShowIsSkipped(); private: /// Owns the throwaway lock table init() points every test at. A pointer @@ -5664,4 +5667,112 @@ void TestMainWindow::theRowSurvivesWithNothingButUnpinnedQueries() QCOMPARE(menuButton->menu()->actions().size(), 2); } +static QString oneAccountWithSent() +{ + return QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "sent=Sent\n" + ); +} + +/// The existing Sent tests reach the generated entry through MIGRATION, since +/// their configs have no queries.json. This one starts from a stored file, so +/// it covers the path a user is on from the second launch onwards. +void TestMainWindow::aStoredGeneratedQueryRunsFlatAndComposed() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })"), oneAccountWithSent()); + + MainWindow window(config); + auto *button = + window.findChild(QStringLiteral("sentButton")); + QVERIFY(button); + + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + auto *model = window.findChild(); + QVERIFY(model); + QVERIFY2(!model->flatMode(), "the model starts threaded"); + + button->click(); + + // Composed from the account, not read from the file: the entry stores no + // query at all. + QCOMPARE(queryEdit->text(), config.allSentQuery()); + QVERIFY(queryEdit->text().contains( + QStringLiteral("path:\"work-mail/Sent/**\""))); + QVERIFY2(model->flatMode(), + "a sent view must be flat, or replies fold back into the thread"); +} + +/// The point of the change: Sent is the user's row now. Renaming it must not +/// break it, which it would if anything keyed on the literal name "Sent". +void TestMainWindow::aRenamedSentEntryKeepsWorking() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Posta inviata", "generated": "sent", "pinned": true } + ] + })"), oneAccountWithSent()); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + QCOMPARE(labels, QStringList{ QStringLiteral("Posta inviata") }); + + auto *button = + window.findChild(QStringLiteral("sentButton")); + QVERIFY2(button, "the generated entry lost its identity when renamed"); + + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + button->click(); + QCOMPARE(queryEdit->text(), config.allSentQuery()); +} + +/// The hardcoded button was hidden entirely when no account configured a sent +/// folder, rather than offering one that always finds nothing. A stored row +/// must behave the same way. +void TestMainWindow::aGeneratedQueryWithNothingToShowIsSkipped() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Sent", "generated": "sent", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + )); + + MainWindow window(config); + + // The guard: the row was built and the other entry did get a button, so a + // missing Sent means it was skipped rather than that nothing was built. + QCOMPARE(savedQueryButtonLabels(window), + QStringList{ QStringLiteral("Inbox") }); + QVERIFY2(!window.findChild(QStringLiteral("sentButton")), + "a generated query with nothing to show must not get a button"); +} + #include "test_mainwindow.moc" -- cgit v1.2.3 From a7872cf56b7f313881f0d5e50d548ffdb5ba68b9 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Thu, 13 Aug 2026 20:07:31 +0200 Subject: feat(queries): edit, pin and delete a saved query from the UI Item 82. Saving a query worked and nothing else did: changing one field meant retyping the whole query under the same name, and deleting one meant editing the file by hand. An action that creates something the UI cannot then change or remove is incomplete, and the user hit it within minutes of the first hand test. Right-clicking a saved query, on its button or its menu entry, now offers Edit, Move to menu / Show as a button, and Delete. Every path funnels through one replaceSavedQuery(), which matches on the name the dialog was OPENED with rather than the one it returns, so a rename replaces the entry instead of leaving the original behind beside a new one, and which merges the stored entry's unknown fields in a single place rather than in three. Delete confirms first: the rule against confirmation dialogs covers tag mutations, which the undo stack can take back, and this writes user config that it cannot. Two cases the item did not anticipate. A generated entry has no query to edit, so the dialog shows its composed query read-only rather than offering a field that changes nothing, and carries `generated` and `flat` through an edit rather than letting it decay into a plain entry holding a snapshot of what it resolved to today. And the overwrite notice had to learn to ignore the entry being edited, since warning that "Inbox" already exists while editing Inbox is noise. This also fixes a defect that predated it and was already reachable from the save path. rebuildSavedQueryRow() called deleteLater() on the old row, which defers destruction to the event loop, so the stale row went on answering findChild() and every lookup after a rebuild reported the state from before the edit. Nothing looked wrong on screen, which is why it surfaced only as three tests failing against a row that had in fact been rebuilt correctly. Five tests, three mutations. Matching on the returned name fails two, never writing the file fails three, and dropping the unknown-field merge fails one. That last one initially proved nothing: it drove UNPIN, which copies the stored entry and so carries `unknown` along by itself, and passed with the merge deleted. It now goes through the edit path with a replacement that has none, which is what the dialog actually returns. Co-Authored-By: Claude Opus 5 --- CHANGELOG.md | 6 + README.md | 6 + .../plans/2026-08-03-post-0.1.0-usability.md | 26 ++- src/mainwindow.cpp | 113 ++++++++++++ src/mainwindow.h | 36 ++++ src/savequerydialog.cpp | 60 ++++++- src/savequerydialog.h | 16 ++ tests/test_mainwindow.cpp | 200 +++++++++++++++++++++ 8 files changed, 455 insertions(+), 8 deletions(-) (limited to 'README.md') diff --git a/CHANGELOG.md b/CHANGELOG.md index b384eda..5b14938 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,12 @@ point at which they are stable. it can now be reordered, renamed, unpinned or deleted like any other entry rather than being a fixed button you did not own. +- **Right-click a saved query** to edit, pin, unpin or delete it. A saved query + could previously be created and never changed: the only route to adjusting one + field was to retype the whole query under the same name, and there was no way + to delete one at all short of editing the file (item 82). Deleting asks first, + since it writes your config and is not undoable. + ### Changed - Saved queries have a **row of their own** beneath the query bar rather than diff --git a/README.md b/README.md index 749d89d..9148923 100644 --- a/README.md +++ b/README.md @@ -244,6 +244,12 @@ sent folder, rather than offering a button that finds nothing. The name is what the button says, so `Important` and `Flagged` can run the same query and differ only in the label. +**Right-click a saved query** (a button, or its entry in the menu) to edit it, +move it between the row and the menu, or delete it. Deleting asks first: it +rewrites this file and there is no undo for it. Editing a generated entry shows +its composed query read-only, since that one is built from your accounts rather +than stored. + **Upgrading from 0.17.0 or earlier.** Saved queries used to live in a `[queries]` section of `qtmaildir.conf`. The first launch after upgrading reads that section, writes `queries.json` from it, and marks every entry pinned so 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 9d19e4d..5e97337 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 @@ -146,7 +146,7 @@ taking that too literally. | 80 | A rule with many conditions squeezes the rule list to one visible row | defect | XS | **done** 2026-08-13 on `rule-builder`, unreleased. Follows item 76 | | 79 | Opening the rules dialog and saving destroys the first rule | defect | XS | **fixed on `rule-builder`** 2026-08-13, unreleased. Shipped in 0.16.0; damaged one real rule, repaired by hand | | 81 | No way to turn a saved query into a tagging rule | workflow | S | open; depends on 23, which builds the dialog, and is excluded from its spec on purpose. Writes to the shared rules file, so it spans this repo and `mailctl` | -| 82 | A saved query cannot be edited, unpinned or deleted from the UI | defect | S | open; found by hand-testing item 23 on 2026-08-13. Saving works, unsaving does not | +| 82 | A saved query cannot be edited, unpinned or deleted from the UI | defect | S | **done** 2026-08-13 on `saved-queries`, unreleased. Right-click offers Edit, Pin/Unpin and Delete | Sizes are rough: XS under an hour, S a sitting, M a session. @@ -618,6 +618,30 @@ queries** entry, offering Edit, Unpin (or Pin) and Delete. **Size: S**, and it should land before the saved-query work is called done. +**Done 2026-08-13.** A context menu on each button and each menu entry, with +Edit, Move to menu / Show as a button, and Delete. Every path goes through one +`replaceSavedQuery()`, matched on the name the dialog was OPENED with, so a +rename replaces rather than duplicating, and merging the stored entry's unknown +fields in one place rather than three. + +Two things the approach above did not anticipate. A GENERATED entry has no +query to edit, so the dialog shows its composed query read-only rather than +offering a field that changes nothing, and carries `generated` and `flat` +through an edit rather than letting it decay into a plain entry holding a +snapshot. And the overwrite notice had to learn to ignore the entry being +edited: warning that "Inbox" already exists while editing Inbox is noise. + +It also exposed a defect that predated it. `rebuildSavedQueryRow()` called +`deleteLater()` on the old row, which defers destruction to the event loop, so +the stale row went on answering `findChild()` and every lookup after a rebuild +saw the state from before the edit. It was already reachable from the save path. +Fixed by reparenting the row out immediately. + +The unknown-fields test initially passed against the merge being deleted: it +drove UNPIN, which copies the stored entry and therefore carries `unknown` +along by itself. It now goes through the edit path with a replacement that has +none, which is what the dialog actually returns. + ## Deferred, unsized, or split out Items noted while triaging but not part of the original list. Same numbering diff --git a/src/mainwindow.cpp b/src/mainwindow.cpp index 62f4751..14e4202 100644 --- a/src/mainwindow.cpp +++ b/src/mainwindow.cpp @@ -1698,6 +1698,7 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) button->setObjectName(QStringLiteral("sentButton")); connect(button, &QPushButton::clicked, this, [this, saved]() { runSavedQuery(saved); }); + addSavedQueryActions(button, saved); box->addWidget(button); } @@ -1718,6 +1719,12 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) QAction *action = menu->addAction(saved.name); connect(action, &QAction::triggered, this, [this, saved]() { runSavedQuery(saved); }); + // A menu entry has no context menu of its own, so its own submenu + // carries the same three actions; an unpinned query would + // otherwise be the one thing that cannot be edited or deleted. + auto *entryMenu = new QMenu(menu); + addSavedQueryActions(entryMenu, saved); + action->setMenu(entryMenu); } menuButton->setMenu(menu); box->addWidget(menuButton); @@ -1733,6 +1740,105 @@ void MainWindow::buildSavedQueryRow(QWidget *parent, QVBoxLayout *layout) row->hide(); } +void MainWindow::addSavedQueryActions(QWidget *target, const SavedQuery &saved) +{ + target->setContextMenuPolicy(Qt::ActionsContextMenu); + + auto *edit = new QAction(tr("Edit..."), target); + edit->setObjectName(QStringLiteral("editQuery")); + connect(edit, &QAction::triggered, this, + [this, saved]() { editSavedQuery(saved); }); + target->addAction(edit); + + auto *pin = new QAction(saved.pinned ? tr("Move to menu") + : tr("Show as a button"), + target); + pin->setObjectName(QStringLiteral("pinQuery")); + connect(pin, &QAction::triggered, this, [this, saved]() { + SavedQuery toggled = saved; + toggled.pinned = !saved.pinned; + replaceSavedQuery(saved.name, toggled); + }); + target->addAction(pin); + + auto *separator = new QAction(target); + separator->setSeparator(true); + target->addAction(separator); + + auto *remove = new QAction(tr("Delete"), target); + remove->setObjectName(QStringLiteral("deleteQuery")); + connect(remove, &QAction::triggered, this, + [this, saved]() { deleteSavedQuery(saved); }); + target->addAction(remove); +} + +void MainWindow::editSavedQuery(const SavedQuery &saved) +{ + SaveQueryDialog dialog(m_config, saved, this); + if (dialog.exec() != QDialog::Accepted) + return; + + // Matched on the name the dialog OPENED with. Using the returned name would + // leave the original entry in place and add a second one under the new + // name, which is a duplicate rather than a rename. + replaceSavedQuery(saved.name, dialog.savedQuery()); +} + +void MainWindow::deleteSavedQuery(const SavedQuery &saved) +{ + // One of the few places in this application that confirms. The rule against + // confirmation dialogs covers tag mutations, which are undoable through the + // undo stack; this writes user config, is not on that stack, and cannot be + // taken back. + if (m_confirmDelete) { + const auto answer = QMessageBox::question( + this, tr("Delete saved query"), + tr("Delete the saved query '%1'?").arg(saved.name), + QMessageBox::Yes | QMessageBox::No, QMessageBox::No); + if (answer != QMessageBox::Yes) + return; + } + + replaceSavedQuery(saved.name, SavedQuery()); +} + +void MainWindow::replaceSavedQuery(const QString &originalName, + const SavedQuery &replacement) +{ + QList queries = m_config.savedQueries(); + const bool removing = replacement.name.isEmpty(); + + for (int i = 0; i < queries.size(); ++i) { + if (queries.at(i).name.compare(originalName, Qt::CaseInsensitive) != 0) + continue; + + if (removing) { + queries.removeAt(i); + } else { + // The unknown fields belong to the STORED entry: a field written by + // a later build survives an edit made here rather than being + // dropped on the next save. + SavedQuery merged = replacement; + merged.unknown = queries.at(i).unknown; + queries[i] = merged; + } + break; + } + + m_config.setSavedQueries(queries); + if (!m_config.saveSavedQueries()) { + QMessageBox::warning(this, tr("Saved queries"), + tr("Could not write the saved queries file.")); + return; + } + + rebuildSavedQueryRow(); + statusBar()->showMessage( + removing ? tr("Deleted saved query '%1'.").arg(originalName) + : tr("Updated saved query '%1'.").arg(replacement.name), + kStatusMessageMs); +} + void MainWindow::runSavedQuery(const SavedQuery &saved) { // Through the dropdown, never by pre-scoping the text: runQuery() applies @@ -1817,6 +1923,13 @@ void MainWindow::rebuildSavedQueryRow() const int index = layout->indexOf(old); layout->removeWidget(old); + // Reparented out NOW, not merely scheduled for deletion. deleteLater() + // defers destruction to the event loop, so the old row goes on answering + // findChild() until it runs, and findChild returns the FIRST match: every + // lookup after a rebuild found the stale row and reported the state from + // before the edit. Nothing visible was wrong, which is why this only + // showed up as three tests failing on a row that had in fact been rebuilt. + old->setParent(nullptr); old->deleteLater(); buildSavedQueryRow(centralWidget(), layout); diff --git a/src/mainwindow.h b/src/mainwindow.h index e6ee322..6e8501a 100644 --- a/src/mainwindow.h +++ b/src/mainwindow.h @@ -125,6 +125,25 @@ public: static void setLocksPathForTesting(const QString &path); static QString locksPath(); + /// Suppresses the delete confirmation. + /// + /// A test seam. Deleting a saved query is destructive and not on the undo + /// stack, so it asks first; a test cannot answer a modal dialog without + /// hanging, and driving one through QTest would assert the dialog rather + /// than the deletion. + void setConfirmDeleteForTesting(bool confirm) { m_confirmDelete = confirm; } + + /// Renames or replaces a stored query, as the edit dialog would on accept. + /// + /// A test seam for the rename path specifically: the dialog is modal, and + /// the property worth asserting is that a rename REPLACES rather than + /// duplicating, which is decided after the dialog returns. + void replaceSavedQueryForTesting(const QString &originalName, + const SavedQuery &replacement) + { + replaceSavedQuery(originalName, replacement); + } + /// How many commands are on the undo stack. /// /// A test seam. The undo QAction is always enabled and checks canUndo() @@ -253,6 +272,21 @@ private: /// Rebuilds the saved-query row in place after the stored list changed. void rebuildSavedQueryRow(); + /// Hangs Edit, Pin/Unpin and Delete on a saved query's button or menu + /// entry. The only route to changing a stored query from the UI. + void addSavedQueryActions(QWidget *target, const SavedQuery &saved); + + /// Replaces the entry named `originalName`, writes the file and rebuilds + /// the row. An empty `replacement.name` deletes it instead. + /// + /// Matched on the ORIGINAL name, not the replacement's: a rename otherwise + /// leaves the old entry in place and adds a second one. + void replaceSavedQuery(const QString &originalName, + const SavedQuery &replacement); + + void editSavedQuery(const SavedQuery &saved); + void deleteSavedQuery(const SavedQuery &saved); + private slots: void runCurrentQuery() { runQuery(FlatResult::No); } @@ -628,6 +662,8 @@ private: QLineEdit *m_queryEdit = nullptr; /// Save query, beside the field. Driven by the save_query action. QToolButton *m_saveQueryButton = nullptr; + /// Whether deleting a saved query asks first. Always true outside tests. + bool m_confirmDelete = true; QueryCompleter *m_queryCompleter = nullptr; /// Its own type, not the QTreeView base. The strip painting and the /// expander column are ThreadListView's, and holding the base here only diff --git a/src/savequerydialog.cpp b/src/savequerydialog.cpp index 1ea97c2..2d09986 100644 --- a/src/savequerydialog.cpp +++ b/src/savequerydialog.cpp @@ -42,19 +42,51 @@ SaveQueryDialog::SaveQueryDialog(const Config &config, const QString &query, : QDialog(parent) , m_config(config) { + SavedQuery initial; + initial.query = query; + initial.account = accountKey; + initial.pinned = true; setWindowTitle(tr("Save query")); + build(initial); +} +SaveQueryDialog::SaveQueryDialog(const Config &config, + const SavedQuery &existing, QWidget *parent) + : QDialog(parent) + , m_config(config) + , m_originalName(existing.name) + , m_generated(existing.generated) + , m_flat(existing.flat) +{ + setWindowTitle(tr("Edit saved query")); + build(existing); +} + +void SaveQueryDialog::build(const SavedQuery &initial) +{ auto *layout = new QVBoxLayout(this); auto *form = new QFormLayout; - m_name = new QLineEdit(this); + m_name = new QLineEdit(initial.name, this); m_name->setObjectName(QStringLiteral("saveQueryName")); m_name->setPlaceholderText(tr("A name for this query")); form->addRow(tr("Name"), m_name); - m_query = new QLineEdit(query, this); + m_query = new QLineEdit(initial.query, this); m_query->setObjectName(QStringLiteral("saveQueryQuery")); - form->addRow(tr("Query"), m_query); + if (initial.isGenerated()) { + // A generated entry has no stored query: it is composed from the + // accounts every time it runs. Shown, so the user can see what it will + // do, but read-only, since editing it would change nothing. + m_query->setText(m_config.resolvedQuery(initial)); + m_query->setReadOnly(true); + m_query->setToolTip(tr("Built from your accounts and not editable. " + "It follows the sent folder each account " + "configures.")); + form->addRow(tr("Query"), m_query); + } else { + form->addRow(tr("Query"), m_query); + } // The scope is stored as an account KEY, so the entries carry the key as // data exactly as the main window's dropdown does. "All accounts" is the @@ -63,15 +95,15 @@ SaveQueryDialog::SaveQueryDialog(const Config &config, const QString &query, m_account = new QComboBox(this); m_account->setObjectName(QStringLiteral("saveQueryAccount")); m_account->addItem(tr("All accounts"), QString()); - for (const Account &account : config.accounts()) + for (const Account &account : m_config.accounts()) m_account->addItem(account.key, account.key); - const int index = m_account->findData(accountKey); + const int index = m_account->findData(initial.account); m_account->setCurrentIndex(index >= 0 ? index : 0); form->addRow(tr("Account"), m_account); m_pinned = new QCheckBox(tr("Show as a button"), this); m_pinned->setObjectName(QStringLiteral("saveQueryPinned")); - m_pinned->setChecked(true); + m_pinned->setChecked(initial.pinned); form->addRow(QString(), m_pinned); layout->addLayout(form); @@ -108,7 +140,14 @@ void SaveQueryDialog::updateOkState() const bool usable = !name.isEmpty() && !m_query->text().trimmed().isEmpty(); m_ok->setEnabled(usable); - if (!name.isEmpty() && namesAnExistingQuery(m_config, name)) { + // Ignores the entry being edited: warning that "Inbox" already exists + // while editing Inbox is noise, and the real case worth catching is a + // rename onto a name something else already holds. + const bool isItsOwnName = + !m_originalName.isEmpty() + && name.compare(m_originalName, Qt::CaseInsensitive) == 0; + if (!name.isEmpty() && !isItsOwnName + && namesAnExistingQuery(m_config, name)) { m_notice->setText( tr("A saved query named '%1' already exists and will be " "replaced.").arg(name)); @@ -124,5 +163,12 @@ SavedQuery SaveQueryDialog::savedQuery() const saved.query = m_query->text().trimmed(); saved.account = m_account->currentData().toString(); saved.pinned = m_pinned->isChecked(); + // Carried through rather than re-derived: an edit must not turn a + // generated entry into a plain one holding a snapshot of what it happened + // to resolve to today. + saved.generated = m_generated; + saved.flat = m_flat; + if (saved.isGenerated()) + saved.query.clear(); return saved; } diff --git a/src/savequerydialog.h b/src/savequerydialog.h index be5a2af..d859254 100644 --- a/src/savequerydialog.h +++ b/src/savequerydialog.h @@ -42,6 +42,11 @@ public: SaveQueryDialog(const Config &config, const QString &query, const QString &accountKey, QWidget *parent = nullptr); + /// Edits an entry that already exists, prefilled from it rather than from + /// the query bar. + SaveQueryDialog(const Config &config, const SavedQuery &existing, + QWidget *parent = nullptr); + /// The query as edited. Only meaningful after exec() returned Accepted. SavedQuery savedQuery() const; @@ -51,8 +56,19 @@ public: static bool namesAnExistingQuery(const Config &config, const QString &name); private: + void build(const SavedQuery &initial); void updateOkState(); + /// The name the dialog was opened on, empty when creating. The caller + /// matches on this rather than on the returned name, so a rename replaces + /// the entry instead of adding a second one beside it. + QString m_originalName; + + /// Set for a generated entry, whose query is composed from the accounts + /// and cannot be edited here. + QString m_generated; + bool m_flat = false; + const Config &m_config; QLineEdit *m_name = nullptr; QLineEdit *m_query = nullptr; diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 883e9a7..d090016 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -203,6 +203,11 @@ private slots: void aStoredGeneratedQueryRunsFlatAndComposed(); void aRenamedSentEntryKeepsWorking(); void aGeneratedQueryWithNothingToShowIsSkipped(); + void aSavedQueryButtonOffersEditUnpinAndDelete(); + void unpinningMovesAQueryToTheMenu(); + void deletingRemovesTheQueryFromTheFile(); + void anEditedQueryKeepsItsUnknownFields(); + void renamingReplacesRatherThanDuplicating(); private: /// Owns the throwaway lock table init() points every test at. A pointer @@ -5775,4 +5780,199 @@ void TestMainWindow::aGeneratedQueryWithNothingToShowIsSkipped() "a generated query with nothing to show must not get a button"); } +/// Reads queries.json back from disk, which is what "it was saved" means. +static QJsonArray storedQueries(const QTemporaryDir &dir) +{ + QFile f(dir.filePath(QStringLiteral("qtmaildir/queries.json"))); + if (!f.open(QIODevice::ReadOnly)) + return {}; + const QJsonObject root = QJsonDocument::fromJson(f.readAll()).object(); + return root.value(QStringLiteral("queries")).toArray(); +} + +static QAction *contextActionNamed(MainWindow &window, QWidget *target, + const QString &objectName) +{ + const QList actions = target->actions(); + for (QAction *action : actions) { + if (action->objectName() == objectName) + return action; + } + Q_UNUSED(window); + return nullptr; +} + +void TestMainWindow::aSavedQueryButtonOffersEditUnpinAndDelete() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild(); + QVERIFY(button); + + // A context menu, so the actions live on the widget itself. + QCOMPARE(button->contextMenuPolicy(), Qt::ActionsContextMenu); + QVERIFY(contextActionNamed(window, button, QStringLiteral("editQuery"))); + QVERIFY(contextActionNamed(window, button, QStringLiteral("pinQuery"))); + QVERIFY(contextActionNamed(window, button, QStringLiteral("deleteQuery"))); +} + +void TestMainWindow::unpinningMovesAQueryToTheMenu() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Other", "query": "tag:other", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + QVERIFY(row); + QCOMPARE(savedQueryButtonLabels(window).size(), 2); + QVERIFY(!window.findChild( + QStringLiteral("savedQueryMenuButton"))); + + auto *button = row->findChild(); + QVERIFY(button); + QAction *pin = contextActionNamed(window, button, QStringLiteral("pinQuery")); + QVERIFY(pin); + pin->trigger(); + + // Off the row, into the menu, and written to the file: an unpin that only + // redrew would come back pinned on the next launch. + QCOMPARE(savedQueryButtonLabels(window), QStringList{ QStringLiteral("Other") }); + auto *menuButton = + window.findChild(QStringLiteral("savedQueryMenuButton")); + QVERIFY(menuButton); + QCOMPARE(menuButton->menu()->actions().size(), 1); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 2); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("Inbox")); + QVERIFY2(!stored.at(0).toObject().contains(QStringLiteral("pinned")), + "the unpin did not reach the file"); +} + +void TestMainWindow::deletingRemovesTheQueryFromTheFile() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Doomed", "query": "tag:doomed", "pinned": true }, + { "name": "Keeper", "query": "tag:keeper", "pinned": true } + ] + })")); + + MainWindow window(config); + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild(); + QVERIFY(button); + QCOMPARE(button->text(), QStringLiteral("Doomed")); + + QAction *del = + contextActionNamed(window, button, QStringLiteral("deleteQuery")); + QVERIFY(del); + // Destructive and not on the undo stack, so it confirms. Suppressed here + // rather than driven through the modal dialog, which would hang the test. + window.setConfirmDeleteForTesting(false); + del->trigger(); + + QCOMPARE(savedQueryButtonLabels(window), + QStringList{ QStringLiteral("Keeper") }); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("Keeper")); +} + +/// A field a later build wrote must survive an edit here, or upgrading and +/// downgrading silently strips config the user set. +void TestMainWindow::anEditedQueryKeepsItsUnknownFields() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true, + "icon": "mail-inbox" } + ] + })")); + + MainWindow window(config); + + // Through the EDIT path, with a replacement carrying no unknown fields of + // its own, which is exactly what SaveQueryDialog returns. Driving this + // through unpin instead proved nothing: unpin copies the stored entry, so + // it carries `unknown` along by itself and the merge is never exercised. + // That version passed with the merge deleted. + SavedQuery edited; + edited.name = QStringLiteral("Inbox"); + edited.query = QStringLiteral("tag:inbox and not tag:muted"); + edited.pinned = true; + QVERIFY(edited.unknown.isEmpty()); + window.replaceSavedQueryForTesting(QStringLiteral("Inbox"), edited); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + const QJsonObject entry = stored.at(0).toObject(); + // The edit landed... + QCOMPARE(entry.value(QStringLiteral("query")).toString(), + QStringLiteral("tag:inbox and not tag:muted")); + // ...and did not take the unknown field down with it. + QCOMPARE(entry.value(QStringLiteral("icon")).toString(), + QStringLiteral("mail-inbox")); +} + +/// Renaming must match on the name the dialog OPENED with. Matching on the +/// returned name leaves the original in place and adds a second entry. +void TestMainWindow::renamingReplacesRatherThanDuplicating() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Old", "query": "tag:old", "pinned": true } + ] + })")); + + MainWindow window(config); + + SavedQuery renamed; + renamed.name = QStringLiteral("New"); + renamed.query = QStringLiteral("tag:old"); + renamed.pinned = true; + window.replaceSavedQueryForTesting(QStringLiteral("Old"), renamed); + + const QJsonArray stored = storedQueries(dir); + QCOMPARE(stored.size(), 1); + QCOMPARE(stored.at(0).toObject().value(QStringLiteral("name")).toString(), + QStringLiteral("New")); + QCOMPARE(savedQueryButtonLabels(window), QStringList{ QStringLiteral("New") }); +} + #include "test_mainwindow.moc" -- cgit v1.2.3