Fix report regression & add threads to each report #536
Conversation
Reviewer's Guide by SourceryThis pull request improves the reporting system by fixing a regression that made reports non-anonymous and introduces a new feature to create a thread for each report. The changes are implemented in the File-Level Changes
Tips
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey @Atmois - I've reviewed your changes - here's some feedback:
Overall Comments:
- Please correct the spelling in the PR title from 'Reportimprovment' to 'Report Improvement'.
- Provide more detailed information on how you tested the anonymity fix and thread creation feature.
- Consider using a constant or configuration value instead of hardcoding 'tux' as the anonymous username.
Here's what I looked at during the review
- 🟡 General issues: 3 issues found
- 🟢 Security: all looks good
- 🟢 Testing: all looks good
- 🟢 Complexity: all looks good
- 🟢 Documentation: all looks good
Help me be more useful! Please click 👍 or 👎 on each comment to tell me if it was helpful.
| embed_type=EmbedCreator.INFO, | ||
| user_name=interaction.user.name, | ||
| user_display_avatar=interaction.user.display_avatar.url, | ||
| user_name="tux", |
There was a problem hiding this comment.
suggestion: Consider using a more generic term for anonymous reports
Instead of hardcoding "tux" as the user name for anonymous reports, consider using a more neutral term like "Anonymous" or "System". This would make the purpose of the change clearer and avoid potential confusion.
| user_name="tux", | |
| user_name="Anonymous", |
| await report_log_channel.create_thread( | ||
| name=f"Anonymous report for {self.short.value}", # type: ignore | ||
| message=message, | ||
| auto_archive_duration=10080, |
There was a problem hiding this comment.
suggestion: Extract magic number to a named constant or configuration value
The value 10080 for auto_archive_duration is a magic number. Consider extracting this to a named constant or configuration value for better maintainability and clarity.
AUTO_ARCHIVE_DURATION = 10080 # 7 days in minutes
class ReportModal(discord.ui.Modal):
# ... existing code ...
async def on_submit(self, interaction: discord.Interaction) -> None:
# ... existing code ...
await report_log_channel.create_thread(
name=f"Anonymous report for {self.short.value}", # type: ignore
message=message,
auto_archive_duration=AUTO_ARCHIVE_DURATION,
)
|
|
||
| await report_log_channel.send(embed=embed) | ||
| message = await report_log_channel.send(embed=embed) | ||
| await report_log_channel.create_thread( |
There was a problem hiding this comment.
suggestion: Add error handling for thread creation
Consider adding a try-except block around the thread creation to handle potential errors, especially since this is an asynchronous operation. This would improve the robustness of the code.
try:
await report_log_channel.create_thread(
name=f"Anonymous report for {self.short.value}", # type: ignore
message=message,
)
except discord.HTTPException as e:
logging.error(f"Failed to create thread: {e}")
# Handle the error appropriately, e.g., send a message to a backup channel
Atmois
changed the title
Description
Fixes a regression making reports non-anonymous aswell as now creates a thread for each post.
Guidelines
My code follows the style guidelines of this project (formatted with Ruff)
I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation if needed
My changes generate no new warnings
I have tested this change
Any dependent changes have been merged and published in downstream modules
I have added all appropriate labels to this PR
I have followed all of these guidelines.
How Has This Been Tested? (if applicable)
n/a
Screenshots (if applicable)
Additional Information
n/a