fix: Address PR review findings — two bugs and cleanup
- Fix int_timestamp() no-arg path returning seconds instead of
milliseconds, which would silently break the daily scout token cap
against the real API
- Acknowledge double-click interactions with ephemeral message instead
of silently returning (Discord requires all interactions to be acked)
- Reorder scout flow: create card copy before consuming token so a
failure doesn't cost the player a token for nothing
- Move build_scouted_card_list import to top of scout_view.py
- Remove unused asyncio import from helpers/scouting.py
- Fix footer text inconsistency ("One scout per player" everywhere)
- Update tests for new operation order and double-click behavior
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
This commit is contained in:
Cal Corum
parent
08c89d7ad2
commit
c39d8d173b
@ -13,7 +13,11 @@ import discord
|
||||
|
||||
from api_calls import db_get, db_post
|
||||
from helpers.main import get_team_by_owner, get_card_embeds
|
||||
from helpers.scouting import SCOUT_TOKENS_PER_DAY, get_scout_tokens_used
|
||||
from helpers.scouting import (
|
||||
SCOUT_TOKENS_PER_DAY,
|
||||
build_scouted_card_list,
|
||||
get_scout_tokens_used,
|
||||
)
|
||||
from helpers.utils import int_timestamp
|
||||
from helpers.discord_utils import get_team_embed
|
||||
from helpers.constants import IMAGES, PD_SEASON
|
||||
@ -72,8 +76,6 @@ class ScoutView(discord.ui.View):
|
||||
if not self.message:
|
||||
return
|
||||
|
||||
from helpers.scouting import build_scouted_card_list
|
||||
|
||||
card_list = build_scouted_card_list(self.card_lines, self.claims)
|
||||
|
||||
title = f"Scout Opportunity! ({self.total_scouts} scouted)"
|
||||
@ -163,6 +165,10 @@ class ScoutButton(discord.ui.Button):
|
||||
|
||||
# Prevent double-click race for same user
|
||||
if interaction.user.id in view.processing_users:
|
||||
await interaction.response.send_message(
|
||||
"Your scout is already being processed!",
|
||||
ephemeral=True,
|
||||
)
|
||||
return
|
||||
|
||||
view.processing_users.add(interaction.user.id)
|
||||
@ -206,6 +212,20 @@ class ScoutButton(discord.ui.Button):
|
||||
)
|
||||
return
|
||||
|
||||
# Create a copy of the card for the scouter (before consuming token
|
||||
# so a failure here doesn't cost the player a token for nothing)
|
||||
await db_post(
|
||||
"cards",
|
||||
payload={
|
||||
"cards": [
|
||||
{
|
||||
"player_id": self.card["player"]["player_id"],
|
||||
"team_id": scouter_team["id"],
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
# Consume a scout token
|
||||
current = await db_get("current")
|
||||
await db_post(
|
||||
@ -219,19 +239,6 @@ class ScoutButton(discord.ui.Button):
|
||||
},
|
||||
)
|
||||
|
||||
# Create a copy of the card for the scouter
|
||||
await db_post(
|
||||
"cards",
|
||||
payload={
|
||||
"cards": [
|
||||
{
|
||||
"player_id": self.card["player"]["player_id"],
|
||||
"team_id": scouter_team["id"],
|
||||
}
|
||||
],
|
||||
},
|
||||
)
|
||||
|
||||
# Track the claim
|
||||
player_id = self.card["player"]["player_id"]
|
||||
if player_id not in view.claims:
|
||||
|
||||
@ -5,7 +5,6 @@ Handles creation of scout opportunities after pack openings
|
||||
and embed formatting for the scouting feature.
|
||||
"""
|
||||
|
||||
import asyncio
|
||||
import datetime
|
||||
import logging
|
||||
import random
|
||||
@ -95,7 +94,7 @@ def build_scout_embed(
|
||||
f"{time_line}"
|
||||
)
|
||||
embed.set_footer(
|
||||
text=f"Paper Dynasty Season {PD_SEASON} \u2022 One player per pack",
|
||||
text=f"Paper Dynasty Season {PD_SEASON} \u2022 One scout per player",
|
||||
icon_url=IMAGES["logo"],
|
||||
)
|
||||
return embed, card_lines
|
||||
|
||||
@ -11,10 +11,13 @@ import discord
|
||||
|
||||
|
||||
def int_timestamp(datetime_obj: Optional[datetime.datetime] = None):
|
||||
"""Convert current datetime to integer timestamp."""
|
||||
if datetime_obj:
|
||||
return int(datetime.datetime.timestamp(datetime_obj) * 1000)
|
||||
return int(datetime.datetime.now().timestamp())
|
||||
"""Convert a datetime to an integer millisecond timestamp.
|
||||
|
||||
If no argument is given, uses the current time.
|
||||
"""
|
||||
if datetime_obj is None:
|
||||
datetime_obj = datetime.datetime.now()
|
||||
return int(datetime.datetime.timestamp(datetime_obj) * 1000)
|
||||
|
||||
|
||||
def midnight_timestamp() -> int:
|
||||
|
||||
@ -162,19 +162,26 @@ class TestScoutButtonGuards:
|
||||
async def test_double_click_silently_ignored(
|
||||
self, sample_cards, opener_team, mock_bot
|
||||
):
|
||||
"""If a user is already being processed, the click should be silently dropped."""
|
||||
"""If a user is already being processed, they should get an ephemeral rejection."""
|
||||
view = self._make_view(sample_cards, opener_team, mock_bot)
|
||||
view.processing_users.add(12345)
|
||||
button = view.children[0]
|
||||
|
||||
interaction = AsyncMock(spec=discord.Interaction)
|
||||
interaction.response = AsyncMock()
|
||||
interaction.response.send_message = AsyncMock()
|
||||
interaction.user = Mock()
|
||||
interaction.user.id = 12345
|
||||
|
||||
await button.callback(interaction)
|
||||
|
||||
# Should not have called defer or send_message
|
||||
interaction.response.defer.assert_not_called()
|
||||
interaction.response.send_message.assert_called_once()
|
||||
call_kwargs = interaction.response.send_message.call_args[1]
|
||||
assert call_kwargs["ephemeral"] is True
|
||||
assert (
|
||||
"already being processed"
|
||||
in interaction.response.send_message.call_args[0][0].lower()
|
||||
)
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
@ -242,22 +249,22 @@ class TestScoutButtonSuccess:
|
||||
# Should have deferred
|
||||
interaction.response.defer.assert_called_once_with(ephemeral=True)
|
||||
|
||||
# db_post should be called 3 times: scout_claims, rewards, cards
|
||||
# db_post should be called 3 times: scout_claims, cards, rewards
|
||||
assert mock_db_post.call_count == 3
|
||||
|
||||
# Verify scout_claims POST
|
||||
claim_call = mock_db_post.call_args_list[0]
|
||||
assert claim_call[0][0] == "scout_claims"
|
||||
|
||||
# Verify rewards POST (token consumption)
|
||||
reward_call = mock_db_post.call_args_list[1]
|
||||
# Verify cards POST (card copy — created before token consumption)
|
||||
card_call = mock_db_post.call_args_list[1]
|
||||
assert card_call[0][0] == "cards"
|
||||
|
||||
# Verify rewards POST (token consumption — after card is safely created)
|
||||
reward_call = mock_db_post.call_args_list[2]
|
||||
assert reward_call[0][0] == "rewards"
|
||||
assert reward_call[1]["payload"]["name"] == "Scout Token"
|
||||
|
||||
# Verify cards POST (card copy)
|
||||
card_call = mock_db_post.call_args_list[2]
|
||||
assert card_call[0][0] == "cards"
|
||||
|
||||
# User should be marked as scouted
|
||||
assert 12345 in view.scouted_users
|
||||
assert view.total_scouts == 1
|
||||
@ -797,9 +804,11 @@ class TestCurrentSeasonFallback:
|
||||
assert 12345 in view.scouted_users
|
||||
|
||||
# Verify the rewards POST used fallback values
|
||||
# Order: scout_claims (0), cards (1), rewards (2)
|
||||
from helpers.constants import PD_SEASON
|
||||
|
||||
reward_call = mock_db_post.call_args_list[1]
|
||||
reward_call = mock_db_post.call_args_list[2]
|
||||
assert reward_call[0][0] == "rewards"
|
||||
assert reward_call[1]["payload"]["season"] == PD_SEASON
|
||||
assert reward_call[1]["payload"]["week"] == 1
|
||||
|
||||
|
||||
Loading…
Reference in New Issue
Block a user