Miscellaneous bug fixes

CHANGELOG.md

@@ -1,6 +1,36 @@
 # Changelog
 
-## 2020/05/10
+## Version 0.2.1, 2021-04-24
+
+Features: N/A
+
+Fixes:
+1. Bot no longer crashes when a mod marks a deleted submission as solved
+2. Bot no longer crashes for solutions whose comment has already been stored in the db
+
+Miscellaneous: N/A
+
+## Version 0.2.0, 2021-03-07
+
+Features:
+1. Points can now be awarded to multiple comments on the same post
+2. The bot summoning keyword has been changed from "solved" to "helped"
+
+Fixes: N/A
+
+Miscellaneous:
+1. Introduced versioning for the bot and database
+
+## 2020-08-21
+
+Features: N/A
+
+Fixes:
+1. Removed freezing to comply with r/RequestABot guidelines
+
+Miscellaneous: N/A
+
+## 2020-05-10
 
 Features:
 1. Adding basic initial logging
@@ -12,12 +42,3 @@ Miscellaneous:
 1. Moved feedback & scoreboard links for bot reply into configuration
 2. Changed program entry point to `PointsBot.py`
 3. Added ability to freeze the app as a simple executable
-
-## 2020/08/21
-
-Features: N/A
-
-Fixes:
-1. Removed freezing to comply with r/RequestABot guidelines
-
-Miscellaneous: N/A

docs/DONE.md

@@ -8,6 +8,18 @@
 * [X] Allow multiple users to be awarded points on a single post
     * So just check whether each user is already awarded a point for a given post
 
+## Bugs
+* [X] mod /helped command doesn't work on one post (some posts?)
+    * the problem here seems to be the fact that when the bot adds a point for a solution comment, it
+    also adds the comment to the comments table without first checking whether it already exists
+    * while it is the case that we should only have one one `solution` row for each comment, it is potentially
+    possible that a comment could already be in the db when we add a solution for it, e.g. if the solution was
+    deleted (meaning the point was removed) and then we want to add it back, or something??
+* [X] bot crashes when a mod is trying to award a point to a solution on a deleted post
+    * the problem is that the new schema had the submission_id as a required foreign key, but a
+    deleted post will not have one
+    * according to them, since only mods will award points on deleted posts, it's fine if the fix is limited to mods
+
 ## File-Specific
 
 ### bot.py

docs/TODO.md

@@ -5,14 +5,10 @@ File-specific lists are in loose descending order of priority.
 ## Current
 
 N/A
+
 ## Bugs
 
-* [ ] Users can get multiple points for same solution
+* [ ] For some posts, the bot adds a point to the database, but crashes before being able to reply
-    - Scenario:
-        - OP comments !solved
-        - OP deletes !solved comment
-        - OP comments !solved again
-    - Hopefully this will be easily solved once db migrations are working
 
 ## General
 

pointsbot/bot.py

@@ -301,10 +301,6 @@ def find_solver_and_comment(solved_comment):
 ### Print & Logging Functions ###
 
 
-def print_separator_line():
-    print('#' * 80)
-
-
 def print_welcome_message():
     print_separator_line()
     print('\n*** Welcome to PointsBot! ***\n')
@@ -321,3 +317,7 @@ def print_welcome_message():
           "moment, this is what we've got to work with! :)\n")
     print_separator_line()
 
+
+def print_separator_line():
+    print('#' * 80)
+

pointsbot/database.py

@@ -71,6 +71,7 @@ class DatabaseVersion:
     def __eq__(self, other):
         return self._to_tuple() == other._to_tuple()
 
+    # TODO return string instead?
     def __hash__(self):
         return hash(self._to_tuple())
 
@@ -96,7 +97,8 @@ class DatabaseVersion:
 
 class Database:
 
-    LATEST_VERSION = DatabaseVersion(0, 2, 0)
+    # TODO why store this separately; could compute from SCHEMA_VERSION_STATEMENTS
+    LATEST_VERSION = DatabaseVersion(0, 2, 1)
 
     # TODO now that I'm separating these statements by version, I could probably make these
     # scripts instead of lists of individual statements...
@@ -153,6 +155,53 @@ class Database:
                 PRIMARY KEY (submission_rowid, author_rowid)
             )
             '''
+        ],
+        DatabaseVersion(0, 2, 1): [
+            # Remove constraints on `solution.submission_rowid` via a temp table
+            '''
+            CREATE TABLE temp_solution (
+                author_rowid INTEGER NOT NULL,
+                comment_rowid INTEGER NOT NULL,
+                chosen_by_comment_rowid INTEGER NOT NULL,
+                removed_by_comment_rowid INTEGER,
+                submission_rowid INTEGER,
+                FOREIGN KEY (author_rowid) REFERENCES redditor (rowid) ON DELETE CASCADE,
+                FOREIGN KEY (comment_rowid) REFERENCES comment (rowid) ON DELETE CASCADE,
+                FOREIGN KEY (chosen_by_comment_rowid) REFERENCES comment (rowid) ON DELETE SET NULL,
+                FOREIGN KEY (removed_by_comment_rowid) REFERENCES comment (rowid) ON DELETE SET NULL,
+                FOREIGN KEY (submission_rowid) REFERENCES submission (rowid) ON DELETE SET NULL
+            )
+            ''',
+            '''
+            INSERT INTO temp_solution (author_rowid, comment_rowid, chosen_by_comment_rowid, removed_by_comment_rowid, submission_rowid)
+            SELECT author_rowid, comment_rowid, chosen_by_comment_rowid, removed_by_comment_rowid, submission_rowid
+            FROM solution
+            ''',
+            '''
+            DROP TABLE solution;
+            ''',
+            '''
+            ALTER TABLE temp_solution RENAME TO solution
+            ''',
+
+            # Remove constraints on `submission.author_id` via a temp table
+           '''
+           CREATE TABLE temp_submission (
+                id TEXT UNIQUE NOT NULL,
+                author_id TEXT
+           )
+           ''',
+           '''
+           INSERT INTO temp_submission (id, author_id)
+           SELECT id, author_id
+           FROM submission
+           ''',
+           '''
+           DROP TABLE submission
+           ''',
+           '''
+           ALTER TABLE temp_submission RENAME TO submission
+           '''
         ]
     }
 
@@ -252,7 +301,9 @@ class Database:
 
     def add_point_for_solution(self, submission, solver, solution_comment, chooser, chosen_by_comment):
         self._add_submission(submission)
+        if not self._is_comment_already_saved(solution_comment):
             self._add_comment(solution_comment, solver)
+        if not self._is_comment_already_saved(chosen_by_comment):
             self._add_comment(chosen_by_comment, chooser)
 
         self._update_points(solver, 1)
@@ -265,6 +316,7 @@ class Database:
         return rowcount
 
     def soft_remove_point_for_solution(self, submission, solver, remover, removed_by_comment):
+        if not self._is_comment_already_saved(removed_by_comment):
             self._add_comment(removed_by_comment, remover)
         rowcount = self._soft_remove_solution(submission, solver, removed_by_comment)
         if rowcount > 0:
@@ -317,7 +369,7 @@ class Database:
 
         return points
 
-    ### Private Methods ###
+    ### Internal Methods ###
 
     def _get_submission_rowid(self, submission):
         return self._get_rowid_from_reddit_id('SELECT rowid FROM submission WHERE id = :reddit_id', {'reddit_id': submission.id})
@@ -348,13 +400,28 @@ class Database:
         self.cursor.execute(insert_stmt, params)
         return self.cursor.rowcount
 
+    @transaction
+    def _is_comment_already_saved(self, comment):
+        select_stmt = 'SELECT COUNT(*) AS comment_count FROM comment WHERE id = :id'
+        self.cursor.execute(select_stmt, {'id': comment.id})
+        row = self.cursor.fetchone()
+        return row['comment_count'] == 1
+
     @transaction
     def _add_submission(self, submission):
+        # A "deleted" submission does not have an author, so we need to check whether it exists
+        if submission.author:
             insert_stmt = '''
                 INSERT OR IGNORE INTO submission (id, author_id)
                 VALUES (:id, :author_id)
             '''
             self.cursor.execute(insert_stmt, {'id': submission.id, 'author_id': submission.author.id})
+        else:
+            insert_stmt = '''
+                INSERT OR IGNORE INTO submission (id)
+                VALUES (:id)
+            '''
+            self.cursor.execute(insert_stmt, {'id': submission.id})
         return self.cursor.rowcount
 
     @transaction