Oofs and Lmaos Starboard #798

java-coding-prodigy · 2023-03-07T10:37:55Z

Completes #781

application/config.json.template

application/src/main/java/org/togetherjava/tjbot/config/OofsAndLmaosConfig.java

application/src/main/java/org/togetherjava/tjbot/features/basic/OofsAndLmaosStarboard.java

application/config.json.template

application/src/main/java/org/togetherjava/tjbot/config/Config.java

Zabuzard · 2023-03-08T09:21:58Z

application/src/main/java/org/togetherjava/tjbot/config/StarboardConfig.java

+@JsonRootName("starboard")
+public final class StarboardConfig {
+    private final String oofEmojiName;
+    private final String lmaoEmojiName;
+    private final long starboardChannelId;
+
+    @JsonCreator(mode = JsonCreator.Mode.PROPERTIES)
+    public StarboardConfig(
+            @JsonProperty(value = "oofEmojiName", required = true) String oofEmojiName,
+            @JsonProperty(value = "lmaoEmojiName", required = true) String lmaoEmojiName,
+            @JsonProperty(value = "starboardChannelId", required = true) long starboardChannelId) {
+        this.oofEmojiName = oofEmojiName;
+        this.lmaoEmojiName = lmaoEmojiName;
+        this.starboardChannelId = starboardChannelId;
+    }
+
+    public String getOofEmojiName() {
+        return oofEmojiName;
+    }
+
+    public String getLmaoEmojiName() {
+        return lmaoEmojiName;
+    }
+
+    public long getStarboardChannelId() {
+        return starboardChannelId;
+    }
+}


missing javadoc

application/config.json.template

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

Zabuzard · 2023-03-10T12:36:53Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

-
+        Guild guild = event.getGuild();
+        GuildChannel channel = event.getGuildChannel();
+        if (!config.getEmojiNames().contains(emojiName) || !guild.getPublicRole().hasPermission(channel, Permission.VIEW_CHANNEL)) {


i feel like this either needs a comment or should be moved into a well named method, so that its clear why we do this check

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

Zabuzard · 2023-03-13T10:30:45Z

application/src/main/java/org/togetherjava/tjbot/config/Config.java

+    /**
+     * Gets the config for the Starboard
+     *
+     * @return the config of the Starboard
+     * */


shitty javadoc. improve. explain the feature. how would u explain ur grandma what the starboard feature does? or someone new to the project.

That's exactly what we have used for getHelpSystemConfig so I thought I should do something similar, nevertheless, I will change it.

then that javadoc is bad as well (im aware that i wrote it 😆)

application/src/main/java/org/togetherjava/tjbot/config/StarboardConfig.java

application/config.json.template

java-coding-prodigy · 2023-03-21T10:31:21Z

Sonar lint fails because StarboardConfig can be a record.
Should i suppress the warning? Or maybe actually convert it to a record?

Zabuzard · 2023-03-21T10:36:28Z

That is because you are not yet checking whether the constructor args are non-null. See the other configs and follow the pattern.

github-actions · 2023-04-23T07:07:15Z

This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

java-coding-prodigy · 2023-04-23T07:28:36Z

I have been out of town since a few weeks that's why I couldn't contribute, I will resume working on this in a few days.
I am on mobile so I cannot remove the stale label .

Tais993 · 2023-05-17T09:23:44Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

+        Optional<TextChannel> starboardChannel =
+                guild.getTextChannelsByName(config.getChannelName(), false).stream().findFirst();


I know this is a minor thing, but personally I feel like it's much nicer to make helper methods for something like this.

Especially when classes get bigger, reading getStarBoardChannel I find much nicer.

So you don't have to change it, but maybe think about it next time, what you prefer.

Tais993 · 2023-05-17T12:10:15Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

+        event.getChannel()
+            .retrieveMessageById(event.getMessageId())


Suggested change

event.getChannel()

.retrieveMessageById(event.getMessageId())

event.retrieveMessage()

Tais993 · 2023-05-17T12:10:56Z

application/src/main/java/org/togetherjava/tjbot/config/StarboardConfig.java

+     * 
+     * @return the name of the channel with the starboard
+     */
+


Suggested change

Tais993 · 2023-05-17T12:11:46Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

+
+    @Override
+    public void onMessageReactionAdd(@NotNull MessageReactionAddEvent event) {
+        String emojiName = event.getReaction().getEmoji().asCustom().getName();


Suggested change

String emojiName = event.getReaction().getEmoji().asCustom().getName();

String emojiName = event.getEmoji().asCustom().getName();

Tais993 · 2023-05-17T12:13:34Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

+        GuildChannel channel = event.getGuildChannel();
+        if (!config.getEmojiNames().contains(emojiName)


everything feels to cramped, think I mentioned this before.

Being able to distinguish sections is much harder if it's a long lap of text.

Tais993 · 2023-05-17T12:21:17Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

+        User author = message.getAuthor();
+        return new EmbedBuilder().setAuthor(author.getName(), null, author.getAvatarUrl())
+            .setDescription(message.getContentDisplay())
+            .build(); // Maybe set footer as reacted emoji?


I'd set the footer as the reacted emoji, and maybe just add timestamp? Always nice

And are you linking the original message? Doesn't seem to be the case, I'd certainly do that.
Either add the link to the footer (like the original starboard), or add a button or something.

Tais993 · 2023-05-19T11:34:21Z

application/src/main/java/org/togetherjava/tjbot/features/basic/Starboard.java

            .queue();
    }

+    private boolean ignoreMessage(String emojiName, Guild guild, GuildChannel channel) {


Suggested change

private boolean ignoreMessage(String emojiName, Guild guild, GuildChannel channel) {

private boolean shouldIgnoreMessage(String emojiName, Guild guild, GuildChannel channel) {

github-actions · 2023-06-19T07:08:45Z

This pull request is stale because it has been open 30 days with no activity. Remove stale label, comment or add the valid label or this will be closed in 5 days.

java-coding-prodigy requested review from a team as code owners March 7, 2023 10:37

java-coding-prodigy self-assigned this Mar 7, 2023

java-coding-prodigy added enhancement New feature or request priority: low labels Mar 7, 2023

Zabuzard marked this pull request as draft March 7, 2023 10:41

java-coding-prodigy requested a review from Zabuzard March 7, 2023 11:04

Zabuzard requested changes Mar 7, 2023

View reviewed changes

Zabuzard requested changes Mar 8, 2023

View reviewed changes

Zabuzard requested changes Mar 10, 2023

View reviewed changes

Zabuzard requested changes Mar 13, 2023

View reviewed changes

Zabuzard requested changes Mar 14, 2023

View reviewed changes

application/config.json.template Outdated Show resolved Hide resolved

github-actions bot added the stale label Apr 23, 2023

Zabuzard removed the stale label Apr 23, 2023

java-coding-prodigy added 12 commits May 1, 2023 15:30

Set up config and create reaction listener

bbee8b2

Refactor class names

199484a

Add \@JsonProperty

b34b074

Remove :

0e369c7

config changes

ce45662

Some stuff

b4cae21

config fix

5079521

log

dc1ba36

added docs

a2ec12f

Make embed

0ec514d

Add feature

cb10c88

json fix

fe83510

java-coding-prodigy added 6 commits May 1, 2023 15:32

config changes

edd9b50

Replace channel name with Starboard

4922242

forgot sptoless

9564d4c

Changes from CR

ad41f3e

Null check

e2b7ee6

Spotless + Fix merge conflict

1ae7a61

java-coding-prodigy force-pushed the oofs_and_lmaos branch from 2042867 to 1ae7a61 Compare May 1, 2023 10:06

java-coding-prodigy marked this pull request as ready for review May 1, 2023 10:13

Tais993 requested changes May 17, 2023

View reviewed changes

Applied changes as per CR

86ceba8

Tais993 requested changes May 19, 2023

View reviewed changes

github-actions bot added stale inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later labels Jun 19, 2023

github-actions bot closed this Jun 25, 2023

		Optional<TextChannel> starboardChannel =
		guild.getTextChannelsByName(config.getChannelName(), false).stream().findFirst();

		event.getChannel()
		.retrieveMessageById(event.getMessageId())

	event.getChannel()
	.retrieveMessageById(event.getMessageId())
	event.retrieveMessage()

	String emojiName = event.getReaction().getEmoji().asCustom().getName();
	String emojiName = event.getEmoji().asCustom().getName();

		GuildChannel channel = event.getGuildChannel();
		if (!config.getEmojiNames().contains(emojiName)

	private boolean ignoreMessage(String emojiName, Guild guild, GuildChannel channel) {
	private boolean shouldIgnoreMessage(String emojiName, Guild guild, GuildChannel channel) {

Uh oh!

Oofs and Lmaos Starboard #798

Oofs and Lmaos Starboard #798

Uh oh!

Conversation

java-coding-prodigy commented Mar 7, 2023

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

java-coding-prodigy commented Mar 21, 2023

Uh oh!

Zabuzard commented Mar 21, 2023

Uh oh!

github-actions bot commented Apr 23, 2023

Uh oh!

java-coding-prodigy commented Apr 23, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

github-actions bot commented Jun 19, 2023

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants