Skip to content

Allow !ban, !vban, and !pban to take a duration#1564

Merged
Xithrius merged 7 commits into
python-discord:mainfrom
Shivansh-007:enhancement/ban
May 8, 2021
Merged

Allow !ban, !vban, and !pban to take a duration#1564
Xithrius merged 7 commits into
python-discord:mainfrom
Shivansh-007:enhancement/ban

Conversation

@Shivansh-007
Copy link
Copy Markdown
Contributor

@Shivansh-007 Shivansh-007 commented May 5, 2021

Closes #1563
Closes #1566

Apply temporary ban if duration is specified while banning a user.

Screenshots

Applying a ban with duration

Screenshot from 2021-05-05 06-45-34

Proof in #all-logs

Screenshot from 2021-05-05 06-46-26

Comment thread bot/exts/moderation/infraction/infractions.py Outdated
Comment thread bot/exts/moderation/infraction/infractions.py Outdated
@Xithrius Xithrius added a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: enhancement Changes or improvements to existing features labels May 5, 2021
@Shivansh-007 Shivansh-007 requested a review from Xithrius May 5, 2021 12:05
Copy link
Copy Markdown
Contributor

@ToxicKidz ToxicKidz left a comment

Choose a reason for hiding this comment

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

:shipit:

@mbaruh
Copy link
Copy Markdown
Member

mbaruh commented May 5, 2021

It looks good in the context of the issue, though it creates inconsistency with pban and vban

Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

To make this consistent with other commands, please make purgeban not take any argument for purge days, but instead take only ban days. If time exists, don't permaban.

Please also do the same thing for voiceban, where if a time is supplied, it won't be permanent.

Full explanation: #1566.

@Shivansh-007 Shivansh-007 requested a review from Xithrius May 6, 2021 04:38
@Shivansh-007 Shivansh-007 requested a review from mbaruh May 7, 2021 05:13
@mbaruh
Copy link
Copy Markdown
Member

mbaruh commented May 7, 2021

You're still going over the character limit for commit titles :)
Keep them concise. Any details of context or reasoning can go into the body.

Also not sure why you're prepending the (infractions) part.

@Shivansh-007 Shivansh-007 reopened this May 7, 2021
Comment thread bot/exts/moderation/infraction/infractions.py Outdated
Comment thread bot/exts/moderation/infraction/infractions.py Outdated
Comment thread bot/exts/moderation/infraction/infractions.py Outdated
Co-authored-by: Boris Muratov <8bee278@gmail.com>
@mbaruh mbaruh changed the title When duration specified for !ban, apply a !tempban Allow !ban, !vban, and !pban to take a duration May 7, 2021
@Shivansh-007 Shivansh-007 requested a review from mbaruh May 7, 2021 12:42
Comment thread bot/exts/moderation/infraction/infractions.py Outdated
@Shivansh-007 Shivansh-007 requested a review from mbaruh May 7, 2021 14:07
Copy link
Copy Markdown
Member

@mbaruh mbaruh left a comment

Choose a reason for hiding this comment

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

This can be an opportunity to add some more related tests, but you don't have to.

Copy link
Copy Markdown
Contributor

@Xithrius Xithrius left a comment

Choose a reason for hiding this comment

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

Looks very well to me.

@Xithrius Xithrius enabled auto-merge May 8, 2021 00:24
@Xithrius Xithrius merged commit a727ca1 into python-discord:main May 8, 2021
@Shivansh-007 Shivansh-007 deleted the enhancement/ban branch May 8, 2021 00:27
@Shivansh-007
Copy link
Copy Markdown
Contributor Author

This can be an opportunity to add some more related tests, but you don't have to.

Tests for the new feature I added in this PR, or the complete pban and ban command?

@mbaruh
Copy link
Copy Markdown
Member

mbaruh commented May 8, 2021

For what you added.

@Shivansh-007
Copy link
Copy Markdown
Contributor Author

For what you added.

Alright, I will do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: backend Related to internal functionality and utilities (error_handler, logging, security, utils and core) a: moderation Related to community moderation functionality: (moderation, defcon, verification) p: 3 - low Low Priority t: enhancement Changes or improvements to existing features

Projects

None yet

4 participants