From a97c2d8c83aecd32e0e9893ef192eef1fd6065c7 Mon Sep 17 00:00:00 2001 From: Mark Brand Date: Fri, 28 Sep 2012 14:21:26 +0200 Subject: [PATCH] QSqlTableModel: restore use of virtual setData() in setRecord() setRecord() should use setData() as intended so that reimplementations of setData() in subclasses will be respected. Commit 11bd543d901f67f79db3f5b5dfaa626c6e8faa45 failed to consider this. Test added which should prevent this mistake being repeated. Change-Id: Ia2d930cd42b5a27521bb389edb1b07fb1bf0fa36 Reviewed-by: Andy Shaw Reviewed-by: Marc Mutz Reviewed-by: Konstantin Ritt Reviewed-by: David Faure Reviewed-by: Mark Brand --- src/sql/models/qsqltablemodel.cpp | 12 ++-- .../qsqltablemodel/tst_qsqltablemodel.cpp | 57 ++++++++++++++++--- 2 files changed, 55 insertions(+), 14 deletions(-) diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index b979a1a23a..a3fdd06442 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -1373,15 +1373,17 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &values) Map::const_iterator i = map.constBegin(); const Map::const_iterator e = map.constEnd(); for ( ; i != e; ++i) { - mrow.setValue(i.value(), values.value(i.key())); - // mrow.setValue() sets generated to TRUE, but source record should prevail. + // have to use virtual setData() here rather than mrow.setValue() + EditStrategy strategy = d->strategy; + d->strategy = OnManualSubmit; + QModelIndex cIndex = createIndex(row, i.value()); + setData(cIndex, values.value(i.key())); + d->strategy = strategy; + // setData() sets generated to TRUE, but source record should prevail. if (!values.isGenerated(i.key())) mrow.recRef().setGenerated(i.value(), false); } - if (columnCount()) - emit dataChanged(createIndex(row, 0), createIndex(row, columnCount() - 1)); - if (d->strategy != OnManualSubmit) return submit(); diff --git a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp index d40ccebefc..d3407de53b 100644 --- a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp +++ b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp @@ -85,6 +85,8 @@ private slots: void submitAll(); void setRecord_data() { generic_data(); } void setRecord(); + void setRecordReimpl_data() { generic_data(); } + void setRecordReimpl(); void insertRow_data() { generic_data_with_strategies(); } void insertRow(); void insertRowFailure_data() { generic_data_with_strategies(); } @@ -505,24 +507,28 @@ void tst_QSqlTableModel::setRecord() rec.setValue(2, rec.value(2).toString() + 'X'); QVERIFY(model.setRecord(i, rec)); + // dataChanged() emitted by setData() for each *changed* column if ((QSqlTableModel::EditStrategy)submitpolicy == QSqlTableModel::OnManualSubmit) { - // setRecord should emit dataChanged() itself for manualSubmit - QCOMPARE(spy.count(), 1); + QCOMPARE(spy.count(), 2); QCOMPARE(spy.at(0).count(), 2); - QCOMPARE(qvariant_cast(spy.at(0).at(0)), model.index(i, 0)); - QCOMPARE(qvariant_cast(spy.at(0).at(1)), model.index(i, rec.count() - 1)); + QCOMPARE(qvariant_cast(spy.at(0).at(0)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(0).at(1)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(1).at(0)), model.index(i, 2)); + QCOMPARE(qvariant_cast(spy.at(1).at(1)), model.index(i, 2)); QVERIFY(model.submitAll()); } else if ((QSqlTableModel::EditStrategy)submitpolicy == QSqlTableModel::OnRowChange && i == model.rowCount() -1) model.submit(); else { - // dataChanged() emitted by selectRow() as well as setRecord() if ((QSqlTableModel::EditStrategy)submitpolicy != QSqlTableModel::OnManualSubmit) - QCOMPARE(spy.count(), 2); + // dataChanged() also emitted by selectRow() + QCOMPARE(spy.count(), 3); else - QCOMPARE(spy.count(), 1); + QCOMPARE(spy.count(), 2); QCOMPARE(spy.at(0).count(), 2); - QCOMPARE(qvariant_cast(spy.at(0).at(0)), model.index(i, 0)); - QCOMPARE(qvariant_cast(spy.at(0).at(1)), model.index(i, rec.count() - 1)); + QCOMPARE(qvariant_cast(spy.at(0).at(0)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(0).at(1)), model.index(i, 1)); + QCOMPARE(qvariant_cast(spy.at(1).at(0)), model.index(i, 2)); + QCOMPARE(qvariant_cast(spy.at(1).at(1)), model.index(i, 2)); } } @@ -535,6 +541,39 @@ void tst_QSqlTableModel::setRecord() } } +class SetRecordReimplModel: public QSqlTableModel +{ + Q_OBJECT +public: + SetRecordReimplModel(QObject *parent, QSqlDatabase db):QSqlTableModel(parent, db) {} + bool setData(const QModelIndex &index, const QVariant &value, int role = Qt::EditRole) + { + return QSqlTableModel::setData(index, QString("Qt"), role); + } +}; + +void tst_QSqlTableModel::setRecordReimpl() +{ + QFETCH(QString, dbName); + QSqlDatabase db = QSqlDatabase::database(dbName); + CHECK_DATABASE(db); + SetRecordReimplModel model(0, db); + model.setEditStrategy(QSqlTableModel::OnManualSubmit); + model.setTable(test3); + model.setSort(0, Qt::AscendingOrder); + QVERIFY_SQL(model, select()); + + // make sure that a reimplemented setData() affects setRecord() + QSqlRecord rec = model.record(0); + rec.setValue(1, QString("x")); + rec.setValue(2, QString("y")); + QVERIFY(model.setRecord(0, rec)); + + rec = model.record(0); + QCOMPARE(rec.value(1).toString(), QString("Qt")); + QCOMPARE(rec.value(2).toString(), QString("Qt")); +} + void tst_QSqlTableModel::insertRow() { QFETCH(QString, dbName);