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

Added ban command #350

Merged
merged 5 commits into from
Aug 25, 2022
Merged

Added ban command #350

merged 5 commits into from
Aug 25, 2022

Conversation

mcpenguin
Copy link
Collaborator

Summary of Changes

Added a custom ban command for Codey.

Motivation and Explanation

Dyno has a generic ban command, but we wanted to make our own in the linked issue. This custom ban command bans the user, as well as sends a customized ban message to the bannee with the reason why they were banned, and how they can appeal the ban (message a mod).

Related Issues

Resolves #185

Steps to Reproduce

Run /ban or .ban.

Demonstration of Changes

image
image
The bannee gets a DM from Codey similar to the following:
image

Further Information and Comments

@adntaha
Copy link
Contributor

adntaha commented Aug 23, 2022

The code could also be adapted to make a "soft-ban" or "kick" command. Also, USER_ID_OF_MOD_FOR_APPEAL could be the mod that triggered the ban. Another thing, a specific permission for banning called BAN_MEMBERS (& KICK_MEMBERS for the other one).

Copy link
Collaborator

@Picowchew Picowchew left a comment

Choose a reason for hiding this comment

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

I think there should be another parameter that determines whether the user's past messages are deleted.

@@ -5,5 +5,6 @@
"ANNOUNCEMENTS_CHANNEL_ID": "CHANNEL_ID",
"OFFICE_STATUS_CHANNEL_ID": "CHANNEL_ID",
"RESUME_CHANNEL_ID": "CHANNEL_ID",
"IRC_USER_ID": "USER_ID"
"IRC_USER_ID": "USER_ID",
"USER_ID_OF_MOD_FOR_APPEAL": "USER_ID"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would MOD_USER_ID_FOR_BAN_APPEAL be more clear?


// Get the GuildMember object corresponding to the user in the guild
// This is needed because we can only ban GuildMembers, not Users
const guild = await client.guilds.fetch(vars.TARGET_GUILD_ID);
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you mind updating the description of TARGET_GUILD_ID in SETUP.md, since right now, it looks like it is only used for coffee chats?

src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
src/components/admin.ts Outdated Show resolved Hide resolved
src/components/admin.ts Outdated Show resolved Hide resolved
src/components/admin.ts Outdated Show resolved Hide resolved
@mcpenguin
Copy link
Collaborator Author

mcpenguin commented Aug 24, 2022

The code could also be adapted to make a "soft-ban" or "kick" command. Also, USER_ID_OF_MOD_FOR_APPEAL could be the mod that triggered the ban. Another thing, a specific permission for banning called BAN_MEMBERS (& KICK_MEMBERS for the other one).

I didn't add the kick/soft ban options since with each of these (I think) the user can just rejoin. With banning this isn't an option, so this functionality is needed.

@mcpenguin
Copy link
Collaborator Author

After further investigation, it turns out that Dyno, a bot in CSC, has more powerful ban commands that also support appeals:
https://dyno.gg/docs/en/commands/ban

No point reinventing the wheel, so I'm closing this PR.

@mcpenguin mcpenguin closed this Aug 24, 2022
@mcpenguin mcpenguin reopened this Aug 24, 2022
@mcpenguin
Copy link
Collaborator Author

On second glance, there might be more nuances to the Dyno ban command that make making a custom ban command more preferable. Reopening this PR for now.

Copy link
Collaborator

@Picowchew Picowchew left a comment

Choose a reason for hiding this comment

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

I left some comments.

config/production/vars.json Outdated Show resolved Hide resolved
config/staging/vars.json Outdated Show resolved Hide resolved
docs/SETUP.md Outdated Show resolved Hide resolved
src/commandDetails/admin/ban.ts Show resolved Hide resolved
src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
src/components/admin.ts Outdated Show resolved Hide resolved
src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
src/commandDetails/admin/ban.ts Outdated Show resolved Hide resolved
@mcpenguin mcpenguin force-pushed the 185-send-ban-message branch 2 times, most recently from 52407d3 to 18fce01 Compare August 25, 2022 03:45
},
{
name: 'days',
description: "Messages in last 'days' days from user are deleted. Default is no days.",
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can no be 0 instead? I think it would sound better this way.

Copy link
Collaborator

@Picowchew Picowchew left a comment

Choose a reason for hiding this comment

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

LGTM!

@mcpenguin mcpenguin merged commit 0a5b75a into main Aug 25, 2022
@mcpenguin mcpenguin deleted the 185-send-ban-message branch August 25, 2022 03:53
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.

[FEATURE] Attach somebody's discord @ to ban messages for appeal channel
3 participants