Bugfixes for Action & ActionRouter + testing

README.rst

@@ -70,6 +70,11 @@ The library also provide an abstract base class on which to built I/O implementa
 Changelog
 ---------
 
+dev
+```
+
+* Bugfix for ``actions.Action`` and ``actions.Router``.
+
 0.3.0
 `````
 

slack/__version__.py

@@ -1,3 +1,3 @@
 # Version information
 
-__version__ = '0.3.0'
+__version__ = '0.3.1'

slack/actions.py

@@ -1,3 +1,4 @@
+import json
 import logging
 
 from collections import defaultdict, MutableMapping
@@ -23,10 +24,10 @@ def __init__(self, raw_action, verification_token=None, team_id=None):
         self.action = raw_action
 
         if verification_token and self.action['token'] != verification_token:
-            raise exceptions.FailedVerification(self.action['token'], self.action['team_id'])
+            raise exceptions.FailedVerification(self.action['token'], self.action['team']['id'])
 
-        if team_id and self.action['team_id'] != team_id:
-            raise exceptions.FailedVerification(self.action['token'], self.action['team_id'])
+        if team_id and self.action['team']['id'] != team_id:
+            raise exceptions.FailedVerification(self.action['token'], self.action['team']['id'])
 
     def __getitem__(self, item):
         return self.action[item]
@@ -46,25 +47,35 @@ def __len__(self):
     def __repr__(self):
         return str(self.action)
 
+    @classmethod
+    def from_http(cls, payload, verification_token=None, team_id=None):
+        action = json.loads(payload['payload'])
+        return cls(action, verification_token=verification_token, team_id=team_id)
+
 
 class Router:
     """
     When creating a slack applications you can only set one action url. This provide a routing mechanism for the
-    incoming actions, based on their `callback_id`, to one or more handlers.
+    incoming actions, based on their `callback_id` and the action name, to one or more handlers.
     """
     def __init__(self):
         self._routes = defaultdict(dict)
 
-    def register(self, callback_id, handler):
+    def register(self, callback_id, handler, name='*'):
         """
         Register a new handler for a specific :class:`slack.actions.Action` `callback_id`.
+        Optional routing based on the action name too.
 
         Args:
             callback_id: Callback_id the handler is interested in
             handler: Callback
+            name: Name of the action (optional).
         """
-        LOG.info('Registering %s to %s', callback_id, handler.__name__)
-        self._routes[callback_id].append(handler)
+        LOG.info('Registering %s, %s to %s', callback_id, name, handler.__name__)
+        if name not in self._routes[callback_id]:
+            self._routes[callback_id][name] = []
+
+        self._routes[callback_id][name].append(handler)
 
     def dispatch(self, action):
         """
@@ -76,7 +87,9 @@ def dispatch(self, action):
         Yields:
             handler
         """
-        LOG.debug('Dispatching action %s', action['callback_id'])
+        LOG.debug('Dispatching action %s, %s', action['callback_id'], action['actions'][0]['name'])
 
-        for callback in self._routes.get(action['callback_id']):
-            yield callback
+        if action['actions'][0]['name'] in self._routes[action['callback_id']]:
+            yield from self._routes[action['callback_id']][action['actions'][0]['name']]
+        else:
+            yield from self._routes[action['callback_id']].get('*', [])

tests/conftest.py

@@ -6,6 +6,7 @@
 
 from slack.io.abc import SlackAPI
 from slack.events import Event, EventRouter, MessageRouter
+from slack.actions import Action, Router as ActionRouter
 
 from . import data
 
@@ -110,3 +111,24 @@ def event_router():
 @pytest.fixture()
 def message_router():
     return MessageRouter()
+
+
+@pytest.fixture(params={**data.actions.actions})
+def action(request):
+    return Action.from_http(raw_action(request))
+
+
+@pytest.fixture(params={**data.actions.actions})
+def raw_action(request):
+    if isinstance(request.param, str):
+        if request.param in data.actions.actions:
+            return copy.deepcopy(data.actions.actions[request.param])
+        else:
+            raise ValueError(f'Raw action "{request.param}" not found')
+    else:
+        return copy.deepcopy(request.param)
+
+
+@pytest.fixture()
+def action_router():
+    return ActionRouter()

tests/data/__init__.py

@@ -1 +1 @@
-from . import events, methods  # noQa
+from . import events, methods, actions  # noQa

tests/data/actions.py

@@ -0,0 +1,71 @@
+import json
+
+button_ok = {
+    'type': 'interactive_message',
+    'actions': [
+        {
+            'name': 'ok',
+            'type': 'button',
+            'value': 'hello'
+        }
+    ],
+    'callback_id': 'test_action',
+    'team': {
+        'id': 'T000AAA0A',
+        'domain': 'team'
+    },
+    'channel': {
+        'id': 'C00000A00',
+        'name': 'general'
+    },
+    'user': {
+        'id': 'U000AA000',
+        'name': 'username'
+    },
+    'action_ts': '987654321.000001',
+    'message_ts': '123456789.000001',
+    'attachment_id': '1',
+    'token': 'supersecuretoken',
+    'is_app_unfurl': False,
+    'response_url': 'https://hooks.slack.com/actions/T000AAA0A/123456789123/YTC81HsJRuuGSLVFbSnlkJlh',
+    'trigger_id': '000000000.0000000000.e1bb750705a2f472e4476c4228cf4784'
+}
+
+button_cancel = {
+    'type': 'interactive_message',
+    'actions': [
+        {
+            'name': 'cancel',
+            'type': 'button',
+            'value': 'hello'
+        }
+    ],
+    'callback_id': 'test_action',
+    'team': {
+        'id': 'T000AAA0A',
+        'domain': 'team'
+    },
+    'channel': {
+        'id': 'C00000A00',
+        'name': 'general'
+    },
+    'user': {
+        'id': 'U000AA000',
+        'name': 'username'
+    },
+    'action_ts': '987654321.000001',
+    'message_ts': '123456789.000001',
+    'attachment_id': '1',
+    'token': 'supersecuretoken',
+    'is_app_unfurl': False,
+    'response_url': 'https://hooks.slack.com/actions/T000AAA0A/123456789123/YTC81HsJRuuGSLVFbSnlkJlh',
+    'trigger_id': '000000000.0000000000.e1bb750705a2f472e4476c4228cf4784'
+}
+
+raw_button_ok = {'payload': json.dumps(button_ok)}
+raw_button_cancel = {'payload': json.dumps(button_cancel)}
+
+actions = {
+    'button_ok': raw_button_ok,
+    'button_cancel': raw_button_cancel
+}

tests/test_actions.py

@@ -0,0 +1,103 @@
+import slack
+import pytest
+
+
+class TestActions:
+    def test_from_http(self, action):
+        assert isinstance(action, slack.actions.Action)
+
+    def test_parsing_token(self, raw_action):
+        slack.actions.Action.from_http(raw_action, verification_token='supersecuretoken')
+
+    def test_parsing_team_id(self, raw_action):
+        slack.actions.Action.from_http(raw_action, team_id='T000AAA0A')
+
+    def test_parsing_wrong_token(self, raw_action):
+        with pytest.raises(slack.exceptions.FailedVerification):
+            slack.actions.Action.from_http(raw_action, verification_token='xxx')
+
+    def test_parsing_wrong_team_id(self, raw_action):
+        with pytest.raises(slack.exceptions.FailedVerification):
+            slack.actions.Action.from_http(raw_action, team_id='xxx')
+
+
+class TestActionRouter:
+    def test_register(self, action_router):
+        def handler():
+            pass
+
+        action_router.register('test_action', handler)
+        assert len(action_router._routes['test_action']['*']) == 1
+        assert action_router._routes['test_action']['*'][0] is handler
+
+    def test_register_name(self, action_router):
+        def handler():
+            pass
+
+        action_router.register('test_action', handler, name='aaa')
+        assert len(action_router._routes['test_action']['aaa']) == 1
+        assert action_router._routes['test_action']['aaa'][0] is handler
+
+    def test_multiple_register(self, action_router):
+        def handler():
+            pass
+
+        def handler_bis():
+            pass
+
+        action_router.register('test_action', handler)
+        action_router.register('test_action', handler_bis)
+        assert len(action_router._routes['test_action']['*']) == 2
+        assert action_router._routes['test_action']['*'][0] is handler
+        assert action_router._routes['test_action']['*'][1] is handler_bis
+
+    def test_dispath(self, action, action_router):
+        def handler():
+            pass
+
+        handlers = list()
+        action_router.register('test_action', handler)
+
+        for h in action_router.dispatch(action):
+            handlers.append(h)
+        assert len(handlers) == 1
+        assert handlers[0] is handler
+
+    def test_no_dispatch(self, action, action_router):
+        def handler():
+            pass
+
+        action_router.register('xxx', handler)
+        for h in action_router.dispatch(action):
+            assert False
+
+    def test_dispatch_details(self, action, action_router):
+        def handler():
+            pass
+
+        handlers = list()
+        action_router.register('test_action', handler, name='ok')
+        action_router.register('test_action', handler, name='cancel')
+
+        for h in action_router.dispatch(action):
+            handlers.append(h)
+        assert len(handlers) == 1
+        assert handlers[0] is handler
+
+    def test_multiple_dispatch(self, action, action_router):
+        def handler():
+            pass
+
+        def handler_bis():
+            pass
+
+        handlers = list()
+        action_router.register('test_action', handler)
+        action_router.register('test_action', handler_bis)
+
+        for h in action_router.dispatch(action):
+            handlers.append(h)
+
+        assert len(handlers) == 2
+        assert handlers[0] is handler
+        assert handlers[1] is handler_bis

tests/test_events.py

@@ -34,6 +34,12 @@ def test_parsing(self, raw_event):
             'event_time': 123456789
         }
 
+    def test_parsing_token(self, raw_event):
+        slack.events.Event.from_http(raw_event, verification_token='supersecuretoken')
+
+    def test_parsing_team_id(self, raw_event):
+        slack.events.Event.from_http(raw_event, team_id='T000AAA0A')
+
     def test_parsing_wrong_token(self, raw_event):
         with pytest.raises(slack.exceptions.FailedVerification):
             slack.events.Event.from_http(raw_event, verification_token='xxx')
@@ -148,7 +154,7 @@ def handler():
         for h in event_router.dispatch(event):
             handlers.append(h)
         assert len(handlers) == 1
-        assert handlers[0] == handler
+        assert handlers[0] is handler
 
     @pytest.mark.parametrize('event', {**data.events.events}, indirect=True)
     def test_no_dispatch(self, event, event_router):
@@ -172,7 +178,7 @@ def handler():
         for h in event_router.dispatch(event):
             handlers.append(h)
         assert len(handlers) == 1
-        assert handlers[0] == handler
+        assert handlers[0] is handler
 
     @pytest.mark.parametrize('event', {**data.events.events}, indirect=True)
     def test_multiple_dispatch(self, event, event_router):
@@ -193,8 +199,8 @@ def handler_bis():
         for h in event_router.dispatch(event):
             handlers.append(h)
         assert len(handlers) == 2
-        assert handlers[0] == handler
-        assert handlers[1] == handler_bis
+        assert handlers[0] is handler
+        assert handlers[1] is handler_bis
 
 
 class TestMessageRouter: