Handle linking items when adding from templates

qgis · Dec 6, 2017 · 885f0a5 · 885f0a5
1 parent a4dea99
commit 885f0a5
Show file tree

Hide file tree

Showing 9 changed files with 53 additions and 21 deletions.
diff --git a/src/core/layout/qgslayout.cpp b/src/core/layout/qgslayout.cpp
@@ -213,16 +213,16 @@ bool QgsLayout::moveItemToBottom( QgsLayoutItem *item, bool deferUpdate )
   return result;
 }
 
-QgsLayoutItem *QgsLayout::itemByUuid( const QString &uuid )
+QgsLayoutItem *QgsLayout::itemByUuid( const QString &uuid, bool includeTemplateUuids )
 {
   QList<QgsLayoutItem *> itemList;
   layoutItems( itemList );
-  Q_FOREACH ( QgsLayoutItem *item, itemList )
+  for ( QgsLayoutItem *item : qgis::as_const( itemList ) )
   {
     if ( item->uuid() == uuid )
-    {
       return item;
-    }
+    else if ( includeTemplateUuids && item->templateUuid() == uuid )
+      return item;
   }
 
   return nullptr;
@@ -503,14 +503,13 @@ QList< QgsLayoutItem * > QgsLayout::loadFromTemplate( const QDomDocument &docume
   QDomDocument doc = document;
 
   // remove all uuid attributes since we don't want duplicates UUIDS
-  QDomNodeList composerItemsNodes = doc.elementsByTagName( QStringLiteral( "ComposerItem" ) );
-  for ( int i = 0; i < composerItemsNodes.count(); ++i )
+  QDomNodeList itemsNodes = doc.elementsByTagName( QStringLiteral( "LayoutItem" ) );
+  for ( int i = 0; i < itemsNodes.count(); ++i )
   {
-    QDomNode composerItemNode = composerItemsNodes.at( i );
-    if ( composerItemNode.isElement() )
+    QDomNode itemNode = itemsNodes.at( i );
+    if ( itemNode.isElement() )
     {
-      composerItemNode.toElement().setAttribute( QStringLiteral( "templateUuid" ), composerItemNode.toElement().attribute( QStringLiteral( "uuid" ) ) );
-      composerItemNode.toElement().removeAttribute( QStringLiteral( "uuid" ) );
+      itemNode.toElement().removeAttribute( QStringLiteral( "uuid" ) );
     }
   }
 

diff --git a/src/core/layout/qgslayout.h b/src/core/layout/qgslayout.h
@@ -215,9 +215,13 @@ class CORE_EXPORT QgsLayout : public QGraphicsScene, public QgsExpressionContext
     /**
      * Returns the layout item with matching \a uuid unique identifier, or a nullptr
      * if a matching item could not be found.
+     *
+     * If \a includeTemplateUuids is true, then item's QgsLayoutItem::templateUuid()
+     * will also be tested when trying to match the uuid.
+     *
      * \see multiFrameByUuid()
      */
-    QgsLayoutItem *itemByUuid( const QString &uuid );
+    QgsLayoutItem *itemByUuid( const QString &uuid, bool includeTemplateUuids = false );
 
     /**
      * Returns the layout multiframe with matching \a uuid unique identifier, or a nullptr

diff --git a/src/core/layout/qgslayoutitem.cpp b/src/core/layout/qgslayoutitem.cpp
@@ -527,6 +527,7 @@ bool QgsLayoutItem::writeXml( QDomElement &parentElement, QDomDocument &doc, con
   element.setAttribute( QStringLiteral( "type" ), QString::number( type() ) );
 
   element.setAttribute( QStringLiteral( "uuid" ), mUuid );
+  element.setAttribute( QStringLiteral( "templateUuid" ), mUuid );
   element.setAttribute( QStringLiteral( "id" ), mId );
   element.setAttribute( QStringLiteral( "referencePoint" ), QString::number( static_cast< int >( mReferencePoint ) ) );
   element.setAttribute( QStringLiteral( "position" ), mItemPosition.encodePoint() );
@@ -626,12 +627,7 @@ bool QgsLayoutItem::readXml( const QDomElement &element, const QDomDocument &doc
       group->addItem( this );
     }
   }
-
-  //TODO
-  /*
-  // temporary for groups imported from templates
-  mTemplateUuid = itemElem.attribute( "templateUuid" );
-  */
+  mTemplateUuid = element.attribute( "templateUuid" );
 
   //position lock for mouse moves/resizes
   QString positionLock = element.attribute( "positionLock" );

diff --git a/src/core/layout/qgslayoutitem.h b/src/core/layout/qgslayoutitem.h
@@ -237,9 +237,19 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      * \note There is no corresponding setter for the uuid - it's created automatically.
      * \see id()
      * \see setId()
+     * \see templateUuid()
     */
     virtual QString uuid() const { return mUuid; }
 
+    /**
+     * Returns the item's original identification string. This may differ from the item's uuid()
+     * for items which have been added to an existing layout from a template. In this case
+     * templateUuid() returns the original item UUID at the time the template was created,
+     * while uuid() returns the current instance of the item's unique identifier.
+     * \see uuid()
+    */
+    QString templateUuid() const { return mTemplateUuid; }
+
     /**
      * Returns the item's ID name. This is not necessarily unique, and duplicate ID names may exist
      * for a layout.
@@ -975,6 +985,9 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
     //! Unique id
     QString mUuid;
 
+    //! Original uuid from template
+    QString mTemplateUuid;
+
     //! Parent group unique id
     QString mParentGroupUuid;
 

diff --git a/src/core/layout/qgslayoutitemattributetable.cpp b/src/core/layout/qgslayoutitemattributetable.cpp
@@ -721,7 +721,7 @@ bool QgsLayoutItemAttributeTable::readPropertiesFromElement( const QDomElement &
   }
   else
   {
-    mMap = qobject_cast< QgsLayoutItemMap *>( mLayout->itemByUuid( mapUuid ) );
+    mMap = qobject_cast< QgsLayoutItemMap *>( mLayout->itemByUuid( mapUuid, true ) );
   }
 
   if ( mMap )

diff --git a/src/core/layout/qgslayoutitemlegend.cpp b/src/core/layout/qgslayoutitemlegend.cpp
@@ -130,7 +130,7 @@ void QgsLayoutItemLegend::finalizeRestoreFromXml()
 #endif
   if ( !mMapUuid.isEmpty() )
   {
-    setMap( qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mMapUuid ) ) );
+    setMap( qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mMapUuid, true ) ) );
   }
 }
 

diff --git a/src/core/layout/qgslayoutitempicture.cpp b/src/core/layout/qgslayoutitempicture.cpp
@@ -841,7 +841,7 @@ void QgsLayoutItemPicture::finalizeRestoreFromXml()
       disconnect( mRotationMap, &QgsLayoutItemMap::mapRotationChanged, this, &QgsLayoutItemPicture::updateMapRotation );
       disconnect( mRotationMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemPicture::updateMapRotation );
     }
-    mRotationMap = qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mRotationMapUuid ) );
+    mRotationMap = qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mRotationMapUuid, true ) );
     if ( mRotationMap )
     {
       connect( mRotationMap, &QgsLayoutItemMap::mapRotationChanged, this, &QgsLayoutItemPicture::updateMapRotation );

diff --git a/src/core/layout/qgslayoutitemscalebar.cpp b/src/core/layout/qgslayoutitemscalebar.cpp
@@ -810,7 +810,7 @@ void QgsLayoutItemScaleBar::finalizeRestoreFromXml()
   else
   {
     disconnectCurrentMap();
-    mMap = qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mMapUuid ) );
+    mMap = qobject_cast< QgsLayoutItemMap * >( mLayout->itemByUuid( mMapUuid, true ) );
     if ( mMap )
     {
       connect( mMap, &QgsLayoutItemMap::extentChanged, this, &QgsLayoutItemScaleBar::updateScale );

diff --git a/tests/src/python/test_qgslayout.py b/tests/src/python/test_qgslayout.py
@@ -235,6 +235,9 @@ def testSaveLoadTemplate(self):
         item2.attemptResize(QgsLayoutSize(2.8, 2.2, QgsUnitTypes.LayoutCentimeters))
         l.addItem(item2)
 
+        uuids = {item1.uuid(), item2.uuid()}
+        original_uuids = {item1.uuid(), item2.uuid()}
+
         self.assertTrue(l.saveAsTemplate(tmpfile, QgsReadWriteContext()))
 
         l2 = QgsLayout(p)
@@ -252,6 +255,13 @@ def testSaveLoadTemplate(self):
         self.assertTrue([i for i in items if i.id() == 'zzyyzz'])
         self.assertTrue(new_items[0] in l2.items())
         self.assertTrue(new_items[1] in l2.items())
+        # double check that new items have a unique uid
+        self.assertNotIn(new_items[0].uuid(), uuids)
+        self.assertIn(new_items[0].templateUuid(), original_uuids)
+        uuids.add(new_items[0].uuid())
+        self.assertNotIn(new_items[1].uuid(), uuids)
+        self.assertIn(new_items[1].templateUuid(), original_uuids)
+        uuids.add(new_items[1].uuid())
 
         # adding to existing items
         new_items2, ok = l2.loadFromTemplate(doc, QgsReadWriteContext(), False)
@@ -265,6 +275,12 @@ def testSaveLoadTemplate(self):
         self.assertTrue(new_items[1] in l2.items())
         self.assertTrue(new_items2[0] in l2.items())
         self.assertTrue(new_items2[1] in l2.items())
+        self.assertNotIn(new_items2[0].uuid(), uuids)
+        self.assertIn(new_items2[0].templateUuid(), original_uuids)
+        uuids.add(new_items[0].uuid())
+        self.assertNotIn(new_items2[1].uuid(), uuids)
+        self.assertIn(new_items2[1].templateUuid(), original_uuids)
+        uuids.add(new_items[1].uuid())
 
         # clearing existing items
         new_items3, ok = l2.loadFromTemplate(doc, QgsReadWriteContext(), True)
@@ -276,6 +292,10 @@ def testSaveLoadTemplate(self):
         self.assertTrue([i for i in items if i.id() == 'zzyyzz'])
         self.assertTrue(new_items3[0] in l2.items())
         self.assertTrue(new_items3[1] in l2.items())
+        self.assertIn(new_items3[0].templateUuid(), original_uuids)
+        self.assertIn(new_items3[1].templateUuid(), original_uuids)
+        self.assertEqual(l2.itemByUuid(new_items3[0].templateUuid(), True), new_items3[0])
+        self.assertEqual(l2.itemByUuid(new_items3[1].templateUuid(), True), new_items3[1])
 
     def testSelectedItems(self):
         p = QgsProject()