Fix regression with generic action notify trigger in notifier service

CHANGELOG.rst

@@ -30,13 +30,13 @@ Changed
   to make it easier for developers to understand. (improvement)
 * Update Python runner code so it prioritizes libraries from pack virtual environment over StackStorm
   system dependencies.
-  
+
   For example, if pack depends on ``six==1.11.0`` in pack ``requirements.txt``, but StackStorm depends
   on ``six==1.10.0``, ``six==1.11.0`` will be used when running Python actions from that pack.
-  
+
   Keep in mind that will not work correctly if pack depends on a library which brakes functionality used
   by Python action wrapper code.
-  
+
   Contributed by Hiroyasu OHYAMA (@userlocalhost). #4571
 
 Fixed
@@ -49,6 +49,11 @@ Fixed
   with items task. (bug fix) #4523
 * Fix orquesta workflow bug where context variables are being overwritten on task join.
   (bug fix) StackStorm/orquesta#112
+* Fix inadvertent regression in notifier service which would cause generic action trigger to only
+  be dispatched for completed states even if custom states were specified using
+  ``action_sensor.emit_when`` config option. (bug fix)
+
+  Reported by Shu Sugimoto (@shusugmt). #4591
 
 2.10.3 - March 06, 2019
 -----------------------
@@ -60,7 +65,7 @@ Fixed
   with ``null`` for the ``Access-Control-Allow-Origin`` header. The fix returns the first of our
   allowed origins if the requesting origin is not a supported origin. Reported by Barak Tawily.
   (bug fix)
-  
+
 2.9.3 - March 06, 2019
 -----------------------
 
@@ -134,7 +139,7 @@ Fixed
   Reported by @johandahlberg (bug fix) #4533
 * Fix ``core.sendmail`` action so it specifies ``charset=UTF-8`` in the ``Content-Type`` email
   header. This way it works correctly when an email subject and / or body contains unicode data.
- 
+
   Reported by @johandahlberg (bug fix) #4533 4534
 
 * Fix CLI ``st2 apikey load`` not being idempotent and API endpoint ``/api/v1/apikeys`` not

st2actions/st2actions/notifier/notifier.py

@@ -97,10 +97,10 @@ def process(self, execution_db):
                     self._post_notify_triggers(liveaction_db=liveaction_db,
                                                execution_db=execution_db)
 
-            if cfg.CONF.action_sensor.enable:
-                with CounterWithTimer(key='notifier.generic_trigger.post'):
-                    self._post_generic_trigger(liveaction_db=liveaction_db,
-                                               execution_db=execution_db)
+        if cfg.CONF.action_sensor.enable:
+            with CounterWithTimer(key='notifier.generic_trigger.post'):
+                self._post_generic_trigger(liveaction_db=liveaction_db,
+                                           execution_db=execution_db)
 
     def _get_execution_for_liveaction(self, liveaction):
         execution = ActionExecution.get(liveaction__id=str(liveaction.id))

st2actions/tests/unit/test_notifier.py

@@ -263,3 +263,83 @@ def test_post_generic_trigger_with_emit_condition(self, dispatch):
                                             payload=exp, trace_context={})
 
         self.assertEqual(dispatch.call_count, 3)
+
+    @mock.patch('oslo_config.cfg.CONF.action_sensor.enable', mock.MagicMock(
+        return_value=True))
+    @mock.patch.object(Notifier, '_get_runner_ref', mock.MagicMock(
+        return_value='local-shell-cmd'))
+    @mock.patch.object(Notifier, '_get_trace_context', mock.MagicMock(
+        return_value={}))
+    @mock.patch('st2common.transport.reactor.TriggerDispatcher.dispatch')
+    @mock.patch('st2actions.notifier.notifier.LiveAction')
+    @mock.patch('st2actions.notifier.notifier.policy_service.apply_post_run_policies', mock.Mock())
+    def test_process_post_generic_notify_trigger_on_completed_state_default(self,
+            mock_LiveAction, mock_dispatch):
+        # Verify that generic action trigger is posted on all completed states when action sensor
+        # is enabled
+        for status in LIVEACTION_STATUSES:
+            notifier = Notifier(connection=None, queues=[])
+
+            liveaction_db = LiveActionDB(id=bson.ObjectId(), action='core.local')
+            liveaction_db.status = status
+            execution = MOCK_EXECUTION
+            execution.liveaction = vars(LiveActionAPI.from_model(liveaction_db))
+            execution.status = liveaction_db.status
+
+            mock_LiveAction.get_by_id.return_value = liveaction_db
+
+            notifier = Notifier(connection=None, queues=[])
+            notifier.process(execution)
+
+            if status in LIVEACTION_COMPLETED_STATES:
+                exp = {'status': status,
+                       'start_timestamp': str(liveaction_db.start_timestamp),
+                       'result': {}, 'parameters': {},
+                       'action_ref': u'core.local',
+                       'runner_ref': 'local-shell-cmd',
+                       'execution_id': str(MOCK_EXECUTION.id),
+                       'action_name': u'core.local'}
+                mock_dispatch.assert_called_with('core.st2.generic.actiontrigger',
+                                                 payload=exp, trace_context={})
+
+        self.assertEqual(mock_dispatch.call_count, len(LIVEACTION_COMPLETED_STATES))
+
+    @mock.patch('oslo_config.cfg.CONF.action_sensor', mock.MagicMock(
+        enable=True, emit_when=['scheduled', 'pending', 'abandoned']))
+    @mock.patch.object(Notifier, '_get_runner_ref', mock.MagicMock(
+        return_value='local-shell-cmd'))
+    @mock.patch.object(Notifier, '_get_trace_context', mock.MagicMock(
+        return_value={}))
+    @mock.patch('st2common.transport.reactor.TriggerDispatcher.dispatch')
+    @mock.patch('st2actions.notifier.notifier.LiveAction')
+    @mock.patch('st2actions.notifier.notifier.policy_service.apply_post_run_policies', mock.Mock())
+    def test_process_post_generic_notify_trigger_on_custom_emit_when_states(self,
+            mock_LiveAction, mock_dispatch):
+        # Verify that generic action trigger is posted on all completed states when action sensor
+        # is enabled
+        for status in LIVEACTION_STATUSES:
+            notifier = Notifier(connection=None, queues=[])
+
+            liveaction_db = LiveActionDB(id=bson.ObjectId(), action='core.local')
+            liveaction_db.status = status
+            execution = MOCK_EXECUTION
+            execution.liveaction = vars(LiveActionAPI.from_model(liveaction_db))
+            execution.status = liveaction_db.status
+
+            mock_LiveAction.get_by_id.return_value = liveaction_db
+
+            notifier = Notifier(connection=None, queues=[])
+            notifier.process(execution)
+
+            if status in ['scheduled', 'pending', 'abandoned']:
+                exp = {'status': status,
+                       'start_timestamp': str(liveaction_db.start_timestamp),
+                       'result': {}, 'parameters': {},
+                       'action_ref': u'core.local',
+                       'runner_ref': 'local-shell-cmd',
+                       'execution_id': str(MOCK_EXECUTION.id),
+                       'action_name': u'core.local'}
+                mock_dispatch.assert_called_with('core.st2.generic.actiontrigger',
+                                                 payload=exp, trace_context={})
+
+        self.assertEqual(mock_dispatch.call_count, 3)