Skip to content
Permalink
Browse files
Merge pull request #45174 from elpaso/bugfix-gh45139-geojson-field-order
Fix geojson attr table field order
  • Loading branch information
rouault committed Oct 14, 2021
2 parents d92b8b0 + d7af7d2 commit 0f3011b5ec8c989af6f2ddc622f172eb63424ec4
@@ -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 );
@@ -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;
@@ -329,6 +329,7 @@ class QgsOgrProvider final: public QgsVectorDataProvider
* and the new file will be opened.
*/
void reloadProviderData() override;

};

///@endcond
@@ -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 );
}
}
}

@@ -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 );
}

}


@@ -144,13 +144,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;
}
@@ -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
@@ -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
@@ -2280,6 +2280,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()

0 comments on commit 0f3011b

Please sign in to comment.