Merge pull request #4806 from nyalldawson/dependent_params

[modeler] Don't allow removal of model parameters on which other parameters depend

Author: nyalldawson

python/core/processing/qgsprocessingmodelalgorithm.sip

@@ -724,6 +724,16 @@ Copies are protected to avoid slicing
 %Docstring
  Returns true if any child algorithms depend on the model parameter
  with the specified ``name``.
+.. seealso:: otherParametersDependOnParameter()
+ :rtype: bool
+%End
+
+    bool otherParametersDependOnParameter( const QString &name ) const;
+%Docstring
+ Returns true if any other model parameters depend on the parameter
+ with the specified ``name`` (e.g. field parameters where ``name``
+ is the parent layer parameter).
+.. seealso:: childAlgorithmsDependOnParameter()
  :rtype: bool
 %End
 

python/core/processing/qgsprocessingparameters.sip

@@ -338,6 +338,13 @@ class QgsProcessingParameterDefinition
 .. seealso:: metadata()
 %End
 
+    virtual QStringList dependsOnOtherParameters() const;
+%Docstring
+ Returns a list of other parameter names on which this parameter is dependent (e.g.
+ field parameters which depend on a parent layer parameter).
+ :rtype: list of str
+%End
+
   protected:
 
 
@@ -1261,6 +1268,8 @@ class QgsProcessingParameterExpression : QgsProcessingParameterDefinition
     virtual QString type() const;
     virtual QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const;
 
+    virtual QStringList dependsOnOtherParameters() const;
+
 
     QString parentLayerParameter() const;
 %Docstring
@@ -1357,6 +1366,8 @@ class QgsProcessingParameterField : QgsProcessingParameterDefinition
 
     virtual QString asScriptCode() const;
 
+    virtual QStringList dependsOnOtherParameters() const;
+
 
     QString parentLayerParameter() const;
 %Docstring

python/plugins/processing/modeler/ModelerGraphicItem.py

@@ -229,8 +229,12 @@ def updateAlgorithm(self, alg):
     def removeElement(self):
         if isinstance(self.element, QgsProcessingModelAlgorithm.ModelParameter):
             if self.model.childAlgorithmsDependOnParameter(self.element.parameterName()):
-                QMessageBox.warning(None, 'Could not remove element',
-                                    'Other elements depend on the selected one.\n'
+                QMessageBox.warning(None, 'Could not remove input',
+                                    'Algorithms depend on the selected input.\n'
+                                    'Remove them before trying to remove it.')
+            elif self.model.otherParametersDependOnParameter(self.element.parameterName()):
+                QMessageBox.warning(None, 'Could not remove input',
+                                    'Other inputs depend on the selected input.\n'
                                     'Remove them before trying to remove it.')
             else:
                 self.model.removeModelParameter(self.element.parameterName())

src/core/processing/qgsprocessingmodelalgorithm.cpp

@@ -949,6 +949,19 @@ bool QgsProcessingModelAlgorithm::childAlgorithmsDependOnParameter( const QStrin
   return false;
 }
 
+bool QgsProcessingModelAlgorithm::otherParametersDependOnParameter( const QString &name ) const
+{
+  Q_FOREACH ( const QgsProcessingParameterDefinition *def, mParameters )
+  {
+    if ( def->name() == name )
+      continue;
+
+    if ( def->dependsOnOtherParameters().contains( name ) )
+      return true;
+  }
+  return false;
+}
+
 QMap<QString, QgsProcessingModelAlgorithm::ModelParameter> QgsProcessingModelAlgorithm::parameterComponents() const
 {
   return mParameterComponents;

src/core/processing/qgsprocessingmodelalgorithm.h

@@ -718,9 +718,18 @@ class CORE_EXPORT QgsProcessingModelAlgorithm : public QgsProcessingAlgorithm
     /**
      * Returns true if any child algorithms depend on the model parameter
      * with the specified \a name.
+     * \see otherParametersDependOnParameter()
      */
     bool childAlgorithmsDependOnParameter( const QString &name ) const;
 
+    /**
+     * Returns true if any other model parameters depend on the parameter
+     * with the specified \a name (e.g. field parameters where \a name
+     * is the parent layer parameter).
+     * \see childAlgorithmsDependOnParameter()
+     */
+    bool otherParametersDependOnParameter( const QString &name ) const;
+
     /**
      * Returns the map of parameter components used by the model. The keys
      * should match the algorithm's parameter names (see parameterDefinitions() ).

src/core/processing/qgsprocessingparameters.cpp

@@ -2047,6 +2047,14 @@ QString QgsProcessingParameterExpression::valueAsPythonString( const QVariant &v
   return s.prepend( '\'' ).append( '\'' );
 }
 
+QStringList QgsProcessingParameterExpression::dependsOnOtherParameters() const
+{
+  QStringList depends;
+  if ( !mParentLayerParameter.isEmpty() )
+    depends << mParentLayerParameter;
+  return depends;
+}
+
 QString QgsProcessingParameterExpression::parentLayerParameter() const
 {
   return mParentLayerParameter;
@@ -2229,6 +2237,14 @@ QString QgsProcessingParameterField::asScriptCode() const
   return code.trimmed();
 }
 
+QStringList QgsProcessingParameterField::dependsOnOtherParameters() const
+{
+  QStringList depends;
+  if ( !mParentLayerParameter.isEmpty() )
+    depends << mParentLayerParameter;
+  return depends;
+}
+
 QString QgsProcessingParameterField::parentLayerParameter() const
 {
   return mParentLayerParameter;

src/core/processing/qgsprocessingparameters.h

@@ -373,6 +373,12 @@ class CORE_EXPORT QgsProcessingParameterDefinition
      */
     void setMetadata( const QVariantMap &metadata ) { mMetadata = metadata; }
 
+    /**
+     * Returns a list of other parameter names on which this parameter is dependent (e.g.
+     * field parameters which depend on a parent layer parameter).
+     */
+    virtual QStringList dependsOnOtherParameters() const { return QStringList(); }
+
   protected:
 
     //! Parameter name
@@ -1219,6 +1225,7 @@ class CORE_EXPORT QgsProcessingParameterExpression : public QgsProcessingParamet
 
     QString type() const override { return QStringLiteral( "expression" ); }
     QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const override;
+    QStringList dependsOnOtherParameters() const override;
 
     /**
      * Returns the name of the parent layer parameter, or an empty string if this is not set.
@@ -1306,6 +1313,7 @@ class CORE_EXPORT QgsProcessingParameterField : public QgsProcessingParameterDef
     bool checkValueIsAcceptable( const QVariant &input, QgsProcessingContext *context = nullptr ) const override;
     QString valueAsPythonString( const QVariant &value, QgsProcessingContext &context ) const override;
     QString asScriptCode() const override;
+    QStringList dependsOnOtherParameters() const override;
 
     /**
      * Returns the name of the parent layer parameter, or an empty string if this is not set.

tests/src/core/testqgsprocessing.cpp

@@ -1314,6 +1314,7 @@ void TestQgsProcessing::parameterGeneral()
   QCOMPARE( param.description(), QString( "desc" ) );
   QCOMPARE( param.defaultValue(), QVariant( true ) );
   QVERIFY( param.flags() & QgsProcessingParameterDefinition::FlagOptional );
+  QVERIFY( param.dependsOnOtherParameters().isEmpty() );
 
   // test getters and setters
   param.setDescription( "p2" );
@@ -2951,6 +2952,10 @@ void TestQgsProcessing::parameterExpression()
   def.reset( dynamic_cast< QgsProcessingParameterExpression *>( QgsProcessingParameters::parameterFromVariantMap( map ) ) );
   QVERIFY( dynamic_cast< QgsProcessingParameterExpression *>( def.get() ) );
 
+  QVERIFY( def->dependsOnOtherParameters().isEmpty() );
+  def->setParentLayerParameter( QStringLiteral( "test_layer" ) );
+  QCOMPARE( def->dependsOnOtherParameters(), QStringList() << QStringLiteral( "test_layer" ) );
+
   // optional
   def.reset( new QgsProcessingParameterExpression( "optional", QString(), QString( "default" ), QString(), true ) );
   QVERIFY( def->checkValueIsAcceptable( 1 ) );
@@ -3011,7 +3016,10 @@ void TestQgsProcessing::parameterField()
   QCOMPARE( fromCode->dataType(), def->dataType() );
   QCOMPARE( fromCode->allowMultiple(), def->allowMultiple() );
 
+  QVERIFY( def->dependsOnOtherParameters().isEmpty() );
   def->setParentLayerParameter( "my_parent" );
+  QCOMPARE( def->dependsOnOtherParameters(), QStringList() << QStringLiteral( "my_parent" ) );
+
   code = def->asScriptCode();
   fromCode.reset( dynamic_cast< QgsProcessingParameterField * >( QgsProcessingParameters::parameterFromScriptCode( code ) ) );
   QVERIFY( fromCode.get() );
@@ -4450,6 +4458,16 @@ void TestQgsProcessing::modelerAlgorithm()
   alg4.setChildAlgorithm( c10 );
   QVERIFY( alg4.childAlgorithmsDependOnParameter( "p1" ) );
 
+  QgsProcessingModelAlgorithm::ModelParameter vlP;
+  alg4.addModelParameter( new QgsProcessingParameterVectorLayer( "layer" ), vlP );
+  QgsProcessingModelAlgorithm::ModelParameter field;
+  alg4.addModelParameter( new QgsProcessingParameterField( "field", QString(), QVariant(), QStringLiteral( "layer" ) ), field );
+  QVERIFY( !alg4.otherParametersDependOnParameter( "p1" ) );
+  QVERIFY( !alg4.otherParametersDependOnParameter( "field" ) );
+  QVERIFY( alg4.otherParametersDependOnParameter( "layer" ) );
+
+
+
 
 
   // to/from XML