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:
parent
3c0fa133fd
commit
755f74be92
@ -13,7 +13,11 @@ import discord
|
|||||||
|
|
||||||
from api_calls import db_get, db_post
|
from api_calls import db_get, db_post
|
||||||
from helpers.main import get_team_by_owner, get_card_embeds
|
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.utils import int_timestamp
|
||||||
from helpers.discord_utils import get_team_embed
|
from helpers.discord_utils import get_team_embed
|
||||||
from helpers.constants import IMAGES, PD_SEASON
|
from helpers.constants import IMAGES, PD_SEASON
|
||||||
@ -72,8 +76,6 @@ class ScoutView(discord.ui.View):
|
|||||||
if not self.message:
|
if not self.message:
|
||||||
return
|
return
|
||||||
|
|
||||||
from helpers.scouting import build_scouted_card_list
|
|
||||||
|
|
||||||
card_list = build_scouted_card_list(self.card_lines, self.claims)
|
card_list = build_scouted_card_list(self.card_lines, self.claims)
|
||||||
|
|
||||||
title = f"Scout Opportunity! ({self.total_scouts} scouted)"
|
title = f"Scout Opportunity! ({self.total_scouts} scouted)"
|
||||||
@ -163,6 +165,10 @@ class ScoutButton(discord.ui.Button):
|
|||||||
|
|
||||||
# Prevent double-click race for same user
|
# Prevent double-click race for same user
|
||||||
if interaction.user.id in view.processing_users:
|
if interaction.user.id in view.processing_users:
|
||||||
|
await interaction.response.send_message(
|
||||||
|
"Your scout is already being processed!",
|
||||||
|
ephemeral=True,
|
||||||
|
)
|
||||||
return
|
return
|
||||||
|
|
||||||
view.processing_users.add(interaction.user.id)
|
view.processing_users.add(interaction.user.id)
|
||||||
@ -206,6 +212,20 @@ class ScoutButton(discord.ui.Button):
|
|||||||
)
|
)
|
||||||
return
|
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
|
# Consume a scout token
|
||||||
current = await db_get("current")
|
current = await db_get("current")
|
||||||
await db_post(
|
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
|
# Track the claim
|
||||||
player_id = self.card["player"]["player_id"]
|
player_id = self.card["player"]["player_id"]
|
||||||
if player_id not in view.claims:
|
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.
|
and embed formatting for the scouting feature.
|
||||||
"""
|
"""
|
||||||
|
|
||||||
import asyncio
|
|
||||||
import datetime
|
import datetime
|
||||||
import logging
|
import logging
|
||||||
import random
|
import random
|
||||||
@ -95,7 +94,7 @@ def build_scout_embed(
|
|||||||
f"{time_line}"
|
f"{time_line}"
|
||||||
)
|
)
|
||||||
embed.set_footer(
|
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"],
|
icon_url=IMAGES["logo"],
|
||||||
)
|
)
|
||||||
return embed, card_lines
|
return embed, card_lines
|
||||||
|
|||||||
@ -11,10 +11,13 @@ import discord
|
|||||||
|
|
||||||
|
|
||||||
def int_timestamp(datetime_obj: Optional[datetime.datetime] = None):
|
def int_timestamp(datetime_obj: Optional[datetime.datetime] = None):
|
||||||
"""Convert current datetime to integer timestamp."""
|
"""Convert a datetime to an integer millisecond timestamp.
|
||||||
if datetime_obj:
|
|
||||||
return int(datetime.datetime.timestamp(datetime_obj) * 1000)
|
If no argument is given, uses the current time.
|
||||||
return int(datetime.datetime.now().timestamp())
|
"""
|
||||||
|
if datetime_obj is None:
|
||||||
|
datetime_obj = datetime.datetime.now()
|
||||||
|
return int(datetime.datetime.timestamp(datetime_obj) * 1000)
|
||||||
|
|
||||||
|
|
||||||
def midnight_timestamp() -> int:
|
def midnight_timestamp() -> int:
|
||||||
|
|||||||
@ -162,19 +162,26 @@ class TestScoutButtonGuards:
|
|||||||
async def test_double_click_silently_ignored(
|
async def test_double_click_silently_ignored(
|
||||||
self, sample_cards, opener_team, mock_bot
|
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 = self._make_view(sample_cards, opener_team, mock_bot)
|
||||||
view.processing_users.add(12345)
|
view.processing_users.add(12345)
|
||||||
button = view.children[0]
|
button = view.children[0]
|
||||||
|
|
||||||
interaction = AsyncMock(spec=discord.Interaction)
|
interaction = AsyncMock(spec=discord.Interaction)
|
||||||
|
interaction.response = AsyncMock()
|
||||||
|
interaction.response.send_message = AsyncMock()
|
||||||
interaction.user = Mock()
|
interaction.user = Mock()
|
||||||
interaction.user.id = 12345
|
interaction.user.id = 12345
|
||||||
|
|
||||||
await button.callback(interaction)
|
await button.callback(interaction)
|
||||||
|
|
||||||
# Should not have called defer or send_message
|
interaction.response.send_message.assert_called_once()
|
||||||
interaction.response.defer.assert_not_called()
|
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
|
# Should have deferred
|
||||||
interaction.response.defer.assert_called_once_with(ephemeral=True)
|
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
|
assert mock_db_post.call_count == 3
|
||||||
|
|
||||||
# Verify scout_claims POST
|
# Verify scout_claims POST
|
||||||
claim_call = mock_db_post.call_args_list[0]
|
claim_call = mock_db_post.call_args_list[0]
|
||||||
assert claim_call[0][0] == "scout_claims"
|
assert claim_call[0][0] == "scout_claims"
|
||||||
|
|
||||||
# Verify rewards POST (token consumption)
|
# Verify cards POST (card copy — created before token consumption)
|
||||||
reward_call = mock_db_post.call_args_list[1]
|
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[0][0] == "rewards"
|
||||||
assert reward_call[1]["payload"]["name"] == "Scout Token"
|
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
|
# User should be marked as scouted
|
||||||
assert 12345 in view.scouted_users
|
assert 12345 in view.scouted_users
|
||||||
assert view.total_scouts == 1
|
assert view.total_scouts == 1
|
||||||
@ -797,9 +804,11 @@ class TestCurrentSeasonFallback:
|
|||||||
assert 12345 in view.scouted_users
|
assert 12345 in view.scouted_users
|
||||||
|
|
||||||
# Verify the rewards POST used fallback values
|
# Verify the rewards POST used fallback values
|
||||||
|
# Order: scout_claims (0), cards (1), rewards (2)
|
||||||
from helpers.constants import PD_SEASON
|
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"]["season"] == PD_SEASON
|
||||||
assert reward_call[1]["payload"]["week"] == 1
|
assert reward_call[1]["payload"]["week"] == 1
|
||||||
|
|
||||||
|
|||||||
Loading…
Reference in New Issue
Block a user