Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions bot/cogs/duck_pond.py
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

from bot import constants
from bot.bot import Bot
from bot.utils.messages import send_attachments
from bot.utils.messages import send_attachments, sub_clyde

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -58,7 +58,7 @@ async def send_webhook(
try:
await self.webhook.send(
content=content,
username=username,
username=sub_clyde(username),
avatar_url=avatar_url,
embed=embed
)
Expand Down
3 changes: 2 additions & 1 deletion bot/cogs/python_news.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

from bot import constants
from bot.bot import Bot
from bot.utils.messages import sub_clyde

PEPS_RSS_URL = "https://www.python.org/dev/peps/peps.rss/"

Expand Down Expand Up @@ -208,7 +209,7 @@ async def send_webhook(self,

return await self.webhook.send(
embed=embed,
username=webhook_profile_name,
username=sub_clyde(webhook_profile_name),
avatar_url=AVATAR_URL,
wait=True
)
Expand Down
8 changes: 5 additions & 3 deletions bot/cogs/reddit.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
from bot.converters import Subreddit
from bot.decorators import with_role
from bot.pagination import LinePaginator
from bot.utils.messages import sub_clyde

log = logging.getLogger(__name__)

Expand Down Expand Up @@ -218,7 +219,8 @@ async def auto_poster_loop(self) -> None:

for subreddit in RedditConfig.subreddits:
top_posts = await self.get_top_posts(subreddit=subreddit, time="day")
message = await self.webhook.send(username=f"{subreddit} Top Daily Posts", embed=top_posts, wait=True)
username = sub_clyde(f"{subreddit} Top Daily Posts")
message = await self.webhook.send(username=username, embed=top_posts, wait=True)

if message.channel.is_news():
await message.publish()
Expand All @@ -228,8 +230,8 @@ async def top_weekly_posts(self) -> None:
for subreddit in RedditConfig.subreddits:
# Send and pin the new weekly posts.
top_posts = await self.get_top_posts(subreddit=subreddit, time="week")

message = await self.webhook.send(wait=True, username=f"{subreddit} Top Weekly Posts", embed=top_posts)
username = sub_clyde(f"{subreddit} Top Weekly Posts")
message = await self.webhook.send(wait=True, username=username, embed=top_posts)

if subreddit.lower() == "r/python":
if not self.channel:
Expand Down
1 change: 1 addition & 0 deletions bot/cogs/watchchannels/watchchannel.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,6 +204,7 @@ async def webhook_send(
embed: Optional[Embed] = None,
) -> None:
"""Sends a message to the webhook with the specified kwargs."""
username = messages.sub_clyde(username)
try:
await self.webhook.send(content=content, username=username, avatar_url=avatar_url, embed=embed)
except discord.HTTPException as exc:
Expand Down
22 changes: 20 additions & 2 deletions bot/utils/messages.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import asyncio
import contextlib
import logging
import re
from io import BytesIO
from typing import List, Optional, Sequence, Union

Expand Down Expand Up @@ -86,7 +87,7 @@ async def send_attachments(
else:
await destination.send(
file=attachment_file,
username=message.author.display_name,
username=sub_clyde(message.author.display_name),
avatar_url=message.author.avatar_url
)
elif link_large:
Expand All @@ -109,8 +110,25 @@ async def send_attachments(
else:
await destination.send(
embed=embed,
username=message.author.display_name,
username=sub_clyde(message.author.display_name),
avatar_url=message.author.avatar_url
)

return urls


def sub_clyde(username: Optional[str]) -> Optional[str]:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In what case username can be None? So much how I see, this should pass str every time.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

It's optional here though every use of that function passes a name.

async def webhook_send(
self,
content: Optional[str] = None,
username: Optional[str] = None,
avatar_url: Optional[str] = None,
embed: Optional[Embed] = None,
) -> None:
"""Sends a message to the webhook with the specified kwargs."""
username = messages.sub_clyde(username)

I don't see a problem with leaving it optional anyway.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

If this internally handles the None (and it does) then I agree the annotation is appropriate, but why does it handle it? Is it to be "shielded" from callers which may accidentally give a None? Won't that just push the error further, since something else that is expecting a string will get the declyded None?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

None is a valid value for webhook username (acts like an empty string and defaults to the webhook name) that may get passed in so I don't think there's any problem with letting None through.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

In that case I agree that this is perfectly fine.

"""
Replace "e"/"E" in any "clyde" in `username` with a Cyrillic "е"/"E" and return the new string.

Discord disallows "clyde" anywhere in the username for webhooks. It will return a 400.
Return None only if `username` is None.
"""
def replace_e(match: re.Match) -> str:
char = "е" if match[2] == "e" else "Е"
return match[1] + char

if username:
return re.sub(r"(clyd)(e)", replace_e, username, flags=re.I)
Comment on lines +120 to +132
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This will also return None if an empty string is given, which I suppose is fine if they are interchangeable in the webhook username field, but the docstring denies that. Explicitly checking for None on L131 seems like a better idea than adjusting the docstring though.

else:
return username # Empty string or None
27 changes: 27 additions & 0 deletions tests/bot/utils/test_messages.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import unittest

from bot.utils import messages


class TestMessages(unittest.TestCase):
"""Tests for functions in the `bot.utils.messages` module."""

def test_sub_clyde(self):
"""Uppercase E's and lowercase e's are substituted with their cyrillic counterparts."""
sub_e = "\u0435"
sub_E = "\u0415" # noqa: N806: Uppercase E in variable name

test_cases = (
(None, None),
("", ""),
("clyde", f"clyd{sub_e}"),
("CLYDE", f"CLYD{sub_E}"),
("cLyDe", f"cLyD{sub_e}"),
("BIGclyde", f"BIGclyd{sub_e}"),
("small clydeus the unholy", f"small clyd{sub_e}us the unholy"),
("BIGCLYDE, babyclyde", f"BIGCLYD{sub_E}, babyclyd{sub_e}"),
)

for username_in, username_out in test_cases:
with self.subTest(input=username_in, expected_output=username_out):
self.assertEqual(messages.sub_clyde(username_in), username_out)