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

coretasks: activate message-tags and use it to ignore other bots' tagged messages #2089

Merged
merged 2 commits into from
Sep 22, 2021

Conversation

dgw
Copy link
Member

@dgw dgw commented Jun 6, 2021

Description

Filter out PRIVMSG and NOTICE commands that have the bot (or draft/bot) tag. Other events should be left alone until there's a decorator to opt back into them, or Sopel itself will no longer even be aware of bot users' existence on networks that implement one of these tags (which per the spec "SHOULD" be added to all commands sent, and numerics caused, by a bot client).

A later pull request can introduce the needed decorator and changes to Rule constructors for certain callables to allow bot messages (à la @plugin.echo). Trying to keep stuff in manageable chunks so nobody gets burned out on reviews. 😸

Part of #2079.

Checklist

  • I have read CONTRIBUTING.md
  • I can and do license this contribution under the EFLv2
  • No issues are reported by make qa (runs make quality and make test)
  • I have tested the functionality of the things this change touches
    • Once again, by the power of pytest!

Notes

I really want to skip over the part where we have to support only draft/bot and then come back later to add support for the final bot tag name. Technically we shouldn't do that if the spec isn't ratified by the time Sopel 8 goes stable, though.

@dgw dgw added this to the 8.0.0 milestone Jun 6, 2021
@dgw dgw requested a review from a team June 6, 2021 11:41
@dgw dgw mentioned this pull request Jun 6, 2021
3 tasks
@dgw
Copy link
Member Author

dgw commented Jun 6, 2021

To @Exirel's point on IRC, I'm about to (force-)push a revision that prevents this from overriding the existing @plugin.echo behavior. The below console input is how I verified the approach:

>>> for b in [True, False]:
...     for e in [True, False]:
...         for a in [True, False]:
...             res = ((not b or (e and a)) and (not e or a))
...             print('bot: {}, echo: {}, allowecho: {}, result: {}'.format(b, e, a, res))
...
bot: True, echo: True, allowecho: True, result: True
bot: True, echo: True, allowecho: False, result: False
bot: True, echo: False, allowecho: True, result: False
bot: True, echo: False, allowecho: False, result: False
bot: False, echo: True, allowecho: True, result: True
bot: False, echo: True, allowecho: False, result: False
bot: False, echo: False, allowecho: True, result: True
bot: False, echo: False, allowecho: False, result: True

Elegant? No. Still useful.

Future preview

Went ahead and worked on some of the "later pull request" to add the tentatively named @plugin.allow_bots decorator. Verified that logic change (based on this branch) with:

for is_bot_message in [True, False]:
    for allow_bots in [True, False]:
        for is_echo_message in [True, False]:
            for allow_echo in [True, False]:
                res = (
                    (not is_bot_message or allow_bots) or
                    (is_echo_message and allow_echo)
                ) and (not is_echo_message or allow_echo)
                print('bot: {:d}, allowbots: {:d}, echo: {:d}, allowecho: {:d}, result: {}'.format(
                    is_bot_message, allow_bots, is_echo_message, allow_echo, res))
bot: 1, allowbots: 1, echo: 1, allowecho: 1, result: True
bot: 1, allowbots: 1, echo: 1, allowecho: 0, result: False
bot: 1, allowbots: 1, echo: 0, allowecho: 1, result: True
bot: 1, allowbots: 1, echo: 0, allowecho: 0, result: True
bot: 1, allowbots: 0, echo: 1, allowecho: 1, result: True
bot: 1, allowbots: 0, echo: 1, allowecho: 0, result: False
bot: 1, allowbots: 0, echo: 0, allowecho: 1, result: False
bot: 1, allowbots: 0, echo: 0, allowecho: 0, result: False
bot: 0, allowbots: 1, echo: 1, allowecho: 1, result: True
bot: 0, allowbots: 1, echo: 1, allowecho: 0, result: False
bot: 0, allowbots: 1, echo: 0, allowecho: 1, result: True
bot: 0, allowbots: 1, echo: 0, allowecho: 0, result: True
bot: 0, allowbots: 0, echo: 1, allowecho: 1, result: True
bot: 0, allowbots: 0, echo: 1, allowecho: 0, result: False
bot: 0, allowbots: 0, echo: 0, allowecho: 1, result: True
bot: 0, allowbots: 0, echo: 0, allowecho: 0, result: True

Preview of code changes in 61544fc, current (draft) HEAD of plugin.allow_bots branch.

@dgw
Copy link
Member Author

dgw commented Sep 18, 2021

Well I was going to properly rebase this after clicking through GitHub's conflict-resolution flow, but Coveralls is down and causing our builds to error. Just another Saturday, I guess. 😅

Note to self: rebased branch is on Lamarr ready to force-push whenever Coveralls comes back.

Only PRIVMSG and NOTICE messages from bots are filtered out, so this
change doesn't affect Sopel's user tracking. JOINs, PARTs, etc. can be
included later, once there exists a decorator that event handlers can
use to turn this `bot` filter off.

Currently supports the `draft/bot` tag AND the `bot` tag that will be
used when the relevant IRCv3 extension leaves draft status.

It seems like the future-proofing is of minimal risk here.
@dgw dgw merged commit 1d30887 into master Sep 22, 2021
@dgw dgw deleted the ignore-tagged-bots branch September 22, 2021 15:28
@dgw dgw mentioned this pull request Apr 12, 2022
10 tasks
@dgw dgw mentioned this pull request Apr 27, 2022
4 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants