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 --- tests/test_mainwindow.cpp | 275 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 275 insertions(+) (limited to 'tests/test_mainwindow.cpp') diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp index 4c1c5d9..2c70e0f 100644 --- a/tests/test_mainwindow.cpp +++ b/tests/test_mainwindow.cpp @@ -188,6 +188,14 @@ private slots: void flatModeDoesNotSurviveTheNextQuery(); void noTwoActionsShareAnIcon(); + void onlyPinnedQueriesBecomeButtons(); + void unpinnedQueriesReachTheMenu(); + void pinnedButtonsFollowTheDocumentOrder(); + void theSavedQueryMenuIsHiddenWhenEveryQueryIsPinned(); + void aScopedSavedQuerySelectsItsAccount(); + void anUnscopedSavedQueryClearsTheAccount(); + void theSaveQueryActionIsDisabledOnAnEmptyQuery(); + private: /// Owns the throwaway lock table init() points every test at. A pointer /// rather than a value because it is rebuilt per test, and QTemporaryDir @@ -5265,4 +5273,271 @@ int main(int argc, char *argv[]) return QTest::qExec(&test, argc, argv); } +// --------------------------------------------------------------------------- +// Saved queries in the query row (item 23) +// --------------------------------------------------------------------------- + +/// Writes a config plus a queries.json beside it, and loads both. +static void loadWithQueries(Config &config, QTemporaryDir &dir, + const QString &queriesJson, + const QString &iniExtra = {}) +{ + QDir().mkpath(dir.filePath(QStringLiteral("qtmaildir"))); + const QString conf = + dir.filePath(QStringLiteral("qtmaildir/qtmaildir.conf")); + QFile ini(conf); + ini.open(QIODevice::WriteOnly | QIODevice::Text); + ini.write(iniExtra.toUtf8()); + ini.close(); + + QFile json(dir.filePath(QStringLiteral("qtmaildir/queries.json"))); + json.open(QIODevice::WriteOnly); + json.write(queriesJson.toUtf8()); + json.close(); + + config.load(conf); +} + +/// Buttons in the saved-query row, by label, in the order they are laid out. +static QStringList savedQueryButtonLabels(MainWindow &window) +{ + QStringList labels; + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + if (!row) + return labels; + const QList buttons = + row->findChildren(QString(), Qt::FindDirectChildrenOnly); + for (QPushButton *button : buttons) { + // The menu button is not a saved query and must not be counted as one. + if (button->objectName() != QStringLiteral("savedQueryMenuButton")) + labels.append(button->text()); + } + return labels; +} + +void TestMainWindow::onlyPinnedQueriesBecomeButtons() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Buried", "query": "tag:buried" } + ] + })")); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + + QVERIFY2(labels.contains(QStringLiteral("Inbox")), + "a pinned query must have a button"); + QVERIFY2(!labels.contains(QStringLiteral("Buried")), + "an unpinned query must NOT have a button"); +} + +void TestMainWindow::unpinnedQueriesReachTheMenu() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Inbox", "query": "tag:inbox", "pinned": true }, + { "name": "Buried", "query": "tag:buried" } + ] + })")); + + MainWindow window(config); + auto *menuButton = + window.findChild(QStringLiteral("savedQueryMenuButton")); + QVERIFY2(menuButton, "an unpinned query needs a menu to live in"); + QVERIFY(menuButton->menu()); + + QStringList entries; + const QList actions = menuButton->menu()->actions(); + for (QAction *action : actions) + entries.append(action->text()); + + QVERIFY2(entries.contains(QStringLiteral("Buried")), + "the unpinned query is missing from the menu"); + // A pinned query is already a button; listing it twice is the duplicate + // this asserts against. + QVERIFY2(!entries.contains(QStringLiteral("Inbox")), + "a pinned query must not also appear in the menu"); +} + +/// The property the whole storage change was made for. "Zebra" is written +/// first and must stay first; alphabetical order would put it last. +void TestMainWindow::pinnedButtonsFollowTheDocumentOrder() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Zebra", "query": "tag:zebra", "pinned": true }, + { "name": "Apple", "query": "tag:apple", "pinned": true } + ] + })")); + + MainWindow window(config); + const QStringList labels = savedQueryButtonLabels(window); + + QCOMPARE(labels.size(), 2); + QCOMPARE(labels.at(0), QStringLiteral("Zebra")); + QCOMPARE(labels.at(1), QStringLiteral("Apple")); +} + +void TestMainWindow::theSavedQueryMenuIsHiddenWhenEveryQueryIsPinned() +{ + 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); + + // The guard the assertion below needs. Asserting only that the menu button + // is absent passed against NO implementation at all, before any of this + // was built, so it has to prove first that the row it is looking in was + // populated and that a button was found. + const QStringList labels = savedQueryButtonLabels(window); + QCOMPARE(labels, QStringList{ QStringLiteral("Inbox") }); + + auto *menuButton = + window.findChild(QStringLiteral("savedQueryMenuButton")); + QVERIFY2(!menuButton, + "an empty menu button is a control that always does nothing"); +} + +/// The scope goes through the account dropdown rather than being baked into +/// the query text. runQuery() already scopes by that dropdown, so pre-scoping +/// the text would apply the path twice, and the selection would be invisible. +void TestMainWindow::aScopedSavedQuerySelectsItsAccount() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Billing", "query": "from:billing", + "account": "work", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + "\n" + "[account.personal]\n" + "name=Test User\n" + "address=me@example.net\n" + "maildir=personal\n" + )); + + MainWindow window(config); + auto *accountBox = + window.findChild(QStringLiteral("accountBox")); + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY(accountBox); + QVERIFY(queryEdit); + + // Start somewhere else, so a passing result cannot be the default. + accountBox->setCurrentIndex(accountBox->findData( + QStringLiteral("personal"))); + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("personal")); + + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild(); + QVERIFY(button); + button->click(); + + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("work")); + // The text is the bare query. The path scope is applied once, by + // runQuery(), from the dropdown this just set. + QCOMPARE(queryEdit->text(), QStringLiteral("from:billing")); + QVERIFY2(!queryEdit->text().contains(QStringLiteral("path:")), + "the scope must not be baked into the query text"); +} + +/// A query with no account must CLEAR the dropdown, not inherit whatever the +/// last one left there. Confirmed against the same defect in the rules +/// preview, where an already-selected account survived the click. +void TestMainWindow::anUnscopedSavedQueryClearsTheAccount() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, + "queries": [ + { "name": "Everywhere", "query": "tag:inbox", "pinned": true } + ] + })"), QStringLiteral( + "[account.work]\n" + "name=Test User\n" + "address=user@example.org\n" + "maildir=work-mail\n" + )); + + MainWindow window(config); + auto *accountBox = + window.findChild(QStringLiteral("accountBox")); + QVERIFY(accountBox); + + accountBox->setCurrentIndex(accountBox->findData(QStringLiteral("work"))); + QCOMPARE(accountBox->currentData().toString(), QStringLiteral("work")); + + auto *row = window.findChild(QStringLiteral("savedQueryRow")); + QVERIFY(row); + auto *button = row->findChild(); + QVERIFY(button); + button->click(); + + QVERIFY2(accountBox->currentData().toString().isEmpty(), + "an unscoped saved query must clear the account selection"); +} + +void TestMainWindow::theSaveQueryActionIsDisabledOnAnEmptyQuery() +{ + QTemporaryDir dir; + QVERIFY(dir.isValid()); + Config config; + loadWithQueries(config, dir, QStringLiteral(R"({ + "version": 1, "queries": [] + })")); + + MainWindow window(config); + auto *save = window.findChild(QStringLiteral("save_query")); + QVERIFY2(save, "there is no way to save a query"); + + auto *queryEdit = + window.findChild(QStringLiteral("queryEdit")); + QVERIFY(queryEdit); + + queryEdit->clear(); + QVERIFY2(!save->isEnabled(), + "saving an empty query would store a query that matches nothing"); + + queryEdit->setText(QStringLiteral("tag:inbox")); + QVERIFY2(save->isEnabled(), "a real query must be savable"); + + // Whitespace is not a query. setText does not drive a completer, but it + // does emit textChanged, which is what the enabling is hung on. + queryEdit->setText(QStringLiteral(" ")); + QVERIFY(!save->isEnabled()); +} + #include "test_mainwindow.moc" -- cgit v1.2.3