[composer] Move ungrouping logic from composer view to composition

As per previous commit, this is useful for plugin authors. It also fixes a potential bad crash (itemRemoved emitted for group item after the group item was already deleted) and adds unit tests.
qgis · Oct 19, 2014 · 0f4bd4e · 0f4bd4e
1 parent 993f0dc
commit 0f4bd4e
Show file tree

Hide file tree

Showing 5 changed files with 72 additions and 11 deletions.
diff --git a/python/core/composer/qgscomposition.sip b/python/core/composer/qgscomposition.sip
@@ -415,6 +415,15 @@ class QgsComposition : QGraphicsScene
     */
     QgsComposerItemGroup* groupItems( QList<QgsComposerItem*> items );
 
+    /**Ungroups items by removing them from an item group and removing the group from the
+     * composition.
+     * @param group item group to ungroup
+     * @returns list of items removed from the group, or an empty list if ungrouping
+     * was not successful
+     * @note added in QGIS 2.6
+    */
+    QList<QgsComposerItem*> ungroupItems( QgsComposerItemGroup* group );
+
     /**Sorts the zList. The only time where this function needs to be called is from QgsComposer
      * after reading all the items from xml file
      * @deprecated use refreshZList instead

diff --git a/src/core/composer/qgscomposition.cpp b/src/core/composer/qgscomposition.cpp
@@ -1811,6 +1811,30 @@ QgsComposerItemGroup *QgsComposition::groupItems( QList<QgsComposerItem *> items
   return itemGroup;
 }
 
+QList<QgsComposerItem *> QgsComposition::ungroupItems( QgsComposerItemGroup* group )
+{
+  QList<QgsComposerItem *> ungroupedItems;
+  if ( !group )
+  {
+    return ungroupedItems;
+  }
+
+  QSet<QgsComposerItem*> groupedItems = group->items();
+  QSet<QgsComposerItem*>::iterator itemIt = groupedItems.begin();
+  for ( ; itemIt != groupedItems.end(); ++itemIt )
+  {
+    ungroupedItems << ( *itemIt );
+  }
+
+  group->removeItems();
+  removeComposerItem( group, false, false );
+
+  emit itemRemoved( group );
+  delete( group );
+
+  return ungroupedItems;
+}
+
 void QgsComposition::updateZValues( const bool addUndoCommands )
 {
   int counter = mItemsModel->zOrderListSize();

diff --git a/src/core/composer/qgscomposition.h b/src/core/composer/qgscomposition.h
@@ -474,10 +474,19 @@ class CORE_EXPORT QgsComposition : public QGraphicsScene
     /**Creates a new group from a list of composer items and adds it to the composition.
      * @param items items to include in group
      * @returns QgsComposerItemGroup of grouped items, if grouping was possible
-     * @note adde in QGIS 2.6
+     * @note added in QGIS 2.6
     */
     QgsComposerItemGroup* groupItems( QList<QgsComposerItem*> items );
 
+    /**Ungroups items by removing them from an item group and removing the group from the
+     * composition.
+     * @param group item group to ungroup
+     * @returns list of items removed from the group, or an empty list if ungrouping
+     * was not successful
+     * @note added in QGIS 2.6
+    */
+    QList<QgsComposerItem*> ungroupItems( QgsComposerItemGroup* group );
+
     /**Sorts the zList. The only time where this function needs to be called is from QgsComposer
      * after reading all the items from xml file
      * @deprecated use refreshZList instead

diff --git a/src/gui/qgscomposerview.cpp b/src/gui/qgscomposerview.cpp
@@ -1798,18 +1798,15 @@ void QgsComposerView::ungroupItems()
     return;
   }
 
+  //hunt through selection for any groups, and ungroup them
   QList<QgsComposerItem*> selectionList = composition()->selectedComposerItems();
   QList<QgsComposerItem*>::iterator itemIter = selectionList.begin();
   for ( ; itemIter != selectionList.end(); ++itemIter )
   {
     QgsComposerItemGroup* itemGroup = dynamic_cast<QgsComposerItemGroup *>( *itemIter );
     if ( itemGroup )
     {
-      itemGroup->removeItems();
-      composition()->removeComposerItem( *itemIter, false, false );
-
-      delete( *itemIter );
-      emit itemRemoved( *itemIter );
+      composition()->ungroupItems( itemGroup );
     }
   }
 }

diff --git a/tests/src/core/testqgscomposergroup.cpp b/tests/src/core/testqgscomposergroup.cpp
@@ -40,6 +40,7 @@ class TestQgsComposerGroup: public QObject
     QgsMapSettings mMapSettings;
     QgsComposerLabel* mItem1;
     QgsComposerLabel* mItem2;
+    QgsComposerItemGroup* mGroup;
     QString mReport;
 };
 
@@ -54,6 +55,8 @@ void TestQgsComposerGroup::initTestCase()
   mItem2 = new QgsComposerLabel( mComposition );
   mComposition->addItem( mItem2 );
 
+  mGroup = 0;
+
   mReport = "<h1>Composer Grouped Item Tests</h1>\n";
 }
 
@@ -86,20 +89,39 @@ void TestQgsComposerGroup::createGroup()
   //group items
   QList<QgsComposerItem*> items;
   items << mItem1 << mItem2;
-  QgsComposerItemGroup* group = mComposition->groupItems( items );
+  mGroup = mComposition->groupItems( items );
 
   //check result
-  QVERIFY( group );
-  QCOMPARE( group->items().size(), 2 );
-  QVERIFY( group->items().contains( mItem1 ) );
-  QVERIFY( group->items().contains( mItem2 ) );
+  QVERIFY( mGroup );
+  QCOMPARE( mGroup->items().size(), 2 );
+  QVERIFY( mGroup->items().contains( mItem1 ) );
+  QVERIFY( mGroup->items().contains( mItem2 ) );
   QVERIFY( mItem1->isGroupMember() );
   QVERIFY( mItem2->isGroupMember() );
 }
 
 void TestQgsComposerGroup::ungroup()
 {
+  //test ungrouping items
+
+  //simple tests - check that we don't crash
+  mComposition->ungroupItems( 0 ); //no item
+
+  //ungroup mGroup
+  QList<QgsComposerItem*> ungroupedItems;
+  ungroupedItems = mComposition->ungroupItems( mGroup );
+
+  QCOMPARE( ungroupedItems.size(), 2 );
+  QVERIFY( ungroupedItems.contains( mItem1 ) );
+  QVERIFY( ungroupedItems.contains( mItem2 ) );
+
+  QVERIFY( !mItem1->isGroupMember() );
+  QVERIFY( !mItem2->isGroupMember() );
 
+  //should also be no groups left in the composition
+  QList<QgsComposerItemGroup*> groups;
+  mComposition->composerItems( groups );
+  QCOMPARE( groups.size(), 0 );
 }
 
 void TestQgsComposerGroup::deleteGroup()