Migrate kick, ban and unban command #244

Zabuzard · 2021-11-03T09:23:09Z

Overview

Continuation from #196 .

Adds the ban, kick and unban commands.

Partially implements #63 and #64 .

Role based instead of Permissions

The moderation commands for this bot are role based instead of being based on permissions. For that, the config introduces two new entries:

"heavyModerationRolePattern": "Moderator",
"softModerationRolePattern": "Moderator|Staff Assistant"

This allows us to give roles, for example Staff Assistant the possibility to kick(mute, delete-messages, ...) without actually having permission to do so in Discord. That way, we can force them to use the bot instead of other bots or the Discord GUI - making it easier for us to control the flow and in particular give visibility through audit logs, since all actions have to go through our bot now.

Test Checklist

Before merging, we now have to test the following scenarios:

Base cases:

Ban an user
Kick an user
Unban a banned user

Permissions:

Ban without having ban perms
Kick without having kick perms
Unban without having ban perms
Ban without the bot having ban perms
Kick without the bot having kick perms
Unban without the bot having ban perms

Interaction power:

Ban users higher up in the hierarchy than the author
Kick users higher up in the hierarchy than the author
Ban users higher up in the hierarchy than the bot
Kick users higher up in the hierarchy than the bot

DM:

Ban (or kick) an user who enabled DMs from non-friends
Ban (or kick) an user who disabled DMs from non-friends

Reason:

Ban (or kick, or unban) an user with a reason longer than 512 characters

Logic:

Unban a user who is not banned (found issue, test again after issue fixed)
Ban an already banned user (found issue, test again after issue fixed)
Kick an already kicked user
Ban a user who does not exist
Kick a user who does not exist
Unban a user who does not exist

RealYusufIsmail · 2021-11-03T09:25:30Z

Do i still get credits @Zabuzard

Zabuzard · 2021-11-03T09:26:52Z

Do i still get credits @Zabuzard

Of course. Look at the commit history, the first commit is still by you.

RealYusufIsmail · 2021-11-03T09:27:32Z

Do i still get credits @Zabuzard

Of course. Look at the commit history, the first commit is still by you.

Ok thank you.

application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java

RealYusufIsmail · 2021-11-03T09:31:48Z

SonarCloud Quality Gate failed.

0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells

0.0% Coverage 19.2% Duplication

Should i try and fix this

Zabuzard · 2021-11-03T09:32:39Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells
0.0% Coverage 19.2% Duplication

Should i try and fix this

You cant contribute to this branch since you lack write permission. Ill just take over and finish this quickly.

RealYusufIsmail · 2021-11-03T09:34:32Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells
0.0% Coverage 19.2% Duplication

Should i try and fix this

You cant contribute to this branch since you lack write permission. Ill just take over and finish this quickly.

Can I get write permission?

Zabuzard · 2021-11-03T09:38:19Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells
0.0% Coverage 19.2% Duplication

Should i try and fix this

You cant contribute to this branch since you lack write permission. Ill just take over and finish this quickly.

Can I get write permission?

Github unfortunately does not allow that. Otherwise I would have continued in your original PR.

java-coding-prodigy · 2021-11-03T09:38:27Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells
0.0% Coverage 19.2% Duplication

Should i try and fix this

You cant contribute to this branch since you lack write permission. Ill just take over and finish this quickly.

Can I get write permission?

Once this gets merged, @Zabuzard will send you an invite to our GitHub team and you will be able to get write access to the base repository itself.

RealYusufIsmail · 2021-11-03T09:46:12Z

SonarCloud Quality Gate failed.
0 Bugs 0 Vulnerabilities 0 Security Hotspots 3 Code Smells
0.0% Coverage 19.2% Duplication

Should i try and fix this

You cant contribute to this branch since you lack write permission. Ill just take over and finish this quickly.

Can I get write permission?

Once this gets merged, @Zabuzard will send you an invite to our GitHub team and you will be able to get write access to the base repository itself.

Ok thank you.

RealYusufIsmail

Yeah all looks good.

RealYusufIsmail · 2021-11-03T10:13:48Z

@Tais993 @java-coding-prodigy Can you approve this

Zabuzard · 2021-11-03T10:16:00Z

@RealYusufIsmail Please check out this branch and do all the tests from the checklist I just added. Blocking this PR until everything has been tested.

RealYusufIsmail · 2021-11-03T10:17:17Z

@RealYusufIsmail Please check out this branch and do all the tests from the checklist I just added. Blocking this PR until everything has been tested.

Ok will I do when i go home

java-coding-prodigy · 2021-11-03T10:19:04Z

Just curious, why is ModerationUtils an enum and not a class?

Zabuzard · 2021-11-03T10:22:53Z

Just curious, why is ModerationUtils an enum and not a class?

Utility-enum-pattern. The safest option to forbid creation of instances, safer than private constructors and other options.

RealYusufIsmail · 2021-11-03T15:07:54Z

@Zabuzard the bot can not dm users who have there dms disabled for non-friends I have tested it

.gitignore

application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java

application/src/main/java/org/togetherjava/tjbot/commands/moderation/KickCommand.java

application/src/main/java/org/togetherjava/tjbot/commands/moderation/UnbanCommand.java

gets rid of code duplication (as flagged by sonar)

* introduced helper method `handleChecks` where all the preconditions are checked (reason, permissions, ...)

* this allows to give selected roles the ability to use moderation commands without giving them the ability to use them without the bot. * for example staff assistants could kick users with the bot, but not without the bot * that way, we can force them to go through the bot instead of using Discord GUI or other bots * which gives us more control and visibility

CLAassistant · 2021-11-15T09:55:59Z

All committers have signed the CLA.

sonarqubecloud · 2021-11-15T10:03:12Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
0 Code Smells

0.0% Coverage
0.0% Duplication

Implemented

Zabuzard · 2021-11-16T08:35:40Z

Merging since 7 days after last unapproved code change.
Was accepted by marko, Yusuf and Tais in the meantime already.

Zabuzard added the new command Add a new command or group of commands to the bot label Nov 3, 2021

Zabuzard added this to the Migrate existing commands milestone Nov 3, 2021

Zabuzard self-assigned this Nov 3, 2021

Zabuzard requested review from a team as code owners November 3, 2021 09:23

Zabuzard mentioned this pull request Nov 3, 2021

Migrate kick, ban and unban command #196

Closed

RealYusufIsmail reviewed Nov 3, 2021

View reviewed changes

application/src/main/java/org/togetherjava/tjbot/commands/moderation/BanCommand.java Outdated Show resolved Hide resolved

Zabuzard force-pushed the feature/kick_and_ban_commands branch from c57f5d4 to 1ee5544 Compare November 3, 2021 10:00

RealYusufIsmail previously approved these changes Nov 3, 2021

View reviewed changes

Zabuzard requested review from d3vel0per, illuminator3, java-coding-prodigy, marko-radosavljevic and Tais993 November 3, 2021 10:16

illuminator3 reviewed Nov 3, 2021

View reviewed changes

.gitignore Outdated Show resolved Hide resolved

Tais993 previously requested changes Nov 14, 2021

View reviewed changes

RealYusufIsmail and others added 20 commits November 15, 2021 10:21

Added moderation commands (Ban, Kick, Unban)

92eba7e

Fixed code flaws

54fafc6

Linter issues

9329ce5

Readability fix (CR yusuf)

0060f60

Moved several handle methods into helper

4db3cc6

gets rid of code duplication (as flagged by sonar)

Fixed some typos

445d617

Not banning already banned users

a1c12f7

Added "unable to send DM" notice

db3d023

Fixed bug with unbanning non-banned users

2f1aba0

Fixed some typos

9c8016f

Using embed instead of plain messages

b720238

using constants to separate magic strings

427c224

Readability cleanup on the "checks"

f7d565f

* introduced helper method `handleChecks` where all the preconditions are checked (reason, permissions, ...)

NotNull for enum (CR by Tais)

c2dd08d

Added back permission based system, additionally as secondary layer

5fec89f

Spotless after rebase

ecad42d

empty line for readability (CR Tais)

0460a19

Simplified ban/kick flow using individual methods (CR Tais)

c71ff66

Moved failure handling for unban into method (CR Tais)

8b48e47

Zabuzard force-pushed the feature/kick_and_ban_commands branch from bff063f to 8b48e47 Compare November 15, 2021 09:55

Missing imports after rebase

60190a5

Zabuzard requested a review from Tais993 November 15, 2021 10:04

Zabuzard merged commit 036b893 into develop Nov 16, 2021

Zabuzard deleted the feature/kick_and_ban_commands branch November 16, 2021 08:35

Uh oh!

Migrate kick, ban and unban command #244

Migrate kick, ban and unban command #244

Uh oh!

Conversation

Zabuzard commented Nov 3, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Overview

Role based instead of Permissions

Test Checklist

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Zabuzard commented Nov 3, 2021

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Zabuzard commented Nov 3, 2021

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Zabuzard commented Nov 3, 2021

Uh oh!

java-coding-prodigy commented Nov 3, 2021

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

RealYusufIsmail left a comment

Choose a reason for hiding this comment

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Zabuzard commented Nov 3, 2021

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

java-coding-prodigy commented Nov 3, 2021

Uh oh!

Zabuzard commented Nov 3, 2021

Uh oh!

RealYusufIsmail commented Nov 3, 2021

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CLAassistant commented Nov 15, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sonarqubecloud bot commented Nov 15, 2021

Uh oh!

Zabuzard commented Nov 16, 2021

Uh oh!

Uh oh!

Zabuzard commented Nov 3, 2021 •

edited

Loading

CLAassistant commented Nov 15, 2021 •

edited

Loading