-
-
Notifications
You must be signed in to change notification settings - Fork 110
Adds "this-is-scam" message context command #1505
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| 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 { | ||
| 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"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this constant is used once at line 277, I think it's better to put its value directly there where without declaring this
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. kinda NIT. ill keep it as is bc this is the style also used in the other mod commands
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is fine as-is (good how it's done). The ACTION_TITLE gives the string some meaning otherwise you wouldn't clearly know what the text "Quarantine" is being used for in the method it's used: vs. same with the other params, if Java had named parameters or the function was using a builder pattern, it would have been fine to inline however since we're not, it's best to keep the context clear. |
||
| 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"); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. those are the constants that someone might want to tweak, so better to have them here. |
||
|
|
||
| private final Cache<Long, Instant> reportedMessageToTimestamp = | ||
| Caffeine.newBuilder().maximumSize(10_000).expireAfterWrite(Duration.ofDays(1)).build(); | ||
| private final Cache<Long, Instant> userToLastCommandUse = Caffeine.newBuilder() | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This other constant has similar builder chain as private static Cache<Long, Instant> createTempCache(){
// you can make the values 10_000 and duration of days = 1 as constants at the top so anybody can see that when they reach this class
return Caffeine.newBuilder().maximumSize(10_000).expireAfterWrite(Duration.ofDays(1)).build();
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. makes it needlessly complicated bc:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
I also don't get how you argue about removing constants because they are used once and now say he should do it here |
||
| .maximumSize(10_000) | ||
| .expireAfterWrite(USER_COMMAND_COOLDOWN) | ||
| .build(); | ||
|
|
||
| private final Config config; | ||
| private final ModerationActionsStore actionsStore; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd put these 2 fields
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe, ill see
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. NIT - optional |
||
| private final Predicate<String> isModAuditLogChannel; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't understand the role of this field, it's not obvious and maybe useless. It's only being used in constructor and one method, so why not simply removing it, then you do this : private Optional<TextChannel> getModAuditLogChannel(MessageContextInteractionEvent event) {
// some code ..
Optional<TextChannel> modAuditLogChannel =
Guilds.findTextChannel(guild, Pattern.compile(config.getModAuditLogChannelPattern()).asMatchPredicate());
// some code ..
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. bc now u compile the pattern on each use. u want to compile it only once and then reuse it. regex is expensive. This is a standard technique used everywhere in the code base |
||
|
|
||
| /** | ||
| * 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); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i dont think it would be useful. remember that we already have JDA built-in logging for everything happening out the box. a metric might be interesting if anything |
||
| } | ||
| 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 -> { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You did check on Optional value already, why doing
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. what do u mean? this is how u get hands on the content of an optional. |
||
| 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 👍"; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure about putting emoji in code as a return, and check this out
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. emojis should be put into the code. our project has a fully working unicode setup and this boosts readability a lot. also, we do this everywhere in the code base |
||
| }).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()); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ill check |
||
|
|
||
| if (lastCommandUse == null) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it would be good to put
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. already covered by jda |
||
| isNotOnCooldownAction.run(); | ||
| return false; | ||
| } | ||
| Instant momentCooldownEnds = lastCommandUse.plus(USER_COMMAND_COOLDOWN); | ||
| if (Instant.now().isAfter(momentCooldownEnds)) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. For consistency:
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since the return type is
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd put as title
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. that would be worse as then its unclear what the bot wants from u. also, it might not even be scam after all. potential scam if anything. |
||
| .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")); | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
enum YesNo {
YES("Y", "Yes" true),
NO("N", "No", false);
private final String code;
private final String label;
private final boolean boolValue;
// getters and some utility methods ..
}
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. don't put together unrelated stuff. the yes/no here has nothing to do with yes/no choices elsewhere. putting such stuff in a common shared place will make stuff worse bc then u have to rip things apart again if one of them had different needs suddenly than the other. only put together related stuff |
||
| } | ||
|
|
||
| @SuppressWarnings("squid:S3457") // %n is wrong, markdown must use \n | ||
| private static String createDescription(Message target) { | ||
|
Comment on lines
+184
to
+185
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i'd rather say "empty description"
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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. i might improve the wording slightly though: |
||
|
|
||
| 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; | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can simplify do like this: boolean isScam = event.getButton().getStyle() == ButtonStyle.SUCCESS;
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. but then we don't have to opportunity anymore to give this otherwise confusing line a good variable name that helps give context |
||
|
|
||
| MessageEmbed resultEmbed = new EmbedBuilder() | ||
| .setDescription( | ||
| isScam ? "This is scam. The user was quarantined and messages were deleted." | ||
| : "This is not scam, no action executed.") | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd rather say
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would this not be the same as what he has now? Feel that would be unnecessarily verbose to add that.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. if anything ud use markdown highlighting by making it bold. |
||
| .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) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add log.debug for better future debugging
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. jda covers that already |
||
| 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
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe 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 :) |
||
There was a problem hiding this comment.
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
ScamReportCommandfor the class, and for the commandscam-reportThere was a problem hiding this comment.
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
There was a problem hiding this comment.
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