Skip to content

Token and bad code#500

Merged
scragly merged 9 commits into
python-discord:masterfrom
kraktus:token_and_bad_code
Dec 12, 2019
Merged

Token and bad code#500
scragly merged 9 commits into
python-discord:masterfrom
kraktus:token_and_bad_code

Conversation

@kraktus
Copy link
Copy Markdown
Contributor

@kraktus kraktus commented Oct 7, 2019

This PR fix #389

Split the token_remover in two functions, one that checks that there is a token in the message (is_token_in_message), and the other takes the actions (deleting the message, etc.)
This new function is_token_in_message is reused in cogs/bot.py to know if the poorly formatted code contains a token. If it true, then the embed message is not displayed, since token_remover.py is already in charge.

Added a new function `is_token_in_message` in `token_remover`. This function returns a `bool` and if the code contains a token then the embed message about the poorly formatted code is not displayed.
After a new bunch of test I found bugs, and this fix resolves them
fix linting error
@MarkKoz MarkKoz added area: cogs t: bug Something isn't working labels Oct 10, 2019
@jchristgit jchristgit self-requested a review October 11, 2019 19:16
@jchristgit jchristgit self-assigned this Oct 11, 2019
Comment thread bot/cogs/token_remover.py Outdated
@kraktus kraktus requested a review from jchristgit October 12, 2019 12:56
Copy link
Copy Markdown
Contributor

@jchristgit jchristgit left a comment

Choose a reason for hiding this comment

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

One more thing. Aside from that LGTM

Comment thread bot/cogs/token_remover.py Outdated
sco1
sco1 previously requested changes Oct 14, 2019
Copy link
Copy Markdown
Contributor

@sco1 sco1 left a comment

Choose a reason for hiding this comment

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

The same checks need to be present for message edits as well.

@kraktus
Copy link
Copy Markdown
Contributor Author

kraktus commented Oct 15, 2019

@sco1 Hi, I am sorry but I am not sure about what you mean. Do you say the bot should also checks if a message contains a token every time a message is edited?

@sco1
Copy link
Copy Markdown
Contributor

sco1 commented Oct 15, 2019

Yes

@lemonsaurus
Copy link
Copy Markdown
Contributor

@kraktus any ETA on when you can add the requested changes from @sco1? We can't merge until that's done.

@kraktus
Copy link
Copy Markdown
Contributor Author

kraktus commented Oct 20, 2019

Pretty busy right now, will do it as soon as I can.

@kraktus kraktus requested review from jchristgit and sco1 October 20, 2019 19:37
@kraktus
Copy link
Copy Markdown
Contributor Author

kraktus commented Oct 20, 2019

Not sure about using a classmethod or a static method.

@kraktus
Copy link
Copy Markdown
Contributor Author

kraktus commented Nov 10, 2019

In case it is not clear, I changed my code to respond to the requested changes.

@scragly scragly dismissed stale reviews from sco1 and jchristgit November 15, 2019 18:52

Addressed

@scragly scragly added p: 1 - high High Priority a: filters Related to message filters: (antimalware, antispam, filtering, token_remover) labels Nov 15, 2019
Copy link
Copy Markdown
Contributor

@tagptroll1 tagptroll1 left a comment

Choose a reason for hiding this comment

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

Looks good, just a small comment.

Comment thread bot/cogs/bot.py Outdated
@lemonsaurus lemonsaurus added the s: waiting for author Waiting for author to address a review or respond to a comment label Nov 29, 2019
Include the check about whether or not there is a token in the posted message in `parse_codeblock` boolean.
@MarkKoz MarkKoz added status: needs review and removed s: waiting for author Waiting for author to address a review or respond to a comment labels Dec 12, 2019
@scragly
Copy link
Copy Markdown
Contributor

scragly commented Dec 12, 2019

A conflict was introduced here due to #681
Conflict is now resolved.

Copy link
Copy Markdown
Contributor

@scragly scragly left a comment

Choose a reason for hiding this comment

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

LGTM

@scragly scragly merged commit 48d8a3a into python-discord:master Dec 12, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: filters Related to message filters: (antimalware, antispam, filtering, token_remover) p: 1 - high High Priority t: bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Token remover and auto codeblock response conflict

8 participants