Refactor slack code

api/ask_astro/rest/controllers/__init__.py

@@ -39,4 +39,4 @@ def register_routes(api: Sanic):
             methods=route_config.methods,
             name=route_config.name,
         )
-        logger.info(f"Registered {route_config.methods[0]} {route_config.uri} controller")
+        logger.info("Registered %s %s controller", route_config.methods[0], route_config.uri)

api/ask_astro/rest/controllers/get_request.py

@@ -34,5 +34,5 @@ async def on_get_request(request: Request, request_id: UUID) -> json:
 
         return json(request.to_dict(), status=200)
     except Exception as e:
-        logger.error(f"Error fetching data for request {request_id}: {str(e)}")
+        logger.error("Error fetching data for request %s: %s", request_id, str(e))
         return json({"error": "Internal Server Error"}, status=500)

api/ask_astro/rest/controllers/list_recent_requests.py

@@ -49,5 +49,5 @@ async def on_list_recent_requests(_: Request) -> json:
 
         return json(RecentRequestsResponse(requests=recent_requests).to_dict(), status=200)
     except Exception as e:
-        logger.error(f"Error while fetching recent requests: {e}")
+        logger.error("Error while fetching recent requests: %s", str(e))
         return json({"error": "An error occurred while processing your request."}, status=500)

api/ask_astro/rest/controllers/submit_feedback.py

@@ -37,5 +37,5 @@ async def on_submit_feedback(request: Request, request_id: UUID) -> HTTPResponse
 
         return HTTPResponse(status=200)
     except Exception as e:
-        logger.error(f"Error occurred while processing feedback for request {request_id}: {str(e)}")
+        logger.error("Error occurred while processing feedback for request %s: %s", request_id, str(e))
         return HTTPResponse(text="An internal error occurred.", status=500)

api/ask_astro/services/feedback.py

@@ -31,11 +31,11 @@ async def submit_feedback(request_id: str, correct: bool, source_info: dict[str,
         request = await firestore_client.collection(FirestoreCollections.requests).document(request_id).get()
 
         if not request.exists:
-            raise ValueError(f"Request {request_id} does not exist")
+            raise ValueError("Request %s does not exist", request_id)
 
         langchain_run_id = request.to_dict().get("langchain_run_id")
         if not langchain_run_id:
-            raise ValueError(f"Request {request_id} does not have a langchain run id")
+            raise ValueError("Request %s does not have a langchain run id", request_id)
 
         # update the db and langsmith
         async with asyncio.TaskGroup() as tg:
@@ -57,5 +57,5 @@ async def submit_feedback(request_id: str, correct: bool, source_info: dict[str,
                 )
             )
     except Exception as e:
-        logger.error(f"Error occurred while processing feedback for request {request_id}: {str(e)}")
-        raise FeedbackSubmissionError(f"Failed to submit feedback for request {request_id}.") from e
+        logger.error("Error occurred while processing feedback for request %s: %s", request_id, str(e))
+        raise FeedbackSubmissionError("Failed to submit feedback for request %s.", request_id) from e

api/ask_astro/slack/controllers/__init__.py

@@ -1,4 +1,5 @@
-"Contains a function to register all controllers with the app."
+"""Contains a function to register all controllers with the app."""
+from __future__ import annotations
 
 from logging import getLogger
 
@@ -13,13 +14,22 @@
 def register_controllers(app: AsyncApp):
     """
     Registers all controllers with the app.
+
+    :param app: The Slack AsyncApp instance where controllers need to be registered.
     """
 
-    app.event("app_mention")(on_mention)
-    logger.info("Registered event:app_mention controller")
+    handlers = {
+        "event:app_mention": on_mention,
+        "action:feedback_good": handle_feedback_good,
+        "action:feedback_bad": handle_feedback_bad,
+    }
+
+    for event_action, handler in handlers.items():
+        event_type, identifier = event_action.split(":")
 
-    app.action("feedback_good")(handle_feedback_good)
-    logger.info("Registered action:feedback_good controller")
+        if event_type == "event":
+            app.event(identifier)(handler)
+        elif event_type == "action":
+            app.action(identifier)(handler)
 
-    app.action("feedback_bad")(handle_feedback_bad)
-    logger.info("Registered action:feedback_bad controller")
+        logger.info("Registered %s:%s controller", event_type, identifier)

api/ask_astro/slack/controllers/feedback/bad.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from asyncio import TaskGroup
 from logging import getLogger
 from typing import Any
@@ -10,54 +12,81 @@
 logger = getLogger(__name__)
 
 
-async def handle_feedback_bad(body: dict[str, Any], ack: AsyncAck, respond: AsyncRespond, client: AsyncWebClient):
-    await ack()
+def extract_feedback_details(body: dict[str, Any]) -> dict[str, str]:
+    """
+    Extract necessary details from Slack body for feedback processing.
 
+    :param body: The slack event body.
+    """
     try:
-        user = body["user"]["id"]
-        channel = body["channel"]["id"]
-        thread_ts = body["message"]["thread_ts"]
-        message_ts = body["message"]["ts"]
-        value = body["actions"][0]["value"]
+        return {
+            "user": body["user"]["id"],
+            "channel": body["channel"]["id"],
+            "thread_ts": body["message"]["thread_ts"],
+            "message_ts": body["message"]["ts"],
+            "value": body["actions"][0]["value"],
+        }
     except KeyError as e:
-        logger.error(f"Missing key: {e}")
-        return
+        logger.error("Missing key: %s", e)
+        return {}
 
-    request_id = value.split(":")[0]
 
-    await submit_feedback(request_id, False, source_info={"type": "slack", "user": user})
+async def handle_feedback_bad(
+    body: dict[str, Any], ack: AsyncAck, respond: AsyncRespond, client: AsyncWebClient
+) -> None:
+    """
+    Handle feedback received from Slack and send appropriate responses.
+
+    :param body: The slack event body.
+    :param ack: Acknowledgement object from slack_bolt.
+    :param respond: Response object from slack_bolt.
+    :param client: Slack API client.
+    """
+    await ack()
+
+    details = extract_feedback_details(body)
+    if not details:
+        return
+
+    request_id = details["value"].split(":")[0]
+    await submit_feedback(request_id, False, source_info={"type": "slack", "user": details["user"]})
 
     async with TaskGroup() as tg:
-        tg.create_task(
-            respond(
-                f"😥 Thank you for your feedback, <@{user}>!",
-                thread_ts=thread_ts,
-                replace_original=False,
-                response_type="ephemeral",
-            )
-        )
-
-        async def update_reaction():
-            try:
-                await client.reactions_add(
-                    name="thumbsdown",
-                    channel=channel,
-                    timestamp=message_ts,
-                )
-            except SlackApiError as e:
-                # ignore the error if the reaction already exists
-                if e.response["error"] != "already_reacted":
-                    raise e
-
-            try:
-                await client.reactions_remove(
-                    name="thumbsup",
-                    channel=channel,
-                    timestamp=message_ts,
-                )
-            except Exception:
-                pass
-
-        tg.create_task(
-            update_reaction(),
-        )
+        tg.create_task(_send_response(details, respond))
+        tg.create_task(_update_reaction(details, client))
+
+
+async def _send_response(details: dict[str, str], respond: AsyncRespond) -> None:
+    """
+    Send a response back to the user in Slack.
+
+    :param details: The details extracted from the Slack body.
+    :param respond: Response object from slack_bolt.
+    """
+    await respond(
+        "😥 Thank you for your feedback, <@%s>!" % details["user"],
+        thread_ts=details["thread_ts"],
+        replace_original=False,
+        response_type="ephemeral",
+    )
+
+
+async def _update_reaction(details: dict[str, str], client: AsyncWebClient) -> None:
+    """
+    Add a 'thumbsdown' reaction and remove the 'thumbsup' reaction from the original message.
+
+    :param details: The details extracted from the Slack body.
+    :param client: Slack API client.
+    """
+    try:
+        await client.reactions_add(name="thumbsdown", channel=details["channel"], timestamp=details["message_ts"])
+    except SlackApiError as e:
+        # ignore the error if the reaction already exists
+        if e.response["error"] != "already_reacted":
+            raise e
+
+    try:
+        await client.reactions_remove(name="thumbsup", channel=details["channel"], timestamp=details["message_ts"])
+    except Exception as e:
+        logger.debug("Failed to remove thumbsup reaction: %s", e)
+        pass

api/ask_astro/slack/controllers/feedback/good.py

@@ -1,3 +1,5 @@
+from __future__ import annotations
+
 from asyncio import TaskGroup
 from logging import getLogger
 from typing import Any
@@ -10,54 +12,81 @@
 logger = getLogger(__name__)
 
 
-async def handle_feedback_good(body: dict[str, Any], ack: AsyncAck, respond: AsyncRespond, client: AsyncWebClient):
-    await ack()
+def extract_feedback_details(body: dict[str, Any]) -> dict[str, str] | None:
+    """
+    Extract necessary details from Slack body for feedback processing.
 
+    :param body: The slack event body.
+    """
     try:
-        user = body["user"]["id"]
-        channel = body["channel"]["id"]
-        thread_ts = body["message"]["thread_ts"]
-        message_ts = body["message"]["ts"]
-        value = body["actions"][0]["value"]
+        return {
+            "user": body["user"]["id"],
+            "channel": body["channel"]["id"],
+            "thread_ts": body["message"]["thread_ts"],
+            "message_ts": body["message"]["ts"],
+            "value": body["actions"][0]["value"],
+        }
     except KeyError as e:
-        logger.error(f"Missing key: {e}")
-        return
+        logger.error("Missing key: %s", e)
+        return None
 
-    request_id = value.split(":")[0]
 
-    await submit_feedback(request_id, True, source_info={"type": "slack", "user": user})
+async def handle_feedback_good(
+    body: dict[str, Any], ack: AsyncAck, respond: AsyncRespond, client: AsyncWebClient
+) -> None:
+    """
+    Handle positive feedback received from Slack and send appropriate responses.
+
+    :param body: The slack event body.
+    :param ack: Acknowledgement object from slack_bolt.
+    :param respond: Response object from slack_bolt.
+    :param client: Slack API client.
+    """
+    await ack()
+
+    details = extract_feedback_details(body)
+    if not details:
+        return
+
+    request_id = details["value"].split(":")[0]
+    await submit_feedback(request_id, True, source_info={"type": "slack", "user": details["user"]})
 
     async with TaskGroup() as tg:
-        tg.create_task(
-            respond(
-                f"☺️ Thank you for your feedback, <@{user}>!",
-                thread_ts=thread_ts,
-                replace_original=False,
-                response_type="ephemeral",
-            )
-        )
-
-        async def update_reaction():
-            try:
-                await client.reactions_add(
-                    name="thumbsup",
-                    channel=channel,
-                    timestamp=message_ts,
-                )
-            except SlackApiError as e:
-                # ignore the error if the reaction already exists
-                if e.response["error"] != "already_reacted":
-                    raise e
-
-            try:
-                await client.reactions_remove(
-                    name="thumbsdown",
-                    channel=channel,
-                    timestamp=message_ts,
-                )
-            except Exception:
-                pass
-
-        tg.create_task(
-            update_reaction(),
-        )
+        tg.create_task(_send_response(details, respond))
+        tg.create_task(_update_reaction(details, client))
+
+
+async def _send_response(details: dict[str, str], respond: AsyncRespond) -> None:
+    """
+    Send a positive response back to the user in Slack.
+
+    :param details: The details extracted from the Slack body.
+    :param respond: Response object from slack_bolt.
+    """
+    await respond(
+        "☺️ Thank you for your feedback, <@%s>!" % details["user"],
+        thread_ts=details["thread_ts"],
+        replace_original=False,
+        response_type="ephemeral",
+    )
+
+
+async def _update_reaction(details: dict[str, str], client: AsyncWebClient) -> None:
+    """
+    Add a 'thumbsup' reaction and remove the 'thumbsdown' reaction from the original message.
+
+    :param details: The details extracted from the Slack body.
+    :param client: Slack API client.
+    """
+    try:
+        await client.reactions_add(name="thumbsup", channel=details["channel"], timestamp=details["message_ts"])
+    except SlackApiError as e:
+        # ignore the error if the reaction already exists
+        if e.response["error"] != "already_reacted":
+            raise e
+
+    try:
+        await client.reactions_remove(name="thumbsdown", channel=details["channel"], timestamp=details["message_ts"])
+    except Exception as e:
+        logger.debug("Failed to remove thumbsdown reaction: %s", e)
+        pass