Overhaul deleted message handling (#73)

* Update DB when unthreaded messages are deleted

* Add required library libatomic1 to Dockerfile (#71)

* Handle quick message deletions

This is a quick and dirty way to handle the bug, but it works.

I also realised that handle_question_deletion() doesn't delete it in #tickets

* Create a system for tracking bot messages attached to tickets

* Ensure resolved message is added to userFacingMsgs

TODO I''ll need to make a function that all the macros can use to send a user-facing message

* Add a reply_to_ticket() helper for updating the DB
while replying to a ticket

* Fix type errors in reply_to_ticket()

* Use reply_to_ticket when resolving tickets

* Fix type errors in message.py

* Create delete_replies_to_ticket()

* Remove unused parameter in delete_replies_to_ticket

* Rename BotMessage.msgTs to BotMessage.ts

* Write a stub delete_and_clean_up_ticket function

* Partially implement delete_and_clean_up_ticket

* Delete "ticket" message in delete_and_clean_up_ticket()

* Use the new ticket methods where appropriate

* Make function name clearer ("delete_bot_replies")

* Log if a question is deleted but not present in DB

* Fix error when normal messages are deleted

* Actually include userFacingMsgs in the query when deleting bot replies

* Add success heartbeat to delete queue

* Document the deletion queue needing workspace admin

* Don't use delete queue in ticket_methods.py

This is because the delete queue requires an admin token, ew

* Fix deleting the backend message on ticket deletion

* Always preserve support threads with >3 bot messages

This is so that if someone runs ?faq, the ticket still counts as being resolved in the DB.

* Debug-log message event data instead of printing

* Use the reply_to_ticket util in macros

---------

Co-authored-by: RandomSearch <101704343+RandomSearch18@users.noreply.github.com>

nephthys/actions/resolve.py

@@ -6,6 +6,7 @@ from nephthys.utils.delete_thread import add_thread_to_delete_queue
 from nephthys.utils.env import env
 from nephthys.utils.logging import send_heartbeat
 from nephthys.utils.permissions import can_resolve
+from nephthys.utils.ticket_methods import reply_to_ticket
 from prisma.enums import TicketStatus
 
 
@@ -63,14 +64,13 @@ async def resolve(
         return
 
     if send_resolved_message:
-        await client.chat_postMessage(
+        await reply_to_ticket(
-            channel=env.slack_help_channel,
+            ticket=tkt,
+            client=client,
             text=env.transcript.ticket_resolve.format(user_id=resolver)
             if not stale
             else env.transcript.ticket_resolve_stale.format(user_id=resolver),
-            thread_ts=ts,
         )
-
     if add_reaction:
         await client.reactions_add(
             channel=env.slack_help_channel,

nephthys/events/message.py

@@ -3,11 +3,13 @@ from datetime import datetime
 from typing import Any
 from typing import Dict
 
+from slack_sdk.errors import SlackApiError
 from slack_sdk.web.async_client import AsyncWebClient
 
 from nephthys.macros import run_macro
 from nephthys.utils.env import env
 from nephthys.utils.logging import send_heartbeat
+from nephthys.utils.ticket_methods import delete_and_clean_up_ticket
 from prisma.enums import TicketStatus
 
 ALLOWED_SUBTYPES = ["file_share", "me_message", "thread_broadcast"]
@@ -46,18 +48,18 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
 
     if event.get("thread_ts"):
         if db_user and db_user.helper:
-            ticket = await env.db.ticket.find_first(
+            ticket_message = await env.db.ticket.find_first(
                 where={"msgTs": event["thread_ts"]},
                 include={"openedBy": True, "tagsOnTickets": True},
             )
-            if not ticket or ticket.status == TicketStatus.CLOSED:
+            if not ticket_message or ticket_message.status == TicketStatus.CLOSED:
                 return
             first_word = text.split()[0].lower()
 
-            if first_word[0] == "?" and ticket:
+            if first_word[0] == "?" and ticket_message:
                 await run_macro(
                     name=first_word.lstrip("?"),
-                    ticket=ticket,
+                    ticket=ticket_message,
                     helper=db_user,
                     text=text,
                     macro_ts=event["ts"],
@@ -70,8 +72,8 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
                         "status": TicketStatus.IN_PROGRESS,
                         "assignedAt": (
                             datetime.now()
-                            if not ticket.assignedAt
+                            if not ticket_message.assignedAt
-                            else ticket.assignedAt
+                            else ticket_message.assignedAt
                         ),
                     },
                 )
@@ -103,17 +105,15 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
             },
         )
 
-    user_info = await client.users_info(user=user)
+    user_info_response = await client.users_info(user=user) or {}
+    user_info = user_info_response.get("user")
     profile_pic = None
     display_name = "Explorer"
     if user_info:
-        profile_pic = user_info["user"]["profile"].get("image_512", "")
+        profile_pic = user_info["profile"].get("image_512", "")
-        display_name = (
+        display_name = user_info["profile"]["display_name"] or user_info["real_name"]
-            user_info["user"]["profile"]["display_name"]
-            or user_info["user"]["real_name"]
-        )
 
-    ticket = await client.chat_postMessage(
+    ticket_message = await client.chat_postMessage(
         channel=env.slack_ticket_channel,
         text=f"New message from <@{user}>: {text}",
         blocks=[
@@ -143,6 +143,11 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
         unfurl_media=True,
     )
 
+    ticket_message_ts = ticket_message["ts"]
+    if not ticket_message_ts:
+        logging.error(f"Ticket message has no ts: {ticket_message}")
+        return
+
     async with env.session.post(
         "https://ai.hackclub.com/chat/completions",
         json={
@@ -167,28 +172,21 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
             data = await res.json()
             title = data["choices"][0]["message"]["content"].strip()
 
-    await env.db.ticket.create(
+    user_facing_message_text = (
-        {
-            "title": title,
-            "description": text,
-            "msgTs": event["ts"],
-            "ticketTs": ticket["ts"],
-            "openedBy": {"connect": {"id": db_user.id}},
-        },
-    )
-
-    text = (
         env.transcript.first_ticket_create.replace("(user)", display_name)
         if past_tickets == 0
         else env.transcript.ticket_create.replace("(user)", display_name)
     )
-    ticket_url = f"https://hackclub.slack.com/archives/{env.slack_ticket_channel}/p{ticket['ts'].replace('.', '')}"
+    ticket_url = f"https://hackclub.slack.com/archives/{env.slack_ticket_channel}/p{ticket_message_ts.replace('.', '')}"
 
-    await client.chat_postMessage(
+    user_facing_message = await client.chat_postMessage(
         channel=event["channel"],
-        text=text,
+        text=user_facing_message_text,
         blocks=[
-            {"type": "section", "text": {"type": "mrkdwn", "text": text}},
+            {
+                "type": "section",
+                "text": {"type": "mrkdwn", "text": user_facing_message_text},
+            },
             {
                 "type": "actions",
                 "elements": [
@@ -216,10 +214,38 @@ async def on_message(event: Dict[str, Any], client: AsyncWebClient):
         unfurl_media=True,
     )
 
-    await client.reactions_add(
+    user_facing_message_ts = user_facing_message["ts"]
-        channel=event["channel"], name="thinking_face", timestamp=event["ts"]
+    if not user_facing_message_ts:
+        logging.error(f"User-facing message has no ts: {user_facing_message}")
+        return
+
+    ticket = await env.db.ticket.create(
+        {
+            "title": title,
+            "description": text,
+            "msgTs": event["ts"],
+            "ticketTs": ticket_message_ts,
+            "openedBy": {"connect": {"id": db_user.id}},
+            "userFacingMsgs": {
+                "create": {
+                    "channelId": event["channel"],
+                    "ts": user_facing_message_ts,
+                }
+            },
+        },
     )
 
+    try:
+        await client.reactions_add(
+            channel=event["channel"], name="thinking_face", timestamp=event["ts"]
+        )
+    except SlackApiError as e:
+        if e.response.get("error") != "message_not_found":
+            raise e
+        # This means the parent message has been deleted while we've been processing it
+        # therefore we should unsend the bot messages and remove the ticket from the DB
+        await delete_and_clean_up_ticket(ticket)
+
     if env.uptime_url and env.environment == "production":
         async with env.session.get(env.uptime_url) as res:
             if res.status != 200:

nephthys/events/message_deletion.py

@@ -7,6 +7,7 @@ from slack_sdk.web.async_client import AsyncWebClient
 
 from nephthys.utils.env import env
 from nephthys.utils.logging import send_heartbeat
+from nephthys.utils.ticket_methods import delete_and_clean_up_ticket
 
 
 async def handle_question_deletion(
@@ -30,41 +31,51 @@ async def handle_question_deletion(
             raise e
     bot_info = await env.slack_client.auth_test()
     bot_user_id = bot_info.get("user_id")
-    messages_to_delete = []
+    bot_replies = []
+    non_bot_replies = []
     for msg in thread_history["messages"]:
+        if msg["ts"] == deleted_msg["ts"]:
+            continue  # Ignore top-level message
         if msg["user"] == bot_user_id:
-            messages_to_delete.append(msg)
+            bot_replies.append(msg)
-        elif msg["ts"] != deleted_msg["ts"]:
+        else:
-            # Don't clear the thread if there are non-bot messages in there
+            non_bot_replies.append(msg)
-            return
 
-    # Delete ticket from DB
+    should_keep_thread = (
-    await env.db.ticket.delete(where={"msgTs": deleted_msg["ts"]})
+        # Preserve if there are any human replies
-
+        len(non_bot_replies) > 0
-    # Delete messages
+        # More than 2 bot replies implies someone ran ?faq or something, so we'll preserve the ticket
-    await send_heartbeat(
+        or len(bot_replies) > 2
-        f"Removing my {len(messages_to_delete)} message(s) in a thread because the question was deleted."
     )
-    for msg in messages_to_delete:
+    if should_keep_thread:
-        await client.chat_delete(
+        return
-            channel=channel,
+
-            ts=msg["ts"],
+    # Delete ticket from DB and clean up bot messages
-        )
+    ticket = await env.db.ticket.find_first(where={"msgTs": deleted_msg["ts"]})
+    if not ticket:
+        message = f"Deleted question doesn't have an associated ticket in DB, ts={deleted_msg['ts']}"
+        logging.warning(message)
+        await send_heartbeat(message)
+        return
+    await delete_and_clean_up_ticket(ticket)
 
 
 async def on_message_deletion(event: Dict[str, Any], client: AsyncWebClient) -> None:
     """Handles the two types of message deletion events
     (i.e. a message being turned into a tombstone, and a message being fully deleted)."""
-    if event.get("subtype") == "message_deleted":
-        # This means the message has been completely deleted with out leaving a "tombstone", so no cleanup to do
-        return
     deleted_msg = event.get("previous_message")
     if not deleted_msg:
         logging.warning("No previous_message found in message deletion event")
         return
-    is_top_level_message = (
+    is_in_thread = (
-        "thread_ts" not in deleted_msg or deleted_msg["ts"] == deleted_msg["thread_ts"]
+        "thread_ts" in deleted_msg and deleted_msg["ts"] != deleted_msg["thread_ts"]
     )
-    if is_top_level_message:
+    if is_in_thread:
-        # A question (i.e. top-level message in help channel) has been deleted
+        return
+    if event.get("subtype") == "message_deleted":
+        # This means the message has been completely deleted with out leaving a "tombstone"
+        # No thread means no messages to delete, but we should delete any associated ticket from the DB
+        await env.db.ticket.delete(where={"msgTs": deleted_msg["ts"]})
+    else:
+        # A parent message (i.e. top-level message in help channel) has been deleted
         await handle_question_deletion(client, event["channel"], deleted_msg)

nephthys/macros/faq.py

@@ -1,6 +1,7 @@
 from nephthys.actions.resolve import resolve
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import reply_to_ticket
 
 
 class FAQ(Macro):
@@ -19,10 +20,10 @@ class FAQ(Macro):
             or user_info["user"]["profile"].get("real_name")
             or user_info["user"]["name"]
         )
-        await env.slack_client.chat_postMessage(
+        await reply_to_ticket(
             text=f"hey, {name}! this question is answered in the faq i sent earlier, please make sure to check it out! :rac_cute:\n\n<{env.transcript.faq_link}|here it is again>",
-            channel=env.slack_help_channel,
+            ticket=ticket,
-            thread_ts=ticket.msgTs,
+            client=env.slack_client,
         )
         await resolve(
             ts=ticket.msgTs,

nephthys/macros/fraud.py

@@ -1,6 +1,7 @@
 from nephthys.actions.resolve import resolve
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import reply_to_ticket
 
 
 class Fraud(Macro):
@@ -19,10 +20,10 @@ class Fraud(Macro):
             or user_info["user"]["profile"].get("real_name")
             or user_info["user"]["name"]
         )
-        await env.slack_client.chat_postMessage(
+        await reply_to_ticket(
             text=f"Hiya {name}! Would you mind directing any fraud related queries to <@U091HC53CE8>? :rac_cute:\n\nIt'll keep your case confidential and make it easier for the fraud team to keep track of!",
-            channel=env.slack_help_channel,
+            ticket=ticket,
-            thread_ts=ticket.msgTs,
+            client=env.slack_client,
         )
         await resolve(
             ts=ticket.msgTs,

nephthys/macros/hello_world.py

@@ -1,5 +1,6 @@
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import reply_to_ticket
 
 
 class HelloWorld(Macro):
@@ -15,8 +16,8 @@ class HelloWorld(Macro):
             or user_info["user"]["profile"].get("real_name")
             or user_info["user"]["name"]
         )
-        await env.slack_client.chat_postMessage(
+        await reply_to_ticket(
             text=f"hey, {name}! i'm heidi :rac_shy: say hi to orpheus for me would you? :rac_cute:",
-            channel=env.slack_help_channel,
+            ticket=ticket,
-            thread_ts=ticket.msgTs,
+            client=env.slack_client,
         )

nephthys/macros/identity.py

@@ -1,6 +1,7 @@
 from nephthys.actions.resolve import resolve
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import reply_to_ticket
 
 
 class Identity(Macro):
@@ -19,10 +20,10 @@ class Identity(Macro):
             or user_info["user"]["profile"].get("real_name")
             or user_info["user"]["name"]
         )
-        await env.slack_client.chat_postMessage(
+        await reply_to_ticket(
             text=f"hey, {name}! please could you ask questions about identity verification in <#{env.transcript.identity_help_channel}>? :rac_cute:\n\nit helps the verification team keep track of questions easier!",
-            channel=env.slack_help_channel,
+            ticket=ticket,
-            thread_ts=ticket.msgTs,
+            client=env.slack_client,
         )
         await resolve(
             ts=ticket.msgTs,

nephthys/macros/shipcertqueue.py

@@ -1,6 +1,7 @@
 from nephthys.actions.resolve import resolve
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import reply_to_ticket
 
 
 class ShipCertQueue(Macro):
@@ -19,10 +20,10 @@ class ShipCertQueue(Macro):
             or user_info["user"]["profile"].get("real_name")
             or user_info["user"]["name"]
         )
-        await env.slack_client.chat_postMessage(
+        await reply_to_ticket(
             text=f"Hi {name}! Unfortunately, there is a backlog of projects awaiting ship certification; please be patient. \n\n *pssst... voting more will move your project further towards the front of the queue.*",
-            channel=env.slack_help_channel,
+            ticket=ticket,
-            thread_ts=ticket.msgTs,
+            client=env.slack_client,
         )
         await resolve(
             ts=ticket.msgTs,

nephthys/macros/thread.py

@@ -1,6 +1,7 @@
 from nephthys.actions.resolve import resolve
 from nephthys.macros.types import Macro
 from nephthys.utils.env import env
+from nephthys.utils.ticket_methods import delete_bot_replies
 
 
 class Thread(Macro):
@@ -15,19 +16,8 @@ class Thread(Macro):
         if not sender:
             return
 
-        # Delete the first (FAQ) message sent by the bot
+        # Deletes the first (FAQ) message sent by the bot
-        bot_info = await env.slack_client.auth_test()
+        await delete_bot_replies(ticket.id)
-        bot_user_id = bot_info.get("user_id")
-        bot_messages = await env.slack_client.conversations_replies(
-            channel=env.slack_help_channel,
-            ts=ticket.msgTs,
-        )
-        for msg in bot_messages["messages"]:
-            if msg["user"] == bot_user_id:
-                await env.slack_client.chat_delete(
-                    channel=env.slack_help_channel,
-                    ts=msg["ts"],
-                )
 
         await resolve(
             ts=ticket.msgTs,

nephthys/utils/delete_thread.py

@@ -15,6 +15,7 @@ async def process_queue():
     """
     Continuously processes messages from the delete_queue.
     Retrieves a message (channel_id, message_ts) and attempts to delete it using Slack API.
+    Uses a user token to delete messages, thus requiring a user token with workspace admin.
     Handles rate limiting by retrying after the specified delay.
     Logs errors for other Slack API failures.
     """
@@ -27,6 +28,9 @@ async def process_queue():
                 as_user=True,
                 token=env.slack_user_token,
             )
+            await send_heartbeat(
+                f"Successfully deleted message {message_ts} in channel {channel_id}."
+            )
         except SlackApiError as e:
             if e.response and e.response["error"] == "ratelimited":
                 retry_after = int(e.response.headers.get("Retry-After", 1))

nephthys/utils/slack.py

@@ -1,3 +1,4 @@
+import logging
 from typing import Any
 from typing import Dict
 
@@ -25,10 +26,10 @@ app = AsyncApp(token=env.slack_bot_token, signing_secret=env.slack_signing_secre
 
 @app.event("message")
 async def handle_message(event: Dict[str, Any], client: AsyncWebClient):
-    print(event)
+    logging.debug(f"Message event: {event}")
     is_message_deletion = (
         event.get("subtype") == "message_changed"
-        and event["message"]["subtype"] == "tombstone"
+        and event["message"].get("subtype") == "tombstone"
     ) or event.get("subtype") == "message_deleted"
     if event["channel"] == env.slack_help_channel:
         if is_message_deletion:

nephthys/utils/ticket_methods.py

@@ -0,0 +1,64 @@
+import logging
+
+from slack_sdk.errors import SlackApiError
+from slack_sdk.web.async_client import AsyncWebClient
+
+from nephthys.utils.env import env
+from prisma.models import Ticket
+
+
+async def delete_message(channel_id: str, message_ts: str):
+    """Deletes a Slack message, or does nothing if the message doesn't exist"""
+    try:
+        await env.slack_client.chat_delete(channel=channel_id, ts=message_ts)
+    except SlackApiError as e:
+        if e.response.get("error") != "message_not_found":
+            raise e
+        logging.warning(
+            f"Tried to delete message {message_ts} in channel {channel_id} but it doesn't exist (already deleted?)"
+        )
+
+
+async def reply_to_ticket(ticket: Ticket, client: AsyncWebClient, text: str) -> None:
+    """Sends a user-facing message in the help thread and records it in the database"""
+    channel = env.slack_help_channel
+    thread_ts = ticket.msgTs
+    msg = await client.chat_postMessage(
+        channel=channel,
+        text=text,
+        thread_ts=thread_ts,
+    )
+    msg_ts = msg["ts"]
+    if not msg_ts:
+        logging.error(f"Bot message has no ts: {msg}")
+        return
+    await env.db.botmessage.create(
+        data={
+            "ts": msg_ts,
+            "channelId": channel,
+            "ticket": {"connect": {"id": ticket.id}},
+        }
+    )
+
+
+async def delete_bot_replies(ticket_ref: int):
+    """Deletes all bot replies sent in a ticket thread"""
+    ticket = await env.db.ticket.find_unique(
+        where={"id": ticket_ref}, include={"userFacingMsgs": True}
+    )
+    if not ticket:
+        raise ValueError(f"Ticket with ID {ticket_ref} does not exist")
+    if not ticket.userFacingMsgs:
+        raise ValueError(f"userFacingMsgs is not present on Ticket ID {ticket_ref}")
+    for bot_msg in ticket.userFacingMsgs:
+        await delete_message(bot_msg.channelId, bot_msg.ts)
+        await env.db.botmessage.delete(where={"id": bot_msg.id})
+
+
+async def delete_and_clean_up_ticket(ticket: Ticket):
+    """Removes a ticket from the DB and deletes all Slack messages associated with it"""
+    await delete_bot_replies(ticket.id)
+    # Delete the backend message in the "tickets" channel
+    await delete_message(env.slack_ticket_channel, ticket.ticketTs)
+    # TODO deal with DMs to tag subscribers?
+    await env.db.ticket.delete(where={"id": ticket.id})

prisma/schema.prisma

@@ -37,9 +37,14 @@ model Ticket {
   description String
   status      TicketStatus @default(OPEN)
 
+  // TS for the original help message (top of the help thread)
   msgTs    String @unique
+  // TS for the ticket message in the "backend" tickets channel
   ticketTs String @unique
 
+  // Messages sent by the bot in the help thread
+  userFacingMsgs BotMessage[]
+
   openedBy   User   @relation("OpenedTickets", fields: [openedById], references: [id])
   openedById Int
 
@@ -89,3 +94,13 @@ model UserTagSubscription {
   @@id([userId, tagId])
   @@map("user_tag_subscriptions")
 }
+
+model BotMessage {
+  id        Int      @id @unique @default(autoincrement())
+  ts        String
+  channelId String
+  ticket    Ticket  @relation(fields: [ticketId], references: [id], onDelete: Cascade)
+  ticketId  Int
+
+  @@unique([ts, channelId])
+}