From a1d41defd51ea8a179bace800f2f4829572fa810 Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 22:56:23 -0400 Subject: [PATCH 1/8] Enhance Discord subscription command filters and status formatting. This aligns slash-command raid selection with RaidHub manifest choices, sends typed rule filters to the API, and standardizes subscription status rule display per player/clan entry for clearer UX. Made-with: Cursor --- src/commands/subscribe.py | 62 +++++++++---- src/commands/subscription.py | 18 ++-- src/commands/subscription_helpers.py | 124 +++++++++++++++++++++++--- src/commands/subscription_messages.py | 33 +++++++ src/commands/unsubscribe.py | 122 +++++++++++++++++++------ src/manifest/__init__.py | 4 +- src/manifest/builders.py | 102 +++++++++++++++------ src/manifest/schema.py | 15 ++++ src/sync_commands.py | 50 ++++++++++- tests/test_manifest_commands.py | 30 ++++++- tests/test_subscription_helpers.py | 45 +++++++++- tests/test_sync_commands.py | 23 +++++ 12 files changed, 531 insertions(+), 97 deletions(-) create mode 100644 src/commands/subscription_messages.py create mode 100644 tests/test_sync_commands.py diff --git a/src/commands/subscribe.py b/src/commands/subscribe.py index 3ed7622..a63defe 100644 --- a/src/commands/subscribe.py +++ b/src/commands/subscribe.py @@ -16,6 +16,14 @@ resolve_player_membership_id, resolve_player_subscription_row, ) +from .subscription_messages import ( + CLAN_ID_NOT_RECOGNIZED_TITLE, + PLAYER_NOT_FOUND_TITLE, + SUBSCRIBE_COMMAND_TITLE, + SUBSCRIBE_FAILED_TITLE, + SUBSCRIPTION_SAVED_TITLE, + subscribe_success_description, +) from .subscription_helpers import ( fetch_subscription_status_envelope, format_clan_display_name, @@ -52,7 +60,7 @@ async def run_subscribe_deferred( app_id, token, warn_embed( - "Subscribe Command", + SUBSCRIBE_COMMAND_TITLE, "Use `/subscribe player` or `/subscribe clan` with a target.", ), ) @@ -63,18 +71,27 @@ async def run_subscribe_deferred( await patch_discord_followup_best_effort( app_id, token, - warn_embed("Subscribe Command", "Unknown `/subscribe` subcommand."), + warn_embed(SUBSCRIBE_COMMAND_TITLE, "Unknown `/subscribe` subcommand."), ) return leaf = flatten_options(top_opts[0].get("options")) target_raw = str(leaf.get("player") or leaf.get("clan") or "").strip() + filters: dict[str, Any] = {} + if "require_fresh" in leaf: + filters["requireFresh"] = bool(leaf["require_fresh"]) + if "require_completed" in leaf: + filters["requireCompleted"] = bool(leaf["require_completed"]) + if "raid" in leaf: + raw_raid = leaf.get("raid") + if isinstance(raw_raid, int): + filters["raid"] = raw_raid if not target_raw: await patch_discord_followup_best_effort( app_id, token, warn_embed( - "Subscribe Command", + SUBSCRIBE_COMMAND_TITLE, "Provide a target (membership id / player name, or clan id / URL).", ), ) @@ -85,7 +102,7 @@ async def run_subscribe_deferred( app_id, token, warn_embed( - "Subscribe Command", + SUBSCRIBE_COMMAND_TITLE, "Run `/subscribe` in a server text channel, not a DM.", ), ) @@ -97,7 +114,7 @@ async def run_subscribe_deferred( app_id, token, error_embed( - "Subscribe Failed", + SUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(status_env), ), ) @@ -112,7 +129,7 @@ async def run_subscribe_deferred( app_id, token, error_embed( - "Player Not Found", + PLAYER_NOT_FOUND_TITLE, "Could not resolve that player. Try a Destiny membership id or a clearer" " name (first RaidHub search hit is used).", ), @@ -124,7 +141,10 @@ async def run_subscribe_deferred( await patch_discord_followup_best_effort( app_id, token, - error_embed("Player Not Found", "Missing membership id for that player."), + error_embed( + PLAYER_NOT_FOUND_TITLE, + "Missing membership id for that player.", + ), ) return if registered: @@ -135,6 +155,8 @@ async def run_subscribe_deferred( body: dict[str, Any] = {"targets": {"playerMembershipIds": merged_players}} else: body = {"targets": {"playerMembershipIds": [resolved_id]}} + if filters: + body["filters"] = filters display_name = format_player_display_name(prow) icon_raw = prow.get("iconPath") thumb_url = ( @@ -149,7 +171,7 @@ async def run_subscribe_deferred( app_id, token, error_embed( - "Clan ID Not Recognized", + CLAN_ID_NOT_RECOGNIZED_TITLE, "Could not parse a clan group id from that value. Use digits only, or a" " raidhub.io/clan/... / Bungie clan URL containing the group id.", ), @@ -164,6 +186,8 @@ async def run_subscribe_deferred( body = {"targets": {"clanGroupIds": merged_clans}} else: body = {"targets": {"clanGroupIds": [resolved_id]}} + if filters: + body["filters"] = filters ctx = discord_invocation_context(interaction, route_id=SUB_ROUTE_PUT) env = await raidhub.request_envelope( @@ -178,19 +202,20 @@ async def run_subscribe_deferred( app_id, token, error_embed( - "Subscribe Failed", + SUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(env), ), ) return if sub == "player": - desc = ( - f"Subscribed to **{display_name}** (`{resolved_id}`) for this channel. " - "Use `/subscription status` to see all rules for this channel." + desc = subscribe_success_description( + display_name, + resolved_id, + str(interaction.get("channel_id") or ""), ) msg = success_embed( - "Subscription Saved", + SUBSCRIPTION_SAVED_TITLE, desc, thumbnail_url=thumb_url, ) @@ -208,12 +233,13 @@ async def run_subscribe_deferred( if clan_row and isinstance(apath, str) else None ) - desc = ( - f"Subscribed to **{c_disp}** (`{resolved_id}`) for this channel. " - "Use `/subscription status` to see all rules for this channel." + desc = subscribe_success_description( + c_disp, + resolved_id, + str(interaction.get("channel_id") or ""), ) msg = success_embed( - "Subscription Saved", + SUBSCRIPTION_SAVED_TITLE, desc, thumbnail_url=c_thumb, ) @@ -226,7 +252,7 @@ async def run_subscribe_deferred( err=err, discord_application_id=app_id, interaction_token=token, - user_message_payload=error_embed("Subscribe Failed", USER_FACING_GENERIC), + user_message_payload=error_embed(SUBSCRIBE_FAILED_TITLE, USER_FACING_GENERIC), ) finally: observe_deferred_completion(command="subscribe", outcome=outcome) diff --git a/src/commands/subscription.py b/src/commands/subscription.py index 2e5e1fd..f324c3f 100644 --- a/src/commands/subscription.py +++ b/src/commands/subscription.py @@ -5,6 +5,11 @@ from ..config import Settings from ..prom_metrics import observe_deferred_completion from ..raidhub_client import RaidHubClient, discord_invocation_context +from .subscription_messages import ( + SUBSCRIPTION_COMMAND_TITLE, + SUBSCRIPTION_REMOVED_TITLE, + SUBSCRIPTION_REQUEST_FAILED_TITLE, +) from .subscription_helpers import ( format_subscription_status_embed, subscription_envelope_error_message, @@ -37,7 +42,7 @@ async def run_subscription_deferred( app_id, token, warn_embed( - "Subscription Command", + SUBSCRIPTION_COMMAND_TITLE, "Pick `status` or `delete` under `/subscription`.", ), ) @@ -48,7 +53,10 @@ async def run_subscription_deferred( await patch_discord_followup_best_effort( app_id, token, - warn_embed("Subscription Command", "Unknown `/subscription` subcommand."), + warn_embed( + SUBSCRIPTION_COMMAND_TITLE, + "Unknown `/subscription` subcommand.", + ), ) return @@ -57,7 +65,7 @@ async def run_subscription_deferred( app_id, token, warn_embed( - "Subscription Command", + SUBSCRIPTION_COMMAND_TITLE, "Run this command in a server channel, not a DM.", ), ) @@ -84,7 +92,7 @@ async def run_subscription_deferred( app_id, token, error_embed( - "Subscription Request Failed", + SUBSCRIPTION_REQUEST_FAILED_TITLE, subscription_envelope_error_message(env), ), ) @@ -95,7 +103,7 @@ async def run_subscription_deferred( msg = await format_subscription_status_embed(raidhub, inner) else: msg = success_embed( - "Subscription Removed", + SUBSCRIPTION_REMOVED_TITLE, "RaidHub will no longer use a webhook in this channel.", ) diff --git a/src/commands/subscription_helpers.py b/src/commands/subscription_helpers.py index 3da73de..fd595b1 100644 --- a/src/commands/subscription_helpers.py +++ b/src/commands/subscription_helpers.py @@ -34,7 +34,7 @@ def subscription_active_clan_ids(inner: dict[str, Any]) -> list[str]: out: list[str] = [] for item in inner.get("clans") or []: if isinstance(item, dict): - raw = item.get("groupId") + raw = item.get("groupId") or item.get("clanGroupId") else: raw = item if raw is None: @@ -132,7 +132,10 @@ def _ordered_membership_ids(pl_raw: list[Any]) -> list[str]: def _ordered_group_ids(cl_raw: list[Any]) -> list[str]: out: list[str] = [] for item in cl_raw: - raw = item.get("groupId") if isinstance(item, dict) else item + if isinstance(item, dict): + raw = item.get("groupId") or item.get("clanGroupId") + else: + raw = item if raw is None: continue s = str(raw).strip() @@ -155,20 +158,36 @@ async def _fetch_clan_basic_card(raidhub: RaidHubClient, group_id: str) -> dict[ return inner if env.get("success") and isinstance(inner, dict) else {} -def _player_rule_line(membership_id: str, card: dict[str, Any]) -> str: +def _standardized_rule_string(rule: dict[str, Any]) -> str: + fresh = "yes" if bool(rule.get("requireFresh")) else "no" + completed = "yes" if bool(rule.get("requireCompleted")) else "no" + raid_id = rule.get("raidId") + raid_path = rule.get("raidPath") + if raid_id is not None and str(raid_id).strip() != "": + raid = f"raid:{raid_id}" + elif raid_path: + raid = f"raid:{str(raid_path).strip()}" + else: + raid = "raid:all" + return f"`fresh:{fresh}` `completed:{completed}` `{raid}`" + + +def _player_rule_line(membership_id: str, card: dict[str, Any], rule: dict[str, Any]) -> str: + rule_suffix = _standardized_rule_string(rule) if card: label = _embed_safe_label(format_player_display_name(card)) url = _raidhub_player_url(membership_id) - return f"• [{label}]({url}) · `{membership_id}`" - return f"• `{membership_id}`" + return f"• [{label}]({url}) · `{membership_id}` · {rule_suffix}" + return f"• `{membership_id}` · {rule_suffix}" -def _clan_rule_line(group_id: str, card: dict[str, Any]) -> str: +def _clan_rule_line(group_id: str, card: dict[str, Any], rule: dict[str, Any]) -> str: + rule_suffix = _standardized_rule_string(rule) if card: label = _embed_safe_label(format_clan_display_name(card)) url = _raidhub_clan_url(group_id) - return f"• [{label}]({url}) · `{group_id}`" - return f"• `{group_id}`" + return f"• [{label}]({url}) · `{group_id}` · {rule_suffix}" + return f"• `{group_id}` · {rule_suffix}" def _id_only_rule_lines(ids: list[str]) -> str: @@ -182,6 +201,59 @@ def _id_only_rule_lines(ids: list[str]) -> str: return body[:1024] +def _indexed_player_rules(pl_raw: list[Any]) -> dict[str, dict[str, Any]]: + out: dict[str, dict[str, Any]] = {} + for item in pl_raw: + if not isinstance(item, dict): + continue + raw = item.get("membershipId") + if raw is None: + continue + s = str(raw).strip() + if not s.isdigit(): + continue + out[str(int(s))] = item + return out + + +def _indexed_clan_rules(cl_raw: list[Any]) -> dict[str, dict[str, Any]]: + out: dict[str, dict[str, Any]] = {} + for item in cl_raw: + if not isinstance(item, dict): + continue + raw = item.get("groupId") or item.get("clanGroupId") + if raw is None: + continue + s = str(raw).strip() + if not s.isdigit(): + continue + out[str(int(s))] = item + return out + + +def _rule_filter_state(items: list[Any], key: str) -> str: + values = [item.get(key) for item in items if isinstance(item, dict) and key in item] + bool_values = [bool(v) for v in values if isinstance(v, bool)] + if not bool_values: + return "unset" + uniq = set(bool_values) + if len(uniq) > 1: + return "mixed" + return "yes" if True in uniq else "no" + + +def _rule_filters_summary(pl_raw: list[Any], cl_raw: list[Any]) -> str: + all_rules = [*pl_raw, *cl_raw] + if not all_rules: + return "—" + fresh = _rule_filter_state(all_rules, "requireFresh") + completed = _rule_filter_state(all_rules, "requireCompleted") + return ( + f"Require Fresh: **{fresh}**\n" + f"Require Completed: **{completed}**" + )[:1024] + + async def format_subscription_status_embed( raidhub: RaidHubClient | None, data: dict[str, Any], @@ -195,7 +267,6 @@ async def format_subscription_status_embed( fails = int(data.get("consecutiveDeliveryFailures") or 0) fields: list[dict[str, Any]] = [ {"name": "Destination Active", "value": active, "inline": True}, - {"name": "Webhook ID", "value": f"`{data.get('webhookId', '—')}`", "inline": True}, {"name": "Delivery Failures", "value": f"**{fails}**", "inline": True}, ] ls = data.get("lastDeliverySuccessAt") @@ -222,8 +293,17 @@ async def format_subscription_status_embed( cl_raw = list(data.get("clans") or []) pl_ids = _ordered_membership_ids(pl_raw) cl_ids = _ordered_group_ids(cl_raw) + player_rules = _indexed_player_rules(pl_raw) + clan_rules = _indexed_clan_rules(cl_raw) pc = len(pl_ids) cc = len(cl_ids) + fields.append( + { + "name": "Rule Filters", + "value": _rule_filters_summary(pl_raw, cl_raw), + "inline": False, + } + ) player_cards: dict[str, dict[str, Any]] = {} clan_cards: dict[str, dict[str, Any]] = {} @@ -244,14 +324,32 @@ async def format_subscription_status_embed( clan_cards[gid] = card if isinstance(card, dict) else {} if raidhub is None: - p_body = _id_only_rule_lines(pl_ids) if pl_ids else "—" - c_body = _id_only_rule_lines(cl_ids) if cl_ids else "—" + p_lines = [ + f"• `{mid}` · {_standardized_rule_string(player_rules.get(mid, {}))}" + for mid in pl_ids + ] + p_body = "\n".join(p_lines) if p_lines else "—" + c_lines = [ + f"• `{gid}` · {_standardized_rule_string(clan_rules.get(gid, {}))}" + for gid in cl_ids + ] + c_body = "\n".join(c_lines) if c_lines else "—" + if len(p_body) > 1024: + p_body = p_body[:1021] + "..." + if len(c_body) > 1024: + c_body = c_body[:1021] + "..." else: - p_lines = [_player_rule_line(mid, player_cards.get(mid, {})) for mid in pl_ids] + p_lines = [ + _player_rule_line(mid, player_cards.get(mid, {}), player_rules.get(mid, {})) + for mid in pl_ids + ] p_body = "\n".join(p_lines) if p_lines else "—" if len(p_body) > 1024: p_body = p_body[:1021] + "..." - c_lines = [_clan_rule_line(gid, clan_cards.get(gid, {})) for gid in cl_ids] + c_lines = [ + _clan_rule_line(gid, clan_cards.get(gid, {}), clan_rules.get(gid, {})) + for gid in cl_ids + ] c_body = "\n".join(c_lines) if c_lines else "—" if len(c_body) > 1024: c_body = c_body[:1021] + "..." diff --git a/src/commands/subscription_messages.py b/src/commands/subscription_messages.py new file mode 100644 index 0000000..5479992 --- /dev/null +++ b/src/commands/subscription_messages.py @@ -0,0 +1,33 @@ +from __future__ import annotations + +SUBSCRIBE_COMMAND_TITLE = "Subscribe Command" +SUBSCRIBE_FAILED_TITLE = "Subscribe Failed" +SUBSCRIPTION_COMMAND_TITLE = "Subscription Command" +SUBSCRIPTION_REQUEST_FAILED_TITLE = "Subscription Request Failed" +UNSUBSCRIBE_COMMAND_TITLE = "Unsubscribe Command" +UNSUBSCRIBE_FAILED_TITLE = "Unsubscribe Failed" +UNSUBSCRIBE_PLAYER_TITLE = "Unsubscribe Player" +UNSUBSCRIBE_CLAN_TITLE = "Unsubscribe Clan" +PLAYER_NOT_FOUND_TITLE = "Player Not Found" +CLAN_ID_NOT_RECOGNIZED_TITLE = "Clan ID Not Recognized" +SUBSCRIPTION_SAVED_TITLE = "Subscription Saved" +SUBSCRIPTION_REMOVED_TITLE = "Subscription Removed" +PLAYER_UNSUBSCRIBED_TITLE = "Player Unsubscribed" +CLAN_UNSUBSCRIBED_TITLE = "Clan Unsubscribed" +SUBSCRIPTION_STATUS_RULES_HINT = ( + "Use `/subscription status` to see all rules for this channel." +) + + +def subscribe_success_description(display_label: str, resolved_id: str, channel_id: str) -> str: + return ( + f"Subscribed to **{display_label}** (`{resolved_id}`) in <#{channel_id}>. " + f"{SUBSCRIPTION_STATUS_RULES_HINT}" + ) + + +def unsubscribe_success_description(display_label: str, resolved_id: str) -> str: + return ( + f"Removed **{display_label}** (`{resolved_id}`) from this channel. Other subscription " + "rules are unchanged." + ) diff --git a/src/commands/unsubscribe.py b/src/commands/unsubscribe.py index b99646d..f4c98eb 100644 --- a/src/commands/unsubscribe.py +++ b/src/commands/unsubscribe.py @@ -5,9 +5,27 @@ from ..config import Settings from ..prom_metrics import observe_deferred_completion from ..raidhub_client import RaidHubClient, discord_invocation_context -from .subscribe_resolution import parse_clan_group_id, resolve_player_membership_id +from .subscribe_resolution import ( + bungie_emblem_url, + format_player_display_name, + parse_clan_group_id, + resolve_player_subscription_row, +) +from .subscription_messages import ( + CLAN_ID_NOT_RECOGNIZED_TITLE, + CLAN_UNSUBSCRIBED_TITLE, + PLAYER_NOT_FOUND_TITLE, + PLAYER_UNSUBSCRIBED_TITLE, + SUBSCRIPTION_REMOVED_TITLE, + UNSUBSCRIBE_CLAN_TITLE, + UNSUBSCRIBE_COMMAND_TITLE, + UNSUBSCRIBE_FAILED_TITLE, + UNSUBSCRIBE_PLAYER_TITLE, + unsubscribe_success_description, +) from .subscription_helpers import ( fetch_subscription_status_envelope, + format_clan_display_name, subscription_active_clan_ids, subscription_active_player_ids, subscription_envelope_error_message, @@ -34,12 +52,37 @@ async def run_unsubscribe_deferred( token = str(interaction.get("token") or "") outcome = "completed" try: + data = interaction.get("data") or {} + top_opts = data.get("options") or [] + sub = "" + if top_opts and isinstance(top_opts[0], dict): + sub = str(top_opts[0].get("name") or "").strip().lower() + if not sub: + sub = "delete" + + if sub == "player": + await run_unsubscribe_player_deferred(interaction, raidhub, settings) + return + if sub == "clan": + await run_unsubscribe_clan_deferred(interaction, raidhub, settings) + return + if sub != "delete": + await patch_discord_followup_best_effort( + app_id, + token, + warn_embed( + UNSUBSCRIBE_COMMAND_TITLE, + "Use `/unsubscribe delete`, `/unsubscribe player`, or `/unsubscribe clan`.", + ), + ) + return + if not interaction.get("guild_id") or not interaction.get("channel_id"): await patch_discord_followup_best_effort( app_id, token, warn_embed( - "Unsubscribe Command", + UNSUBSCRIBE_COMMAND_TITLE, "Run `/unsubscribe` in a server text channel, not a DM.", ), ) @@ -56,7 +99,7 @@ async def run_unsubscribe_deferred( app_id, token, error_embed( - "Unsubscribe Failed", + UNSUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(env), ), ) @@ -66,7 +109,7 @@ async def run_unsubscribe_deferred( app_id, token, success_embed( - "Subscription Removed", + SUBSCRIPTION_REMOVED_TITLE, "RaidHub will no longer use a webhook in this channel.", ), ) @@ -78,7 +121,7 @@ async def run_unsubscribe_deferred( err=err, discord_application_id=app_id, interaction_token=token, - user_message_payload=error_embed("Unsubscribe Failed", USER_FACING_GENERIC), + user_message_payload=error_embed(UNSUBSCRIBE_FAILED_TITLE, USER_FACING_GENERIC), ) finally: observe_deferred_completion(command="unsubscribe", outcome=outcome) @@ -100,7 +143,7 @@ async def run_unsubscribe_player_deferred( app_id, token, warn_embed( - "Unsubscribe Player", + UNSUBSCRIBE_PLAYER_TITLE, "Provide a Destiny membership id or player search text.", ), ) @@ -111,23 +154,32 @@ async def run_unsubscribe_player_deferred( app_id, token, warn_embed( - "Unsubscribe Player", + UNSUBSCRIBE_PLAYER_TITLE, "Run this command in a server text channel, not a DM.", ), ) return - resolved_id = await resolve_player_membership_id(raidhub, target_raw) - if not resolved_id: + prow = await resolve_player_subscription_row(raidhub, target_raw) + if not prow: await patch_discord_followup_best_effort( app_id, token, error_embed( - "Player Not Found", + PLAYER_NOT_FOUND_TITLE, "Could not resolve that player. Try a membership id or a clearer name.", ), ) return + raw_mid = prow.get("membershipId") + resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" + if not resolved_id or not resolved_id.isdigit(): + await patch_discord_followup_best_effort( + app_id, + token, + error_embed(PLAYER_NOT_FOUND_TITLE, "Missing membership id for that player."), + ) + return status_env = await fetch_subscription_status_envelope(raidhub, interaction) if not status_env.get("success"): @@ -135,7 +187,7 @@ async def run_unsubscribe_player_deferred( app_id, token, error_embed( - "Unsubscribe Failed", + UNSUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(status_env), ), ) @@ -146,7 +198,7 @@ async def run_unsubscribe_player_deferred( app_id, token, warn_embed( - "Unsubscribe Player", + UNSUBSCRIBE_PLAYER_TITLE, "This channel has no RaidHub subscription to update.", ), ) @@ -158,7 +210,7 @@ async def run_unsubscribe_player_deferred( app_id, token, warn_embed( - "Unsubscribe Player", + UNSUBSCRIBE_PLAYER_TITLE, f"This channel is not subscribed to player `{resolved_id}`.", ), ) @@ -179,7 +231,7 @@ async def run_unsubscribe_player_deferred( app_id, token, error_embed( - "Unsubscribe Failed", + UNSUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(env), ), ) @@ -189,9 +241,12 @@ async def run_unsubscribe_player_deferred( app_id, token, success_embed( - "Player Unsubscribed", - f"Removed player `{resolved_id}` from this channel. Other subscription rules are" - " unchanged.", + PLAYER_UNSUBSCRIBED_TITLE, + unsubscribe_success_description( + format_player_display_name(prow), + resolved_id, + ), + thumbnail_url=bungie_emblem_url(str(prow.get("iconPath") or "")), ), ) except Exception as err: @@ -202,7 +257,7 @@ async def run_unsubscribe_player_deferred( err=err, discord_application_id=app_id, interaction_token=token, - user_message_payload=error_embed("Unsubscribe Failed", USER_FACING_GENERIC), + user_message_payload=error_embed(UNSUBSCRIBE_FAILED_TITLE, USER_FACING_GENERIC), ) finally: observe_deferred_completion(command="unsubscribe-player", outcome=outcome) @@ -224,7 +279,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, warn_embed( - "Unsubscribe Clan", + UNSUBSCRIBE_CLAN_TITLE, "Provide a clan group id or clan URL.", ), ) @@ -235,7 +290,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, warn_embed( - "Unsubscribe Clan", + UNSUBSCRIBE_CLAN_TITLE, "Run this command in a server text channel, not a DM.", ), ) @@ -247,7 +302,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, error_embed( - "Clan ID Not Recognized", + CLAN_ID_NOT_RECOGNIZED_TITLE, "Could not parse a clan group id from that value.", ), ) @@ -260,7 +315,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, error_embed( - "Unsubscribe Failed", + UNSUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(status_env), ), ) @@ -271,7 +326,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, warn_embed( - "Unsubscribe Clan", + UNSUBSCRIBE_CLAN_TITLE, "This channel has no RaidHub subscription to update.", ), ) @@ -283,7 +338,7 @@ async def run_unsubscribe_clan_deferred( app_id, token, warn_embed( - "Unsubscribe Clan", + UNSUBSCRIBE_CLAN_TITLE, f"This channel is not subscribed to clan `{resolved_id}`.", ), ) @@ -304,19 +359,28 @@ async def run_unsubscribe_clan_deferred( app_id, token, error_embed( - "Unsubscribe Failed", + UNSUBSCRIBE_FAILED_TITLE, subscription_envelope_error_message(env), ), ) return + c_env = await raidhub.request_envelope("GET", f"/clan/{resolved_id}/basic") + clan_row: dict[str, Any] = {} + if c_env.get("success") and isinstance(c_env.get("response"), dict): + clan_row = c_env["response"] + c_disp = format_clan_display_name(clan_row) if clan_row else f"Clan `{resolved_id}`" + c_thumb = ( + bungie_emblem_url(str(clan_row.get("avatarPath") or "")) if clan_row else None + ) + await patch_discord_followup_best_effort( app_id, token, success_embed( - "Clan Unsubscribed", - f"Removed clan `{resolved_id}` from this channel. Other subscription rules are" - " unchanged.", + CLAN_UNSUBSCRIBED_TITLE, + unsubscribe_success_description(c_disp, resolved_id), + thumbnail_url=c_thumb, ), ) except Exception as err: @@ -327,7 +391,7 @@ async def run_unsubscribe_clan_deferred( err=err, discord_application_id=app_id, interaction_token=token, - user_message_payload=error_embed("Unsubscribe Failed", USER_FACING_GENERIC), + user_message_payload=error_embed(UNSUBSCRIBE_FAILED_TITLE, USER_FACING_GENERIC), ) finally: observe_deferred_completion(command="unsubscribe-clan", outcome=outcome) diff --git a/src/manifest/__init__.py b/src/manifest/__init__.py index ed90112..986e7aa 100644 --- a/src/manifest/__init__.py +++ b/src/manifest/__init__.py @@ -1,8 +1,8 @@ from .builders import build_commands -def build_command_manifest() -> list[dict]: - return [c.to_json() for c in build_commands()] +def build_command_manifest(raid_filter_choices: list[tuple[str, int]] | None = None) -> list[dict]: + return [c.to_json() for c in build_commands(raid_filter_choices=raid_filter_choices)] __all__ = ["build_command_manifest", "build_commands"] diff --git a/src/manifest/builders.py b/src/manifest/builders.py index 0b4567d..a127de1 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -1,9 +1,14 @@ from __future__ import annotations -from .schema import CommandDto, CommandOptionDto, CommandOptionType +from .schema import CommandDto, CommandOptionChoiceDto, CommandOptionDto, CommandOptionType -def build_commands() -> list[CommandDto]: +def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> list[CommandDto]: + raid_choices = [ + CommandOptionChoiceDto(name=name[:100], value=value) + for name, value in (raid_filter_choices or []) + ][:25] + raid_option_choices = raid_choices or None return [ CommandDto( name="instance", @@ -56,7 +61,26 @@ def build_commands() -> list[CommandDto]: name="player", description="Destiny membership id (digits) or player name", required=True, - ) + ), + CommandOptionDto( + type=CommandOptionType.BOOLEAN, + name="require_fresh", + description="Only send fresh completions", + required=False, + ), + CommandOptionDto( + type=CommandOptionType.BOOLEAN, + name="require_completed", + description="Only send completed runs", + required=False, + ), + CommandOptionDto( + type=CommandOptionType.INTEGER, + name="raid", + description="Optional raid filter", + required=False, + choices=raid_option_choices, + ), ], ), CommandOptionDto( @@ -69,7 +93,26 @@ def build_commands() -> list[CommandDto]: name="clan", description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", required=True, - ) + ), + CommandOptionDto( + type=CommandOptionType.BOOLEAN, + name="require_fresh", + description="Only send fresh completions", + required=False, + ), + CommandOptionDto( + type=CommandOptionType.BOOLEAN, + name="require_completed", + description="Only send completed runs", + required=False, + ), + CommandOptionDto( + type=CommandOptionType.INTEGER, + name="raid", + description="Optional raid filter", + required=False, + choices=raid_option_choices, + ), ], ), ], @@ -95,34 +138,41 @@ def build_commands() -> list[CommandDto]: ), CommandDto( name="unsubscribe", - description="Remove the RaidHub subscription webhook from this channel.", - dm_permission=False, - options=[], - ), - CommandDto( - name="unsubscribe-player", - description="Remove one subscribed Destiny player from this channel (other rules unchanged).", + description="Remove player/clan rules or the webhook destination for this channel.", dm_permission=False, options=[ CommandOptionDto( - type=CommandOptionType.STRING, + type=CommandOptionType.SUB_COMMAND, + name="delete", + description="Remove the webhook destination and all rules for this channel.", + options=[], + ), + CommandOptionDto( + type=CommandOptionType.SUB_COMMAND, name="player", - description="Destiny membership id (digits) or player name", - required=True, - ) - ], - ), - CommandDto( - name="unsubscribe-clan", - description="Remove one subscribed Bungie clan from this channel (other rules unchanged).", - dm_permission=False, - options=[ + description="Remove one subscribed Destiny player from this channel.", + options=[ + CommandOptionDto( + type=CommandOptionType.STRING, + name="player", + description="Destiny membership id (digits) or player name", + required=True, + ) + ], + ), CommandOptionDto( - type=CommandOptionType.STRING, + type=CommandOptionType.SUB_COMMAND, name="clan", - description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", - required=True, - ) + description="Remove one subscribed Bungie clan from this channel.", + options=[ + CommandOptionDto( + type=CommandOptionType.STRING, + name="clan", + description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", + required=True, + ) + ], + ), ], ), ] diff --git a/src/manifest/schema.py b/src/manifest/schema.py index ae4b53d..814181b 100644 --- a/src/manifest/schema.py +++ b/src/manifest/schema.py @@ -18,6 +18,18 @@ class CommandOptionType(IntEnum): BOOLEAN = int(ApplicationCommandOptionType.BOOLEAN) +@dataclass(frozen=True, slots=True) +class CommandOptionChoiceDto: + name: str + value: str | int + + def to_json(self) -> dict[str, Any]: + return { + "name": self.name, + "value": self.value, + } + + @dataclass(frozen=True, slots=True) class CommandOptionDto: type: CommandOptionType @@ -25,6 +37,7 @@ class CommandOptionDto: description: str required: bool | None = None options: list["CommandOptionDto"] | None = None + choices: list[CommandOptionChoiceDto] | None = None def to_json(self) -> dict[str, Any]: data: dict[str, Any] = { @@ -34,6 +47,8 @@ def to_json(self) -> dict[str, Any]: } if self.required is not None: data["required"] = self.required + if self.choices is not None: + data["choices"] = [c.to_json() for c in self.choices] if self.options is not None: data["options"] = [o.to_json() for o in self.options] return data diff --git a/src/sync_commands.py b/src/sync_commands.py index 2427427..023ccab 100644 --- a/src/sync_commands.py +++ b/src/sync_commands.py @@ -3,6 +3,7 @@ import asyncio import json import sys +from typing import Any import httpx @@ -20,8 +21,33 @@ async def main() -> int: settings = get_settings() app_id = _required(settings.discord_application_id, "DISCORD_APPLICATION_ID") guild_id = (settings.discord_guild_id or "").strip() + raid_filter_choices: list[tuple[str, int]] = [] + manifest_url = f"{settings.raidhub_api_base_url.rstrip('/')}/manifest" + try: + async with httpx.AsyncClient(timeout=20) as client: + headers: dict[str, str] = {} + if settings.raidhub_api_key: + headers["x-api-key"] = settings.raidhub_api_key + resp = await client.get(manifest_url, headers=headers) + if resp.is_success: + env = resp.json() + inner = env.get("response") if isinstance(env, dict) else None + if isinstance(inner, dict): + raid_filter_choices = _extract_raid_filter_choices(inner) + else: + print( + f"Warning: could not fetch raid choices from {manifest_url} " + f"({resp.status_code}). Continuing without raid dropdown choices.", + file=sys.stderr, + ) + except Exception as err: + print( + f"Warning: failed to fetch raid choices from {manifest_url}: {err}. " + "Continuing without raid dropdown choices.", + file=sys.stderr, + ) - payload = build_command_manifest() + payload = build_command_manifest(raid_filter_choices=raid_filter_choices) if guild_id: endpoint = ( f"https://discord.com/api/v10/applications/{app_id}/guilds/{guild_id}/commands" @@ -69,5 +95,27 @@ def cli() -> int: return asyncio.run(main()) +def _extract_raid_filter_choices(manifest_response: dict[str, Any]) -> list[tuple[str, int]]: + listed_ids = manifest_response.get("listedRaidIds") or [] + activity_defs = manifest_response.get("activityDefinitions") or {} + if not isinstance(listed_ids, list) or not isinstance(activity_defs, dict): + return [] + out: list[tuple[str, int]] = [] + for rid in listed_ids: + key = str(rid) + row = activity_defs.get(key) + if not isinstance(row, dict): + continue + name = str(row.get("name") or "").strip() + if not name: + continue + if not isinstance(rid, int): + continue + out.append((name, rid)) + if len(out) >= 25: + break + return out + + if __name__ == "__main__": raise SystemExit(cli()) diff --git a/tests/test_manifest_commands.py b/tests/test_manifest_commands.py index 47f0d40..dd9ef6b 100644 --- a/tests/test_manifest_commands.py +++ b/tests/test_manifest_commands.py @@ -3,6 +3,7 @@ import unittest from src.manifest import build_command_manifest, build_commands +from src.manifest.schema import CommandOptionType class ManifestCommandsTests(unittest.TestCase): @@ -16,8 +17,6 @@ def test_build_commands_has_stable_slash_names(self) -> None: "subscribe", "subscription", "unsubscribe", - "unsubscribe-clan", - "unsubscribe-player", }, ) @@ -32,6 +31,33 @@ def test_subscribe_dm_permission_false(self) -> None: cmds = {c.name: c for c in build_commands()} self.assertIs(cmds["subscribe"].dm_permission, False) + def test_subscribe_subcommands_expose_rule_filter_options(self) -> None: + cmds = {c.name: c for c in build_commands()} + subscribe = cmds["subscribe"] + options = {o.name: o for o in (subscribe.options or [])} + player_opt_names = {o.name for o in (options["player"].options or [])} + clan_opt_names = {o.name for o in (options["clan"].options or [])} + self.assertEqual( + player_opt_names, {"player", "require_fresh", "require_completed", "raid"} + ) + self.assertEqual( + clan_opt_names, + {"clan", "require_fresh", "require_completed", "raid"}, + ) + player_raid = next( + o for o in (options["player"].options or []) if o.name == "raid" + ) + clan_raid = next(o for o in (options["clan"].options or []) if o.name == "raid") + self.assertEqual(player_raid.type, CommandOptionType.INTEGER) + self.assertEqual(clan_raid.type, CommandOptionType.INTEGER) + + def test_unsubscribe_has_player_and_clan_subcommands(self) -> None: + cmds = {c.name: c for c in build_commands()} + unsub = cmds["unsubscribe"] + self.assertIsNotNone(unsub.options) + option_names = {o.name for o in (unsub.options or [])} + self.assertEqual(option_names, {"delete", "player", "clan"}) + def test_manifest_json_serializable_shape(self) -> None: manifest = build_command_manifest() self.assertIsInstance(manifest, list) diff --git a/tests/test_subscription_helpers.py b/tests/test_subscription_helpers.py index beb56d2..570357b 100644 --- a/tests/test_subscription_helpers.py +++ b/tests/test_subscription_helpers.py @@ -85,7 +85,50 @@ def test_registered_minimal(self) -> None: ) fields = {f["name"]: f["value"] for f in out["embeds"][0]["fields"]} self.assertIn("Destination Active", fields) - self.assertIn("`123`", fields["Webhook ID"]) + self.assertIn("Delivery Failures", fields) + self.assertEqual(fields["Rule Filters"], "—") + + def test_registered_uses_clan_group_id_key(self) -> None: + out = asyncio.run( + format_subscription_status_embed( + None, + { + "registered": True, + "destinationActive": True, + "consecutiveDeliveryFailures": 0, + "clans": [{"clanGroupId": "4927161"}], + }, + ) + ) + fields = {f["name"]: f["value"] for f in out["embeds"][0]["fields"]} + self.assertIn("• `4927161`", fields["Clan Rules (1)"]) + self.assertIn("`raid:all`", fields["Clan Rules (1)"]) + self.assertIn("Require Fresh", fields["Rule Filters"]) + + def test_registered_rule_filters_summary(self) -> None: + out = asyncio.run( + format_subscription_status_embed( + None, + { + "registered": True, + "destinationActive": True, + "consecutiveDeliveryFailures": 0, + "players": [ + { + "membershipId": "4611686018488107374", + "requireFresh": True, + "requireCompleted": False, + } + ], + "clans": [{"groupId": "4927161", "requireFresh": True, "requireCompleted": False}], + }, + ) + ) + fields = {f["name"]: f["value"] for f in out["embeds"][0]["fields"]} + self.assertIn("**yes**", fields["Rule Filters"]) + self.assertIn("**no**", fields["Rule Filters"]) + self.assertIn("`fresh:yes`", fields["Player Rules (1)"]) + self.assertIn("`completed:no`", fields["Player Rules (1)"]) class SubscriptionRulesSuffixTests(unittest.TestCase): diff --git a/tests/test_sync_commands.py b/tests/test_sync_commands.py new file mode 100644 index 0000000..8894ecf --- /dev/null +++ b/tests/test_sync_commands.py @@ -0,0 +1,23 @@ +from __future__ import annotations + +import unittest + +from src.sync_commands import _extract_raid_filter_choices + + +class SyncCommandsTests(unittest.TestCase): + def test_extract_raid_filter_choices_uses_listed_raid_order(self) -> None: + out = _extract_raid_filter_choices( + { + "listedRaidIds": [9, 1], + "activityDefinitions": { + "1": {"name": "Leviathan", "path": "leviathan"}, + "9": {"name": "Vault of Glass", "path": "vaultofglass"}, + }, + } + ) + self.assertEqual(out, [("Vault of Glass", 9), ("Leviathan", 1)]) + + +if __name__ == "__main__": + unittest.main() From d321659589fb1ef89d0acd161a8516cf33c5cbbb Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:02:14 -0400 Subject: [PATCH 2/8] Fix unsubscribe metrics delegation and safe membership-id normalization. This avoids wrapper double-counting for delegated unsubscribe subcommands and prevents ValueError paths when membership ids are malformed in subscribe/unsubscribe player flows. Made-with: Cursor --- src/commands/subscribe.py | 8 +++++--- src/commands/unsubscribe.py | 33 ++++++++++++++++++--------------- 2 files changed, 23 insertions(+), 18 deletions(-) diff --git a/src/commands/subscribe.py b/src/commands/subscribe.py index a63defe..47489df 100644 --- a/src/commands/subscribe.py +++ b/src/commands/subscribe.py @@ -13,7 +13,6 @@ bungie_emblem_url, format_player_display_name, parse_clan_group_id, - resolve_player_membership_id, resolve_player_subscription_row, ) from .subscription_messages import ( @@ -136,7 +135,10 @@ async def run_subscribe_deferred( ) return raw_mid = prow.get("membershipId") - resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" + try: + resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" + except (TypeError, ValueError): + resolved_id = "" if not resolved_id or not resolved_id.isdigit(): await patch_discord_followup_best_effort( app_id, @@ -258,4 +260,4 @@ async def run_subscribe_deferred( observe_deferred_completion(command="subscribe", outcome=outcome) -__all__ = ["run_subscribe_deferred", "resolve_player_membership_id", "parse_clan_group_id"] +__all__ = ["run_subscribe_deferred", "parse_clan_group_id"] diff --git a/src/commands/unsubscribe.py b/src/commands/unsubscribe.py index f4c98eb..f082194 100644 --- a/src/commands/unsubscribe.py +++ b/src/commands/unsubscribe.py @@ -50,22 +50,22 @@ async def run_unsubscribe_deferred( ) -> None: app_id = application_id(interaction, settings) token = str(interaction.get("token") or "") + data = interaction.get("data") or {} + top_opts = data.get("options") or [] + sub = "" + if top_opts and isinstance(top_opts[0], dict): + sub = str(top_opts[0].get("name") or "").strip().lower() + if not sub: + sub = "delete" + if sub == "player": + await run_unsubscribe_player_deferred(interaction, raidhub, settings) + return + if sub == "clan": + await run_unsubscribe_clan_deferred(interaction, raidhub, settings) + return + outcome = "completed" try: - data = interaction.get("data") or {} - top_opts = data.get("options") or [] - sub = "" - if top_opts and isinstance(top_opts[0], dict): - sub = str(top_opts[0].get("name") or "").strip().lower() - if not sub: - sub = "delete" - - if sub == "player": - await run_unsubscribe_player_deferred(interaction, raidhub, settings) - return - if sub == "clan": - await run_unsubscribe_clan_deferred(interaction, raidhub, settings) - return if sub != "delete": await patch_discord_followup_best_effort( app_id, @@ -172,7 +172,10 @@ async def run_unsubscribe_player_deferred( ) return raw_mid = prow.get("membershipId") - resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" + try: + resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" + except (TypeError, ValueError): + resolved_id = "" if not resolved_id or not resolved_id.isdigit(): await patch_discord_followup_best_effort( app_id, From 65c9bc8b64b99ba845ccff1f1794373c80c70acc Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:11:19 -0400 Subject: [PATCH 3/8] Simplify unsubscribe command surface and remove subscription delete mode. This keeps `/unsubscribe` as the full remove-all command, restores dedicated `/unsubscribe-player` and `/unsubscribe-clan` commands, and limits `/subscription` to status-only behavior. Made-with: Cursor --- src/commands/subscription.py | 56 +++++++-------------------- src/commands/unsubscribe.py | 24 ------------ src/manifest/builders.py | 68 ++++++++++++--------------------- tests/test_manifest_commands.py | 8 ++-- 4 files changed, 41 insertions(+), 115 deletions(-) diff --git a/src/commands/subscription.py b/src/commands/subscription.py index f324c3f..7819ccb 100644 --- a/src/commands/subscription.py +++ b/src/commands/subscription.py @@ -7,21 +7,19 @@ from ..raidhub_client import RaidHubClient, discord_invocation_context from .subscription_messages import ( SUBSCRIPTION_COMMAND_TITLE, - SUBSCRIPTION_REMOVED_TITLE, SUBSCRIPTION_REQUEST_FAILED_TITLE, ) from .subscription_helpers import ( format_subscription_status_embed, subscription_envelope_error_message, ) -from .subscription_routes import SUB_ROUTE_DELETE, SUB_ROUTE_STATUS +from .subscription_routes import SUB_ROUTE_STATUS from .shared import ( USER_FACING_GENERIC, application_id, error_embed, patch_discord_followup_best_effort, report_deferred_exception, - success_embed, warn_embed, ) @@ -37,26 +35,14 @@ async def run_subscription_deferred( try: data = interaction.get("data") or {} top_opts = data.get("options") or [] - if not top_opts or not isinstance(top_opts[0], dict): + sub = "status" + if top_opts and isinstance(top_opts[0], dict): + sub = str(top_opts[0].get("name") or "").strip().lower() or "status" + if sub != "status": await patch_discord_followup_best_effort( app_id, token, - warn_embed( - SUBSCRIPTION_COMMAND_TITLE, - "Pick `status` or `delete` under `/subscription`.", - ), - ) - return - - sub = str(top_opts[0].get("name") or "").strip().lower() - if sub not in ("delete", "status"): - await patch_discord_followup_best_effort( - app_id, - token, - warn_embed( - SUBSCRIPTION_COMMAND_TITLE, - "Unknown `/subscription` subcommand.", - ), + warn_embed(SUBSCRIPTION_COMMAND_TITLE, "Use `/subscription` to view status."), ) return @@ -71,21 +57,12 @@ async def run_subscription_deferred( ) return - route_id = {"delete": SUB_ROUTE_DELETE, "status": SUB_ROUTE_STATUS}[sub] - ctx = discord_invocation_context(interaction, route_id=route_id) - - if sub == "status": - env = await raidhub.request_envelope( - "GET", - "/subscriptions/discord/webhooks", - discord_context=ctx, - ) - else: - env = await raidhub.request_envelope( - "DELETE", - "/subscriptions/discord/webhooks", - discord_context=ctx, - ) + ctx = discord_invocation_context(interaction, route_id=SUB_ROUTE_STATUS) + env = await raidhub.request_envelope( + "GET", + "/subscriptions/discord/webhooks", + discord_context=ctx, + ) if not env.get("success"): await patch_discord_followup_best_effort( @@ -99,14 +76,7 @@ async def run_subscription_deferred( return inner = env.get("response") or {} - if sub == "status": - msg = await format_subscription_status_embed(raidhub, inner) - else: - msg = success_embed( - SUBSCRIPTION_REMOVED_TITLE, - "RaidHub will no longer use a webhook in this channel.", - ) - + msg = await format_subscription_status_embed(raidhub, inner) await patch_discord_followup_best_effort(app_id, token, msg) except Exception as err: outcome = "error" diff --git a/src/commands/unsubscribe.py b/src/commands/unsubscribe.py index f082194..66345df 100644 --- a/src/commands/unsubscribe.py +++ b/src/commands/unsubscribe.py @@ -50,33 +50,9 @@ async def run_unsubscribe_deferred( ) -> None: app_id = application_id(interaction, settings) token = str(interaction.get("token") or "") - data = interaction.get("data") or {} - top_opts = data.get("options") or [] - sub = "" - if top_opts and isinstance(top_opts[0], dict): - sub = str(top_opts[0].get("name") or "").strip().lower() - if not sub: - sub = "delete" - if sub == "player": - await run_unsubscribe_player_deferred(interaction, raidhub, settings) - return - if sub == "clan": - await run_unsubscribe_clan_deferred(interaction, raidhub, settings) - return outcome = "completed" try: - if sub != "delete": - await patch_discord_followup_best_effort( - app_id, - token, - warn_embed( - UNSUBSCRIBE_COMMAND_TITLE, - "Use `/unsubscribe delete`, `/unsubscribe player`, or `/unsubscribe clan`.", - ), - ) - return - if not interaction.get("guild_id") or not interaction.get("channel_id"): await patch_discord_followup_best_effort( app_id, diff --git a/src/manifest/builders.py b/src/manifest/builders.py index a127de1..7c8971f 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -119,60 +119,40 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> ), CommandDto( name="subscription", - description="Inspect or remove the RaidHub subscription webhook for this channel.", + description="Inspect the RaidHub subscription webhook for this channel.", dm_permission=False, - options=[ - CommandOptionDto( - type=CommandOptionType.SUB_COMMAND, - name="status", - description="Show whether this channel is registered and delivery health.", - options=[], - ), - CommandOptionDto( - type=CommandOptionType.SUB_COMMAND, - name="delete", - description="Remove the webhook destination and rules for this channel.", - options=[], - ), - ], + options=[], ), CommandDto( name="unsubscribe", - description="Remove player/clan rules or the webhook destination for this channel.", + description="Remove the RaidHub webhook destination and all rules for this channel.", + dm_permission=False, + options=[], + ), + CommandDto( + name="unsubscribe-player", + description="Remove one subscribed Destiny player from this channel.", dm_permission=False, options=[ CommandOptionDto( - type=CommandOptionType.SUB_COMMAND, - name="delete", - description="Remove the webhook destination and all rules for this channel.", - options=[], - ), - CommandOptionDto( - type=CommandOptionType.SUB_COMMAND, + type=CommandOptionType.STRING, name="player", - description="Remove one subscribed Destiny player from this channel.", - options=[ - CommandOptionDto( - type=CommandOptionType.STRING, - name="player", - description="Destiny membership id (digits) or player name", - required=True, - ) - ], - ), + description="Destiny membership id (digits) or player name", + required=True, + ) + ], + ), + CommandDto( + name="unsubscribe-clan", + description="Remove one subscribed Bungie clan from this channel.", + dm_permission=False, + options=[ CommandOptionDto( - type=CommandOptionType.SUB_COMMAND, + type=CommandOptionType.STRING, name="clan", - description="Remove one subscribed Bungie clan from this channel.", - options=[ - CommandOptionDto( - type=CommandOptionType.STRING, - name="clan", - description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", - required=True, - ) - ], - ), + description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", + required=True, + ) ], ), ] diff --git a/tests/test_manifest_commands.py b/tests/test_manifest_commands.py index dd9ef6b..5f2faa0 100644 --- a/tests/test_manifest_commands.py +++ b/tests/test_manifest_commands.py @@ -17,6 +17,8 @@ def test_build_commands_has_stable_slash_names(self) -> None: "subscribe", "subscription", "unsubscribe", + "unsubscribe-player", + "unsubscribe-clan", }, ) @@ -51,12 +53,10 @@ def test_subscribe_subcommands_expose_rule_filter_options(self) -> None: self.assertEqual(player_raid.type, CommandOptionType.INTEGER) self.assertEqual(clan_raid.type, CommandOptionType.INTEGER) - def test_unsubscribe_has_player_and_clan_subcommands(self) -> None: + def test_unsubscribe_is_simple_all_command(self) -> None: cmds = {c.name: c for c in build_commands()} unsub = cmds["unsubscribe"] - self.assertIsNotNone(unsub.options) - option_names = {o.name for o in (unsub.options or [])} - self.assertEqual(option_names, {"delete", "player", "clan"}) + self.assertEqual(unsub.options, []) def test_manifest_json_serializable_shape(self) -> None: manifest = build_command_manifest() From 488edc85bfa48e77e1c465daa7daf154a24f34c3 Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:15:57 -0400 Subject: [PATCH 4/8] Refine unsubscribe command shape and user-facing subscription copy. This switches `/unsubscribe` to all/player/clan subcommands and updates subscription status/unsubscribe wording to alert-focused language without internal webhook terminology. Made-with: Cursor --- src/app_factory.py | 4 --- src/commands/subscription_helpers.py | 4 +-- src/commands/unsubscribe.py | 27 +++++++++++++-- src/manifest/builders.py | 51 ++++++++++++++++------------ tests/test_manifest_commands.py | 8 ++--- 5 files changed, 60 insertions(+), 34 deletions(-) diff --git a/src/app_factory.py b/src/app_factory.py index 9eea212..1dc5487 100644 --- a/src/app_factory.py +++ b/src/app_factory.py @@ -16,9 +16,7 @@ run_player_search_deferred, run_subscribe_deferred, run_subscription_deferred, - run_unsubscribe_clan_deferred, run_unsubscribe_deferred, - run_unsubscribe_player_deferred, ) from .config import Settings, get_settings from .discord_auth import verify_discord_signature_with_reason @@ -48,8 +46,6 @@ def create_app() -> FastAPI: "subscribe": run_subscribe_deferred, "subscription": run_subscription_deferred, "unsubscribe": run_unsubscribe_deferred, - "unsubscribe-player": run_unsubscribe_player_deferred, - "unsubscribe-clan": run_unsubscribe_clan_deferred, } def _msg(content: str) -> dict[str, Any]: diff --git a/src/commands/subscription_helpers.py b/src/commands/subscription_helpers.py index fd595b1..eba0395 100644 --- a/src/commands/subscription_helpers.py +++ b/src/commands/subscription_helpers.py @@ -261,7 +261,7 @@ async def format_subscription_status_embed( if not data.get("registered"): return info_embed( "Subscription Status", - "No RaidHub subscription webhook is registered for this channel.", + "RaidHub alerts are currently turned off for this channel.", ) active = "**yes**" if data.get("destinationActive") else "**no**" fails = int(data.get("consecutiveDeliveryFailures") or 0) @@ -357,7 +357,7 @@ async def format_subscription_status_embed( fields.append({"name": f"Player Rules ({pc})", "value": p_body, "inline": False}) fields.append({"name": f"Clan Rules ({cc})", "value": c_body, "inline": False}) - desc = "Current webhook destination and delivery health." + desc = "Current alert delivery health and active rules." if raidhub is not None and (pc + cc) > 1: desc = ( f"{desc}\n\nShowing **{pc}** player(s) and **{cc}** clan(s) by name below " diff --git a/src/commands/unsubscribe.py b/src/commands/unsubscribe.py index 66345df..0a05bdc 100644 --- a/src/commands/unsubscribe.py +++ b/src/commands/unsubscribe.py @@ -50,16 +50,39 @@ async def run_unsubscribe_deferred( ) -> None: app_id = application_id(interaction, settings) token = str(interaction.get("token") or "") + data = interaction.get("data") or {} + top_opts = data.get("options") or [] + sub = "" + if top_opts and isinstance(top_opts[0], dict): + sub = str(top_opts[0].get("name") or "").strip().lower() + if not sub: + sub = "all" + if sub == "player": + await run_unsubscribe_player_deferred(interaction, raidhub, settings) + return + if sub == "clan": + await run_unsubscribe_clan_deferred(interaction, raidhub, settings) + return outcome = "completed" try: + if sub != "all": + await patch_discord_followup_best_effort( + app_id, + token, + warn_embed( + UNSUBSCRIBE_COMMAND_TITLE, + "Use `/unsubscribe all`, `/unsubscribe player`, or `/unsubscribe clan`.", + ), + ) + return if not interaction.get("guild_id") or not interaction.get("channel_id"): await patch_discord_followup_best_effort( app_id, token, warn_embed( UNSUBSCRIBE_COMMAND_TITLE, - "Run `/unsubscribe` in a server text channel, not a DM.", + "Run `/unsubscribe all` in a server text channel, not a DM.", ), ) return @@ -86,7 +109,7 @@ async def run_unsubscribe_deferred( token, success_embed( SUBSCRIPTION_REMOVED_TITLE, - "RaidHub will no longer use a webhook in this channel.", + "RaidHub alerts are now turned off for this channel.", ), ) except Exception as err: diff --git a/src/manifest/builders.py b/src/manifest/builders.py index 7c8971f..24c4fa7 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -119,39 +119,46 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> ), CommandDto( name="subscription", - description="Inspect the RaidHub subscription webhook for this channel.", + description="View this channel's RaidHub subscription status and rule health.", dm_permission=False, options=[], ), CommandDto( name="unsubscribe", - description="Remove the RaidHub webhook destination and all rules for this channel.", - dm_permission=False, - options=[], - ), - CommandDto( - name="unsubscribe-player", - description="Remove one subscribed Destiny player from this channel.", + description="Turn off all RaidHub alerts for this channel.", dm_permission=False, options=[ CommandOptionDto( - type=CommandOptionType.STRING, + type=CommandOptionType.SUB_COMMAND, + name="all", + description="Turn off all RaidHub alerts for this channel.", + options=[], + ), + CommandOptionDto( + type=CommandOptionType.SUB_COMMAND, name="player", - description="Destiny membership id (digits) or player name", - required=True, - ) - ], - ), - CommandDto( - name="unsubscribe-clan", - description="Remove one subscribed Bungie clan from this channel.", - dm_permission=False, - options=[ + description="Remove one subscribed player from this channel.", + options=[ + CommandOptionDto( + type=CommandOptionType.STRING, + name="player", + description="Destiny membership id (digits) or player name", + required=True, + ) + ], + ), CommandOptionDto( - type=CommandOptionType.STRING, + type=CommandOptionType.SUB_COMMAND, name="clan", - description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", - required=True, + description="Remove one subscribed clan from this channel.", + options=[ + CommandOptionDto( + type=CommandOptionType.STRING, + name="clan", + description="Numeric group id, raidhub.io/clan/…, or Bungie clan URL", + required=True, + ) + ], ) ], ), diff --git a/tests/test_manifest_commands.py b/tests/test_manifest_commands.py index 5f2faa0..2b976f4 100644 --- a/tests/test_manifest_commands.py +++ b/tests/test_manifest_commands.py @@ -17,8 +17,6 @@ def test_build_commands_has_stable_slash_names(self) -> None: "subscribe", "subscription", "unsubscribe", - "unsubscribe-player", - "unsubscribe-clan", }, ) @@ -53,10 +51,12 @@ def test_subscribe_subcommands_expose_rule_filter_options(self) -> None: self.assertEqual(player_raid.type, CommandOptionType.INTEGER) self.assertEqual(clan_raid.type, CommandOptionType.INTEGER) - def test_unsubscribe_is_simple_all_command(self) -> None: + def test_unsubscribe_has_all_player_clan_subcommands(self) -> None: cmds = {c.name: c for c in build_commands()} unsub = cmds["unsubscribe"] - self.assertEqual(unsub.options, []) + self.assertIsNotNone(unsub.options) + option_names = {o.name for o in (unsub.options or [])} + self.assertEqual(option_names, {"all", "player", "clan"}) def test_manifest_json_serializable_shape(self) -> None: manifest = build_command_manifest() From cb66bda769e62642ad745de92cfff9c30cff2ae2 Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:50:02 -0400 Subject: [PATCH 5/8] update: defer raid filter input until better slash-command UX Remove temporary raid option wiring from subscribe commands, keep display compatibility for raidIds, and leave explicit TODOs for a future multi-select-friendly design. Made-with: Cursor --- src/commands/subscribe.py | 5 +---- src/commands/subscription_helpers.py | 26 +++++++++++++++++++------- src/manifest/builders.py | 19 ++++--------------- 3 files changed, 24 insertions(+), 26 deletions(-) diff --git a/src/commands/subscribe.py b/src/commands/subscribe.py index 47489df..892019c 100644 --- a/src/commands/subscribe.py +++ b/src/commands/subscribe.py @@ -81,10 +81,7 @@ async def run_subscribe_deferred( filters["requireFresh"] = bool(leaf["require_fresh"]) if "require_completed" in leaf: filters["requireCompleted"] = bool(leaf["require_completed"]) - if "raid" in leaf: - raw_raid = leaf.get("raid") - if isinstance(raw_raid, int): - filters["raid"] = raw_raid + # TODO: Add raid filter input once we have a solid multi-select UX. if not target_raw: await patch_discord_followup_best_effort( app_id, diff --git a/src/commands/subscription_helpers.py b/src/commands/subscription_helpers.py index eba0395..7faabdf 100644 --- a/src/commands/subscription_helpers.py +++ b/src/commands/subscription_helpers.py @@ -161,14 +161,26 @@ async def _fetch_clan_basic_card(raidhub: RaidHubClient, group_id: str) -> dict[ def _standardized_rule_string(rule: dict[str, Any]) -> str: fresh = "yes" if bool(rule.get("requireFresh")) else "no" completed = "yes" if bool(rule.get("requireCompleted")) else "no" - raid_id = rule.get("raidId") - raid_path = rule.get("raidPath") - if raid_id is not None and str(raid_id).strip() != "": - raid = f"raid:{raid_id}" - elif raid_path: - raid = f"raid:{str(raid_path).strip()}" + raid_ids_raw = rule.get("raidIds") + raid_ids: list[str] = [] + if isinstance(raid_ids_raw, list): + for value in raid_ids_raw: + s = str(value).strip() + if s.isdigit(): + raid_ids.append(str(int(s))) + + # Backward compatibility for old API payloads. + if not raid_ids: + legacy_raid_id = rule.get("raidId") + if legacy_raid_id is not None and str(legacy_raid_id).strip() != "": + s = str(legacy_raid_id).strip() + if s.isdigit(): + raid_ids = [str(int(s))] + + if raid_ids: + raid = f"raids:{','.join(raid_ids)}" else: - raid = "raid:all" + raid = "raids:all" return f"`fresh:{fresh}` `completed:{completed}` `{raid}`" diff --git a/src/manifest/builders.py b/src/manifest/builders.py index 24c4fa7..ce4192c 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -8,7 +8,6 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> CommandOptionChoiceDto(name=name[:100], value=value) for name, value in (raid_filter_choices or []) ][:25] - raid_option_choices = raid_choices or None return [ CommandDto( name="instance", @@ -74,13 +73,8 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> description="Only send completed runs", required=False, ), - CommandOptionDto( - type=CommandOptionType.INTEGER, - name="raid", - description="Optional raid filter", - required=False, - choices=raid_option_choices, - ), + # TODO: Reintroduce raid filtering after finalizing a user-friendly + # multi-select UX for Discord slash commands. ], ), CommandOptionDto( @@ -106,13 +100,8 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> description="Only send completed runs", required=False, ), - CommandOptionDto( - type=CommandOptionType.INTEGER, - name="raid", - description="Optional raid filter", - required=False, - choices=raid_option_choices, - ), + # TODO: Reintroduce raid filtering after finalizing a user-friendly + # multi-select UX for Discord slash commands. ], ), ], From 8c21ca0cc525513d0062b1ec05da32c6eeea7d95 Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:54:24 -0400 Subject: [PATCH 6/8] fix: remove dormant raid-choice wiring from command manifest Drop unused raid choice construction now that raid filter inputs are deferred, while keeping status formatting compatible with raidIds payloads. Made-with: Cursor --- src/commands/subscription_helpers.py | 33 ++++++++++++++++------------ src/manifest/builders.py | 6 +---- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/commands/subscription_helpers.py b/src/commands/subscription_helpers.py index 7faabdf..6c6adb4 100644 --- a/src/commands/subscription_helpers.py +++ b/src/commands/subscription_helpers.py @@ -60,9 +60,7 @@ async def fetch_subscription_status_envelope( def build_subscription_json_body(leaf_opts: dict[str, Any]) -> dict[str, Any]: body: dict[str, Any] = {} wn = str( - leaf_opts.get("discord_webhook_name") - or leaf_opts.get("webhook_name") - or "" + leaf_opts.get("discord_webhook_name") or leaf_opts.get("webhook_name") or "" ).strip() if wn: body["name"] = wn[:80] @@ -146,13 +144,17 @@ def _ordered_group_ids(cl_raw: list[Any]) -> list[str]: return out -async def _fetch_player_basic_card(raidhub: RaidHubClient, membership_id: str) -> dict[str, Any]: +async def _fetch_player_basic_card( + raidhub: RaidHubClient, membership_id: str +) -> dict[str, Any]: env = await raidhub.request_envelope("GET", f"/player/{membership_id}/basic") inner = env.get("response") return inner if env.get("success") and isinstance(inner, dict) else {} -async def _fetch_clan_basic_card(raidhub: RaidHubClient, group_id: str) -> dict[str, Any]: +async def _fetch_clan_basic_card( + raidhub: RaidHubClient, group_id: str +) -> dict[str, Any]: env = await raidhub.request_envelope("GET", f"/clan/{group_id}/basic") inner = env.get("response") return inner if env.get("success") and isinstance(inner, dict) else {} @@ -184,7 +186,9 @@ def _standardized_rule_string(rule: dict[str, Any]) -> str: return f"`fresh:{fresh}` `completed:{completed}` `{raid}`" -def _player_rule_line(membership_id: str, card: dict[str, Any], rule: dict[str, Any]) -> str: +def _player_rule_line( + membership_id: str, card: dict[str, Any], rule: dict[str, Any] +) -> str: rule_suffix = _standardized_rule_string(rule) if card: label = _embed_safe_label(format_player_display_name(card)) @@ -260,10 +264,9 @@ def _rule_filters_summary(pl_raw: list[Any], cl_raw: list[Any]) -> str: return "—" fresh = _rule_filter_state(all_rules, "requireFresh") completed = _rule_filter_state(all_rules, "requireCompleted") - return ( - f"Require Fresh: **{fresh}**\n" - f"Require Completed: **{completed}**" - )[:1024] + return (f"Require Fresh: **{fresh}**\n" f"Require Completed: **{completed}**")[ + :1024 + ] async def format_subscription_status_embed( @@ -321,12 +324,16 @@ async def format_subscription_status_embed( clan_cards: dict[str, dict[str, Any]] = {} if raidhub is not None and (pl_ids or cl_ids): p_res = ( - await asyncio.gather(*[_fetch_player_basic_card(raidhub, mid) for mid in pl_ids]) + await asyncio.gather( + *[_fetch_player_basic_card(raidhub, mid) for mid in pl_ids] + ) if pl_ids else [] ) c_res = ( - await asyncio.gather(*[_fetch_clan_basic_card(raidhub, gid) for gid in cl_ids]) + await asyncio.gather( + *[_fetch_clan_basic_card(raidhub, gid) for gid in cl_ids] + ) if cl_ids else [] ) @@ -406,5 +413,3 @@ def _comma_separated_digit_ids(raw: str) -> list[str]: if s and re.fullmatch(r"\d+", s): out.append(s) return out - - diff --git a/src/manifest/builders.py b/src/manifest/builders.py index ce4192c..d239a71 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -1,13 +1,9 @@ from __future__ import annotations -from .schema import CommandDto, CommandOptionChoiceDto, CommandOptionDto, CommandOptionType +from .schema import CommandDto, CommandOptionDto, CommandOptionType def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> list[CommandDto]: - raid_choices = [ - CommandOptionChoiceDto(name=name[:100], value=value) - for name, value in (raid_filter_choices or []) - ][:25] return [ CommandDto( name="instance", From d9eff711f3775df5818554a691800d97c8f724ea Mon Sep 17 00:00:00 2001 From: owen Date: Thu, 23 Apr 2026 23:55:27 -0400 Subject: [PATCH 7/8] fix: normalize unsubscribe player membership ids via resolver Reuse shared membership-id normalization for unsubscribe player flow so invalid or malformed IDs follow expected user-facing handling instead of exception fallback. Made-with: Cursor --- src/commands/unsubscribe.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/commands/unsubscribe.py b/src/commands/unsubscribe.py index 0a05bdc..c1bdc2e 100644 --- a/src/commands/unsubscribe.py +++ b/src/commands/unsubscribe.py @@ -9,6 +9,7 @@ bungie_emblem_url, format_player_display_name, parse_clan_group_id, + resolve_player_membership_id, resolve_player_subscription_row, ) from .subscription_messages import ( @@ -170,11 +171,7 @@ async def run_unsubscribe_player_deferred( ), ) return - raw_mid = prow.get("membershipId") - try: - resolved_id = str(int(str(raw_mid).strip())) if raw_mid is not None else "" - except (TypeError, ValueError): - resolved_id = "" + resolved_id = await resolve_player_membership_id(raidhub, target_raw) or "" if not resolved_id or not resolved_id.isdigit(): await patch_discord_followup_best_effort( app_id, From 0a4cb4d1dce60e4b43bc977ea47174c67e74cfa0 Mon Sep 17 00:00:00 2001 From: owen Date: Fri, 24 Apr 2026 00:00:06 -0400 Subject: [PATCH 8/8] update: simplify slash command surface for search flow Rename the player search command to search and remove the instance command from the manifest to reduce command clutter. Made-with: Cursor --- src/manifest/builders.py | 14 +------------- 1 file changed, 1 insertion(+), 13 deletions(-) diff --git a/src/manifest/builders.py b/src/manifest/builders.py index d239a71..6551d28 100644 --- a/src/manifest/builders.py +++ b/src/manifest/builders.py @@ -6,19 +6,7 @@ def build_commands(raid_filter_choices: list[tuple[str, int]] | None = None) -> list[CommandDto]: return [ CommandDto( - name="instance", - description="Lookup a RaidHub instance by id.", - options=[ - CommandOptionDto( - type=CommandOptionType.STRING, - name="raid_instance_id", - description="Instance id to lookup", - required=False, - ) - ], - ), - CommandDto( - name="player-search", + name="search", description="Search RaidHub players by Bungie name or platform name.", options=[ CommandOptionDto(