-
Notifications
You must be signed in to change notification settings - Fork 103
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
filters.py: Filter required users #517
base: master
Are you sure you want to change the base?
Conversation
utils/filters.py
Outdated
def filter_ignored_users(self, msg, cmd, args, dry_run): | ||
if msg.frm.nick in self.bot_config.IGNORE_USERNAMES: | ||
def filter_users(self, msg, cmd, args, dry_run): | ||
name = msg.frm.fullname if BACKEND == 'Zulip' else msg.frm.nick |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this PR isnt about Zulip. Please remove. This isnt how we will do Zulip anyway; the patches for Zulip are public.
@jayvdb Updated |
@gitmate-bot rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
Automated rebase with GitMate.io was successful! 🎉 |
Up for grabs, or is anyone working on it @jayvdb. |
@gitmate-bot rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
@gitmate-bot rebase |
Hey! I'm GitMate.io! This pull request is being rebased automatically. Please DO NOT push while rebase is in progress or your changes would be lost permanently |
Automated rebase with GitMate.io was successful! 🎉 |
utils/filters.py
Outdated
if msg.frm.nick in self.bot_config.IGNORE_USERNAMES: | ||
return None, None, None | ||
if msg.frm.nick not in self.bot_config.REQUIRED_USERS \ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove \
utils/filters.py
Outdated
if msg.frm.nick in self.bot_config.IGNORE_USERNAMES: | ||
return None, None, None | ||
if msg.frm.nick not in self.bot_config.REQUIRED_USERS \ | ||
and self.bot_config.RESP_ONLY_REQ_USER: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this must be indented four more spaces to avoid confusion with the statements inside the if
block
Travis tests have failedHey @Vamshi99, 1st Buildcoala --non-interactive -V
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ideally we need tests for this.
I think the tests should be easy to do now, and would be similar to the tests that @nvzard recently added for non-org members
c9d6cf1
to
f0c514d
Compare
If RESP_ONLY_REQ_USERS configuration is true, then stop responding to users other than those in the REQUIRED_USERS list in config. Closes coala#515
class FiltersTest(IsolatedTestCase): | ||
|
||
def test_filter_users(self): | ||
self.bot_config.RESP_ONLY_REQ_USERS = False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@nvzard Am I doing anything wrong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Vamshi99 IsolatedTestCase
loads all the plugins present in plugins/
directory except LabHub, hence this filter is not loaded by the bot since it is in utils/
dir.
Travis tests have failedHey @Vamshi99, 1st Buildpython -m pytest
|
If RESP_ONLY_REQ_USERS configuration is true,
then stop responding to users other than those
in the REQUIRED_USERS list in config.
Closes #515
Reviewers Checklist
botcmd
andre_botcmd
decorators.