[composer] Fix polylines would be removed if they had < 3 points

nyalldawson · nyalldawson · commit 38fa2980ad83 · 2016-04-01T14:23:19.000+11:00
when deleting nodes (2 node lines should be allowed)
diff --git a/python/core/composer/qgscomposernodesitem.sip b/python/core/composer/qgscomposernodesitem.sip
@@ -87,6 +87,9 @@ class QgsComposerNodesItem: QgsComposerItem
     /** Method called in addNode. */
     virtual bool _addNode( const int nodeIndex, const QPointF &newPoint, const double radius ) = 0;
 
+    /** Method called in removeNode. */
+    virtual bool _removeNode( const int nodeIndex ) = 0;
+
     /** Method called in paint. */
     virtual void _draw( QPainter *painter ) = 0;
 
diff --git a/python/core/composer/qgscomposerpolygon.sip b/python/core/composer/qgscomposerpolygon.sip
@@ -23,10 +23,10 @@ class QgsComposerPolygon: QgsComposerNodesItem
 
   protected:
 
-    /** Add the point newPoint at the given position according to some
-     * criteres. */
     bool _addNode( const int indexPoint, const QPointF &newPoint, const double radius );
 
+    bool _removeNode( const int indexPoint );
+
     /** Draw points for the current shape. */
     void _draw( QPainter *painter );
 
diff --git a/python/core/composer/qgscomposerpolyline.sip b/python/core/composer/qgscomposerpolyline.sip
@@ -23,10 +23,10 @@ class QgsComposerPolyline: QgsComposerNodesItem
 
   protected:
 
-    /** Add the point newPoint at the given position according to some
-     * criteres. */
     bool _addNode( const int indexPoint, const QPointF &newPoint, const double radius );
 
+    bool _removeNode( const int indexPoint );
+
     /** Draw points for the current shape. */
     void _draw( QPainter *painter );
 
diff --git a/src/core/composer/qgscomposernodesitem.cpp b/src/core/composer/qgscomposernodesitem.cpp
@@ -255,27 +255,9 @@ bool QgsComposerNodesItem::nodePosition( const int index, QPointF &position )
 
 bool QgsComposerNodesItem::removeNode( const int index )
 {
-  bool rc( false );
-
-  if ( index >= 0 && index < mPolygon.size() )
-  {
-    mPolygon.remove( index );
-
-    if ( mPolygon.size() < 3 )
-      mPolygon.clear();
-    else
-    {
-      int newSelectNode = index;
-      if ( index == mPolygon.size() )
-        newSelectNode = 0;
-      setSelectedNode( newSelectNode );
-    }
-
+  bool rc = _removeNode( index );
+  if ( rc )
     updateSceneRect();
-
-    rc = true;
-  }
-
   return rc;
 }
 
diff --git a/src/core/composer/qgscomposernodesitem.h b/src/core/composer/qgscomposernodesitem.h
@@ -128,6 +128,9 @@ class CORE_EXPORT QgsComposerNodesItem: public QgsComposerItem
     /** Method called in addNode. */
     virtual bool _addNode( const int nodeIndex, const QPointF &newNode, const double radius ) = 0;
 
+    /** Method called in removeNode. */
+    virtual bool _removeNode( const int nodeIndex ) = 0;
+
     /** Method called in paint. */
     virtual void _draw( QPainter *painter ) = 0;
 
diff --git a/src/core/composer/qgscomposerpolygon.cpp b/src/core/composer/qgscomposerpolygon.cpp
@@ -120,3 +120,23 @@ void QgsComposerPolygon::_writeXMLStyle( QDomDocument &doc, QDomElement &elmt )
                          doc );
   elmt.appendChild( pe );
 }
+
+bool QgsComposerPolygon::_removeNode( const int index )
+{
+  if ( index < 0 || index >= mPolygon.size() )
+    return false;
+
+  mPolygon.remove( index );
+
+  if ( mPolygon.size() < 3 )
+    mPolygon.clear();
+  else
+  {
+    int newSelectNode = index;
+    if ( index == mPolygon.size() )
+      newSelectNode = 0;
+    setSelectedNode( newSelectNode );
+  }
+
+  return true;
+}
diff --git a/src/core/composer/qgscomposerpolygon.h b/src/core/composer/qgscomposerpolygon.h
@@ -69,6 +69,8 @@ class CORE_EXPORT QgsComposerPolygon: public QgsComposerNodesItem
      * criteres. */
     bool _addNode( const int indexPoint, const QPointF &newPoint, const double radius ) override;
 
+    bool _removeNode( const int nodeIndex ) override;
+
     /** Draw nodes for the current shape. */
     void _draw( QPainter *painter ) override;
 
diff --git a/src/core/composer/qgscomposerpolyline.cpp b/src/core/composer/qgscomposerpolyline.cpp
@@ -59,6 +59,26 @@ bool QgsComposerPolyline::_addNode( const int indexPoint,
   return true;
 }
 
+bool QgsComposerPolyline::_removeNode( const int index )
+{
+  if ( index < 0 || index >= mPolygon.size() )
+    return false;
+
+  mPolygon.remove( index );
+
+  if ( mPolygon.size() < 2 )
+    mPolygon.clear();
+  else
+  {
+    int newSelectNode = index;
+    if ( index >= mPolygon.size() )
+      newSelectNode = mPolygon.size() - 1;
+    setSelectedNode( newSelectNode );
+  }
+
+  return true;
+}
+
 void QgsComposerPolyline::createDefaultPolylineStyleSymbol()
 {
   QgsStringMap properties;
diff --git a/src/core/composer/qgscomposerpolyline.h b/src/core/composer/qgscomposerpolyline.h
@@ -68,6 +68,8 @@ class CORE_EXPORT QgsComposerPolyline: public QgsComposerNodesItem
      * criteres. */
     bool _addNode( const int indexPoint, const QPointF &newPoint, const double radius ) override;
 
+    bool _removeNode( const int nodeIndex ) override;
+
     /** Draw nodes for the current shape. */
     void _draw( QPainter *painter ) override;
 
diff --git a/src/gui/qgscomposerview.cpp b/src/gui/qgscomposerview.cpp
@@ -1577,27 +1577,24 @@ void QgsComposerView::deleteSelectedItems()
     if ( mNodesItemIndex != -1 )
     {
       composition()->beginCommand( mNodesItem, tr( "Remove item node" ) );
-      bool rc = mNodesItem->removeNode( mNodesItemIndex );
-      composition()->endCommand();
-
-      bool nodeDeleted = true;
-      if ( rc )
+      if ( mNodesItem->removeNode( mNodesItemIndex ) )
       {
-        mNodesItemIndex = mNodesItem->selectedNode();
-
-        if ( mNodesItemIndex != -1 )
+        composition()->endCommand();
+        if ( mNodesItem->nodesSize() > 0 )
         {
-          nodeDeleted = false;
-          setSelectedNode( mNodesItem, mNodesItemIndex );
+          mNodesItemIndex = mNodesItem->selectedNode();
+          // setSelectedNode( mNodesItem, mNodesItemIndex );
+        }
+        else
+        {
+          mNodesItemIndex = -1;
+          mNodesItem = nullptr;
         }
+        scene()->update();
       }
-
-      if ( nodeDeleted )
+      else
       {
-        scene()->update();
-
-        mNodesItemIndex = -1;
-        mNodesItem = nullptr;
+        composition()->cancelCommand();
       }
     }
   }
diff --git a/tests/src/python/test_qgscomposerpolygon.py b/tests/src/python/test_qgscomposerpolygon.py
@@ -139,15 +139,6 @@ def testRemoveNode(self):
         assert myTestResult, myMessage
 
         self.assertEqual(self.mComposerPolygon.nodesSize(), 4)
-        rc = self.mComposerPolygon.removeNode(3)
-        self.assertEqual(rc, True)
-        self.assertEqual(self.mComposerPolygon.nodesSize(), 3)
-
-        checker = QgsCompositionChecker(
-            'composerpolygon_removednode', self.mComposition)
-        checker.setControlPathPrefix("composer_polygon")
-        myTestResult, myMessage = checker.testComposition()
-        assert myTestResult, myMessage
 
     def testAddNode(self):
         """Test addNode method"""

Original file line number	Diff line number	Diff line change
`@@ -1577,27 +1577,24 @@ void QgsComposerView::deleteSelectedItems()`
`1577`	`1577`	`if ( mNodesItemIndex != -1 )`
`1578`	`1578`	`{`
`1579`	`1579`	`composition()->beginCommand( mNodesItem, tr( "Remove item node" ) );`
`1580`		`- bool rc = mNodesItem->removeNode( mNodesItemIndex );`
`1581`		`- composition()->endCommand();`
`1582`		`-`
`1583`		`- bool nodeDeleted = true;`
`1584`		`- if ( rc )`
	`1580`	`+ if ( mNodesItem->removeNode( mNodesItemIndex ) )`
`1585`	`1581`	`{`
`1586`		`- mNodesItemIndex = mNodesItem->selectedNode();`
`1587`		`-`
`1588`		`- if ( mNodesItemIndex != -1 )`
	`1582`	`+ composition()->endCommand();`
	`1583`	`+ if ( mNodesItem->nodesSize() > 0 )`
`1589`	`1584`	`{`
`1590`		`- nodeDeleted = false;`
`1591`		`- setSelectedNode( mNodesItem, mNodesItemIndex );`
	`1585`	`+ mNodesItemIndex = mNodesItem->selectedNode();`
	`1586`	`+ // setSelectedNode( mNodesItem, mNodesItemIndex );`
	`1587`	`+ }`
	`1588`	`+ else`
	`1589`	`+ {`
	`1590`	`+ mNodesItemIndex = -1;`
	`1591`	`+ mNodesItem = nullptr;`
`1592`	`1592`	`}`
	`1593`	`+ scene()->update();`
`1593`	`1594`	`}`
`1594`		`-`
`1595`		`- if ( nodeDeleted )`
	`1595`	`+ else`
`1596`	`1596`	`{`
`1597`		`- scene()->update();`
`1598`		`-`
`1599`		`- mNodesItemIndex = -1;`
`1600`		`- mNodesItem = nullptr;`
	`1597`	`+ composition()->cancelCommand();`
`1601`	`1598`	`}`
`1602`	`1599`	`}`
`1603`	`1600`	`}`