Follow up map layer action changes

- Switch to flags instead of boolean argument - Move logic for layer validity to canRunUsingLayer - Add unit test Also remove settings flag to hide duplicate features action
qgis · Feb 24, 2018 · 0559e46 · 0559e46
1 parent 2520c28
commit 0559e46
Show file tree

Hide file tree

Showing 11 changed files with 212 additions and 69 deletions.
diff --git a/python/gui/qgsmaplayeractionregistry.sip.in b/python/gui/qgsmaplayeractionregistry.sip.in
@@ -30,7 +30,15 @@ An action which can run on map layers
     typedef QFlags<QgsMapLayerAction::Target> Targets;
 
 
-    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, Targets targets = AllActions, const QIcon &icon = QIcon() );
+    enum Flag
+    {
+      EnabledOnlyWhenEditable,
+    };
+
+    typedef QFlags<QgsMapLayerAction::Flag> Flags;
+
+
+    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = 0 );
 %Docstring
 Creates a map layer action which can run on any layer
 
@@ -39,18 +47,25 @@ Creates a map layer action which can run on any layer
    using AllActions as a target probably does not make a lot of sense. This default action was settled for API compatibility reasons.
 %End
 
-    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, QgsMapLayer *layer, Targets targets = AllActions, const QIcon &icon = QIcon() );
+    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, QgsMapLayer *layer, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = 0 );
 %Docstring
 Creates a map layer action which can run only on a specific layer
 %End
 
-    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, QgsMapLayer::LayerType layerType, Targets targets = AllActions, const QIcon &icon = QIcon() );
+    QgsMapLayerAction( const QString &name, QObject *parent /TransferThis/, QgsMapLayer::LayerType layerType, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = 0 );
 %Docstring
 Creates a map layer action which can run on a specific type of layer
 %End
 
     ~QgsMapLayerAction();
 
+    QgsMapLayerAction::Flags flags() const;
+%Docstring
+Layer behavior flags.
+
+.. versionadded:: 3.0
+%End
+
     bool canRunUsingLayer( QgsMapLayer *layer ) const;
 %Docstring
 True if action can run using the specified layer
@@ -78,6 +93,13 @@ Define the targets of the action
     const Targets &targets() const;
 %Docstring
 Return availibity of action
+%End
+
+    bool isEnabledOnlyWhenEditable() const;
+%Docstring
+Returns true if the action is only enabled for layers in editable mode.
+
+.. versionadded:: 3.0
 %End
 
   signals:
@@ -158,6 +180,9 @@ Triggered when an action is added or removed from the registry
 
 };
 
+QFlags<QgsMapLayerAction::Flag> operator|(QgsMapLayerAction::Flag f1, QFlags<QgsMapLayerAction::Flag> f2);
+
+
 /************************************************************************
  * This file has been generated automatically from                      *
  *                                                                      *

diff --git a/resources/qgis_global_settings.ini b/resources/qgis_global_settings.ini
@@ -34,7 +34,3 @@ connections-xyz\OpenStreetMap\zmin=0
 # for now this is online version of the User Guide for latest (LTR) release
 helpSearchPath=https://docs.qgis.org/$qgis_short_version/$qgis_locale/docs/user_manual/
 
-[app]
-
-# If true, the experimental "duplicate feature" actions will be shown in the QGIS UI
-tools\showDuplicateFeatureActions=false
diff --git a/src/app/qgisapp.cpp b/src/app/qgisapp.cpp
@@ -6488,9 +6488,6 @@ void QgisApp::refreshFeatureActions()
 
   for ( int i = 0; i < registeredActions.size(); i++ )
   {
-    if ( !vlayer->isEditable() && registeredActions.at( i )->isEnabledOnlyWhenEditable() )
-      continue;
-
     mFeatureActionMenu->addAction( registeredActions.at( i ) );
     if ( registeredActions.at( i ) == QgsGui::mapLayerActionRegistry()->defaultActionForLayer( vlayer ) )
     {
@@ -7498,31 +7495,27 @@ void QgisApp::setupLayoutManagerConnections()
 
 void QgisApp::setupDuplicateFeaturesAction()
 {
-  QgsSettings settings;
-  if ( settings.value( QStringLiteral( "tools/showDuplicateFeatureActions" ), false, QgsSettings::App ).toBool() )
-  {
-    mDuplicateFeatureAction.reset( new QgsMapLayerAction( tr( "Duplicate feature" ),
-                                   nullptr, QgsMapLayerAction::SingleFeature,
-                                   QgsApplication::getThemeIcon( QStringLiteral( "/mActionDuplicateFeature.svg" ) ), true ) );
+  mDuplicateFeatureAction.reset( new QgsMapLayerAction( tr( "Duplicate feature" ),
+                                 nullptr, QgsMapLayerAction::SingleFeature,
+                                 QgsApplication::getThemeIcon( QStringLiteral( "/mActionDuplicateFeature.svg" ) ), QgsMapLayerAction::EnabledOnlyWhenEditable ) );
 
-    QgsGui::mapLayerActionRegistry()->addMapLayerAction( mDuplicateFeatureAction.get() );
-    connect( mDuplicateFeatureAction.get(), &QgsMapLayerAction::triggeredForFeature, this, [this]( QgsMapLayer * layer, const QgsFeature & feat )
-    {
-      duplicateFeatures( layer, feat );
-    }
-           );
+  QgsGui::mapLayerActionRegistry()->addMapLayerAction( mDuplicateFeatureAction.get() );
+  connect( mDuplicateFeatureAction.get(), &QgsMapLayerAction::triggeredForFeature, this, [this]( QgsMapLayer * layer, const QgsFeature & feat )
+  {
+    duplicateFeatures( layer, feat );
+  }
+         );
 
-    mDuplicateFeatureDigitizeAction.reset( new QgsMapLayerAction( tr( "Duplicate feature and digitize" ),
-                                           nullptr, QgsMapLayerAction::SingleFeature,
-                                           QgsApplication::getThemeIcon( QStringLiteral( "/mActionDuplicateFeatureDigitized.svg" ) ), true ) );
+  mDuplicateFeatureDigitizeAction.reset( new QgsMapLayerAction( tr( "Duplicate feature and digitize" ),
+                                         nullptr, QgsMapLayerAction::SingleFeature,
+                                         QgsApplication::getThemeIcon( QStringLiteral( "/mActionDuplicateFeatureDigitized.svg" ) ), QgsMapLayerAction::EnabledOnlyWhenEditable ) );
 
-    QgsGui::mapLayerActionRegistry()->addMapLayerAction( mDuplicateFeatureDigitizeAction.get() );
-    connect( mDuplicateFeatureDigitizeAction.get(), &QgsMapLayerAction::triggeredForFeature, this, [this]( QgsMapLayer * layer, const QgsFeature & feat )
-    {
-      duplicateFeatureDigitized( layer, feat );
-    }
-           );
+  QgsGui::mapLayerActionRegistry()->addMapLayerAction( mDuplicateFeatureDigitizeAction.get() );
+  connect( mDuplicateFeatureDigitizeAction.get(), &QgsMapLayerAction::triggeredForFeature, this, [this]( QgsMapLayer * layer, const QgsFeature & feat )
+  {
+    duplicateFeatureDigitized( layer, feat );
   }
+         );
 }
 
 void QgisApp::setupAtlasMapLayerAction( QgsPrintLayout *layout, bool enableAction )

diff --git a/src/gui/attributetable/qgsattributetableview.cpp b/src/gui/attributetable/qgsattributetableview.cpp
@@ -201,9 +201,6 @@ QWidget *QgsAttributeTableView::createActionWidget( QgsFeatureId fid )
               QgsGui::mapLayerActionRegistry()->mapLayerActions( mFilterModel->layer(),
                   QgsMapLayerAction::SingleFeature ) )
   {
-    if ( !mFilterModel->layer()->isEditable() && mapLayerAction->isEnabledOnlyWhenEditable() )
-      continue;
-
     QAction *action = new QAction( mapLayerAction->icon(), mapLayerAction->text(), container );
     action->setData( "map_layer_action" );
     action->setToolTip( mapLayerAction->text() );

diff --git a/src/gui/attributetable/qgsdualview.cpp b/src/gui/attributetable/qgsdualview.cpp
@@ -592,9 +592,6 @@ void QgsDualView::viewWillShowContextMenu( QMenu *menu, const QModelIndex &atInd
 
     Q_FOREACH ( QgsMapLayerAction *action, registeredActions )
     {
-      if ( !vl->isEditable() && action->isEnabledOnlyWhenEditable() )
-        continue;
-
       QgsAttributeTableMapLayerAction *a = new QgsAttributeTableMapLayerAction( action->text(), this, action, sourceIndex );
 #if QT_VERSION < QT_VERSION_CHECK(5, 6, 0)
       menu->addAction( action->text(), a, SLOT( execut() ) );

diff --git a/src/gui/qgsactionmenu.cpp b/src/gui/qgsactionmenu.cpp
@@ -154,9 +154,6 @@ void QgsActionMenu::reloadActions()
     {
       QgsMapLayerAction *qaction = mapLayerActions.at( i );
 
-      if ( !mLayer->isEditable() && qaction->isEnabledOnlyWhenEditable() )
-        continue;
-
       if ( qaction->isEnabledOnlyWhenEditable() && ( mMode == QgsAttributeForm::AddFeatureMode || mMode == QgsAttributeForm::IdentifyMode ) )
         continue;
 

diff --git a/src/gui/qgsidentifymenu.cpp b/src/gui/qgsidentifymenu.cpp
@@ -346,6 +346,7 @@ void QgsIdentifyMenu::addVectorLayer( QgsVectorLayer *layer, const QList<QgsMapT
     if ( mShowFeatureActions )
     {
       featureActionMenu = new QgsActionMenu( layer, result.mFeature, QStringLiteral( "Feature" ), layerMenu );
+      featureActionMenu->setMode( QgsAttributeForm::IdentifyMode );
       featureActionMenu->setExpressionContextScope( mExpressionContextScope );
     }
 

diff --git a/src/gui/qgsmaplayeractionregistry.cpp b/src/gui/qgsmaplayeractionregistry.cpp
@@ -15,35 +15,36 @@
 
 #include "qgsmaplayeractionregistry.h"
 #include "qgsgui.h"
+#include "qgsvectorlayer.h"
 
-QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, Targets targets, const QIcon &icon, const bool enabledOnlyWhenEditable )
+QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, Targets targets, const QIcon &icon, QgsMapLayerAction::Flags flags )
   : QAction( icon, name, parent )
   , mSingleLayer( false )
   , mSpecificLayerType( false )
   , mLayerType( QgsMapLayer::VectorLayer )
   , mTargets( targets )
-  , mEnabledOnlyWhenEditable( enabledOnlyWhenEditable )
+  , mFlags( flags )
 {
 }
 
-QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, QgsMapLayer *layer, Targets targets, const QIcon &icon, const bool enabledOnlyWhenEditable )
+QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, QgsMapLayer *layer, Targets targets, const QIcon &icon, QgsMapLayerAction::Flags flags )
   : QAction( icon, name, parent )
   , mSingleLayer( true )
   , mActionLayer( layer )
   , mSpecificLayerType( false )
   , mLayerType( QgsMapLayer::VectorLayer )
   , mTargets( targets )
-  , mEnabledOnlyWhenEditable( enabledOnlyWhenEditable )
+  , mFlags( flags )
 {
 }
 
-QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, QgsMapLayer::LayerType layerType, Targets targets, const QIcon &icon, const bool enabledOnlyWhenEditable )
+QgsMapLayerAction::QgsMapLayerAction( const QString &name, QObject *parent, QgsMapLayer::LayerType layerType, Targets targets, const QIcon &icon, QgsMapLayerAction::Flags flags )
   : QAction( icon, name, parent )
   , mSingleLayer( false )
   , mSpecificLayerType( true )
   , mLayerType( layerType )
   , mTargets( targets )
-  , mEnabledOnlyWhenEditable( enabledOnlyWhenEditable )
+  , mFlags( flags )
 {
 }
 
@@ -53,8 +54,24 @@ QgsMapLayerAction::~QgsMapLayerAction()
   QgsGui::mapLayerActionRegistry()->removeMapLayerAction( this );
 }
 
+QgsMapLayerAction::Flags QgsMapLayerAction::flags() const
+{
+  return mFlags;
+}
+
 bool QgsMapLayerAction::canRunUsingLayer( QgsMapLayer *layer ) const
 {
+  if ( mFlags & EnabledOnlyWhenEditable )
+  {
+    // action is only enabled for editable layers
+    if ( !layer )
+      return false;
+    if ( layer->type() != QgsMapLayer::VectorLayer )
+      return false;
+    if ( !qobject_cast<QgsVectorLayer *>( layer )->isEditable() )
+      return false;
+  }
+
   //check layer details
   if ( !mSingleLayer && !mSpecificLayerType )
   {
@@ -67,7 +84,7 @@ bool QgsMapLayerAction::canRunUsingLayer( QgsMapLayer *layer ) const
     //action is a single layer type and layer matches
     return true;
   }
-  else if ( mSpecificLayerType && layer->type() == mLayerType )
+  else if ( mSpecificLayerType && layer && layer->type() == mLayerType )
   {
     //action is for a layer type and layer type matches
     return true;
@@ -91,6 +108,11 @@ void QgsMapLayerAction::triggerForLayer( QgsMapLayer *layer )
   emit triggeredForLayer( layer );
 }
 
+bool QgsMapLayerAction::isEnabledOnlyWhenEditable() const
+{
+  return mFlags & EnabledOnlyWhenEditable;
+}
+
 //
 // Main class begins now...
 //

diff --git a/src/gui/qgsmaplayeractionregistry.h b/src/gui/qgsmaplayeractionregistry.h
@@ -46,29 +46,42 @@ class GUI_EXPORT QgsMapLayerAction : public QAction
     Q_DECLARE_FLAGS( Targets, Target )
     Q_FLAG( Targets )
 
+    /**
+     * Flags which control action behavior
+     * /since QGIS 3.0
+     */
+    enum Flag
+    {
+      EnabledOnlyWhenEditable = 1 << 1, //!< Action should be shown only for editable layers
+    };
+
+    /**
+     * Action behavior flags.
+     * \since QGIS 3.0
+     */
+    Q_DECLARE_FLAGS( Flags, Flag )
+    Q_FLAG( Flags )
+
     /**
      * Creates a map layer action which can run on any layer
      * \note using AllActions as a target probably does not make a lot of sense. This default action was settled for API compatibility reasons.
      */
-#ifndef SIP_RUN
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, Targets targets = AllActions, const QIcon &icon = QIcon(), const bool enabledOnlyWhenEditable = false );
-#else
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, Targets targets = AllActions, const QIcon &icon = QIcon() );
-#endif
+    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = nullptr );
+
     //! Creates a map layer action which can run only on a specific layer
-#ifndef SIP_RUN
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer *layer, Targets targets = AllActions, const QIcon &icon = QIcon(), const bool enabledOnlyWhenEditable = false );
-#else
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer *layer, Targets targets = AllActions, const QIcon &icon = QIcon() );
-#endif
+    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer *layer, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = nullptr );
+
     //! Creates a map layer action which can run on a specific type of layer
-#ifndef SIP_RUN
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer::LayerType layerType, Targets targets = AllActions, const QIcon &icon = QIcon(), const bool enabledOnlyWhenEditable = false );
-#else
-    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer::LayerType layerType, Targets targets = AllActions, const QIcon &icon = QIcon() );
-#endif
+    QgsMapLayerAction( const QString &name, QObject *parent SIP_TRANSFERTHIS, QgsMapLayer::LayerType layerType, Targets targets = AllActions, const QIcon &icon = QIcon(), QgsMapLayerAction::Flags flags = nullptr );
+
     ~QgsMapLayerAction() override;
 
+    /**
+     * Layer behavior flags.
+     * \since QGIS 3.0
+     */
+    QgsMapLayerAction::Flags flags() const;
+
     //! True if action can run using the specified layer
     bool canRunUsingLayer( QgsMapLayer *layer ) const;
 
@@ -86,8 +99,11 @@ class GUI_EXPORT QgsMapLayerAction : public QAction
     //! Return availibity of action
     const Targets &targets() const {return mTargets;}
 
-    //! Return whether only enabled in editable mode
-    bool isEnabledOnlyWhenEditable() const { return mEnabledOnlyWhenEditable; }
+    /**
+     * Returns true if the action is only enabled for layers in editable mode.
+     * \since QGIS 3.0
+     */
+    bool isEnabledOnlyWhenEditable() const;
 
   signals:
     //! Triggered when action has been run for a specific list of features
@@ -102,19 +118,19 @@ class GUI_EXPORT QgsMapLayerAction : public QAction
   private:
 
     // true if action is only valid for a single layer
-    bool mSingleLayer;
+    bool mSingleLayer = false;
     // layer if action is only valid for a single layer
     QgsMapLayer *mActionLayer = nullptr;
 
     // true if action is only valid for a specific layer type
-    bool mSpecificLayerType;
+    bool mSpecificLayerType = false;
     // layer type if action is only valid for a specific layer type
-    QgsMapLayer::LayerType mLayerType;
+    QgsMapLayer::LayerType mLayerType = QgsMapLayer::VectorLayer;
 
     // determine if the action can be run on layer and/or single feature and/or multiple features
-    Targets mTargets;
+    Targets mTargets = nullptr;
 
-    bool mEnabledOnlyWhenEditable;
+    QgsMapLayerAction::Flags mFlags = nullptr;
 };
 
 Q_DECLARE_OPERATORS_FOR_FLAGS( QgsMapLayerAction::Targets )
@@ -168,4 +184,6 @@ class GUI_EXPORT QgsMapLayerActionRegistry : public QObject
 
 };
 
+Q_DECLARE_OPERATORS_FOR_FLAGS( QgsMapLayerAction::Flags )
+
 #endif // QGSMAPLAYERACTIONREGISTRY_H
diff --git a/tests/src/python/CMakeLists.txt b/tests/src/python/CMakeLists.txt
@@ -101,6 +101,7 @@ ADD_PYTHON_TEST(PyQgsLocator test_qgslocator.py)
 ADD_PYTHON_TEST(PyQgsMapCanvas test_qgsmapcanvas.py)
 ADD_PYTHON_TEST(PyQgsMapCanvasAnnotationItem test_qgsmapcanvasannotationitem.py)
 ADD_PYTHON_TEST(PyQgsMapLayer test_qgsmaplayer.py)
+ADD_PYTHON_TEST(PyQgsMapLayerAction test_qgsmaplayeraction.py)
 ADD_PYTHON_TEST(PyQgsMapLayerModel test_qgsmaplayermodel.py)
 ADD_PYTHON_TEST(PyQgsMapLayerStore test_qgsmaplayerstore.py)
 ADD_PYTHON_TEST(PyQgsMapRenderer test_qgsmaprenderer.py)