From 56722df753cbc2eee29dccaf6821f9044c8483ae Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 21 Jan 2022 20:16:32 -0600 Subject: [PATCH] forum: delete posts when topic is deleted. Fix it so that when a forum topic is deleted, all posts in the topic are deleted too. Also make it so that when a forum topic is undeleted, all posts in it are undeleted too. Before when a topic was deleted, only the topic itself was marked as deleted, not the posts inside the topic. This meant that when a spam topic was deleted, the OP wouldn't be marked as deleted, so any modreports against it wouldn't be marked as handled. Also change it so that it's not possible to undelete a post in a deleted topic, or to delete the OP of a topic without deleting the topic itself. Finally, add a fix script to delete all active posts in deleted topics, and to undelete all deleted OPs in active topics. --- app/controllers/forum_posts_controller.rb | 4 ++ app/models/forum_post.rb | 21 ++++--- app/models/forum_topic.rb | 14 ++++- app/policies/forum_post_policy.rb | 4 +- app/views/forum_posts/destroy.js.erb | 2 +- app/views/forum_posts/undelete.js.erb | 2 +- ...95_delete_forum_posts_in_deleted_topics.rb | 17 ++++++ .../functional/forum_posts_controller_test.rb | 59 +++++++++++++------ .../forum_topics_controller_test.rb | 20 ++++--- 9 files changed, 104 insertions(+), 39 deletions(-) create mode 100755 script/fixes/095_delete_forum_posts_in_deleted_topics.rb diff --git a/app/controllers/forum_posts_controller.rb b/app/controllers/forum_posts_controller.rb index e6ef8ea68..0ab945b58 100644 --- a/app/controllers/forum_posts_controller.rb +++ b/app/controllers/forum_posts_controller.rb @@ -55,12 +55,16 @@ class ForumPostsController < ApplicationController def destroy @forum_post = authorize ForumPost.find(params[:id]) @forum_post.delete! + + flash[:notice] = @forum_post.errors.none? ? "Post deleted" : @forum_post.errors.full_messages.join("; ") respond_with(@forum_post) end def undelete @forum_post = authorize ForumPost.find(params[:id]) @forum_post.undelete! + + flash[:notice] = @forum_post.errors.none? ? "Post undeleted" : @forum_post.errors.full_messages.join("; ") respond_with(@forum_post) end end diff --git a/app/models/forum_post.rb b/app/models/forum_post.rb index 5bd66a95b..17ea1d6f9 100644 --- a/app/models/forum_post.rb +++ b/app/models/forum_post.rb @@ -15,13 +15,14 @@ class ForumPost < ApplicationRecord has_one :bulk_update_request validates :body, presence: true, length: { maximum: 200_000 }, if: :body_changed? + validate :validate_deletion_of_original_post + validate :validate_undeletion_of_post before_create :autoreport_spam before_save :handle_reports_on_deletion after_create :update_topic_updated_at_on_create after_update :update_topic_updated_at_on_update_for_original_posts after_destroy :update_topic_updated_at_on_destroy - after_save :delete_topic_if_original_post after_update(:if => ->(rec) {rec.updater_id != rec.creator_id}) do |rec| ModAction.log("#{CurrentUser.user.name} updated forum ##{rec.id}", :forum_post_update) end @@ -83,6 +84,18 @@ class ForumPost < ApplicationRecord votes.exists?(creator_id: user.id, score: score) end + def validate_deletion_of_original_post + if is_original_post? && is_deleted? && !topic.is_deleted? + errors.add(:base, "Can't delete original post without deleting the topic first") + end + end + + def validate_undeletion_of_post + if topic.is_deleted? && !is_deleted? + errors.add(:base, "Can't undelete post without undeleting the topic first") + end + end + def autoreport_spam if SpamDetector.new(self, user_ip: CurrentUser.ip_addr).spam? moderation_reports << ModerationReport.new(creator: User.system, reason: "Spam.") @@ -153,12 +166,6 @@ class ForumPost < ApplicationRecord end end - def delete_topic_if_original_post - if is_deleted? && is_original_post? - topic.update_attribute(:is_deleted, true) - end - end - def handle_reports_on_deletion return unless moderation_reports.pending.present? && is_deleted_change == [false, true] diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index 76056a957..bdf6cea95 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -24,12 +24,13 @@ class ForumTopic < ApplicationRecord has_many :tag_implications validates :title, presence: true, length: { maximum: 200 }, if: :title_changed? - validates_associated :original_post validates :category_id, inclusion: { in: CATEGORIES.keys } validates :min_level, inclusion: { in: MIN_LEVELS.values } accepts_nested_attributes_for :original_post - after_update :update_orignal_post + + after_update :update_posts_on_deletion_or_undeletion + after_update :update_original_post after_save(:if => ->(rec) {rec.is_locked? && rec.saved_change_to_is_locked?}) do |rec| ModAction.log("locked forum topic ##{id} (title: #{title})", :forum_topic_lock) end @@ -179,7 +180,14 @@ class ForumTopic < ApplicationRecord (response_count / Danbooru.config.posts_per_page.to_f).ceil end - def update_orignal_post + # Delete all posts when the topic is deleted. Undelete all posts when the topic is undeleted. + def update_posts_on_deletion_or_undeletion + if saved_change_to_is_deleted? + forum_posts.update!(is_deleted: is_deleted) # XXX depends on current user + end + end + + def update_original_post original_post&.update_columns(:updater_id => updater.id, :updated_at => Time.now) end diff --git a/app/policies/forum_post_policy.rb b/app/policies/forum_post_policy.rb index bbcc370c1..641b3e6e8 100644 --- a/app/policies/forum_post_policy.rb +++ b/app/policies/forum_post_policy.rb @@ -18,11 +18,11 @@ class ForumPostPolicy < ApplicationPolicy end def destroy? - unbanned? && show? && user.is_moderator? + unbanned? && show? && !record.is_deleted? && user.is_moderator? end def undelete? - unbanned? && show? && user.is_moderator? + unbanned? && show? && record.is_deleted? && !record.topic.is_deleted? && user.is_moderator? end def reply? diff --git a/app/views/forum_posts/destroy.js.erb b/app/views/forum_posts/destroy.js.erb index 7ccf4d7cd..345366b9b 100644 --- a/app/views/forum_posts/destroy.js.erb +++ b/app/views/forum_posts/destroy.js.erb @@ -1 +1 @@ -$("#forum_post_<%= @forum_post.id %>").replaceWith("<%= j render(ForumPostComponent.new(forum_post: @forum_post, current_user: CurrentUser.user)) %>"); +location.reload(); diff --git a/app/views/forum_posts/undelete.js.erb b/app/views/forum_posts/undelete.js.erb index 7ccf4d7cd..345366b9b 100644 --- a/app/views/forum_posts/undelete.js.erb +++ b/app/views/forum_posts/undelete.js.erb @@ -1 +1 @@ -$("#forum_post_<%= @forum_post.id %>").replaceWith("<%= j render(ForumPostComponent.new(forum_post: @forum_post, current_user: CurrentUser.user)) %>"); +location.reload(); diff --git a/script/fixes/095_delete_forum_posts_in_deleted_topics.rb b/script/fixes/095_delete_forum_posts_in_deleted_topics.rb new file mode 100755 index 000000000..01629a732 --- /dev/null +++ b/script/fixes/095_delete_forum_posts_in_deleted_topics.rb @@ -0,0 +1,17 @@ +#!/usr/bin/env ruby + +require_relative "base" + +with_confirmation do + CurrentUser.scoped(User.system) do + # delete all posts in deleted topics + ForumPost.undeleted.where(topic: ForumTopic.deleted).update_all(is_deleted: true, updated_at: Time.zone.now, updater_id: User.system.id) + + # undelete all deleted OPs in active topics + ForumPost.deleted.where(topic: ForumTopic.undeleted).find_each do |forum_post| + if forum_post.is_original_post? + forum_post.update_columns(is_deleted: false, updated_at: Time.zone.now, updater_id: User.system.id) + end + end + end +end diff --git a/test/functional/forum_posts_controller_test.rb b/test/functional/forum_posts_controller_test.rb index aa2e284f9..dfacd294e 100644 --- a/test/functional/forum_posts_controller_test.rb +++ b/test/functional/forum_posts_controller_test.rb @@ -219,24 +219,36 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest context "destroy action" do should "allow mods to delete posts" do - delete_auth forum_post_path(@forum_post), @mod - assert_redirected_to(forum_post_path(@forum_post)) - assert_equal(true, @forum_post.reload.is_deleted?) + @forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user) } + delete_auth forum_post_path(@forum_reply), @mod + + assert_redirected_to(@forum_reply) + assert_equal(true, @forum_reply.reload.is_deleted?) end should "not allow users to delete their own posts" do - delete_auth forum_post_path(@forum_post), @user + @forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user) } + delete_auth forum_post_path(@forum_reply), @user + assert_response 403 - assert_equal(false, @forum_post.reload.is_deleted?) + assert_equal(false, @forum_reply.reload.is_deleted?) + end + + should "not allow deleting the OP of an active topic" do + delete_auth forum_post_path(@forum_topic.original_post), @mod + + assert_redirected_to(@forum_topic.original_post) + assert_equal(false, @forum_topic.original_post.is_deleted?) end should "mark all pending moderation reports against the post as handled" do - report1 = create(:moderation_report, model: @forum_post, status: :pending) - report2 = create(:moderation_report, model: @forum_post, status: :rejected) - delete_auth forum_post_path(@forum_post), @mod + forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user) } + report1 = create(:moderation_report, model: forum_reply, status: :pending) + report2 = create(:moderation_report, model: forum_reply, status: :rejected) + delete_auth forum_post_path(forum_reply), @mod - assert_redirected_to(forum_post_path(@forum_post)) - assert_equal(true, @forum_post.reload.is_deleted?) + assert_redirected_to(forum_post_path(forum_reply)) + assert_equal(true, forum_reply.reload.is_deleted?) assert_equal(true, report1.reload.handled?) assert_equal(true, report2.reload.rejected?) assert_equal(1, ModAction.moderation_report_handled.where(creator: @mod).count) @@ -245,17 +257,30 @@ class ForumPostsControllerTest < ActionDispatch::IntegrationTest context "undelete action" do should "allow mods to undelete posts" do - as(@mod) { @forum_post.update!(is_deleted: true) } - post_auth undelete_forum_post_path(@forum_post), @mod - assert_redirected_to(forum_post_path(@forum_post)) - assert_equal(false, @forum_post.reload.is_deleted?) + @forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user, is_deleted: true) } + post_auth undelete_forum_post_path(@forum_reply), @mod + + assert_redirected_to(@forum_reply) + assert_equal(false, @forum_reply.reload.is_deleted?) end should "not allow users to undelete their own posts" do - as(@mod) { @forum_post.update!(is_deleted: true) } - post_auth undelete_forum_post_path(@forum_post), @user + @forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user, is_deleted: true) } + post_auth undelete_forum_post_path(@forum_reply), @user + assert_response 403 - assert_equal(true, @forum_post.reload.is_deleted?) + assert_equal(true, @forum_reply.reload.is_deleted?) + end + + should "not allow undeleting posts in deleted topics" do + @forum_reply = as(@user) { create(:forum_post, topic: @forum_topic, creator: @user) } + as(@user) { @forum_topic.update!(is_deleted: true) } + assert_equal(true, @forum_reply.reload.is_deleted?) + + post_auth undelete_forum_post_path(@forum_reply), @mod + + assert_response 403 + assert_equal(true, @forum_reply.reload.is_deleted?) end end end diff --git a/test/functional/forum_topics_controller_test.rb b/test/functional/forum_topics_controller_test.rb index d59707331..9a7fa3c6e 100644 --- a/test/functional/forum_topics_controller_test.rb +++ b/test/functional/forum_topics_controller_test.rb @@ -306,11 +306,13 @@ class ForumTopicsControllerTest < ActionDispatch::IntegrationTest end end - should "destroy the topic and any associated posts" do + should "mark the topic and all posts as deleted" do delete_auth forum_topic_path(@forum_topic), @mod - assert_redirected_to(forum_topic_path(@forum_topic)) - @forum_topic.reload - assert(@forum_topic.is_deleted?) + + assert_redirected_to(@forum_topic) + assert_equal(true, @forum_topic.reload.is_deleted?) + assert_equal(true, @forum_topic.original_post.is_deleted?) + assert_equal(true, @forum_topic.forum_posts.all?(&:is_deleted?)) end end @@ -321,11 +323,13 @@ class ForumTopicsControllerTest < ActionDispatch::IntegrationTest end end - should "restore the topic" do + should "mark the topic and all posts as undeleted" do post_auth undelete_forum_topic_path(@forum_topic), @mod - assert_redirected_to(forum_topic_path(@forum_topic)) - @forum_topic.reload - assert(!@forum_topic.is_deleted?) + + assert_redirected_to(@forum_topic) + assert_equal(false, @forum_topic.reload.is_deleted?) + assert_equal(false, @forum_topic.original_post.is_deleted?) + assert_equal(false, @forum_topic.forum_posts.all?(&:is_deleted?)) end end end