Skip to content
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

Better Embeds, Pardons + Purges #11

Open
wants to merge 27 commits into
base: master
Choose a base branch
from

Conversation

DirectorReiuji
Copy link

@DirectorReiuji DirectorReiuji commented Nov 25, 2022

Additions

  • Redesign of all embeds, with Bandwidth-esque icons
  • Now detects member joins
  • Mod logs now implemented
  • Pardon and Purge commands
  • Joke Regexes (Current list is Redux, Rawr, 2014, Linux, and Chubby)
  • Switches out /setting-which to just Discord options
  • Bugfixes

Overhaulled all embeds within the bot, added mod logs, and first implantation of buttons.
@DirectorReiuji DirectorReiuji marked this pull request as draft November 25, 2022 09:26
Added purge.js command, which can either delete 2-99 messages or delete 100 messages from a user.
Added pardon command, removed useless ??? else comment, replaced generic icon in purge, and other misc. fixes.
@DirectorReiuji DirectorReiuji changed the title embed-overhaul embed-overhaul-and-misc Nov 29, 2022
@DirectorReiuji DirectorReiuji marked this pull request as ready for review November 29, 2022 03:49
Logs unbans and now follows the correct naming convention.
@DirectorReiuji DirectorReiuji changed the title embed-overhaul-and-misc Better Embeds, Pardons + Purges Nov 30, 2022
Fixed the major issue within this PR with nsfw-check.js, and fixed an error within guildMemberUpdate.js, which would crash Chubby if a timedout was revoked.
@DirectorReiuji
Copy link
Author

Ok, all issues I can think of has been fix, this should be able to be merged without causing too much issues.

DirectorReiuji and others added 7 commits December 1, 2022 19:41
Adds joke regexes, which will reply (within the select channel) with a certain prompt.
Adds the missing bracket of the switch function
Fixes problem where it'd ping the bot itself instead of the user, and removed useless switch function.
Some minor bugfixes, just to wrap the joke regexes perfectly.
Makes sure to check hierarchy before banning/kicking/warning, also fixes crashing in the case no reasoning is given when manually banning/kicking.
Replaces settings-which to discord options, similarly to Bandwidth.
Fixed a lil bug where it would say "Undefined cannot be warned". Also added Eyad's warnings suggestion.
Copy link
Member

@jonbarrow jonbarrow left a comment

Choose a reason for hiding this comment

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

There's quite a few spacing/indentation issues in various files, and several pieces of information that were there to give context to the user was removed without reason

example.config.json Outdated Show resolved Hide resolved
bansListEmbed.setColor(0xFFA500);
bansListEmbed.setTitle('User Bans');
bansListEmbed.setColor(0xa30000);
const image = new Discord.MessageAttachment('./src/images/mod/mod-ban.png');
Copy link
Member

Choose a reason for hiding this comment

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

This is a very awkward place to define this image variable, it should either be moved to above the embed creation or further down closer to where it's used

Copy link
Author

Choose a reason for hiding this comment

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

It's positioned there due to the fact of where it's used. However, I have moved it to be closer to the related functions.

Comment on lines -97 to -134
sendMemberEmbeds.push(banEmbed);

if (count > 0) {
const pastBansEmbed = new Discord.MessageEmbed();
pastBansEmbed.setTitle('Past Bans');
pastBansEmbed.setDescription('For clarifty purposes here is a list of your past bans');
pastBansEmbed.setColor(0xEF7F31);
pastBansEmbed.setTimestamp(Date.now());
pastBansEmbed.setFooter({
text: 'Pretendo Network',
iconURL: guild.iconURL()
});

for (let i = 0; i < rows.length; i++) {
const ban = rows[i];
const bannedBy = await interaction.client.users.fetch(ban.admin_user_id);

pastBansEmbed.addFields(
{
name: `${util.ordinal(i + 1)} Ban`,
value: ban.reason
},
{
name: 'Punished By',
value: bannedBy.tag,
inline: true
},
{
name: 'Date',
value: ban.timestamp.toLocaleDateString(),
inline: true
}
);
}

sendMemberEmbeds.push(pastBansEmbed);
}

Copy link
Member

Choose a reason for hiding this comment

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

What is the reason for this removal?

Copy link
Author

@DirectorReiuji DirectorReiuji Mar 6, 2024

Choose a reason for hiding this comment

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

It's due to possible errors with the new pardon system.

Copy link
Member

Choose a reason for hiding this comment

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

What possible errors? Can you be more specific?

src/commands/ban.js Outdated Show resolved Hide resolved
src/commands/ban.js Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

Is this neccessary? There's already a channel which logs this

Copy link
Author

Choose a reason for hiding this comment

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

Yeah, but it does clean it up a bit. If you do want me to simply use the default one, then I'll remove this.

Copy link
Member

Choose a reason for hiding this comment

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

No I'm saying we have a channel which logs joins. What is the point of this?
Screenshot from 2024-04-11 19-43-10

src/util.js Outdated
Comment on lines 38 to 42
if (components === null) {
await logChannel.send({ embeds: [embed], files: [file] });
} else {
await logChannel.send({ embeds: [embed], files: [file], components: [components] });
}
Copy link
Member

Choose a reason for hiding this comment

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

Is this check neccessary? Won't Discord.JS just not do anything with components if it's falsey?

Copy link
Author

Choose a reason for hiding this comment

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

Perhaps, might need some testing.

README.md Outdated Show resolved Hide resolved
src/events/messageCreate.js Outdated Show resolved Hide resolved

const { executor } = latestLog;

if (executor.id = guild.me.id) return;
Copy link
Member

Choose a reason for hiding this comment

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

Bad equals check

Copy link
Author

Choose a reason for hiding this comment

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

Possibly fixed?

@DirectorReiuji
Copy link
Author

Outside of spacing, most issues should be resolved. although in my end it's hard to deal with indents/spacing since i cannot discern which is which, and it seems like they do mix around indents with spacing, making fixing said issues harder. If you could do a quick re-review, that'd be great.

@jonbarrow
Copy link
Member

jonbarrow commented Mar 6, 2024

although in my end it's hard to deal with indents/spacing since i cannot discern which is which

There are tools for this. Depending on your editor there's usually a setting to show indentation. In VS Code this is the editor.renderWhitespace setting. Using all will render spaces with a dot and tabs as a small arrow

This project also uses ESLint for linting, and has a rule for indentation. Depending on your editor, there is usually some setting or plugin/extension which shows ESLint errors in the editor. You can also run npm run lint to run ESLint from the command line. For VS Code I use both:

Also for your browser there's an extension called Refined GitHub which adds many missing QoL features to the site, one of which is showing spaces vs tabs

If you could do a quick re-review, that'd be great.

I'll do another review once the spacing issues are fixed and the previous comments are resolved, just to save everyone some time

@DirectorReiuji
Copy link
Author

Fixed spacing and fixed some other stuff that was clearly broken via testing. Feel free to review now.

@DirectorReiuji
Copy link
Author

Actually, I have noticed that this one doesn’t have an easy way to see amount of kicks, so don’t merge the PR just yet until I add that, but a review of anything I need to fix/revise as of now would be good.

@DirectorReiuji
Copy link
Author

Alright, did all of the last minute tweaks, feel free to do a re-review.

Chubby can now delete polls
README.md Outdated
Comment on lines 5 to 11
Chubby can:
- Detect and remove NSFW content
- Warn, kick, and ban multiple users at once
- Pardon warns and kicks from multiple users
- Purge messages from a channel or from a specified user
- Log user and moderation events
- Remove Discord polls
Copy link
Member

Choose a reason for hiding this comment

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

You removed the names of the commands for these features?

Copy link
Author

Choose a reason for hiding this comment

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

Apologies, re-added them in the next commit.

bansListEmbed.setColor(0xa30000);
bansListEmbed.setThumbnail('attachment://mod-ban.png');

const image = new Discord.MessageAttachment('./src/images/mod/mod-ban.png');
Copy link
Member

Choose a reason for hiding this comment

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

Mostly a nitpick, but please try to define variables like these close to where they will be used. Right now the way code like this reads, it's not clear right away where image is used when going top down. If going bottom up then it's not clear where image comes from

Copy link
Author

Choose a reason for hiding this comment

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

Again, I can't get as close as I wish I could, however I will rename them to be more specific.

Copy link
Member

Choose a reason for hiding this comment

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

No you're correct. I missed where this was used inside the loop. This placement is fine


for (const userId of userIds) {
const member = await interaction.guild.members.fetch(userId);
const user = member.user;

// Checks if they're above/equal to the executor
Copy link
Member

Choose a reason for hiding this comment

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

This goes for all comments. We use the syntax from Better Comments now. I've been migrating our older code to use it, but newer code should use it by default

Copy link
Author

Choose a reason for hiding this comment

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

Ah, I use VS2022, so I'm not certain if I'll be able to see them, but nonetheless, I'll try implementing that after I resolve everything else.

Comment on lines 58 to 70
name: 'From bot /kick command',
name: 'From Bot',
Copy link
Member

Choose a reason for hiding this comment

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

It's already clear this is from the bot, that's the only way these messages get sent. The point of this naming convention was to note if the kick was specifically from calling the /kick command, or if it was from the result of multiple warnings. Both of those actions come from the bot, this is used to know WHICH action it was

Copy link
Author

Choose a reason for hiding this comment

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

Ahhh, gotcha. Sorry, I have reverted that change now.

Comment on lines 138 to 174
await util.sendEventLogMessage(guild, null, eventLogEmbed);

if (count > 0) {
const pastKicksEmbed = new Discord.MessageEmbed();
pastKicksEmbed.setTitle('Past Kicks');
pastKicksEmbed.setDescription('For clarifty purposes here is a list of your past kicks');
pastKicksEmbed.setColor(0xEF7F31);
pastKicksEmbed.setTimestamp(Date.now());
pastKicksEmbed.setFooter({
text: 'Pretendo Network',
iconURL: guild.iconURL()
});

for (let i = 0; i < rows.length; i++) {
const kick = rows[i];
const kickedBy = await interaction.client.users.fetch(kick.admin_user_id);

pastKicksEmbed.addFields(
{
name: `${util.ordinal(i + 1)} Kick`,
value: kick.reason
},
{
name: 'Punished By',
value: kickedBy.tag,
inline: true
},
{
name: 'Date',
value: kick.timestamp.toLocaleDateString(),
inline: true
}
);
}

sendMemberEmbeds.push(pastKicksEmbed);
}
Copy link
Member

Choose a reason for hiding this comment

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

What breaks? I'm confused what specific issues this caused. All this seems to do is removed additional context?

Comment on lines +35 to +48
warningListEmbed.addFields(
{
name: `${user.username}'s warns`,
value: count.toString()
},
{
name: 'Warnings Left Until Kick',
value: Math.max(0, 3 - count).toString()
},
{
name: 'Warnings Left Until Ban',
value: Math.max(0, 4 - count).toString()
}
);
Copy link
Member

Choose a reason for hiding this comment

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

Information about the warnings?

Copy link
Member

Choose a reason for hiding this comment

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

No I'm saying we have a channel which logs joins. What is the point of this?
Screenshot from 2024-04-11 19-43-10

Comment on lines 14 to 15
message.delete();
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
if (message.content == '' && message.attachments.size == 0)
message.delete();
if (!message.content.trim() && message.attachments.size == 0) {
await message.delete();
return
}

Comment on lines 26 to 27
eventLogEmbed.setTitle('_Poll Delete_');
eventLogEmbed.setDescription(`${user.username}'s poll in ${message.channel.name} has been deleted`);
Copy link
Member

Choose a reason for hiding this comment

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

Why was this changed to only mention polls...? This should trigger on any message being deleted?

Copy link
Author

Choose a reason for hiding this comment

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

It does, the function prior to this makes sure to only make these the string if the message was infact a poll. On any other occassion, it will show up as message deleted.

The image below shows off attachment deletion, poll deletion, and message deletion all working as intended.
image

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 unsure how I feel about using just the message content check to assume it's a poll. Messages seem to have a poll field now, according to this screenshot from Discord? Does Discord.js support this already? Checking for the actual poll data feels like a much better check imo
image (16)

Copy link
Author

Choose a reason for hiding this comment

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

No, Discord.js doesn’t support polls yet, which is why most of the poll deletion is hacky. Ideally, you would have some sort of “event/PollRemove.js” event which fires off when client detects a poll deletion.

Technically, you could call some function after poll deletion in “event/MessageCreate.js”, but again not ideal.

src/util.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants