Skip to content

Commit

Permalink
Add a changed signal to QgsRelationManager
Browse files Browse the repository at this point in the history
  • Loading branch information
@nyalldawson
nyalldawson committed Sep 20, 2014
1 parent 808464f commit fb2279f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 1 deletion.
6 changes: 6 additions & 0 deletions python/core/qgsrelationmanager.sip
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,4 +22,10 @@ class QgsRelationManager : QObject

signals:
void relationsLoaded();

/**
* Emitted when relations are added or removed to the manager.
* @note added in QGIS 2.5
*/
void changed();
};
14 changes: 13 additions & 1 deletion src/core/qgsrelationmanager.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,7 @@ void QgsRelationManager::setRelations( const QList<QgsRelation>& relations )
{
addRelation( rel );
}
emit changed();
}

const QMap<QString, QgsRelation>& QgsRelationManager::relations() const
Expand All @@ -52,16 +53,19 @@ void QgsRelationManager::addRelation( const QgsRelation& relation )
mRelations.insert( relation.id(), relation );

mProject->dirty( true );
emit changed();
}

void QgsRelationManager::removeRelation( const QString& name )
{
mRelations.remove( name );
emit changed();
}

void QgsRelationManager::removeRelation( const QgsRelation& relation )
{
mRelations.remove( relation.id() );
emit changed();
}

QgsRelation QgsRelationManager::relation( const QString& id ) const
Expand All @@ -72,6 +76,7 @@ QgsRelation QgsRelationManager::relation( const QString& id ) const
void QgsRelationManager::clear()
{
mRelations.clear();
emit changed();
}

QList<QgsRelation> QgsRelationManager::referencingRelations( QgsVectorLayer* layer, int fieldIdx ) const
Expand Down Expand Up @@ -152,6 +157,7 @@ void QgsRelationManager::readProject( const QDomDocument & doc )
}

emit( relationsLoaded() );
emit changed();
}

void QgsRelationManager::writeProject( QDomDocument & doc )
Expand All @@ -175,7 +181,8 @@ void QgsRelationManager::writeProject( QDomDocument & doc )

void QgsRelationManager::layersRemoved( const QStringList& layers )
{
Q_FOREACH ( const QString& layer, layers )
bool relationsChanged = false;
Q_FOREACH( const QString& layer, layers )
{
QMapIterator<QString, QgsRelation> it( mRelations );

Expand All @@ -187,7 +194,12 @@ void QgsRelationManager::layersRemoved( const QStringList& layers )
|| it.value().referencingLayerId() == layer )
{
mRelations.remove( it.key() );
relationsChanged = true;
}
}
}
if ( relationsChanged )
{
emit changed();
}
}
6 changes: 6 additions & 0 deletions src/core/qgsrelationmanager.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,12 @@ class CORE_EXPORT QgsRelationManager : public QObject
signals:
void relationsLoaded();

/**
* Emitted when relations are added or removed to the manager.
* @note added in QGIS 2.5
*/
void changed();

private slots:
void readProject( const QDomDocument &doc );
void writeProject( QDomDocument &doc );
Expand Down

4 comments on commit fb2279f

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on fb2279f Sep 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you think about relationAdded( const QgsRelation& ) and relationRemoved( id ) signals? This would allow for more specific reactions.

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn they would definitely be useful, but I think there's merit in an all-encompassing "changed" signal too.

@m-kuhn
Copy link
Member

@m-kuhn m-kuhn commented on fb2279f Sep 22, 2014

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@nyalldawson I agree, there are use-cases for this as well.
I just fear that people will start to implement the logic to calculate the delta of the change operation all over the place (or will get it wrong and connect multiple times to signals of already existing relations).
Would it be possible to add the other signals as well?

@nyalldawson
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@m-kuhn ahh... I thought you were volunteering to do this ;) The biggest issue is with QgsRelationManager::setRelations, as that method clears the existing relations and totally rebuilds them. This makes it harder to correctly emit removed/added signals, as you'd need to manually check whether each has been added or removed before clearing the list. Unfortunately this is the method which the project properties dialog uses to set the relations. It's more work to tackle than I've got time left for the 2.6 freeze!

Please sign in to comment.