QSqlTableModel: do not suppress dataChanged() on inserted record

Affects setData() and setRecord().

Previously dataChanged() was suppressed when editing an inserted
record, except for OnManualSubmit. The motivation was probably to
allow setData() to be used while handling primeInsert().

Suppressing dataChanged() is not a good idea since views other than
the one which made the change will not know of the change.

It is a terrible idea to call setData() or setRecord() while
handling primeInsert(), so this is now expressly forbidden.
setData() and setRecord() now do nothing and return false if called
while rows are being inserted.

Change-Id: I96738c09a6268704c5626d95b72bfb46378e3242
Reviewed-by: Oswald Buddenhagen <oswald.buddenhagen@nokia.com>

diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0
index 5317137..c62d8b1 100644 (file)
--- a/dist/changes-5.0.0
+++ b/dist/changes-5.0.0
@@ -328,6 +328,20 @@ QTestLib
 * [QTBUG-20615] Autotests can now log test output to multiple destinations
   and log formats simultaneously.
 
+QtSql
+-----
+QSqlTableModel/QSqlRelationalTableModel
+
+* The dataChanged() signal is now emitted for changes made to an inserted
+record that has not yet been committed. Previously, dataChanged() was
+suppressed in this case for OnRowChange and OnFieldChange. This was probably
+an attempt to avoid trouble if setData() was called while handling
+primeInsert(). By emitting dataChanged(), we ensure that all views are aware
+of the change.
+
+* While handling primeInsert() signal, the record must be manipulated using
+the provided reference. Do not attempt to manipulate the records using the
+model methods setData() or setRecord().
 
 * removeRows() no longer emits extra beforeDelete signal for out of range row.
 

diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp
index e83115b..d943b5d 100644 (file)
--- a/src/sql/models/qsqltablemodel.cpp
+++ b/src/sql/models/qsqltablemodel.cpp
@@ -256,7 +256,9 @@ QSqlRecord QSqlTableModelPrivate::primaryValues(int row)
     initiated in the given \a row of the currently active database
     table. The \a record parameter can be written to (since it is a
     reference), for example to populate some fields with default
-    values.
+    values and set the generated flags of the fields. Do not try to
+    edit the record via other means such as setData() or setRecord()
+    while handling this signal.
 */
 
 /*!
@@ -479,6 +481,9 @@ bool QSqlTableModel::isDirty(const QModelIndex &index) const
 bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, int role)
 {
     Q_D(QSqlTableModel);
+    if (d->busyInsertingRows)
+        return false;
+
     if (role != Qt::EditRole)
         return QSqlQueryModel::setData(index, value, role);
 
@@ -512,19 +517,7 @@ bool QSqlTableModel::setData(const QModelIndex &index, const QVariant &value, in
             select();
     }
 
-    // historical bug: dataChanged() is suppressed for OnFieldChange and OnRowChange
-    // when operating on an "insert" record. This is to accomodate
-    // applications that call setData() while handling primeInsert().
-    // Otherwise dataChanged() would be emitted between beginInsert()
-    // and endInsert().
-    // The price of this workaround is that, although the view making
-    // the change will already display the new value, other views connected
-    // to the model probably will not.
-    // It's not clear why OnManualSubmit is excluded from this workaround.
-    // Calling setData() while handling primeInsert() is arguably very wrong anyway.
-    // primeInsert() provides a ref to the record for settings values.
-    if (d->strategy == OnManualSubmit || row.op() != QSqlTableModelPrivate::Insert)
-        emit dataChanged(index, index);
+    emit dataChanged(index, index);
 
     return isOk;
 }
@@ -1049,6 +1042,7 @@ bool QSqlTableModel::insertRows(int row, int count, const QModelIndex &parent)
     if (d->strategy != OnManualSubmit && count != 1)
         return false;
 
+    d->busyInsertingRows = true;
     beginInsertRows(parent, row, row + count - 1);
 
     if (d->strategy != OnManualSubmit)
@@ -1071,6 +1065,7 @@ bool QSqlTableModel::insertRows(int row, int count, const QModelIndex &parent)
     }
 
     endInsertRows();
+    d->busyInsertingRows = false;
     return true;
 }
 
@@ -1205,6 +1200,9 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &record)
 {
     Q_D(QSqlTableModel);
     Q_ASSERT_X(row >= 0, "QSqlTableModel::setRecord()", "Cannot set a record to a row less than 0");
+    if (d->busyInsertingRows)
+        return false;
+
     if (row >= rowCount())
         return false;
 
@@ -1233,10 +1231,8 @@ bool QSqlTableModel::setRecord(int row, const QSqlRecord &record)
             // historical bug: it's a bad idea to check for change here
             // historical bug: should test oldValue.isNull() != value.isNull()
             if (oldValue.isNull() || oldValue != value) {
-            // historical bug: dataChanged() is suppressed for Insert. See also setData().
                 mrow.setValue(idx, record.value(i));
-                if (mrow.op() != QSqlTableModelPrivate::Insert)
-                    emit dataChanged(cIndex, cIndex);
+                emit dataChanged(cIndex, cIndex);
             }
         } else {
             mrow.setValue(idx, record.value(i));

diff --git a/src/sql/models/qsqltablemodel_p.h b/src/sql/models/qsqltablemodel_p.h
index e6e70d2..dbf2e27 100644 (file)
--- a/src/sql/models/qsqltablemodel_p.h
+++ b/src/sql/models/qsqltablemodel_p.h
@@ -66,7 +66,8 @@ public:
     QSqlTableModelPrivate()
         : sortColumn(-1),
           sortOrder(Qt::AscendingOrder),
-          strategy(QSqlTableModel::OnRowChange)
+          strategy(QSqlTableModel::OnRowChange),
+          busyInsertingRows(false)
     {}
     void clear();
     QSqlRecord primaryValues(int index);
@@ -86,6 +87,7 @@ public:
     Qt::SortOrder sortOrder;
 
     QSqlTableModel::EditStrategy strategy;
+    bool busyInsertingRows;
 
     QSqlQuery editQuery;
     QSqlIndex primaryIndex;