Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix geojson attr table field order #45174

Merged
merged 3 commits into from
Oct 14, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
Expand Down
4 changes: 2 additions & 2 deletions src/core/providers/ogr/qgsogrprovider.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 )
{
Expand All @@ -3734,9 +3734,9 @@ bool QgsOgrProvider::leaveUpdateMode()
{
bool ignoreErrorOut = false;
addAttributeOGRLevel( field, ignoreErrorOut );
mAttributeFields.append( field );
}
}
mAttributeFields = oldFields;
}
}
return true;
Expand Down
1 change: 1 addition & 0 deletions src/core/providers/ogr/qgsogrprovider.h
Original file line number Diff line number Diff line change
Expand Up @@ -329,6 +329,7 @@ class QgsOgrProvider final: public QgsVectorDataProvider
* and the new file will be opened.
*/
void reloadProviderData() override;

};

///@endcond
Expand Down
10 changes: 8 additions & 2 deletions src/core/qgsattributetableconfig.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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 );
}
}
}

Expand Down
11 changes: 10 additions & 1 deletion src/core/vector/qgsvectorlayer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -4025,6 +4033,7 @@ void QgsVectorLayer::updateFields()
emit updatedFields();
mEditFormConfig.setFields( mFields );
}

}


Expand Down
4 changes: 2 additions & 2 deletions src/gui/attributetable/qgsattributetablefiltermodel.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
Expand Down
4 changes: 3 additions & 1 deletion src/gui/attributetable/qgsattributetablefiltermodel.h
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 7 additions & 0 deletions src/gui/attributetable/qgsdualview.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
66 changes: 66 additions & 0 deletions tests/src/python/test_provider_ogr.py
Original file line number Diff line number Diff line change
Expand Up @@ -2247,6 +2247,72 @@ 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())
self.assertEqual(vl.fields().names(), ['A', 'C', 'B'])
elpaso marked this conversation as resolved.
Show resolved Hide resolved

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()