diff --git a/CHANGELOG.md b/CHANGELOG.md index 0b16649..64b391b 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/docs/DONE.md b/docs/DONE.md index c3b65c0..a882ff9 100644 --- a/docs/DONE.md +++ b/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 diff --git a/docs/TODO.md b/docs/TODO.md index aa8cef7..6f5dcdb 100644 --- a/docs/TODO.md +++ b/docs/TODO.md @@ -4,15 +4,11 @@ File-specific lists are in loose descending order of priority. ## Current -N / A +N/A + ## Bugs -* [ ] Users can get multiple points for same solution - - Scenario: - - OP comments !solved - - OP deletes !solved comment - - OP comments !solved again - - Hopefully this will be easily solved once db migrations are working +* [ ] For some posts, the bot adds a point to the database, but crashes before being able to reply ## General diff --git a/pointsbot/bot.py b/pointsbot/bot.py index 69b6c40..50d78cc 100644 --- a/pointsbot/bot.py +++ b/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) + diff --git a/pointsbot/database.py b/pointsbot/database.py index 6113469..761b0f1 100644 --- a/pointsbot/database.py +++ b/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,8 +301,10 @@ class Database: def add_point_for_solution(self, submission, solver, solution_comment, chooser, chosen_by_comment): self._add_submission(submission) - self._add_comment(solution_comment, solver) - self._add_comment(chosen_by_comment, chooser) + 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) rowcount = self._add_solution(submission, solver, solution_comment, chosen_by_comment) @@ -265,7 +316,8 @@ class Database: return rowcount def soft_remove_point_for_solution(self, submission, solver, remover, removed_by_comment): - self._add_comment(removed_by_comment, remover) + 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: rowcount = self._update_points(solver, -1) @@ -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): - 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}) + # 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