From 70ba75519d66243b6fef6d452e9e158f7d3ea438 Mon Sep 17 00:00:00 2001 From: Luca Beldi Date: Fri, 17 Aug 2018 09:04:17 +0100 Subject: [PATCH] QSortFilterProxyModel inserting at bottom of source model Before this change, if you try to insert a row at the bottom of QSortFilterProxyModel the row will be inserted in the source model at position proxy->rowCount rather than at the bottom. This causes insert at apparently random positions in the source. [ChangeLog][QtCore][QSortFilterProxyModel] QSortFilterProxyModel::insertRows(row,count,parent) with row == QSortFilterProxyModel::rowCount will insert at the bottom of the source model rather than at the row QSortFilterProxyModel::rowCount of the source model Task-number: QTBUG-58499 Task-number: QTBUG-69158 Change-Id: Ie78416c8fbc429303b8c9c98375630e3e4d85f6d Reviewed-by: David Faure --- .../itemmodels/qsortfilterproxymodel.cpp | 4 +- .../tst_qsortfilterproxymodel.cpp | 237 ++++++++++++++++++ 2 files changed, 239 insertions(+), 2 deletions(-) diff --git a/src/corelib/itemmodels/qsortfilterproxymodel.cpp b/src/corelib/itemmodels/qsortfilterproxymodel.cpp index 08279cb244..6728f0106b 100644 --- a/src/corelib/itemmodels/qsortfilterproxymodel.cpp +++ b/src/corelib/itemmodels/qsortfilterproxymodel.cpp @@ -2191,7 +2191,7 @@ bool QSortFilterProxyModel::insertRows(int row, int count, const QModelIndex &pa if (row > m->source_rows.count()) return false; int source_row = (row >= m->source_rows.count() - ? m->source_rows.count() + ? m->proxy_rows.count() : m->source_rows.at(row)); return d->model->insertRows(source_row, count, source_parent); } @@ -2211,7 +2211,7 @@ bool QSortFilterProxyModel::insertColumns(int column, int count, const QModelInd if (column > m->source_columns.count()) return false; int source_column = (column >= m->source_columns.count() - ? m->source_columns.count() + ? m->proxy_columns.count() : m->source_columns.at(column)); return d->model->insertColumns(source_column, count, source_parent); } diff --git a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp index 947c4e8112..bced3f5790 100644 --- a/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp +++ b/tests/auto/corelib/itemmodels/qsortfilterproxymodel/tst_qsortfilterproxymodel.cpp @@ -152,6 +152,11 @@ private slots: void emitLayoutChangedOnlyIfSortingChanged(); void checkSetNewModel(); + void filterAndInsertRow_data(); + void filterAndInsertRow(); + void filterAndInsertColumn_data(); + void filterAndInsertColumn(); + protected: void buildHierarchy(const QStringList &data, QAbstractItemModel *model); void checkHierarchy(const QStringList &data, const QAbstractItemModel *model); @@ -4612,5 +4617,237 @@ void tst_QSortFilterProxyModel::checkSetNewModel() QCoreApplication::processEvents(); } +enum ColumnFilterMode { + FilterNothing, + FilterOutMiddle, + FilterOutBeginEnd, + FilterAll +}; +Q_DECLARE_METATYPE(ColumnFilterMode) + +void tst_QSortFilterProxyModel::filterAndInsertColumn_data() +{ + + QTest::addColumn("insertCol"); + QTest::addColumn("filterMode"); + QTest::addColumn("expectedModelList"); + QTest::addColumn("expectedProxyModelList"); + + QTest::newRow("at_beginning_filter_out_middle") + << 0 + << FilterOutMiddle + << QStringList{{"", "A1", "B1", "C1", "D1"}} + << QStringList{{"", "D1"}} + ; + QTest::newRow("at_end_filter_out_middle") + << 2 + << FilterOutMiddle + << QStringList{{"A1", "B1", "C1", "D1", ""}} + << QStringList{{"A1", ""}} + ; + QTest::newRow("in_the_middle_filter_out_middle") + << 1 + << FilterOutMiddle + << QStringList{{"A1", "B1", "C1", "", "D1"}} + << QStringList{{"A1", "D1"}} + ; + QTest::newRow("at_beginning_filter_out_begin_and_end") + << 0 + << FilterOutBeginEnd + << QStringList{{"A1", "", "B1", "C1", "D1"}} + << QStringList{{"", "B1", "C1"}} + ; + QTest::newRow("at_end_filter_out_begin_and_end") + << 2 + << FilterOutBeginEnd + << QStringList{{"A1", "B1", "C1", "D1", ""}} + << QStringList{{"B1", "C1", "D1"}} + ; + QTest::newRow("in_the_middle_filter_out_begin_and_end") + << 1 + << FilterOutBeginEnd + << QStringList{{"A1", "B1", "", "C1", "D1"}} + << QStringList{{"B1", "", "C1"}} + ; + + QTest::newRow("at_beginning_filter_nothing") + << 0 + << FilterAll + << QStringList{{"", "A1", "B1", "C1", "D1"}} + << QStringList{{"", "A1", "B1", "C1", "D1"}} + ; + QTest::newRow("at_end_filter_nothing") + << 4 + << FilterAll + << QStringList{{"A1", "B1", "C1", "D1", ""}} + << QStringList{{"A1", "B1", "C1", "D1", ""}} + ; + QTest::newRow("in_the_middle_nothing") + << 2 + << FilterAll + << QStringList{{"A1", "B1", "", "C1", "D1"}} + << QStringList{{"A1", "B1", "", "C1", "D1"}} + ; + + QTest::newRow("filter_all") + << 0 + << FilterNothing + << QStringList{{"A1", "B1", "C1", "D1", ""}} + << QStringList{} + ; +} +void tst_QSortFilterProxyModel::filterAndInsertColumn() +{ + + class ColumnFilterProxy : public QSortFilterProxyModel { + Q_DISABLE_COPY(ColumnFilterProxy) + ColumnFilterMode filerMode; + public: + ColumnFilterProxy(ColumnFilterMode mode) + : filerMode(mode) + {} + bool filterAcceptsColumn(int source_column, const QModelIndex &source_parent) const override + { + Q_UNUSED(source_parent) + switch (filerMode){ + case FilterAll: + return true; + case FilterNothing: + return false; + case FilterOutMiddle: + return source_column == 0 || source_column == sourceModel()->columnCount() - 1; + case FilterOutBeginEnd: + return source_column > 0 && source_column< sourceModel()->columnCount() - 1; + } + Q_UNREACHABLE(); + } + }; + QFETCH(int, insertCol); + QFETCH(ColumnFilterMode, filterMode); + QFETCH(QStringList, expectedModelList); + QFETCH(QStringList, expectedProxyModelList); + QStandardItemModel model; + model.insertColumns(0, 4); + model.insertRows(0, 1); + for (int i = 0; i < model.rowCount(); ++i) { + for (int j = 0; j < model.columnCount(); ++j) + model.setData(model.index(i, j), QString('A' + j) + QString::number(i + 1)); + } + ColumnFilterProxy proxy(filterMode); + proxy.setSourceModel(&model); + QVERIFY(proxy.insertColumn(insertCol)); + proxy.invalidate(); + QStringList modelStringList; + for (int i = 0; i < model.rowCount(); ++i) { + for (int j = 0; j < model.columnCount(); ++j) + modelStringList.append(model.index(i, j).data().toString()); + } + QCOMPARE(expectedModelList, modelStringList); + modelStringList.clear(); + for (int i = 0; i < proxy.rowCount(); ++i) { + for (int j = 0; j < proxy.columnCount(); ++j) + modelStringList.append(proxy.index(i, j).data().toString()); + } + QCOMPARE(expectedProxyModelList, modelStringList); +} + +void tst_QSortFilterProxyModel::filterAndInsertRow_data() +{ + QTest::addColumn("initialModelList"); + QTest::addColumn("row"); + QTest::addColumn("filterRegExp"); + QTest::addColumn("expectedModelList"); + QTest::addColumn("expectedProxyModelList"); + + QTest::newRow("at_beginning_filter_out_middle") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 0 + << "^A" + << QStringList{{"", "A5", "B5", "B6", "A7"}} + << QStringList{{"A5", "A7"}}; + QTest::newRow("at_end_filter_out_middle") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 2 + << "^A" + << QStringList{{"A5", "B5", "B6", "A7", ""}} + << QStringList{{"A5", "A7"}}; + QTest::newRow("in_the_middle_filter_out_middle") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 1 + << "^A" + << QStringList{{"A5", "B5", "B6", "", "A7"}} + << QStringList{{"A5", "A7"}}; + + QTest::newRow("at_beginning_filter_out_first_and_last") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 0 + << "^B" + << QStringList{{"A5", "", "B5", "B6", "A7"}} + << QStringList{{"B5", "B6"}}; + QTest::newRow("at_end_filter_out_first_and_last") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 2 + << "^B" + << QStringList{{"A5", "B5", "B6", "A7", ""}} + << QStringList{{"B5", "B6"}}; + QTest::newRow("in_the_middle_filter_out_first_and_last") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 1 + << "^B" + << QStringList{{"A5", "B5", "", "B6", "A7"}} + << QStringList{{"B5", "B6"}}; + + QTest::newRow("at_beginning_no_filter") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 0 + << ".*" + << QStringList{{"", "A5", "B5", "B6", "A7"}} + << QStringList{{"", "A5", "B5", "B6", "A7"}}; + QTest::newRow("at_end_no_filter") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 4 + << ".*" + << QStringList{{"A5", "B5", "B6", "A7", ""}} + << QStringList{{"A5", "B5", "B6", "A7", ""}}; + QTest::newRow("in_the_middle_no_filter") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 2 + << ".*" + << QStringList{{"A5", "B5", "", "B6", "A7"}} + << QStringList{{"A5", "B5", "", "B6", "A7"}}; + + QTest::newRow("filter_all") + << QStringList{{"A5", "B5", "B6", "A7"}} + << 0 + << "$a" + << QStringList{{"A5", "B5", "B6", "A7", ""}} + << QStringList{}; +} + +void tst_QSortFilterProxyModel::filterAndInsertRow() +{ + QFETCH(QStringList, initialModelList); + QFETCH(int, row); + QFETCH(QString, filterRegExp); + QFETCH(QStringList, expectedModelList); + QFETCH(QStringList, expectedProxyModelList); + QStringListModel model; + QSortFilterProxyModel proxyModel; + + model.setStringList(initialModelList); + proxyModel.setSourceModel(&model); + proxyModel.setDynamicSortFilter(true); + proxyModel.setFilterRegExp(filterRegExp); + + QVERIFY(proxyModel.insertRow(row)); + QCOMPARE(model.stringList(), expectedModelList); + QCOMPARE(proxyModel.rowCount(), expectedProxyModelList.count()); + for (int r = 0; r < proxyModel.rowCount(); ++r) { + QModelIndex index = proxyModel.index(r, 0); + QVERIFY(index.isValid()); + QCOMPARE(proxyModel.data(index).toString(), expectedProxyModelList.at(r)); + } +} + QTEST_MAIN(tst_QSortFilterProxyModel) #include "tst_qsortfilterproxymodel.moc"