From d7cb5b137461b553dd40c87c6a9c8cebf40f059d Mon Sep 17 00:00:00 2001 From: Agoston Szepessy Date: Sun, 28 Dec 2025 18:36:38 -0800 Subject: [PATCH] Factor out QSortProxyFilterModel implementations Had to delay initialization of these classes in `ReportsWidgetBase` because they caused crashes in the constructor initialization list. This is because the base class hasn't been initialized enough yet. --- src/gui/reports/ProxyModels.h | 72 ++++++++ src/gui/reports/ReportsWidgetBase.cpp | 27 ++- src/gui/reports/ReportsWidgetBase.h | 12 +- src/gui/reports/ReportsWidgetHealthcheck.cpp | 165 +------------------ src/gui/reports/ReportsWidgetHealthcheck.h | 17 +- 5 files changed, 120 insertions(+), 173 deletions(-) create mode 100644 src/gui/reports/ProxyModels.h diff --git a/src/gui/reports/ProxyModels.h b/src/gui/reports/ProxyModels.h new file mode 100644 index 000000000..a76f74f52 --- /dev/null +++ b/src/gui/reports/ProxyModels.h @@ -0,0 +1,72 @@ +/* + * Copyright (C) 2025 KeePassXC Team + * + * This program is free software: you can redistribute it and/or modify + * it under the terms of the GNU General Public License as published by + * the Free Software Foundation, either version 2 or (at your option) + * version 3 of the License. + * + * This program is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + * GNU General Public License for more details. + * + * You should have received a copy of the GNU General Public License + * along with this program. If not, see . + */ + +#ifndef KEEPASSXC_PROXYMODELS_H +#define KEEPASSXC_PROXYMODELS_H + +#include +#include + +enum class SortProxyModelKind +{ + Default = 0, + Hibp, + Healthcheck, +}; + +class HibpReportSortPoxyModel : public QSortFilterProxyModel +{ +public: + HibpReportSortPoxyModel(QObject* parent) + : QSortFilterProxyModel(parent){} + ~HibpReportSortPoxyModel() override = default; + +protected: + bool lessThan(const QModelIndex& left, const QModelIndex& right) const override + { + // Sort count column by user data + if (left.column() == 2) { + return sourceModel()->data(left, Qt::UserRole).toInt() + < sourceModel()->data(right, Qt::UserRole).toInt(); + } + // Otherwise use default sorting + return QSortFilterProxyModel::lessThan(left, right); + } +}; + +class HealthcheckReportSortProxyModel : public QSortFilterProxyModel +{ +public: + HealthcheckReportSortProxyModel(QObject* parent) + : QSortFilterProxyModel(parent){} + ~HealthcheckReportSortProxyModel() override = default; + +protected: + bool lessThan(const QModelIndex& left, const QModelIndex& right) const override + { + // Check if the display data is a number, convert and compare if so + bool ok = false; + int leftInt = sourceModel()->data(left).toString().toInt(&ok); + if (ok) { + return leftInt < sourceModel()->data(right).toString().toInt(); + } + // Otherwise use default sorting + return QSortFilterProxyModel::lessThan(left, right); + } +}; + +#endif // KEEPASSXC_PROXYMODELS_H diff --git a/src/gui/reports/ReportsWidgetBase.cpp b/src/gui/reports/ReportsWidgetBase.cpp index 59ccbd4a0..fd37f67cb 100644 --- a/src/gui/reports/ReportsWidgetBase.cpp +++ b/src/gui/reports/ReportsWidgetBase.cpp @@ -28,17 +28,36 @@ #include #include #include +#include -ReportsWidgetBase::ReportsWidgetBase(QWidget* parent) +ReportsWidgetBase::ReportsWidgetBase(QWidget* parent, SortProxyModelKind proxyModel) : QWidget{parent} + , m_referencesModel(new QStandardItemModel(this)) { + // We have to initialize this here; if we do it in the constructor initializer list, + // the base object isn't setup enough and the constructor for QSortFilterProxyModel + // crashes. + switch (proxyModel) { + case SortProxyModelKind::Default: + m_modelProxy.reset(new QSortFilterProxyModel(this)); + break; + case SortProxyModelKind::Healthcheck: + m_modelProxy.reset(new HealthcheckReportSortProxyModel(this)); + break; + case SortProxyModelKind::Hibp: + m_modelProxy.reset(new HibpReportSortPoxyModel(this)); + break; + } } -void ReportsWidgetBase::customMenuRequestedBase() +ReportsWidgetBase::~ReportsWidgetBase() +{} + +QMenu *ReportsWidgetBase::customMenuRequestedBase() { auto selected = getTableView()->selectionModel()->selectedRows(); if (selected.isEmpty()) { - return; + return nullptr; } // Create the context menu @@ -161,6 +180,8 @@ void ReportsWidgetBase::customMenuRequestedBase() } updateWidget(); }); + + return menu; } QList ReportsWidgetBase::getSelectedEntries() diff --git a/src/gui/reports/ReportsWidgetBase.h b/src/gui/reports/ReportsWidgetBase.h index 5ffa19f0f..3208cf516 100644 --- a/src/gui/reports/ReportsWidgetBase.h +++ b/src/gui/reports/ReportsWidgetBase.h @@ -19,9 +19,9 @@ #define KEEPASSXC_REPORTSWIDGETBASE_H #include -#include #include "gui/entry/EntryModel.h" +#include "gui/reports/ProxyModels.h" class Database; class Entry; @@ -29,6 +29,8 @@ class Group; class PasswordHealth; class QSortFilterProxyModel; class QStandardItemModel; +class QMenu; +class QTableView; /** * @brief The ReportsWidgetBase class implements functionality common across the various @@ -38,15 +40,15 @@ class ReportsWidgetBase : public QWidget { Q_OBJECT public: - explicit ReportsWidgetBase(QWidget* parent = nullptr); + explicit ReportsWidgetBase(QWidget* parent, SortProxyModelKind); + virtual ~ReportsWidgetBase(); protected: - virtual QScopedPointer getTableView() = 0; + virtual QTableView *getTableView() = 0; virtual void updateWidget() = 0; - void customMenuRequestedBase(); + QMenu *customMenuRequestedBase(); public slots: - void customMenuRequested(QPoint); QList getSelectedEntries(); void expireSelectedEntries(); void deleteSelectedEntries(); diff --git a/src/gui/reports/ReportsWidgetHealthcheck.cpp b/src/gui/reports/ReportsWidgetHealthcheck.cpp index ee77eadf4..92edb0e2f 100644 --- a/src/gui/reports/ReportsWidgetHealthcheck.cpp +++ b/src/gui/reports/ReportsWidgetHealthcheck.cpp @@ -26,6 +26,7 @@ #include "gui/Icons.h" #include "gui/MessageBox.h" #include "gui/styles/StateColorPalette.h" +#include "gui/reports/ProxyModels.h" #include #include @@ -138,10 +139,8 @@ Health::Health(QSharedPointer db) } ReportsWidgetHealthcheck::ReportsWidgetHealthcheck(QWidget* parent) - : QWidget(parent) + : ReportsWidgetBase(parent, SortProxyModelKind::Healthcheck) , m_ui(new Ui::ReportsWidgetHealthcheck()) - , m_referencesModel(new QStandardItemModel(this)) - , m_modelProxy(new ReportSortProxyModel(this)) { m_ui->setupUi(this); @@ -324,137 +323,12 @@ void ReportsWidgetHealthcheck::emitEntryActivated(const QModelIndex& index) void ReportsWidgetHealthcheck::customMenuRequested(QPoint pos) { - auto selected = m_ui->healthcheckTableView->selectionModel()->selectedRows(); - if (selected.isEmpty()) { + auto menu = customMenuRequestedBase(); + + if(!menu) { return; } - // Create the context menu - const auto menu = new QMenu(this); - menu->setObjectName("customMenu"); - - // Create the "edit entry" menu item (only if 1 row is selected) - if (selected.size() == 1) { - const auto edit = new QAction(icons()->icon("entry-edit"), tr("Edit Entry…"), this); - edit->setObjectName("contextMenuEditAction"); - menu->addAction(edit); - connect(edit, &QAction::triggered, edit, [this, selected] { - auto row = m_modelProxy->mapToSource(selected[0]).row(); - auto entry = m_rowToEntry[row].second; - emit entryActivated(entry); - }); - } - - // Create the "Expire entry" menu item - const auto expEntry = new QAction(icons()->icon("entry-expire"), tr("Expire Entry(s)…", "", selected.size()), this); - expEntry->setObjectName("contextMenuExpireAction"); - menu->addAction(expEntry); - connect(expEntry, &QAction::triggered, this, &ReportsWidgetHealthcheck::expireSelectedEntries); - - // Create the "delete entry" menu item - const auto delEntry = new QAction(icons()->icon("entry-delete"), tr("Delete Entry(s)…", "", selected.size()), this); - menu->addAction(delEntry); - connect(delEntry, &QAction::triggered, this, &ReportsWidgetHealthcheck::deleteSelectedEntries); - - // Create the "exclude from reports" menu item - const auto excludeAction = new QAction(icons()->icon("reports-exclude"), tr("Exclude Entry(s) from reports"), this); - excludeAction->setObjectName("contextMenuExcludeAction"); - const auto excludeGroupsAction = - new QAction(icons()->icon("reports-exclude"), tr("Exclude Group(s) from reports"), this); - excludeGroupsAction->setObjectName("contextMenuExcludeGroupAction"); - - bool isExcluded = false; - bool isGroupExcluded = false; - - for (auto index : selected) { - auto row = m_modelProxy->mapToSource(index).row(); - auto entry = m_rowToEntry[row].second; - if (entry) { - // If at least one entry is excluded switch to inclusion - if (entry->excludeFromReports() || entry->group()->excludeFromReports()) { - isExcluded = true; - } - if (entry->group()->excludeFromReports()) { - isGroupExcluded = true; - } - - break; - } - } - excludeAction->setCheckable(true); - excludeAction->setChecked(isExcluded); - - excludeGroupsAction->setCheckable(true); - excludeGroupsAction->setChecked(isGroupExcluded); - - menu->addAction(excludeAction); - connect(excludeAction, &QAction::toggled, excludeAction, [this, selected](bool checked) { - QSet groups; - - // If we are including entries (checked is false) but a group is excluded, ask the user if they - // would like to include the rest of the group as well (or keep it excluded). - // If they exclude it, we need to include the whole group, and then exclude - // the entries that aren't selected here. - if (!checked) { - for (const auto index : selected) { - auto row = m_modelProxy->mapToSource(index).row(); - auto entry = m_rowToEntry[row].second; - - if (entry) { - auto* group = entry->group(); - if (group->excludeFromReports() && !groups.contains(group)) { - QString msg = tr("The Group for \"%1\" is excluded. Would you like to include all Entries from " - "there as well?") - .arg(entry->title()); - auto response = MessageBox::question(this, - tr("Include Group?"), - msg, - MessageBox::Yes | MessageBox::No | MessageBox::Cancel, - MessageBox::No); - - if (response == MessageBox::Cancel) { - return; - } else if (response == MessageBox::Yes) { - group->setExcludeFromReports(false); - } else if (response == MessageBox::No) { - // We'll exclude all entries from the group here and then - // include the selected ones below - group->setExcludeFromReports(false); - group->markAllEntriesExcludedFromReports(); - } - - groups.insert(group); - } - } - } - } - - for (auto index : selected) { - auto row = m_modelProxy->mapToSource(index).row(); - auto entry = m_rowToEntry[row].second; - - // If the containing group is excluded but the user wants to include - // this entry, ask if they want to keep the remaining items in the group - // excluded or included - if (entry) { - entry->setExcludeFromReports(checked); - } - } - calculateHealth(); - }); - - menu->addAction(excludeGroupsAction); - connect(excludeGroupsAction, &QAction::toggled, excludeGroupsAction, [this, selected](bool checked) { - for (const auto index : selected) { - auto row = m_modelProxy->mapToSource(index).row(); - auto entry = m_rowToEntry[row].second; - if (entry) { - entry->group()->setExcludeFromReports(checked); - } - } - calculateHealth(); - }); - // Show the context menu menu->popup(m_ui->healthcheckTableView->viewport()->mapToGlobal(pos)); } @@ -464,35 +338,12 @@ void ReportsWidgetHealthcheck::saveSettings() // nothing to do - the tab is passive } -QList ReportsWidgetHealthcheck::getSelectedEntries() +void ReportsWidgetHealthcheck::updateWidget() { - QList selectedEntries; - for (auto index : m_ui->healthcheckTableView->selectionModel()->selectedRows()) { - auto row = m_modelProxy->mapToSource(index).row(); - auto entry = m_rowToEntry[row].second; - if (entry) { - selectedEntries << entry; - } - } - return selectedEntries; -} - -void ReportsWidgetHealthcheck::expireSelectedEntries() -{ - for (auto entry : getSelectedEntries()) { - entry->expireNow(); - } - calculateHealth(); } -void ReportsWidgetHealthcheck::deleteSelectedEntries() +QTableView *ReportsWidgetHealthcheck::getTableView() { - QList selectedEntries = getSelectedEntries(); - bool permanent = !m_db->metadata()->recycleBinEnabled(); - if (GuiTools::confirmDeleteEntries(this, selectedEntries, permanent)) { - GuiTools::deleteEntriesResolveReferences(this, selectedEntries, permanent); - } - - calculateHealth(); + return m_ui->healthcheckTableView; } diff --git a/src/gui/reports/ReportsWidgetHealthcheck.h b/src/gui/reports/ReportsWidgetHealthcheck.h index f5a27d678..cbe0e9d55 100644 --- a/src/gui/reports/ReportsWidgetHealthcheck.h +++ b/src/gui/reports/ReportsWidgetHealthcheck.h @@ -19,6 +19,7 @@ #define KEEPASSXC_REPORTSWIDGETHEALTHCHECK_H #include "gui/entry/EntryModel.h" +#include "gui/reports/ReportsWidgetBase.h" #include class Database; @@ -27,13 +28,14 @@ class Group; class PasswordHealth; class QSortFilterProxyModel; class QStandardItemModel; +class QTableView; namespace Ui { class ReportsWidgetHealthcheck; } -class ReportsWidgetHealthcheck : public QWidget +class ReportsWidgetHealthcheck : public ReportsWidgetBase { Q_OBJECT public: @@ -45,6 +47,8 @@ public: protected: void showEvent(QShowEvent* event) override; + void updateWidget() override; + QTableView *getTableView() override; signals: void entryActivated(Entry*); @@ -54,9 +58,6 @@ public slots: void calculateHealth(); void emitEntryActivated(const QModelIndex& index); void customMenuRequested(QPoint); - QList getSelectedEntries(); - void expireSelectedEntries(); - void deleteSelectedEntries(); private: void addHealthRow(QSharedPointer, Group*, Entry*, bool excluded); @@ -64,10 +65,10 @@ private: QScopedPointer m_ui; bool m_healthCalculated = false; - QScopedPointer m_referencesModel; - QScopedPointer m_modelProxy; - QSharedPointer m_db; - QList> m_rowToEntry; + // QScopedPointer m_referencesModel; + // QScopedPointer m_modelProxy; + // QSharedPointer m_db; + // QList> m_rowToEntry; }; #endif // KEEPASSXC_REPORTSWIDGETHEALTHCHECK_H