From 3474936404df92ac08534c010aed94f97e6fca15 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Sat, 31 Oct 2026 11:58:57 +0100 Subject: fix: keep the rules banner true after Apply, delete and copy The dialog stays open after Apply, so a refusal from a failed attempt and the load warnings about a file Apply has just rewritten both outlived what they described. A successful Apply rereads the file and redraws the banner. Deleting the last rule now disables Apply, and copying or deleting a rule re-checks which rules the sync hook will skip. Co-Authored-By: Claude Opus 5.5 --- src/tagrulesdialog.cpp | 19 +++++++++++ src/tagrulesdialog.h | 7 +++++ tests/test_tagrules.cpp | 83 ++++++++++++++++++++++++++++++++++++++++++++++++- 3 files changed, 108 insertions(+), 1 deletion(-) diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp index 5f503ea..eb3154f 100644 --- a/src/tagrulesdialog.cpp +++ b/src/tagrulesdialog.cpp @@ -662,6 +662,9 @@ void TagRulesDialog::reloadList() if (!m_working.isEmpty()) m_list->setCurrentItem(m_list->topLevelItem(0)); + // Here as well as on selection: with no rule left, nothing is selected + // and onSelectionChanged() never runs to disable it. + updateApplyButton(); } int TagRulesDialog::currentIndex() const @@ -776,7 +779,11 @@ void TagRulesDialog::applyEditsToCurrentRule() fillItem(m_list->topLevelItem(index), rule); updateApplyButton(); + refreshSkipWarning(); +} +void TagRulesDialog::refreshSkipWarning() +{ if (hookSkipWarning() != m_lastSkipWarning) showWarnings(); } @@ -837,6 +844,7 @@ void TagRulesDialog::onCopyRule() m_working.insert(index + 1, copy); reloadList(); m_list->setCurrentItem(m_list->topLevelItem(index + 1)); + refreshSkipWarning(); } void TagRulesDialog::onDeleteRule() @@ -846,6 +854,7 @@ void TagRulesDialog::onDeleteRule() return; m_working.removeAt(index); reloadList(); + refreshSkipWarning(); } QStringList TagRulesDialog::countQueries() const @@ -915,12 +924,22 @@ void TagRulesDialog::onApply() // The dialog stays open; only OK closes it. if (!saveRules()) return; + // The dialog stays open, so the banner must stop describing a file that + // has just been rewritten: rereading it clears the load warnings, and + // showWarnings() replaces any refusal from an earlier attempt. + m_rules.load(); + showWarnings(); const int index = currentIndex(); if (index < 0 || index >= m_working.size()) return; emit applyRequested(m_working.at(index)); } +bool TagRulesDialog::applyEnabledForTest() const +{ + return m_applyButton->isEnabled(); +} + void TagRulesDialog::setArchiveForTest(bool on) { m_archive->setChecked(on); // toggled -> applyEditsToCurrentRule diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h index b43e79a..1c0ae45 100644 --- a/src/tagrulesdialog.h +++ b/src/tagrulesdialog.h @@ -98,6 +98,9 @@ public: void applyForTest() { onApply(); } /// Ticks or clears the archive box, as a click would. void setArchiveForTest(bool on); + bool applyEnabledForTest() const; + void copyRuleForTest() { onCopyRule(); } + void deleteRuleForTest() { onDeleteRule(); } /// Types into the query field and toggles the mode, so the refusal path /// can be reached without a synthetic click. The refusal reports through @@ -242,6 +245,10 @@ private: /// Enables Apply only for a rule worth applying. void updateApplyButton(); + /// Re-renders the banner when the skip sentence changed. Not called from + /// reloadList(), which runs once before the constructor's showWarnings(). + void refreshSkipWarning(); + /// The only way the warning label is written. An empty string hides it. /// One route in so the icon and the red styling cannot drift between the /// load path, the save refusal and the text-mode notice. diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp index 27496ce..9501213 100644 --- a/tests/test_tagrules.cpp +++ b/tests/test_tagrules.cpp @@ -39,6 +39,9 @@ private slots: void applyEmitsNothingWhenTheSaveFails(); void theArchiveBoxMapsToAllowArchive(); void aRuleTheHookWillSkipIsNamedInTheWarning(); + void aSuccessfulApplyClearsStaleBanners(); + void deletingTheLastRuleDisablesApply(); + void copyingAndDeletingRecheckTheSkipWarning(); void allowArchiveRoundTripsAndIsOmittedWhenFalse(); void allowArchiveIsDroppedFromARuleThatKeepsInbox(); void hookWouldSkipMirrorsThePostNewRefusal(); @@ -1665,12 +1668,17 @@ void TestTagRules::aRuleTheHookWillSkipIsNamedInTheWarning() writeDialogRules(home, R"({"version": 1, "rules": [ {"id": "promo", "add": ["promo"], "remove": ["inbox"], "query": "from:a@example.org"}, - {"id": "fine", "add": ["x"], "query": "from:b@example.org"}]})"); + {"id": "fine", "add": ["x"], "query": "from:b@example.org"}, + {"id": "dormant", "add": ["y"], "remove": ["inbox"], "enabled": false, + "query": "from:c@example.org"}]})"); TagRulesDialog dialog; QVERIFY2(dialog.warningTextForTest().contains(QStringLiteral("promo")), qPrintable(dialog.warningTextForTest())); QVERIFY(!dialog.warningTextForTest().contains(QStringLiteral("fine"))); + // The hook never runs a disabled rule, so it skips nothing there. + QVERIFY2(!dialog.warningTextForTest().contains(QStringLiteral("dormant")), + qPrintable(dialog.warningTextForTest())); dialog.selectRuleForTest(0); dialog.setArchiveForTest(true); @@ -1678,5 +1686,78 @@ void TestTagRules::aRuleTheHookWillSkipIsNamedInTheWarning() qPrintable(dialog.warningTextForTest())); } +void TestTagRules::aSuccessfulApplyClearsStaleBanners() +{ + // The dialog stays open after Apply, so a refusal from a failed attempt + // and a load warning about a file Apply has just rewritten would both + // outlive what they describe. + QTemporaryDir home; + QVERIFY(home.isValid()); + qputenv("XDG_CONFIG_HOME", home.path().toUtf8()); + writeDialogRules(home, R"({"version": 1, "rules": [ + {"id": "justeat orders", "add": ["promo"], + "query": "from:a@example.org"}]})"); + + TagRulesDialog dialog; + const QString needsAttention = QStringLiteral("need attention"); + QVERIFY2(dialog.warningTextForTest().contains(needsAttention), + qPrintable(dialog.warningTextForTest())); + + QSignalSpy spy(&dialog, &TagRulesDialog::applyRequested); + dialog.selectRuleForTest(0); + dialog.setTagsForTest(QString()); + dialog.applyForTest(); + QVERIFY(spy.isEmpty()); + const QString refusal = QStringLiteral("cannot be saved"); + QVERIFY2(dialog.warningTextForTest().contains(refusal), + qPrintable(dialog.warningTextForTest())); + + dialog.setTagsForTest(QStringLiteral("promo")); + dialog.applyForTest(); + QCOMPARE(spy.size(), 1); + QVERIFY2(!dialog.warningTextForTest().contains(refusal), + qPrintable(dialog.warningTextForTest())); + QVERIFY2(!dialog.warningTextForTest().contains(needsAttention), + qPrintable(dialog.warningTextForTest())); +} + +void TestTagRules::deletingTheLastRuleDisablesApply() +{ + QTemporaryDir home; + QVERIFY(home.isValid()); + qputenv("XDG_CONFIG_HOME", home.path().toUtf8()); + writeDialogRules(home, R"({"version": 1, "rules": [ + {"id": "promo", "add": ["promo"], "query": "from:a@example.org"}]})"); + + TagRulesDialog dialog; + dialog.selectRuleForTest(0); + QVERIFY(dialog.applyEnabledForTest()); + dialog.deleteRuleForTest(); + QCOMPARE(dialog.ruleCountForTest(), 0); + QVERIFY(!dialog.applyEnabledForTest()); +} + +void TestTagRules::copyingAndDeletingRecheckTheSkipWarning() +{ + QTemporaryDir home; + QVERIFY(home.isValid()); + qputenv("XDG_CONFIG_HOME", home.path().toUtf8()); + writeDialogRules(home, R"({"version": 1, "rules": [ + {"id": "promo", "add": ["promo"], "remove": ["inbox"], + "query": "from:a@example.org"}]})"); + + TagRulesDialog dialog; + dialog.selectRuleForTest(0); + dialog.copyRuleForTest(); // selects the copy + QVERIFY2(dialog.warningTextForTest().contains(QStringLiteral("promo-copy")), + qPrintable(dialog.warningTextForTest())); + + dialog.deleteRuleForTest(); + QVERIFY2(!dialog.warningTextForTest().contains(QStringLiteral("promo-copy")), + qPrintable(dialog.warningTextForTest())); + QVERIFY2(dialog.warningTextForTest().contains(QStringLiteral("promo")), + qPrintable(dialog.warningTextForTest())); +} + QTEST_MAIN(TestTagRules) #include "test_tagrules.moc" -- cgit v1.2.3