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

Ban multiple users in a single message #24

Closed
MKRhere opened this issue Oct 2, 2017 · 6 comments
Closed

Ban multiple users in a single message #24

MKRhere opened this issue Oct 2, 2017 · 6 comments
Labels
enhancement This issue/pr proposes/adds a feature to the bot

Comments

@MKRhere
Copy link
Member

MKRhere commented Oct 2, 2017

The /ban command can take multiple usernames as parameters to ban multiple users at the same time. Could be useful during a flood.

Example implementation:

> admin: /ban @user1083 @user1976 @user9176 Flood!
> bot: @admin banned @user1083, @user1976, and @user9176
Reason: Flood!
@MKRhere MKRhere added the enhancement This issue/pr proposes/adds a feature to the bot label Oct 2, 2017
@wojpawlik
Copy link
Contributor

Not sure how useful would it be with usernames
I'd definietly be useful with ids

The Engine has that... kinda: https://github.com/lkd70/the-engine/blob/943bb1aa0b02daaf0476238e1b4236ef7f950274/plugins/ban.js#L37-L50

@wojpawlik wojpawlik moved this from Ideas to To be implemented eventually in The Guard Bot's main board Oct 2, 2017
@wojpawlik wojpawlik moved this from To be implemented eventually to In progress in The Guard Bot's main board Jun 16, 2018
@wojpawlik wojpawlik moved this from In progress to Todo 0.9 in The Guard Bot's main board Jun 16, 2018
@thedevs-network thedevs-network deleted a comment from MKRhere Jun 16, 2018
@wojpawlik
Copy link
Contributor

We settled on implementing batch ban as a separate command, /batchban.

Simon 'SitiSchu' Schürrle, [15.06.18 18:34]
or do an extra command

Ginger++, [15.06.18 18:37]
[In reply to Simon 'SitiSchu' Schürrle]
that's an interesting idea

Ginger++, [15.06.18 18:37]
/batchban

Simon 'SitiSchu' Schürrle, [15.06.18 18:37]
ye might be better than switching the syntax

Ginger++, [15.06.18 18:37]
which only accepts ids, not usernames, not mentions

Simon 'SitiSchu' Schürrle, [15.06.18 18:37]
I guess ye

Simon 'SitiSchu' Schürrle, [15.06.18 18:37]
should also be faster

Simon 'SitiSchu' Schürrle, [15.06.18 18:37]
if you dont do mentions

Ginger++, [15.06.18 18:38]
I mean, it'd be cool to have that all in 1 command, but it ain't so simple

We need to accept ids, we could accept usernames, but we shouldn't bother with inline mentions, at least not initially.

I guess we kind of agreed on separating ban reason from ids by newline, there's no one opposing that.

Ginger++, [15.06.18 18:46]
well, what if reason is multi-word?

μ2, [15.06.18 18:46]
[In reply to Ginger++]
Put them in quotes?

μ2, [15.06.18 18:46]
Or mark with line break

Simon 'SitiSchu' Schürrle, [15.06.18 18:46]
[In reply to Ginger++]
heh

Simon 'SitiSchu' Schürrle, [15.06.18 18:46]
theres a reason I use "Spam(SpamWatch)" as reason lol

Thomas, [15.06.18 18:47]
[In reply to Ginger++]
Reason is all space-separated tokens until there are only numerics left

μ2, [15.06.18 18:47]
[In reply to Thomas]
What if reason ends with a number?

Simon 'SitiSchu' Schürrle, [15.06.18 18:47]
btw if its  on the end youd have the same problem

Simon 'SitiSchu' Schürrle, [15.06.18 18:47]
so quotes is the best way I guess

μ2, [15.06.18 18:48]
[In reply to μ2]
This is better I think

μ2, [15.06.18 18:48]
Just linebreak before you start IDs

Thomas, [15.06.18 18:48]
[In reply to μ2]
Then that won't be counted

Thomas, [15.06.18 18:48]
I'll do linebreak if simpler

Simon 'SitiSchu' Schürrle, [15.06.18 18:52]
it would skip the requirement to parse quotes

Simon 'SitiSchu' Schürrle, [15.06.18 18:52]
so

Simon 'SitiSchu' Schürrle, [15.06.18 18:52]
[command]
[reason]
[space seperated list of ids]

@wojpawlik wojpawlik self-assigned this Jun 16, 2018
@wojpawlik wojpawlik moved this from Todo 0.9 to In progress in The Guard Bot's main board Jun 17, 2018
@wojpawlik
Copy link
Contributor

wojpawlik commented Jun 24, 2018

It's impossible to batchban in 1 Mongo / nedb query, due to upsert only inserting up to 1 record, only if no record matched.

The best workaround I came up with would be to try to insert a dummy { id, status: 'member' } record for each user to ban which doesn't exist in database (not atomic, but not an issue), and then atomically update (not upsert) them all to be banned.

@trgwii
Copy link
Member

trgwii commented Jan 28, 2019

This is a problem is because we don't want to have a situation where some of the bans succeed and others fail?

@trgwii
Copy link
Member

trgwii commented Jan 28, 2019

I think it would be fine if the bot just listed the ones that failed (if any)

@wojpawlik
Copy link
Contributor

I think I'll start with naive implementation

The Guard Bot's main board automation moved this from In progress to Done Jan 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement This issue/pr proposes/adds a feature to the bot
Projects
None yet
Development

No branches or pull requests

3 participants