Feat: Add slash command to generate application form for various community roles #1049

christolis · 2024-03-08T13:57:49Z

Closes #1024.

Screenshots

Configuration changes

Property	Description	Type	Default
`applicationForm.applicationChannelPattern`	Where submitted applications end up.	String	`"applications-log"`

TODO

Add a 5 minute cooldown per member for sending applications to prevent spam (feel free to recommend alternatives).
- This could be done by having a HashMap<Member, OffsetDateTime> where the key is the Member who sent an application and the value is the date and time that they sent it. There should be a condition every time a Member attempts to send any application which utilizes the aforementioned HashMap to prevent spam.
Add JavaDocs for every method where it's necessary.
~~Switch to EntitySelectInteractionEvent for the dropdown menu.~~ (EDIT: Impossible since this would forcibly include all existing roles)
Move selectable roles from the config to the command parameters in a similar fashion with previous commands.
- This eliminates emoji/description metadata as well that was previously defined in the configuration file.

Suleman70 · 2024-03-08T14:07:18Z

Can i be assigned to this?

Taz03

branch update is required, it is using removed APIs

christolis · 2024-03-19T22:17:24Z

Marked PR as draft since there needs to be some rework on certain things which will be specified in the TODO.

ankitsmt211 · 2024-05-19T01:37:43Z

@christolis what happens to the generated form after x user applies through form? Does the form need to be re-created? Or is it only unavailable for x user to a certain time period?

Also how do we update the form if need be?
Delete message and recreate?

christolis · 2024-10-06T12:42:25Z

@christolis what happens to the generated form after x user applies through form? Does the form need to be re-created? Or is it only unavailable for x user to a certain time period?

Also how do we update the form if need be? Delete message and recreate?

The generated form is persistent and used by everybody. It does not need to be recreated by whoever has the MANAGE_ROLES permission. As for updating it, that's right. Currently, the way it is developed, the message has to be deleted and get the command executed again, which is not that big of a problem considering the rarity of updating such form.

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

application/config.json.template

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

surajkumar

Great PR and the thought put into the development of the feature is evident.

Please can you look into the comments, additionally, there are some duplicate checks happening e.g. multiple guild == null checks. This can be done once much earlier and then you don't have to worry about it.

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

.../src/main/java/org/togetherjava/tjbot/features/roleapplication/ApplicationCreateCommand.java

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

tj-wazei

LGTM!

LovinLifee · 2025-06-27T18:58:12Z

Commenting so I can be assigned

Co-authored-by: Suraj Kumar <76599223+surajkumar@users.noreply.github.com>

This removes the permission check for the bot to have the `MANAGE_ROLES` permission in order to execute the command.

Certain configuration values pertaining to the application form creating and handling for custom roles do not need to be there and are deemed more appropriate as hardcoded values inside the code itself. Remove the configuration entries from the `config.json.template` as well as references to such keys in the codebase and introduce them as static constants in the appropriate class files. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Rename the command to match the package name that this class is located in. Primarily for organizational purposes and to not come up with differnt names every time we reference this feature. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Rename the handler to match the package name that this class is located in. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Rename the variable name of the class-wide `RoleApplicationSystemConfig` to `config` and the variable name of `RoleApplicationHandler` to `handler`, more simple and recognizable names. The original ones are too verbose and unnecessarily yield long horizontal lines of code where unnecessary. These verbose names do work but only in the situation where there are multiple configurations or handlers used in the same class. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

As it currently is, the helper method `generateRoleOptions` takes one `SlashCommandData` parameter and is used once throughout the entire project, specifically in the same class it is defined. In that one case, `generateRoleOptions` would take in `getData()` as its input, a method that is accessible through the entire class due to the simple fact that `CreateRoleApplicationCommand` inherits methods from `SlashCommandAdapter`. Modify the `generateRoleOptions` helper method to take no input and inside it, store a reference of `getData()` for the method to make use of. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Rename `VALUE_DELIMITER` to `OPTION_PARAM_ID_DELIMITER` and `ARG_COUNT` to `OPTIONS_PER_ROLE` for better and less ambiguous naming. The current ones do not accurately reflect what they are delimiting and where they should be used specifically, so we use more verbose names for that purpose. They are constants that aren't widely used anywhere in major places, though we should still take good care of all naming regardless for maximum code readability. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

In some part of the code, we have the `OPTIONS_PER_ROLE` value hardcoded as in return frequencyMap.values().stream().filter(value -> value != 3).count(); Replace the hardcoded 3 with the actual constant we have previously made so that it mirrors any potential changes we make in the future regarding this value (perhaps we decide to remove emojis as arguments and suddenly we find ourselves in a situation where the `OPTIONS_PER_ROLE` constant is now 2. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

There is currently the following message displayed in an embed to those who make use of the `/application-form` slash command: We are always looking for community members that want to contribute to our community and take charge. If you are interested, you can apply for various positions here! This is too hardened and serious according to feedback. Add a smiley face in the form of a cool sunglasses emoji to soften it up. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

The `selectWordFromCount` helper method, while serving a seemingly helpful purpose, is only used once and is also private to be used in the same class only. Besides the fact that we can technically move it somewhere more appropriate and increase its visibility so other classes can utilize it, remove the helepr method and resort to ternary operators for that one time we need to determine the plurarity of the word "minute" in our `CreateRoleApplicationCommand` class. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Move the `correctMinutesWord` string inside the if-statement below it due to the fact that that is the only place where that variable is being used. `remainingMinutes` is left intact in the same scope since it is needed as a reference in the if-statement itself to compare against zero. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Move the call chain order to be ordered in such a way so that the story telling is being told better and so that the code is more readable. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

Get a reference of `roleString` as early as possible, and be more explicit about which argument we want to get from the arguments list. At any given point (think, a future update to some library like JDA), the list we are getting all of our arguments from could end up looking from this: [1192015871509315, "Role String"] to: [1192015871509315, "Role String", 24] Therefore, making `args.getLast()` entirely obsolete. Being more explicit about the argument number we would like to get reduces the chances of something like this happening. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

There are currently many undesired conditions the code could run into when it comes to the `RoleApplicationHandler` which we would not desire and we would not be prepared for, particularly in situations where: - We would somehow find ourselves handling application form submissions in a non-guild environment. - The arguments size including the clicked component ID as well as the role name (the title of that component ID in essence) would have an unexpected size. - The staff-monitored application submissions text channel would not be found by the bot. We do not have entire control over these cases, but we need to handle them properly and set ourselves up for success in the rare case they happen. Make `sendApplicationResult` throw an `IllegalArgumentException` with descriptive messages if anything goes wrong, and log appropriate stacktraces if anything goes wrong in the logger for easy debugging. I thought about printing straight to the logger and not using any `IllegalArgumentException` in the code (or at least, using a different or maybe custom `Exception` but that would be too much), but with this method, we get to see custom messages along with exceptions for each individual case - all that without unnecessarily overcomplicating the code and sacrificing its readability to a considerable extent. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

@Zabuzard

The `RoleApplicationHandler` class is not making any use of inheritance as suggested by @Zabuzard and what's more, certain methods are unnecessarily marked as protected. Reduce visibility of methods marked as protected and make the `RoleApplicationHandler` class final. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

During the event `onStringSelectSelection` which would be called when someone would select one role in the application form, there would be a possibility for: - The `member` to be null (most likely from interacting in a non-guild environment, which is rare and almost impossible). - `event.getSelectedOptions()` to be an empty list, effectively meaning that a form was somehow made with no options and a user (or should we say member in our case) managed to interact with it. In any case, properly log errors in the logger in case either of these happen to be null or otherwise have unexpected values. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

christolis added enhancement New feature or request new command Add a new command or group of commands to the bot priority: major labels Mar 8, 2024

christolis self-assigned this Mar 8, 2024

SquidXTV assigned Suleman70 Mar 8, 2024

christolis marked this pull request as ready for review March 9, 2024 10:17

christolis requested a review from a team as a code owner March 9, 2024 10:17

Suleman70 removed their assignment Mar 16, 2024

Taz03 requested changes Mar 19, 2024

View reviewed changes

christolis force-pushed the feature/application-form branch from 5c0ef6e to 3159bed Compare March 19, 2024 21:28

christolis marked this pull request as draft March 19, 2024 22:16

christolis force-pushed the feature/application-form branch 2 times, most recently from cb7815a to 042a0e0 Compare April 4, 2024 09:48

christolis marked this pull request as ready for review April 4, 2024 22:12

christolis requested a review from Taz03 April 4, 2024 22:18

ankitsmt211 added the config-changes if your PR contains any changes related to config file label May 18, 2024

christolis force-pushed the feature/application-form branch from 1a8e458 to 29c8ee6 Compare October 6, 2024 11:39

SquidXTV requested changes Oct 14, 2024

View reviewed changes

surajkumar suggested changes Oct 14, 2024

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/features/basic/ApplicationCreateCommand.java Outdated Show resolved Hide resolved

application/src/main/java/org/togetherjava/tjbot/features/basic/ApplicationCreateCommand.java Outdated Show resolved Hide resolved

christolis force-pushed the feature/application-form branch 4 times, most recently from 0b18118 to a8178b9 Compare October 17, 2024 20:02

tj-wazei requested changes Oct 17, 2024

View reviewed changes

tj-wazei reviewed Oct 17, 2024

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/config/RoleApplicationSystemConfig.java Show resolved Hide resolved

tj-wazei previously approved these changes Oct 17, 2024

View reviewed changes

SquidXTV assigned LovinLifee Jun 27, 2025

christolis and others added 8 commits June 28, 2025 00:23

feat: add role application system config

6d8ceee

feat: create role application command

a42a428

Co-authored-by: Suraj Kumar <76599223+surajkumar@users.noreply.github.com>

fix(handle-permissions): remove unnecessary check

dcb241c

This removes the permission check for the bot to have the `MANAGE_ROLES` permission in order to execute the command.

refactor: make cooldown minutes check into separate method

375ae45

feat: move substring of label into config record

8651807

docs: rewrite documentation for RoleApplicationSystemConfig

0a04d90

style: run spotlessApply gradle task

8ab2f40

feat(cooldown): use the proper word depending on count

f46c983

christolis force-pushed the feature/application-form branch from a226d33 to f46c983 Compare June 27, 2025 21:24

Zabuzard added priority: normal and removed priority: major labels Jul 1, 2025

christolis added 14 commits July 1, 2025 15:45

refactor: rename to RoleApplicationHandler

ee96742

Rename the handler to match the package name that this class is located in. Suggested-by: Zabuzard <zabuza.dev@gmail.com> Signed-off-by: Chris Sdogkos <work@chris-sdogkos.com>

christolis dismissed SquidXTV’s stale review via c4aeb1a July 1, 2025 16:27

christolis requested a review from Zabuzard July 1, 2025 16:40

Uh oh!

Feat: Add slash command to generate application form for various community roles #1049

Are you sure you want to change the base?

Feat: Add slash command to generate application form for various community roles #1049

Uh oh!

Conversation

christolis commented Mar 8, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Screenshots

Configuration changes

TODO

Uh oh!

Suleman70 commented Mar 8, 2024

Uh oh!

Taz03 left a comment

Choose a reason for hiding this comment

Uh oh!

christolis commented Mar 19, 2024

Uh oh!

ankitsmt211 commented May 19, 2024

Uh oh!

christolis commented Oct 6, 2024

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

surajkumar left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tj-wazei left a comment

Choose a reason for hiding this comment

Uh oh!

LovinLifee commented Jun 27, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants

christolis commented Mar 8, 2024 •

edited

Loading