Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -0,0 +1,275 @@
package net.discordjug.javabot.systems.staff_commands.forms;

import java.text.DateFormat;
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.Instant;
import java.util.List;
import java.util.Locale;
import java.util.Objects;
import java.util.Optional;
import java.util.TimeZone;
import java.util.function.Function;

import lombok.RequiredArgsConstructor;
import net.discordjug.javabot.annotations.AutoDetectableComponentHandler;
import net.discordjug.javabot.systems.staff_commands.forms.dao.FormsRepository;
import net.discordjug.javabot.systems.staff_commands.forms.model.FormData;
import net.discordjug.javabot.systems.staff_commands.forms.model.FormField;
import net.dv8tion.jda.api.EmbedBuilder;
import net.dv8tion.jda.api.entities.Guild;
import net.dv8tion.jda.api.entities.Member;
import net.dv8tion.jda.api.entities.Message;
import net.dv8tion.jda.api.entities.MessageEmbed;
import net.dv8tion.jda.api.entities.channel.concrete.TextChannel;
import net.dv8tion.jda.api.events.interaction.ModalInteractionEvent;
import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent;
import net.dv8tion.jda.api.events.interaction.component.ButtonInteractionEvent;
import net.dv8tion.jda.api.interactions.commands.OptionMapping;
import net.dv8tion.jda.api.interactions.components.ActionRow;
import net.dv8tion.jda.api.interactions.components.ItemComponent;
import net.dv8tion.jda.api.interactions.components.buttons.Button;
import net.dv8tion.jda.api.interactions.modals.Modal;
import net.dv8tion.jda.api.interactions.modals.ModalMapping;
import xyz.dynxsty.dih4jda.interactions.components.ButtonHandler;
import xyz.dynxsty.dih4jda.interactions.components.ModalHandler;
import xyz.dynxsty.dih4jda.util.ComponentIdBuilder;

/**
* Handle forms interactions, including buttons and submissions modals.
*/
@AutoDetectableComponentHandler(FormInteractionManager.FORM_COMPONENT_ID)
@RequiredArgsConstructor
public class FormInteractionManager implements ButtonHandler, ModalHandler {

/**
* Date and time format used in forms.
*/
public static final DateFormat DATE_FORMAT;

/**
* String representation of the date and time format used in forms.
*/
public static final String DATE_FORMAT_STRING;

/**
* Component ID used for form buttons and modals.
*/
public static final String FORM_COMPONENT_ID = "modal-form";
private static final String FORM_NOT_FOUND_MSG = "This form was not found in the database. Please report this to the server staff.";

private final FormsRepository formsRepo;

static {
DATE_FORMAT_STRING = "dd/MM/yyyy HH:mm";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure about that date format. While we currently don't have US staff members, I am also confused with dd/MM/YYYY vs MM/dd/YYYY.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the best date formats are yyyy-MM-dd and dd.MM.yyyy. Please also explicitly set UTC as the timezone or alternatively allow the user to specify an OffsetDateTime with e.g. +0000.

Copy link
Member Author

@Defective4 Defective4 Sep 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also explicitly set UTC as the timezone

I did, actually.
Regarding the date format, I saw something similar elsewhere in the code and went with it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Interesting. I didn't know of anything in the bot where someone would need to enter a date.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See the expiration fields in create and modify subcommands

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I thought you meant the bot using date inputs in existing code (outside of this PR).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, sorry, I misunderstood the previous sentence. I saw the dd/MM/yyyy format in the TimeUtils class, I think, although I didn't check where it's actually used.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure why that specific time format used there but it doesn't seem like a common thing at least (that seems to be for the message deletion/editing logs if the message is too long for the embed).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yup, that's my bad. I'll change the format.

DATE_FORMAT = new SimpleDateFormat(FormInteractionManager.DATE_FORMAT_STRING, Locale.ENGLISH);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From the SimpleDateFormat Javadoc:

Consider using DateTimeFormatter as an immutable and thread-safe alternative.

This should also get rid of the need for a static block I think.

DATE_FORMAT.setTimeZone(TimeZone.getTimeZone("UTC"));
}

/**
* Closes the form, preventing further submissions and disabling associated
* buttons from a message this form is attached to, if any.
*
* @param guild guild this form is located in.
* @param form form to close.
*/
public void closeForm(Guild guild, FormData form) {
formsRepo.closeForm(form);

if (form.getMessageChannel() != null && form.getMessageId() != null) {
TextChannel formChannel = guild.getTextChannelById(form.getMessageChannel());
formChannel.retrieveMessageById(form.getMessageId()).queue(msg -> {
mapFormMessageButtons(msg, btn -> {
String cptId = btn.getId();
String[] split = ComponentIdBuilder.split(cptId);
if (split[0].equals(FormInteractionManager.FORM_COMPONENT_ID)
&& split[1].equals(Long.toString(form.getId()))) {
return btn.asDisabled();
}
return btn;
});
}, t -> {});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please at least log the error, even if it isn't shown to users. See ExceptionLogger. The cleaner solution would be adding a callback but logging it might be fine.

}
}

@Override
public void handleButton(ButtonInteractionEvent event, Button button) {
long formId = Long.parseLong(ComponentIdBuilder.split(button.getId())[1]);
Optional<FormData> formOpt = formsRepo.getForm(formId);
if (!formOpt.isPresent()) {
event.reply(FORM_NOT_FOUND_MSG).setEphemeral(true).queue();
return;
}
FormData form = formOpt.get();
if (!checkNotClosed(form)) {
event.reply("This form is not accepting new submissions.").setEphemeral(true).queue();
if (!form.isClosed()) {
closeForm(event.getGuild(), form);
}
return;
}

if (form.isOnetime() && formsRepo.hasSubmitted(event.getUser(), form)) {
event.reply("You have already submitted this form").setEphemeral(true).queue();
return;
}

Modal modal = createFormModal(form);

event.replyModal(modal).queue();
}

@Override
public void handleModal(ModalInteractionEvent event, List<ModalMapping> values) {
event.deferReply().setEphemeral(true).queue();
long formId = Long.parseLong(ComponentIdBuilder.split(event.getModalId())[1]);
Optional<FormData> formOpt = formsRepo.getForm(formId);
if (!formOpt.isPresent()) {
event.reply(FORM_NOT_FOUND_MSG).setEphemeral(true).queue();
return;
}

FormData form = formOpt.get();

if (!checkNotClosed(form)) {
event.getHook().sendMessage("This form is not accepting new submissions.").queue();
return;
}

if (form.isOnetime() && formsRepo.hasSubmitted(event.getUser(), form)) {
event.getHook().sendMessage("You have already submitted this form").queue();
return;
}

TextChannel channel = event.getGuild().getTextChannelById(form.getSubmitChannel());
if (channel == null) {
event.getHook()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please also add a log message (using SLF4j) that notes that the channel is missing for that form. If that happens, we should at least have some option of checking what happened.

.sendMessage("We couldn't receive your submission due to an error. Please contact server staff.")
.queue();
return;
}

channel.sendMessageEmbeds(createSubmissionEmbed(form, values, event.getMember())).queue();
formsRepo.logSubmission(event.getUser(), form);

event.getHook()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both the submission and the confirmation should be sent only if the message has been sent successfully. If it hasn't, there should be an error message sent to the user saying there was an error with the submission and a log message.
My guess is that the most likely reason this could happen would be hitting the message character limit or similar.

.sendMessage(
form.getSubmitMessage() == null ? "Your submission was received!" : form.getSubmitMessage())
.queue();
}

/**
* Modifies buttons in a message using given function for mapping.
*
* @param msg message to modify buttons in.
* @param mapper mapping function.
*/
public void mapFormMessageButtons(Message msg, Function<Button, Button> mapper) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The name map is confusing as it suggests that it returns the mapped buttons as opposed to modifying the buttons and sending it to Discord.

List<ActionRow> components = msg.getActionRows().stream().map(row -> {
ItemComponent[] cpts = row.getComponents().stream().map(cpt -> {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the use of arrays here is unnecessary. Also, stream.toArray(ItemComponent::new) exists.

if (cpt instanceof Button btn) {
return mapper.apply(btn);
}
return cpt;
}).toList().toArray(new ItemComponent[0]);
if (cpts.length == 0) {
return null;
}
return ActionRow.of(cpts);
}).filter(Objects::nonNull).toList();
msg.editMessageComponents(components).queue();
}

/**
* Re-opens the form, re-enabling associated buttons in message it's attached
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in the message

* to, if any.
*
* @param guild guild this form is contained in.
* @param form form to re-open.
*/
public void reopenForm(Guild guild, FormData form) {
formsRepo.reopenForm(form);

if (form.getMessageChannel() != null && form.getMessageId() != null) {
TextChannel formChannel = guild.getTextChannelById(form.getMessageChannel());
formChannel.retrieveMessageById(form.getMessageId()).queue(msg -> {
mapFormMessageButtons(msg, btn -> {
String cptId = btn.getId();
String[] split = ComponentIdBuilder.split(cptId);
if (split[0].equals(FormInteractionManager.FORM_COMPONENT_ID)
&& split[1].equals(Long.toString(form.getId()))) {
return btn.asEnabled();
}
return btn;
});
}, t -> {});
}
}

/**
* Creates a submission modal for the given form.
*
* @param form form to open submission modal for.
* @return submission modal to be presented to the user.
*/
public static Modal createFormModal(FormData form) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename this to createSubmissionModal

Modal modal = Modal.create(ComponentIdBuilder.build(FORM_COMPONENT_ID, form.getId()), form.getTitle())
.addComponents(form.createComponents()).build();
return modal;
}

/**
* Gets expiration time from the slash comamnd event.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

*
* @param event slash event to get expiration from.
* @return an optional containing expiration time,
* {@link FormData#EXPIRATION_PERMANENT} if none given, or an empty
* optional if it's invalid.
*/
public static Optional<Long> parseExpiration(SlashCommandInteractionEvent event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method is not just parsing the expiration but also sending an error message if this process fails. This should be reflected both in the method name and Javadoc.

String expirationStr = event.getOption("expiration", null, OptionMapping::getAsString);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since DATE_FORMAT and similar things are reused, I think it would be good to create a constant for expiration as well.

Optional<Long> expiration;
if (expirationStr == null) {
expiration = Optional.of(FormData.EXPIRATION_PERMANENT);
} else {
try {
expiration = Optional.of(FormInteractionManager.DATE_FORMAT.parse(expirationStr).getTime());
} catch (ParseException e) {
event.getHook().sendMessage("Invalid date. You should follow the format `"
+ FormInteractionManager.DATE_FORMAT_STRING + "`.").setEphemeral(true).queue();
expiration = Optional.empty();
}
}

if (expiration.isPresent() && expiration.get() != FormData.EXPIRATION_PERMANENT
&& expiration.get() < System.currentTimeMillis()) {
event.getHook().sendMessage("The expiration date shouldn't be in the past").setEphemeral(true).queue();
return Optional.empty();
}
return expiration;
}

private static boolean checkNotClosed(FormData data) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

if (data.isClosed() || data.hasExpired()) {
return false;
}

return true;
}

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);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@Defective4 Defective4 Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume you mean the author of the form?

Copy link
Member

@danthe1st danthe1st Sep 17, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant the user who submitted the form.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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


int len = Math.min(values.size(), form.getFields().size());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

` ` `

or truncate them - or generally prohibit that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

builder.addField(field.getLabel(), value == null ? "*Empty*" : "```\n" + value + "\n```", false);
}

return builder.build();
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,103 @@
package net.discordjug.javabot.systems.staff_commands.forms.commands;

import java.util.Arrays;
import java.util.Optional;

import net.discordjug.javabot.systems.staff_commands.forms.dao.FormsRepository;
import net.discordjug.javabot.systems.staff_commands.forms.model.FormData;
import net.discordjug.javabot.systems.staff_commands.forms.model.FormField;
import net.dv8tion.jda.api.events.interaction.command.CommandAutoCompleteInteractionEvent;
import net.dv8tion.jda.api.events.interaction.command.SlashCommandInteractionEvent;
import net.dv8tion.jda.api.interactions.AutoCompleteQuery;
import net.dv8tion.jda.api.interactions.commands.Command.Choice;
import net.dv8tion.jda.api.interactions.commands.OptionMapping;
import net.dv8tion.jda.api.interactions.commands.OptionType;
import net.dv8tion.jda.api.interactions.commands.build.SubcommandData;
import net.dv8tion.jda.api.interactions.components.text.TextInputStyle;
import xyz.dynxsty.dih4jda.interactions.AutoCompletable;
import xyz.dynxsty.dih4jda.interactions.commands.application.SlashCommand.Subcommand;

/**
* The `/form add-field` command.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

*/
public class AddFieldFormSubcommand extends Subcommand implements AutoCompletable {

private final FormsRepository formsRepo;

/**
* The main constructor of this subcommand.
*
* @param formsRepo the forms repository
*/
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)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think there isn't any way to list forms with their IDs other than autocomplete (which I think is limited to a given amount of options) so users don't deal with IDs. I think naming that parameter form might be better name.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's true, but I named the parameter form-id because it technically still requires an ID. But yeah, form will probably be a better name

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

.addOption(OptionType.STRING, "label", "Field label", true)
.addOption(OptionType.INTEGER, "min", "Minimum number of characters")
.addOption(OptionType.INTEGER, "max", "Maximum number of characters")
.addOption(OptionType.STRING, "placeholder", "Field placeholder")
.addOption(OptionType.BOOLEAN, "required",
"Whether or not the user has to input data in this field. Default: false")
.addOption(OptionType.STRING, "style", "Input style. Default: SHORT", false, true)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think a choice option would be better than autocomplete for the style.

.addOption(OptionType.STRING, "value", "Initial field value")
.addOption(OptionType.INTEGER, "index", "Index to insert the field at"));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

}

@Override
public void execute(SlashCommandInteractionEvent event) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.


Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Member Author

@Defective4 Defective4 Sep 11, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

event.deferReply(true).queue();
Optional<FormData> formOpt = formsRepo.getForm(event.getOption("form-id", OptionMapping::getAsLong));
if (formOpt.isEmpty()) {
event.getHook().sendMessage("A form with this ID was not found.").queue();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For simple error messages, you can also use Responses.error(event).queue() (and also with event.getHook() after deferReply) if you prefer an embed. This would also make it ephemeral.

return;
}
FormData form = formOpt.get();

if (form.getFields().size() >= 5) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is Modal.MAX_COMPONENTS for this.

event.getHook().sendMessage("Can't add more than 5 components to a form").queue();
return;
}

int index = event.getOption("index", -1, OptionMapping::getAsInt);
if (index < -1 || index >= form.getFields().size()) {
event.getHook().sendMessage("Field index out of bounds").queue();
return;
}

formsRepo.addField(form, createFormFieldFromEvent(event), index);
event.getHook().sendMessage("Added a new field to the form.").queue();
}

@Override
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())
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use AutoCompleteUtils.filterChoices (DIH4JDA) for the replyChoices calls. This ensures that users can enter partial texts and that it doesn't fail when there are too many options.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

.queue();
case "style" ->
event.replyChoices(Arrays.stream(TextInputStyle.values()).filter(t -> t != TextInputStyle.UNKNOWN)
.map(style -> new Choice(style.name(), style.name())).toList()).queue();
default -> {}
}
}

private static FormField createFormFieldFromEvent(SlashCommandInteractionEvent e) {
String label = e.getOption("label", OptionMapping::getAsString);
int min = e.getOption("min", 0, OptionMapping::getAsInt);
int max = e.getOption("max", 64, OptionMapping::getAsInt);
String placeholder = e.getOption("placeholder", OptionMapping::getAsString);
boolean required = e.getOption("required", false, OptionMapping::getAsBoolean);
TextInputStyle style = e.getOption("style", TextInputStyle.SHORT, t -> {
try {
return TextInputStyle.valueOf(t.getAsString().toUpperCase());
} catch (IllegalArgumentException e2) {
return TextInputStyle.SHORT;
}
});
String value = e.getOption("value", OptionMapping::getAsString);

return new FormField(label, max, min, placeholder, required, style.name(), value);
}
}
Loading