Merge pull request #5597 from pblottiere/bugfix-clickxy

[bugfix]  Fixes #16852 by adding click_x and click_y variables to resolve actions

python/core/qgsaction.sip

@@ -184,6 +184,21 @@ Checks if the action is runable on the current platform
 .. versionadded:: 3.0
 %End
 
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+%Docstring
+ Sets an expression context scope to use for running the action.
+
+.. versionadded:: 3.0
+%End
+
+    QgsExpressionContextScope expressionContextScope() const;
+%Docstring
+ Returns an expression context scope used for running the action.
+
+.. versionadded:: 3.0
+ :rtype: QgsExpressionContextScope
+%End
+
 };
 
 

python/core/qgsactionmanager.sip

@@ -62,10 +62,15 @@ Constructor
 .. versionadded:: 3.0
 %End
 
-    void doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex = 0 ) /PyName=doActionFeature/;
+    void doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex = 0, const QgsExpressionContextScope &scope = QgsExpressionContextScope() ) /PyName=doActionFeature/;
 %Docstring
- Does the given action. defaultValueIndex is the index of the
-  field to be used if the action has a $currfield placeholder.
+ Does the given action.
+
+ \param actionId action id
+ \param feature feature to run action for
+ \param defaultValueIndex index of the field to be used if the action has a $currfield placeholder.
+ \param scope expression context scope to add during expression evaluation
+
 .. note::
 
    available in Python bindings as doActionFeature

python/gui/qgsactionmenu.sip

@@ -73,6 +73,21 @@ class QgsActionMenu : QMenu
                  as long as the menu is displayed and the action is running.
 %End
 
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+%Docstring
+ Sets an expression context scope used to resolve underlying actions.
+
+.. versionadded:: 3.0
+%End
+
+    QgsExpressionContextScope expressionContextScope() const;
+%Docstring
+ Returns an expression context scope used to resolve underlying actions.
+
+.. versionadded:: 3.0
+ :rtype: QgsExpressionContextScope
+%End
+
   signals:
     void reinit();
 

python/gui/qgsidentifymenu.sip

@@ -74,6 +74,21 @@ define if the menu will be shown with a single idetify result
  :rtype: bool
 %End
 
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+%Docstring
+ Sets an expression context scope used to resolve underlying actions.
+
+.. versionadded:: 3.0
+%End
+
+    QgsExpressionContextScope expressionContextScope() const;
+%Docstring
+ Returns an expression context scope used to resolve underlying actions.
+
+.. versionadded:: 3.0
+ :rtype: QgsExpressionContextScope
+%End
+
     void setShowFeatureActions( bool showFeatureActions );
 %Docstring
  define if attribute actions(1) and map layer actions(2) can be listed and run from the menu

src/app/qgisapp.cpp

@@ -1273,6 +1273,9 @@ QgisApp::QgisApp()
   mUndoWidget = new QgsUndoWidget( nullptr, mMapCanvas );
   mInfoBar = new QgsMessageBar( centralWidget() );
   mAdvancedDigitizingDockWidget = new QgsAdvancedDigitizingDockWidget( mMapCanvas, this );
+  mPanelMenu = new QMenu( this );
+  mProgressBar = new QProgressBar( this );
+  mStatusBar = new QgsStatusBar( this );
   // More tests may need more members to be initialized
 }
 

src/app/qgsidentifyresultsdialog.cpp

@@ -1164,6 +1164,8 @@ void QgsIdentifyResultsDialog::clear()
     delete curve;
   mPlotCurves.clear();
 
+  mExpressionContextScope = QgsExpressionContextScope();
+
   // keep it visible but disabled, it can switch from disabled/enabled
   // after raster format change
   mActionPrint->setDisabled( true );
@@ -1245,7 +1247,7 @@ void QgsIdentifyResultsDialog::doAction( QTreeWidgetItem *item, const QString &a
   }
 
   int featIdx = featItem->data( 0, Qt::UserRole + 1 ).toInt();
-  layer->actions()->doAction( action, mFeatures[ featIdx ], idx );
+  layer->actions()->doAction( action, mFeatures[ featIdx ], idx, mExpressionContextScope );
 }
 
 void QgsIdentifyResultsDialog::doMapLayerAction( QTreeWidgetItem *item, QgsMapLayerAction *action )
@@ -1978,3 +1980,13 @@ void QgsIdentifyResultsDialog::showHelp()
 {
   QgsHelp::openHelp( QStringLiteral( "introduction/general_tools.html#identify" ) );
 }
+
+void QgsIdentifyResultsDialog::setExpressionContextScope( const QgsExpressionContextScope &scope )
+{
+  mExpressionContextScope = scope;
+}
+
+QgsExpressionContextScope QgsIdentifyResultsDialog::expressionContextScope() const
+{
+  return mExpressionContextScope;
+}

src/app/qgsidentifyresultsdialog.h

@@ -25,6 +25,7 @@
 #include "qgscoordinatereferencesystem.h"
 #include "qgsmaptoolidentify.h"
 #include "qgswebview.h"
+#include "qgsexpressioncontext.h"
 
 #include <QWidget>
 #include <QList>
@@ -150,6 +151,22 @@ class APP_EXPORT QgsIdentifyResultsDialog: public QDialog, private Ui::QgsIdenti
     //! Map tool was activated
     void activate();
 
+    /**
+     * Sets an expression context scope to consider for resolving underlying
+     * actions.
+     *
+     * \since QGIS 3.0
+     */
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+
+    /**
+     * Returns an expression context scope used for resolving underlying
+     * actions.
+     *
+     * \since QGIS 3.0
+     */
+    QgsExpressionContextScope expressionContextScope() const;
+
   signals:
     void selectedFeatureChanged( QgsVectorLayer *, QgsFeatureId featureId );
 
@@ -233,6 +250,7 @@ class APP_EXPORT QgsIdentifyResultsDialog: public QDialog, private Ui::QgsIdenti
     QgsMapCanvas *mCanvas = nullptr;
     QList<QgsFeature> mFeatures;
     QMap< QString, QMap< QString, QVariant > > mWidgetCaches;
+    QgsExpressionContextScope mExpressionContextScope;
 
     QgsMapLayer *layer( QTreeWidgetItem *item );
     QgsVectorLayer *vectorLayer( QTreeWidgetItem *item );

src/app/qgsmaptoolidentifyaction.cpp

@@ -37,6 +37,7 @@
 #include "qgsrenderer.h"
 #include "qgsunittypes.h"
 #include "qgsstatusbar.h"
+#include "qgsactionscoperegistry.h"
 
 #include "qgssettings.h"
 #include <QMouseEvent>
@@ -119,6 +120,8 @@ void QgsMapToolIdentifyAction::canvasReleaseEvent( QgsMapMouseEvent *e )
   connect( this, &QgsMapToolIdentifyAction::identifyProgress, QgisApp::instance(), &QgisApp::showProgress );
   connect( this, &QgsMapToolIdentifyAction::identifyMessage, QgisApp::instance(), &QgisApp::showStatusMessage );
 
+  setClickContextScope( toMapCoordinates( e->pos() ) );
+
   identifyMenu()->setResultsIfExternalAction( false );
 
   // enable the right click for extended menu so it behaves as a contextual menu
@@ -201,4 +204,16 @@ void QgsMapToolIdentifyAction::handleCopyToClipboard( QgsFeatureStore &featureSt
   emit copyToClipboard( featureStore );
 }
 
+void QgsMapToolIdentifyAction::setClickContextScope( const QgsPointXY &point )
+{
+  QgsExpressionContextScope clickScope;
+  clickScope.addVariable( QgsExpressionContextScope::StaticVariable( QString( "click_x" ), point.x(), true ) );
+  clickScope.addVariable( QgsExpressionContextScope::StaticVariable( QString( "click_y" ), point.y(), true ) );
 
+  resultsDialog()->setExpressionContextScope( clickScope );
+
+  if ( mIdentifyMenu )
+  {
+    mIdentifyMenu->setExpressionContextScope( clickScope );
+  }
+}

src/app/qgsmaptoolidentifyaction.h

@@ -78,7 +78,9 @@ class APP_EXPORT QgsMapToolIdentifyAction : public QgsMapToolIdentify
 
     virtual QgsUnitTypes::DistanceUnit displayDistanceUnits() const override;
     virtual QgsUnitTypes::AreaUnit displayAreaUnits() const override;
+    void setClickContextScope( const QgsPointXY &point );
 
+    friend class TestQgsMapToolIdentifyAction;
 };
 
 #endif

src/core/qgsaction.cpp

@@ -59,7 +59,11 @@ void QgsAction::run( const QgsExpressionContext &expressionContext ) const
     return;
   }
 
-  QString expandedAction = QgsExpression::replaceExpressionText( mCommand, &expressionContext );
+  QgsExpressionContextScope *scope = new QgsExpressionContextScope( mExpressionContextScope );
+  QgsExpressionContext context( expressionContext );
+  context << scope;
+
+  QString expandedAction = QgsExpression::replaceExpressionText( mCommand, &context );
 
   if ( mType == QgsAction::OpenUrl )
   {
@@ -146,3 +150,13 @@ void QgsAction::writeXml( QDomNode &actionsNode ) const
 
   actionsNode.appendChild( actionSetting );
 }
+
+void QgsAction::setExpressionContextScope( const QgsExpressionContextScope &scope )
+{
+  mExpressionContextScope = scope;
+}
+
+QgsExpressionContextScope QgsAction::expressionContextScope() const
+{
+  return mExpressionContextScope;
+};

src/core/qgsaction.h

@@ -190,6 +190,20 @@ class CORE_EXPORT QgsAction
      */
     void writeXml( QDomNode &actionsNode ) const;
 
+    /**
+     * Sets an expression context scope to use for running the action.
+     *
+     * \since QGIS 3.0
+     */
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+
+    /**
+     * Returns an expression context scope used for running the action.
+     *
+     * \since QGIS 3.0
+     */
+    QgsExpressionContextScope expressionContextScope() const;
+
   private:
     ActionType mType = Generic;
     QString mDescription;
@@ -201,6 +215,7 @@ class CORE_EXPORT QgsAction
     QString mNotificationMessage;
     mutable std::shared_ptr<QAction> mAction;
     QUuid mId;
+    QgsExpressionContextScope mExpressionContextScope;
 };
 
 Q_DECLARE_METATYPE( QgsAction )

src/core/qgsactionmanager.cpp

@@ -123,10 +123,10 @@ void QgsActionManager::removeAction( const QUuid &actionId )
   }
 }
 
-void QgsActionManager::doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex )
+void QgsActionManager::doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex, const QgsExpressionContextScope &scope )
 {
   QgsExpressionContext context = createExpressionContext();
-  QgsExpressionContextScope *actionScope = new QgsExpressionContextScope();
+  QgsExpressionContextScope *actionScope = new QgsExpressionContextScope( scope );
   actionScope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "field_index" ), defaultValueIndex, true ) );
   if ( defaultValueIndex >= 0 && defaultValueIndex < feature.fields().size() )
     actionScope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "field_name" ), feature.fields().at( defaultValueIndex ).name(), true ) );

src/core/qgsactionmanager.h

@@ -89,11 +89,16 @@ class CORE_EXPORT QgsActionManager: public QObject
     void removeAction( const QUuid &actionId );
 
     /**
-     * Does the given action. defaultValueIndex is the index of the
-     *  field to be used if the action has a $currfield placeholder.
-     *  \note available in Python bindings as doActionFeature
+     * Does the given action.
+     *
+     * \param actionId action id
+     * \param feature feature to run action for
+     * \param defaultValueIndex index of the field to be used if the action has a $currfield placeholder.
+     * \param scope expression context scope to add during expression evaluation
+     *
+     * \note available in Python bindings as doActionFeature
      */
-    void doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex = 0 ) SIP_PYNAME( doActionFeature );
+    void doAction( const QUuid &actionId, const QgsFeature &feature, int defaultValueIndex = 0, const QgsExpressionContextScope &scope = QgsExpressionContextScope() ) SIP_PYNAME( doActionFeature );
 
     /**
      * Does the action using the expression engine to replace any embedded expressions

src/gui/qgsactionmenu.cpp

@@ -107,8 +107,11 @@ void QgsActionMenu::reloadActions()
 
   Q_FOREACH ( const QgsAction &action, mActions )
   {
+    QgsAction act( action );
+    act.setExpressionContextScope( mExpressionContextScope );
+
     QAction *qAction = new QAction( action.icon(), action.name(), this );
-    qAction->setData( QVariant::fromValue<ActionData>( ActionData( action, mFeatureId, mLayer ) ) );
+    qAction->setData( QVariant::fromValue<ActionData>( ActionData( act, mFeatureId, mLayer ) ) );
     qAction->setIcon( action.icon() );
 
     // Only enable items on supported platforms
@@ -160,3 +163,15 @@ QgsActionMenu::ActionData::ActionData( const QgsAction &action, QgsFeatureId fea
   , featureId( featureId )
   , mapLayer( mapLayer )
 {}
+
+
+void QgsActionMenu::setExpressionContextScope( const QgsExpressionContextScope &scope )
+{
+  mExpressionContextScope = scope;
+  reloadActions();
+}
+
+QgsExpressionContextScope QgsActionMenu::expressionContextScope() const
+{
+  return mExpressionContextScope;
+}

src/gui/qgsactionmenu.h

@@ -91,6 +91,20 @@ class GUI_EXPORT QgsActionMenu : public QMenu
      */
     void setFeature( const QgsFeature &feature );
 
+    /**
+     * Sets an expression context scope used to resolve underlying actions.
+     *
+     * \since QGIS 3.0
+     */
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+
+    /**
+     * Returns an expression context scope used to resolve underlying actions.
+     *
+     * \since QGIS 3.0
+     */
+    QgsExpressionContextScope expressionContextScope() const;
+
   signals:
     void reinit();
 
@@ -107,6 +121,7 @@ class GUI_EXPORT QgsActionMenu : public QMenu
     QgsFeature mFeature;
     QgsFeatureId mFeatureId;
     QString mActionScope;
+    QgsExpressionContextScope mExpressionContextScope;
 };
 
 

src/gui/qgsidentifymenu.cpp

@@ -345,6 +345,7 @@ void QgsIdentifyMenu::addVectorLayer( QgsVectorLayer *layer, const QList<QgsMapT
     if ( mShowFeatureActions )
     {
       featureActionMenu = new QgsActionMenu( layer, result.mFeature, QStringLiteral( "Feature" ), layerMenu );
+      featureActionMenu->setExpressionContextScope( mExpressionContextScope );
     }
 
     // feature title
@@ -639,3 +640,13 @@ void QgsIdentifyMenu::removeCustomActions()
   mCustomActionRegistry.clear();
 
 }
+
+void QgsIdentifyMenu::setExpressionContextScope( const QgsExpressionContextScope &scope )
+{
+  mExpressionContextScope = scope;
+}
+
+QgsExpressionContextScope QgsIdentifyMenu::expressionContextScope() const
+{
+  return mExpressionContextScope;
+}

src/gui/qgsidentifymenu.h

@@ -103,6 +103,20 @@ class GUI_EXPORT QgsIdentifyMenu : public QMenu
     void setExecWithSingleResult( bool execWithSingleResult ) { mExecWithSingleResult = execWithSingleResult;}
     bool execWithSingleResult() { return mExecWithSingleResult;}
 
+    /**
+     * Sets an expression context scope used to resolve underlying actions.
+     *
+     * \since QGIS 3.0
+     */
+    void setExpressionContextScope( const QgsExpressionContextScope &scope );
+
+    /**
+     * Returns an expression context scope used to resolve underlying actions.
+     *
+     * \since QGIS 3.0
+     */
+    QgsExpressionContextScope expressionContextScope() const;
+
     /**
      * \brief define if attribute actions(1) and map layer actions(2) can be listed and run from the menu
      * \note custom actions will be shown in any case if they exist.
@@ -178,6 +192,8 @@ class GUI_EXPORT QgsIdentifyMenu : public QMenu
     int mMaxLayerDisplay;
     int mMaxFeatureDisplay;
 
+    QgsExpressionContextScope mExpressionContextScope;
+
     // name of the action to be displayed for feature default action, if other actions are shown
     QString mDefaultActionName;
 

tests/src/app/CMakeLists.txt

@@ -32,6 +32,7 @@ INCLUDE_DIRECTORIES(
 )
 INCLUDE_DIRECTORIES(SYSTEM
   ${QT_INCLUDE_DIR}
+  ${QWT_INCLUDE_DIR}
   ${GDAL_INCLUDE_DIR}
   ${PROJ_INCLUDE_DIR}
   ${GEOS_INCLUDE_DIR}

tests/src/app/testqgsmaptoolidentifyaction.cpp

@@ -26,6 +26,14 @@
 #include "qgsunittypes.h"
 #include "qgsmaptoolidentifyaction.h"
 #include "qgssettings.h"
+#include "qgsidentifymenu.h"
+#include "qgisapp.h"
+#include "qgsaction.h"
+#include "qgsactionmanager.h"
+#include "qgsactionmenu.h"
+#include "qgsidentifyresultsdialog.h"
+
+#include <QTimer>
 
 #include "cpl_conv.h"
 
@@ -46,9 +54,14 @@ class TestQgsMapToolIdentifyAction : public QObject
     void identifyRasterFloat32(); // test pixel identification and decimal precision
     void identifyRasterFloat64(); // test pixel identification and decimal precision
     void identifyInvalidPolygons(); // test selecting invalid polygons
+    void clickxy(); // test if clicked_x and clicked_y variables are propagated
 
   private:
+    void doAction();
+
     QgsMapCanvas *canvas = nullptr;
+    QgsMapToolIdentifyAction *mIdentifyAction = nullptr;
+    QgisApp *mQgisApp = nullptr;
 
     QString testIdentifyRaster( QgsRasterLayer *layer, double xGeoref, double yGeoref );
     QList<QgsMapToolIdentify::IdentifyResult> testIdentifyVector( QgsVectorLayer *layer, double xGeoref, double yGeoref );
@@ -91,6 +104,8 @@ void TestQgsMapToolIdentifyAction::initTestCase()
   // enforce C locale because the tests expect it
   // (decimal separators / thousand separators)
   QLocale::setDefault( QLocale::c() );
+
+  mQgisApp = new QgisApp();
 }
 
 void TestQgsMapToolIdentifyAction::cleanupTestCase()
@@ -108,6 +123,92 @@ void TestQgsMapToolIdentifyAction::cleanup()
   delete canvas;
 }
 
+void TestQgsMapToolIdentifyAction::doAction()
+{
+  bool ok = false;
+  int clickxOk = 2484588;
+  int clickyOk = 2425722;
+
+  // test QActionMenu
+  QList<QAction *> actions = mIdentifyAction->identifyMenu()->actions();
+  bool testDone = false;
+
+  for ( int i = 0; i < actions.count(); i++ )
+  {
+    if ( actions[i]->text().compare( "MyAction" ) == 0 )
+    {
+      QgsActionMenu::ActionData data = actions[i]->data().value<QgsActionMenu::ActionData>();
+      QgsAction act = data.actionData.value<QgsAction>();
+
+      int clickx = act.expressionContextScope().variable( "click_x" ).toString().toInt( &ok, 10 );
+      QCOMPARE( clickx, clickxOk );
+
+      int clicky = act.expressionContextScope().variable( "click_y" ).toString().toInt( &ok, 10 );
+      QCOMPARE( clicky, clickyOk );
+
+      testDone = true;
+    }
+  }
+
+  QCOMPARE( testDone, true );
+
+  // test QgsIdentifyResultsDialog expression context scope
+  QgsIdentifyResultsDialog *dlg = mIdentifyAction->resultsDialog();
+  int clickx = dlg->expressionContextScope().variable( "click_x" ).toString().toInt( &ok, 10 );
+  QCOMPARE( clickx, clickxOk );
+
+  int clicky = dlg->expressionContextScope().variable( "click_y" ).toString().toInt( &ok, 10 );
+  QCOMPARE( clicky, clickyOk );
+
+  // close
+  mIdentifyAction->identifyMenu()->close();
+}
+
+void TestQgsMapToolIdentifyAction::clickxy()
+{
+  // create temp layer
+  std::unique_ptr< QgsVectorLayer> tempLayer( new QgsVectorLayer( QStringLiteral( "Point?crs=epsg:3111" ), QStringLiteral( "vl" ), QStringLiteral( "memory" ) ) );
+  QVERIFY( tempLayer->isValid() );
+
+  // add feature
+  QgsFeature f1( tempLayer->dataProvider()->fields(), 1 );
+  QgsPointXY wordPoint( 2484588, 2425722 );
+  QgsGeometry geom = QgsGeometry::fromPointXY( wordPoint ) ;
+  f1.setGeometry( geom );
+  tempLayer->dataProvider()->addFeatures( QgsFeatureList() << f1 );
+
+  // prepare canvas
+  QList<QgsMapLayer *> layers;
+  layers.append( tempLayer.get() );
+
+  QgsCoordinateReferenceSystem srs( 3111, QgsCoordinateReferenceSystem::EpsgCrsId );
+  canvas->setDestinationCrs( srs );
+  canvas->setLayers( layers );
+  canvas->setCurrentLayer( tempLayer.get() );
+
+  // create/add action
+  QgsAction act( QgsAction::GenericPython, "MyAction", "", true );
+
+  QSet<QString> scopes;
+  scopes << "Feature";
+  act.setActionScopes( scopes );
+  tempLayer->actions()->addAction( act );
+
+  // init map tool identify action
+  mIdentifyAction = new QgsMapToolIdentifyAction( canvas );
+
+  // simulate a click on the canvas
+  QgsPointXY mapPoint = canvas->getCoordinateTransform()->transform( 2484588, 2425722 );
+  QPoint point = QPoint( mapPoint.x(), mapPoint.y() );
+  QMouseEvent releases( QEvent::MouseButtonRelease, point,
+                        Qt::RightButton, Qt::LeftButton, Qt::NoModifier );
+  QgsMapMouseEvent mapReleases( 0, &releases );
+
+  // simulate a click on the corresponding action
+  QTimer::singleShot( 2000, this, &TestQgsMapToolIdentifyAction::doAction );
+  mIdentifyAction->canvasReleaseEvent( &mapReleases );
+}
+
 void TestQgsMapToolIdentifyAction::lengthCalculation()
 {
   QgsSettings s;