diff options
| -rw-r--r-- | src/tagrulesdialog.cpp | 97 | ||||
| -rw-r--r-- | src/tagrulesdialog.h | 18 | ||||
| -rw-r--r-- | tests/test_tagrules.cpp | 72 |
3 files changed, 179 insertions, 8 deletions
diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp index f7db936..9f4cf6e 100644 --- a/src/tagrulesdialog.cpp +++ b/src/tagrulesdialog.cpp @@ -295,12 +295,23 @@ void TagRulesDialog::onSelectionChanged() const int index = currentIndex(); if (index < 0 || index >= m_working.size()) return; - const TagRule &rule = m_working.at(index); - - // The note's textChanged fires from setPlainText below, which would then - // write the rule just loaded back over the rule now current. Harmless when - // they are the same rule, destructive when the selection is what changed. - const QSignalBlocker blockNote(m_note); + // By value: every setter below can reach applyEditsToCurrentRule(), which + // writes into m_working, and a reference into that list would then be read + // back half overwritten. + const TagRule rule = m_working.at(index); + + // Populating the form emits change signals whose handlers write the form + // back onto the working copy, so the rule just loaded would be written over + // whichever rule is now current. Harmless when they are the same rule, + // destructive when the selection is what changed. m_enabled::toggled and + // the note's textChanged both do this, and the widgets are populated in an + // order where m_enabled fires while m_query still holds the PREVIOUS rule's + // text, which emptied the query of the first rule opened. The reloading + // flag is the existing guard for exactly this, so it covers the whole load, + // including rebuildRows: populating combo boxes and line edits fires + // currentIndexChanged, which runs syncQueryLine. + const bool wasReloading = m_reloading; + m_reloading = true; m_id->setText(rule.id); m_stage->setValue(rule.stage); @@ -309,6 +320,15 @@ void TagRulesDialog::onSelectionChanged() m_remove->setText(rule.remove.join(QStringLiteral(", "))); m_query->setText(rule.query); m_note->setPlainText(rule.note); + + // Parse once, on load, and keep it: Task 10's save path compares against + // this to decide whether the stored string may be left alone. + m_loadedQuery = RuleQuery::parse(rule.query); + + if (m_loadedQuery.parsed) + rebuildRows(m_loadedQuery); + + m_reloading = wasReloading; } void TagRulesDialog::applyEditsToCurrentRule() @@ -405,6 +425,17 @@ void TagRulesDialog::onSave() accept(); } +QString TagRulesDialog::queryLineForTest() const +{ + return m_query->text(); +} + +void TagRulesDialog::selectRuleForTest(int index) +{ + if (index >= 0 && index < m_list->topLevelItemCount()) + m_list->setCurrentItem(m_list->topLevelItem(index)); +} + TagRulesDialog::Row *TagRulesDialog::addRow(bool exclusion) { Row row; @@ -529,7 +560,7 @@ void TagRulesDialog::populateOperators(bool exclusion) } } -void TagRulesDialog::syncQueryLine() +RuleQuery TagRulesDialog::currentQueryFromRows() const { RuleQuery query; query.parsed = true; @@ -552,6 +583,56 @@ void TagRulesDialog::syncQueryLine() RuleTerm::Op(row.op->currentData().toInt()), value}); } + return query; +} + +void TagRulesDialog::rebuildRows(const RuleQuery &query) +{ + while (!m_rows.isEmpty()) + delete m_rows.takeLast().container; + while (!m_exclusionRows.isEmpty()) + delete m_exclusionRows.takeLast().container; + + m_matchAll->setChecked(query.join == RuleQuery::All); + m_matchAny->setChecked(query.join == RuleQuery::Any); + + for (const RuleTerm &term : query.terms) + applyTermToRow(addRow(false), term); + for (const RuleTerm &term : query.exclusions) + applyTermToRow(addRow(true), term); - m_query->setText(query.compile()); + if (m_rows.isEmpty()) + addRow(false); // Always one row to type into. + + updateExclusionsVisibility(); +} + +void TagRulesDialog::applyTermToRow(Row *row, const RuleTerm &term) +{ + const QSignalBlocker blockField(row->field); + const QSignalBlocker blockOp(row->op); + const QSignalBlocker blockValue(row->value); + + const int fieldIndex = row->field->findData(int(term.field)); + if (fieldIndex >= 0) + row->field->setCurrentIndex(fieldIndex); + + // The operator list depends on the field just set, so it must be rebuilt + // before the operator can be found in it. + populateOperators(false); + populateOperators(true); + + const int opIndex = row->op->findData(int(term.op)); + if (opIndex >= 0) + row->op->setCurrentIndex(opIndex); + + row->value->setText(term.value); +} + +void TagRulesDialog::syncQueryLine() +{ + if (m_reloading) + return; + m_query->setText(currentQueryFromRows().compile()); + applyEditsToCurrentRule(); } diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h index b60b095..6676da9 100644 --- a/src/tagrulesdialog.h +++ b/src/tagrulesdialog.h @@ -55,6 +55,15 @@ public: /// handle and notmuch permits one per process. QStringList countQueries() const; + /// Test seams. The builder's state is otherwise reachable only through + /// synthetic clicks on widgets whose geometry the offscreen platform does + /// not guarantee. + int rowCountForTest() const { return m_rows.size(); } + QString queryLineForTest() const; + + /// Selects the rule at `index` as a click on the list would. + void selectRuleForTest(int index); + signals: /// Asks the owner to run countQueries() through the worker. void countsRequested(); @@ -95,6 +104,15 @@ private: void updateExclusionsVisibility(); void syncQueryLine(); + void rebuildRows(const RuleQuery &query); + void applyTermToRow(Row *row, const RuleTerm &term); + RuleQuery currentQueryFromRows() const; + + /// The query as parsed when the current rule was loaded. Task 10's save + /// path compares against this to decide whether the stored string may be + /// left alone. + RuleQuery m_loadedQuery; + QList<Row> m_rows; QList<Row> m_exclusionRows; diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp index 8c7e1a9..fa5c0b5 100644 --- a/tests/test_tagrules.cpp +++ b/tests/test_tagrules.cpp @@ -19,7 +19,9 @@ #include <QTemporaryDir> #include <QtTest> +#include "rulequery.h" #include "tagrules.h" +#include "tagrulesdialog.h" /// The risk in TagRules is the format, not painting: a field silently dropped /// on save mis-tags real mail on the next sync, and does it quietly. These @@ -38,6 +40,8 @@ private slots: void aQueryWithQuotesRoundTrips(); void aMissingFileIsEmptyNotAnError(); void aNewerVersionIsRefused(); + void openingARuleFillsTheBuilderRows(); + void switchingRulesDoesNotLeakRowsBetweenThem(); private: QString writeRules(const QString &json); @@ -241,5 +245,73 @@ void TestTagRules::aNewerVersionIsRefused() QCOMPARE(rules.warnings().size(), 1); } +void TestTagRules::openingARuleFillsTheBuilderRows() +{ + // The dialog reads the shared store from its default path, so point the + // whole process at a temporary one. XDG_CONFIG_HOME is what + // TagRules::defaultPath() honours. + QTemporaryDir configHome; + QVERIFY(configHome.isValid()); + qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8()); + QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules")))); + + QFile out(configHome.filePath(QStringLiteral("mailrules/rules.json"))); + QVERIFY(out.open(QIODevice::WriteOnly)); + out.write(R"({ + "version": 1, + "rules": [ + {"id": "vendor", + "query": "from:vendor.example.org and subject:receipt", + "add": ["vendor"], "stage": 50, "enabled": true} + ] + })"); + out.close(); + + TagRulesDialog dialog; + + QCOMPARE(dialog.rowCountForTest(), 2); + QCOMPARE(dialog.queryLineForTest(), + QStringLiteral("from:vendor.example.org and subject:receipt")); +} + +void TestTagRules::switchingRulesDoesNotLeakRowsBetweenThem() +{ + QTemporaryDir configHome; + QVERIFY(configHome.isValid()); + qputenv("XDG_CONFIG_HOME", configHome.path().toUtf8()); + QVERIFY(QDir().mkpath(configHome.filePath(QStringLiteral("mailrules")))); + + QFile out(configHome.filePath(QStringLiteral("mailrules/rules.json"))); + QVERIFY(out.open(QIODevice::WriteOnly)); + out.write(R"({ + "version": 1, + "rules": [ + {"id": "one", "query": "from:one.example.org", + "add": ["one"], "stage": 50, "enabled": true}, + {"id": "two", + "query": "from:two.example.org or from:three.example.org", + "add": ["two"], "stage": 50, "enabled": true} + ] + })"); + out.close(); + + TagRulesDialog dialog; + + // The first rule is selected on open: one row, joined All by default. + QCOMPARE(dialog.rowCountForTest(), 1); + QCOMPARE(dialog.queryLineForTest(), QStringLiteral("from:one.example.org")); + + dialog.selectRuleForTest(1); + QCOMPARE(dialog.rowCountForTest(), 2); + QCOMPARE(dialog.queryLineForTest(), + QStringLiteral("from:two.example.org or from:three.example.org")); + + // And back, to prove the first rule was not overwritten by loading the + // second. + dialog.selectRuleForTest(0); + QCOMPARE(dialog.rowCountForTest(), 1); + QCOMPARE(dialog.queryLineForTest(), QStringLiteral("from:one.example.org")); +} + QTEST_MAIN(TestTagRules) #include "test_tagrules.moc" |