From bed6f313ca877ad227f8763434cbd28d251d74ff Mon Sep 17 00:00:00 2001 From: MMK21 <50421330+MMK21Hub@users.noreply.github.com> Date: Sat, 25 Oct 2025 08:52:21 +0100 Subject: [PATCH] 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 | 8 +-- nephthys/events/message.py | 86 +++++++++++++++++++---------- nephthys/events/message_deletion.py | 57 +++++++++++-------- nephthys/macros/faq.py | 7 ++- nephthys/macros/fraud.py | 7 ++- nephthys/macros/hello_world.py | 7 ++- nephthys/macros/identity.py | 7 ++- nephthys/macros/shipcertqueue.py | 7 ++- nephthys/macros/thread.py | 16 +----- nephthys/utils/delete_thread.py | 4 ++ nephthys/utils/slack.py | 5 +- nephthys/utils/ticket_methods.py | 64 +++++++++++++++++++++ prisma/schema.prisma | 15 +++++ 13 files changed, 203 insertions(+), 87 deletions(-) create mode 100644 nephthys/utils/ticket_methods.py diff --git a/nephthys/actions/resolve.py b/nephthys/actions/resolve.py index 11b7782..4f963fa 100644 --- a/nephthys/actions/resolve.py +++ b/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( - channel=env.slack_help_channel, + await reply_to_ticket( + 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, diff --git a/nephthys/events/message.py b/nephthys/events/message.py index eaa5414..746201f 100644 --- a/nephthys/events/message.py +++ b/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 - else ticket.assignedAt + if not ticket_message.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", "") - display_name = ( - user_info["user"]["profile"]["display_name"] - or user_info["user"]["real_name"] - ) + profile_pic = user_info["profile"].get("image_512", "") + display_name = user_info["profile"]["display_name"] or user_info["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( - { - "title": title, - "description": text, - "msgTs": event["ts"], - "ticketTs": ticket["ts"], - "openedBy": {"connect": {"id": db_user.id}}, - }, - ) - - text = ( + user_facing_message_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( - channel=event["channel"], name="thinking_face", timestamp=event["ts"] + user_facing_message_ts = user_facing_message["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: diff --git a/nephthys/events/message_deletion.py b/nephthys/events/message_deletion.py index 40775a3..b52ba59 100644 --- a/nephthys/events/message_deletion.py +++ b/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) - elif msg["ts"] != deleted_msg["ts"]: - # Don't clear the thread if there are non-bot messages in there - return + bot_replies.append(msg) + else: + non_bot_replies.append(msg) - # Delete ticket from DB - await env.db.ticket.delete(where={"msgTs": deleted_msg["ts"]}) - - # Delete messages - await send_heartbeat( - f"Removing my {len(messages_to_delete)} message(s) in a thread because the question was deleted." + should_keep_thread = ( + # Preserve if there are any human replies + len(non_bot_replies) > 0 + # More than 2 bot replies implies someone ran ?faq or something, so we'll preserve the ticket + or len(bot_replies) > 2 ) - for msg in messages_to_delete: - await client.chat_delete( - channel=channel, - ts=msg["ts"], - ) + if should_keep_thread: + return + + # 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 = ( - "thread_ts" not in deleted_msg or deleted_msg["ts"] == deleted_msg["thread_ts"] + is_in_thread = ( + "thread_ts" in deleted_msg and deleted_msg["ts"] != deleted_msg["thread_ts"] ) - if is_top_level_message: - # A question (i.e. top-level message in help channel) has been deleted + if is_in_thread: + 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) diff --git a/nephthys/macros/faq.py b/nephthys/macros/faq.py index 8a17a8d..835cb1c 100644 --- a/nephthys/macros/faq.py +++ b/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, - thread_ts=ticket.msgTs, + ticket=ticket, + client=env.slack_client, ) await resolve( ts=ticket.msgTs, diff --git a/nephthys/macros/fraud.py b/nephthys/macros/fraud.py index dc7607c..17c109c 100644 --- a/nephthys/macros/fraud.py +++ b/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, - thread_ts=ticket.msgTs, + ticket=ticket, + client=env.slack_client, ) await resolve( ts=ticket.msgTs, diff --git a/nephthys/macros/hello_world.py b/nephthys/macros/hello_world.py index ab82880..975d21d 100644 --- a/nephthys/macros/hello_world.py +++ b/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, - thread_ts=ticket.msgTs, + ticket=ticket, + client=env.slack_client, ) diff --git a/nephthys/macros/identity.py b/nephthys/macros/identity.py index 7cdf475..150746c 100644 --- a/nephthys/macros/identity.py +++ b/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, - thread_ts=ticket.msgTs, + ticket=ticket, + client=env.slack_client, ) await resolve( ts=ticket.msgTs, diff --git a/nephthys/macros/shipcertqueue.py b/nephthys/macros/shipcertqueue.py index 780d18e..00d4627 100644 --- a/nephthys/macros/shipcertqueue.py +++ b/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, - thread_ts=ticket.msgTs, + ticket=ticket, + client=env.slack_client, ) await resolve( ts=ticket.msgTs, diff --git a/nephthys/macros/thread.py b/nephthys/macros/thread.py index 4e559cc..986e6aa 100644 --- a/nephthys/macros/thread.py +++ b/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 - bot_info = await env.slack_client.auth_test() - 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"], - ) + # Deletes the first (FAQ) message sent by the bot + await delete_bot_replies(ticket.id) await resolve( ts=ticket.msgTs, diff --git a/nephthys/utils/delete_thread.py b/nephthys/utils/delete_thread.py index 14f641d..194d0a4 100644 --- a/nephthys/utils/delete_thread.py +++ b/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)) diff --git a/nephthys/utils/slack.py b/nephthys/utils/slack.py index 8bcc861..b6ecce5 100644 --- a/nephthys/utils/slack.py +++ b/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: diff --git a/nephthys/utils/ticket_methods.py b/nephthys/utils/ticket_methods.py new file mode 100644 index 0000000..3bc7d8e --- /dev/null +++ b/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}) diff --git a/prisma/schema.prisma b/prisma/schema.prisma index 3608ccd..ecca778 100644 --- a/prisma/schema.prisma +++ b/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]) +} \ No newline at end of file