[processing] Fix issues with edits to model outputs getting discarded

Specifically, this fixes two issues 1. If a user edits a dark green output block in a model and changes the name of the output, that new name was always discarded and the only way to change it was by editing the algorithm it was attached to 2. If an output was renamed through the algorithm properties dialog, then any properties previously associated with that output (like comments, coloring, placement, default value, mandatory flag) would get reset back to their default settings
qgis · Feb 7, 2022 · d899da1 · d899da1
1 parent 8563a5e
commit d899da1
Show file tree

Hide file tree

Showing 6 changed files with 34 additions and 8 deletions.
diff --git a/python/plugins/processing/modeler/ModelerGraphicItem.py b/python/plugins/processing/modeler/ModelerGraphicItem.py
@@ -24,6 +24,7 @@
 from qgis.PyQt.QtCore import QCoreApplication
 
 from qgis.core import (QgsProcessingParameterDefinition,
+                       QgsProcessingModelOutput,
                        QgsProject,
                        Qgis)
 from qgis.gui import (
@@ -202,13 +203,22 @@ def edit(self, edit_comment=False):
             dlg.switchToCommentTab()
 
         if dlg.exec_():
-            model_output = child_alg.modelOutput(self.component().name())
+            model_outputs = child_alg.modelOutputs()
+
+            model_output = QgsProcessingModelOutput(model_outputs[self.component().name()])
+            del model_outputs[self.component().name()]
+
+            model_output.setName(dlg.param.description())
             model_output.setDescription(dlg.param.description())
             model_output.setDefaultValue(dlg.param.defaultValue())
             model_output.setMandatory(not (dlg.param.flags() & QgsProcessingParameterDefinition.FlagOptional))
             model_output.comment().setDescription(dlg.comments())
             model_output.comment().setColor(dlg.commentColor())
+            model_outputs[model_output.name()] = model_output
+            child_alg.setModelOutputs(model_outputs)
+
             self.aboutToChange.emit(self.tr('Edit {}').format(model_output.description()))
+
             self.model().updateDestinationParameters()
             self.requestModelRepaint.emit()
             self.changed.emit()

diff --git a/python/plugins/processing/modeler/ModelerParameterDefinitionDialog.py b/python/plugins/processing/modeler/ModelerParameterDefinitionDialog.py
@@ -214,29 +214,29 @@ def accept(self):
         if (isinstance(self.param, QgsProcessingParameterFeatureSink)):
             self.param = QgsProcessingParameterFeatureSink(
                 name=name,
-                description=self.param.description(),
+                description=description,
                 type=self.param.dataType(),
                 defaultValue=self.defaultWidget.value())
         elif (isinstance(self.param, QgsProcessingParameterFileDestination)):
             self.param = QgsProcessingParameterFileDestination(
                 name=name,
-                description=self.param.description(),
+                description=description,
                 fileFilter=self.param.fileFilter(),
                 defaultValue=self.defaultWidget.value())
         elif (isinstance(self.param, QgsProcessingParameterFolderDestination)):
             self.param = QgsProcessingParameterFolderDestination(
                 name=name,
-                description=self.param.description(),
+                description=description,
                 defaultValue=self.defaultWidget.value())
         elif (isinstance(self.param, QgsProcessingParameterRasterDestination)):
             self.param = QgsProcessingParameterRasterDestination(
                 name=name,
-                description=self.param.description(),
+                description=description,
                 defaultValue=self.defaultWidget.value())
         elif (isinstance(self.param, QgsProcessingParameterVectorDestination)):
             self.param = QgsProcessingParameterVectorDestination(
                 name=name,
-                description=self.param.description(),
+                description=description,
                 type=self.param.dataType(),
                 defaultValue=self.defaultWidget.value())
 

diff --git a/python/plugins/processing/modeler/ModelerParametersDialog.py b/python/plugins/processing/modeler/ModelerParametersDialog.py
@@ -178,6 +178,7 @@ def __init__(self, alg, model, algName=None, configuration=None, dialog=None, co
         self.context = context
         self.dialog = dialog
         self.widget_labels = {}
+        self.previous_output_definitions = {}
 
         class ContextGenerator(QgsProcessingContextGenerator):
 
@@ -350,6 +351,7 @@ def showAdvancedParametersClicked(self):
     def setPreviousValues(self):
         if self.childId is not None:
             alg = self.model.childAlgorithm(self.childId)
+
             self.descriptionBox.setText(alg.description())
             for param in alg.algorithm().parameterDefinitions():
                 if param.isDestination() or param.flags() & QgsProcessingParameterDefinition.FlagHidden:
@@ -386,6 +388,7 @@ def setPreviousValues(self):
                     if out.childId() == self.childId and out.childOutputName() == output.name():
                         # this destination parameter is linked to a model output
                         model_output_name = out.name()
+                        self.previous_output_definitions[output.name()] = out
                         break
 
                 value = None
@@ -458,7 +461,10 @@ def createAlgorithm(self):
                 if wrapper.isModelOutput():
                     name = wrapper.modelOutputName()
                     if name:
-                        model_output = QgsProcessingModelOutput(name, name)
+                        # if there was a previous output definition already for this output, we start with it,
+                        # otherwise we'll lose any existing output comments, coloring, position, etc
+                        model_output = self.previous_output_definitions.get(output.name(), QgsProcessingModelOutput(name, name))
+                        model_output.setDescription(name)
                         model_output.setChildId(alg.childId())
                         model_output.setChildOutputName(output.name())
                         outputs[name] = model_output

diff --git a/src/core/processing/models/qgsprocessingmodelchildalgorithm.cpp b/src/core/processing/models/qgsprocessingmodelchildalgorithm.cpp
@@ -67,6 +67,9 @@ void QgsProcessingModelChildAlgorithm::copyNonDefinitionPropertiesFromModel( Qgs
   int i = 0;
   for ( auto it = mModelOutputs.begin(); it != mModelOutputs.end(); ++it )
   {
+    if ( !existingChild.modelOutputs().contains( it.key() ) )
+      continue;
+
     if ( !existingChild.modelOutputs().value( it.key() ).position().isNull() )
     {
       it.value().setPosition( existingChild.modelOutputs().value( it.key() ).position() );

diff --git a/src/gui/processing/models/qgsmodelcomponentgraphicitem.cpp b/src/gui/processing/models/qgsmodelcomponentgraphicitem.cpp
@@ -1129,7 +1129,7 @@ QgsModelOutputGraphicItem::QgsModelOutputGraphicItem( QgsProcessingModelOutput *
   QPainter painter( &mPicture );
   svg.render( &painter );
   painter.end();
-  setLabel( output->name() );
+  setLabel( output->description() );
 }
 
 QColor QgsModelOutputGraphicItem::fillColor( QgsModelComponentGraphicItem::State state ) const

diff --git a/tests/src/analysis/testqgsprocessingmodelalgorithm.cpp b/tests/src/analysis/testqgsprocessingmodelalgorithm.cpp
@@ -557,6 +557,11 @@ void TestQgsProcessingModelAlgorithm::modelerAlgorithm()
   const QgsProcessingModelOutput oo2;
   QMap< QString, QgsProcessingModelOutput > a2Outs2;
   a2Outs2.insert( QStringLiteral( "out1" ), oo2 );
+  // this one didn't already exist in the algorithm
+  QgsProcessingModelOutput oo3;
+  oo3.comment()->setDescription( QStringLiteral( "my comment" ) );
+  a2Outs2.insert( QStringLiteral( "out3" ), oo3 );
+
   a2other.setModelOutputs( a2Outs2 );
 
   a2other.copyNonDefinitionPropertiesFromModel( &alg );
@@ -576,6 +581,8 @@ void TestQgsProcessingModelAlgorithm::modelerAlgorithm()
   // should be copied for outputs
   QCOMPARE( a2other.modelOutput( QStringLiteral( "out1" ) ).comment()->description(), QStringLiteral( "c3" ) );
   QCOMPARE( a2other.modelOutput( QStringLiteral( "out1" ) ).comment()->color(), QColor( 155, 14, 353 ) );
+  // new outputs should not be affected
+  QCOMPARE( a2other.modelOutput( QStringLiteral( "out3" ) ).comment()->description(), QStringLiteral( "my comment" ) );
 
   QgsProcessingModelChildAlgorithm a3;
   a3.setChildId( QStringLiteral( "c" ) );