fix: re-ordering list of filters to put Masternode between "Mined" and "Platform transfer" #7612
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -118,16 +118,16 @@ class TransactionTypeSettingRestorer | |
| { | ||
| public: | ||
| TransactionTypeSettingRestorer() : | ||
| m_had_value(m_settings.contains("transactionTypeFilter")), | ||
| m_value(m_settings.value("transactionTypeFilter")) | ||
| { | ||
| } | ||
| ~TransactionTypeSettingRestorer() | ||
| { | ||
| if (m_had_value) { | ||
| m_settings.setValue("transactionTypeFilter", m_value); | ||
| } else { | ||
| m_settings.remove("transactionTypeFilter"); | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -158,55 +158,38 @@ void CheckProviderRecords(const TransactionTableModel& model, const std::vector< | |
| QVERIFY(tooltip.contains(record.label)); | ||
| QVERIFY(tooltip.contains(record.tooltip_text)); | ||
| QVERIFY(!tooltip.contains("Payment to yourself")); | ||
| } | ||
| } | ||
|
|
||
| } // namespace | ||
|
|
||
| void ProviderTransactionTests::transactionTypeSettingPersistence_data() | ||
| { | ||
| QTest::addColumn<quint32>("saved_filter"); | ||
| QTest::addColumn<QString>("expected_text"); | ||
|
|
||
| QTest::newRow("masternode") << (TransactionFilterProxy::TYPE(TransactionRecord::MasternodeRegistration) | | ||
| TransactionFilterProxy::TYPE(TransactionRecord::MasternodeUpdate)) | ||
| << QString{"Masternode"}; | ||
| QTest::newRow("data") << TransactionFilterProxy::TYPE(TransactionRecord::DataTransaction) | ||
| << QString{"Data Transaction"}; | ||
| // An unknown stored filter selects nothing instead of an arbitrary entry. | ||
| QTest::newRow("unknown") << quint32{0} << QString{}; | ||
| } | ||
|
|
||
| void ProviderTransactionTests::transactionTypeSettingPersistence() | ||
| { | ||
| QFETCH(quint32, saved_filter); | ||
| QFETCH(QString, expected_text); | ||
|
|
||
| TransactionTypeSettingRestorer setting_restorer; | ||
| QSettings{}.setValue("transactionTypeFilter", saved_filter); | ||
|
|
||
| TransactionView transaction_view; | ||
| QComboBox* const type_widget{FindTransactionTypeWidget(transaction_view)}; | ||
| QVERIFY(type_widget != nullptr); | ||
| QCOMPARE(type_widget->currentText(), expected_text); | ||
| QCOMPARE(type_widget->currentData().toUInt(), saved_filter); | ||
| } | ||
|
|
||
| void ProviderTransactionTests::providerTransactionHistory() | ||
|
|
@@ -301,13 +284,6 @@ void ProviderTransactionTests::providerTransactionHistory() | |
|
|
||
| // Transactions loaded before the model is constructed exercise the wallet-restart path. | ||
| CheckProviderRecords(*model, expected); | ||
|
|
||
| const std::vector<int> other_rows{FindTransactionRows(*model, other_special_tx->GetHash())}; | ||
| QCOMPARE(other_rows.size(), size_t{1}); | ||
|
|
@@ -355,20 +331,22 @@ void ProviderTransactionTests::providerTransactionHistory() | |
|
|
||
| QListView* const type_list{qobject_cast<QListView*>(type_widget->view())}; | ||
| QVERIFY(type_list != nullptr); | ||
| // Match CoinJoin entries by title so filters added later are covered without | ||
| // duplicating the implementation's filter list. | ||
| int hidden_coinjoin_rows{0}; | ||
| for (int row{0}; row < type_widget->count(); ++row) { | ||
| const QString title{type_widget->itemText(row)}; | ||
| if (title.contains("coinjoin", Qt::CaseInsensitive) || title.contains("coin join", Qt::CaseInsensitive)) { | ||
| QVERIFY(type_list->isRowHidden(row)); | ||
| ++hidden_coinjoin_rows; | ||
| } | ||
| } | ||
| QVERIFY(hidden_coinjoin_rows > 0); | ||
| QVERIFY(!type_list->isRowHidden(masternode_row)); | ||
|
|
||
| type_widget->setCurrentIndex(masternode_row); | ||
| transaction_view.chooseType(masternode_row); | ||
| QCOMPARE(QSettings{}.value("transactionTypeFilter").toUInt(), masternode_filter); | ||
|
Comment on lines
345
to
+349
There was a problem hiding this comment. 📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win Keep coverage for CoinJoin row visibility.
As per coding guidelines, choose targeted C++ unit tests for changed behavior. 🤖 Prompt for AI AgentsSource: Coding guidelines There was a problem hiding this comment. The current head restores disabled-state coverage with a title-based loop, so that part of the comment is now addressed. The enabled-state and selection-transition assertions are still missing: the test should verify that enabling unhides the CoinJoin rows, visible selections survive either transition, and a selected CoinJoin row falls back only when disabling hides it. There was a problem hiding this comment. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
echo '== TransactionView structure =='
ast-grep outline src/qt/transactionview.cpp --items all --match 'updateCoinJoinVisibility|chooseType|setModel' --view expanded || true
echo '== Implementation and call sites =='
rg -n -A35 -B12 --glob 'src/qt/transactionview.{cpp,h}' \
'\b(updateCoinJoinVisibility|chooseType|setModel)\s*\(' src/qt
echo '== CoinJoin visibility test coverage =='
rg -n -A30 -B12 --glob 'src/qt/test/*.{cpp,h}' \
'(CoinJoin|coinjoin|transactionTypeFilter|chooseType)\s*\(' src/qt/test
echo '== Current branch and relevant diff =='
git status --short
git diff -- src/qt/transactionview.cpp src/qt/test/providertransactiontests.cppLength of output: 30488
The title-based loop covers the disabled state. The test does not cover the enabled state.
The finding remains open. ✏️ Learnings added
You are interacting with an AI system.
Comment on lines
332
to
+349
There was a problem hiding this comment. 🟡 Suggestion: Restore CoinJoin visibility and selection coverage This PR changes source: ['codex', 'coderabbit'] |
||
| QTableView* const table{transaction_view.findChild<QTableView*>("transactionView")}; | ||
| QVERIFY(table != nullptr); | ||
| QCOMPARE(table->model()->rowCount(), static_cast<int>(expected.size())); | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🟡 Suggestion: Retain provider transaction detail-summary coverage
CheckProviderRecords()now stops after checking the tooltip, removing all coverage ofLongDescriptionRole. The remaining type, amount, and tooltip checks do not exercise the separate transaction-description rendering path atTransactionTableModel::LongDescriptionRole, and the removed assertions guarded the recently added behavior that provider summaries omit misleading From, To, Debit, Credit, output-index, and external-destination details. No other Qt test requestsLongDescriptionRole, so regressions in that user-facing summary would now pass. Restore focused assertions for those behavioral guarantees while avoiding checks for incidental wording or markup where possible.source: ['codex']