diff --git a/python/gui/auto_generated/attributetable/qgsattributetablefiltermodel.sip.in b/python/gui/auto_generated/attributetable/qgsattributetablefiltermodel.sip.in index 7f48cf848dd2..6f3b68ad1fb2 100644 --- a/python/gui/auto_generated/attributetable/qgsattributetablefiltermodel.sip.in +++ b/python/gui/auto_generated/attributetable/qgsattributetablefiltermodel.sip.in @@ -203,11 +203,13 @@ Returns -1 if none is defined. virtual int columnCount( const QModelIndex &parent ) const; - void setAttributeTableConfig( const QgsAttributeTableConfig &config ); + void setAttributeTableConfig( const QgsAttributeTableConfig &config); %Docstring Set the attribute table configuration to control which fields are shown, in which order they are shown as well as if and where an action column is shown. + +:param config: attribute table config %End void setFilterExpression( const QgsExpression &expression, const QgsExpressionContext &context ); diff --git a/src/core/providers/ogr/qgsogrprovider.cpp b/src/core/providers/ogr/qgsogrprovider.cpp index 3a647535d928..41be54fae64c 100644 --- a/src/core/providers/ogr/qgsogrprovider.cpp +++ b/src/core/providers/ogr/qgsogrprovider.cpp @@ -3720,7 +3720,7 @@ bool QgsOgrProvider::leaveUpdateMode() { // Backup fields since if we created new fields, but didn't populate it // with any feature yet, it will disappear. - QgsFields oldFields = mAttributeFields; + const QgsFields oldFields = mAttributeFields; reloadData(); if ( mValid ) { @@ -3734,9 +3734,9 @@ bool QgsOgrProvider::leaveUpdateMode() { bool ignoreErrorOut = false; addAttributeOGRLevel( field, ignoreErrorOut ); + mAttributeFields.append( field ); } } - mAttributeFields = oldFields; } } return true; diff --git a/src/core/providers/ogr/qgsogrprovider.h b/src/core/providers/ogr/qgsogrprovider.h index 5f46139c7c5e..6c1bbff6037c 100644 --- a/src/core/providers/ogr/qgsogrprovider.h +++ b/src/core/providers/ogr/qgsogrprovider.h @@ -329,6 +329,7 @@ class QgsOgrProvider final: public QgsVectorDataProvider * and the new file will be opened. */ void reloadProviderData() override; + }; ///@endcond diff --git a/src/core/qgsattributetableconfig.cpp b/src/core/qgsattributetableconfig.cpp index f5eaca85a3f2..93eba80de9b5 100644 --- a/src/core/qgsattributetableconfig.cpp +++ b/src/core/qgsattributetableconfig.cpp @@ -86,8 +86,14 @@ void QgsAttributeTableConfig::update( const QgsFields &fields ) newColumn.hidden = false; newColumn.type = Field; newColumn.name = field.name(); - - mColumns.append( newColumn ); + if ( containsActionColumn ) + { + mColumns.insert( mColumns.size() - 1, newColumn ); + } + else + { + mColumns.append( newColumn ); + } } } diff --git a/src/core/vector/qgsvectorlayer.cpp b/src/core/vector/qgsvectorlayer.cpp index 6d8e10109a70..96d03cd354d6 100644 --- a/src/core/vector/qgsvectorlayer.cpp +++ b/src/core/vector/qgsvectorlayer.cpp @@ -3495,10 +3495,18 @@ bool QgsVectorLayer::commitChanges( bool stopEditing ) } updateFields(); - mDataProvider->updateExtents(); + mDataProvider->updateExtents(); mDataProvider->leaveUpdateMode(); + // This second call is required because OGR provider with JSON + // driver might have changed fields order after the call to + // leaveUpdateMode + if ( mFields.names() != mDataProvider->fields().names() ) + { + updateFields(); + } + triggerRepaint(); return success; @@ -4025,6 +4033,7 @@ void QgsVectorLayer::updateFields() emit updatedFields(); mEditFormConfig.setFields( mFields ); } + } diff --git a/src/gui/attributetable/qgsattributetablefiltermodel.cpp b/src/gui/attributetable/qgsattributetablefiltermodel.cpp index 23c99cd6f7ef..70e439bde066 100644 --- a/src/gui/attributetable/qgsattributetablefiltermodel.cpp +++ b/src/gui/attributetable/qgsattributetablefiltermodel.cpp @@ -143,13 +143,13 @@ int QgsAttributeTableFilterModel::columnCount( const QModelIndex &parent ) const return mColumnMapping.count(); } -void QgsAttributeTableFilterModel::setAttributeTableConfig( const QgsAttributeTableConfig &config ) +void QgsAttributeTableFilterModel::setAttributeTableConfig( const QgsAttributeTableConfig &config, bool force ) { const QgsAttributeTableConfig oldConfig = mConfig; mConfig = config; mConfig.update( layer()->fields() ); - if ( mConfig.hasSameColumns( oldConfig ) ) + if ( !force && mConfig.hasSameColumns( oldConfig ) ) { return; } diff --git a/src/gui/attributetable/qgsattributetablefiltermodel.h b/src/gui/attributetable/qgsattributetablefiltermodel.h index e6c25673a7f9..ffe370164bbd 100644 --- a/src/gui/attributetable/qgsattributetablefiltermodel.h +++ b/src/gui/attributetable/qgsattributetablefiltermodel.h @@ -238,8 +238,10 @@ class GUI_EXPORT QgsAttributeTableFilterModel: public QSortFilterProxyModel, pub * Set the attribute table configuration to control which fields are shown, * in which order they are shown as well as if and where an action column * is shown. + * \param config attribute table config + * \param force default FALSE, if TRUE the attribute table configuration will be reset even if it is not changed. */ - void setAttributeTableConfig( const QgsAttributeTableConfig &config ); + void setAttributeTableConfig( const QgsAttributeTableConfig &config, bool force SIP_PYARGREMOVE = false ); /** * Set the \a expression and the \a context to be stored in case of the features diff --git a/src/gui/attributetable/qgsdualview.cpp b/src/gui/attributetable/qgsdualview.cpp index 53f77cd38d57..0064644c3a19 100644 --- a/src/gui/attributetable/qgsdualview.cpp +++ b/src/gui/attributetable/qgsdualview.cpp @@ -128,6 +128,13 @@ void QgsDualView::init( QgsVectorLayer *layer, QgsMapCanvas *mapCanvas, const Qg mAttributeForm = nullptr; mLayer = layer; + + // Keep fields order in sync: force config reset + connect( mLayer, &QgsVectorLayer::updatedFields, this, [ = ] + { + mFilterModel->setAttributeTableConfig( attributeTableConfig(), /* force */ true ); + } ); + mEditorContext = context; // create an empty form to find out if it needs geometry or not diff --git a/tests/src/python/test_provider_ogr.py b/tests/src/python/test_provider_ogr.py index 329cbb966917..d475fcdf8f8e 100644 --- a/tests/src/python/test_provider_ogr.py +++ b/tests/src/python/test_provider_ogr.py @@ -2247,6 +2247,77 @@ def test_provider_sidecar_files_for_uri(self): ['/home/me/special.gfs', '/home/me/special.xsd']) self.assertEqual(metadata.sidecarFilesForUri('/home/me/special.csv'), ['/home/me/special.csvt']) + def testGeoJsonFieldOrder(self): + """Test issue GH #45139""" + + d = QTemporaryDir() + json_path = os.path.join(d.path(), 'test.geojson') + with open(json_path, 'w+') as f: + f.write(""" + { + "type": "FeatureCollection", + "features": [ + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [11.1215698,46.0677293] + }, + "properties": { + "A": "A", + } + }, + { + "type": "Feature", + "geometry": { + "type": "Point", + "coordinates": [11.1214686,46.0677385] + }, + "properties": { + "A": "A", + "B": "B", + } + } + ] + } + """) + + vl = QgsVectorLayer(json_path, 'json') + self.assertTrue(vl.isValid()) + self.assertEqual(vl.featureCount(), 2) + self.assertEqual(vl.fields().names(), ['A', 'B']) + + # Append a field + self.assertTrue(vl.startEditing()) + self.assertTrue(vl.addAttribute(QgsField('C', QVariant.String))) + + for f in vl.getFeatures(): + vl.changeAttributeValue(f.id(), 2, 'C') + + self.assertEqual(vl.fields().names(), ['A', 'B', 'C']) + + features = [f for f in vl.getFeatures()] + + self.assertEqual(features[0].attribute('B'), NULL) + self.assertEqual(features[0].attribute('C'), 'C') + self.assertEqual(features[1].attribute('B'), 'B') + self.assertEqual(features[1].attribute('C'), 'C') + + self.assertTrue(vl.commitChanges()) + + # This has been fixed in GDAL >= 3.4 + if int(gdal.VersionInfo('VERSION_NUM')) >= GDAL_COMPUTE_VERSION(3, 4, 0): + self.assertEqual(vl.fields().names(), ['A', 'B', 'C']) + else: + self.assertEqual(vl.fields().names(), ['A', 'C', 'B']) + + features = [f for f in vl.getFeatures()] + + self.assertEqual(features[0].attribute('B'), NULL) + self.assertEqual(features[0].attribute('C'), 'C') + self.assertEqual(features[1].attribute('B'), 'B') + self.assertEqual(features[1].attribute('C'), 'C') + if __name__ == '__main__': unittest.main()