Bug fixes #18
Bug fixes #18
Conversation
(permissions), small fixes, ref
WalkthroughThe recent updates across various files focus on refining string values, improving error handling, enhancing data models, and ensuring proper group chat interactions. These changes aim to enhance user experience, data integrity, and overall functionality of the Telegram bot. Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Review Status
Actionable comments generated: 3
Configuration used: CodeRabbit UI
Files selected for processing (12)
- README.md (1 hunks)
- src/main.py (2 hunks)
- src/config.py (2 hunks)
- src/data_models/Game.py (1 hunks)
- src/data_models/Player.py (2 hunks)
- src/data_models/Poll.py (2 hunks)
- src/exceptions.py (1 hunks)
- src/handlers/game.py (2 hunks)
- src/handlers/save.py (3 hunks)
- src/handlers/start.py (1 hunks)
- src/sql/init.sql (3 hunks)
- src/utils.py (1 hunks)
Files skipped from review due to trivial changes (1)
- src/data_models/Game.py
Additional comments: 16
src/data_models/Poll.py (1)
- 12-12: The addition of the
creator_usernamefield as an optional string with a default value ofNoneis correctly implemented using Pydantic'sField. This change aligns with the PR's objective to adjust data models for better data handling and clarity.src/exceptions.py (1)
- 1-10: The implementation of
GroupChatRequiredExceptionis well-done, with a clear custom message and user ID attribute to enhance error handling in group chat contexts. This aligns with the PR's objective to improve error handling and group chat specific logic.src/utils.py (1)
- 11-11: The modification to
is_message_from_group_chatto check for both "group" and "supergroup" chat types is correct and aligns with the PR's objective to enhance group chat functionality.src/data_models/Player.py (2)
- 8-8: Making the
usernamefield optional and nullable aligns with the PR's objective to adjust data models for better data handling. This change is correctly implemented using Pydantic'sField.- 20-23: The addition of
validate_usernameto clean non-alphanumeric characters from the username is a good practice for data sanitization. However, ensure that this cleaning process aligns with the expected username format in your application context.src/handlers/start.py (1)
- 7-20: The addition of an
AppConfigparameter to thestartfunction and the expanded response message enhance user interaction and guidance, aligning with the PR's objectives. Ensure that all calls to this function are updated to include the new parameter.src/__main__.py (1)
- 19-21: The addition of the
post_initfunction to set bot commands aligns with the PR's objective to streamline the bot's startup process. This is a good practice for initializing bot commands based on configuration.README.md (1)
- 15-15: Correcting the link for adding the bot to a group chat in the README.md file aligns with the PR's objective to enhance functionality and user experience. Ensure the link is tested and functional.
src/config.py (1)
- 47-56: Adding a
commandslist toAppConfigwithBotCommandinstances for various bot commands is a good practice for organized and accessible command management, aligning with the PR's objectives.src/handlers/game.py (2)
- 19-23: The implementation for checking if a message is from a group chat before starting a game is well done. It ensures that the game functionality is restricted to the intended context, enhancing user experience and data integrity.
- 63-67: The addition of message deletion at the end of the game function is a thoughtful touch, helping to keep the chat tidy. Good practice!
src/sql/init.sql (3)
- 4-4: Allowing the
usernamefield in theplayerstable to be NULL is a thoughtful adjustment, accommodating the reality that not all Telegram users have a username. This enhances data integrity and inclusivity.- 46-46: Updating the
rolefield value fromDHtoHDin therecordstable likely improves consistency and clarity. Ensure that all related code and documentation reflect this change for uniformity across the application.- 60-60: Making the
creator_usernamefield in thepollstable nullable is consistent with accommodating users without a Telegram username. This change aligns well with the principle of inclusivity in data modeling.src/handlers/save.py (2)
- 113-121: The adjustments to message deletion logic after saving game results are well thought out, contributing to a cleaner chat environment. Good integration with post-game tasks.
- 65-67: The adjustment in the
_pass_checksfunction to correctly reference the user attempting to stop a poll is a significant improvement. It enhances clarity and provides targeted feedback, improving the user experience.
src/handlers/game.py
Outdated
| message = await context.bot.send_poll( | ||
| update.effective_chat.id, | ||
| f"@{update.effective_user.username} wants you to record the last game. Please choose your outcome:", | ||
| questions, | ||
| is_anonymous=False, | ||
| allows_multiple_answers=False, | ||
| disable_notification=True, | ||
| ) | ||
| except Exception as e: | ||
| await update.effective_message.reply_text( | ||
| f"The bot need permission to create Telegram Polls to start a game\n" | ||
| f"Please grant those permissions in chat settings." | ||
| ) | ||
| return |
There was a problem hiding this comment.
Consider logging the exception caught when sending a poll fails. This can aid in debugging and understanding the specific issues encountered during operation.
+ import logging
...
except Exception as e:
+ logging.error(f"Failed to send poll: {e}")
await update.effective_message.reply_text(Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| message = await context.bot.send_poll( | |
| update.effective_chat.id, | |
| f"@{update.effective_user.username} wants you to record the last game. Please choose your outcome:", | |
| questions, | |
| is_anonymous=False, | |
| allows_multiple_answers=False, | |
| disable_notification=True, | |
| ) | |
| except Exception as e: | |
| await update.effective_message.reply_text( | |
| f"The bot need permission to create Telegram Polls to start a game\n" | |
| f"Please grant those permissions in chat settings." | |
| ) | |
| return | |
| import logging | |
| message = await context.bot.send_poll( | |
| update.effective_chat.id, | |
| f"@{update.effective_user.username} wants you to record the last game. Please choose your outcome:", | |
| questions, | |
| is_anonymous=False, | |
| allows_multiple_answers=False, | |
| disable_notification=True, | |
| ) | |
| except Exception as e: | |
| logging.error(f"Failed to send poll: {e}") | |
| await update.effective_message.reply_text( | |
| f"The bot need permission to create Telegram Polls to start a game\n" | |
| f"Please grant those permissions in chat settings." | |
| ) | |
| return |
| async def send_result(context, update, game: Game, records: list[Record]): | ||
| try: | ||
| await context.bot.send_photo( | ||
| chat_id=game.chat_id, | ||
| photo=await draw_result_image( | ||
| records=records, result=game.results, update=update, context=context | ||
| ), | ||
| caption=f"The Game has been saved! Result: {game.results}", | ||
| disable_notification=True, | ||
| ) | ||
| except Exception as e: | ||
| await update.effective_message.reply_text( | ||
| f"Result: {game.results}\nP.S. this bot can send you a result image, allow it to send photos. {e}" | ||
| ) |
There was a problem hiding this comment.
The send_result function introduces a robust way to communicate game results. Consider logging the exception caught when sending a photo fails to aid in debugging and understanding the specific issues encountered.
+ import logging
...
except Exception as e:
+ logging.error(f"Failed to send result photo: {e}")
await update.effective_message.reply_text(Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def send_result(context, update, game: Game, records: list[Record]): | |
| try: | |
| await context.bot.send_photo( | |
| chat_id=game.chat_id, | |
| photo=await draw_result_image( | |
| records=records, result=game.results, update=update, context=context | |
| ), | |
| caption=f"The Game has been saved! Result: {game.results}", | |
| disable_notification=True, | |
| ) | |
| except Exception as e: | |
| await update.effective_message.reply_text( | |
| f"Result: {game.results}\nP.S. this bot can send you a result image, allow it to send photos. {e}" | |
| ) | |
| import logging | |
| async def send_result(context, update, game: Game, records: list[Record]): | |
| try: | |
| await context.bot.send_photo( | |
| chat_id=game.chat_id, | |
| photo=await draw_result_image( | |
| records=records, result=game.results, update=update, context=context | |
| ), | |
| caption=f"The Game has been saved! Result: {game.results}", | |
| disable_notification=True, | |
| ) | |
| except Exception as e: | |
| logging.error(f"Failed to send result photo: {e}") | |
| await update.effective_message.reply_text( | |
| f"Result: {game.results}\nP.S. this bot can send you a result image, allow it to send photos. {e}" | |
| ) |
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (5)
- src/config.py (3 hunks)
- src/data_models/Game.py (3 hunks)
- src/data_models/Record.py (1 hunks)
- src/handlers/game.py (2 hunks)
- src/sql/init.sql (4 hunks)
Additional comments not posted (15)
src/data_models/Game.py (3)
14-14: The correction in the comment for theresultsattribute improves clarity and accuracy.
23-23: Correcting the typo in the validation logic ensures consistency and clarity in the game's outcome processing.
40-40: Correcting the error message to accurately reference theresultsvariable enhances error reporting.src/data_models/Record.py (1)
21-21: Correcting the typo in theshorten_rolefunction improves consistency and clarity in role processing.src/config.py (3)
6-6: The addition of theBotCommandimport is necessary for defining bot commands and enhances functionality.
30-30: Correcting the typo ingame_poll_outcomesimproves consistency and clarity in game outcome processing.
47-56: The addition of thecommandslist within theAppConfigclass enhances the bot's configurability and usability.src/handlers/game.py (4)
8-8: The addition of theGroupChatRequiredExceptionimport is necessary for ensuring the game starts only in appropriate contexts.
19-23: The logic to check if a message is from a group chat before starting the game enhances the bot's robustness and user experience.
28-40: The improved error handling when sending polls, including a specific error message for permission issues, makes the bot more user-friendly.
63-67: The addition of a message deletion function at the end of the game function improves the bot's cleanliness and user experience.src/sql/init.sql (4)
4-4: Making theusernamefield nullable in theplayerstable improves the database schema's flexibility and inclusivity.
34-34: Updating the possible values in theresultfield of thegamestable ensures consistency and clarity in the database schema.
46-46: Correcting the typo and updating the comment in therolefield of therecordstable enhances the database schema's clarity.
60-60: Making thecreator_usernamefield nullable in thepollstable improves the database schema's flexibility and inclusivity.
| allows_multiple_answers=False, | ||
| disable_notification=True, | ||
| ) | ||
| except Exception as e: |
There was a problem hiding this comment.
Consider logging the exception caught when sending a poll fails. This can aid in debugging and understanding the specific issues encountered during operation.
+ import logging
...
except Exception as e:
+ logging.error(f"Failed to send poll: {e}")
await update.effective_message.reply_text(Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| except Exception as e: | |
| import logging | |
| ... | |
| except Exception as e: | |
| logging.error(f"Failed to send poll: {e}") | |
| await update.effective_message.reply_text( |
There was a problem hiding this comment.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (4)
- src/main.py (2 hunks)
- src/data_models/Player.py (2 hunks)
- src/handlers/save.py (3 hunks)
- src/utils.py (1 hunks)
Additional comments not posted (6)
src/utils.py (1)
11-11: LGTM! The logic for checking if a message is from a group chat is correctly implemented.src/data_models/Player.py (2)
8-8: Making theusernamefield optional with a default value ofNoneis a sensible change for flexibility.
22-25: The implementation ofvalidate_usernameusing a regular expression for sanitization is a good practice.src/__main__.py (1)
19-20: The implementation ofpost_initfor setting bot commands dynamically is a good practice.src/handlers/save.py (2)
65-69: Correctly referencing the user attempting to stop a poll in_pass_checksenhances game integrity.
117-125: Usingtry_to_delete_messagefor message deletion and includingsend_resultin thesavefunction are good practices for error handling and effective communication.
| async def try_to_delete_message(context, chat_id, message_id): | ||
| try: | ||
| await context.bot.delete_message(chat_id=chat_id, message_id=message_id) | ||
| except Exception as e: | ||
| return |
There was a problem hiding this comment.
Consider logging exceptions in try_to_delete_message to aid in debugging.
except Exception as e:
+ logging.error(f"Failed to delete message: {e}")
returnCommittable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.
| async def try_to_delete_message(context, chat_id, message_id): | |
| try: | |
| await context.bot.delete_message(chat_id=chat_id, message_id=message_id) | |
| except Exception as e: | |
| return | |
| async def try_to_delete_message(context, chat_id, message_id): | |
| try: | |
| await context.bot.delete_message(chat_id=chat_id, message_id=message_id) | |
| except Exception as e: | |
| logging.error(f"Failed to delete message: {e}") | |
| return |
Summary by CodeRabbit
commandslist within theAppConfigclass for enhanced bot functionality.creator_usernamefield in the poll data model.