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

When loading a QLR, try to match the layer name if id does not #30151

Expand Up @@ -121,6 +121,15 @@ Check whether the ``feature`` has all values required by the ``expression``
.. versionadded:: 3.2
%End

static QgsVectorLayer *resolveLayer( const QVariantMap &config );
%Docstring
Returns the (possibly NULL) layer from the widget's ``config``

.. versionadded:: 3.8
%End



};


Expand Down
8 changes: 8 additions & 0 deletions python/core/auto_generated/qgsmaplayer.sip.in
Expand Up @@ -1260,6 +1260,14 @@ The storage format for the XML is qlr
.. versionadded:: 3.6
%End

static QString generateId( const QString &layerName );
%Docstring
Generates an unique identifier for this layer, the generate ID is prefixed by ``layerName``

.. versionadded:: 3.8
%End


public slots:

void setMinimumScale( double scale );
Expand Down
25 changes: 24 additions & 1 deletion src/core/fieldformatter/qgsvaluerelationfieldformatter.cpp
Expand Up @@ -118,7 +118,7 @@ QgsValueRelationFieldFormatter::ValueRelationCache QgsValueRelationFieldFormatte
{
ValueRelationCache cache;

QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config.value( QStringLiteral( "Layer" ) ).toString() );
const QgsVectorLayer *layer = resolveLayer( config );

if ( !layer )
return cache;
Expand Down Expand Up @@ -273,3 +273,26 @@ bool QgsValueRelationFieldFormatter::expressionIsUsable( const QString &expressi
return false;
return true;
}

QgsVectorLayer *QgsValueRelationFieldFormatter::resolveLayer( const QVariantMap &config )
elpaso marked this conversation as resolved.
Show resolved Hide resolved
{
QgsVectorLayer *layer { QgsProject::instance()->mapLayer<QgsVectorLayer *>( config.value( QStringLiteral( "Layer" ) ).toString() ) };
if ( ! layer )
{
const auto name { config.value( QStringLiteral( "LayerName" ) ).toString() };
if ( ! name.isEmpty() )
{
const auto constLayers { QgsProject::instance()->mapLayers( true ) };
for ( QgsMapLayer *l : constLayers )
{
QgsVectorLayer *vl { qobject_cast<QgsVectorLayer *>( l ) };
if ( vl && vl->name() == name )
{
layer = vl;
}
}
}
}
return layer;
}

8 changes: 8 additions & 0 deletions src/core/fieldformatter/qgsvaluerelationfieldformatter.h
Expand Up @@ -119,6 +119,14 @@ class CORE_EXPORT QgsValueRelationFieldFormatter : public QgsFieldFormatter
*/
static bool expressionIsUsable( const QString &expression, const QgsFeature &feature );

/**
* Returns the (possibly NULL) layer from the widget's \a config
* \since QGIS 3.8
*/
static QgsVectorLayer *resolveLayer( const QVariantMap &config );



};

Q_DECLARE_METATYPE( QgsValueRelationFieldFormatter::ValueRelationCache )
Expand Down
55 changes: 38 additions & 17 deletions src/core/qgslayerdefinition.cpp
Expand Up @@ -87,26 +87,25 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj

// IDs of layers should be changed otherwise we may have more then one layer with the same id
// We have to replace the IDs before we load them because it's too late once they are loaded
QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
for ( int i = 0; i < ids.size(); ++i )
QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );
for ( int i = 0; i < treeLayerNodes.size(); ++i )
{
QDomNode idnode = ids.at( i );
QDomElement idElem = idnode.toElement();
QString oldid = idElem.text();
// Strip the date part because we will replace it.
QString layername = oldid.left( oldid.length() - 17 );
QDateTime dt = QDateTime::currentDateTime();
QString newid = layername + dt.toString( QStringLiteral( "yyyyMMddhhmmsszzz" ) ) + QString::number( qrand() );
idElem.firstChild().setNodeValue( newid );
QDomNodeList treeLayerNodes = doc.elementsByTagName( QStringLiteral( "layer-tree-layer" ) );

for ( int i = 0; i < treeLayerNodes.count(); ++i )
QDomNode treeLayerNode = treeLayerNodes.at( i );
QDomElement treeLayerElem = treeLayerNode.toElement();
QString oldid = treeLayerElem.attribute( QStringLiteral( "id" ) );
QString layername = treeLayerElem.attribute( QStringLiteral( "name" ) );
QString newid = QgsMapLayer::generateId( layername );
treeLayerElem.setAttribute( QStringLiteral( "id" ), newid );

// Replace IDs for map layers
QDomNodeList ids = doc.elementsByTagName( QStringLiteral( "id" ) );
for ( int i = 0; i < ids.size(); ++i )
{
QDomNode layerNode = treeLayerNodes.at( i );
QDomElement layerElem = layerNode.toElement();
if ( layerElem.attribute( QStringLiteral( "id" ) ) == oldid )
QDomNode idnode = ids.at( i );
QDomElement idElem = idnode.toElement();
if ( idElem.text() == oldid )
{
layerNode.toElement().setAttribute( QStringLiteral( "id" ), newid );
idElem.firstChild().setNodeValue( newid );
}
}

Expand Down Expand Up @@ -137,6 +136,28 @@ bool QgsLayerDefinition::loadLayerDefinition( QDomDocument doc, QgsProject *proj
}
}

// Change IDs of widget config values
QDomNodeList widgetConfig = doc.elementsByTagName( QStringLiteral( "editWidget" ) );
for ( int i = 0; i < widgetConfig.size(); i++ )
{
QDomNodeList config = widgetConfig.at( i ).childNodes();
for ( int j = 0; j < config.size(); j++ )
{
QDomNodeList optMap = config.at( j ).childNodes();
for ( int z = 0; z < optMap.size(); z++ )
{
QDomNodeList opts = optMap.at( z ).childNodes();
for ( int k = 0; k < opts.size(); k++ )
{
QDomElement opt = opts.at( k ).toElement();
if ( opt.attribute( QStringLiteral( "value" ) ) == oldid )
{
opt.setAttribute( QStringLiteral( "value" ), newid );
}
}
}
}
}
}

QDomElement layerTreeElem = doc.documentElement().firstChildElement( QStringLiteral( "layer-tree-group" ) );
Expand Down
33 changes: 17 additions & 16 deletions src/core/qgsmaplayer.cpp
Expand Up @@ -78,24 +78,9 @@ QgsMapLayer::QgsMapLayer( QgsMapLayerType type,
, mStyleManager( new QgsMapLayerStyleManager( this ) )
, mRefreshTimer( new QTimer( this ) )
{
//mShortName.replace( QRegExp( "[\\W]" ), "_" );

// Generate the unique ID of this layer
QString uuid = QUuid::createUuid().toString();
// trim { } from uuid
mID = lyrname + '_' + uuid.mid( 1, uuid.length() - 2 );

// Tidy the ID up to avoid characters that may cause problems
// elsewhere (e.g in some parts of XML). Replaces every non-word
// character (word characters are the alphabet, numbers and
// underscore) with an underscore.
// Note that the first backslashe in the regular expression is
// there for the compiler, so the pattern is actually \W
mID.replace( QRegExp( "[\\W]" ), QStringLiteral( "_" ) );

mID = generateId( lyrname );
connect( this, &QgsMapLayer::crsChanged, this, &QgsMapLayer::configChanged );
connect( this, &QgsMapLayer::nameChanged, this, &QgsMapLayer::configChanged );

connect( mRefreshTimer, &QTimer::timeout, this, [ = ] { triggerRepaint( true ); } );
}

Expand Down Expand Up @@ -1876,6 +1861,22 @@ void QgsMapLayer::setOriginalXmlProperties( const QString &originalXmlProperties
mOriginalXmlProperties = originalXmlProperties;
}

QString QgsMapLayer::generateId( const QString &layerName )
{
// Generate the unique ID of this layer
QString uuid = QUuid::createUuid().toString();
// trim { } from uuid
QString id = layerName + '_' + uuid.mid( 1, uuid.length() - 2 );
// Tidy the ID up to avoid characters that may cause problems
// elsewhere (e.g in some parts of XML). Replaces every non-word
// character (word characters are the alphabet, numbers and
// underscore) with an underscore.
// Note that the first backslash in the regular expression is
// there for the compiler, so the pattern is actually \W
id.replace( QRegExp( "[\\W]" ), QStringLiteral( "_" ) );
return id;
}

void QgsMapLayer::setProviderType( const QString &providerType )
{
mProviderKey = providerType;
Expand Down
7 changes: 7 additions & 0 deletions src/core/qgsmaplayer.h
Expand Up @@ -1133,6 +1133,13 @@ class CORE_EXPORT QgsMapLayer : public QObject
*/
void setOriginalXmlProperties( const QString &originalXmlProperties );

/**
* Generates an unique identifier for this layer, the generate ID is prefixed by \a layerName
* \since QGIS 3.8
*/
static QString generateId( const QString &layerName );


public slots:

/**
Expand Down
4 changes: 3 additions & 1 deletion src/gui/editorwidgets/qgsvaluerelationconfigdlg.cpp
Expand Up @@ -18,6 +18,7 @@
#include "qgsvectorlayer.h"
#include "qgsexpressionbuilderdialog.h"
#include "qgsexpressioncontextutils.h"
#include "qgsvaluerelationfieldformatter.h"

QgsValueRelationConfigDlg::QgsValueRelationConfigDlg( QgsVectorLayer *vl, int fieldIdx, QWidget *parent )
: QgsEditorConfigWidget( vl, fieldIdx, parent )
Expand Down Expand Up @@ -55,6 +56,7 @@ QVariantMap QgsValueRelationConfigDlg::config()
QVariantMap cfg;

cfg.insert( QStringLiteral( "Layer" ), mLayerName->currentLayer() ? mLayerName->currentLayer()->id() : QString() );
cfg.insert( QStringLiteral( "LayerName" ), mLayerName->currentLayer() ? mLayerName->currentLayer()->name() : QString() );
cfg.insert( QStringLiteral( "Key" ), mKeyColumn->currentField() );
cfg.insert( QStringLiteral( "Value" ), mValueColumn->currentField() );
cfg.insert( QStringLiteral( "AllowMulti" ), mAllowMulti->isChecked() );
Expand All @@ -69,7 +71,7 @@ QVariantMap QgsValueRelationConfigDlg::config()

void QgsValueRelationConfigDlg::setConfig( const QVariantMap &config )
{
QgsVectorLayer *lyr = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config.value( QStringLiteral( "Layer" ) ).toString() );
QgsVectorLayer *lyr = QgsValueRelationFieldFormatter::resolveLayer( config );
mLayerName->setLayer( lyr );
mKeyColumn->setField( config.value( QStringLiteral( "Key" ) ).toString() );
mValueColumn->setField( config.value( QStringLiteral( "Value" ) ).toString() );
Expand Down
3 changes: 2 additions & 1 deletion src/gui/editorwidgets/qgsvaluerelationwidgetwrapper.cpp
Expand Up @@ -299,9 +299,10 @@ int QgsValueRelationWidgetWrapper::columnCount() const
return std::max( 1, config( QStringLiteral( "NofColumns" ) ).toInt() );
}


QVariant::Type QgsValueRelationWidgetWrapper::fkType() const
{
QgsVectorLayer *layer = QgsProject::instance()->mapLayer<QgsVectorLayer *>( config().value( QStringLiteral( "Layer" ) ).toString() );
const QgsVectorLayer *layer = QgsValueRelationFieldFormatter::resolveLayer( config() );
if ( layer )
{
QgsFields fields = layer->fields();
Expand Down
54 changes: 54 additions & 0 deletions tests/src/gui/testqgsvaluerelationwidgetwrapper.cpp
Expand Up @@ -53,6 +53,7 @@ class TestQgsValueRelationWidgetWrapper : public QObject
void testWithJsonInGPKG();
void testWithJsonInSpatialite();
void testWithJsonInSpatialiteTextFk();
void testMatchLayerName();
};

void TestQgsValueRelationWidgetWrapper::initTestCase()
Expand Down Expand Up @@ -921,5 +922,58 @@ void TestQgsValueRelationWidgetWrapper::testWithJsonInSpatialiteTextFk()

}

void TestQgsValueRelationWidgetWrapper::testMatchLayerName()
{
// create a vector layer
QgsVectorLayer vl1( QStringLiteral( "Polygon?crs=epsg:4326&field=pk:int&field=province:int&field=municipality:string" ), QStringLiteral( "vl1" ), QStringLiteral( "memory" ) );
QgsVectorLayer vl2( QStringLiteral( "Point?crs=epsg:4326&field=pk:int&field=fk_province:int&field=fk_municipality:int" ), QStringLiteral( "vl2" ), QStringLiteral( "memory" ) );
QgsProject::instance()->addMapLayer( &vl1, false, false );
QgsProject::instance()->addMapLayer( &vl2, false, false );

// insert some features
QgsFeature f1( vl1.fields() );
f1.setAttribute( QStringLiteral( "pk" ), 0 ); // !!! Notice: pk 0
f1.setAttribute( QStringLiteral( "province" ), 123 );
f1.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Some Place By The River" ) );
f1.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 0 0, 0 1, 1 1, 1 0, 0 0 ))" ) ) );
QVERIFY( f1.isValid() );
QgsFeature f2( vl1.fields() );
f2.setAttribute( QStringLiteral( "pk" ), 2 );
f2.setAttribute( QStringLiteral( "province" ), 245 );
f2.setAttribute( QStringLiteral( "municipality" ), QStringLiteral( "Dreamland By The Clouds" ) );
f2.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POLYGON(( 1 0, 1 1, 2 1, 2 0, 1 0 ))" ) ) );
QVERIFY( f2.isValid() );
QVERIFY( vl1.dataProvider()->addFeatures( QgsFeatureList() << f1 << f2 ) );

QgsFeature f3( vl2.fields() );
f3.setAttribute( QStringLiteral( "fk_province" ), 123 );
f3.setAttribute( QStringLiteral( "fk_municipality" ), QStringLiteral( "0" ) );
f3.setGeometry( QgsGeometry::fromWkt( QStringLiteral( "POINT( 0.5 0.5)" ) ) );
QVERIFY( f3.isValid() );
QVERIFY( f3.geometry().isGeosValid() );
QVERIFY( vl2.dataProvider()->addFeature( f3 ) );

// build a value relation widget wrapper for municipality
QgsValueRelationWidgetWrapper w_municipality( &vl2, vl2.fields().indexOf( QStringLiteral( "fk_municipality" ) ), nullptr, nullptr );
QVariantMap cfg_municipality;
cfg_municipality.insert( QStringLiteral( "Layer" ), QStringLiteral( "wrong_id_here_hope_name_is_good" ) );
cfg_municipality.insert( QStringLiteral( "LayerName" ), vl1.name() );
cfg_municipality.insert( QStringLiteral( "Key" ), QStringLiteral( "pk" ) );
cfg_municipality.insert( QStringLiteral( "Value" ), QStringLiteral( "municipality" ) );
cfg_municipality.insert( QStringLiteral( "AllowMulti" ), false );
cfg_municipality.insert( QStringLiteral( "NofColumns" ), 1 );
cfg_municipality.insert( QStringLiteral( "AllowNull" ), true );
cfg_municipality.insert( QStringLiteral( "OrderByValue" ), false );
cfg_municipality.insert( QStringLiteral( "UseCompleter" ), false );
w_municipality.setConfig( cfg_municipality );
w_municipality.widget();
w_municipality.setEnabled( true );

w_municipality.setValue( 0 );
QCOMPARE( w_municipality.mComboBox->currentIndex(), 1 );
QCOMPARE( w_municipality.mComboBox->currentText(), QStringLiteral( "Some Place By The River" ) );

}

QGSTEST_MAIN( TestQgsValueRelationWidgetWrapper )
#include "testqgsvaluerelationwidgetwrapper.moc"