From 7453bc14cef2c5a6e9a704720ce4cec9f3772700 Mon Sep 17 00:00:00 2001 From: SillyNY Date: Thu, 4 Jun 2026 13:52:22 +0800 Subject: [PATCH 1/6] Replace manual username input with Select dropdown for student removal The Remove Student button now shows a dropdown list of all students currently in the queue, eliminates exact username matching. --- ui/modals.py | 38 ++++++++++++++++++---------- ui/views/ta_view.py | 60 +++++++++++++++++++++++++++++++++++++++++++-- 2 files changed, 83 insertions(+), 15 deletions(-) diff --git a/ui/modals.py b/ui/modals.py index 4839409..1b9f8d9 100644 --- a/ui/modals.py +++ b/ui/modals.py @@ -160,22 +160,34 @@ async def on_submit(self, interaction: discord.Interaction): class RemoveConfirmModal(discord.ui.Modal, title="Removal Confirmation"): - warning = discord.ui.TextDisplay("Are you sure? This will remove the student from the queue.") - input: discord.ui.TextInput = discord.ui.TextInput(label="Please input student username.", placeholder="bsharplydian", min_length=1) - reason: discord.ui.TextInput = discord.ui.TextInput(label="Provide a reason (optional)", placeholder="You've used the queue too many times today", required=False) + reason: discord.ui.TextInput = discord.ui.TextInput( + label="Provide a reason (optional)", + placeholder="You've used the queue too many times today", + required=False, + max_length=200 + ) + + def __init__(self, student_user_id: int, student_name: str): + super().__init__() + self.student_user_id = student_user_id + self.student_name = student_name + self.add_item(discord.ui.TextDisplay( + f"This will remove {student_name} from the queue." + )) async def on_submit(self, interaction: discord.Interaction): - for entry in interaction.client.queue.entries: - if entry.username == self.input.value: - user: discord.User = await interaction.client.fetch_user(entry.user_id) - await interaction.client.queue.remove(user.id) - await update_queue_messages(interaction.client) - await user.send(f"You have been removed from the CS240 help queue. {"Reason: " if self.reason.value != "" else ""}{self.reason.value}") - await interaction.response.send_message(f"{user.display_name} has been removed from the queue by {interaction.user.display_name}.", delete_after=60*5) - return - - await interaction.response.send_message(f"No student with the username \"{self.input.value}\" in the queue.", ephemeral=True, delete_after=10) + user: discord.User = await interaction.client.fetch_user(self.student_user_id) + await interaction.client.queue.remove(self.student_user_id) + await update_queue_messages(interaction.client) + reason_suffix = f" Reason: {self.reason.value}" if self.reason.value else "" + await user.send( + f"You have been removed from the CS240 help queue.{reason_suffix}" + ) + await interaction.response.send_message( + f"{user.display_name} has been removed from the queue by {interaction.user.display_name}.", + delete_after=60 * 5 + ) class EditQueueHoursModal(discord.ui.Modal, title="Edit Queue Hours"): open_hour = discord.ui.TextInput( diff --git a/ui/views/ta_view.py b/ui/views/ta_view.py index 331491a..59b3631 100644 --- a/ui/views/ta_view.py +++ b/ui/views/ta_view.py @@ -8,7 +8,55 @@ from ui.helpers.utils import fixed_width from ui.helpers.discord_helpers import get_channel, get_role, move_to_breakout, safe_dm_user, notify_next_if_changed, update_queue_messages +class RemoveStudentView(discord.ui.View): + def __init__(self, entries): + super().__init__(timeout=30) + + options = [] + for i, entry in enumerate(entries, start=1): + label = entry.student_name if entry.student_name else entry.username + if len(label) > 100: + label = label[:97] + "..." + desc = entry.details if entry.details else "" + if len(desc) > 100: + desc = desc[:97] + "..." + + emoji = "✅" if entry.is_passoff else "❓" + + options.append( + discord.SelectOption( + label=label, + value=str(entry.user_id), + description=f"#{i} {desc}" if desc else f"#{i} in queue", + emoji=emoji, + ) + ) + + select = discord.ui.Select( + placeholder="Select a student to remove...", + options=options, + min_values=1, + max_values=1, + ) + select.callback = self.select_callback + self.add_item(select) + + async def select_callback(self, interaction: discord.Interaction): + user_id = int(self.children[0].values[0]) + + entry = next( + (e for e in interaction.client.queue.entries if e.user_id == user_id), + None + ) + if not entry: + await interaction.response.send_message( + "That student is no longer in the queue.", + ephemeral=True, delete_after=10 + ) + return + student_name = entry.student_name if entry.student_name else entry.username + await interaction.response.send_modal(RemoveConfirmModal(user_id, student_name)) class TAView(discord.ui.View): @@ -161,8 +209,16 @@ async def clear_queue(self, interaction: discord.Interaction, button): @discord.ui.button(label="Remove Student", style=discord.ButtonStyle.danger, custom_id="remove_from_queue", emoji="🗑️") async def remove_from_queue(self, interaction: discord.Interaction, button): - await interaction.response.send_modal(RemoveConfirmModal()) - + entries = interaction.client.queue.entries + if not entries: + await interaction.response.send_message( + "Queue is empty.", ephemeral=True, delete_after=10 + ) + return + view = RemoveStudentView(entries) + await interaction.response.send_message( + "Select a student to remove:", view=view, ephemeral=True, delete_after=60 + ) @discord.ui.button(label="Finish", style=discord.ButtonStyle.green, custom_id="finish", emoji="🔚") async def finish_button(self, interaction: discord.Interaction, button): online_ta_vc: discord.VoiceChannel = get_channel(interaction, TA_VOICE_CHANNEL_NAME) From 7a2f23dbf034f5e72e8db9ac0eba3be52ce3f81a Mon Sep 17 00:00:00 2001 From: SillyNY Date: Fri, 5 Jun 2026 14:51:59 +0800 Subject: [PATCH 2/6] =?UTF-8?q?=E5=AE=8C=E5=96=84=E7=A7=BB=E9=99=A4?= =?UTF-8?q?=E5=AD=A6=E7=94=9F=E4=B8=8B=E6=8B=89=E8=8F=9C=E5=8D=95=EF=BC=9A?= =?UTF-8?q?Cancel=E9=80=89=E9=A1=B9=E3=80=81=E9=98=9F=E9=A6=96=E9=80=9A?= =?UTF-8?q?=E7=9F=A5=E3=80=81emoji=E5=9B=BE=E4=BE=8B?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- test_remove_student.py | 433 +++++++++++++++++++++++++++++++++++++++++ ui/modals.py | 4 +- ui/views/ta_view.py | 18 +- 3 files changed, 451 insertions(+), 4 deletions(-) create mode 100644 test_remove_student.py diff --git a/test_remove_student.py b/test_remove_student.py new file mode 100644 index 0000000..1702167 --- /dev/null +++ b/test_remove_student.py @@ -0,0 +1,433 @@ +""" +============================================================ +如何避免漏掉边界情况（本次踩坑复盘） +============================================================ + +这次最初写的测试没覆盖到"取消 Modal 后无法重选""移除队首应通知新队首" +这类边界问题，根本原因是按"happy path"写测试，而非按"状态机"写测试。 + +一套可复用的自查清单： + +1. 【状态迁移图】 + 对每个交互组件，画出它经历的所有状态。 + 本例中 Select 的状态：初始未选中 → 选中学生 → 弹出 Modal → + → 确认 → 学生被移除 ✓ + → 取消 → 回到未选中状态（本次遗漏！Discord 默认做不到） + 每个箭头 = 一个 test case。 + +2. 【0 / 1 / N 法则】 + 每个涉及集合的操作，至少测 3 种数据量： + - 0：空队列点 Remove Student → 应提示 "Queue is empty." + - 1：队列只有一人时取消重选（本次遗漏！因为 1 人时 Cancel 是唯一出路） + - N：多人时选中间的人、选队首、选队尾 + +3. 【"反悔"路径】 + 任何需要用户确认的操作，必须测"点了确认再取消"的路径： + - 点了删除 → Modal 弹出 → 按 Esc 关闭 → 还能重新选吗？ + - 本次的 Cancel 选项就是为了给这个路径兜底 + +4. 【副作用传播】 + 操作 A 影响了谁？列出所有"利益相关方"： + - 被移除的学生本人 → 应收到 DM + - 新队首 → 应收到 "you are next"（本次遗漏！） + - 其他 TA → 他们打开的下拉菜单里该项应失效（已测） + +5. 【并发 / 竞赛】 + 两个 TA 同时操作同一学生： + - TA1 选中 Alice，弹出 Modal 但不提交 + - TA2 也选中 Alice，确认移除 + - TA1 再点确认 → 应提示 "已不在队列中"（已测） +""" + +import unittest +from unittest.mock import AsyncMock, MagicMock, patch, PropertyMock +from datetime import datetime + +import discord + +from records import QueueEntry +from help_queue import HelpQueue + + +# ========================================================== +# Fixtures +# ========================================================== + +def make_entry(user_id: int, username: str = "", student_name: str = "", + details: str = "", is_passoff: bool = False, in_person: bool = False): + return QueueEntry( + user_id=user_id, + username=username or f"user_{user_id}", + student_name=student_name, + details=details, + is_passoff=is_passoff, + timestamp=datetime.now(), + in_person=in_person, + ) + + +def make_mock_interaction(queue: HelpQueue, user_id: int = 999, + display_name: str = "TA_Test"): + """构造一个带 client.queue 的 mock Interaction""" + mock = MagicMock(spec=discord.Interaction) + mock.client = MagicMock() + mock.client.queue = queue + mock.user = MagicMock() + mock.user.id = user_id + mock.user.display_name = display_name + mock.response = MagicMock() + mock.response.send_message = AsyncMock() + mock.response.send_modal = AsyncMock() + mock.response.defer = AsyncMock() + mock.followup = MagicMock() + mock.followup.send = AsyncMock() + return mock + + + +# ========================================================== +# HelpQueue +# ========================================================== + +class TestHelpQueue(unittest.IsolatedAsyncioTestCase): + + async def test_remove_existing_user(self): + q = HelpQueue() + await q.add(make_entry(1)) + await q.add(make_entry(2)) + await q.remove(1) + self.assertEqual(len(q.entries), 1) + self.assertEqual(q.entries[0].user_id, 2) + + async def test_remove_nonexistent_user_does_nothing(self): + q = HelpQueue() + await q.add(make_entry(1)) + await q.remove(999) + self.assertEqual(len(q.entries), 1) + + async def test_remove_from_empty_queue(self): + q = HelpQueue() + await q.remove(1) + self.assertEqual(len(q.entries), 0) + + async def test_get_front_after_remove(self): + q = HelpQueue() + await q.add(make_entry(1)) + await q.add(make_entry(2)) + self.assertEqual((await q.get_front()).user_id, 1) + await q.remove(1) + self.assertEqual((await q.get_front()).user_id, 2) + + async def test_get_front_empty_queue(self): + q = HelpQueue() + self.assertIsNone(await q.get_front()) + + +# ========================================================== +# RemoveStudentView — 构造 +# ========================================================== + +class TestRemoveStudentViewConstruction(unittest.IsolatedAsyncioTestCase): + + def test_options_include_cancel_first(self): + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(1, username="alice")]) + select = view.children[0] + self.assertEqual(select.options[0].value, "__cancel__") + self.assertEqual(select.options[0].label, "— Cancel —") + + def test_emoji_passoff_vs_question(self): + from ui.views.ta_view import RemoveStudentView + entries = [ + make_entry(1, username="a", is_passoff=True), + make_entry(2, username="b", is_passoff=False), + ] + view = RemoveStudentView(entries) + select = view.children[0] + self.assertEqual(select.options[1].emoji.name, "✅") + self.assertEqual(select.options[2].emoji.name, "❓") + + def test_label_prefers_student_name(self): + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(1, username="discord_abc", student_name="张三")]) + self.assertEqual(view.children[0].options[1].label, "张三") + + def test_label_falls_back_to_username(self): + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(1, username="discord_abc", student_name="")]) + self.assertEqual(view.children[0].options[1].label, "discord_abc") + + def test_label_truncated_at_100_chars(self): + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(1, username="x", student_name="A" * 150)]) + opt = view.children[0].options[1] + self.assertLessEqual(len(opt.label), 100) + self.assertTrue(opt.label.endswith("...")) + + def test_description_truncated(self): + """description ≤ 100 字符（含 "#N " 前缀）""" + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(1, username="x", details="D" * 200)]) + opt = view.children[0].options[1] + self.assertLessEqual(len(opt.description), 100) + + def test_value_is_user_id_string(self): + from ui.views.ta_view import RemoveStudentView + view = RemoveStudentView([make_entry(123456, username="alice")]) + self.assertEqual(view.children[0].options[1].value, "123456") + + +# ========================================================== +# RemoveStudentView — select_callback +# ========================================================== + +class TestRemoveStudentViewCallback(unittest.IsolatedAsyncioTestCase): + + async def test_cancel_option_defers_and_returns(self): + """选 Cancel → defer，不弹 Modal""" + from ui.views.ta_view import RemoveStudentView + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + + interaction = make_mock_interaction(q) + view = RemoveStudentView(q.entries) + + with patch.object(discord.ui.Select, 'values', + new_callable=PropertyMock) as mock_values: + mock_values.return_value = ["__cancel__"] + await view.select_callback(interaction) + + interaction.response.defer.assert_awaited_once() + interaction.response.send_modal.assert_not_awaited() + + async def test_select_student_sends_modal(self): + """选学生 → 弹出 RemoveConfirmModal，携带正确参数""" + from ui.views.ta_view import RemoveStudentView + q = HelpQueue() + await q.add(make_entry(1, username="alice", student_name="Alice")) + + interaction = make_mock_interaction(q) + view = RemoveStudentView(q.entries) + + with patch.object(discord.ui.Select, 'values', + new_callable=PropertyMock) as mock_values: + mock_values.return_value = ["1"] + await view.select_callback(interaction) + + interaction.response.send_modal.assert_awaited_once() + modal = interaction.response.send_modal.call_args[0][0] + self.assertEqual(modal.student_user_id, 1) + self.assertEqual(modal.student_name, "Alice") + + async def test_student_already_removed_by_another_ta(self): + """并发：选中的学生已被其他 TA 移除""" + from ui.views.ta_view import RemoveStudentView + q = HelpQueue() + # entry 不在队列中（模拟已移除） + entry = make_entry(2, username="bob") + + interaction = make_mock_interaction(q) + view = RemoveStudentView([entry]) + + with patch.object(discord.ui.Select, 'values', + new_callable=PropertyMock) as mock_values: + mock_values.return_value = ["2"] + await view.select_callback(interaction) + + interaction.response.send_message.assert_awaited_once() + msg = interaction.response.send_message.call_args[0][0] + self.assertIn("no longer in the queue", msg) + + +# ========================================================== +# RemoveConfirmModal +# ========================================================== + +class TestRemoveConfirmModal(unittest.IsolatedAsyncioTestCase): + + def test_init_stores_user_id_and_display_name(self): + from ui.modals import RemoveConfirmModal + modal = RemoveConfirmModal(123, "Alice") + self.assertEqual(modal.student_user_id, 123) + self.assertEqual(modal.student_name, "Alice") + + async def test_on_submit_removes_student(self): + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + await q.add(make_entry(2, username="bob")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 1 + mock_user.display_name = "alice" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()): + modal = RemoveConfirmModal(1, "Alice") + await modal.on_submit(interaction) + + self.assertEqual(len(q.entries), 1) + self.assertEqual(q.entries[0].user_id, 2) + + async def test_on_submit_notifies_new_front(self): + """移除队首 → notify_next_if_changed 被调用，传入旧队首""" + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + await q.add(make_entry(2, username="bob")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 1 + mock_user.display_name = "alice" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()) as mock_notify: + modal = RemoveConfirmModal(1, "Alice") + await modal.on_submit(interaction) + + mock_notify.assert_awaited_once() + # 第二个参数 front_before 是旧队首（alice, user_id=1） + self.assertEqual(mock_notify.call_args[0][1].user_id, 1) + + async def test_on_submit_no_notify_when_removed_not_front(self): + """移除非队首 → notify_next_if_changed 仍被调用，但传入的旧队首没变""" + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + await q.add(make_entry(2, username="bob")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 2 + mock_user.display_name = "bob" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()) as mock_notify: + modal = RemoveConfirmModal(2, "Bob") + await modal.on_submit(interaction) + + mock_notify.assert_awaited_once() + # 旧队首是 alice (user_id=1)，与移除的 bob (user_id=2) 不同， + # notify_next_if_changed 内部判断前后队首都是 alice，因此不发 DM + self.assertEqual(mock_notify.call_args[0][1].user_id, 1) + + async def test_on_submit_dm_to_student(self): + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 1 + mock_user.display_name = "alice" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()): + modal = RemoveConfirmModal(1, "Alice") + await modal.on_submit(interaction) + + mock_user.send.assert_awaited_once() + self.assertIn("removed from the CS240 help queue", mock_user.send.call_args[0][0]) + + async def test_on_submit_dm_includes_reason(self): + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 1 + mock_user.display_name = "alice" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()): + modal = RemoveConfirmModal(1, "Alice") + modal.reason = MagicMock() + modal.reason.value = "Asked too many questions" + await modal.on_submit(interaction) + + dm_text = mock_user.send.call_args[0][0] + self.assertIn("Reason:", dm_text) + self.assertIn("Asked too many questions", dm_text) + + async def test_on_submit_success_message(self): + from ui.modals import RemoveConfirmModal + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + + interaction = make_mock_interaction(q) + mock_user = MagicMock() + mock_user.id = 1 + mock_user.display_name = "alice" + mock_user.send = AsyncMock() + interaction.client.fetch_user = AsyncMock(return_value=mock_user) + interaction.response.send_message = AsyncMock() + + with patch("ui.modals.update_queue_messages", AsyncMock()), \ + patch("ui.modals.notify_next_if_changed", AsyncMock()): + modal = RemoveConfirmModal(1, "Alice") + await modal.on_submit(interaction) + + interaction.response.send_message.assert_awaited_once() + msg = interaction.response.send_message.call_args[0][0] + self.assertIn("has been removed from the queue by", msg) + + +# ========================================================== +# TAView.remove_from_queue 按钮 +# ========================================================== + +class TestTAViewRemoveButton(unittest.IsolatedAsyncioTestCase): + + async def test_empty_queue_shows_message(self): + from ui.views.ta_view import TAView + q = HelpQueue() + interaction = make_mock_interaction(q) + view = TAView() + + for item in view.children: + if item.custom_id == "remove_from_queue": + await item.callback(interaction) + break + + interaction.response.send_message.assert_awaited_once() + msg = interaction.response.send_message.call_args[0][0] + self.assertEqual(msg, "Queue is empty.") + + async def test_nonempty_queue_shows_select_with_legend(self): + from ui.views.ta_view import TAView, RemoveStudentView + q = HelpQueue() + await q.add(make_entry(1, username="alice")) + + interaction = make_mock_interaction(q) + view = TAView() + + for item in view.children: + if item.custom_id == "remove_from_queue": + await item.callback(interaction) + break + + interaction.response.send_message.assert_awaited_once() + sent_view = interaction.response.send_message.call_args[1]["view"] + self.assertIsInstance(sent_view, RemoveStudentView) + text = interaction.response.send_message.call_args[0][0] + self.assertIn("✅", text) + self.assertIn("❓", text) + self.assertIn("Passoff", text) + self.assertIn("Question", text) + + +if __name__ == "__main__": + unittest.main() diff --git a/ui/modals.py b/ui/modals.py index 1b9f8d9..0b77ece 100644 --- a/ui/modals.py +++ b/ui/modals.py @@ -3,7 +3,7 @@ from db import get_times_helped_today, record_bot_issue from ui.helpers.discord_helpers import get_channel, get_role, update_queue_messages from ui.helpers.constants import SHORT_TIMEOUT, TA_TEXT_CHANNEL_NAME - +from ui.helpers.discord_helpers import get_channel, get_role, notify_next_if_changed, update_queue_messages class HelpModal(discord.ui.Modal, title="Request Help"): name = discord.ui.TextInput( @@ -176,10 +176,12 @@ def __init__(self, student_user_id: int, student_name: str): )) async def on_submit(self, interaction: discord.Interaction): + front_before = await interaction.client.queue.get_front() user: discord.User = await interaction.client.fetch_user(self.student_user_id) await interaction.client.queue.remove(self.student_user_id) await update_queue_messages(interaction.client) + await notify_next_if_changed(interaction.client, front_before) reason_suffix = f" Reason: {self.reason.value}" if self.reason.value else "" await user.send( f"You have been removed from the CS240 help queue.{reason_suffix}" diff --git a/ui/views/ta_view.py b/ui/views/ta_view.py index 59b3631..23ee60a 100644 --- a/ui/views/ta_view.py +++ b/ui/views/ta_view.py @@ -12,14 +12,21 @@ class RemoveStudentView(discord.ui.View): def __init__(self, entries): super().__init__(timeout=30) - options = [] + options = [ + discord.SelectOption( + label="— Cancel —", + value="__cancel__", + description="Reset selection without removing anyone", + emoji="↩️", + ) + ] for i, entry in enumerate(entries, start=1): label = entry.student_name if entry.student_name else entry.username if len(label) > 100: label = label[:97] + "..." desc = entry.details if entry.details else "" if len(desc) > 100: - desc = desc[:97] + "..." + desc = desc[:93] + "..." # 96 chars + "#N " prefix ≤ 100 emoji = "✅" if entry.is_passoff else "❓" @@ -42,6 +49,10 @@ def __init__(self, entries): self.add_item(select) async def select_callback(self, interaction: discord.Interaction): + selected = self.children[0].values[0] + if selected == "__cancel__": + await interaction.response.defer() + return user_id = int(self.children[0].values[0]) entry = next( @@ -217,7 +228,8 @@ async def remove_from_queue(self, interaction: discord.Interaction, button): return view = RemoveStudentView(entries) await interaction.response.send_message( - "Select a student to remove:", view=view, ephemeral=True, delete_after=60 + "Select a student to remove:\n✅ = Passoff | ❓ = Question", + view=view, ephemeral=True, delete_after=60 ) @discord.ui.button(label="Finish", style=discord.ButtonStyle.green, custom_id="finish", emoji="🔚") async def finish_button(self, interaction: discord.Interaction, button): From 8f7c533a507815a44fe4b753dce19c877f161acf Mon Sep 17 00:00:00 2001 From: SillyNY Date: Sat, 6 Jun 2026 21:44:47 +0800 Subject: [PATCH 3/6] Move test_remove_student.py into tests/ package --- test_remove_student.py => tests/test_remove_student.py | 0 1 file changed, 0 insertions(+), 0 deletions(-) rename test_remove_student.py => tests/test_remove_student.py (100%) diff --git a/test_remove_student.py b/tests/test_remove_student.py similarity index 100% rename from test_remove_student.py rename to tests/test_remove_student.py From e19ee046dbb47eecfdbf7e1b7e5743ae2fce2ae7 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Mon, 8 Jun 2026 14:54:59 -0600 Subject: [PATCH 4/6] Translation and removal of non-english comments --- tests/test_remove_student.py | 77 +++++++++--------------------------- 1 file changed, 18 insertions(+), 59 deletions(-) diff --git a/tests/test_remove_student.py b/tests/test_remove_student.py index 1702167..73c8dd8 100644 --- a/tests/test_remove_student.py +++ b/tests/test_remove_student.py @@ -1,44 +1,3 @@ -""" -============================================================ -如何避免漏掉边界情况（本次踩坑复盘） -============================================================ - -这次最初写的测试没覆盖到"取消 Modal 后无法重选""移除队首应通知新队首" -这类边界问题，根本原因是按"happy path"写测试，而非按"状态机"写测试。 - -一套可复用的自查清单： - -1. 【状态迁移图】 - 对每个交互组件，画出它经历的所有状态。 - 本例中 Select 的状态：初始未选中 → 选中学生 → 弹出 Modal → - → 确认 → 学生被移除 ✓ - → 取消 → 回到未选中状态（本次遗漏！Discord 默认做不到） - 每个箭头 = 一个 test case。 - -2. 【0 / 1 / N 法则】 - 每个涉及集合的操作，至少测 3 种数据量： - - 0：空队列点 Remove Student → 应提示 "Queue is empty." - - 1：队列只有一人时取消重选（本次遗漏！因为 1 人时 Cancel 是唯一出路） - - N：多人时选中间的人、选队首、选队尾 - -3. 【"反悔"路径】 - 任何需要用户确认的操作，必须测"点了确认再取消"的路径： - - 点了删除 → Modal 弹出 → 按 Esc 关闭 → 还能重新选吗？ - - 本次的 Cancel 选项就是为了给这个路径兜底 - -4. 【副作用传播】 - 操作 A 影响了谁？列出所有"利益相关方"： - - 被移除的学生本人 → 应收到 DM - - 新队首 → 应收到 "you are next"（本次遗漏！） - - 其他 TA → 他们打开的下拉菜单里该项应失效（已测） - -5. 【并发 / 竞赛】 - 两个 TA 同时操作同一学生： - - TA1 选中 Alice，弹出 Modal 但不提交 - - TA2 也选中 Alice，确认移除 - - TA1 再点确认 → 应提示 "已不在队列中"（已测） -""" - import unittest from unittest.mock import AsyncMock, MagicMock, patch, PropertyMock from datetime import datetime @@ -48,9 +7,9 @@ from records import QueueEntry from help_queue import HelpQueue - +"""This file conducts tests on the funcitonality of the Remove button using Mocks to simulate interactions with Discord's APi.""" # ========================================================== -# Fixtures +# Helper functions # ========================================================== def make_entry(user_id: int, username: str = "", student_name: str = "", @@ -68,7 +27,7 @@ def make_entry(user_id: int, username: str = "", student_name: str = "", def make_mock_interaction(queue: HelpQueue, user_id: int = 999, display_name: str = "TA_Test"): - """构造一个带 client.queue 的 mock Interaction""" + """Construct a mock Interaction with a client.queue.""" mock = MagicMock(spec=discord.Interaction) mock.client = MagicMock() mock.client.queue = queue @@ -86,7 +45,7 @@ def make_mock_interaction(queue: HelpQueue, user_id: int = 999, # ========================================================== -# HelpQueue +# HelpQueue Modification tests (Most important) # ========================================================== class TestHelpQueue(unittest.IsolatedAsyncioTestCase): @@ -124,7 +83,7 @@ async def test_get_front_empty_queue(self): # ========================================================== -# RemoveStudentView — 构造 +# RemoveStudentView construction # ========================================================== class TestRemoveStudentViewConstruction(unittest.IsolatedAsyncioTestCase): @@ -165,7 +124,7 @@ def test_label_truncated_at_100_chars(self): self.assertTrue(opt.label.endswith("...")) def test_description_truncated(self): - """description ≤ 100 字符（含 "#N " 前缀）""" + """description is at most 100 characters long (including the "#N " prefix).""" from ui.views.ta_view import RemoveStudentView view = RemoveStudentView([make_entry(1, username="x", details="D" * 200)]) opt = view.children[0].options[1] @@ -178,13 +137,13 @@ def test_value_is_user_id_string(self): # ========================================================== -# RemoveStudentView — select_callback +# RemoveStudentView Callback # ========================================================== class TestRemoveStudentViewCallback(unittest.IsolatedAsyncioTestCase): async def test_cancel_option_defers_and_returns(self): - """选 Cancel → defer，不弹 Modal""" + """Select Cancel → defer and do not show the Modal.""" from ui.views.ta_view import RemoveStudentView q = HelpQueue() await q.add(make_entry(1, username="alice")) @@ -201,7 +160,7 @@ async def test_cancel_option_defers_and_returns(self): interaction.response.send_modal.assert_not_awaited() async def test_select_student_sends_modal(self): - """选学生 → 弹出 RemoveConfirmModal，携带正确参数""" + """Select a student → show RemoveConfirmModal with correct parameters.""" from ui.views.ta_view import RemoveStudentView q = HelpQueue() await q.add(make_entry(1, username="alice", student_name="Alice")) @@ -220,10 +179,10 @@ async def test_select_student_sends_modal(self): self.assertEqual(modal.student_name, "Alice") async def test_student_already_removed_by_another_ta(self): - """并发：选中的学生已被其他 TA 移除""" + """Concurrency: the selected student has already been removed by another TA.""" from ui.views.ta_view import RemoveStudentView q = HelpQueue() - # entry 不在队列中（模拟已移除） + # entry is not in the queue (simulate already removed) entry = make_entry(2, username="bob") interaction = make_mock_interaction(q) @@ -240,7 +199,7 @@ async def test_student_already_removed_by_another_ta(self): # ========================================================== -# RemoveConfirmModal +# RemoveConfirmModal functionality # ========================================================== class TestRemoveConfirmModal(unittest.IsolatedAsyncioTestCase): @@ -273,7 +232,7 @@ async def test_on_submit_removes_student(self): self.assertEqual(q.entries[0].user_id, 2) async def test_on_submit_notifies_new_front(self): - """移除队首 → notify_next_if_changed 被调用，传入旧队首""" + """Removing the front of the queue calls notify_next_if_changed to notify the new front.""" from ui.modals import RemoveConfirmModal q = HelpQueue() await q.add(make_entry(1, username="alice")) @@ -292,11 +251,11 @@ async def test_on_submit_notifies_new_front(self): await modal.on_submit(interaction) mock_notify.assert_awaited_once() - # 第二个参数 front_before 是旧队首（alice, user_id=1） + # The second argument front_before is the previous front of the queue (alice, user_id=1). self.assertEqual(mock_notify.call_args[0][1].user_id, 1) async def test_on_submit_no_notify_when_removed_not_front(self): - """移除非队首 → notify_next_if_changed 仍被调用，但传入的旧队首没变""" + """Removing a non-front student still calls notify_next_if_changed, but the old front remains unchanged.""" from ui.modals import RemoveConfirmModal q = HelpQueue() await q.add(make_entry(1, username="alice")) @@ -315,8 +274,8 @@ async def test_on_submit_no_notify_when_removed_not_front(self): await modal.on_submit(interaction) mock_notify.assert_awaited_once() - # 旧队首是 alice (user_id=1)，与移除的 bob (user_id=2) 不同， - # notify_next_if_changed 内部判断前后队首都是 alice，因此不发 DM + # The old front is alice (user_id=1), which differs from the removed bob (user_id=2). + # notify_next_if_changed will compare the front before and after and should not send a DM if it is still alice. self.assertEqual(mock_notify.call_args[0][1].user_id, 1) async def test_on_submit_dm_to_student(self): @@ -386,7 +345,7 @@ async def test_on_submit_success_message(self): # ========================================================== -# TAView.remove_from_queue 按钮 +# TAView.remove_from_queue button # ========================================================== class TestTAViewRemoveButton(unittest.IsolatedAsyncioTestCase): From 3037ab5fee43c409c24a2be722a09d996a2c6de5 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Mon, 8 Jun 2026 15:25:27 -0600 Subject: [PATCH 5/6] Fix lost functionality from missed refactor. In discord_helpers.update_queue_messages, the old names for update_status and update_count were used instead of what @Aryan-Shastri ended up naming them. The naming conflict was then remedied and more useful names provided for those methods. --- bot.py | 31 +++++++++++++++---------------- ui/helpers/discord_helpers.py | 8 ++++---- 2 files changed, 19 insertions(+), 20 deletions(-) diff --git a/bot.py b/bot.py index 9258048..8ce2d40 100644 --- a/bot.py +++ b/bot.py @@ -45,8 +45,7 @@ async def setup_hook(self): self.add_view(QueueView()) self.add_view(TAView()) daily_reset.start() - auto_queue_scheduler.start(self) - + auto_queue_scheduler.start(self) await self.tree.sync() async def on_ready(self): @@ -57,7 +56,7 @@ async def on_ready(self): if len(self.queue.entries) > 0: self._player_task = asyncio.create_task(self._play_notifications()) - async def get_ta_voice_channel(self) -> discord.VoiceChannel | None: + async def _get_ta_voice_channel(self) -> discord.VoiceChannel | None: for guild in self.guilds: return get(guild.voice_channels, name=TA_VOICE_CHANNEL_NAME) @@ -75,7 +74,7 @@ async def _play_notifications(self) -> None: if empty: break - channel = await self.get_ta_voice_channel() + channel = await self._get_ta_voice_channel() if channel is None: await asyncio.sleep(60) continue @@ -122,23 +121,23 @@ async def _play_notifications(self) -> None: self._player_task = None - async def get_ta_channel(self) -> discord.TextChannel | None: + async def _get_ta_channel(self) -> discord.TextChannel | None: for guild in self.guilds: return get(guild.text_channels, name=TA_TEXT_CHANNEL_NAME) return None - async def get_help_channel(self) -> discord.TextChannel | None: + async def _get_help_channel(self) -> discord.TextChannel | None: for guild in self.guilds: return get(guild.text_channels, name=HELP_CHANNEL_NAME) return None - async def build_queue_status(self) -> str: + async def _build_queue_status(self) -> str: status = "OPEN" if self.queue.is_open else "CLOSED" queue_text = await self.queue.view() return f"**Help Queue Status: {status}**\n{queue_text}" async def _get_status_message(self) -> discord.Message | None: - ta_channel = await self.get_ta_channel() + ta_channel = await self._get_ta_channel() if ta_channel is None: return None @@ -153,25 +152,25 @@ async def _get_status_message(self) -> discord.Message | None: self.queue_status_message_id = message.id return message - status_message = await ta_channel.send(await self.build_queue_status()) + status_message = await ta_channel.send(await self._build_queue_status()) self.queue_status_message_id = status_message.id return status_message - async def _update_status(self) -> None: + async def update_status_for_students(self) -> None: status_message = await self._get_status_message() if status_message is None: return - await status_message.edit(content=await self.build_queue_status()) + await status_message.edit(content=await self._build_queue_status()) - async def build_help_queue_count(self) -> str: + async def _build_help_queue_count(self) -> str: status = "OPEN" if self.queue.is_open else "CLOSED" async with self.queue.lock: count = len(self.queue.entries) return f"**Help Queue Status: {status} — {count} student{'s' if count != 1 else ''} in queue**" async def _get_count_message(self) -> discord.Message | None: - help_channel = await self.get_help_channel() + help_channel = await self._get_help_channel() if help_channel is None: return None @@ -186,16 +185,16 @@ async def _get_count_message(self) -> discord.Message | None: self.help_queue_count_message_id = message.id return message - count_message = await help_channel.send(await self.build_help_queue_count()) + count_message = await help_channel.send(await self._build_help_queue_count()) self.help_queue_count_message_id = count_message.id return count_message - async def _update_count(self) -> None: + async def update_status_for_tas(self) -> None: count_message = await self._get_count_message() if count_message is None: return - await count_message.edit(content=await self.build_help_queue_count()) + await count_message.edit(content=await self._build_help_queue_count()) async def queue_handler(self, interaction: discord.Interaction, question, is_passoff, in_person, student_name: str): """ diff --git a/ui/helpers/discord_helpers.py b/ui/helpers/discord_helpers.py index b41503c..4c89e04 100644 --- a/ui/helpers/discord_helpers.py +++ b/ui/helpers/discord_helpers.py @@ -30,10 +30,10 @@ async def notify_next_if_changed(client: discord.Client, before: Optional[QueueE await safe_dm_user(client, after.user_id, NEXT_IN_LINE_MSG) async def update_queue_messages(client: discord.Client) -> None: - if hasattr(client, "update_queue_status_message"): - await client.update_queue_status_message() - if hasattr(client, "update_help_queue_count_message"): - await client.update_help_queue_count_message() + if hasattr(client, "update_status_for_students"): + await client.update_status_for_students() + if hasattr(client, "update_status_for_tas"): + await client.update_status_for_tas() async def move_to_breakout(interaction: discord.Interaction, entry: QueueEntry): student: discord.Member = interaction.guild.get_member(entry.user_id) From da09dc332b9eef59361f68fc52101a5d17b9dbc0 Mon Sep 17 00:00:00 2001 From: Grant Harris Date: Mon, 8 Jun 2026 15:28:05 -0600 Subject: [PATCH 6/6] Remove redundant imports --- ui/modals.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ui/modals.py b/ui/modals.py index 0b77ece..bf92943 100644 --- a/ui/modals.py +++ b/ui/modals.py @@ -1,9 +1,8 @@ import discord from datetime import datetime from db import get_times_helped_today, record_bot_issue -from ui.helpers.discord_helpers import get_channel, get_role, update_queue_messages +from ui.helpers.discord_helpers import get_channel, get_role, update_queue_messages, notify_next_if_changed from ui.helpers.constants import SHORT_TIMEOUT, TA_TEXT_CHANNEL_NAME -from ui.helpers.discord_helpers import get_channel, get_role, notify_next_if_changed, update_queue_messages class HelpModal(discord.ui.Modal, title="Request Help"): name = discord.ui.TextInput(