Fix Requested Payments History Multiselect

john-moffett · john-moffett · commit 2d7585e5a2e3 · 2022-12-02T16:45:20.000-05:00
The recent payment requests list has somewhat broken functionality. You can only
select contiguous rows. Sorting the list loses the selection data. The context
menu appears when multiple rows are selected despite the actions only affecting
the first in the list. These issues are all corrected.

First, a QSortFilterProxyModel is inserted to take care of sorting instead of sorting
the model manually. This also preserves selection data when sorting. Second, the
selection model is changed to ExtendedSelection to allow for non-contiguous multi-
select. Third, the context menu is disabled when multiple rows are selected, as none
of the menu items operate on multiple selected items.
diff --git a/src/qt/receivecoinsdialog.cpp b/src/qt/receivecoinsdialog.cpp
@@ -2,6 +2,7 @@
 // Distributed under the MIT software license, see the accompanying
 // file COPYING or http://www.opensource.org/licenses/mit-license.php.
 
+#include <reverse_iterator.h>
 #include <wallet/wallet.h>
 
 #include <qt/receivecoinsdialog.h>
@@ -30,6 +31,9 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid
 {
     ui->setupUi(this);
 
+    m_sort_proxy = new QSortFilterProxyModel(this);
+    m_sort_proxy->setSortRole(Qt::UserRole);
+
     if (!_platformStyle->getImagesOnButtons()) {
         ui->clearButton->setIcon(QIcon());
         ui->receiveButton->setIcon(QIcon());
@@ -57,7 +61,7 @@ ReceiveCoinsDialog::ReceiveCoinsDialog(const PlatformStyle *_platformStyle, QWid
     tableView->verticalHeader()->hide();
     tableView->setAlternatingRowColors(true);
     tableView->setSelectionBehavior(QAbstractItemView::SelectRows);
-    tableView->setSelectionMode(QAbstractItemView::ContiguousSelection);
+    tableView->setSelectionMode(QAbstractItemView::ExtendedSelection);
 
     QSettings settings;
     if (!tableView->horizontalHeader()->restoreState(settings.value("RecentRequestsViewHeaderState").toByteArray())) {
@@ -75,12 +79,12 @@ void ReceiveCoinsDialog::setModel(WalletModel *_model)
 
     if(_model && _model->getOptionsModel())
     {
-        _model->getRecentRequestsTableModel()->sort(RecentRequestsTableModel::Date, Qt::DescendingOrder);
         connect(_model->getOptionsModel(), &OptionsModel::displayUnitChanged, this, &ReceiveCoinsDialog::updateDisplayUnit);
         updateDisplayUnit();
 
         QTableView* tableView = ui->recentRequestsView;
-        tableView->setModel(_model->getRecentRequestsTableModel());
+        tableView->setModel(m_sort_proxy);
+        m_sort_proxy->setSourceModel(_model->getRecentRequestsTableModel());
         tableView->sortByColumn(RecentRequestsTableModel::Date, Qt::DescendingOrder);
 
         connect(tableView->selectionModel(),
@@ -192,12 +196,7 @@ void ReceiveCoinsDialog::on_receiveButton_clicked()
 
 void ReceiveCoinsDialog::on_recentRequestsView_doubleClicked(const QModelIndex &index)
 {
-    const RecentRequestsTableModel *submodel = model->getRecentRequestsTableModel();
-    ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this);
-    dialog->setModel(model);
-    dialog->setInfo(submodel->entry(index.row()).recipient);
-    dialog->setAttribute(Qt::WA_DeleteOnClose);
-    dialog->show();
+    ShowReceiveRequestDialogForItem(selectedRow());
 }
 
 void ReceiveCoinsDialog::recentRequestsView_selectionChanged(const QItemSelection &selected, const QItemSelection &deselected)
@@ -210,37 +209,55 @@ void ReceiveCoinsDialog::recentRequestsView_selectionChanged(const QItemSelectio
 
 void ReceiveCoinsDialog::on_showRequestButton_clicked()
 {
-    if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel())
-        return;
-    QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows();
+    QModelIndexList selection = SelectedRows();
 
     for (const QModelIndex& index : selection) {
-        on_recentRequestsView_doubleClicked(index);
+        ShowReceiveRequestDialogForItem(index);
     }
 }
 
 void ReceiveCoinsDialog::on_removeRequestButton_clicked()
 {
-    if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel())
-        return;
-    QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows();
+    QModelIndexList selection = SelectedRows();
     if(selection.empty())
         return;
-    // correct for selection mode ContiguousSelection
-    QModelIndex firstIndex = selection.at(0);
-    model->getRecentRequestsTableModel()->removeRows(firstIndex.row(), selection.length(), firstIndex.parent());
+
+    // Collect row indices in a set (sorted) and pass in reverse order to removeRows
+    // to avoid having to keep track of changed source indices after each removal
+    std::set<int> row_indices;
+    for (const QModelIndex& ind : selection) {
+        row_indices.insert(ind.row());
+    }
+
+    for (auto row_ind : reverse_iterate(row_indices)) {
+        model->getRecentRequestsTableModel()->removeRows(row_ind, 1);
+    }
 }
 
 QModelIndex ReceiveCoinsDialog::selectedRow()
 {
     if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel())
         return QModelIndex();
     QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows();
-    if(selection.empty())
+    // Only allow single-selection in this method
+    if (selection.size() != 1)
         return QModelIndex();
-    // correct for selection mode ContiguousSelection
-    QModelIndex firstIndex = selection.at(0);
-    return firstIndex;
+
+    QModelIndex row_ind = m_sort_proxy->mapToSource(selection.at(0));
+    return row_ind;
+}
+
+QModelIndexList ReceiveCoinsDialog::SelectedRows()
+{
+    if(!model || !model->getRecentRequestsTableModel() || !ui->recentRequestsView->selectionModel())
+        return QModelIndexList();
+    QModelIndexList selection = ui->recentRequestsView->selectionModel()->selectedRows();
+    QModelIndexList source_mapped;
+    for (auto row : selection) {
+        source_mapped.append(m_sort_proxy->mapToSource(row));
+    }
+
+    return source_mapped;
 }
 
 // copy column of selected row to clipboard
@@ -253,6 +270,19 @@ void ReceiveCoinsDialog::copyColumnToClipboard(int column)
     GUIUtil::setClipboard(model->getRecentRequestsTableModel()->index(firstIndex.row(), column).data(Qt::EditRole).toString());
 }
 
+void ReceiveCoinsDialog::ShowReceiveRequestDialogForItem(const QModelIndex& index)
+{
+    if (!index.isValid()) {
+        return;
+    }
+    const RecentRequestsTableModel *submodel = model->getRecentRequestsTableModel();
+    ReceiveRequestDialog *dialog = new ReceiveRequestDialog(this);
+    dialog->setModel(model);
+    dialog->setInfo(submodel->entry(index.row()).recipient);
+    dialog->setAttribute(Qt::WA_DeleteOnClose);
+    dialog->show();
+}
+
 // context menu
 void ReceiveCoinsDialog::showMenu(const QPoint &point)
 {
diff --git a/src/qt/receivecoinsdialog.h b/src/qt/receivecoinsdialog.h
@@ -13,6 +13,7 @@
 #include <QKeyEvent>
 #include <QMenu>
 #include <QPoint>
+#include <QSortFilterProxyModel>
 #include <QVariant>
 
 class PlatformStyle;
@@ -51,15 +52,19 @@ public Q_SLOTS:
 
 private:
     Ui::ReceiveCoinsDialog *ui;
+    QSortFilterProxyModel* m_sort_proxy;
     WalletModel *model;
     QMenu *contextMenu;
     QAction* copyLabelAction;
     QAction* copyMessageAction;
     QAction* copyAmountAction;
     const PlatformStyle *platformStyle;
 
+    // Returns QModelIndex in source model
     QModelIndex selectedRow();
+    QModelIndexList SelectedRows();
     void copyColumnToClipboard(int column);
+    void ShowReceiveRequestDialogForItem(const QModelIndex& index);
 
 private Q_SLOTS:
     void on_receiveButton_clicked();
diff --git a/src/qt/recentrequeststablemodel.cpp b/src/qt/recentrequeststablemodel.cpp
@@ -90,11 +90,23 @@ QVariant RecentRequestsTableModel::data(const QModelIndex &index, int role) cons
             else
                 return BitcoinUnits::format(walletModel->getOptionsModel()->getDisplayUnit(), rec->recipient.amount);
         }
-    }
-    else if (role == Qt::TextAlignmentRole)
-    {
+    } else if (role == Qt::TextAlignmentRole) {
         if (index.column() == Amount)
-            return (int)(Qt::AlignRight|Qt::AlignVCenter);
+            return (int)(Qt::AlignRight | Qt::AlignVCenter);
+    } else if (role == Qt::UserRole) {
+        const RecentRequestEntry* rec = &list[index.row()];
+        switch (index.column()) {
+        case Date:
+            return rec->date;
+        case Label:
+            return rec->recipient.label;
+        case Message:
+            return rec->recipient.message;
+        case Amount:
+            return rec->recipient.amount;
+        default:
+            return QVariant();
+        }
     }
     return QVariant();
 }
@@ -210,35 +222,7 @@ void RecentRequestsTableModel::addNewRequest(RecentRequestEntry &recipient)
     endInsertRows();
 }
 
-void RecentRequestsTableModel::sort(int column, Qt::SortOrder order)
-{
-    std::sort(list.begin(), list.end(), RecentRequestEntryLessThan(column, order));
-    Q_EMIT dataChanged(index(0, 0, QModelIndex()), index(list.size() - 1, NUMBER_OF_COLUMNS - 1, QModelIndex()));
-}
-
 void RecentRequestsTableModel::updateDisplayUnit()
 {
     updateAmountColumnTitle();
 }
-
-bool RecentRequestEntryLessThan::operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const
-{
-    const RecentRequestEntry* pLeft = &left;
-    const RecentRequestEntry* pRight = &right;
-    if (order == Qt::DescendingOrder)
-        std::swap(pLeft, pRight);
-
-    switch(column)
-    {
-    case RecentRequestsTableModel::Date:
-        return pLeft->date.toSecsSinceEpoch() < pRight->date.toSecsSinceEpoch();
-    case RecentRequestsTableModel::Label:
-        return pLeft->recipient.label < pRight->recipient.label;
-    case RecentRequestsTableModel::Message:
-        return pLeft->recipient.message < pRight->recipient.message;
-    case RecentRequestsTableModel::Amount:
-        return pLeft->recipient.amount < pRight->recipient.amount;
-    default:
-        return pLeft->id < pRight->id;
-    }
-}
diff --git a/src/qt/recentrequeststablemodel.h b/src/qt/recentrequeststablemodel.h
@@ -34,18 +34,6 @@ class RecentRequestEntry
     }
 };
 
-class RecentRequestEntryLessThan
-{
-public:
-    RecentRequestEntryLessThan(int nColumn, Qt::SortOrder fOrder):
-        column(nColumn), order(fOrder) {}
-    bool operator()(const RecentRequestEntry& left, const RecentRequestEntry& right) const;
-
-private:
-    int column;
-    Qt::SortOrder order;
-};
-
 /** Model for list of recently generated payment requests / bitcoin: URIs.
  * Part of wallet model.
  */
@@ -75,7 +63,6 @@ class RecentRequestsTableModel: public QAbstractTableModel
     QModelIndex index(int row, int column, const QModelIndex &parent = QModelIndex()) const override;
     bool removeRows(int row, int count, const QModelIndex &parent = QModelIndex()) override;
     Qt::ItemFlags flags(const QModelIndex &index) const override;
-    void sort(int column, Qt::SortOrder order = Qt::AscendingOrder) override;
     /*@}*/
 
     const RecentRequestEntry &entry(int row) const { return list[row]; }
