Reviewing main so far - #4
Conversation
Added check to ignore direct messages in review listener.
Add checks for None results in review processing
Added check to ignore direct messages in review edits.
Add check for mirror_channel existence before fetching message.
Added checks for None parent_channel to prevent errors.
Bow gaming patch 1
Added detailed information about the Amalgam Discord bot, including its functionality, setup instructions, configuration, permissions, and requirements.
Rocked03
left a comment
Rocked03
left a comment
There was a problem hiding this comment.
Looking pretty sweet so far! I think we can do a decent bit to tidy it up though so that's what most of my comments are surrounding. lmk if you want me to explain anything clearer :)
| self.conn = sqlite3.connect("forward_reviews.db") | ||
| self.cursor = self.conn.cursor() | ||
|
|
||
| # Create tables if missing | ||
| # forward_reviews stores all db info needed to forward, edit and delete reviews | ||
| self.cursor.execute(""" | ||
| CREATE TABLE IF NOT EXISTS forward_reviews ( | ||
| original_id INTEGER PRIMARY KEY, | ||
| mirrored_channel_id INTEGER NOT NULL, | ||
| mirrored_id INTEGER NOT NULL | ||
| ) | ||
| """) | ||
| self.conn.commit() | ||
|
|
||
| # forward_threads stores all db info needed to determine what original thread belongs to what mirrored thread. This info is used in cog threads.py | ||
| self.cursor.execute(""" | ||
| CREATE TABLE IF NOT EXISTS forward_threads ( | ||
| original_thread_id INTEGER PRIMARY KEY, | ||
| mirrored_thread_id INTEGER NOT NULL, | ||
| owner_id INTEGER NOT NULL | ||
| ) | ||
| """) | ||
| self.conn.commit() |
There was a problem hiding this comment.
A few things:
- these should belong in their own function
- these should all be async as well otherwise we could have blocking issues
- this is duplicated in
threads.pywhich is typically just a code smell but because these are database operations being triggered automatically we gotta make sure we're not just creating a race condition
| CREATE TABLE IF NOT EXISTS forward_reviews ( | ||
| original_id INTEGER PRIMARY KEY, | ||
| mirrored_channel_id INTEGER NOT NULL, | ||
| mirrored_id INTEGER NOT NULL | ||
| ) |
There was a problem hiding this comment.
We can be a bit clearer with how we design our database here
Our main object here is a review, so that should live in a reviews table. The only information specific to a review is:
- the review message ID
- the review message guild
- the review author ID
- for each mirror
- the mirror message ID
- the mirror message guild
- (n.b. I know we only have one place we're mirroring to, but we should design with the ability to expand rather than fixing us in a spot where we can only mirror to one place)
This means our reviews table should only include the original message ID (primary (unique) key) and guild ID.
We should then have a separate table that forms our 1:N relationship, which we could call something like review_mirrors-- basically what we have currently. This table would have
- the original review message ID (primary key)
- the mirrored guild ID
- also primary key, so they become a primary key pair ie you can have multiple rows with the same message ID or same guild ID but each pair is unique
- the mirrored review message ID
And then when we use this we can fetch all rows matching the review message ID and edit mirrors for each guild listed
We can also ditch the threads table because thread channel IDs are identical to the message ID that the thread is created on
And we could also potentially even have a guilds table with guild ID (PK) and channel ID so that we have a solid mapping from guild ID to channel ID, meaning we can use the guild ID in the above tables rather than sorting things by individual channel. (and just then use LEFT JOIN when we make our queries)
This might be a little confusing, lmk if you want me to try to explain this better (hopefully with less 1am brain 😅)
| import sqlite3 | ||
|
|
||
|
|
||
| class ThreadCog(commands.Cog) : |
There was a problem hiding this comment.
a lot of my comments in review.py are also applicable here, largely due to a lot of the code being duplicated
a lot of the time, it's possible to deduplicate code, typically by moving it into a shared location
also I won't duplicate too many comments in here if I've already mentioned it in review.py but please do make sure you make changes in here too if relevant
| # Open database | ||
| self.conn = sqlite3.connect("forward_reviews.db") | ||
| self.cursor = self.conn.cursor() | ||
|
|
||
| # Create tables if missing | ||
| # forward_threads stores all db info needed to determine what original thread belongs to what mirrored thread. This info is gathered in cog review.py | ||
| self.cursor.execute(""" | ||
| CREATE TABLE IF NOT EXISTS forward_threads ( | ||
| original_thread_id INTEGER PRIMARY KEY, | ||
| mirrored_thread_id INTEGER NOT NULL, | ||
| owner_id INTEGER NOT NULL | ||
| ) | ||
| """) | ||
| self.conn.commit() | ||
|
|
||
| # forward_thread_messages stores all db info needed to determine what original thread belongs to what mirrored thread | ||
| self.cursor.execute(""" | ||
| CREATE TABLE IF NOT EXISTS forward_thread_messages ( | ||
| original_thread_message_id INTEGER PRIMARY KEY, | ||
| mirrored_thread_message_id INTEGER NOT NULL | ||
| ) | ||
| """) | ||
| self.conn.commit() |
There was a problem hiding this comment.
will reiterate here that this should really be moved into some shared helper class
3 participants
don't merge, just using this to add comments