Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
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
Expand Up @@ -53,6 +53,7 @@
import org.togetherjava.tjbot.features.moderation.QuarantineCommand;
import org.togetherjava.tjbot.features.moderation.RejoinModerationRoleListener;
import org.togetherjava.tjbot.features.moderation.ReportCommand;
import org.togetherjava.tjbot.features.moderation.ThisIsScamCommand;
import org.togetherjava.tjbot.features.moderation.TransferQuestionCommand;
import org.togetherjava.tjbot.features.moderation.UnbanCommand;
import org.togetherjava.tjbot.features.moderation.UnmuteCommand;
Expand Down Expand Up @@ -191,6 +192,7 @@ public static Collection<Feature> createFeatures(JDA jda, Database database, Con

// Message context commands
features.add(new TransferQuestionCommand(config, chatGptService));
features.add(new ThisIsScamCommand(config, actionsStore));

// User context commands

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,300 @@
package org.togetherjava.tjbot.features.moderation;

import com.github.benmanes.caffeine.cache.Cache;
import com.github.benmanes.caffeine.cache.Caffeine;
import net.dv8tion.jda.api.EmbedBuilder;
import net.dv8tion.jda.api.Permission;
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.Role;
import net.dv8tion.jda.api.entities.User;
import net.dv8tion.jda.api.entities.channel.concrete.TextChannel;
import net.dv8tion.jda.api.events.interaction.command.MessageContextInteractionEvent;
import net.dv8tion.jda.api.events.interaction.component.ButtonInteractionEvent;
import net.dv8tion.jda.api.exceptions.ErrorHandler;
import net.dv8tion.jda.api.interactions.commands.build.Commands;
import net.dv8tion.jda.api.interactions.components.buttons.Button;
import net.dv8tion.jda.api.interactions.components.buttons.ButtonStyle;
import net.dv8tion.jda.api.requests.ErrorResponse;
import net.dv8tion.jda.api.requests.RestAction;
import net.dv8tion.jda.api.requests.restaction.MessageCreateAction;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import org.togetherjava.tjbot.config.Config;
import org.togetherjava.tjbot.features.BotCommandAdapter;
import org.togetherjava.tjbot.features.CommandVisibility;
import org.togetherjava.tjbot.features.MessageContextCommand;
import org.togetherjava.tjbot.features.utils.Guilds;
import org.togetherjava.tjbot.features.utils.MessageUtils;
import org.togetherjava.tjbot.logging.LogMarkers;

import java.awt.Color;
import java.time.Duration;
import java.time.Instant;
import java.util.ArrayList;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.function.Predicate;
import java.util.regex.Pattern;
import java.util.stream.Collectors;

/**
* Allows users to report a message as potential scam. Moderators can confirm the report from the
* audit log, causing the author to be quarantined plus message history getting deleted.
*/
public final class ThisIsScamCommand extends BotCommandAdapter implements MessageContextCommand {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd use a name like ScamReportCommand for the class, and for the command scam-report

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

imo all that does is making it more confusing for the user and the devs. a context command needs to be a verb, especially bc they dont have any description in the discord ui

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Both work fine, agree it's NIT

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

imo all that does is making it more confusing for the user and the devs. a context command needs to be a verb, especially bc they dont have any description in the discord ui

Then "Report Scam"

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I will join the "it's a NIT" team here. It's just the name at the end of the day, and the class has a JavaDoc which makes it easier to understand what this class focuses on.

private static final Logger logger = LoggerFactory.getLogger(ThisIsScamCommand.class);

private static final String COMMAND_NAME = "this-is-scam";

private static final String ACTION_TITLE = "Quarantine";
Comment thread
firasrg marked this conversation as resolved.
private static final String ACTION_REASON = "Message was reported and confirmed as scam";

private static final String FAILED_MESSAGE =
"Sorry, there was an issue forwarding your scam report to the moderators. We are investigating.";
private static final Duration USER_COMMAND_COOLDOWN = Duration.ofMinutes(1);
private static final Color AMBIENT_COLOR = Color.decode("#CFBFF5");
Comment thread
firasrg marked this conversation as resolved.

private final Cache<Long, Instant> reportedMessageToTimestamp =
Caffeine.newBuilder().maximumSize(10_000).expireAfterWrite(Duration.ofDays(1)).build();
private final Cache<Long, Instant> userToLastCommandUse = Caffeine.newBuilder()
Comment thread
firasrg marked this conversation as resolved.
.maximumSize(10_000)
.expireAfterWrite(USER_COMMAND_COOLDOWN)
.build();

private final Config config;
private final ModerationActionsStore actionsStore;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I'd put these 2 fields config and actionsStore above reportedMessageToTimestamp and userToLastCommandUse, because they represent a higher level

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

maybe, ill see

@surajkumar surajkumar Jun 20, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

NIT - optional

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I agree that this is a NIT comment.

At the very least this should be handled by our Spotless plugin.

private final Predicate<String> isModAuditLogChannel;
Comment thread
firasrg marked this conversation as resolved.

/**
* Creates a new instance.
*
* @param config to resolve the moderation audit log channel and quarantined role
* @param actionsStore used to store issued quarantine actions
*/
public ThisIsScamCommand(Config config, ModerationActionsStore actionsStore) {
super(Commands.message(COMMAND_NAME), CommandVisibility.GUILD);

this.config = Objects.requireNonNull(config);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I think config cannot be null, since it's loaded at startup and it has validations inside of it

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

but we want fail fast. classes shouldn't trust their callers, only themselves.

this.actionsStore = Objects.requireNonNull(actionsStore);
isModAuditLogChannel =
Pattern.compile(config.getModAuditLogChannelPattern()).asMatchPredicate();
}

@Override
public void onMessageContext(MessageContextInteractionEvent event) {
if (handleIsOnCooldown(event)) {
return;
Comment thread
firasrg marked this conversation as resolved.
}
if (handleWasAlreadyReportedMessage(event)) {
return;
}

Optional<TextChannel> modAuditLog = getModAuditLogChannel(event);
if (modAuditLog.isEmpty()) {
event.reply(FAILED_MESSAGE).setEphemeral(true).queue();
return;
}

Message message = event.getTarget();
reportToMods(message, modAuditLog.orElseThrow()).mapToResult().map(result -> {
Comment thread
firasrg marked this conversation as resolved.
if (result.isFailure()) {
logger.warn("Unable to forward a scam report to the mod audit log channel.",
result.getFailure());
return FAILED_MESSAGE;
}
return "Thank you for your report, a moderator will take care of it 👍";
Comment thread
firasrg marked this conversation as resolved.
}).flatMap(response -> event.reply(response).setEphemeral(true)).queue();
}

private boolean handleIsOnCooldown(MessageContextInteractionEvent event) {
Instant lastCommandUse = userToLastCommandUse.getIfPresent(event.getUser().getIdLong());
Runnable isNotOnCooldownAction =
() -> userToLastCommandUse.put(event.getUser().getIdLong(), Instant.now());

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For consistency: event.getUser().getIdLong() is duplicated/redundent between this line and 117, it can be defined as a local constant

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

ill check


if (lastCommandUse == null) {
Comment thread
firasrg marked this conversation as resolved.
isNotOnCooldownAction.run();
return false;
}
Instant momentCooldownEnds = lastCommandUse.plus(USER_COMMAND_COOLDOWN);
if (Instant.now().isAfter(momentCooldownEnds)) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

For consistency: Instant.now() is duplicated between this line and 119, it can be defined once as final Instant now = Instant.now();, then that constant gets called instead

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

mh, ill check

isNotOnCooldownAction.run();
return false;
}

event.reply("You just reported a message as scam, please wait a bit.")
.setEphemeral(true)
.queue();
return true;
}

private boolean handleWasAlreadyReportedMessage(MessageContextInteractionEvent event) {
long messageId = event.getTarget().getIdLong();
if (reportedMessageToTimestamp.getIfPresent(messageId) != null) {
event.reply("This message was already reported as potential scam.")
.setEphemeral(true)
.queue();
return true;
}

reportedMessageToTimestamp.put(messageId, Instant.now());
return false;
}

private Optional<TextChannel> getModAuditLogChannel(MessageContextInteractionEvent event) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since the return type is Optional<>, I'd name it findModAuditLogChannel(). From my experience especially with JPA, there is a difference between Get and Find, it comes down to semantic expectations regarding predictability, presence of data, and execution cost

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

kinda NIT but okay

Guild guild = Objects.requireNonNull(event.getGuild());
Optional<TextChannel> modAuditLogChannel =
Guilds.findTextChannel(guild, isModAuditLogChannel);
if (modAuditLogChannel.isEmpty()) {
logger.warn(
"Cannot find the designated mod audit log channel in guild '{}' with the pattern '{}'",
guild.getId(), config.getModAuditLogChannelPattern());
}
return modAuditLogChannel;
}

private MessageCreateAction reportToMods(Message message, TextChannel auditChannel) {
User author = message.getAuthor();
String description = createDescription(message);

MessageEmbed reportEmbed = new EmbedBuilder().setTitle("Is this Scam?")
Comment thread
firasrg marked this conversation as resolved.
.setDescription(
MessageUtils.abbreviate(description, MessageEmbed.DESCRIPTION_MAX_LENGTH))
.setAuthor(author.getName(), null, author.getEffectiveAvatarUrl())
.setTimestamp(message.getTimeCreated())
.setColor(AMBIENT_COLOR)
.setFooter(author.getId())
.build();

long guildId = message.getGuild().getIdLong();
long authorId = author.getIdLong();
String[] args = {String.valueOf(guildId), String.valueOf(authorId)};

return auditChannel.sendMessageEmbeds(reportEmbed)
.addActionRow(Button.success(generateComponentId(args), "Yes"),
Button.danger(generateComponentId(args), "No"));
Comment thread
firasrg marked this conversation as resolved.
}

@SuppressWarnings("squid:S3457") // %n is wrong, markdown must use \n
private static String createDescription(Message target) {
Comment on lines +184 to +185

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Couldn't you easily use multiline strings here to make it cleaner and get rid of the suppress warnings?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

great idea

String content = target.getContentStripped();
String description = content.isBlank() ? "(empty message)" : content;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

i'd rather say "empty description"

@Zabuzard Zabuzard Jun 20, 2026

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

that would be confusing as "description" is right in the context of the embed but not in the context of the reading user in discord.
the scan message that was reported is empty, so that is what we need to tell the user.
empty message.

i might improve the wording slightly though:
(message had no text content)


List<Message.Attachment> attachments = target.getAttachments();
if (!attachments.isEmpty()) {
String attachmentInfo = attachments.stream()
.map(Message.Attachment::getFileName)
.collect(Collectors.joining("\n"));
description += "\n\nAttachments:\n" + attachmentInfo;
}

description += "\n\n[Go to message](%s)".formatted(target.getJumpUrl());
return description;
}

@Override
public void onButtonClick(ButtonInteractionEvent event, List<String> args) {
long guildId = Long.parseLong(args.get(0));
long targetId = Long.parseLong(args.get(1));

ButtonStyle clickedStyle = event.getButton().getStyle();
boolean isScam = clickedStyle == ButtonStyle.SUCCESS;
Comment thread
firasrg marked this conversation as resolved.

MessageEmbed resultEmbed = new EmbedBuilder()
.setDescription(
isScam ? "This is scam. The user was quarantined and messages were deleted."
: "This is not scam, no action executed.")
Comment thread
firasrg marked this conversation as resolved.
.setColor(isScam ? Color.GREEN : Color.RED)
.build();

List<MessageEmbed> embeds = new ArrayList<>(event.getMessage().getEmbeds());
embeds.add(resultEmbed);

event.editMessageEmbeds(embeds).setComponents().queue();

if (!isScam) {
Comment thread
firasrg marked this conversation as resolved.
return;
}

Guild guild = Objects.requireNonNull(event.getJDA().getGuildById(guildId));
Member moderator = Objects.requireNonNull(event.getMember());

ErrorHandler errorHandler = new ErrorHandler()
.handle(ErrorResponse.UNKNOWN_USER, failure -> logger.debug(LogMarkers.SENSITIVE,
"Attempted to handle user-reported scam, but user '{}' does not exist anymore.",
targetId))
.handle(ErrorResponse.UNKNOWN_MEMBER, failure -> logger.debug(LogMarkers.SENSITIVE,
"Attempted to handle user-reported scam, but user '{}' is not a member of guild '{}' anymore.",
targetId, guildId));

guild.retrieveMemberById(targetId)
.queue(target -> handleConfirmedScam(guild, target, moderator, event), errorHandler);
}

private void handleConfirmedScam(Guild guild, Member target, Member moderator,
ButtonInteractionEvent event) {
if (!handleCanQuarantineAndBan(guild, target, event)) {
return;
}

Consumer<? super Void> onSuccess = _ -> {
};
Consumer<? super Throwable> onFailure = failure -> logger.warn(LogMarkers.SENSITIVE,
"Failed to finish user-reported scam handling for user '{}' in guild '{}'.",
target.getId(), guild.getId(), failure);

sendQuarantineDm(target.getUser(), guild)
.flatMap(hasSentDm -> quarantineUser(guild, target, moderator))
.flatMap(_ -> deleteMessagesByBanAndUnban(guild, target.getUser()))
.queue(onSuccess, onFailure);
}

private boolean handleCanQuarantineAndBan(Guild guild, Member target,
ButtonInteractionEvent event) {
Member bot = guild.getSelfMember();
Member moderator = Objects.requireNonNull(event.getMember());
Role quarantinedRole = ModerationUtils.getQuarantinedRole(guild, config).orElse(null);

return ModerationUtils.handleRoleChangeChecks(quarantinedRole, "quarantine", target, bot,
moderator, guild, ACTION_REASON, event)
&& ModerationUtils.handleHasBotPermissions("ban", Permission.BAN_MEMBERS, bot,
guild, event);
}

private RestAction<Boolean> sendQuarantineDm(User target, Guild guild) {
String description =
"""
Hey there, sorry to tell you but unfortunately you have been put under quarantine.
This means you can no longer interact with anyone in the server until you have been unquarantined again.""";

return ModerationUtils.sendModActionDm(ModerationUtils.getModActionEmbed(guild,
ACTION_TITLE, description, ACTION_REASON, true), target);
}

private RestAction<Void> quarantineUser(Guild guild, Member target, Member moderator) {
logger.info(LogMarkers.SENSITIVE,
"'{}' ({}) quarantined the user '{}' ({}) in guild '{}' for reason '{}'.",
moderator.getUser().getName(), moderator.getId(), target.getUser().getName(),
target.getId(), guild.getName(), ACTION_REASON);

actionsStore.addAction(guild.getIdLong(), moderator.getIdLong(), target.getIdLong(),
ModerationAction.QUARANTINE, null, ACTION_REASON);

return guild
.addRoleToMember(target,
ModerationUtils.getQuarantinedRole(guild, config).orElseThrow())
.reason(ACTION_REASON);
}

private RestAction<Void> deleteMessagesByBanAndUnban(Guild guild, User target) {
return guild.ban(target, 1, TimeUnit.DAYS)
.reason(ACTION_REASON)
.flatMap(_ -> guild.unban(target).reason(ACTION_REASON));
}
}
Comment on lines +258 to +300

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can't all of this be reused from the existing quarantine/moderation logic?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

not easily. but yeah, some of it can probably be moved around.

the main issue is that we only want part of the behavior but not everything.

so moving the code around could possibly result in making it too complex to understand.

but its a good comment, ill have a second look :)

Loading