From 4bb45cb70d98314ad30049c3fc6e466a0797a403 Mon Sep 17 00:00:00 2001
From: Pieter Hijma <pieterhijma@users.noreply.github.com>
Date: Sun, 27 Oct 2024 20:02:16 +0100
Subject: [PATCH] Gui: Make VarSet dialog modal

Also tracks its own transactions to prevent interfering with other
transactions.
---
 src/Gui/CommandStructure.cpp     |  1 +
 src/Gui/DlgAddPropertyVarSet.cpp | 50 +++++++++++++++++++++++++-------
 src/Gui/DlgAddPropertyVarSet.h   |  8 +++++
 src/Gui/ViewProviderVarSet.cpp   |  4 +++
 4 files changed, 52 insertions(+), 11 deletions(-)

diff --git a/src/Gui/CommandStructure.cpp b/src/Gui/CommandStructure.cpp
index 06093e550a..1b14ce5069 100644
--- a/src/Gui/CommandStructure.cpp
+++ b/src/Gui/CommandStructure.cpp
@@ -166,6 +166,7 @@ void StdCmdVarSet::activated(int iMsg)
             group->addObject(doc->getObject(VarSetName.c_str()));
         }
     }
+    commitCommand();
 
     doCommand(Doc, "App.ActiveDocument.getObject('%s').ViewObject.doubleClicked()", VarSetName.c_str());
 }
diff --git a/src/Gui/DlgAddPropertyVarSet.cpp b/src/Gui/DlgAddPropertyVarSet.cpp
index b3805523de..1c370cce8f 100644
--- a/src/Gui/DlgAddPropertyVarSet.cpp
+++ b/src/Gui/DlgAddPropertyVarSet.cpp
@@ -47,6 +47,8 @@ using namespace Gui::Dialog;
 const std::string DlgAddPropertyVarSet::GROUP_BASE = "Base";
 
 const bool CLEAR_NAME = true;
+const bool ABORT = true;
+const bool COMMIT = false;
 
 DlgAddPropertyVarSet::DlgAddPropertyVarSet(QWidget* parent,
                                            ViewProviderVarSet* viewProvider)
@@ -55,7 +57,8 @@ DlgAddPropertyVarSet::DlgAddPropertyVarSet(QWidget* parent,
       ui(new Ui_DlgAddPropertyVarSet),
       comboBoxGroup(this),
       completerType(this),
-      editor(nullptr)
+      editor(nullptr),
+      transactionID(0)
 {
     ui->setupUi(this);
 
@@ -329,13 +332,40 @@ void DlgAddPropertyVarSet::changePropertyToAdd() {
 }
 
 
+/* We use these functions rather than the functions provided by App::Document
+ * because this dialog may be opened when another transaction is in progress.
+ * An example is opening a sketch.  If this dialog uses the functions provided
+ * by App::Document, a reject of the dialog would close that transaction.  By
+ * checking whether the transaction ID is "our" transaction ID, we prevent this
+ * behavior.
+ */
+void DlgAddPropertyVarSet::openTransaction()
+{
+    transactionID = App::GetApplication().setActiveTransaction("Add property VarSet");
+}
+
+
+bool DlgAddPropertyVarSet::hasPendingTransaction()
+{
+    return transactionID != 0;
+}
+
+
+void DlgAddPropertyVarSet::closeTransaction(bool abort)
+{
+    if (transactionID != 0) {
+        App::GetApplication().closeActiveTransaction(abort, transactionID);
+        transactionID = 0;
+    }
+}
+
+
 void DlgAddPropertyVarSet::clearCurrentProperty()
 {
     removeEditor();
     varSet->removeDynamicProperty(namePropertyToAdd.c_str());
-    App::Document* doc = varSet->getDocument();
-    if (doc->hasPendingTransaction()) {
-        doc->abortTransaction();
+    if (hasPendingTransaction()) {
+        closeTransaction(ABORT);
     }
     setOkEnabled(false);
     namePropertyToAdd.clear();
@@ -429,8 +459,7 @@ void DlgAddPropertyVarSet::onEditFinished() {
 
     if (namePropertyToAdd.empty()) {
         // we are adding a new property
-        App::Document* doc = varSet->getDocument();
-        doc->openTransaction("Add property VarSet");
+        openTransaction();
         createProperty();
     }
     else {
@@ -487,12 +516,11 @@ void DlgAddPropertyVarSet::addDocumentation() {
 void DlgAddPropertyVarSet::accept()
 {
     addDocumentation();
-    App::Document* doc = varSet->getDocument();
-    doc->commitTransaction();
+    closeTransaction(COMMIT);
 
     if (ui->checkBoxAdd->isChecked()) {
         clearEditors();
-        doc->openTransaction();
+        openTransaction();
         ui->lineEditName->setFocus();
         return;
     }
@@ -520,8 +548,8 @@ void DlgAddPropertyVarSet::reject()
     disconnect(connLineEditNameTextChanged);
 
     // a transaction is not pending if a name has not been determined.
-    if (doc->hasPendingTransaction()) {
-        doc->abortTransaction();
+    if (hasPendingTransaction()) {
+        closeTransaction(ABORT);
     }
     QDialog::reject();
 }
diff --git a/src/Gui/DlgAddPropertyVarSet.h b/src/Gui/DlgAddPropertyVarSet.h
index 33ac60e5de..f837e1297c 100644
--- a/src/Gui/DlgAddPropertyVarSet.h
+++ b/src/Gui/DlgAddPropertyVarSet.h
@@ -96,6 +96,11 @@ private:
     void createProperty();
     void changePropertyToAdd();
 
+    void openTransaction();
+    bool hasPendingTransaction();
+    void abortTransaction();
+    void closeTransaction(bool abort);
+
     void checkName();
     void checkGroup();
     void checkType();
@@ -128,6 +133,9 @@ private:
     std::unique_ptr<PropertyEditor::PropertyItem> propertyItem;
     std::unique_ptr<App::ObjectIdentifier> objectIdentifier;
 
+    // a transactionID of 0 means that there is no active transaction.
+    int transactionID;
+
     // connections
     QMetaObject::Connection connComboBoxGroup;
     QMetaObject::Connection connComboBoxType;
diff --git a/src/Gui/ViewProviderVarSet.cpp b/src/Gui/ViewProviderVarSet.cpp
index e65b384650..bd091d5256 100644
--- a/src/Gui/ViewProviderVarSet.cpp
+++ b/src/Gui/ViewProviderVarSet.cpp
@@ -46,6 +46,10 @@ bool ViewProviderVarSet::doubleClicked()
         dialog = std::make_unique<DlgAddPropertyVarSet>(getMainWindow(), this);
     }
 
+    // Do not use exec() here because it blocks and prevents command Std_VarSet
+    // to commit the autotransaction.  This in turn prevents the dialog to
+    // handle transactions well.
+    dialog->setWindowModality(Qt::ApplicationModal);
     dialog->show();
     dialog->raise();
     dialog->activateWindow();