Swfarnsworth/fix auto upload #3247
Conversation
I manually tested the functionality implemented here.
bot/exts/filtering/filtering.py
Outdated
| async def _extract_text_file_content(att: discord.Attachment) -> str: | ||
| """Extract up to the first 30 lines and first 2000 characters (whichever is shorter) of an attachment.""" | ||
| file_encoding = re.search(r"charset=(\S+)", att.content_type).group(1) | ||
| file_lines: list[str] = (await att.read()).decode(encoding=file_encoding).splitlines() |
There was a problem hiding this comment.
Can we split this into multiple lines? I don't like the inline await and the bunch of stuff chained after.
| await _extract_text_file_content(a) | ||
| for a in msg.attachments if "charset" in a.content_type | ||
| ] | ||
| if text_contents: |
| DELETE_PASTE_EMOJI = Emojis.trashcan | ||
|
|
||
|
|
||
| class EmbedFileHandler(commands.Cog): |
There was a problem hiding this comment.
Not sure this class name makes sense.
| Handles automatic uploading of attachments to the paste bin. | ||
|
|
||
| Whenever a user uploads one or more attachments that is text-based (py, txt, csv, etc.), this cog offers to upload | ||
| all the attachments to the paste bin automatically. The steps are as follows: |
There was a problem hiding this comment.
Alignment of bullet items in this documentation list is wrong.
There was a problem hiding this comment.
Where would I find an example of them being used correctly?
| async def _convert_attachment(attachment: discord.Attachment) -> paste_service.PasteFile: | ||
| """Converts an attachment to a PasteFile, according to the attachment's file encoding.""" | ||
| encoding = re.search(r"charset=(\S+)", attachment.content_type).group(1) | ||
| file_content = (await attachment.read()).decode(encoding) |
| self.pending_messages.discard(message.id) | ||
|
|
||
| @commands.Cog.listener() | ||
| async def on_message(self, message: discord.Message) -> None: |
There was a problem hiding this comment.
This whole method is too long imo, and would benefit from being split up.
There was a problem hiding this comment.
I've written a new method that handles both instances of waiting for the user to do a certain reaction. I don't see any other opportunity for modularization that I think would actually improve comprehendability.
| ) | ||
| await bot_reply.add_reaction(PASTEBIN_UPLOAD_EMOJI) | ||
|
|
||
| def wait_for_upload_permission(reaction: discord.Reaction, user: discord.User) -> bool: |
There was a problem hiding this comment.
I don't like this inline predicate midway through the function, if we need to use variables from a higher scope then a method specifically to create the predicate at the top of the file is much cleaner.
| # Wait for the reaction with a timeout of 60 seconds. | ||
| await self.bot.wait_for("reaction_add", timeout=60.0, check=wait_for_upload_permission) | ||
| except TimeoutError: | ||
| # The user does not grant permission before the timeout. Exit early. |
There was a problem hiding this comment.
There is nothing here to actually exit early.
|
|
||
| # Wait for the user to react with a trash can, which they can use to delete the paste. | ||
|
|
||
| def wait_for_delete_reaction(reaction: discord.Reaction, user: discord.User) -> bool: |
There was a problem hiding this comment.
This predicate suffers from the same problem as above.
| @@ -68,6 +69,7 @@ async def test_message_with_illegal_extension(self): | |||
|
|
|||
| self.assertEqual(result, ({}, ["`.disallowed`"], {ListType.ALLOW: []})) | |||
|
|
|||
| @pytest.mark.xfail | |||
There was a problem hiding this comment.
If these tests passed before, they should pass now.
If they are no longer appropriate, please update the tests to support the new behaviour.
Previously, (txt, csv, json, and py) files evoked special behavior from the filtering system. This is no longer the case.
No description provided.