[composer] Add a basic test suite for QgsComposerModel

nyalldawson · nyalldawson · commit 1c03067504c0 · 2014-09-29T19:50:46.000+10:00
diff --git a/src/core/composer/qgscomposerlabel.cpp b/src/core/composer/qgscomposerlabel.cpp
@@ -61,10 +61,12 @@ QgsComposerLabel::QgsComposerLabel( QgsComposition *composition ):
     setExpressionContext( mComposition->atlasComposition().currentFeature(), mComposition->atlasComposition().coverageLayer() );
   }
 
-  //connect to atlas feature changes
-  //to update the expression context
-  connect( &mComposition->atlasComposition(), SIGNAL( featureChanged( QgsFeature* ) ), this, SLOT( refreshExpressionContext() ) );
-
+  if ( mComposition )
+  {
+    //connect to atlas feature changes
+    //to update the expression context
+    connect( &mComposition->atlasComposition(), SIGNAL( featureChanged( QgsFeature* ) ), this, SLOT( refreshExpressionContext() ) );
+  }
 }
 
 QgsComposerLabel::~QgsComposerLabel()
diff --git a/src/core/composer/qgscomposermodel.cpp b/src/core/composer/qgscomposermodel.cpp
@@ -693,6 +693,11 @@ void QgsComposerModel::updateItemSelectStatus( QgsComposerItem *item )
 
 bool QgsComposerModel::reorderItemUp( QgsComposerItem *item )
 {
+  if ( !item )
+  {
+    return false;
+  }
+
   if ( mItemsInScene.first() == item )
   {
     //item is already topmost item present in scene, nothing to do
@@ -738,6 +743,11 @@ bool QgsComposerModel::reorderItemUp( QgsComposerItem *item )
 
 bool QgsComposerModel::reorderItemDown( QgsComposerItem *item )
 {
+  if ( !item )
+  {
+    return false;
+  }
+
   if ( mItemsInScene.last() == item )
   {
     //item is already lowest item present in scene, nothing to do
@@ -783,6 +793,11 @@ bool QgsComposerModel::reorderItemDown( QgsComposerItem *item )
 
 bool QgsComposerModel::reorderItemToTop( QgsComposerItem *item )
 {
+  if ( !item || !mItemsInScene.contains( item ) )
+  {
+    return false;
+  }
+
   if ( mItemsInScene.first() == item )
   {
     //item is already topmost item present in scene, nothing to do
@@ -814,6 +829,11 @@ bool QgsComposerModel::reorderItemToTop( QgsComposerItem *item )
 
 bool QgsComposerModel::reorderItemToBottom( QgsComposerItem *item )
 {
+  if ( !item || !mItemsInScene.contains( item ) )
+  {
+    return false;
+  }
+
   if ( mItemsInScene.last() == item )
   {
     //item is already lowest item present in scene, nothing to do
diff --git a/src/core/composer/qgscomposermodel.h b/src/core/composer/qgscomposermodel.h
@@ -237,6 +237,14 @@ class CORE_EXPORT QgsComposerModel: public QAbstractItemModel
      */
     void setSelected( const QModelIndex &index );
 
+  protected:
+
+    /**Maintains z-Order of items. Starts with item at position 1 (position 0 is always paper item)*/
+    QList<QgsComposerItem*> mItemZList;
+
+    /**Cached list of items from mItemZList which are currently in the scene*/
+    QList<QgsComposerItem*> mItemsInScene;
+
   private:
 
     enum Columns
@@ -249,12 +257,6 @@ class CORE_EXPORT QgsComposerModel: public QAbstractItemModel
     /**Parent composition*/
     QgsComposition* mComposition;
 
-    /**Maintains z-Order of items. Starts with item at position 1 (position 0 is always paper item)*/
-    QList<QgsComposerItem*> mItemZList;
-
-    /**Cached list of items from mItemZList which are currently in the scene*/
-    QList<QgsComposerItem*> mItemsInScene;
-
     /**Returns the QgsComposerItem corresponding to a QModelIndex, if possible
      * @param index QModelIndex for item
      * @returns item corresponding to index
@@ -283,6 +285,8 @@ class CORE_EXPORT QgsComposerModel: public QAbstractItemModel
      * @see refreshItemsInScene
     */
     void rebuildSceneItemList();
+
+    friend class TestQgsComposerModel;
 };
 
 #endif //QGSCOMPOSERMODEL
diff --git a/tests/src/core/CMakeLists.txt b/tests/src/core/CMakeLists.txt
@@ -108,6 +108,7 @@ ADD_QGIS_TEST(dataitemtest testqgsdataitem.cpp)
 ADD_QGIS_TEST(composerobject testqgscomposerobject.cpp)
 ADD_QGIS_TEST(composerutils testqgscomposerutils.cpp)
 ADD_QGIS_TEST(compositiontest testqgscomposition.cpp)
+ADD_QGIS_TEST(composermodel testqgscomposermodel.cpp)
 ADD_QGIS_TEST(composermultiframetest testqgscomposermultiframe.cpp)
 ADD_QGIS_TEST(composerpapertest testqgscomposerpaper.cpp)
 ADD_QGIS_TEST(composermaptest testqgscomposermap.cpp)
diff --git a/tests/src/core/testqgscomposermodel.cpp b/tests/src/core/testqgscomposermodel.cpp

Original file line number	Diff line number	Diff line change
`@@ -693,6 +693,11 @@ void QgsComposerModel::updateItemSelectStatus( QgsComposerItem *item )`
`693`	`693`
`694`	`694`	`bool QgsComposerModel::reorderItemUp( QgsComposerItem *item )`
`695`	`695`	`{`
	`696`	`+ if ( !item )`
	`697`	`+ {`
	`698`	`+ return false;`
	`699`	`+ }`
	`700`	`+`
`696`	`701`	`if ( mItemsInScene.first() == item )`
`697`	`702`	`{`
`698`	`703`	`//item is already topmost item present in scene, nothing to do`
`@@ -738,6 +743,11 @@ bool QgsComposerModel::reorderItemUp( QgsComposerItem *item )`
`738`	`743`
`739`	`744`	`bool QgsComposerModel::reorderItemDown( QgsComposerItem *item )`
`740`	`745`	`{`
	`746`	`+ if ( !item )`
	`747`	`+ {`
	`748`	`+ return false;`
	`749`	`+ }`
	`750`	`+`
`741`	`751`	`if ( mItemsInScene.last() == item )`
`742`	`752`	`{`
`743`	`753`	`//item is already lowest item present in scene, nothing to do`
`@@ -783,6 +793,11 @@ bool QgsComposerModel::reorderItemDown( QgsComposerItem *item )`
`783`	`793`
`784`	`794`	`bool QgsComposerModel::reorderItemToTop( QgsComposerItem *item )`
`785`	`795`	`{`
	`796`	`+ if ( !item \|\| !mItemsInScene.contains( item ) )`
	`797`	`+ {`
	`798`	`+ return false;`
	`799`	`+ }`
	`800`	`+`
`786`	`801`	`if ( mItemsInScene.first() == item )`
`787`	`802`	`{`
`788`	`803`	`//item is already topmost item present in scene, nothing to do`
`@@ -814,6 +829,11 @@ bool QgsComposerModel::reorderItemToTop( QgsComposerItem *item )`
`814`	`829`
`815`	`830`	`bool QgsComposerModel::reorderItemToBottom( QgsComposerItem *item )`
`816`	`831`	`{`
	`832`	`+ if ( !item \|\| !mItemsInScene.contains( item ) )`
	`833`	`+ {`
	`834`	`+ return false;`
	`835`	`+ }`
	`836`	`+`
`817`	`837`	`if ( mItemsInScene.last() == item )`
`818`	`838`	`{`
`819`	`839`	`//item is already lowest item present in scene, nothing to do`