-
-
Notifications
You must be signed in to change notification settings - Fork 105
Feat/help thread stats #1361
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Feat/help thread stats #1361
Conversation
replaces fetching metrics directly from database instead of discord, uses embed for showcases stats and making duration as optional choice in terms of days
spotless fixes, helper for calculating duration in seconds between datetime fields to be more compliant with code quality tests
change emojis to unicode character constants with descriptive names, single/blank line of space to be more intentional and clear
tj-wazei
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is fine, just some small changes to the imports.
application/src/main/java/org/togetherjava/tjbot/features/Features.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Outdated
Show resolved
Hide resolved
SquidXTV
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good job!
I kinda abused some review comments for future ideas and questions.
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Outdated
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Show resolved
Hide resolved
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Show resolved
Hide resolved
…/HelpThreadStatsCommand.java use a more simpler way to get days and a fallback if user entered option is missing Co-authored-by: Connor Schweighöfer <[email protected]>
since when saving thread meta data, participant count excludes author which means thread with 0 participants should be counted as ghost threads instead of single participant
to avoid users from spamming, this adds a cooldown period of a min per channel
application/src/main/java/org/togetherjava/tjbot/features/help/HelpThreadStatsCommand.java
Outdated
Show resolved
Hide resolved
uses Duration instance for expiration of cache items instead of seperate values for ease of maintenance and clarity
| private static final String EMOJI_CHART = "\uD83D\uDCCA"; | ||
| private static final String EMOJI_MEMO = "\uD83D\uDCDD"; | ||
| private static final String EMOJI_SPEECH_BUBBLE = "\uD83D\uDCAC"; | ||
| private static final String EMOJI_LABEL = "\uD83C\uDFF7\uFE0F"; | ||
| private static final String EMOJI_LIGHTNING = "\u26A1"; | ||
|
|
||
| private static final String EMBED_BLANK_LINE = "\u200B"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please prefer having the actual UTF-8 emoji characters here, so the source code is readable.
| } | ||
|
|
||
| @Override | ||
| public void onSlashCommand(SlashCommandInteractionEvent event) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that method is quite long. think about breaking it into maybe 2 or 3 helper methods? looks like there is a clear structure with: handle cooldown, database management, create and send response
| if (rawResRate >= 70) | ||
| return Color.GREEN; | ||
| if (rawResRate >= 30) | ||
| return Color.YELLOW; | ||
| if (rawResRate >= 0) | ||
| return Color.RED; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
what is Res? improve name
continues work of #1159 with /help-stats slash command, refactors to fetch stats from database instead of discord APIs. Also adds stats to embed for better UX.
embed color has three states
edited: also adds per channel limit, can be used once per minute.