tag qol #602

illuminator3 · 2022-10-03T22:01:09Z

In BotCommandAdapter the Interactor's name was used without prefix, but to forward from the button the prefix is needed.

…nto feature/tag-qol

Zabuzard · 2022-10-04T08:22:48Z

Im confused. What is this? Is that for #594 ? If so, link it please. Also, that is already assigned to @Taz03 .

Taz03 · 2022-10-04T08:36:37Z

Im confused. What is this? Is that for #594 ? If so, link it please. Also, that is already assigned to @Taz03 .

its a little different, the auto-complete provides option before the command usage, but lets say the user still writes and invalid id (which is possible) then the reply will contain some buttons for possible suggestions

Zabuzard · 2022-10-04T08:45:24Z

i see. fair.

application/src/main/java/org/togetherjava/tjbot/commands/system/BotCore.java

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

application/src/main/java/org/togetherjava/tjbot/commands/BotCommandAdapter.java

application/src/test/java/org/togetherjava/tjbot/commands/SlashCommandAdapterTest.java

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagCommand.java

sonarqubecloud · 2022-10-04T16:06:11Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

Zabuzard · 2022-10-05T06:54:15Z

application/src/main/java/org/togetherjava/tjbot/commands/BotCommandAdapter.java

+    /**
+     * @return the component id generator used by {@link #generateComponentId(String...)} and
+     *         {@link #generateComponentId(Lifespan, String...)}
+     */


This is a core API class, it needs outstanding and full Javadoc.

Maybe write:

/** * Gets the generator used to create component IDs. * <p> * In general, prefer using {@link ...} and {@link ...} instead of interacting with the generator directly. * * @return the generator */

application/src/main/java/org/togetherjava/tjbot/commands/BotCommandAdapter.java

Zabuzard · 2022-10-05T06:59:03Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagCommand.java

+    }
+
+    /**
+     * Sends the reply for a successfull /tag use (i.e. the given tag exists)


missing dot

Zabuzard · 2022-10-05T07:01:59Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagCommand.java

+    private void sendTagReply(IReplyCallback callback, String userName, String tag,
+            @Nullable String commandString, @Nullable String replyToUser) {


I have some trouble with some variable names. Some of them dont really say what they are about.

callback - callback of what? I think it would be easier and more idiomatic if you just call it event (even if the type is a level higher).

its easy to confuse the two users that play a role here, since both are called "user". maybe keep userName, but rename replyToUser to replyTargetUser. if u want, rename userName to invokerUserName maybe, then its super clear.

Zabuzard · 2022-10-05T07:02:55Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagCommand.java

-        }
-        message.queue();
+                .build())
+            .setContent(replyToUserOpt.orElse(""))


dont know the javadoc of setContent, but many JDA methods accept null. If thats the case, this could be a nice improvement, since u dont need the optional anymore then

Zabuzard · 2022-10-05T07:08:24Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

+                .comparingInt(candidate -> StringDistances.editDistance(id, candidate)))
+            .limit(TAG_SUGGESTIONS_AMOUNT)
+            .toList();
+        List<List<String>> partition = ListUtils.partition(candidates, 5);


move that down a few lines, to where its used first

Zabuzard · 2022-10-05T07:08:49Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

+                                    : ", did you perhaps mean any of the following?"))
+                    .setEphemeral(true);
+
+        for (List<String> part : partition) {


naming issue: partition and partitions, would be easier to understand.

Or batch and batches

Zabuzard · 2022-10-05T07:09:23Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

+
+        for (List<String> part : partition) {
+            action.addActionRow(part.stream()
+                .map(i -> Button.secondary(componentIdGenerator.generate(new ComponentId(


i is confusing, sounds like an index, but its actually the suggestion. please write it out

Zabuzard · 2022-10-05T07:10:25Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

+                .map(i -> Button.secondary(componentIdGenerator.generate(new ComponentId(
+                        UserInteractorPrefix.getPrefixedNameFromClass(TagCommand.class, "tag"),
+                        Arrays.asList(i,
+                                userToReplyTo != null ? userToReplyTo.getAsUser().getAsMention()
+                                        : null)),
+                        Lifespan.REGULAR), i))


This whole thing reads a bit complex, being nested in the map and loop. Id move this button creation out to a quick private helper method. sth like map(this::createSuggestionButton) or similar.

Zabuzard · 2022-10-05T07:11:50Z

application/src/main/java/org/togetherjava/tjbot/commands/tags/TagSystem.java

+        for (List<String> part : partition) {
+            action.addActionRow(part.stream()
+                .map(i -> Button.secondary(componentIdGenerator.generate(new ComponentId(
+                        UserInteractorPrefix.getPrefixedNameFromClass(TagCommand.class, "tag"),


the usage of "tag" here is a bit dangerous. if someone renames the command, they will for sure forget to adjust it here as well.

try to link it somehow. either by giving it as method argument (like u did with the component ID genererator), or by having a constant between here and TagCommand.

Zabuzard · 2022-10-06T07:09:32Z

application/src/main/java/org/togetherjava/tjbot/commands/BotCommandAdapter.java

+     * @return the generator
     */
-    protected final ComponentIdGenerator getComponentIdGenerator() {
+    protected final @Nullable ComponentIdGenerator getComponentIdGenerator() {


the method is not supposed to return a nullable.

its a mistake to call the method before intiialization, not allowed. the method should fail-fast if its called before.

should probably also be added to its javadoc. thanks

Taz03 · 2023-01-07T11:49:38Z

this pr is not valid anymore, we use auto-complete feature now, and while using this feature a user can only put the registered option, u can close this pr now

illuminator3 · 2023-01-07T22:34:38Z

thank you for appreciating all of the work I put into this already

Tais993 · 2023-01-09T04:50:55Z

this pr is not valid anymore, we use auto-complete feature now, and while using this feature a user can only put the registered option, u can close this pr now

This is false, the user can still put any text that they want, autocomplete doesn't force anything.

ankitsmt211 · 2024-05-18T09:43:27Z

This PR is being closed due to inactivity, can be opened later if need be.

Tais993 and others added 4 commits October 3, 2022 22:50

Bugfix

4ff1dc1

In BotCommandAdapter the Interactor's name was used without prefix, but to forward from the button the prefix is needed.

for tijs

53ed3f0

Fixed tests not working

a2c7a63

yes

4632190

illuminator3 requested review from a team as code owners October 3, 2022 22:01

illuminator3 requested review from Tais993, Zabuzard and marko-radosavljevic October 3, 2022 22:01

illuminator3 self-assigned this Oct 3, 2022

illuminator3 added enhance command Modify or improve an existing command or group of commands of the bot priority: low valid This issue/PR is validated and ready to be picked. This auto adds items to TJ project board. labels Oct 3, 2022

illuminator3 added 2 commits October 4, 2022 00:05

Merge remote-tracking branch 'origin/bugfix/components-not-working' i…

82725aa

…nto feature/tag-qol

why don't you just fix the imports for me sonar

f45bf2e

Zabuzard mentioned this pull request Oct 4, 2022

added auto-complete for tag command #598

Merged

fix tests

c6885a2

Zabuzard requested changes Oct 4, 2022

View reviewed changes

illuminator3 added 3 commits October 4, 2022 13:32

Merge branch 'develop' into feature/tag-qol

d3e9e5d

fix compilation & requested changes

1217a4e

stuff

f0ac722

illuminator3 requested a review from Zabuzard October 4, 2022 15:51

illuminator3 added 2 commits October 4, 2022 17:52

remove unused import

0e6e8fc

fix imports

9cfdc39

Zabuzard requested changes Oct 5, 2022

View reviewed changes

requested changes

e137e05

spotless

94d1353

Zabuzard requested changes Oct 6, 2022

View reviewed changes

Zabuzard force-pushed the develop branch from 3fecdae to ec8f08c Compare December 12, 2022 14:08

Zabuzard added stale and removed valid This issue/PR is validated and ready to be picked. This auto adds items to TJ project board. labels Mar 6, 2023

github-actions bot added the inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later label Mar 12, 2023

github-actions bot closed this Mar 12, 2023

illuminator3 added valid This issue/PR is validated and ready to be picked. This auto adds items to TJ project board. and removed inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later labels Mar 12, 2023

illuminator3 reopened this Mar 12, 2023

github-actions bot removed the stale label Mar 13, 2023

ankitsmt211 mentioned this pull request Nov 20, 2023

tag qol alternative? #958

Closed

ankitsmt211 added inactivity-closed Issues that have been closed due to inactivity, but are otherwise valid and might be reopened later and removed valid This issue/PR is validated and ready to be picked. This auto adds items to TJ project board. labels May 18, 2024

ankitsmt211 closed this May 18, 2024

Zabuzard deleted the feature/tag-qol branch August 6, 2025 06:17

		private void sendTagReply(IReplyCallback callback, String userName, String tag,
		@Nullable String commandString, @Nullable String replyToUser) {

Uh oh!

tag qol #602

tag qol #602

Uh oh!

Conversation

illuminator3 commented Oct 3, 2022

Uh oh!

Zabuzard commented Oct 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Taz03 commented Oct 4, 2022

Uh oh!

Zabuzard commented Oct 4, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sonarqubecloud bot commented Oct 4, 2022

Uh oh!

Choose a reason for hiding this comment

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!

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!

Zabuzard Oct 6, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Taz03 commented Jan 7, 2023

Uh oh!

illuminator3 commented Jan 7, 2023

Uh oh!

Tais993 commented Jan 9, 2023

Uh oh!

ankitsmt211 commented May 18, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

Zabuzard commented Oct 4, 2022 •

edited

Loading

Zabuzard Oct 6, 2022 •

edited

Loading