From 46cdd10e9407dfc605ddc29425c9dac99e47d011 Mon Sep 17 00:00:00 2001 From: "Danilo M." Date: Mon, 17 Aug 2026 13:49:23 +0200 Subject: feat(rules): show each rule's note in the rule list The `note` field explains why a rule is shaped the way it is, and it was reachable only by selecting the rule and reading the editor form, which is the wrong way round for the one field that says what a rule is for. Note is the LAST column, after Matches, at the user's request: a note is prose and the widest thing in the table, so it belongs where it can run on without pushing the narrow columns off screen. That is fiddlier than it looks, because "Matches" is not in the Column enum at all: it is appended past the end at index ColumnCount. Note therefore has to be declared before ColumnCount and still draw after it, and setColumnCount takes a new ColumnTotal rather than ColumnCount + 1. Both columns hold text, so a mix-up puts the counts under Note and looks entirely plausible; the test asserts the counts land under Matches as well as asserting the header order, since the header assertion alone passes with the two swapped. The cell is simplified(), because a note is free text and a newline truncates a tree row at it. The full text is the cell's tooltip and is untouched in the editor. Also fixes a defect found on the way, which is not in the backlog entry. QHeaderView::restoreState REFUSES a state saved with a different column count, returning false and leaving the header untouched, which is what every existing uistate.conf now does. The restore path set m_columnsSized and m_countColumnSized regardless, spending the one auto-size each column gets on a restore that did nothing: the new Note column would have opened at its default width, once, permanently. Now guarded on the return value. Upgrading costs one reset of this dialog's column widths, which is unavoidable, since the saved state genuinely describes a table that no longer exists. Backlog item 102. --- tests/test_tagrules.cpp | 69 +++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) (limited to 'tests') diff --git a/tests/test_tagrules.cpp b/tests/test_tagrules.cpp index 1779245..c831864 100644 --- a/tests/test_tagrules.cpp +++ b/tests/test_tagrules.cpp @@ -57,6 +57,7 @@ private slots: void aNewerVersionIsRefused(); void openingARuleFillsTheBuilderRows(); void switchingRulesDoesNotLeakRowsBetweenThem(); + void theRuleListShowsEachRulesNote(); void openingARuleWithoutEditingLeavesItByteIdentical(); void anUnrepresentableRuleOpensInTextMode(); void editingARowRewritesTheQuery(); @@ -506,6 +507,74 @@ void TestTagRules::switchingRulesDoesNotLeakRowsBetweenThem() QCOMPARE(dialog.queryLineForTest(), QStringLiteral("from:one.example.org")); } +void TestTagRules::theRuleListShowsEachRulesNote() +{ + // Item 102. `note` is the field explaining why a rule is shaped the way it + // is, and it was reachable only by selecting the rule and reading the + // editor form. The LIST is what the item is about, so this asserts on the + // list and never on the form, which was always correct. + 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, + "note": "Only the digest,\nnot the alerts."}, + {"id": "two", "query": "from:two.example.org", + "add": ["two"], "stage": 50, "enabled": true} + ] + })"); + out.close(); + + TagRulesDialog dialog; + + const QStringList headers = dialog.listHeaderLabelsForTest(); + const int noteColumn = headers.indexOf(QStringLiteral("Note")); + QVERIFY2(noteColumn >= 0, + qPrintable(QStringLiteral("no Note column in the rule list: %1") + .arg(headers.join(QStringLiteral(", "))))); + + // Note is LAST, per the user: it is prose and the widest thing here, so it + // runs on at the end rather than pushing the narrow columns off screen. + // + // The trap this guards is that "Matches" is NOT in the Column enum, it is + // appended past the end, so Note has to be declared before ColumnCount and + // still draw after it. Both columns hold text, so a mix-up puts the counts + // under Note and looks perfectly plausible. + QCOMPARE(headers.constLast(), QStringLiteral("Note")); + QCOMPARE(headers.at(headers.size() - 2), QStringLiteral("Matches")); + + // simplified(), because a newline in a tree cell truncates the row at it + // and a note is free text. The full text stays in the tooltip. + QCOMPARE(dialog.listCellTextForTest(0, noteColumn), + QStringLiteral("Only the digest, not the alerts.")); + QCOMPARE(dialog.listCellToolTipForTest(0, noteColumn), + QStringLiteral("Only the digest,\nnot the alerts.")); + + // A rule with no note leaves the cell empty rather than showing anything. + QVERIFY(dialog.listCellTextForTest(1, noteColumn).isEmpty()); + + // And the counts still land under Matches, not under Note. Asserting the + // header order alone would pass with the two columns swapped in the enum, + // since both hold text and nothing else would complain. + const int matchesColumn = headers.indexOf(QStringLiteral("Matches")); + QVERIFY(matchesColumn >= 0); + dialog.setCounts({ 7, 9 }); + QCOMPARE(dialog.listCellTextForTest(0, matchesColumn), QStringLiteral("7")); + QCOMPARE(dialog.listCellTextForTest(1, matchesColumn), QStringLiteral("9")); + + // The note survived the counts arriving: setCounts writes by column index, + // so an off-by-one there would overwrite the note with a number. + QCOMPARE(dialog.listCellTextForTest(0, noteColumn), + QStringLiteral("Only the digest, not the alerts.")); +} + void TestTagRules::openingARuleWithoutEditingLeavesItByteIdentical() { // Recompiling on open would rewrite the shared file for no reason, and -- cgit v1.2.3