diff --git a/apps/discord_bot/src/five08/discord_bot/cogs/jobs.py b/apps/discord_bot/src/five08/discord_bot/cogs/jobs.py index 9173eb39..ba199ab3 100644 --- a/apps/discord_bot/src/five08/discord_bot/cogs/jobs.py +++ b/apps/discord_bot/src/five08/discord_bot/cogs/jobs.py @@ -2373,7 +2373,18 @@ async def _rename_gig_thread_for_status( base_title = stripped_title base_title = base_title.strip() or f"Discord gig {thread.id}" next_name = f"[{status_marker}] {base_title}"[:100] - if thread.name == next_name: + should_close_thread = status is EngagementStatus.LOST + was_lost_thread = parse_status_from_title(raw_title) is EngagementStatus.LOST + is_locked = bool(getattr(thread, "locked", False)) + is_archived = bool(getattr(thread, "archived", False)) + needs_rename = thread.name != next_name + needs_reopen = ( + not should_close_thread and was_lost_thread and (is_locked or is_archived) + ) + needs_close = should_close_thread and (not is_locked or not is_archived) + needs_unarchive_for_rename = needs_rename and is_archived + needs_restore_closed = should_close_thread and needs_unarchive_for_rename + if not needs_rename and not needs_close and not needs_reopen: return next_name if thread.guild is None or thread.guild.me is None: @@ -2382,9 +2393,14 @@ async def _rename_gig_thread_for_status( if not permissions.manage_threads: raise PermissionError("missing_manage_threads_permission") - if thread.archived: + if needs_reopen: + await thread.edit(locked=False, archived=False, reason=reason) + elif needs_unarchive_for_rename: await thread.edit(archived=False, reason=reason) - await thread.edit(name=next_name, reason=reason) + if needs_rename: + await thread.edit(name=next_name, reason=reason) + if needs_close or needs_restore_closed: + await thread.edit(locked=True, archived=True, reason=reason) return next_name @staticmethod @@ -3747,10 +3763,15 @@ async def update_gig_status( ) return + close_note = ( + " and closed this thread" + if normalized_status is EngagementStatus.LOST + else "" + ) await interaction.followup.send( "✅ Updated status to " - f"**{status_label(normalized_status)}** and renamed this thread to " - f"`{next_title}`.", + f"**{status_label(normalized_status)}**, renamed this thread to " + f"`{next_title}`{close_note}.", ephemeral=True, ) diff --git a/apps/discord_bot/src/five08/discord_bot/utils/internal_api.py b/apps/discord_bot/src/five08/discord_bot/utils/internal_api.py index db80cf9f..5db44779 100644 --- a/apps/discord_bot/src/five08/discord_bot/utils/internal_api.py +++ b/apps/discord_bot/src/five08/discord_bot/utils/internal_api.py @@ -11,7 +11,12 @@ from pydantic import BaseModel, ValidationError from five08.discord_bot.config import settings -from five08.engagements import normalize_engagement_status, strip_status_from_title +from five08.engagements import ( + EngagementStatus, + normalize_engagement_status, + parse_status_from_title, + strip_status_from_title, +) logger = logging.getLogger(__name__) @@ -335,36 +340,70 @@ async def _update_gig_thread_status( **permission_payload, }, 403 - base_title = strip_status_from_title(channel.name) or channel.name + raw_title = str(channel.name or "").strip() + stripped_title = strip_status_from_title(raw_title) + if ( + parse_status_from_title(raw_title) is not EngagementStatus.UNKNOWN + and stripped_title == raw_title + ): + base_title = "" + else: + base_title = stripped_title base_title = base_title.strip() or f"Discord gig {thread_id}" next_name = f"[{status_marker}] {base_title}"[:100] - if channel.name == next_name: + should_close_thread = normalized_status is EngagementStatus.LOST + was_lost_thread = parse_status_from_title(raw_title) is EngagementStatus.LOST + is_locked = bool(getattr(channel, "locked", False)) + is_archived = bool(getattr(channel, "archived", False)) + needs_rename = channel.name != next_name + needs_reopen = ( + not should_close_thread and was_lost_thread and (is_locked or is_archived) + ) + needs_close = should_close_thread and (not is_locked or not is_archived) + needs_unarchive_for_rename = needs_rename and is_archived + needs_restore_closed = should_close_thread and needs_unarchive_for_rename + if not needs_rename and not needs_close and not needs_reopen: return { "status": "unchanged", "thread_id": str(thread_id), "title": next_name, + "closed": should_close_thread, }, 200 try: - if channel.archived: + if needs_reopen: await channel.edit( + locked=False, archived=False, reason="Dashboard gig status update", ) - await channel.edit( - name=next_name, - reason="Dashboard gig status update", - ) + elif needs_unarchive_for_rename: + await channel.edit( + archived=False, + reason="Dashboard gig status update", + ) + if needs_rename: + await channel.edit( + name=next_name, + reason="Dashboard gig status update", + ) + if needs_close or needs_restore_closed: + await channel.edit( + locked=True, + archived=True, + reason="Dashboard gig status update", + ) except discord.Forbidden: - return {"error": "thread_rename_forbidden"}, 403 + return {"error": "thread_update_forbidden"}, 403 except discord.HTTPException as exc: - logger.warning("Failed renaming gig thread %s: %s", thread_id, exc) - return {"error": "thread_rename_failed"}, 502 + logger.warning("Failed updating gig thread %s: %s", thread_id, exc) + return {"error": "thread_update_failed"}, 502 return { "status": "updated", "thread_id": str(thread_id), "title": next_name, + "closed": should_close_thread, }, 200 async def gig_thread_status_handler(self, request: web.Request) -> web.Response: diff --git a/tests/unit/test_internal_api.py b/tests/unit/test_internal_api.py index 9c5808b7..35798cb0 100644 --- a/tests/unit/test_internal_api.py +++ b/tests/unit/test_internal_api.py @@ -3,7 +3,7 @@ import asyncio from types import SimpleNamespace import json -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock, Mock, call import discord import pytest @@ -162,6 +162,182 @@ def permissions_for(self, _member: object) -> SimpleNamespace: reason="Dashboard gig status update", ) + @pytest.mark.asyncio + async def test_update_gig_thread_status_uses_fallback_for_marker_only_title( + self, internal_api_routes, monkeypatch: pytest.MonkeyPatch + ): + """Dashboard status sync should not stack markers for marker-only titles.""" + + class FakeThread: + id = 123 + name = "[RECRUITING]" + archived = False + locked = False + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace( + manage_threads=True, + view_channel=True, + send_messages_in_threads=True, + ) + + thread = FakeThread() + monkeypatch.setattr( + "five08.discord_bot.utils.internal_api.discord.Thread", + FakeThread, + ) + internal_api_routes.bot.get_channel.return_value = thread + + result, status_code = await internal_api_routes._update_gig_thread_status( + GigThreadStatusRequest(thread_id="123", status="filled") + ) + + assert status_code == 200 + assert result["title"] == "[FILLED] Discord gig 123" + thread.edit.assert_awaited_once_with( + name="[FILLED] Discord gig 123", + reason="Dashboard gig status update", + ) + + @pytest.mark.asyncio + async def test_update_gig_thread_status_closes_lost_thread( + self, internal_api_routes, monkeypatch: pytest.MonkeyPatch + ): + """Dashboard lost status changes should lock and archive the Discord thread.""" + + class FakeThread: + id = 123 + name = "[RECRUITING] Old gig" + archived = False + locked = False + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace( + manage_threads=True, + view_channel=True, + send_messages_in_threads=True, + ) + + thread = FakeThread() + monkeypatch.setattr( + "five08.discord_bot.utils.internal_api.discord.Thread", + FakeThread, + ) + internal_api_routes.bot.get_channel.return_value = thread + + result, status_code = await internal_api_routes._update_gig_thread_status( + GigThreadStatusRequest(thread_id="123", status="lost") + ) + + assert status_code == 200 + assert result["status"] == "updated" + assert result["title"] == "[LOST] Old gig" + assert result["closed"] is True + assert thread.edit.await_args_list == [ + call(name="[LOST] Old gig", reason="Dashboard gig status update"), + call( + locked=True, + archived=True, + reason="Dashboard gig status update", + ), + ] + + @pytest.mark.asyncio + async def test_update_gig_thread_status_reopens_non_lost_thread( + self, internal_api_routes, monkeypatch: pytest.MonkeyPatch + ): + """Moving a closed lost thread away from lost should make it usable again.""" + + class FakeThread: + id = 123 + name = "[LOST] Old gig" + archived = True + locked = True + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace( + manage_threads=True, + view_channel=True, + send_messages_in_threads=True, + ) + + thread = FakeThread() + monkeypatch.setattr( + "five08.discord_bot.utils.internal_api.discord.Thread", + FakeThread, + ) + internal_api_routes.bot.get_channel.return_value = thread + + result, status_code = await internal_api_routes._update_gig_thread_status( + GigThreadStatusRequest(thread_id="123", status="recruiting") + ) + + assert status_code == 200 + assert result["status"] == "updated" + assert result["title"] == "[RECRUITING] Old gig" + assert result["closed"] is False + assert thread.edit.await_args_list == [ + call( + locked=False, + archived=False, + reason="Dashboard gig status update", + ), + call(name="[RECRUITING] Old gig", reason="Dashboard gig status update"), + ] + + @pytest.mark.asyncio + async def test_update_gig_thread_status_preserves_moderator_lock( + self, internal_api_routes, monkeypatch: pytest.MonkeyPatch + ): + """Non-lost status sync should not clear locks unrelated to lost closure.""" + + class FakeThread: + id = 123 + name = "[RECRUITING] Old gig" + archived = False + locked = True + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace( + manage_threads=True, + view_channel=True, + send_messages_in_threads=True, + ) + + thread = FakeThread() + monkeypatch.setattr( + "five08.discord_bot.utils.internal_api.discord.Thread", + FakeThread, + ) + internal_api_routes.bot.get_channel.return_value = thread + + result, status_code = await internal_api_routes._update_gig_thread_status( + GigThreadStatusRequest(thread_id="123", status="filled") + ) + + assert status_code == 200 + assert result["title"] == "[FILLED] Old gig" + thread.edit.assert_awaited_once_with( + name="[FILLED] Old gig", + reason="Dashboard gig status update", + ) + @pytest.mark.asyncio async def test_update_gig_thread_status_reports_missing_manage_threads( self, internal_api_routes, monkeypatch: pytest.MonkeyPatch diff --git a/tests/unit/test_jobs.py b/tests/unit/test_jobs.py index 1a1660e4..e6253371 100644 --- a/tests/unit/test_jobs.py +++ b/tests/unit/test_jobs.py @@ -4,7 +4,7 @@ import asyncio from datetime import datetime, timedelta, timezone -from unittest.mock import AsyncMock, Mock +from unittest.mock import AsyncMock, Mock, call from types import SimpleNamespace from unittest.mock import patch @@ -176,6 +176,96 @@ def permissions_for(self, _member: object) -> SimpleNamespace: ) +def test_rename_gig_thread_for_lost_status_locks_and_archives() -> None: + class FakeThread: + id = 200 + name = "[RECRUITING] Need help" + archived = False + locked = False + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace(manage_threads=True) + + thread = FakeThread() + + result = asyncio.run( + JobsCog._rename_gig_thread_for_status( + thread, + EngagementStatus.LOST, + reason="test", + ) + ) + + assert result == "[LOST] Need help" + assert thread.edit.await_args_list == [ + call(name="[LOST] Need help", reason="test"), + call(locked=True, archived=True, reason="test"), + ] + + +def test_rename_gig_thread_for_non_lost_status_reopens_thread() -> None: + class FakeThread: + id = 200 + name = "[LOST] Need help" + archived = True + locked = True + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace(manage_threads=True) + + thread = FakeThread() + + result = asyncio.run( + JobsCog._rename_gig_thread_for_status( + thread, + EngagementStatus.RECRUITING, + reason="test", + ) + ) + + assert result == "[RECRUITING] Need help" + assert thread.edit.await_args_list == [ + call(locked=False, archived=False, reason="test"), + call(name="[RECRUITING] Need help", reason="test"), + ] + + +def test_rename_gig_thread_for_non_lost_status_preserves_moderator_lock() -> None: + class FakeThread: + id = 200 + name = "[RECRUITING] Need help" + archived = False + locked = True + guild = SimpleNamespace(me=object()) + + def __init__(self) -> None: + self.edit = AsyncMock() + + def permissions_for(self, _member: object) -> SimpleNamespace: + return SimpleNamespace(manage_threads=True) + + thread = FakeThread() + + result = asyncio.run( + JobsCog._rename_gig_thread_for_status( + thread, + EngagementStatus.FILLED, + reason="test", + ) + ) + + assert result == "[FILLED] Need help" + thread.edit.assert_awaited_once_with(name="[FILLED] Need help", reason="test") + + def test_build_match_candidate_lines_handles_non_string_names() -> None: candidate = _make_candidate( is_member=True,