Forms system #524

Defective4 · 2025-09-10T11:36:47Z

This pull request adds the forms system to the bot.
The forms can be attached to any of the bot's messages. Users can fill in a form by clicking on the button added to the message the form is attached. After doing so, the user is presented with a modal dialog containing up to 5 fields defined in the form.
Upon submission, the contents of the fields are sent to a dedicated channel (configurable per form).

Included in this PR are commands to create, manage, and customize forms.
Forms can also be configured to accept only one submission per user. This was achieved by keeping track of who had submitted the form before in the dedicated forms repository.

Below is a video showcasing how the system works, along with most of the commands:
link

I'm aware this can be improved in some areas. I'm looking forward to your reviews!

A command to attach form buttons to bot's messages Form data deserialization Store forms in database Use an array of form fields instead of a form data object Move `FormsRepository` and `FormField` to different packages Reading forms from database Open modals on form button interaction Log submissions in a configured channel Allow setting a custom submission message Use an *Empty* value if user ommits a field Ignore case when parsing form field text input styles Store forms' origin messages, their channels, and expiration date Form delete command autocompletion Form delete command Forms closing and reopening commands Form details command Form modification command Check if the form is not closed or expired on submission Reword expiration command parameter description Additional checks in all form commands Don't accept raw JSON data and don't immediately attach forms Added `add-field` form command Form field remove command Form show command Put a limit of 5 components for forms Allow field inserting Forms attach and detach commands Prevent removing the last field from an attached form Javadocs Remove redundant checks from form commands Move some common methods to the form interaction manager Added `hasExpired` method to `FormData` Add `onetime` form parameter Logging form submissions to the database Add a repository method to query forms for a specific `closed` state Include total submissions count in the form details message Additional null checks in the forms repository Prevent further submissions to one-time forms A command to export all form submissions Delete form submissions on form deletion Command to delete user's submissions Fix checkstyle violations

danthe1st

This is my initial review which is about the model and database.

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

src/main/resources/database/migrations/09-08-2025_forms.sql

danthe1st · 2025-09-10T16:57:25Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormData.java

+		return array;
+	}
+
+	public long getExpiration() {


Wouldn't it make more sense to use an Instant or LocalDateTime for the expiration? This could also be represented in the DB using TIMESTAMP

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormField.java

danthe1st · 2025-09-10T17:00:18Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormField.java

+import net.dv8tion.jda.api.interactions.components.text.TextInputStyle;
+
+/**
+ * Represents a form field.


Maybe add a short descrion of what a form field is.

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormField.java

danthe1st · 2025-09-10T17:04:24Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormUser.java

+/**
+ * Represents an user who submitted a form.
+ */
+public class FormUser {


This could be a record as well (if it is even needed)

danthe1st

Round 2 of reviewing, FormInteractionManager, /form add-field, /form attach

danthe1st · 2025-09-10T18:42:14Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/commands/FormCommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand;
+
+/**
+ * The `/form` command.


Maybe add a bit of documentation explaining what that command does and mention the workflow there.

danthe1st · 2025-09-10T18:50:09Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;
+
+/**
+ * The `/form add-field` command.


For each of the subcommands, please make sure that the Javadoc at least contains a short sentence explaining what it does. e.g. The {@code /form add-field} command which adds a text input field to a {@link FormData form} (you might add an @see or similar to FormCommand if that contains basic information of the workflow).

Please also add information that only 5 fields are allowed per form.

danthe1st · 2025-09-10T18:52:18Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+						"Whether or not the user has to input data in this field. Default: false")
+				.addOption(OptionType.STRING, "style", "Input style. Default: SHORT", false, true)
+				.addOption(OptionType.STRING, "value", "Initial field value")
+				.addOption(OptionType.INTEGER, "index", "Index to insert the field at"));


I think that description could benefit from information that the form would be inserted at the end if no index is specified.

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

danthe1st · 2025-09-10T18:56:17Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+	}
+
+	@Override
+	public void execute(SlashCommandInteractionEvent event) {


Can you please add a check similar to the following to the form commands?

if (!Checks.hasStaffRole(botConfig, event.getMember())) { Responses.replyStaffOnly(event, botConfig.get(event.getGuild())).queue(); return; }

While Discord permissions should handle this, they have been a bit weird in the past and to be honest, I prefer having at least a simple check in the code.

Alright. There WAS such a check in place, but I decided it's useless since the command is only enabled for administrators by default.
I will make sure to add this back.

danthe1st · 2025-09-10T20:26:23Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/FormInteractionManager.java

+	}
+
+	/**
+	 * Gets expiration time from the slash comamnd event.


Please document the fact that this method already sends an error response when the parsing fails.

danthe1st · 2025-09-10T20:28:05Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/FormInteractionManager.java

+		return expiration;
+	}
+
+	private static boolean checkNotClosed(FormData data) {


isOpen would probably be a better name. Negations are confusing and check might imply that this could also send an error.

danthe1st · 2025-09-10T20:29:25Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/FormInteractionManager.java

+	private static MessageEmbed createSubmissionEmbed(FormData form, List<ModalMapping> values, Member author) {
+		EmbedBuilder builder = new EmbedBuilder().setTitle("New form submission received")
+				.setAuthor(author.getEffectiveName(), null, author.getEffectiveAvatarUrl()).setTimestamp(Instant.now());
+		builder.addField("Sender", author.getAsMention(), true).addField("Title", form.getTitle(), true);


I think it would be good to also add the author ID.

I assume you mean the author of the form?

I meant the user who submitted the form.

Now I see what you mean. Yeah, I'll add this

danthe1st · 2025-09-10T20:30:33Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/FormInteractionManager.java

+				.setAuthor(author.getEffectiveName(), null, author.getEffectiveAvatarUrl()).setTimestamp(Instant.now());
+		builder.addField("Sender", author.getAsMention(), true).addField("Title", form.getTitle(), true);
+
+		int len = Math.min(values.size(), form.getFields().size());


Does this work properly if there are optional fields before required fields? I assume empty optional fields will be present with a null value?

Close, they will be empty. It's actually briefly shown in my recording, although I probably should add a clear indicator that the user omitted the field.

danthe1st · 2025-09-10T20:33:56Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/FormInteractionManager.java

+		for (int i = 0; i < len; i++) {
+			ModalMapping mapping = values.get(i);
+			FormField field = form.getFields().get(i);
+			String value = mapping.getAsString();


I guess you might want to replace markdown codeblocks with something like

` ` `

or truncate them - or generally prohibit that?

That's a good point, I'll definitely do that

danthe1st

With this, most of the changed files should be reviewed.

TODO for myself: check with spotbugs

danthe1st · 2025-09-11T16:06:55Z

...n/java/net/discordjug/javabot/systems/staff_commands/forms/commands/CloseFormSubcommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;
+
+/**
+ * The `/form close` command.


Please mention what closing means here.

danthe1st · 2025-09-11T16:10:59Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+	public AddFieldFormSubcommand(FormsRepository formsRepo) {
+		this.formsRepo = formsRepo;
+		setCommandData(new SubcommandData("add-field", "Adds a field to an existing form")
+				.addOption(OptionType.INTEGER, "form-id", "Form ID to add the field to", true, true)


Also I guess it would be possible to use a constant for this.

danthe1st · 2025-09-11T16:12:07Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+	public void handleAutoComplete(CommandAutoCompleteInteractionEvent event, AutoCompleteQuery target) {
+		switch (target.getName()) {
+			case "form-id" -> event.replyChoices(
+					formsRepo.getAllForms().stream().map(form -> new Choice(form.toString(), form.getId())).toList())


Since you have the autocomplete for form-id in many almost all subcommands, it would make sense to extract that.

If it helps, you could also create a superclass with the common logic (including the code for retrieving the FormData and call an abstract method with the FormData that contains the actual command logic.

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

danthe1st · 2025-09-11T16:27:13Z

...ava/net/discordjug/javabot/systems/staff_commands/forms/commands/AddFieldFormSubcommand.java

+
+	@Override
+	public void execute(SlashCommandInteractionEvent event) {
+


Is there a reason why you have these empty lines at the beginning of the execute methods?

Yeah, there used to be the exact check you mentioned in #524 (comment), but I later decided to remove it from all subcommands by using the Replace tool, thus leaving empty lines in its place.

The common superclass I mentioned would be a good place for that check.

.../net/discordjug/javabot/systems/staff_commands/forms/commands/RemoveFieldFormSubcommand.java

danthe1st · 2025-09-11T17:28:42Z

.../net/discordjug/javabot/systems/staff_commands/forms/commands/RemoveFieldFormSubcommand.java

+						for (int i = 0; i < fields.size(); i++) {
+							choices.add(new Choice(fields.get(i).getLabel(), i));
+						}
+						event.replyChoices(choices).queue();


Again, reminder to use filterChoices.

danthe1st · 2025-09-11T17:29:23Z

.../java/net/discordjug/javabot/systems/staff_commands/forms/commands/ReopenFormSubcommand.java

+	 * @param botConfig          main bot configuration
+	 */
+	public ReopenFormSubcommand(FormsRepository formsRepo, FormInteractionManager interactionManager,
+			BotConfig botConfig) {


I think BotConfig isn't used here?

danthe1st · 2025-09-11T17:30:03Z

.../java/net/discordjug/javabot/systems/staff_commands/forms/commands/ReopenFormSubcommand.java

+			BotConfig botConfig) {
+		this.formsRepo = formsRepo;
+		this.interactionManager = interactionManager;
+		setCommandData(new SubcommandData("reopen", "Reopen a closed form").addOptions(


Maybe add that this allows further submissions

danthe1st · 2025-09-11T17:32:23Z

.../java/net/discordjug/javabot/systems/staff_commands/forms/commands/ReopenFormSubcommand.java

+		if (!form.isClosed()) {
+			event.reply("This form is already opened").setEphemeral(true).queue();
+			return;
+		}


Maybe also add an error if the form is expired? After all, there isn't much of a point in reopening an expired form...

danthe1st

This should be the last review of your initial version.
Overall, the structure is mostly good but there are quite a few things I found.
I think you could use JDA's capabilities more in some places and you are repeating yourself quite a bit between all of the subcommands. This can be refactored using a superclass for form subcommands that operate on a form. This would allow you using common handling for autocomplete, addition (the constructor in the superclass can accept FormRepository (if necessary), the description and the OptionDatas (or similar) of the subcommand) and extraction of the form-id (or form) argument and the loading of the FormData. If the FormData is present, you could call an abstract method, e.g. protected abstract execute(SlashCommandInteractionEvent event, FormData form)
This would also allow you using Optional#ifPresentOrElse for the FormData instead of Optional#get.

danthe1st · 2025-09-11T18:52:26Z

...in/java/net/discordjug/javabot/systems/staff_commands/forms/commands/ShowFormSubcommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;
+
+/**
+ * The `/form show` command.


Please add that this command opens the form modal

danthe1st · 2025-09-11T18:54:50Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsDeleteFormSubcommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;
+
+/**
+ * The `/form submissions-delete` command.


This should also include what it means for a form submission to be deleted.

danthe1st · 2025-09-11T18:55:27Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsDeleteFormSubcommand.java

+				new SubcommandData("submissions-delete", "Deletes submissions of an user in the form").addOptions(
+						new OptionData(OptionType.INTEGER, "form-id", "The ID of a form to get submissions for", true,
+								true),
+						new OptionData(OptionType.STRING, "user-id", "User to delete submissions of", true, true)));


Any reason to not just use OptionType.USER?

danthe1st · 2025-09-11T18:56:08Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsDeleteFormSubcommand.java

+	@Override
+	public void handleAutoComplete(CommandAutoCompleteInteractionEvent event, AutoCompleteQuery target) {
+		switch (target.getName()) {
+			case "user-id" -> {


This wouldn't be necessary if you used OptionType.USER.

danthe1st · 2025-09-11T18:57:17Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsExportFormSubcommand.java

+/**
+ * The `/form submissions-export` command.
+ */
+public class SubmissionsExportFormSubcommand extends Subcommand implements AutoCompletable {


I don't know how much value this adds but if you think this would be useful for you, I'm fine with it.

danthe1st · 2025-09-11T18:57:48Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsExportFormSubcommand.java

+import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;
+
+/**
+ * The `/form submissions-export` command.


Please add that it exports the users who submitted a form in JSON.

danthe1st · 2025-09-11T19:00:55Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsExportFormSubcommand.java

+		JsonObject root = new JsonObject();
+		JsonObject details = new JsonObject();
+		JsonArray users = new JsonArray();
+		for (Entry<FormUser, Integer> entry : submissions.entrySet()) {


I think Map#forEach would probably be cleaner (you can name the key and value) and not require dealing with entries.

danthe1st

TODO for myself: test with native-image - there shouldn't be any issues but just in case...

danthe1st

Please still act on the other requested changes like unifying autocomplete.

danthe1st · 2025-09-17T16:33:12Z

src/main/resources/database/migrations/09-08-2025_forms.sql

-    form_id BIGINT NOT NULL,
-    user_name VARCHAR NOT NULL,
-    PRIMARY KEY ("timestamp")
+CREATE TABLE FORM_FIELDS (


I think it would be good to keep the table names consistent i.e. all lowercase

I didn't think it mattered since h2 seems to store all names in uppercase, but I will change that

danthe1st · 2025-09-17T16:34:25Z

src/main/resources/database/migrations/09-08-2025_forms.sql

+CREATE TABLE FORM_FIELDS (
+    ID BIGINT NOT NULL AUTO_INCREMENT,
+    FORM_ID BIGINT NOT NULL,
+    LABEL CHARACTER VARYING NOT NULL,


Is there any reason you are writing CHARACTER VARYING instead of VARCHAR?

This was generated by DBeaver, and I haven't changed it yet because I'm still working on this part

danthe1st · 2025-09-17T16:38:48Z

src/main/resources/database/migrations/09-08-2025_forms.sql

+
+CREATE TABLE FORM_SUBMISSIONS (
+    ID BIGINT NOT NULL AUTO_INCREMENT,
+    MESSAGE_ID BIGINT NOT NULL,


If you are storing a message ID, is there a need to add a distinct ID for the primary key?

Right, I guess not

danthe1st · 2025-09-17T16:42:26Z

.../java/net/discordjug/javabot/systems/staff_commands/forms/commands/DeleteFormSubcommand.java

-		if (form.getMessageChannel() != null && form.getMessageId() != null) {
+		if (form.isAttached()) {
 			DetachFormSubcommand.detachFromMessage(form, event.getGuild());
+			// TODO send a warning


This TODO comment should be implemented before this PR is merged (and then you also wouldn't need the detaching here) ;)

Yup, that's the plan.

danthe1st · 2025-09-17T16:49:20Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/dao/FormsRepository.java

 	 *
 	 * @return A list of forms
 	 */
 	public List<FormData> getAllForms() {


I think you only need this for autocomplete? If so, I think it should be sufficient to return the form names and IDs?

I think the same applies to getAllForms(boolean)

That's true. I made this method in case I needed a list of all forms somewhere else.

danthe1st · 2025-09-17T16:58:00Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/dao/FormsRepository.java

 	 *
-	 * @param user user to log
-	 * @param form form to log on
+	 * @param user    user to log


You replaced "log" in the summary but not the parameters. I think user who submitted the form and submitted form or something like that would be better.

danthe1st · 2025-09-17T17:07:06Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormData.java

-	 * @return Lsit of layout components for use in the submission modal.
+	 * @return List of layout components for use in the submission modal.
 	 */
 	public LayoutComponent[] createComponents() {


I don't think this needs to be an array.

danthe1st · 2025-09-17T17:08:48Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormField.java

+				.setRequired(required()).setValue(value()).build();
 	}

 	@Override


Any reason you are still overriding the toString here?

danthe1st · 2025-09-17T17:13:18Z

src/main/java/net/discordjug/javabot/systems/staff_commands/forms/model/FormData.java

 		this.onetime = onetime;
 	}

+	public boolean isAttached() {


You are often using the check of calling isAttached() and then calling Optional#get if isAttached returned true. Alternatively, you could create another record for the channel+message and return an Optional of that record. Then you could use Optionals capabilities instead of Optional#get (and tbh I'd prefer Optional#orElseThrow over Optional#get because that's more clear about what it does):

form.getMessageInformation().ifPresentOrElse(messageInfo -> doSomething(messageInfo), () -> notAttached()); // or similar

or

String text = form.getMessageInformation().map(messageInfo -> String.format("[Link](https://discord.com/channels/%s/%s/%s)", guild.getId(), messageInfo.channelId(), messageInfo.messageId()), ).orElse(() -> "*Not attached*");

danthe1st · 2025-09-17T17:20:47Z

...iscordjug/javabot/systems/staff_commands/forms/commands/SubmissionsDeleteFormSubcommand.java

-								true),
-						new OptionData(OptionType.STRING, "user-id", "User to delete submissions of", true, true)));
+		setCommandData(new SubcommandData("submissions-delete", "Deletes submissions of a user in the form")
+				.addOptions(new OptionData(OptionType.INTEGER, "form-id", "The ID of a form to get submissions for",


get submissions for is misleading.

Yup, it's just another oversight of mine. I did a lot of copy-pasting when creating the commands, and it seems I forgot to change the parameter description here

danthe1st · 2025-09-17T17:22:39Z

By the way, the last build failed because of checkstyle.

Defective4 · 2025-09-17T17:40:07Z

By the way, the last build failed because of checkstyle.

I know. I will take care of checkstyle once I'm done writing the actual code.

Signed-off-by: Defective <def3ctive4@gmail.com>

Defective4 · 2025-09-18T10:29:36Z

Also, please keep in mind that even though I'm pushing new commits, I'm still not done addressing most of the issues. I have a habit of pushing immediately after committing new changes to the code.

danthe1st · 2025-09-18T14:15:46Z

Also, please keep in mind that even though I'm pushing new commits, I'm still not done addressing most of the issues. I have a habit of pushing immediately after committing new changes to the code.

Just comment when you think I should re-review it.

Defective4 requested a review from a team as a code owner September 10, 2025 11:36

danthe1st requested changes Sep 10, 2025

View reviewed changes

danthe1st requested changes Sep 11, 2025

View reviewed changes

danthe1st reviewed Sep 11, 2025

View reviewed changes

Defective4 added 8 commits September 17, 2025 15:58

Store form fields in a separate table

bdf0b1a

Use default values for all NOT NULL fields

b62574d

Store form's message channel and id as longs instead of Strings

2c97172

Changed submissions table structure to use unique ids

7db911a

Correct FormsRepository javadoc

e2c6372

Adjustments to the FormsRepository class

7cb72ed

Fix javadoc spelling mistakes

61d7e36

Make the FormField into a record

21d9a42

danthe1st requested changes Sep 17, 2025

View reviewed changes

Convert FormData into a record and use Instants for expiration

3425fa0

Signed-off-by: Defective <def3ctive4@gmail.com>

Defective4 force-pushed the forms-feature branch from b1b596f to 3425fa0 Compare September 18, 2025 10:26

Defective4 added 4 commits September 20, 2025 11:01

Form field javadoc

fdd328e

Convert FormUser to a record

123282b

FormCommand javadoc

7fccf8e

Fix checkstyle

385820d


		@Override
		public void execute(SlashCommandInteractionEvent event) {

Forms system #524

Are you sure you want to change the base?

Forms system #524

Uh oh!

Conversation

Defective4 commented Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

danthe1st left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danthe1st left a comment

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!

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!

Defective4 Sep 10, 2025 • 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!

danthe1st Sep 17, 2025 • 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!

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!

danthe1st left a comment

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!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Defective4 Sep 11, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Defective4 commented Sep 10, 2025 •

edited

Loading

Defective4 Sep 10, 2025 •

edited

Loading

danthe1st Sep 17, 2025 •

edited

Loading

Defective4 Sep 11, 2025 •

edited

Loading