From 51c81bf1edb34f0719d089a9af6641f340add08e Mon Sep 17 00:00:00 2001 From: lesteenman Date: Thu, 29 Apr 2021 09:15:41 +0200 Subject: [PATCH] Fixes #43: add an optional min or max to games while creating. --- Makefile | 7 + bin/register_commands.py | 12 ++ .../eternal_guesses/model/data/game.py | 8 +- .../eternal_guesses/model/data/game_guess.py | 4 + .../repositories/dynamodb_models.py | 2 + .../repositories/games_repository.py | 51 ++++--- discord_app/eternal_guesses/routes/create.py | 6 +- discord_app/eternal_guesses/routes/guess.py | 25 ++++ .../eternal_guesses/util/message_provider.py | 22 ++- discord_app/tests/integration/helpers.py | 14 +- .../repositories/test_games_repository.py | 9 ++ .../test_integration_create_and_vote.py | 22 ++- discord_app/tests/unit/routes/test_create.py | 47 ++++++- discord_app/tests/unit/routes/test_guess.py | 127 ++++++++++++++++++ .../tests/unit/util/test_game_post_manager.py | 16 +-- 15 files changed, 329 insertions(+), 43 deletions(-) diff --git a/Makefile b/Makefile index 39b40ce..a3e9ebd 100644 --- a/Makefile +++ b/Makefile @@ -2,6 +2,13 @@ DISCORD_APP_DIR=discord_app ERROR_PARSER_DIR=error_parser_function INFRA_DIR=infra +autopep: + python -m autopep8 --recursive --in-place --aggressive discord_app/eternal_guesses + python -m autopep8 --recursive --in-place --aggressive discord_app/tests + python -m autopep8 --recursive --in-place --aggressive error_parser_function/ + python -m autopep8 --recursive --in-place --aggressive infra/app.py + python -m autopep8 --recursive --in-place --aggressive infra/infra/ + quality: cd ${DISCORD_APP_DIR} && poetry run flake8 --config ../.flake8 ./tests ./eternal_guesses ../error_parser_function ../infra diff --git a/bin/register_commands.py b/bin/register_commands.py index d9c6044..6448fc9 100755 --- a/bin/register_commands.py +++ b/bin/register_commands.py @@ -127,6 +127,18 @@ def register(config: Dict): "type": COMMAND_OPTION_TYPE_STRING, "required": False, }, + { + "name": "min", + "description": "The lowest guess allowed.", + "type": COMMAND_OPTION_TYPE_INTEGER, + "required": False, + }, + { + "name": "max", + "description": "The highest guess allowed.", + "type": COMMAND_OPTION_TYPE_INTEGER, + "required": False, + }, ], }, { diff --git a/discord_app/eternal_guesses/model/data/game.py b/discord_app/eternal_guesses/model/data/game.py index 8ef7261..9fba3ec 100644 --- a/discord_app/eternal_guesses/model/data/game.py +++ b/discord_app/eternal_guesses/model/data/game.py @@ -9,7 +9,7 @@ class Game: def __init__(self, guild_id: int = None, game_id: str = None, created_by: int = None, closed: bool = None, guesses: Mapping[int, GameGuess] = None, channel_messages: List[ChannelMessage] = None, create_datetime: datetime = None, close_datetime: Optional[datetime] = None, title: str = None, - description: str = None): + description: str = None, min_guess: int = None, max_guess: int = None): self.guild_id = guild_id self.game_id = game_id self.title = title @@ -22,8 +22,14 @@ def __init__(self, guild_id: int = None, game_id: str = None, created_by: int = self.guesses = guesses self.channel_messages = channel_messages + self.min_guess = min_guess + self.max_guess = max_guess + if channel_messages is None: self.channel_messages = [] if guesses is None: self.guesses = {} + + def is_numeric(self): + return self.min_guess is not None or self.max_guess is not None diff --git a/discord_app/eternal_guesses/model/data/game_guess.py b/discord_app/eternal_guesses/model/data/game_guess.py index 36d5d3c..b0cd0be 100644 --- a/discord_app/eternal_guesses/model/data/game_guess.py +++ b/discord_app/eternal_guesses/model/data/game_guess.py @@ -7,3 +7,7 @@ def __init__(self, user_id: int = None, guess: str = None, nickname: str = None, self.guess = guess self.nickname = nickname self.timestamp = timestamp + + def __repr__(self): + return f" Game: if model.close_datetime is not None: close_datetime = datetime.fromisoformat(model.close_datetime) - closed = False - if model.closed is not None: - closed = model.closed - - title = "" - if model.title is not None: - title = model.title - - description = "" - if model.description is not None: - description = model.description + closed = model.closed or False + title = model.title or "" + description = model.description or "" + min_guess = model.min_guess + max_guess = model.max_guess channel_messages = [] if model.channel_messages is not None: @@ -69,12 +63,14 @@ def _game_from_model(model: EternalGuessesTable) -> Game: game_id=game_id, title=title, description=description, + min_guess=min_guess, + max_guess=max_guess, created_by=model.created_by, create_datetime=create_datetime, close_datetime=close_datetime, closed=closed, channel_messages=channel_messages, - guesses=guesses + guesses=guesses, ) @@ -142,6 +138,12 @@ def save(self, game: Game): if game.description: model.description = game.description + if game.min_guess: + model.min_guess = game.min_guess + + if game.max_guess: + model.max_guess = game.max_guess + if game.create_datetime is not None: model.create_datetime = game.create_datetime.isoformat() @@ -149,15 +151,7 @@ def save(self, game: Game): model.close_datetime = game.close_datetime.isoformat() if game.guesses is not None: - guesses = {} - for user_id, game_guess in game.guesses.items(): - guesses[user_id] = { - 'user_id': game_guess.user_id, - 'nickname': game_guess.nickname, - 'guess': game_guess.guess, - 'timestamp': game_guess.timestamp.isoformat(), - } - model.guesses = json.dumps(guesses) + model.guesses = json.dumps(self._guesses(game)) if game.channel_messages is not None: channel_messages = [] @@ -166,3 +160,16 @@ def save(self, game: Game): model.channel_messages = channel_messages model.save() + + def _guesses(self, game): + guesses = {} + + for user_id, game_guess in game.guesses.items(): + guesses[user_id] = { + 'user_id': game_guess.user_id, + 'nickname': game_guess.nickname, + 'guess': game_guess.guess, + 'timestamp': game_guess.timestamp.isoformat(), + } + + return guesses diff --git a/discord_app/eternal_guesses/routes/create.py b/discord_app/eternal_guesses/routes/create.py index d6d02ff..f761125 100644 --- a/discord_app/eternal_guesses/routes/create.py +++ b/discord_app/eternal_guesses/routes/create.py @@ -26,6 +26,8 @@ async def call(self, event: DiscordEvent) -> DiscordResponse: game_id = event.command.options.get('game-id') title = event.command.options.get('title') description = event.command.options.get('description') + min_guess = event.command.options.get('min') + max_guess = event.command.options.get('max') if game_id is None: game_id = self._generate_game_id(guild_id) @@ -41,10 +43,12 @@ async def call(self, event: DiscordEvent) -> DiscordResponse: game_id=game_id, title=title, description=description, + min_guess=min_guess, + max_guess=max_guess, created_by=event.member.user_id, create_datetime=datetime.now(), close_datetime=None, - closed=False + closed=False, ) self.games_repository.save(game) diff --git a/discord_app/eternal_guesses/routes/guess.py b/discord_app/eternal_guesses/routes/guess.py index 55b7292..856d671 100644 --- a/discord_app/eternal_guesses/routes/guess.py +++ b/discord_app/eternal_guesses/routes/guess.py @@ -1,5 +1,6 @@ from datetime import datetime +from eternal_guesses.model.data.game import Game from eternal_guesses.model.data.game_guess import GameGuess from eternal_guesses.model.discord.discord_event import DiscordEvent from eternal_guesses.model.discord.discord_response import DiscordResponse @@ -38,6 +39,11 @@ async def call(self, event: DiscordEvent) -> DiscordResponse: error_message = self.message_provider.error_guess_on_closed_game(game_id) return DiscordResponse.ephemeral_channel_message(error_message) + if game.is_numeric(): + if not self.validate_guess(game=game, guess=guess): + error_message = self.message_provider.invalid_guess(game) + return DiscordResponse.ephemeral_channel_message(error_message) + game_guess = GameGuess() game_guess.user_id = user_id game_guess.user_nickname = user_nickname @@ -51,3 +57,22 @@ async def call(self, event: DiscordEvent) -> DiscordResponse: guess_added_message = self.message_provider.guess_added(game_id, guess) return DiscordResponse.ephemeral_channel_message(content=guess_added_message) + + def validate_guess(self, game: Game, guess: str): + if not self.is_numeric(guess): + return False + + if game.max_guess is not None and int(guess) > game.max_guess: + return False + + if game.min_guess is not None and int(guess) < game.min_guess: + return False + + return True + + def is_numeric(self, guess: str): + try: + int(guess) + return True + except ValueError: + return False diff --git a/discord_app/eternal_guesses/util/message_provider.py b/discord_app/eternal_guesses/util/message_provider.py index 50bc555..13e54ae 100644 --- a/discord_app/eternal_guesses/util/message_provider.py +++ b/discord_app/eternal_guesses/util/message_provider.py @@ -90,6 +90,9 @@ def guess_edited(self) -> str: def guess_deleted(self) -> str: raise NotImplementedError() + def invalid_guess(self, game: Game) -> str: + raise NotImplementedError() + class MessageProviderImpl(MessageProvider): def game_post_embed(self, game: Game) -> discord.Embed: @@ -105,7 +108,7 @@ def game_post_embed(self, game: Game) -> discord.Embed: description += "Guesses:\n\n" guess_list = [] - for user_id, guess in sorted(game.guesses.items(), key=lambda i: i[1].guess): + for user_id, guess in _sorted_guesses(game): guess_list.append(f"<@{user_id}>: {guess.guess}") if len(guess_list) > 0: @@ -232,3 +235,20 @@ def guess_edited(self) -> str: def guess_deleted(self) -> str: return "The guess has been deleted." + + def invalid_guess(self, game: Game) -> str: + if game.min_guess is not None and game.max_guess is not None: + return f"The guess must be a number between {game.min_guess} and {game.max_guess}" + elif game.min_guess is not None: + return f"The guess must be a number higher than {game.min_guess}" + elif game.max_guess is not None: + return f"The guess must be a number lower than {game.max_guess}" + + raise NotImplementedError() + + +def _sorted_guesses(game): + if game.is_numeric(): + return sorted(game.guesses.items(), key=lambda i: int(i[1].guess)) + else: + return sorted(game.guesses.items(), key=lambda i: i[1].guess) diff --git a/discord_app/tests/integration/helpers.py b/discord_app/tests/integration/helpers.py index 7d6829a..58d72e7 100644 --- a/discord_app/tests/integration/helpers.py +++ b/discord_app/tests/integration/helpers.py @@ -43,7 +43,7 @@ def make_discord_guess_event(guild_id: int, game_id: str, guess: str, user_id: i def make_discord_create_event(guild_id: int, game_id: str, game_title: str = None, game_description: str = None, channel_id: int = DEFAULT_CHANNEL_ID, user_id: int = DEFAULT_USER_ID, member_nickname: str = DEFAULT_MEMBER_NICK, user_name: str = DEFAULT_USER_NAME, - role_id: int = DEFAULT_ROLE_ID): + role_id: int = DEFAULT_ROLE_ID, min_guess: int = None, max_guess: int = None): event_body = _base_event_body(guild_id=guild_id, channel_id=channel_id, user_id=user_id, member_nickname=member_nickname, user_name=user_name, role_id=role_id, is_admin=False) @@ -66,6 +66,18 @@ def make_discord_create_event(guild_id: int, game_id: str, game_title: str = Non "value": game_description, }) + if min_guess is not None: + options.append({ + "name": "min", + "value": min_guess, + }) + + if max_guess is not None: + options.append({ + "name": "max", + "value": max_guess, + }) + event_body['data'] = { "id": "2001", "name": "eternal-guess", diff --git a/discord_app/tests/integration/repositories/test_games_repository.py b/discord_app/tests/integration/repositories/test_games_repository.py index b3e8339..5a7ee9e 100644 --- a/discord_app/tests/integration/repositories/test_games_repository.py +++ b/discord_app/tests/integration/repositories/test_games_repository.py @@ -37,6 +37,7 @@ def test_get_minimal_game(): # Then assert game.guild_id == retrieved_game.guild_id assert game.game_id == retrieved_game.game_id + assert not game.is_numeric() def test_get_game(): @@ -57,6 +58,8 @@ def test_get_game(): closed = True title = 'A mockery of fools' description = 'A tale as old as time' + min_guess = 1 + max_guess = 500 game = Game( guild_id=guild_id, @@ -67,6 +70,8 @@ def test_get_game(): game_id=game_id, title=title, description=description, + min_guess=min_guess, + max_guess=max_guess, guesses={ user_id: GameGuess( user_id=user_id, @@ -95,6 +100,10 @@ def test_get_game(): assert retrieved_game.title == title assert retrieved_game.description == description + assert retrieved_game.min_guess == min_guess + assert retrieved_game.max_guess == max_guess + assert retrieved_game.is_numeric() + assert user_id in retrieved_game.guesses game_guess = retrieved_game.guesses[user_id] diff --git a/discord_app/tests/integration/test_integration_create_and_vote.py b/discord_app/tests/integration/test_integration_create_and_vote.py index 9f4ab9e..6919639 100644 --- a/discord_app/tests/integration/test_integration_create_and_vote.py +++ b/discord_app/tests/integration/test_integration_create_and_vote.py @@ -29,6 +29,8 @@ def test_integration_full_flow(): game_id=game_id, title=game_title, description=game_description, + min_guess=50, + max_guess=5000, channel_id=management_channel) game = games_repository.get(guild_id, game_id) @@ -61,7 +63,7 @@ def test_integration_full_flow(): assert len(game.guesses) == 2 # Change a member's guess as a management user - new_guess = "5600" + new_guess = "4900" change_guess(guild_id=guild_id, game_id=game_id, guessing_user_id=user_id, new_guess=new_guess, channel_id=management_channel) @@ -75,6 +77,19 @@ def test_integration_full_flow(): assert user_id not in game.guesses assert another_user_id in game.guesses + # Place a guess that's below the min + one_more_user_id = 102 + guess_on_game(guild_id=guild_id, game_id=game_id, guess="49", user_id=one_more_user_id) + + game = games_repository.get(guild_id, game_id) + assert one_more_user_id not in game.guesses + + # Place a guess that's above the max + guess_on_game(guild_id=guild_id, game_id=game_id, guess="5001", user_id=one_more_user_id) + + game = games_repository.get(guild_id, game_id) + assert one_more_user_id not in game.guesses + def guess_on_game(guild_id: int, game_id: str, guess: str, user_id: int): response = handler.handle_lambda( @@ -85,7 +100,8 @@ def guess_on_game(guild_id: int, game_id: str, guess: str, user_id: int): assert response['statusCode'] == 200 -def create_new_game(guild_id: int, game_id: str, title: str, description: str, channel_id: int): +def create_new_game(guild_id: int, game_id: str, title: str, description: str, channel_id: int, + min_guess: int, max_guess: int): response = handler.handle_lambda( make_discord_create_event( guild_id=guild_id, @@ -93,6 +109,8 @@ def create_new_game(guild_id: int, game_id: str, title: str, description: str, c game_title=title, game_description=description, channel_id=channel_id, + min_guess=min_guess, + max_guess=max_guess, ), create_context() ) diff --git a/discord_app/tests/unit/routes/test_create.py b/discord_app/tests/unit/routes/test_create.py index 10a9b29..fab4d56 100644 --- a/discord_app/tests/unit/routes/test_create.py +++ b/discord_app/tests/unit/routes/test_create.py @@ -11,11 +11,10 @@ from eternal_guesses.routes.create import CreateRoute from eternal_guesses.util.discord_messaging import DiscordMessaging from eternal_guesses.util.message_provider import MessageProvider -from tests.fakes import FakeDiscordMessaging +from tests.fakes import FakeDiscordMessaging, FakeGamesRepository pytestmark = pytest.mark.asyncio - GAME_CREATED_MESSAGE = "Game created." DUPLICATE_GAME_ID = "Duplicate game id." @@ -178,7 +177,7 @@ async def test_create_sets_title_and_description(): mock_games_repository = MagicMock(GamesRepositoryImpl, autospec=True) mock_games_repository.get.return_value = None - event = _make_event(description, title) + event = _make_event(description=description, title=title) # When create_route = _route(games_repository=mock_games_repository) @@ -195,19 +194,53 @@ async def test_create_sets_title_and_description(): assert response.content == GAME_CREATED_MESSAGE +async def test_create_with_min_max(): + # Given + min_guess = 1 + max_guess = 20 + + guild_id = 1 + game_id = 'game-with-min-max' + + games_repository = FakeGamesRepository() + + event = _make_event(guild_id=guild_id, game_id=game_id, min_guess=min_guess, + max_guess=max_guess) + + # When + create_route = _route(games_repository=games_repository) + response = await create_route.call(event) + + # Then + saved_game = games_repository.get(guild_id, game_id) + + assert saved_game.min_guess == min_guess + assert saved_game.max_guess == max_guess + + assert response.is_ephemeral + assert response.content == GAME_CREATED_MESSAGE + + def _make_event( - description="", - title=""): + game_id: str = 'game-id', + guild_id: int = -1, + description: str = None, + title: str = None, + min_guess: int = None, + max_guess: int = None, +): return DiscordEvent( command=DiscordCommand( command_name="create", options={ - 'game-id': 'game-id', + 'game-id': game_id, + 'min': min_guess, + 'max': max_guess, 'title': title, 'description': description, } ), - guild_id=1000, + guild_id=guild_id, member=DiscordMember(), ) diff --git a/discord_app/tests/unit/routes/test_guess.py b/discord_app/tests/unit/routes/test_guess.py index f1a2bad..c692f30 100644 --- a/discord_app/tests/unit/routes/test_guess.py +++ b/discord_app/tests/unit/routes/test_guess.py @@ -268,6 +268,133 @@ async def test_guess_closed_game(): assert response.content == duplicate_guess_message +async def test_guess_non_numerical_on_numeric_game(): + # Given + guild_id = 1005 + user_id = 13000 + game_id = 'game-id' + + response_message = "Guess must be a number" + message_provider = MagicMock(MessageProvider, autospec=True) + message_provider.invalid_guess.return_value = response_message + + # And we have a game with a min and a max + game = Game( + guild_id=guild_id, + game_id=game_id, + min_guess=1, + max_guess=100 + ) + + games_repository = FakeGamesRepository([game]) + guess_route = _route( + games_repository=games_repository, + message_provider=message_provider, + ) + + # When + event = _create_guess_event(guild_id, game.game_id, user_id, 'nickname') + response = await guess_route.call(event) + + # Then + assert response.is_ephemeral + assert response.content == response_message + + # And the guess was not added + updated_game = games_repository.get(guild_id=guild_id, game_id=game_id) + assert len(updated_game.guesses) == 0 + + +async def test_guess_higher_than_max(): + # Given + guild_id = 1005 + user_id = 13000 + game_id = 'game-id' + + max_guess = 100 + + response_message = "Guess must be a number below 100" + message_provider = MagicMock(MessageProvider, autospec=True) + message_provider.invalid_guess.return_value = response_message + + # And we have a game with a max + game = Game( + guild_id=guild_id, + game_id=game_id, + max_guess=max_guess, + ) + + games_repository = FakeGamesRepository([game]) + guess_route = _route( + games_repository=games_repository, + message_provider=message_provider, + ) + + # When we guess higher than that + guess = "101" + event = _create_guess_event( + guild_id=guild_id, + game_id=game.game_id, + user_id=user_id, + user_nickname='nickname', + guess=guess, + ) + response = await guess_route.call(event) + + # Then + assert response.is_ephemeral + assert response.content == response_message + + # And the guess was not added + updated_game = games_repository.get(guild_id=guild_id, game_id=game_id) + assert len(updated_game.guesses) == 0 + + +async def test_lower_than_min(): + # Given + guild_id = 1005 + user_id = 13000 + game_id = 'game-id' + + min_guess = -5 + + response_message = "Guess must be a number higher than -5" + message_provider = MagicMock(MessageProvider, autospec=True) + message_provider.invalid_guess.return_value = response_message + + # And we have a game with a max + game = Game( + guild_id=guild_id, + game_id=game_id, + min_guess=min_guess, + ) + + games_repository = FakeGamesRepository([game]) + guess_route = _route( + games_repository=games_repository, + message_provider=message_provider, + ) + + # When we guess lower than that + guess = "-6" + event = _create_guess_event( + guild_id=guild_id, + game_id=game.game_id, + user_id=user_id, + user_nickname='nickname', + guess=guess, + ) + response = await guess_route.call(event) + + # Then + assert response.is_ephemeral + assert response.content == response_message + + # And the guess was not added + updated_game = games_repository.get(guild_id=guild_id, game_id=game_id) + assert len(updated_game.guesses) == 0 + + def _create_guess_event(guild_id: int, game_id: str, user_id: int = -1, user_nickname: str = 'nickname', guess: str = 'not-relevant', event_channel_id: int = -1, member: DiscordMember = None): if member is None: diff --git a/discord_app/tests/unit/util/test_game_post_manager.py b/discord_app/tests/unit/util/test_game_post_manager.py index 562836a..7f8c435 100644 --- a/discord_app/tests/unit/util/test_game_post_manager.py +++ b/discord_app/tests/unit/util/test_game_post_manager.py @@ -42,15 +42,15 @@ async def test_update_post(): update_channel_message_calls = discord_messaging.updated_channel_messages assert len(update_channel_message_calls) == 2 assert { - 'channel_id': game_post_1.channel_id, - 'message_id': game_post_1.message_id, - 'embed': post_embed, - } in update_channel_message_calls + 'channel_id': game_post_1.channel_id, + 'message_id': game_post_1.message_id, + 'embed': post_embed, + } in update_channel_message_calls assert { - 'channel_id': game_post_2.channel_id, - 'message_id': game_post_2.message_id, - 'embed': post_embed, - } in update_channel_message_calls + 'channel_id': game_post_2.channel_id, + 'message_id': game_post_2.message_id, + 'embed': post_embed, + } in update_channel_message_calls async def test_guess_channel_message_gone_silently_fails():