From 01ea9e9d7df04dc771430e4c378202b8ef37b8db Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Tue, 25 Aug 2026 09:54:10 +0200 Subject: feat(compose): report autosave state in a status bar Autosave worked and said nothing on success. The only feedback was m_banner, which is the failure channel and whose persistence is load-bearing for the quit path, so success got its own channel rather than sharing one. The fix is a funnel, not a label. m_dirty had seven writers, four of which clear it and only two of those are a save: the constructor clears it because seeding is not an edit, and the send handler clears it because the message is gone. A cue hung off saveDraftNow() would have been silently wrong in both. setDirty() is the only writer now, and it refreshes the status cue and setWindowModified() together so neither display can drift from the flag. The age line needs a tick of its own, since it moves with no edit to drive it. Five seconds against a label that reads in tens of them. Two defects found by probing rather than by reading. The %n plural rendered as "2 minute(s) ago" for every English user, because Qt picks a plural form only when a translation supplies the forms and there is no English .ts; it uses %1 and "min" now, which Italian substitutes identically. And the status mark was inside the translatable string, where a translator could drop it; it is concatenated outside tr(). Presentation reworked after the user looked at it. The first version reused item 151's yellow ribbon treatment, which reads as a misplaced widget on a bare status label rather than as a warning, and put both labels in the permanent widget area, which is the right-hand tray. They are ordinary status text on the left now. onlyTheSetterWritesTheDirtyFlag() asserts the funnel structurally, by reading composewindow.cpp: the first test for the send path called markClean() directly and a mutation restoring a direct assignment left the whole suite green. Four mutations now fail. The suite still cannot see the presentation, which is why that half needed a hand test. lrelease reports 487 finished, 0 unfinished. Closes item 160, and unblocks 161. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UUQS6n3cmsFrsjCNmwNtf8 --- tests/test_composewindow.cpp | 196 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 196 insertions(+) (limited to 'tests/test_composewindow.cpp') diff --git a/tests/test_composewindow.cpp b/tests/test_composewindow.cpp index 472c103..68ec4d4 100644 --- a/tests/test_composewindow.cpp +++ b/tests/test_composewindow.cpp @@ -23,7 +23,10 @@ #include #include #include +#include +#include #include +#include #include #include @@ -49,6 +52,11 @@ private slots: void changingTheAccountStopsFollowingOnceTheSwitchIsUsed(); void aResumedDraftDoesNotReseedOnAnAccountChange(); void savingADraftEmitsItsPathAndTheReplacedOne(); + void anUnsavedEditIsAnnouncedInBothPlaces(); + void aSavedDraftReportsItAndClearsTheCue(); + void aSentMessageLeavesNoUnsavedCue(); + void onlyTheSetterWritesTheDirtyFlag(); + void theAgeLineFollowsTheClock(); private: /// A config pointing at a signatures directory holding \p files, with one @@ -62,6 +70,9 @@ private: /// non-void function. A void helper keeps the check and sidesteps that. void writeFile(const QString &path, const QString &content); + /// A config whose one account has a drafts folder, so a save can write. + Config configWithDrafts(); + QTemporaryDir *m_dir = nullptr; QString m_signatureDir; }; @@ -447,5 +458,190 @@ void TestComposeWindow::savingADraftEmitsItsPathAndTheReplacedOne() QVERIFY2(second != first, "a rewrite reused the old filename"); } +/// A helper for the status-bar tests: a config whose account has a drafts +/// folder, which makeConfig() deliberately does not set. +Config TestComposeWindow::configWithDrafts() +{ + const QString confPath = m_dir->path() + QStringLiteral("/qtmaildir.conf"); + QString conf; + { + QTextStream out(&conf); + out << "[account.work]\n" + << "name = Someone\n" + << "address = someone@example.org\n" + << "maildir = work\n" + << "drafts = Drafts\n" + << "send_command = /bin/cat\n"; + } + writeFile(confPath, conf); + + Config config; + config.load(confPath); + return config; +} + +/// Both cues answer the same question and must agree. The status label is +/// what the user reads while typing; the title marker is what they see when +/// the composer is behind another window. +void TestComposeWindow::anUnsavedEditIsAnnouncedInBothPlaces() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + + // Seeding is not an edit: the constructor clears the flag after filling + // the fields, so a composer nobody has typed into is clean. + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + QVERIFY(unsaved); + QVERIFY2(unsaved->isHidden(), "a freshly opened composer is not dirty"); + QVERIFY2(!window.isWindowModified(), + "a freshly opened composer must not claim unsaved edits"); + + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("Something typed.")); + + QVERIFY2(!unsaved->isHidden(), "the status cue must appear on an edit"); + QVERIFY2(window.isWindowModified(), + "the title marker must appear on an edit"); +} + +/// The gap item 160 exists to close: a successful save said nothing at all. +void TestComposeWindow::aSavedDraftReportsItAndClearsTheCue() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + auto *age = window.findChild(QStringLiteral("draftAge")); + QVERIFY(unsaved); + QVERIFY(age); + + QVERIFY2(age->text().isEmpty(), + "nothing has been saved yet, so there is no age to report"); + + body->setPlainText(QStringLiteral("First revision.")); + QVERIFY(window.saveDraftNow()); + + QVERIFY2(unsaved->isHidden(), "a save must clear the unsaved cue"); + QVERIFY2(!window.isWindowModified(), + "a save must clear the title marker"); + QVERIFY2(!age->text().isEmpty(), "a save must be reported"); +} + +/// The send path clears the flag WITHOUT saving a draft, and it is one of the +/// four sites that write it. A cue hung off the save alone would leave a sent +/// message claiming unsaved edits on the way out. +void TestComposeWindow::aSentMessageLeavesNoUnsavedCue() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("Outgoing.")); + QVERIFY(window.isWindowModified()); + + // What the send handler does on success, without running a real send. + window.markClean(); + + auto *unsaved = window.findChild(QStringLiteral("unsavedCue")); + QVERIFY(unsaved); + QVERIFY2(unsaved->isHidden(), "a sent message has no unsaved edits"); + QVERIFY2(!window.isWindowModified(), + "a sent message must not claim unsaved edits"); +} + +/// The test above drives markClean() directly, which proves what the SETTER +/// does and nothing about whether the send path calls it: a mutation putting +/// `m_dirty = false` back into the send handler left the whole suite green, +/// measured. That is CLAUDE.md's "a probe pointed at the wrong object". +/// +/// The property that actually matters is structural, so it is asserted +/// structurally: m_dirty has ONE writer. Four of the seven sites that used to +/// assign it clear it, and only two of those are a save, so a cue hung off +/// the save path alone silently missed the constructor and the send. +void TestComposeWindow::onlyTheSetterWritesTheDirtyFlag() +{ + QFile source(QStringLiteral(SOURCE_DIR "/src/composewindow.cpp")); + QVERIFY2(source.open(QIODevice::ReadOnly | QIODevice::Text), + qPrintable(source.errorString())); + const QStringList lines = + QString::fromUtf8(source.readAll()).split(QLatin1Char('\n')); + source.close(); + + // A guard against the probe itself rotting: if the member is ever + // renamed, this test must fail rather than quietly verify nothing. + QVERIFY2(lines.join(QLatin1Char('\n')).contains(QStringLiteral("m_dirty")), + "m_dirty is gone; this test needs updating, not deleting"); + + QStringList offenders; + for (int i = 0; i < lines.size(); ++i) { + const QString line = lines.at(i); + // An assignment, not a read: `m_dirty =` but not `m_dirty ==`. + static const QRegularExpression assignment( + QStringLiteral("\\bm_dirty\\s*=[^=]")); + if (!assignment.match(line).hasMatch()) + continue; + // The one legitimate writer. + if (line.contains(QStringLiteral("m_dirty = dirty"))) + continue; + offenders.append(QStringLiteral("%1: %2").arg(i + 1).arg(line.trimmed())); + } + + QVERIFY2(offenders.isEmpty(), + qPrintable(QStringLiteral( + "m_dirty must only be written by setDirty(), or the status " + "cue and the title marker drift from it. Offending lines:\n%1") + .arg(offenders.join(QLatin1Char('\n'))))); +} + +/// The age changes with no edit to drive it, so it needs a tick of its own. +/// Asserting on the TEXT changing rather than on a wording, which is +/// translated and would pin the test to one locale. +void TestComposeWindow::theAgeLineFollowsTheClock() +{ + const Config config = configWithDrafts(); + + ComposeContext context; + context.kind = ComposeContext::Kind::New; + context.accountKey = QStringLiteral("work"); + + ComposeWindow window(context, config, m_dir->path()); + auto *body = window.findChild(QStringLiteral("body")); + QVERIFY(body); + body->setPlainText(QStringLiteral("First revision.")); + QVERIFY(window.saveDraftNow()); + + auto *age = window.findChild(QStringLiteral("draftAge")); + QVERIFY(age); + const QString justSaved = age->text(); + QVERIFY(!justSaved.isEmpty()); + + // Driven rather than waited for: a real wait would put seconds into the + // suite for a label that reads in tens of them. + auto *tick = window.findChild(QStringLiteral("draftAgeTick")); + QVERIFY2(tick, "the age needs a tick of its own; an edit cannot drive it"); + QVERIFY(tick->isActive()); + + window.reportDraftAgeFor(90); + QVERIFY2(age->text() != justSaved, + "the age line must move as the clock does"); +} + QTEST_MAIN(TestComposeWindow) #include "test_composewindow.moc" -- cgit v1.2.3