Fix slackapi#80 by making app.action/options more flexible

seratch · seratch · commit a65c125ccc8b · 2020-09-12T17:00:23.000+09:00
diff --git a/samples/dialogs_app.py b/samples/dialogs_app.py
@@ -51,8 +51,13 @@ def test_command(body, client, ack, logger):
     logger.info(res)
 
 
-@app.action({"type": "dialog_submission", "callback_id": "dialog-callback-id"})
-def dialog_submission(ack: Ack, body: dict):
+@app.action("dialog-callback-id")
+def dialog_submission_or_cancellation(ack: Ack, body: dict):
+    if body["type"] == "dialog_cancellation":
+        # This can be sent only when notify_on_cancel is True
+        ack()
+        return
+
     errors = []
     submission = body["submission"]
     if len(submission["loc_origin"]) <= 3:
@@ -69,7 +74,30 @@ def dialog_submission(ack: Ack, body: dict):
         ack()
 
 
-@app.options({"type": "dialog_suggestion", "callback_id": "dialog-callback-id"})
+# @app.action({"type": "dialog_submission", "callback_id": "dialog-callback-id"})
+# def dialog_submission_or_cancellation(ack: Ack, body: dict):
+#     errors = []
+#     submission = body["submission"]
+#     if len(submission["loc_origin"]) <= 3:
+#         errors = [
+#             {
+#                 "name": "loc_origin",
+#                 "error": "Pickup Location must be longer than 3 characters"
+#             }
+#         ]
+#     if len(errors) > 0:
+#         # or ack({"errors": errors})
+#         ack(errors=errors)
+#     else:
+#         ack()
+#
+# @app.action({"type": "dialog_cancellation", "callback_id": "dialog-callback-id"})
+# def dialog_cancellation(ack):
+#     ack()
+
+
+# @app.options({"type": "dialog_suggestion", "callback_id": "dialog-callback-id"})
+@app.options("dialog-callback-id")
 def dialog_suggestion(ack):
     ack(
         {
@@ -88,10 +116,6 @@ def dialog_suggestion(ack):
     )
 
 
-@app.action({"type": "dialog_cancellation", "callback_id": "dialog-callback-id"})
-def dialog_cancellation(ack):
-    ack()
-
 
 if __name__ == "__main__":
     app.start(3000)
diff --git a/slack_bolt/listener_matcher/builtins.py b/slack_bolt/listener_matcher/builtins.py
@@ -179,7 +179,18 @@ def action(
     asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     if isinstance(constraints, (str, Pattern)):
-        return block_action(constraints, asyncio)
+
+        def func(body: Dict[str, Any]) -> bool:
+            return (
+                _block_action(constraints, body)
+                or _attachment_action(constraints, body)
+                or _dialog_submission(constraints, body)
+                or _dialog_cancellation(constraints, body)
+                or _workflow_step_edit(constraints, body)
+            )
+
+        return build_listener_matcher(func, asyncio)
+
     elif "type" in constraints:
         action_type = constraints["type"]
         if action_type == "block_actions":
@@ -206,66 +217,89 @@ def action(
     )
 
 
+def _block_action(
+    constraints: Union[str, Pattern, Dict[str, Union[str, Pattern]]],
+    body: Dict[str, Any],
+) -> bool:
+    if is_block_actions(body) is False:
+        return False
+
+    action = to_action(body)
+    if isinstance(constraints, (str, Pattern)):
+        action_id = constraints
+        return _matches(action_id, action["action_id"])
+    elif isinstance(constraints, dict):
+        # block_id matching is optional
+        block_id: Optional[Union[str, Pattern]] = constraints.get("block_id")
+        block_id_matched = block_id is None or _matches(
+            block_id, action.get("block_id")
+        )
+        action_id_matched = _matches(constraints["action_id"], action["action_id"])
+        return block_id_matched and action_id_matched
+
+
 def block_action(
     constraints: Union[str, Pattern, Dict[str, Union[str, Pattern]]],
     asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        if is_block_actions(body) is False:
-            return False
-
-        action = to_action(body)
-        if isinstance(constraints, (str, Pattern)):
-            action_id = constraints
-            return _matches(action_id, action["action_id"])
-        elif isinstance(constraints, dict):
-            # block_id matching is optional
-            block_id: Optional[Union[str, Pattern]] = constraints.get("block_id")
-            block_id_matched = block_id is None or _matches(
-                block_id, action.get("block_id")
-            )
-            action_id_matched = _matches(constraints["action_id"], action["action_id"])
-            return block_id_matched and action_id_matched
+        return _block_action(constraints, body)
 
     return build_listener_matcher(func, asyncio)
 
 
+def _attachment_action(callback_id: Union[str, Pattern], body: Dict[str, Any],) -> bool:
+    return is_attachment_action(body) and _matches(callback_id, body["callback_id"])
+
+
 def attachment_action(
     callback_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_attachment_action(body) and _matches(callback_id, body["callback_id"])
+        return _attachment_action(callback_id, body)
 
     return build_listener_matcher(func, asyncio)
 
 
+def _dialog_submission(callback_id: Union[str, Pattern], body: Dict[str, Any],) -> bool:
+    return is_dialog_submission(body) and _matches(callback_id, body["callback_id"])
+
+
 def dialog_submission(
     callback_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_dialog_submission(body) and _matches(callback_id, body["callback_id"])
+        return _dialog_submission(callback_id, body)
 
     return build_listener_matcher(func, asyncio)
 
 
+def _dialog_cancellation(
+    callback_id: Union[str, Pattern], body: Dict[str, Any],
+) -> bool:
+    return is_dialog_cancellation(body) and _matches(callback_id, body["callback_id"])
+
+
 def dialog_cancellation(
     callback_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_dialog_cancellation(body) and _matches(
-            callback_id, body["callback_id"]
-        )
+        return _dialog_cancellation(callback_id, body)
 
     return build_listener_matcher(func, asyncio)
 
 
+def _workflow_step_edit(
+    callback_id: Union[str, Pattern], body: Dict[str, Any],
+) -> bool:
+    return is_workflow_step_edit(body) and _matches(callback_id, body["callback_id"])
+
+
 def workflow_step_edit(
     callback_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_workflow_step_edit(body) and _matches(
-            callback_id, body["callback_id"]
-        )
+        return _workflow_step_edit(callback_id, body)
 
     return build_listener_matcher(func, asyncio)
 
@@ -322,7 +356,14 @@ def options(
     asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     if isinstance(constraints, (str, Pattern)):
-        return block_suggestion(constraints, asyncio)
+
+        def func(body: Dict[str, Any]) -> bool:
+            return _block_suggestion(constraints, body) or _dialog_suggestion(
+                constraints, body
+            )
+
+        return build_listener_matcher(func, asyncio)
+
     if "action_id" in constraints:
         return block_suggestion(constraints["action_id"], asyncio)
     if "callback_id" in constraints:
@@ -333,20 +374,28 @@ def options(
         )
 
 
+def _block_suggestion(action_id: Union[str, Pattern], body: Dict[str, Any],) -> bool:
+    return is_block_suggestion(body) and _matches(action_id, body["action_id"])
+
+
 def block_suggestion(
     action_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_block_suggestion(body) and _matches(action_id, body["action_id"])
+        return _block_suggestion(action_id, body)
 
     return build_listener_matcher(func, asyncio)
 
 
+def _dialog_suggestion(callback_id: Union[str, Pattern], body: Dict[str, Any],) -> bool:
+    return is_dialog_suggestion(body) and _matches(callback_id, body["callback_id"])
+
+
 def dialog_suggestion(
     callback_id: Union[str, Pattern], asyncio: bool = False,
 ) -> Union[ListenerMatcher, "AsyncListenerMatcher"]:
     def func(body: Dict[str, Any]) -> bool:
-        return is_dialog_suggestion(body) and _matches(callback_id, body["callback_id"])
+        return _dialog_suggestion(callback_id, body)
 
     return build_listener_matcher(func, asyncio)
 
diff --git a/tests/scenario_tests/test_attachment_actions.py b/tests/scenario_tests/test_attachment_actions.py
@@ -51,6 +51,15 @@ def test_mock_server_is_running(self):
         resp = self.web_client.api_test()
         assert resp != None
 
+    def test_success_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        app.action("pick_channel_for_fun")(simple_listener)
+
+        request = self.build_valid_request()
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert self.mock_received_requests["/auth.test"] == 1
+
     def test_success(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         app.action(
@@ -86,6 +95,18 @@ def test_process_before_response(self):
         assert response.status == 200
         assert self.mock_received_requests["/auth.test"] == 1
 
+    def test_failure_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        request = self.build_valid_request()
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 1
+
+        app.action("unknown")(simple_listener)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 2
+
     def test_failure(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         request = self.build_valid_request()
diff --git a/tests/scenario_tests/test_dialogs.py b/tests/scenario_tests/test_dialogs.py
@@ -49,6 +49,30 @@ def test_mock_server_is_running(self):
         resp = self.web_client.api_test()
         assert resp != None
 
+    def test_success_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        app.options("dialog-callback-id")(handle_suggestion)
+        app.action("dialog-callback-id")(handle_submission_cancellation)
+
+        request = self.build_valid_request(suggestion_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert response.body != ""
+        assert response.headers["content-type"][0] == "application/json;charset=utf-8"
+        assert self.mock_received_requests["/auth.test"] == 1
+
+        request = self.build_valid_request(submission_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert response.body == ""
+        assert self.mock_received_requests["/auth.test"] == 2
+
+        request = self.build_valid_request(cancellation_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 200
+        assert response.body == ""
+        assert self.mock_received_requests["/auth.test"] == 3
+
     def test_success(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         app.options({"type": "dialog_suggestion", "callback_id": "dialog-callback-id"})(
@@ -169,6 +193,18 @@ def test_process_before_response_2(self):
         assert response.body == ""
         assert self.mock_received_requests["/auth.test"] == 3
 
+    def test_suggestion_failure_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        request = self.build_valid_request(suggestion_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 1
+
+        app.options("dialog-callback-iddddd")(handle_suggestion)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 2
+
     def test_suggestion_failure(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         request = self.build_valid_request(suggestion_raw_body)
@@ -195,6 +231,18 @@ def test_suggestion_failure_2(self):
         assert response.status == 404
         assert self.mock_received_requests["/auth.test"] == 2
 
+    def test_submission_failure_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        request = self.build_valid_request(suggestion_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 1
+
+        app.action("dialog-callback-iddddd")(handle_submission)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 2
+
     def test_submission_failure(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         request = self.build_valid_request(suggestion_raw_body)
@@ -221,6 +269,18 @@ def test_submission_failure_2(self):
         assert response.status == 404
         assert self.mock_received_requests["/auth.test"] == 2
 
+    def test_cancellation_failure_without_type(self):
+        app = App(client=self.web_client, signing_secret=self.signing_secret,)
+        request = self.build_valid_request(suggestion_raw_body)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 1
+
+        app.action("dialog-callback-iddddd")(handle_cancellation)
+        response = app.dispatch(request)
+        assert response.status == 404
+        assert self.mock_received_requests["/auth.test"] == 2
+
     def test_cancellation_failure(self):
         app = App(client=self.web_client, signing_secret=self.signing_secret,)
         request = self.build_valid_request(suggestion_raw_body)
@@ -336,3 +396,9 @@ def handle_cancellation(ack, body, payload, action):
     assert body == action
     assert payload == action
     ack()
+
+
+def handle_submission_cancellation(ack, body, payload, action):
+    assert body == action
+    assert payload == action
+    ack()
diff --git a/tests/scenario_tests_async/test_attachment_actions.py b/tests/scenario_tests_async/test_attachment_actions.py
diff --git a/tests/scenario_tests_async/test_dialogs.py b/tests/scenario_tests_async/test_dialogs.py
