Conversation
PR Analysis
PR Feedback
How to use
|
PR TitleRefactor bot modules PR TypeRefactoring PR DescriptionThis PR is a major refactor of the bot's codebase. It introduces a new structure for the bot's commands and functionalities, grouping them into separate modules. The main changes include the creation of a new 'UntappdModule' for handling Untappd-related commands and functionalities, and the refactoring of existing commands into this new structure. The PR also includes changes to the bot's dependencies and the removal of some unused files. PR Main Files Walkthrough- |
|
@CodiumAI-Agent /improve |
| ...UntappdModule().commands, | ||
| ]; | ||
|
|
||
| static Future<void> _helpCommand(ISlashCommandInteractionEvent ctx) async { | ||
| var helpMessage = MessageBuilder() | ||
| ..append(ctx.interaction.userAuthor!.mention) | ||
| ..appendNewLine() | ||
| ..append('Did anyone say beer? This is what I can do for you:') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/oel') | ||
| ..appendNewLine() | ||
| ..append('Lists all known beer releases.') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/subscribe') | ||
| ..appendNewLine() | ||
| ..append( | ||
| 'Subscribe to automatic beer release reminders. Reminders will be posted 3 times during the day before release.') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/release YYYY-MM-dd') | ||
| ..appendNewLine() | ||
| ..append( | ||
| 'Posts the beer release for given date in the format YYYY-MM-dd. e.g ') | ||
| ..appendItalics('/release 1970-01-30') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/help') | ||
| ..append(_mainHelpMessage) | ||
| ..appendNewLine() | ||
| ..append('Shows you this help message.') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/untappd untappd_username') | ||
| ..appendNewLine() | ||
| ..append( | ||
| 'Let me know your untappd username so I can post automatic updates from your untappd account. e.g ') | ||
| ..appendItalics('/untappd cornholio') | ||
| ..appendNewLine() | ||
| ..appendNewLine() | ||
| ..appendBold('/setup') | ||
| ..appendNewLine() | ||
| ..append( | ||
| 'Setup the bot to post untappd updates to the current channel. Only admins can issue this command. Also, this is needed before any untappd updates can occur.'); | ||
| ..append(UntappdModule().helpMessage); |
There was a problem hiding this comment.
Suggestion: Avoid creating multiple instances of UntappdModule in the code. Instead, create a single instance and reuse it. This will improve performance and memory usage.
| ...UntappdModule().commands, | |
| ]; | |
| static Future<void> _helpCommand(ISlashCommandInteractionEvent ctx) async { | |
| var helpMessage = MessageBuilder() | |
| ..append(ctx.interaction.userAuthor!.mention) | |
| ..appendNewLine() | |
| ..append('Did anyone say beer? This is what I can do for you:') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/oel') | |
| ..appendNewLine() | |
| ..append('Lists all known beer releases.') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/subscribe') | |
| ..appendNewLine() | |
| ..append( | |
| 'Subscribe to automatic beer release reminders. Reminders will be posted 3 times during the day before release.') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/release YYYY-MM-dd') | |
| ..appendNewLine() | |
| ..append( | |
| 'Posts the beer release for given date in the format YYYY-MM-dd. e.g ') | |
| ..appendItalics('/release 1970-01-30') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/help') | |
| ..append(_mainHelpMessage) | |
| ..appendNewLine() | |
| ..append('Shows you this help message.') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/untappd untappd_username') | |
| ..appendNewLine() | |
| ..append( | |
| 'Let me know your untappd username so I can post automatic updates from your untappd account. e.g ') | |
| ..appendItalics('/untappd cornholio') | |
| ..appendNewLine() | |
| ..appendNewLine() | |
| ..appendBold('/setup') | |
| ..appendNewLine() | |
| ..append( | |
| 'Setup the bot to post untappd updates to the current channel. Only admins can issue this command. Also, this is needed before any untappd updates can occur.'); | |
| ..append(UntappdModule().helpMessage); | |
| var untappdModule = UntappdModule(); | |
| ... | |
| untappdModule.commands, | |
| ... | |
| untappdModule.helpMessage |
| @override | ||
| void init(INyxxWebsocket bot, | ||
| {Duration updateInterval = const Duration(minutes: 12)}) { | ||
| // Set up Hive for local data storage | ||
| Hive.init('/data'); | ||
| Hive.openBox(HiveConstants.untappdBox); | ||
|
|
||
| _bot = bot; | ||
|
|
||
| // Start timer to check for untappd updates | ||
| Timer.periodic(updateInterval, (timer) => _checkUntappd()); | ||
|
|
||
| // Set module as initialized | ||
| _isInitialized = true; |
There was a problem hiding this comment.
Suggestion: Use dependency injection to provide the INyxxWebsocket bot to the UntappdModule instead of setting it in the init method. This will make the code more testable and modular.
| @override | |
| void init(INyxxWebsocket bot, | |
| {Duration updateInterval = const Duration(minutes: 12)}) { | |
| // Set up Hive for local data storage | |
| Hive.init('/data'); | |
| Hive.openBox(HiveConstants.untappdBox); | |
| _bot = bot; | |
| // Start timer to check for untappd updates | |
| Timer.periodic(updateInterval, (timer) => _checkUntappd()); | |
| // Set module as initialized | |
| _isInitialized = true; | |
| UntappdModule(INyxxWebsocket bot) { | |
| ... | |
| _bot = bot; | |
| ... | |
| } |
| } catch (e) { | ||
| print(e.toString()); | ||
| } |
There was a problem hiding this comment.
Suggestion: Consider handling exceptions in a more user-friendly way. Instead of just printing the error, you could log it and show a user-friendly message to the user.
| } catch (e) { | |
| print(e.toString()); | |
| } | |
| catch (e) { | |
| Logger().e('Error while checking Untappd: $e'); | |
| throw UserFriendlyException('An error occurred while checking Untappd.'); | |
| } |
| void _checkUntappd() async { | ||
| if (!_isInitialized) { | ||
| print('Untappd module not initialized!'); | ||
| throw Exception('Untappd module not initialized!'); | ||
| } | ||
| var box = Hive.box(HiveConstants.untappdBox); | ||
|
|
||
| Map<dynamic, dynamic> listOfUsers = | ||
| await box.get(HiveConstants.untappdUserList, defaultValue: {}); | ||
| var latestCheckins = await box | ||
| .get(HiveConstants.untappdLatestUserCheckins, defaultValue: {}); | ||
|
|
||
| var updateChannelId = await box.get(HiveConstants.untappdUpdateChannelId); | ||
|
|
||
| if (updateChannelId == null) { | ||
| print('No channel available for updates!'); | ||
| return; | ||
| } | ||
|
|
||
| if (listOfUsers.isEmpty) print('No users available to scrape!'); | ||
|
|
||
| listOfUsers.forEach((userSnowflake, untappdUsername) async { | ||
| var latestCheckinDisk = latestCheckins[untappdUsername]; | ||
| try { | ||
| var latestCheckinUntappd = await _getLatestCheckin(untappdUsername); | ||
|
|
||
| // If a new ID is available, post update! | ||
| if (latestCheckinUntappd != null && | ||
| latestCheckinDisk != latestCheckinUntappd.id) { | ||
| // Update latest saved checkin | ||
| latestCheckins.addAll({untappdUsername: latestCheckinUntappd.id}); | ||
| await box.put( | ||
| HiveConstants.untappdLatestUserCheckins, latestCheckins); | ||
|
|
||
| // Build update message with info from untappd checkin | ||
| var user = await _bot.fetchUser(Snowflake(userSnowflake)); | ||
| var embedBuilder = EmbedBuilder(); | ||
| embedBuilder.title = '${user.username} is drinking beer!'; | ||
| embedBuilder.url = | ||
| _getCheckinUrl(latestCheckinUntappd.id, untappdUsername); | ||
| embedBuilder.description = latestCheckinUntappd.title; | ||
| if (latestCheckinUntappd.comment.isNotEmpty) { | ||
| embedBuilder.addField( | ||
| field: | ||
| EmbedFieldBuilder('Comment', latestCheckinUntappd.comment)); | ||
| } | ||
| if (latestCheckinUntappd.rating.isNotEmpty) { | ||
| embedBuilder.addField( | ||
| field: EmbedFieldBuilder( | ||
| 'Rating', | ||
| _buildRatingEmoji( | ||
| double.parse(latestCheckinUntappd.rating)))); | ||
| } | ||
| if (latestCheckinUntappd.photoAddress != null) { | ||
| embedBuilder.imageUrl = latestCheckinUntappd.photoAddress; | ||
| } | ||
|
|
||
| // Get channel used for untappd updates, previously set by discord admin. | ||
| var updateChannel = await _bot | ||
| .fetchChannel(Snowflake(updateChannelId)) | ||
| .then((value) => (value as ITextChannel)); | ||
|
|
||
| // Send update message | ||
| await updateChannel.sendMessage(MessageBuilder.embed(embedBuilder)); | ||
| } | ||
| // Sleep 5 seconds per user to avoid suspicious requests to untappd server | ||
| await Future.delayed(Duration(seconds: 5)); | ||
| } catch (e) { | ||
| print(e.toString()); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
Suggestion: Consider using a more descriptive name for the _checkUntappd method. The current name does not clearly indicate what the method does.
| void _checkUntappd() async { | |
| if (!_isInitialized) { | |
| print('Untappd module not initialized!'); | |
| throw Exception('Untappd module not initialized!'); | |
| } | |
| var box = Hive.box(HiveConstants.untappdBox); | |
| Map<dynamic, dynamic> listOfUsers = | |
| await box.get(HiveConstants.untappdUserList, defaultValue: {}); | |
| var latestCheckins = await box | |
| .get(HiveConstants.untappdLatestUserCheckins, defaultValue: {}); | |
| var updateChannelId = await box.get(HiveConstants.untappdUpdateChannelId); | |
| if (updateChannelId == null) { | |
| print('No channel available for updates!'); | |
| return; | |
| } | |
| if (listOfUsers.isEmpty) print('No users available to scrape!'); | |
| listOfUsers.forEach((userSnowflake, untappdUsername) async { | |
| var latestCheckinDisk = latestCheckins[untappdUsername]; | |
| try { | |
| var latestCheckinUntappd = await _getLatestCheckin(untappdUsername); | |
| // If a new ID is available, post update! | |
| if (latestCheckinUntappd != null && | |
| latestCheckinDisk != latestCheckinUntappd.id) { | |
| // Update latest saved checkin | |
| latestCheckins.addAll({untappdUsername: latestCheckinUntappd.id}); | |
| await box.put( | |
| HiveConstants.untappdLatestUserCheckins, latestCheckins); | |
| // Build update message with info from untappd checkin | |
| var user = await _bot.fetchUser(Snowflake(userSnowflake)); | |
| var embedBuilder = EmbedBuilder(); | |
| embedBuilder.title = '${user.username} is drinking beer!'; | |
| embedBuilder.url = | |
| _getCheckinUrl(latestCheckinUntappd.id, untappdUsername); | |
| embedBuilder.description = latestCheckinUntappd.title; | |
| if (latestCheckinUntappd.comment.isNotEmpty) { | |
| embedBuilder.addField( | |
| field: | |
| EmbedFieldBuilder('Comment', latestCheckinUntappd.comment)); | |
| } | |
| if (latestCheckinUntappd.rating.isNotEmpty) { | |
| embedBuilder.addField( | |
| field: EmbedFieldBuilder( | |
| 'Rating', | |
| _buildRatingEmoji( | |
| double.parse(latestCheckinUntappd.rating)))); | |
| } | |
| if (latestCheckinUntappd.photoAddress != null) { | |
| embedBuilder.imageUrl = latestCheckinUntappd.photoAddress; | |
| } | |
| // Get channel used for untappd updates, previously set by discord admin. | |
| var updateChannel = await _bot | |
| .fetchChannel(Snowflake(updateChannelId)) | |
| .then((value) => (value as ITextChannel)); | |
| // Send update message | |
| await updateChannel.sendMessage(MessageBuilder.embed(embedBuilder)); | |
| } | |
| // Sleep 5 seconds per user to avoid suspicious requests to untappd server | |
| await Future.delayed(Duration(seconds: 5)); | |
| } catch (e) { | |
| print(e.toString()); | |
| } | |
| }); | |
| } | |
| void _fetchAndPostUntappdUpdates() async { | |
| ... | |
| } |
Hampusheglert
left a comment
Hampusheglert
left a comment
There was a problem hiding this comment.
Nice job!
Slightly to big PR to actually give good feedback, but got some small inputs 👯
|
|
||
| late INyxxWebsocket _bot; | ||
|
|
||
| static final BeerAgentModule _singleton = BeerAgentModule._internal(); |
There was a problem hiding this comment.
Could use injectable package for singleton and to provide the INyxxWebsocket bot.
| Future<void> _updateSubscribers() async { | ||
| await _updateBeerSales(); | ||
|
|
||
| var myFile = File('sub.dat'); |
There was a problem hiding this comment.
Should this path be moved to a constants file?
| ..appendNewLine() | ||
| ..append('There is a fresh beer release tomorrow, ') | ||
| ..appendBold(DateFormat('yyyy-MM-dd').format(saleDate)) | ||
| ..append('. Bolaget opens 10:00') |
There was a problem hiding this comment.
Would be cool if this actually fetches the openings hours instead of hardcoding, in case of the opening hours actually changing to 9 and all the people in this discord misses the beer and it's already sold out when they arrive.
| } | ||
|
|
||
| /// Fetches a list of all beer sales from online API. | ||
| Future<Map<String, dynamic>> _fetchBeerList() async { |
|
|
||
| /// Returns a list of all channels (users) that are subscribed to beer updates. | ||
| Future<List<IDMChannel>> _getSubChannels(INyxxWebsocket bot) async { | ||
| var myFile = File('sub.dat'); |
There was a problem hiding this comment.
Again, this could be saved in a constant file to avoid bugs in the future.
| Future<void> _releaseCommand(ISlashCommandInteractionEvent ctx) async { | ||
| var input = ctx.args; | ||
| if (input.length == 1) { | ||
| var parsedDate = DateTime.tryParse(input[0].value); |
There was a problem hiding this comment.
could use "input.first.value"
it will throw stateerror if empty.
| ..append(ctx.interaction.userAuthor!.mention) | ||
| ..appendNewLine() | ||
| ..append(' :beers: ') | ||
| ..appendBold(input[0].value) |
| class BeerList { | ||
| final String saleDate; | ||
| final List<Beer> beerList; | ||
|
|
||
| BeerList(this.saleDate, this.beerList); | ||
| BeerList.fromJson(Map<String, dynamic> json) | ||
| : saleDate = json['first_sale'], | ||
| beerList = createListFromMap(json['items']); | ||
|
|
||
| static List<Beer> createListFromMap(List<dynamic> json) { | ||
| var toReturn = <Beer>[]; | ||
| for (var item in json) { | ||
| toReturn.add(Beer.fromJson(item)); | ||
| } | ||
| return toReturn; | ||
| } | ||
| } |
There was a problem hiding this comment.
freezed package could clean up this a bit
| String _buildRatingEmoji(double rating) { | ||
| var ratingString = ''; | ||
| for (var i = 0; i < rating.toInt(); i++) { | ||
| ratingString += ':beer: '; | ||
| } | ||
| return '$ratingString ($rating)'; | ||
| } |
There was a problem hiding this comment.
final ratingString = List.filled(rating.toInt(),':beer: ', growable: false).join();
| throw 'No checkins are available for $untappdUsername'; | ||
| } | ||
|
|
||
| var latestCheckin = checkins.first!; |
There was a problem hiding this comment.
bang bang, you shot me down... bang bang
PR Code Suggestions ✨
|
3 participants
No description provided.