From 06333a8417219ddb6e18b1946b8ad89a60d1f194 Mon Sep 17 00:00:00 2001 From: Deimos Date: Wed, 8 Aug 2018 19:37:14 -0600 Subject: [PATCH] Update comments triggers to handle removals Well, this alembic migration was a gigantic pain in the ass. Might be worth writing a tool to be able to deal with things like this. --- ...04_update_comment_triggers_for_removals.py | 328 ++++++++++++++++++ .../comments/comment_notifications.sql | 5 +- .../sql/init/triggers/comments/comments.sql | 30 +- .../init/triggers/comments/topic_visits.sql | 27 +- tildes/sql/init/triggers/comments/topics.sql | 46 ++- tildes/tildes/models/comment/comment.py | 16 +- 6 files changed, 414 insertions(+), 38 deletions(-) create mode 100644 tildes/alembic/versions/fab922a8bb04_update_comment_triggers_for_removals.py diff --git a/tildes/alembic/versions/fab922a8bb04_update_comment_triggers_for_removals.py b/tildes/alembic/versions/fab922a8bb04_update_comment_triggers_for_removals.py new file mode 100644 index 0000000..2851c1d --- /dev/null +++ b/tildes/alembic/versions/fab922a8bb04_update_comment_triggers_for_removals.py @@ -0,0 +1,328 @@ +"""Update comment triggers for removals + +Revision ID: fab922a8bb04 +Revises: f1ecbf24c212 +Create Date: 2018-08-09 00:56:40.718440 + +""" +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision = 'fab922a8bb04' +down_revision = 'f1ecbf24c212' +branch_labels = None +depends_on = None + + +def upgrade(): + # comment_notifications + op.execute("DROP TRIGGER delete_comment_notifications_update ON comments") + op.execute(""" + CREATE TRIGGER delete_comment_notifications_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN ((OLD.is_deleted = false AND NEW.is_deleted = true) + OR (OLD.is_removed = false AND NEW.is_removed = true)) + EXECUTE PROCEDURE delete_comment_notifications(); + """) + + # comments + op.execute(""" + CREATE OR REPLACE FUNCTION set_comment_deleted_time() RETURNS TRIGGER AS $$ + BEGIN + IF (NEW.is_deleted = TRUE) THEN + NEW.deleted_time := current_timestamp; + ELSE + NEW.deleted_time := NULL; + END IF; + + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER delete_comment_set_deleted_time_update ON comments") + op.execute(""" + CREATE TRIGGER delete_comment_set_deleted_time_update + BEFORE UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + EXECUTE PROCEDURE set_comment_deleted_time(); + """) + + op.execute(""" + CREATE OR REPLACE FUNCTION set_comment_removed_time() RETURNS TRIGGER AS $$ + BEGIN + IF (NEW.is_removed = TRUE) THEN + NEW.removed_time := current_timestamp; + ELSE + NEW.removed_time := NULL; + END IF; + + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + """) + op.execute(""" + CREATE TRIGGER remove_comment_set_removed_time_update + BEFORE UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed IS DISTINCT FROM NEW.is_removed) + EXECUTE PROCEDURE set_comment_removed_time(); + """) + + # topic_visits + op.execute("DROP TRIGGER update_topic_visits_num_comments_update ON comments") + op.execute("DROP FUNCTION decrement_all_topic_visit_num_comments()") + op.execute(""" + CREATE OR REPLACE FUNCTION update_all_topic_visit_num_comments() RETURNS TRIGGER AS $$ + DECLARE + old_visible BOOLEAN := NOT (OLD.is_deleted OR OLD.is_removed); + new_visible BOOLEAN := NOT (NEW.is_deleted OR NEW.is_removed); + BEGIN + IF (old_visible AND NOT new_visible) THEN + UPDATE topic_visits + SET num_comments = num_comments - 1 + WHERE topic_id = OLD.topic_id AND + visit_time > OLD.created_time; + ELSIF (NOT old_visible AND new_visible) THEN + UPDATE topic_visits + SET num_comments = num_comments + 1 + WHERE topic_id = OLD.topic_id AND + visit_time > OLD.created_time; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute(""" + CREATE TRIGGER update_topic_visits_num_comments_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) + EXECUTE PROCEDURE update_all_topic_visit_num_comments(); + """) + + # topics + op.execute(""" + CREATE OR REPLACE FUNCTION update_topics_num_comments() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE topics + SET num_comments = num_comments + 1 + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'DELETE' + AND OLD.is_deleted = FALSE + AND OLD.is_removed = FALSE) THEN + UPDATE topics + SET num_comments = num_comments - 1 + WHERE topic_id = OLD.topic_id; + ELSIF (TG_OP = 'UPDATE') THEN + DECLARE + old_visible BOOLEAN := NOT (OLD.is_deleted OR OLD.is_removed); + new_visible BOOLEAN := NOT (NEW.is_deleted OR NEW.is_removed); + BEGIN + IF (old_visible AND NOT new_visible) THEN + UPDATE topics + SET num_comments = num_comments - 1 + WHERE topic_id = NEW.topic_id; + ELSIF (NOT old_visible AND new_visible) THEN + UPDATE topics + SET num_comments = num_comments + 1 + WHERE topic_id = NEW.topic_id; + END IF; + END; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER update_topics_num_comments_update ON comments") + op.execute(""" + CREATE TRIGGER update_topics_num_comments_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) + EXECUTE PROCEDURE update_topics_num_comments(); + """) + + op.execute(""" + CREATE OR REPLACE FUNCTION update_topics_last_activity_time() RETURNS TRIGGER AS $$ + DECLARE + most_recent_comment RECORD; + BEGIN + IF (TG_OP = 'INSERT') THEN + UPDATE topics + SET last_activity_time = NOW() + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'UPDATE') THEN + SELECT MAX(created_time) AS max_created_time + INTO most_recent_comment + FROM comments + WHERE topic_id = NEW.topic_id + AND is_deleted = FALSE + AND is_removed = FALSE; + + IF most_recent_comment.max_created_time IS NOT NULL THEN + UPDATE topics + SET last_activity_time = most_recent_comment.max_created_time + WHERE topic_id = NEW.topic_id; + ELSE + UPDATE topics + SET last_activity_time = created_time + WHERE topic_id = NEW.topic_id; + END IF; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER update_topics_last_activity_time_update ON comments") + op.execute(""" + CREATE TRIGGER update_topics_last_activity_time_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) + EXECUTE PROCEDURE update_topics_last_activity_time(); + """) + + +def downgrade(): + # comment_notifications + op.execute("DROP TRIGGER delete_comment_notifications_update ON comments") + op.execute(""" + CREATE TRIGGER delete_comment_notifications_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + EXECUTE PROCEDURE delete_comment_notifications(); + """) + + # comments + op.execute(""" + CREATE OR REPLACE FUNCTION set_comment_deleted_time() RETURNS TRIGGER AS $$ + BEGIN + NEW.deleted_time := current_timestamp; + + RETURN NEW; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER delete_comment_set_deleted_time_update ON comments") + op.execute(""" + CREATE TRIGGER delete_comment_set_deleted_time_update + BEFORE UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + EXECUTE PROCEDURE set_comment_deleted_time(); + """) + + op.execute("DROP TRIGGER remove_comment_set_removed_time_update ON comments") + op.execute("DROP FUNCTION set_comment_removed_time()") + + # topic_visits + op.execute("DROP TRIGGER update_topic_visits_num_comments_update ON comments") + op.execute("DROP FUNCTION update_all_topic_visit_num_comments()") + op.execute(""" + CREATE OR REPLACE FUNCTION decrement_all_topic_visit_num_comments() RETURNS TRIGGER AS $$ + BEGIN + UPDATE topic_visits + SET num_comments = num_comments - 1 + WHERE topic_id = OLD.topic_id AND + visit_time > OLD.created_time; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute(""" + CREATE TRIGGER update_topic_visits_num_comments_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + EXECUTE PROCEDURE decrement_all_topic_visit_num_comments(); + """) + + # topics + op.execute(""" + CREATE OR REPLACE FUNCTION update_topics_num_comments() RETURNS TRIGGER AS $$ + BEGIN + IF (TG_OP = 'INSERT' AND NEW.is_deleted = FALSE) THEN + UPDATE topics + SET num_comments = num_comments + 1 + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'DELETE' AND OLD.is_deleted = FALSE) THEN + UPDATE topics + SET num_comments = num_comments - 1 + WHERE topic_id = OLD.topic_id; + ELSIF (TG_OP = 'UPDATE') THEN + IF (OLD.is_deleted = FALSE AND NEW.is_deleted = TRUE) THEN + UPDATE topics + SET num_comments = num_comments - 1 + WHERE topic_id = NEW.topic_id; + ELSIF (OLD.is_deleted = TRUE AND NEW.is_deleted = FALSE) THEN + UPDATE topics + SET num_comments = num_comments + 1 + WHERE topic_id = NEW.topic_id; + END IF; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER update_topics_num_comments_update ON comments") + op.execute(""" + CREATE TRIGGER update_topics_num_comments_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + EXECUTE PROCEDURE update_topics_num_comments(); + """) + + op.execute(""" + CREATE OR REPLACE FUNCTION update_topics_last_activity_time() RETURNS TRIGGER AS $$ + DECLARE + most_recent_comment RECORD; + BEGIN + IF (TG_OP = 'INSERT' AND NEW.is_deleted = FALSE) THEN + UPDATE topics + SET last_activity_time = NOW() + WHERE topic_id = NEW.topic_id; + ELSIF (TG_OP = 'UPDATE') THEN + SELECT MAX(created_time) AS max_created_time + INTO most_recent_comment + FROM comments + WHERE topic_id = NEW.topic_id AND + is_deleted = FALSE; + + IF most_recent_comment.max_created_time IS NOT NULL THEN + UPDATE topics + SET last_activity_time = most_recent_comment.max_created_time + WHERE topic_id = NEW.topic_id; + ELSE + UPDATE topics + SET last_activity_time = created_time + WHERE topic_id = NEW.topic_id; + END IF; + END IF; + + RETURN NULL; + END; + $$ LANGUAGE plpgsql; + """) + op.execute("DROP TRIGGER update_topics_last_activity_time_update ON comments") + op.execute(""" + CREATE TRIGGER update_topics_last_activity_time_update + AFTER UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + EXECUTE PROCEDURE update_topics_last_activity_time(); + """) diff --git a/tildes/sql/init/triggers/comments/comment_notifications.sql b/tildes/sql/init/triggers/comments/comment_notifications.sql index 5561388..3a91e53 100644 --- a/tildes/sql/init/triggers/comments/comment_notifications.sql +++ b/tildes/sql/init/triggers/comments/comment_notifications.sql @@ -1,4 +1,4 @@ --- delete any comment notifications related to a comment when it's deleted +-- delete any notifications related to a comment when it's deleted or removed CREATE OR REPLACE FUNCTION delete_comment_notifications() RETURNS TRIGGER AS $$ BEGIN DELETE FROM comment_notifications @@ -11,5 +11,6 @@ $$ LANGUAGE plpgsql; CREATE TRIGGER delete_comment_notifications_update AFTER UPDATE ON comments FOR EACH ROW - WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + WHEN ((OLD.is_deleted = false AND NEW.is_deleted = true) + OR (OLD.is_removed = false AND NEW.is_removed = true)) EXECUTE PROCEDURE delete_comment_notifications(); diff --git a/tildes/sql/init/triggers/comments/comments.sql b/tildes/sql/init/triggers/comments/comments.sql index 6fa3446..0ff787e 100644 --- a/tildes/sql/init/triggers/comments/comments.sql +++ b/tildes/sql/init/triggers/comments/comments.sql @@ -1,7 +1,11 @@ --- set comment.deleted_time when it's deleted +-- set comment.deleted_time when is_deleted changes CREATE OR REPLACE FUNCTION set_comment_deleted_time() RETURNS TRIGGER AS $$ BEGIN - NEW.deleted_time := current_timestamp; + IF (NEW.is_deleted = TRUE) THEN + NEW.deleted_time := current_timestamp; + ELSE + NEW.deleted_time := NULL; + END IF; RETURN NEW; END; @@ -10,5 +14,25 @@ $$ LANGUAGE plpgsql; CREATE TRIGGER delete_comment_set_deleted_time_update BEFORE UPDATE ON comments FOR EACH ROW - WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) + WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) EXECUTE PROCEDURE set_comment_deleted_time(); + + +-- set comment.removed_time when is_removed changes +CREATE OR REPLACE FUNCTION set_comment_removed_time() RETURNS TRIGGER AS $$ +BEGIN + IF (NEW.is_removed = TRUE) THEN + NEW.removed_time := current_timestamp; + ELSE + NEW.removed_time := NULL; + END IF; + + RETURN NEW; +END; +$$ LANGUAGE plpgsql; + +CREATE TRIGGER remove_comment_set_removed_time_update + BEFORE UPDATE ON comments + FOR EACH ROW + WHEN (OLD.is_removed IS DISTINCT FROM NEW.is_removed) + EXECUTE PROCEDURE set_comment_removed_time(); diff --git a/tildes/sql/init/triggers/comments/topic_visits.sql b/tildes/sql/init/triggers/comments/topic_visits.sql index 0d9f68a..3a2e048 100644 --- a/tildes/sql/init/triggers/comments/topic_visits.sql +++ b/tildes/sql/init/triggers/comments/topic_visits.sql @@ -16,13 +16,23 @@ CREATE TRIGGER update_topic_visits_num_comments_insert EXECUTE PROCEDURE increment_user_topic_visit_num_comments(); --- decrement all users' topic visit comment counts when a comment is deleted -CREATE OR REPLACE FUNCTION decrement_all_topic_visit_num_comments() RETURNS TRIGGER AS $$ +-- adjust all users' topic visit comment counts when a comment is deleted/removed +CREATE OR REPLACE FUNCTION update_all_topic_visit_num_comments() RETURNS TRIGGER AS $$ +DECLARE + old_visible BOOLEAN := NOT (OLD.is_deleted OR OLD.is_removed); + new_visible BOOLEAN := NOT (NEW.is_deleted OR NEW.is_removed); BEGIN - UPDATE topic_visits - SET num_comments = num_comments - 1 - WHERE topic_id = OLD.topic_id AND - visit_time > OLD.created_time; + IF (old_visible AND NOT new_visible) THEN + UPDATE topic_visits + SET num_comments = num_comments - 1 + WHERE topic_id = OLD.topic_id AND + visit_time > OLD.created_time; + ELSIF (NOT old_visible AND new_visible) THEN + UPDATE topic_visits + SET num_comments = num_comments + 1 + WHERE topic_id = OLD.topic_id AND + visit_time > OLD.created_time; + END IF; RETURN NULL; END; @@ -31,5 +41,6 @@ $$ LANGUAGE plpgsql; CREATE TRIGGER update_topic_visits_num_comments_update AFTER UPDATE ON comments FOR EACH ROW - WHEN (OLD.is_deleted = false AND NEW.is_deleted = true) - EXECUTE PROCEDURE decrement_all_topic_visit_num_comments(); + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) + EXECUTE PROCEDURE update_all_topic_visit_num_comments(); diff --git a/tildes/sql/init/triggers/comments/topics.sql b/tildes/sql/init/triggers/comments/topics.sql index 7837101..c274431 100644 --- a/tildes/sql/init/triggers/comments/topics.sql +++ b/tildes/sql/init/triggers/comments/topics.sql @@ -1,23 +1,30 @@ CREATE OR REPLACE FUNCTION update_topics_num_comments() RETURNS TRIGGER AS $$ BEGIN - IF (TG_OP = 'INSERT' AND NEW.is_deleted = FALSE) THEN + IF (TG_OP = 'INSERT') THEN UPDATE topics SET num_comments = num_comments + 1 WHERE topic_id = NEW.topic_id; - ELSIF (TG_OP = 'DELETE' AND OLD.is_deleted = FALSE) THEN + ELSIF (TG_OP = 'DELETE' + AND OLD.is_deleted = FALSE + AND OLD.is_removed = FALSE) THEN UPDATE topics SET num_comments = num_comments - 1 WHERE topic_id = OLD.topic_id; ELSIF (TG_OP = 'UPDATE') THEN - IF (OLD.is_deleted = FALSE AND NEW.is_deleted = TRUE) THEN - UPDATE topics - SET num_comments = num_comments - 1 - WHERE topic_id = NEW.topic_id; - ELSIF (OLD.is_deleted = TRUE AND NEW.is_deleted = FALSE) THEN - UPDATE topics - SET num_comments = num_comments + 1 - WHERE topic_id = NEW.topic_id; - END IF; + DECLARE + old_visible BOOLEAN := NOT (OLD.is_deleted OR OLD.is_removed); + new_visible BOOLEAN := NOT (NEW.is_deleted OR NEW.is_removed); + BEGIN + IF (old_visible AND NOT new_visible) THEN + UPDATE topics + SET num_comments = num_comments - 1 + WHERE topic_id = NEW.topic_id; + ELSIF (NOT old_visible AND new_visible) THEN + UPDATE topics + SET num_comments = num_comments + 1 + WHERE topic_id = NEW.topic_id; + END IF; + END; END IF; RETURN NULL; @@ -32,20 +39,21 @@ CREATE TRIGGER update_topics_num_comments_insert_delete EXECUTE PROCEDURE update_topics_num_comments(); --- update trigger only needs to execute if is_deleted was changed +-- update trigger only needs to execute if is_deleted or is_removed was changed CREATE TRIGGER update_topics_num_comments_update AFTER UPDATE ON comments FOR EACH ROW - WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) EXECUTE PROCEDURE update_topics_num_comments(); --- update a topic's last activity time when a comment is posted or deleted +-- update a topic's last activity time when a comment is posted, deleted, or removed CREATE OR REPLACE FUNCTION update_topics_last_activity_time() RETURNS TRIGGER AS $$ DECLARE most_recent_comment RECORD; BEGIN - IF (TG_OP = 'INSERT' AND NEW.is_deleted = FALSE) THEN + IF (TG_OP = 'INSERT') THEN UPDATE topics SET last_activity_time = NOW() WHERE topic_id = NEW.topic_id; @@ -53,8 +61,9 @@ BEGIN SELECT MAX(created_time) AS max_created_time INTO most_recent_comment FROM comments - WHERE topic_id = NEW.topic_id AND - is_deleted = FALSE; + WHERE topic_id = NEW.topic_id + AND is_deleted = FALSE + AND is_removed = FALSE; IF most_recent_comment.max_created_time IS NOT NULL THEN UPDATE topics @@ -79,5 +88,6 @@ CREATE TRIGGER update_topics_last_activity_time_insert CREATE TRIGGER update_topics_last_activity_time_update AFTER UPDATE ON comments FOR EACH ROW - WHEN (OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + WHEN ((OLD.is_deleted IS DISTINCT FROM NEW.is_deleted) + OR (OLD.is_removed IS DISTINCT FROM NEW.is_removed)) EXECUTE PROCEDURE update_topics_last_activity_time(); diff --git a/tildes/tildes/models/comment/comment.py b/tildes/tildes/models/comment/comment.py index e1a4063..ca51e09 100644 --- a/tildes/tildes/models/comment/comment.py +++ b/tildes/tildes/models/comment/comment.py @@ -39,21 +39,23 @@ class Comment(DatabaseModel): - num_votes will be incremented and decremented by insertions and deletions in comment_votes. Outgoing: - - Inserting, deleting, or updating is_deleted will increment or - decrement num_comments on the relevant topic. + - Inserting or deleting rows, or updating is_deleted/is_removed to + change visibility will increment or decrement num_comments + accordingly on the relevant topic. - Inserting a row will increment num_comments on any topic_visit rows for the comment's author and the relevant topic. - - Inserting or updating is_deleted will update last_activity_time on - the relevant topic. - - Setting is_deleted to true will delete any rows in + - Inserting a new comment or updating is_deleted or is_removed will + update last_activity_time on the relevant topic. + - Setting is_deleted or is_removed to true will delete any rows in comment_notifications related to the comment. - - Setting is_deleted to true will decrement num_comments on all + - Changing is_deleted or is_removed will adjust num_comments on all topic_visit rows for the relevant topic, where the visit_time was after the time the comment was originally posted. - Inserting a row or updating markdown will send a rabbitmq message for "comment.created" or "comment.edited" respectively. Internal: - - deleted_time will be set when is_deleted is set to true + - deleted_time will be set or unset when is_deleted is changed + - removed_time will be set or unset when is_removed is changed """ schema_class = CommentSchema