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 <andy.shaw@digia.com>
Reviewed-by: Marc Mutz <marc.mutz@kdab.com>
Reviewed-by: Konstantin Ritt <ritt.ks@gmail.com>
Reviewed-by: David Faure <david.faure@kdab.com>
Reviewed-by: Mark Brand <mabrand@mabrand.nl>

diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp
index b979a1a..a3fdd06 100644 (file)
--- 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 d40cceb..d3407de 100644 (file)
--- 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<QModelIndex>(spy.at(0).at(0)), model.index(i, 0));
-                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(1)), model.index(i, rec.count() - 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(0)), model.index(i, 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(1)), model.index(i, 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(1).at(0)), model.index(i, 2));
+                QCOMPARE(qvariant_cast<QModelIndex>(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<QModelIndex>(spy.at(0).at(0)), model.index(i, 0));
-                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(1)), model.index(i, rec.count() - 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(0)), model.index(i, 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(0).at(1)), model.index(i, 1));
+                QCOMPARE(qvariant_cast<QModelIndex>(spy.at(1).at(0)), model.index(i, 2));
+                QCOMPARE(qvariant_cast<QModelIndex>(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);