diff options
| -rw-r--r-- | src/tagrulesdialog.cpp | 59 | ||||
| -rw-r--r-- | src/tagrulesdialog.h | 12 | ||||
| -rw-r--r-- | tests/test_tagrules.cpp | 69 |
3 files changed, 135 insertions, 5 deletions
diff --git a/src/tagrulesdialog.cpp b/src/tagrulesdialog.cpp index f208527..e037e37 100644 --- a/src/tagrulesdialog.cpp +++ b/src/tagrulesdialog.cpp @@ -45,8 +45,21 @@ namespace { /// Columns of the rule list. +/// +/// Item 102 added ColumnNote, and the user asked for it LAST, after Matches: +/// a note is prose and the widest thing here, so it belongs at the end where +/// it can run on without pushing the narrow columns off screen. +/// +/// "Matches" is the counts column and is the one that is NOT in this enum: it +/// sits at index ColumnCount, appended past the end. That is why Note is +/// declared before ColumnCount and still draws after Matches, and why +/// ColumnNote must be given its index explicitly rather than left to follow +/// ColumnTags. Get this wrong and the counts land in the Note column, which +/// no test would call an error since both hold text. enum Column { ColumnEnabled = 0, ColumnStage, ColumnId, ColumnTags, - ColumnCount }; + ColumnCount, ///< "Matches", filled by the preview + ColumnNote, ///< last, per the user + ColumnTotal }; QStringList splitTags(const QString &text) { @@ -112,9 +125,11 @@ TagRulesDialog::TagRulesDialog(const TagRule &seed, QWidget *parent) layout->addWidget(intro); m_list = new QTreeWidget(this); - m_list->setColumnCount(ColumnCount + 1); + // ColumnTotal, not ColumnCount + 1: the counts column used to be the one + // past the end, and since item 102 put Note after it there are two. + m_list->setColumnCount(ColumnTotal); m_list->setHeaderLabels({ tr("On"), tr("Stage"), tr("Rule"), tr("Tags"), - tr("Matches") }); + tr("Matches"), tr("Note") }); m_list->setRootIsDecorated(false); m_list->setUniformRowHeights(true); @@ -390,8 +405,14 @@ void TagRulesDialog::restoreUiState() const QByteArray header = state.value(QStringLiteral("tagrules/header")).toByteArray(); - if (!header.isEmpty()) { - m_list->header()->restoreState(header); + // Only when the restore actually took. QHeaderView::restoreState REFUSES a + // state saved with a different column count, returning false and leaving + // the header untouched, which is exactly what every existing state file + // does now that item 102 added a column. Setting the sized flags anyway + // would spend the one auto-size each column gets on a restore that did + // nothing, and the new Note column would open at whatever width it + // defaulted to, once, for good. + if (!header.isEmpty() && m_list->header()->restoreState(header)) { // Counts as the one auto-size each column gets, so the restore is not // immediately overwritten. reloadList() runs before this in the // constructor and has already sized the first two; the count column @@ -482,6 +503,26 @@ void TagRulesDialog::setColumnWidthForTest(int column, int width) m_list->setColumnWidth(column, width); } +QString TagRulesDialog::listCellTextForTest(int row, int column) const +{ + const QTreeWidgetItem *item = m_list->topLevelItem(row); + return item ? item->text(column) : QString(); +} + +QString TagRulesDialog::listCellToolTipForTest(int row, int column) const +{ + const QTreeWidgetItem *item = m_list->topLevelItem(row); + return item ? item->toolTip(column) : QString(); +} + +QStringList TagRulesDialog::listHeaderLabelsForTest() const +{ + QStringList labels; + for (int column = 0; column < m_list->columnCount(); ++column) + labels.append(m_list->headerItem()->text(column)); + return labels; +} + void TagRulesDialog::reloadListForTest() { reloadList(); @@ -531,6 +572,14 @@ void TagRulesDialog::fillItem(QTreeWidgetItem *item, const TagRule &rule) const for (const QString &tag : rule.remove) tags.append(QStringLiteral("-") + tag); item->setText(ColumnTags, tags.join(QStringLiteral(" "))); + + // Item 102. The note explains why a rule is shaped the way it is, and was + // reachable only by selecting the rule and reading the form. simplified() + // because a note is free text and a newline in a tree cell truncates the + // row at it; the full text stays in the tooltip and in the editor. + const QString note = rule.note.simplified(); + item->setText(ColumnNote, note); + item->setToolTip(ColumnNote, rule.note); } void TagRulesDialog::reloadList() diff --git a/src/tagrulesdialog.h b/src/tagrulesdialog.h index 1001745..3bfb3d5 100644 --- a/src/tagrulesdialog.h +++ b/src/tagrulesdialog.h @@ -143,6 +143,18 @@ public: int columnWidthForTest(int column) const; void setColumnWidthForTest(int column, int width); + /// The text and tooltip of one cell of the rule list, by top-level row. + /// + /// Reads the LIST rather than the editor form, which is the distinction + /// item 102 is about: the note was always in the form and invisible until + /// a rule was selected. + QString listCellTextForTest(int row, int column) const; + QString listCellToolTipForTest(int row, int column) const; + + /// The header labels, so a test can assert where a column landed without + /// hardcoding the enum it is checking. + QStringList listHeaderLabelsForTest() const; + /// Repopulates the rule list, as adding or deleting a rule does. Exposed /// because a restored column width has to survive one of these, not only /// a close and reopen: `resizeColumnToContents` on every reload discarded 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 |