Conversation
|
I think explicit assertions (or inline snapshots) over snapshots in a separate file are overall easier to read. This aside, if a user disables DMs or blocks the bot, wouldn't |
It does right.. I think I remember something like that while building the ban spammer command. If the user have messages disabled, we will just simple catch the error and kick them anyway. |
|
@zachmmeyer We still need to take care of creating the role for kicking and configure it for Dyno |
|
@Mclilzee that would be on @TheOdinProject/core-team to add that in, maintainers don't have permissions like that on the server. |
mao-sz
left a comment
There was a problem hiding this comment.
Thanks @Mclilzee
I'll leave the main reviewing to another maint who's also part of the mod team since I'm not familiar with or participating in this feature discussion.
With #794 now merged, would you be able to rewrite the tests to use the new class-based mocks? If you need to edit those classes with the methods you need to mock then of course that's fine, like the channels.cache Collection for Guild.
Those mocks should be much closer to what discord.js actually uses when it runs and we'd have more unified mocks across the repo, less per-test suite mocking and context switching
|
@mao-sz I will get to it after we have configured the role that we require, can someone please take care of that? @TheOdinProject/core-team |
Role has been added, information has been shared in Discord. :) |
f7d0ef6 to
d772b90
Compare
|
@mao-sz I requested a review from you since I had to change how the mocks were created, some of them had to be changed which also reflected some changes in old tests. @zachmmeyer Since Mao requested a Mod-Maint you are already involved in this, so I will leave the rest to you. If you already have a playground discord server you will have to update the channel / role ID to reflect that otherwise I can set you up with the playground server I have set up where I test things on. |
mao-sz
left a comment
There was a problem hiding this comment.
I'll leave the actual feature review for someone else. No problem on the mock classes thing. There are additional changes to the mocks I need to make as part of #812, but given I've still got a fair chunk to do on that as well as this feature being more important, I'm happy to wait until this is done, then handle my changes with these ones merged. Would be tricky to coordinate the PRs otherwise.
|
@Mclilzee can I get that invite please? |
Mock changes implemented - prevent blocking feature review
Co-authored-by: mao-sz <122839503+mao-sz@users.noreply.github.com>
|
After discussion with @zachmmeyer we gonna test a new functionaliy to kick on spamm instead of on role. We found out that it might be hard to configure Dyno / Automod to add a custom role to users. Fortunately we already have messages listener, we just need to add a line to check if a particular message contains 4 or more attachments and if that user is not part of the admin roles we kick them. The kick service didn't have to change much, it only had to remove the role check and it should work the same. |
Because
Spammer that gets flagged to be muted after spamming multiple images will get kicked by our bot since Dyno is unable to handle it.
We still need to figure out which role we need for the kicking mechanics to kicks in. no pun intended. Currently, in config.js we are using muted ID, but we might need to reconsider and add new role that dyno will add it to a user like "kick" role.
We also need to test and see if those roles are persistence, if the user rejoins the server, and they still have the muted or kick role, then this makes our kick redundant.
This PR
Implements the kicking mechanics