Rename listActions to actions and fix test

python/core/qgsactionmanager.sip

@@ -59,6 +59,13 @@ class QgsActionManager
      */
     void addAction( const QgsAction& action );
 
+    /**
+     * Remove an action by its id.
+     *
+     * @note Added in QGIS 3.0
+     */
+    void removeAction( const QUuid& actionId );
+
     /** Does the given values. defaultValueIndex is the index of the
      *  field to be used if the action has a $currfield placeholder.
      *  @note available in python bindings as doActionFeature
@@ -80,7 +87,7 @@ class QgsActionManager
      * Return a list of actions that are available in the given action scope.
      * If no action scope is provided, all actions will be returned.
      */
-    QList<QgsAction> listActions( const QString& actionScope = QString() ) const;
+    QList<QgsAction> actions( const QString& actionScope = QString() ) const;
 
     //! Return the layer
     QgsVectorLayer* layer() const;

src/app/qgisapp.cpp

@@ -5812,7 +5812,7 @@ void QgisApp::refreshFeatureActions()
   if ( !vlayer )
     return;
 
-  QList<QgsAction> actions = vlayer->actions()->listActions( QStringLiteral( "Canvas" ) );
+  QList<QgsAction> actions = vlayer->actions()->actions( QStringLiteral( "Canvas" ) );
   Q_FOREACH ( const QgsAction& action, actions )
   {
     QAction* qAction = new QAction( action.icon(), action.shortTitle(), mFeatureActionMenu );
@@ -10623,7 +10623,7 @@ void QgisApp::activateDeactivateLayerRelatedActions( QgsMapLayer* layer )
 
     bool isEditable = vlayer->isEditable();
     bool layerHasSelection = vlayer->selectedFeatureCount() > 0;
-    bool layerHasActions = !vlayer->actions()->listActions( QStringLiteral( "Canvas" ) ).isEmpty() || !QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty();
+    bool layerHasActions = !vlayer->actions()->actions( QStringLiteral( "Canvas" ) ).isEmpty() || !QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty();
 
     mActionLocalHistogramStretch->setEnabled( false );
     mActionFullHistogramStretch->setEnabled( false );
@@ -10909,7 +10909,7 @@ void QgisApp::refreshActionFeatureAction()
   if ( !vlayer )
     return;
 
-  bool layerHasActions = !vlayer->actions()->listActions( QStringLiteral( "Canvas" ) ).isEmpty() || !QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty();
+  bool layerHasActions = !vlayer->actions()->actions( QStringLiteral( "Canvas" ) ).isEmpty() || !QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty();
   mActionFeatureAction->setEnabled( layerHasActions );
 }
 

src/app/qgsattributeactiondialog.cpp

@@ -67,7 +67,7 @@ void QgsAttributeActionDialog::init( const QgsActionManager& actions, const QgsA
 
   int i = 0;
   // Populate with our actions.
-  Q_FOREACH ( const QgsAction& action, actions.listActions() )
+  Q_FOREACH ( const QgsAction& action, actions.actions() )
   {
     insertRow( i++, action );
   }

src/app/qgsattributetabledialog.cpp

@@ -309,7 +309,7 @@ QgsAttributeTableDialog::QgsAttributeTableDialog( QgsVectorLayer *theLayer, QWid
     mActionSearchForm->setToolTip( tr( "Search is not supported when using custom UI forms" ) );
   }
 
-  QList<QgsAction> actions = mLayer->actions()->listActions( QStringLiteral( "Layer" ) );
+  QList<QgsAction> actions = mLayer->actions()->actions( QStringLiteral( "Layer" ) );
 
   if ( actions.isEmpty() )
   {

src/app/qgsfeatureaction.cpp

@@ -66,7 +66,7 @@ QgsAttributeDialog *QgsFeatureAction::newDialog( bool cloneFeature )
   QgsAttributeDialog *dialog = new QgsAttributeDialog( mLayer, f, cloneFeature, parentWidget(), true, context );
   dialog->setWindowFlags( dialog->windowFlags() | Qt::Tool );
 
-  QList<QgsAction> actions = mLayer->actions()->listActions( QStringLiteral( "Feature" ) );
+  QList<QgsAction> actions = mLayer->actions()->actions( QStringLiteral( "Feature" ) );
   if ( !actions.isEmpty() )
   {
     dialog->setContextMenuPolicy( Qt::ActionsContextMenu );

src/app/qgsidentifyresultsdialog.cpp

@@ -480,7 +480,7 @@ void QgsIdentifyResultsDialog::addFeature( QgsVectorLayer *vlayer, const QgsFeat
 
   //get valid QgsMapLayerActions for this layer
   QList< QgsMapLayerAction* > registeredActions = QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer );
-  QList<QgsAction> actions = vlayer->actions()->listActions( QStringLiteral( "Feature" ) );
+  QList<QgsAction> actions = vlayer->actions()->actions( QStringLiteral( "Feature" ) );
 
   if ( !vlayer->fields().isEmpty() || !actions.isEmpty() || !registeredActions.isEmpty() )
   {
@@ -1092,7 +1092,7 @@ void QgsIdentifyResultsDialog::contextMenuEvent( QContextMenuEvent* event )
 
   if ( featItem && vlayer )
   {
-    QList<QgsAction> actions = vlayer->actions()->listActions( QStringLiteral( "Field" ) );
+    QList<QgsAction> actions = vlayer->actions()->actions( QStringLiteral( "Field" ) );
     if ( !actions.isEmpty() )
     {
       mActionPopup->addSeparator();

src/app/qgsmaptoolfeatureaction.cpp

@@ -73,7 +73,7 @@ void QgsMapToolFeatureAction::canvasReleaseEvent( QgsMapMouseEvent* e )
   }
 
   QgsVectorLayer *vlayer = qobject_cast<QgsVectorLayer *>( layer );
-  if ( vlayer->actions()->listActions( QStringLiteral( "Canvas" ) ).isEmpty() && QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty() )
+  if ( vlayer->actions()->actions( QStringLiteral( "Canvas" ) ).isEmpty() && QgsMapLayerActionRegistry::instance()->mapLayerActions( vlayer ).isEmpty() )
   {
     emit messageEmitted( tr( "The active vector layer has no defined actions" ), QgsMessageBar::INFO );
     return;

src/core/qgsactionmanager.cpp

@@ -59,13 +59,27 @@ void QgsActionManager::addAction( const QgsAction& action )
   mActions.append( action );
 }
 
+void QgsActionManager::removeAction( const QUuid& actionId )
+{
+  int i = 0;
+  Q_FOREACH ( const QgsAction& action, mActions )
+  {
+    if ( action.id() == actionId )
+    {
+      mActions.removeAt( i );
+      return;
+    }
+    ++i;
+  }
+}
+
 void QgsActionManager::doAction( const QUuid& actionId, const QgsFeature& feature, int defaultValueIndex )
 {
   QgsExpressionContext context = createExpressionContext();
   QgsExpressionContextScope* actionScope = new QgsExpressionContextScope();
   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 ), true ) );
+    actionScope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "field_name" ), feature.fields().at( defaultValueIndex ).name(), true ) );
   actionScope->addVariable( QgsExpressionContextScope::StaticVariable( QStringLiteral( "field_value" ), feature.attribute( defaultValueIndex ), true ) );
   context << actionScope;
   doAction( actionId, feature, context );
@@ -97,7 +111,7 @@ void QgsActionManager::clearActions()
   mActions.clear();
 }
 
-QList<QgsAction> QgsActionManager::listActions( const QString& actionScope ) const
+QList<QgsAction> QgsActionManager::actions( const QString& actionScope ) const
 {
   if ( actionScope.isNull() )
     return mActions;

src/core/qgsactionmanager.h

@@ -74,6 +74,13 @@ class CORE_EXPORT QgsActionManager
      */
     void addAction( const QgsAction& action );
 
+    /**
+     * Remove an action by its id.
+     *
+     * @note Added in QGIS 3.0
+     */
+    void removeAction( const QUuid& actionId );
+
     /** Does the given values. defaultValueIndex is the index of the
      *  field to be used if the action has a $currfield placeholder.
      *  @note available in python bindings as doActionFeature
@@ -94,8 +101,10 @@ class CORE_EXPORT QgsActionManager
     /**
      * Return a list of actions that are available in the given action scope.
      * If no action scope is provided, all actions will be returned.
+     *
+     * @note Added in QGIS 3.0
      */
-    QList<QgsAction> listActions( const QString& actionScope = QString() ) const;
+    QList<QgsAction> actions( const QString& actionScope = QString() ) const;
 
     //! Return the layer
     QgsVectorLayer* layer() const { return mLayer; }

src/gui/attributetable/qgsattributetableview.cpp

@@ -176,7 +176,7 @@ QWidget* QgsAttributeTableView::createActionWidget( QgsFeatureId fid )
   QAction* defaultAction = nullptr;
 
   // first add user created layer actions
-  QList<QgsAction> actions = mFilterModel->layer()->actions()->listActions( QStringLiteral( "Feature" ) );
+  QList<QgsAction> actions = mFilterModel->layer()->actions()->actions( QStringLiteral( "Feature" ) );
   Q_FOREACH ( const QgsAction& action, actions )
   {
     QString actionTitle = !action.shortTitle().isEmpty() ? action.shortTitle() : action.icon().isNull() ? action.name() : QLatin1String( "" );

src/gui/attributetable/qgsdualview.cpp

@@ -390,7 +390,7 @@ void QgsDualView::viewWillShowContextMenu( QMenu* menu, const QModelIndex& atInd
   }
 
   //add user-defined actions to context menu
-  QList<QgsAction> actions = mLayerCache->layer()->actions()->listActions( QStringLiteral( "Field" ) );
+  QList<QgsAction> actions = mLayerCache->layer()->actions()->actions( QStringLiteral( "Field" ) );
   if ( !actions.isEmpty() )
   {
     QAction* a = menu->addAction( tr( "Run layer action" ) );

src/gui/qgsactionmenu.cpp

@@ -105,7 +105,7 @@ void QgsActionMenu::reloadActions()
 {
   clear();
 
-  mActions = mLayer->actions()->listActions( mActionScope );
+  mActions = mLayer->actions()->actions( mActionScope );
 
   Q_FOREACH ( const QgsAction& action, mActions )
   {

tests/src/python/test_qgsactionmanager.py

@@ -22,7 +22,7 @@
                        QgsField,
                        QgsFields
                        )
-from qgis.PyQt.QtCore import QDir, QTemporaryFile
+from qgis.PyQt.QtCore import QDir, QTemporaryFile, QUuid
 
 from qgis.testing import (start_app,
                           unittest
@@ -70,26 +70,26 @@ def testAddAction(self):
         """ Test adding actions """
 
         # should be empty to start with
-        self.assertEqual(self.manager.listActions(), [])
+        self.assertEqual(self.manager.actions(), [])
 
         # add an action
         action1 = QgsAction(QgsAction.GenericPython, 'Test Action', 'i=1')
         self.manager.addAction(action1)
-        self.assertEqual(len(self.manager.listActions()), 1)
-        self.assertEqual(self.manager.listActions()[0].type(), QgsAction.GenericPython)
-        self.assertEqual(self.manager.listActions()[0].name(), 'Test Action')
-        self.assertEqual(self.manager.listActions()[0].command(), 'i=1')
+        self.assertEqual(len(self.manager.actions()), 1)
+        self.assertEqual(self.manager.actions()[0].type(), QgsAction.GenericPython)
+        self.assertEqual(self.manager.actions()[0].name(), 'Test Action')
+        self.assertEqual(self.manager.actions()[0].command(), 'i=1')
 
         # add another action
         action2 = QgsAction(QgsAction.Windows, 'Test Action2', 'i=2')
         self.manager.addAction(action2)
-        self.assertEqual(len(self.manager.listActions()), 2)
+        self.assertEqual(len(self.manager.actions()), 2)
         self.assertEqual(self.manager.action(action2.id()).type(), QgsAction.Windows)
         self.assertEqual(self.manager.action(action2.id()).name(), 'Test Action2')
         self.assertEqual(self.manager.action(action2.id()).command(), 'i=2')
 
         id3 = self.manager.addAction(QgsAction.Generic, 'Test Action3', 'i=3')
-        self.assertEqual(len(self.manager.listActions()), 3)
+        self.assertEqual(len(self.manager.actions()), 3)
         self.assertEqual(self.manager.action(id3).type(), QgsAction.Generic)
         self.assertEqual(self.manager.action(id3).name(), 'Test Action3')
         self.assertEqual(self.manager.action(id3).command(), 'i=3')
@@ -102,75 +102,52 @@ def testRemoveActions(self):
 
         # clear the manager and check that it's empty
         self.manager.clearActions()
-        self.assertEqual(self.manager.size(), 0)
-        self.assertEqual(self.manager.listActions(), [])
+        self.assertEqual(self.manager.actions(), [])
 
         # add some actions
         id1 = self.manager.addAction(QgsAction.GenericPython, 'test_action', 'i=1')
         id2 = self.manager.addAction(QgsAction.GenericPython, 'test_action2', 'i=2')
         id3 = self.manager.addAction(QgsAction.GenericPython, 'test_action3', 'i=3')
 
         # remove non-existant action
-        self.manager.removeAction(5)
+        self.manager.removeAction(QUuid.createUuid())
 
         # remove them one by one
-        self.manager.removeAction(1)
-        self.assertEqual(self.manager.size(), 2)
-        self.assertEqual(self.manager.listActions()[0].name(), 'test_action')
-        self.assertEqual(self.manager.listActions()[1].name(), 'test_action3')
-        self.manager.removeAction(0)
-        self.assertEqual(self.manager.size(), 1)
-        self.assertEqual(self.manager.listActions()[0].name(), 'test_action3')
-        self.manager.removeAction(0)
-        self.assertEqual(self.manager.size(), 0)
-
-    def testRetrieveAction(self):
-        """ test retrieving actions """
-        self.manager.clearActions()
-
-        # test that exceptions are thrown when retrieving bad indices
-
-        with self.assertRaises(KeyError):
-            self.manager[0]
-
-        with self.assertRaises(KeyError):
-            self.manager.at(0)
-
-        self.manager.addAction(QgsAction.GenericPython, 'test_action', 'i=1')
-
-        with self.assertRaises(KeyError):
-            self.manager[-1]
-
-        with self.assertRaises(KeyError):
-            self.manager.at(-1)
-
-        with self.assertRaises(KeyError):
-            self.manager[5]
-
-        with self.assertRaises(KeyError):
-            self.manager.at(5)
+        self.manager.removeAction(id2)
+        self.assertEqual(len(self.manager.actions()), 2)
+        self.assertEqual(self.manager.action(id1).name(), 'test_action')
+        self.assertEqual(self.manager.action(id3).name(), 'test_action3')
+        self.manager.removeAction(id1)
+        self.assertEqual(len(self.manager.actions()), 1)
+        self.assertEqual(self.manager.action(id3).name(), 'test_action3')
+        self.manager.removeAction(id3)
+        self.assertEqual(len(self.manager.actions()), 0)
 
     def testDefaultAction(self):
         """ test default action for layer"""
 
         self.manager.clearActions()
-        self.manager.addAction(QgsAction.GenericPython, 'test_action', 'i=1')
-        self.manager.addAction(QgsAction.GenericPython, 'test_action2', 'i=2')
+        action1 = QgsAction(QgsAction.GenericPython, 'test_action', '', 'i=1', False, actionScopes={'Feature'})
+        self.manager.addAction(action1)
+        action2 = QgsAction(QgsAction.GenericPython, 'test_action2', 'i=2')
+        self.manager.addAction(action2)
 
         # initially should be not set
-        self.assertEqual(self.manager.defaultAction(), -1)
+        self.assertFalse(self.manager.defaultAction('Feature').isValid())
 
         # set bad default action
-        self.manager.setDefaultAction(10)
-        self.assertEqual(self.manager.defaultAction(), -1)
+        self.manager.setDefaultAction('Feature', QUuid.createUuid())
+        self.assertFalse(self.manager.defaultAction('Feature').isValid())
 
         # set good default action
-        self.manager.setDefaultAction(1)
-        self.assertEqual(self.manager.defaultAction(), 1)
+        self.manager.setDefaultAction('Feature', action1.id())
+        self.assertTrue(self.manager.defaultAction('Feature').isValid())
+        self.assertEquals(self.manager.defaultAction('Feature').id(), action1.id())
+        self.assertNotEquals(self.manager.defaultAction('Feature').id(), action2.id())
 
         # if default action is removed, should be reset to -1
         self.manager.clearActions()
-        self.assertEqual(self.manager.defaultAction(), -1)
+        self.assertFalse(self.manager.defaultAction('Feature').isValid())
 
     def check_action_result(self, temp_file):
         with open(temp_file, 'r') as result:
@@ -185,7 +162,7 @@ def testDoAction(self):
 
         # simple action
         temp_file = self.get_temp_filename()
-        self.manager.addAction(QgsAction.Unix, 'test_action', self.create_action(temp_file, 'test output'))
+        id1 = self.manager.addAction(QgsAction.Unix, 'test_action', self.create_action(temp_file, 'test output'))
 
         fields = QgsFields()
         fields.append(QgsField('my_field'))
@@ -195,28 +172,29 @@ def testDoAction(self):
         f.setAttributes([5, 'val'])
 
         c = QgsExpressionContext()
-        self.manager.doAction(0, f, c)
+        self.manager.doAction(id1, f, c)
         time.sleep(0.5)
 
-        self.assertEqual(self.check_action_result(temp_file), 'test output')
+        self.assertEquals(self.check_action_result(temp_file), 'test output')
 
         # action with substitutions
         temp_file = self.get_temp_filename()
-        self.manager.addAction(QgsAction.Unix, 'test_action', self.create_action(temp_file, 'test [% $id %] output [% @layer_name %]'))
-        self.manager.doAction(1, f, c)
+        id2 = self.manager.addAction(QgsAction.Unix, 'test_action', self.create_action(temp_file, 'test [% $id %] output [% @layer_name %]'))
+        self.manager.doAction(id2, f, c)
         time.sleep(0.5)
 
-        self.assertEqual(self.check_action_result(temp_file), 'test 1 output test_layer')
+        self.assertEquals(self.check_action_result(temp_file), 'test 1 output test_layer')
 
         # test doAction using field variant
         temp_file = self.get_temp_filename()
-        self.manager.addAction(QgsAction.Unix, 'test_action', self.create_action(temp_file, 'test [% @current_field %]'))
-        self.manager.doActionFeature(2, f, 0)
+        id3 = self.manager.addAction(QgsAction.Unix, 'test_action',
+                                     self.create_action(temp_file, 'test : [% @field_index %] : [% @field_name %] : [% @field_value%]'))
+        self.manager.doActionFeature(id3, f, 0)
         time.sleep(0.5)
-        self.assertEqual(self.check_action_result(temp_file), 'test 5')
-        self.manager.doActionFeature(2, f, 1)
+        self.assertEquals(self.check_action_result(temp_file), 'test : 0 : my_field : 5')
+        self.manager.doActionFeature(id3, f, 1)
         time.sleep(0.5)
-        self.assertEqual(self.check_action_result(temp_file), 'test val')
+        self.assertEquals(self.check_action_result(temp_file), 'test : 1 : my_other_field : val')
 
 if __name__ == '__main__':
     unittest.main()