More work on layout multiframe undo/redo

python/core/layout/qgslayoutframe.sip

@@ -37,6 +37,8 @@ class QgsLayoutFrame: QgsLayoutItem
 
     virtual QString stringType() const;
 
+    virtual QString uuid() const;
+
 
     virtual QString displayName() const;
 

python/core/layout/qgslayoutitem.sip

@@ -135,7 +135,7 @@ class QgsLayoutItem : QgsLayoutObject, QGraphicsRectItem, QgsLayoutUndoObjectInt
  :rtype: str
 %End
 
-    QString uuid() const;
+    virtual QString uuid() const;
 %Docstring
  Returns the item identification string. This is a unique random string set for the item
  upon creation.

src/core/layout/qgslayoutframe.cpp

@@ -21,6 +21,7 @@
 QgsLayoutFrame::QgsLayoutFrame( QgsLayout *layout, QgsLayoutMultiFrame *multiFrame )
   : QgsLayoutItem( layout )
   , mMultiFrame( multiFrame )
+  , mMultiFrameUuid( multiFrame ? multiFrame->uuid() : QString() )
 {
 
   //default to no background
@@ -93,6 +94,14 @@ QString QgsLayoutFrame::stringType() const
   return QStringLiteral( "ItemFrame" );
 }
 
+QString QgsLayoutFrame::uuid() const
+{
+  if ( mMultiFrame )
+    return mMultiFrame->uuid() + ':' + mMultiFrame->frameIndex( const_cast< QgsLayoutFrame * >( this ) );
+  else
+    return QgsLayoutItem::uuid();
+}
+
 void QgsLayoutFrame::setHidePageIfEmpty( const bool hidePageIfEmpty )
 {
   mHidePageIfEmpty = hidePageIfEmpty;

src/core/layout/qgslayoutframe.h

@@ -47,6 +47,7 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
 
     int type() const override;
     QString stringType() const override;
+    QString uuid() const override;
 
     //Overridden to allow multiframe to set display name
     QString displayName() const override;
@@ -126,6 +127,7 @@ class CORE_EXPORT QgsLayoutFrame: public QgsLayoutItem
   private:
     QgsLayoutFrame() = delete;
     QgsLayoutMultiFrame *mMultiFrame = nullptr;
+    QString mMultiFrameUuid;
     QRectF mSection;
 
     //! If true, layout will not export page if this frame is empty

src/core/layout/qgslayoutitem.h

@@ -172,7 +172,7 @@ class CORE_EXPORT QgsLayoutItem : public QgsLayoutObject, public QGraphicsRectIt
      * \see id()
      * \see setId()
     */
-    QString uuid() const { return mUuid; }
+    virtual QString uuid() const { return mUuid; }
 
     /**
      * Returns the item's ID name. This is not necessarily unique, and duplicate ID names may exist

src/gui/layout/qgslayoutviewtooladditem.cpp

@@ -84,9 +84,23 @@ void QgsLayoutViewToolAddItem::layoutReleaseEvent( QgsLayoutViewMouseEvent *even
 
   QRectF rect = mRubberBand->finish( event->snappedPoint(), event->modifiers() );
 
+  QString undoText;
+  if ( QgsLayoutItemAbstractGuiMetadata *metadata = QgsGui::layoutItemGuiRegistry()->itemMetadata( mItemMetadataId ) )
+  {
+    undoText = tr( "Create %1" ).arg( metadata->visibleName() );
+  }
+  else
+  {
+    undoText = tr( "Create Item" );
+  }
+  layout()->undoStack()->beginMacro( undoText );
+
   QgsLayoutItem *item = QgsGui::layoutItemGuiRegistry()->createItem( mItemMetadataId, layout() );
   if ( !item )
+  {
+    layout()->undoStack()->endMacro();
     return;
+  }
 
   // click? or click-and-drag?
   bool clickOnly = !isClickAndDrag( mMousePressStartPos, event->pos() );
@@ -104,6 +118,7 @@ void QgsLayoutViewToolAddItem::layoutReleaseEvent( QgsLayoutViewMouseEvent *even
     else
     {
       delete item;
+      layout()->undoStack()->endMacro();
       return;
     }
   }
@@ -126,6 +141,8 @@ void QgsLayoutViewToolAddItem::layoutReleaseEvent( QgsLayoutViewMouseEvent *even
   if ( item->layout() != layout() )
     layout()->addLayoutItem( item );
   layout()->setSelectedItem( item );
+
+  layout()->undoStack()->endMacro();
   emit createdItem();
 }
 

tests/src/core/testqgslayoutmultiframe.cpp

@@ -43,6 +43,7 @@ class TestQgsLayoutMultiFrame : public QObject
     void addRemovePage(); //test if page is added and removed for RepeatUntilFinished mode
     void undoRedo(); //test that combinations of frame/multiframe undo/redo don't crash
     void undoRedoRemovedFrame(); //test that undo doesn't crash with removed frames
+    void undoRedoRemovedFrame2();
     void registry();
 
   private:
@@ -455,11 +456,13 @@ void TestQgsLayoutMultiFrame::undoRedoRemovedFrame()
 
   auto dumpStack = [ = ]
   {
+#if 0 // for debugging
     // dump stack
     for ( int i = 0; i < mLayout->undoStack()->stack()->count(); ++i )
     {
       QgsDebugMsg( QString( "%1: %2 %3" ).arg( i ).arg( mLayout->undoStack()->stack()->command( i )->text(), i + 1 == mLayout->undoStack()->stack()->index() ? QString( "<---" ) : QString() ) );
     }
+#endif
   };
   dumpStack();
   //undo changes
@@ -500,6 +503,15 @@ void TestQgsLayoutMultiFrame::undoRedoRemovedFrame()
   delete htmlItem;
 }
 
+void TestQgsLayoutMultiFrame::undoRedoRemovedFrame2()
+{
+  QgsLayoutItemHtml *htmlItem = new QgsLayoutItemHtml( mLayout );
+  QgsLayoutFrame *frame1 = new QgsLayoutFrame( mLayout, htmlItem );
+  frame1->attemptSetSceneRect( QRectF( 0, 0, 100, 200 ) );
+  htmlItem->addFrame( frame1 );
+
+}
+
 void TestQgsLayoutMultiFrame::registry()
 {
   // test QgsLayoutItemRegistry