fix(startup): let startup_query name a built-in filter, and run it

Two defects, both reachable only after item 93.

startupSavedQuery() searched the saved queries alone, so a startup_query of
"Inbox" matched nothing once Inbox shipped as a built-in filter and the
duplicated entry was removed from queries.json. It then fell back to
m_savedQueries.first(), which is an arbitrary choice that used to look
reasonable while every install carried an Inbox entry: with the duplicates gone
it opened on a leftover search for one sender, and an empty queries.json opened
on nothing at all. The search now covers the saved queries first, so the user's
own entry wins a name collision, then the built-in filters; the fallback is the
Unread filter, which is always present.

The default startup name has always been "Unread" and now resolves for the
first time: before this it named nothing unless the user happened to have such
an entry.

The constructor also read startup.query directly, and a generated entry stores
no query at all, so even a matching filter opened an empty bar. It resolves
through Config::resolvedQuery() now, unscoped, since the account dropdown
starts on "All accounts".

Icons per the user's choices: a star for Important rather than the flag
action's own icon, since on the query row an icon reads as a category rather
than as an instruction, and mail-folder-sent rather than mail-sent.

Three tests changed rather than adapted, because their premises were the
defect. Two asserted the first-saved-query fallback.
aCronSyncDoesNotRefreshBeforeAnyQueryHasRun assumed a fresh window had run no
query, which is no longer true; it is now
aCronSyncRefreshesTheLastRunQueryNotTheQueryBar and asserts the property that
actually matters on a cron timer, through a new lastRunQueryForTesting() seam,
since a legitimate refresh bumps the generation and the counter cannot tell the
two apart.

2 files changed, 161 insertions, 11 deletions

diff --git a/tests/test_config.cpp b/tests/test_config.cpp
index e5487b8..0cacb04 100644
--- a/tests/test_config.cpp
+++ b/tests/test_config.cpp
@@ -60,7 +60,7 @@ private slots:
     void jsonWinsOnceItExists();
     void malformedQueriesFileIsAProblemNotACrash();
     void futureVersionIsRefusedAndReported();
-    void startupQueryFallsBackToDocumentOrder();
+    void startupQueryFallsBackToABuiltinFilter();
     void scopedSavedQueryParenthesisesADisjunction();
     void aGeneratedQueryResolvesFromTheAccounts();
     void aGeneratedQueryTracksAConfigChange();
@@ -96,6 +96,9 @@ private slots:
     void allSentQuerySkipsAccountsWithoutTheKey();
     void allSentQueryJoinsEveryConfiguredAccount();
     void aStoredGeneratedQueryIsUnpinnedNotDropped();
+    void theStartupQueryCanNameABuiltinFilter();
+    void theStartupQueryPrefersASavedQueryOverAFilterOfTheSameName();
+    void anUnmatchedStartupQueryFallsBackToAFilterNotAStrayQuery();
     void theFlaggedFilterIsCalledImportant();
     void everyBuiltinFilterIsAKnownGenerator();
     void aFilterAcrossAllAccountsIsTheUnscopedQuery();
@@ -532,7 +535,12 @@ void TestConfig::unknownStartupQueryFallsBackAndReports()
         "[queries]\n"
         "Inbox=tag:inbox\n")));
 
-    QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Inbox"));
+    // The fallback is a BUILT-IN filter, not the first saved query. The old
+    // behaviour looked reasonable while every install carried an Inbox entry
+    // and became "startup opens a search for one sender" once the duplicated
+    // entries were removed.
+    QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Unread"));
+    QVERIFY(config.startupSavedQuery().isGenerated());
     QCOMPARE(config.problems().size(), 1);
 
     // The built-in default naming a query the user never created is NOT a
@@ -543,7 +551,13 @@ void TestConfig::unknownStartupQueryFallsBackAndReports()
         "[queries]\n"
         "Inbox=tag:inbox\n")));
 
-    QCOMPARE(silent.startupSavedQuery().name, QStringLiteral("Inbox"));
+    // And it now RESOLVES rather than falling through. The default has always
+    // been "Unread"; before item 93 that named nothing unless the user happened
+    // to have such an entry, so an install without one silently opened on
+    // whatever came first in the file. The built-in filter of that name is
+    // always there.
+    QCOMPARE(silent.startupSavedQuery().name, QStringLiteral("Unread"));
+    QVERIFY(silent.startupSavedQuery().isGenerated());
     QVERIFY(silent.problems().isEmpty());
 }
 
@@ -1043,6 +1057,31 @@ static QString writeTwoAccounts(const QTemporaryDir &dir)
         "maildir=personal\n"));
 }
 
+void TestConfig::theStartupQueryCanNameABuiltinFilter()
+{
+    // The defect: startup_query searched the SAVED queries only. A user whose
+    // startup view was "Inbox" had that name in queries.json until item 93
+    // shipped Inbox as a built-in filter and the duplicate was removed; the
+    // name then matched nothing and the app started on whatever query happened
+    // to be first in the file.
+    QTemporaryDir dir;
+    Config config;
+    config.load(writeIni(dir, QStringLiteral(
+        "[general]\n"
+        "startup_query=Inbox\n"
+        "\n"
+        "[account.work]\n"
+        "maildir=work\n"
+        "sent=Sent\n")));
+
+    const SavedQuery startup = config.startupSavedQuery();
+    QCOMPARE(startup.name, QStringLiteral("Inbox"));
+    QVERIFY2(startup.isGenerated(),
+             "the startup query matched something other than the built-in");
+    QCOMPARE(config.resolvedQuery(startup, QString()),
+             QStringLiteral("tag:inbox"));
+}
+
 void TestConfig::theFlaggedFilterIsCalledImportant()
 {
     // Item 57 decided this and item 93 contradicted it. The `flag` ACTION has
@@ -1531,6 +1570,60 @@ void TestConfig::aStoredGeneratedQueryIsUnpinnedNotDropped()
     }
 }
 
+void TestConfig::theStartupQueryPrefersASavedQueryOverAFilterOfTheSameName()
+{
+    // The user's own entry wins a name collision. They named it deliberately;
+    // the filter's name is one this application chose for them.
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[general]\n"
+        "startup_query=Inbox\n"
+        "\n"
+        "[account.work]\n"
+        "maildir=work\n"));
+    writeQueries(dir, QStringLiteral(R"({
+        "version": 1,
+        "queries": [ { "name": "Inbox", "query": "tag:inbox and not tag:muted" } ]
+    })"));
+
+    Config config;
+    config.load(path);
+
+    const SavedQuery startup = config.startupSavedQuery();
+    QCOMPARE(startup.name, QStringLiteral("Inbox"));
+    QVERIFY2(!startup.isGenerated(),
+             "the built-in filter shadowed the user's own query of that name");
+    QCOMPARE(startup.query, QStringLiteral("tag:inbox and not tag:muted"));
+}
+
+void TestConfig::anUnmatchedStartupQueryFallsBackToAFilterNotAStrayQuery()
+{
+    // The old fallback was m_savedQueries.first(), which after item 93 removed
+    // the duplicated entries could be any leftover query: the user's startup
+    // view became a search for one sender, and an empty queries.json started
+    // nothing at all. A built-in filter is always present, so the fallback can
+    // be one.
+    QTemporaryDir dir;
+    const QString path = writeIni(dir, QStringLiteral(
+        "[general]\n"
+        "startup_query=NoSuchThing\n"
+        "\n"
+        "[account.work]\n"
+        "maildir=work\n"));
+    writeQueries(dir, QStringLiteral(R"({
+        "version": 1,
+        "queries": [ { "name": "from bu", "query": "from:someone" } ]
+    })"));
+
+    Config config;
+    config.load(path);
+
+    const SavedQuery startup = config.startupSavedQuery();
+    QVERIFY2(startup.isGenerated(),
+             "an unmatched startup query fell back to a stray saved query");
+    QCOMPARE(startup.name, QStringLiteral("Unread"));
+}
+
 void TestConfig::malformedQueriesFileIsAProblemNotACrash()
 {
     QTemporaryDir dir;
@@ -1567,7 +1660,11 @@ void TestConfig::futureVersionIsRefusedAndReported()
 /// The fallback stops meaning "alphabetically first" and starts meaning "first
 /// in the user's own order". "Zebra" first proves it: alphabetical would pick
 /// "Apple".
-void TestConfig::startupQueryFallsBackToDocumentOrder()
+/// Was startupQueryFallsBackToDocumentOrder, asserting the first query in the
+/// file. That WAS the defect: the first entry is an arbitrary thing to open on,
+/// and once item 93's duplicated entries were removed it became a leftover
+/// search for one sender.
+void TestConfig::startupQueryFallsBackToABuiltinFilter()
 {
     QTemporaryDir dir;
     const QString path = writeIni(dir, QStringLiteral(
@@ -1585,7 +1682,10 @@ void TestConfig::startupQueryFallsBackToDocumentOrder()
     Config config;
     config.load(path);
 
-    QCOMPARE(config.startupSavedQuery().name, QStringLiteral("Zebra"));
+    const SavedQuery startup = config.startupSavedQuery();
+    QVERIFY2(startup.isGenerated(),
+             "the fallback picked a saved query out of the file");
+    QCOMPARE(startup.name, QStringLiteral("Unread"));
 }
 
 /// The parentheses are load-bearing. Without them `path:... and a or b` binds
diff --git a/tests/test_mainwindow.cpp b/tests/test_mainwindow.cpp
index 7563365..949b59e 100644
--- a/tests/test_mainwindow.cpp
+++ b/tests/test_mainwindow.cpp
@@ -169,6 +169,7 @@ private slots:
     void narrowingAnEmptyQueryBarIsAPlainSearch();
     void aMalformedAccountIsReportedWithoutBlockingTheConstructor();
     void aWorkerBackedWindowReturnsRealThreads();
+    void aGeneratedStartupQueryActuallyRuns();
     void everyBuiltinFilterButtonCarriesAnIconAndItsText();
     void aQueryInTheMenuCanActuallyBeRun();
     void theFourBuiltinFiltersAreOnTheRowInOrder();
@@ -206,7 +207,7 @@ private slots:
     void aSkippedLocalSyncStillReportsTheOtherRunFinishing();
     void aCronSyncRefreshesTheListWithoutAQuery();
     void aCronSyncRefreshesOverASelectionWithoutClearingIt();
-    void aCronSyncDoesNotRefreshBeforeAnyQueryHasRun();
+    void aCronSyncRefreshesTheLastRunQueryNotTheQueryBar();
     void aRefreshAddsNewMailAndDropsWhatStoppedMatching();
     void theOpenThreadLeavingTheListRaisesTheStaleNotice();
     void aThreadStillMatchingRaisesNoStaleNotice();
@@ -2776,25 +2777,43 @@ void TestMainWindow::aCronSyncRefreshesOverASelectionWithoutClearingIt()
              "one unusable on a cron timer");
 }
 
-void TestMainWindow::aCronSyncDoesNotRefreshBeforeAnyQueryHasRun()
+void TestMainWindow::aCronSyncRefreshesTheLastRunQueryNotTheQueryBar()
 {
     // The query bar holds text the user has typed but not run, and a refresh
     // must not execute it: that is a search they never asked for. The refresh
     // re-runs the LAST RUN query, so with none there is nothing to do.
+    //
+    // Since item 93 a default Config DOES run a query at startup: the default
+    // startup name resolves to the built-in Unread filter, where before it
+    // named nothing and a fresh window had no last-run query at all.
+    //
+    // The property under test survives that, and is the one that matters on a
+    // cron timer: the refresh re-runs the LAST RUN query, never the text
+    // sitting in the bar. So the bar is given something the user has typed and
+    // not run, and the assertion is that what the refresh runs is still the
+    // startup query.
     const Config config;
     MainWindow window(config);
 
     auto *queryEdit = window.findChild<QLineEdit *>();
     QVERIFY(queryEdit);
-    queryEdit->setText(QStringLiteral("tag:draft-i-was-typing"));
 
-    const quint64 before = window.currentGenerationForTesting();
+    const QString ranAtStartup = window.lastRunQueryForTesting();
+    QVERIFY2(!ranAtStartup.isEmpty(),
+             "no startup query ran, so a refresh has nothing to re-run and "
+             "this test cannot distinguish the two sources");
+
+    queryEdit->setText(QStringLiteral("tag:draft-i-was-typing"));
 
     QMetaObject::invokeMethod(&window, "onExternalSyncStateChanged",
                               Q_ARG(SyncMonitor::State,
                                     SyncMonitor::State::Idle));
 
-    QCOMPARE(window.currentGenerationForTesting(), before);
+    QCOMPARE(window.lastRunQueryForTesting(), ranAtStartup);
+    QVERIFY2(!window.lastRunQueryForTesting().contains(
+                 QStringLiteral("draft-i-was-typing")),
+             "the refresh executed the text in the query bar, which is a "
+             "search the user never asked for");
 }
 
 void TestMainWindow::aRefreshAddsNewMailAndDropsWhatStoppedMatching()
@@ -5070,10 +5089,17 @@ namespace {
 /// A config whose accounts carry the given maildir/sent pairs. An empty `sent`
 /// writes no key at all, which is the account-without-a-sent-folder case.
 QString writeSentConfig(const QTemporaryDir &dir,
-                        const QList<QPair<QString, QString>> &accounts)
+                        const QList<QPair<QString, QString>> &accounts,
+                        const QString &startupQuery = QString())
 {
     const QString path = dir.filePath(QStringLiteral("qtmaildir.conf"));
     QSettings s(path, QSettings::IniFormat);
+    // [general] keys are read WITHOUT the prefix: QSettings' INI backend treats
+    // a section literally named [general] as its own fallback section and
+    // strips it, so setValue("general/startup_query") would write a key nothing
+    // reads.
+    if (!startupQuery.isEmpty())
+        s.setValue(QStringLiteral("startup_query"), startupQuery);
     for (const auto &account : accounts) {
         s.beginGroup(QStringLiteral("account.") + account.first);
         s.setValue(QStringLiteral("maildir"), account.first);
@@ -6356,6 +6382,30 @@ void TestMainWindow::aWorkerBackedWindowReturnsRealThreads()
     QTRY_VERIFY_WITH_TIMEOUT(model->rowCount(QModelIndex()) == 1, 15000);
 }
 
+void TestMainWindow::aGeneratedStartupQueryActuallyRuns()
+{
+    // The second half of the startup defect. The constructor read
+    // startup.query directly, and a generated entry stores no query: its text
+    // is composed from the accounts at run time. So even once
+    // startupSavedQuery() could return a built-in filter, the window opened on
+    // an empty bar and ran nothing.
+    QTemporaryDir dir;
+    QVERIFY(dir.isValid());
+    Config config;
+    config.load(writeSentConfig(dir, {
+        {QStringLiteral("work"), QStringLiteral("Sent")},
+    }, QStringLiteral("Sent")));
+
+    MainWindow window(config);
+    auto *queryEdit = window.findChild<QLineEdit *>(QStringLiteral("queryEdit"));
+    QVERIFY(queryEdit);
+
+    // Sent, because it is the filter whose query is composed rather than
+    // constant: a tag filter would pass against code that only handled the
+    // easy half.
+    QCOMPARE(queryEdit->text(), config.allSentQuery());
+}
+
 void TestMainWindow::everyBuiltinFilterButtonCarriesAnIconAndItsText()
 {
     // The filters are part of the application now, so they carry icons like the