Conversation
application/config.json.template
Outdated
| } | ||
| "starboard": { | ||
| "emojiNames" : ["star"], | ||
| "channelName": "starboard" |
There was a problem hiding this comment.
channelNamePattern please (and in the code Pattern.compile(...) then, matching the channel)
There was a problem hiding this comment.
In that case, I would only have one pattern and not an array right? The design you are referring to I presume is the one where if the user wants multiple emojis they would need to seperate it by a pipe?
There was a problem hiding this comment.
the emoij names can stay non-pattern like that. but the channel name should be a pattern, so that we have some flexibility in naming it.
| * Gets the config for the Starboard Contains the List of emoji names recognized by the | ||
| * starboard as well as the name of the channel with the starboard. |
There was a problem hiding this comment.
these details are outdated extremely quick, cause people editing the config wont notice that its duplicated here. get rid of that. instead, describe what a starboard is.
| import java.util.Objects; | ||
|
|
||
| @JsonRootName("starboard") | ||
| public final class StarboardConfig { |
| private final String channelName; | ||
|
|
||
| @JsonCreator(mode = JsonCreator.Mode.PROPERTIES) | ||
| public StarboardConfig( |
| * Gets the name of the channel with the starboard Deactivate by using a non-existent channel | ||
| * name | ||
| * | ||
| * @return the name of the channel with the starboard | ||
| */ | ||
| public String getChannelName() { | ||
| return channelName; | ||
| } |
There was a problem hiding this comment.
(pattern, see earlier)
missing dot in the first and second sentence.
| if (database.read(context -> context.selectFrom(STARBOARD_MESSAGES) | ||
| .fetch("message_id") | ||
| .contains(event.getMessageIdLong()))) { |
There was a problem hiding this comment.
too complex for a if, put it into a helper method
| .queue(); | ||
| } | ||
|
|
||
| private boolean ignoreMessage(String emojiName, Guild guild, GuildChannel channel) { |
There was a problem hiding this comment.
bad method name. its missing a shouldXXX, otherwise it sounds as if this method, when called, will ignore the message
| User author = message.getAuthor(); | ||
| return new EmbedBuilder().setAuthor(author.getName(), null, author.getAvatarUrl()) | ||
| .setDescription(message.getContentDisplay()) | ||
| .build(); // TODO make footer with link and reacted emojis |
There was a problem hiding this comment.
Sonar and CodeQL won't let me forget it. :)
| CREATE TABLE starboard_messages | ||
| (message_id BIGINT NOT NULL PRIMARY KEY | ||
| ) |
There was a problem hiding this comment.
formatting looks a bit weird with the (
| if (database.read(context -> context.selectFrom(STARBOARD_MESSAGES) | ||
| .fetch("message_id") | ||
| .contains(event.getMessageIdLong()))) { | ||
| database.write(context -> context.newRecord(STARBOARD_MESSAGES) | ||
| .setMessageId(event.getMessageIdLong())); | ||
| } |
There was a problem hiding this comment.
whats the point of the database entry? its not used anywhere right now. id guess u want to check whether it exists to not duplicate-post? but that check seems to be missing right now.
also, u will have to additionally put an in-memory cache as layer above the DB to prevent checking the DB a million times in 5 seconds when someone posts an announcement. u can look in some of the other classes for that, search for Caffeine(cache).
There was a problem hiding this comment.
The database is being used for the check, that is what the if is for. However the current approach seems to be insanely slow as I fetch the entire list of entries.
I'll look into it.
There was a problem hiding this comment.
i think your missing my point on the question. what is the point of the database entry? it does not control any logic right now.
you have:
if (database doesnt contain it) {
put it into database
}but thats it. what point does it serve, sitting in the db? theres nothing in ur code that would control anything based on it.
u likely intended to stop transfering the message if its contained in the db, i.e.
if (database does not contain it) {
copy message to starboard
}but u dont have that. you always copy the message to the starboard, regardless of database
There was a problem hiding this comment.
Oh oops. Ur right mb 😅
5267e3f to
9e129af
Compare
|
Uh oh. |
|
Issue seems to be resolved after fixing merge conflicts. Weird but I’ll take it. |
| private static MessageEmbed formEmbed(Message message) { | ||
| User author = message.getAuthor(); | ||
| return new EmbedBuilder().setAuthor(author.getName(), null, author.getAvatarUrl()) | ||
| .setDescription(message.getContentDisplay()) | ||
| .appendDescription(" [Link](%s)".formatted(message.getJumpUrl())) | ||
| .build(); | ||
| } |
There was a problem hiding this comment.
can u also check for attachments and add that in the embed
There was a problem hiding this comment.
I set the image of the embed as the first image file in the attachments, if any.
|
SonarCloud Quality Gate failed.
|
aef2361 to
9a43a3a
Compare
|
39d1e71 to
2127d4f
Compare
|
|
This PR has been superceeded by #1029 and will be closed upon merge of that. |












This feature adds a Starboard.
Certain emojis which when added as a reaction to a message in a channel which everyone has permissions to, will be sent as an embed in the starboard channel.
Both the emojis and the channel are customizable through
config.json