From: Mark Brand Date: Tue, 21 Feb 2012 08:22:26 +0000 (+0100) Subject: QSqlTableModel: use selectRow() for field and row edit strategies X-Git-Tag: qt-v5.0.0-alpha1~525 X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=888fed8065f708baeb6efa5f280c2f1aec3dee78;p=profile%2Fivi%2Fqtbase.git QSqlTableModel: use selectRow() for field and row edit strategies Calling select refreshes the query data but disrupts view navigation. For OnFieldChange and OnRecordChange it makes sense to only select the row in question. This does not disturb view navigation. Assume disruption of view navigation is not a problem for OnManualSubmit because the user or application decides when submitAll is called. Task-number: QTBUG-2875 Change-Id: I1e5f68668fb9102f6296d67d543e80daa403f1c4 Reviewed-by: Yunqiao Yin --- diff --git a/dist/changes-5.0.0 b/dist/changes-5.0.0 index dd0a069..33a8869 100644 --- a/dist/changes-5.0.0 +++ b/dist/changes-5.0.0 @@ -406,6 +406,14 @@ ignore the rest of the range. QSqlTableModel::indexInQuery() as example of how to implement in a subclass. +* QSqlTableModel edit strategies OnFieldChange/OnRowChange QTBUG-2875 +Previously, after changes were submitted in these edit strategies, select() +was called which removed and inserted all rows. This ruined navigation +in QTableView. Now, with these edit strategies, there is no implicit select() +done after committing. This includes deleted rows which remain in +the model as blank rows until the application calls select(). Instead, +selectRow() is called to refresh only the affected row. + **************************************************************************** * Database Drivers * **************************************************************************** diff --git a/src/sql/models/qsqltablemodel.cpp b/src/sql/models/qsqltablemodel.cpp index 40230c3..aa7dc3d 100644 --- a/src/sql/models/qsqltablemodel.cpp +++ b/src/sql/models/qsqltablemodel.cpp @@ -663,8 +663,8 @@ bool QSqlTableModel::deleteRowFromTable(int row) Returns false on error, detailed error information can be obtained with lastError(). - On success the model will be repopulated. Any views - presenting it will lose their selections. + In OnManualSubmit, on success the model will be repopulated. + Any views presenting it will lose their selections. Note: In OnManualSubmit mode, already submitted changes won't be cleared from the cache when submitAll() fails. This allows @@ -677,6 +677,8 @@ bool QSqlTableModel::submitAll() { Q_D(QSqlTableModel); + bool success = true; + for (QSqlTableModelPrivate::CacheMap::Iterator it = d->cache.begin(); it != d->cache.constEnd(); ++it) { if (it.value().submitted()) @@ -684,25 +686,35 @@ bool QSqlTableModel::submitAll() switch (it.value().op()) { case QSqlTableModelPrivate::Insert: - if (!insertRowIntoTable(it.value().rec())) - return false; + success = insertRowIntoTable(it.value().rec()); break; case QSqlTableModelPrivate::Update: - if (!updateRowInTable(it.key(), it.value().rec())) - return false; + success = updateRowInTable(it.key(), it.value().rec()); break; case QSqlTableModelPrivate::Delete: - if (!deleteRowFromTable(it.key())) - return false; + success = deleteRowFromTable(it.key()); break; case QSqlTableModelPrivate::None: Q_ASSERT_X(false, "QSqlTableModel::submitAll()", "Invalid cache operation"); break; } - it.value().setSubmitted(); + + if (success) { + it.value().setSubmitted(); + if (d->strategy != OnManualSubmit) + success = selectRow(it.key()); + } + + if (!success) + break; + } + + if (success) { + if (d->strategy == OnManualSubmit) + success = select(); } - return select(); + return success; } /*! @@ -719,8 +731,8 @@ bool QSqlTableModel::submitAll() Returns true on success; otherwise returns false. Use lastError() to query detailed error information. - On success the model will be repopulated. Any views - presenting it will lose their selections. + Does not automatically repopulate the model. Submitted rows are + refreshed from the database on success. \sa revert(), revertRow(), submitAll(), revertAll(), lastError() */ diff --git a/tests/auto/sql/models/qsqlrelationaltablemodel/tst_qsqlrelationaltablemodel.cpp b/tests/auto/sql/models/qsqlrelationaltablemodel/tst_qsqlrelationaltablemodel.cpp index ddafeea..ce0d8db 100644 --- a/tests/auto/sql/models/qsqlrelationaltablemodel/tst_qsqlrelationaltablemodel.cpp +++ b/tests/auto/sql/models/qsqlrelationaltablemodel/tst_qsqlrelationaltablemodel.cpp @@ -542,6 +542,13 @@ void tst_QSqlRelationalTableModel::setRecord() model.setSort(0, Qt::AscendingOrder); QVERIFY_SQL(model, submit()); + if (model.editStrategy() != QSqlTableModel::OnManualSubmit) { + QCOMPARE(model.data(model.index(1, 0)).toInt(), 7); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("tester")); + QCOMPARE(model.data(model.index(1, 2)).toString(), QString("herr")); + QVERIFY_SQL(model, select()); + } + QCOMPARE(model.data(model.index(3, 0)).toInt(), 7); QCOMPARE(model.data(model.index(3, 1)).toString(), QString("tester")); QCOMPARE(model.data(model.index(3, 2)).toString(), QString("herr")); @@ -599,6 +606,8 @@ void tst_QSqlRelationalTableModel::insertWithStrategies() QVERIFY_SQL(model, submitAll()); model.setEditStrategy(QSqlTableModel::OnManualSubmit); + // The changes were submitted, but there was no automatic select to resort + QVERIFY_SQL(model, select()); QCOMPARE(model.data(model.index(0,0)).toInt(), 1); QCOMPARE(model.data(model.index(0,1)).toString(), QString("harry")); @@ -1401,6 +1410,8 @@ void tst_QSqlRelationalTableModel::whiteSpaceInIdentifiers() QVERIFY_SQL(model, insertRecord(-1, rec)); model.submitAll(); + if (model.editStrategy() != QSqlTableModel::OnManualSubmit) + QVERIFY_SQL(model, select()); QCOMPARE(model.data(model.index(0, 0)).toInt(), 3); QCOMPARE(model.data(model.index(0, 1)).toString(), QString("Washington")); diff --git a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp index 4382998..448111c 100644 --- a/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp +++ b/tests/auto/sql/models/qsqltablemodel/tst_qsqltablemodel.cpp @@ -411,8 +411,11 @@ void tst_QSqlTableModel::setRecord() } else if ((QSqlTableModel::EditStrategy)submitpolicy == QSqlTableModel::OnRowChange && i == model.rowCount() -1) model.submit(); else { - // dataChanged() is not emitted when submitAll() is called - QCOMPARE(spy.count(), 1); + // dataChanged() emitted by selectRow() as well as setRecord() + if ((QSqlTableModel::EditStrategy)submitpolicy == QSqlTableModel::OnFieldChange) + QCOMPARE(spy.count(), 2); + else + QCOMPARE(spy.count(), 1); 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)); @@ -471,8 +474,7 @@ void tst_QSqlTableModel::insertRow() rec.setValue(0, 42); rec.setValue(1, QString("francis")); - // FieldChange updates immediately and resorts - // Row/Manual submit does not resort + // Setting record does not cause resort QVERIFY(model.setRecord(2, rec)); QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); @@ -482,8 +484,23 @@ void tst_QSqlTableModel::insertRow() QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); - // See comment above setRecord - if (submitpolicy == QSqlTableModel::OnFieldChange) { + QCOMPARE(model.data(model.index(2, 0)).toInt(), 42); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("francis")); + QCOMPARE(model.data(model.index(2, 2)).toInt(), 2); + QCOMPARE(model.data(model.index(3, 0)).toInt(), 3); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(3, 2)).toInt(), 3); + + QVERIFY(model.submitAll()); + + if (submitpolicy == QSqlTableModel::OnManualSubmit) { + // After the submit we should have the resorted view + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); QCOMPARE(model.data(model.index(2, 0)).toInt(), 3); QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); QCOMPARE(model.data(model.index(2, 2)).toInt(), 3); @@ -491,6 +508,13 @@ void tst_QSqlTableModel::insertRow() QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis")); QCOMPARE(model.data(model.index(3, 2)).toInt(), 2); } else { + // Submit does not select, therefore not resorted + QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 2); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(1, 2)).toInt(), 2); QCOMPARE(model.data(model.index(2, 0)).toInt(), 42); QCOMPARE(model.data(model.index(2, 1)).toString(), QString("francis")); QCOMPARE(model.data(model.index(2, 2)).toInt(), 2); @@ -499,9 +523,8 @@ void tst_QSqlTableModel::insertRow() QCOMPARE(model.data(model.index(3, 2)).toInt(), 3); } - QVERIFY(model.submitAll()); - - // After the submit we should have the resorted view + QVERIFY(model.select()); + // After the select we should have the resorted view in all strategies QCOMPARE(model.data(model.index(0, 0)).toInt(), 1); QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); QCOMPARE(model.data(model.index(0, 2)).toInt(), 1); @@ -514,7 +537,6 @@ void tst_QSqlTableModel::insertRow() QCOMPARE(model.data(model.index(3, 0)).toInt(), 42); QCOMPARE(model.data(model.index(3, 1)).toString(), QString("francis")); QCOMPARE(model.data(model.index(3, 2)).toInt(), 2); - } void tst_QSqlTableModel::insertRecord() @@ -647,8 +669,7 @@ void tst_QSqlTableModel::removeRow() QVERIFY_SQL(model, select()); QCOMPARE(model.rowCount(), 3); - // headerDataChanged must be emitted by the model when the edit strategy is OnManualSubmit, - // when OnFieldChange or OnRowChange it's not needed because the model will re-select. + // headerDataChanged must be emitted by the model since the row won't vanish until select qRegisterMetaType("Qt::Orientation"); QSignalSpy headerDataChangedSpy(&model, SIGNAL(headerDataChanged(Qt::Orientation, int, int))); @@ -673,7 +694,10 @@ void tst_QSqlTableModel::removeRow() headerDataChangedSpy.clear(); QVERIFY(model.removeRow(1)); - QCOMPARE(headerDataChangedSpy.count(), 0); + QCOMPARE(headerDataChangedSpy.count(), 1); + QCOMPARE(model.rowCount(), 3); + + QVERIFY_SQL(model, select()); QCOMPARE(model.rowCount(), 2); QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); @@ -706,6 +730,11 @@ void tst_QSqlTableModel::removeRows() QCOMPARE(beforeDeleteSpy.count(), 2); QVERIFY(beforeDeleteSpy.at(0).at(0).toInt() == 0); QVERIFY(beforeDeleteSpy.at(1).at(0).toInt() == 1); + // deleted rows shown as empty until select + QCOMPARE(model.rowCount(), 3); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("")); + QVERIFY(model.select()); + // deleted rows are gone QCOMPARE(model.rowCount(), 1); QCOMPARE(model.data(model.index(0, 1)).toString(), QString("vohi")); model.clear(); @@ -778,6 +807,14 @@ void tst_QSqlTableModel::removeInsertedRow() model.submitAll(); + if (model.editStrategy() != QSqlTableModel::OnManualSubmit) { + QCOMPARE(model.rowCount(), 4); + QCOMPARE(model.data(model.index(1, 0)).toInt(), 55); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("null columns")); + QCOMPARE(model.data(model.index(1, 2)).isNull(), true); + QVERIFY(model.select()); + } + QCOMPARE(model.rowCount(), 4); QCOMPARE(model.data(model.index(3, 0)).toInt(), 55); QCOMPARE(model.data(model.index(3, 1)).toString(), QString("null columns")); @@ -785,8 +822,17 @@ void tst_QSqlTableModel::removeInsertedRow() QVERIFY(model.removeRow(3)); model.submitAll(); - QCOMPARE(model.rowCount(), 3); + if (model.editStrategy() != QSqlTableModel::OnManualSubmit) { + QCOMPARE(model.rowCount(), 4); + QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); + QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); + QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); + QCOMPARE(model.data(model.index(3, 1)).toString(), QString("")); + QVERIFY(model.select()); + } + + QCOMPARE(model.rowCount(), 3); QCOMPARE(model.data(model.index(0, 1)).toString(), QString("harry")); QCOMPARE(model.data(model.index(1, 1)).toString(), QString("trond")); QCOMPARE(model.data(model.index(2, 1)).toString(), QString("vohi")); @@ -1129,6 +1175,11 @@ void tst_QSqlTableModel::insertRecordBeforeSelect() buffer.setValue("title", 0); QVERIFY_SQL(model, insertRecord(1, buffer)); + if (model.editStrategy() != QSqlTableModel::OnManualSubmit) { + QCOMPARE(model.rowCount(), 2); + QVERIFY_SQL(model, select()); + } + int rowCount = model.rowCount(); model.clear(); QCOMPARE(model.rowCount(), 0);