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