From 56722df753cbc2eee29dccaf6821f9044c8483ae Mon Sep 17 00:00:00 2001 From: evazion Date: Fri, 21 Jan 2022 20:16:32 -0600 Subject: [PATCH 01/11] 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 From a4279ceff2913a15dc82397b23af52adac821e99 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 00:07:22 -0600 Subject: [PATCH 02/11] css: hide "*" next to required form fields. Fix regression in d6b1302e. --- app/javascript/src/styles/common/simple_form.scss | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/app/javascript/src/styles/common/simple_form.scss b/app/javascript/src/styles/common/simple_form.scss index f17155525..7ceba5e6e 100644 --- a/app/javascript/src/styles/common/simple_form.scss +++ b/app/javascript/src/styles/common/simple_form.scss @@ -83,6 +83,11 @@ form.simple_form { line-height: 1.5em; } + // Hide "*" in label next to required fields. + &.required abbr[title="required"] { + display: none; + } + &.radio_buttons { span.radio label { font-weight: normal; From 90be15e0b5e8270b995e081adf9b16a808d1a6fb Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 16:52:20 -0600 Subject: [PATCH 03/11] Fix #4973: Wiki pages json index returns 404. Fix regression introduced in 0db20e0ca. Setting `format: false` on the wiki pages resource disabled format negotiation on all wiki page routes, not just the show page, which meant /wiki_pages.json no longer worked. The fix to monkey patch the internal Rails method that parses the file extension from the URL, and have it ignore everything but the .html, .json, .js, and .xml extensions. This is really hacky and may break in future Rails releases. --- app/controllers/wiki_pages_controller.rb | 9 --------- config/initializers/core_extensions.rb | 17 +++++++++++++++++ config/routes.rb | 2 +- test/functional/wiki_pages_controller_test.rb | 7 +++++++ 4 files changed, 25 insertions(+), 10 deletions(-) diff --git a/app/controllers/wiki_pages_controller.rb b/app/controllers/wiki_pages_controller.rb index f9f6ece3f..7386018b1 100644 --- a/app/controllers/wiki_pages_controller.rb +++ b/app/controllers/wiki_pages_controller.rb @@ -30,15 +30,6 @@ class WikiPagesController < ApplicationController end def show - if params[:format].present? - request.format = params[:format] - elsif params[:id].ends_with?(".html", ".json", ".xml") - request.format = params[:id].split(".").last - params[:id].delete_suffix!(".#{request.format.symbol}") - else - request.format = "html" - end - @wiki_page, found_by = WikiPage.find_by_id_or_title(params[:id]) if request.format.html? && @wiki_page.blank? && found_by == :title diff --git a/config/initializers/core_extensions.rb b/config/initializers/core_extensions.rb index 3cb5d9f70..ef828ddde 100644 --- a/config/initializers/core_extensions.rb +++ b/config/initializers/core_extensions.rb @@ -63,6 +63,23 @@ class String include Danbooru::Extensions::String end +module MimeNegotationExtension + # Ignore all file extensions except for .html, .js, .json, and .xml when + # parsing the file extension from the URL. Needed for wiki pages (e.g. + # /wiki_pages/rnd.jpg). + private def format_from_path_extension + mime = super + + if mime&.symbol.in?(%i[html js json xml]) + mime + else + nil + end + end +end + +ActionDispatch::Http::MimeNegotiation.prepend(MimeNegotationExtension) + # Make Symbol#to_s return a frozen string. This reduces allocations, but may be # incompatible with some libraries. # diff --git a/config/routes.rb b/config/routes.rb index 950025f90..0f19afcd5 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -280,7 +280,7 @@ Rails.application.routes.draw do resources :webhooks do post :receive, on: :collection end - resources :wiki_pages, id: /.+/, format: false do + resources :wiki_pages, id: /.+?(?=\.json|\.xml|\.html)|.+/ do put :revert, on: :member get :search, on: :collection get :show_or_new, on: :collection diff --git a/test/functional/wiki_pages_controller_test.rb b/test/functional/wiki_pages_controller_test.rb index 92f7f1135..1aa0b386e 100644 --- a/test/functional/wiki_pages_controller_test.rb +++ b/test/functional/wiki_pages_controller_test.rb @@ -31,6 +31,13 @@ class WikiPagesControllerTest < ActionDispatch::IntegrationTest assert_equal(WikiPage.count, response.parsed_body.css("urlset url loc").size) end + should "render for a JSON response" do + get wiki_pages_path(format: :json) + + assert_response :success + assert_equal("application/json", response.media_type) + end + should "redirect the legacy title param to the show page" do get wiki_pages_path(title: "tagme") assert_redirected_to wiki_pages_path(search: { title_normalize: "tagme" }, redirect: true) From d2a24e6b104b77f9c37e0f8728c9d35e6915fa4d Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 18:12:07 -0600 Subject: [PATCH 04/11] Fix #4971: NoMethodError when trying to display some modreports. Delete modreports for hard-deleted comments. There were a total of six invalid modreports for deleted comments. --- .../fixes/096_delete_invalid_moderation_reports.rb | 14 ++++++++++++++ 1 file changed, 14 insertions(+) create mode 100755 script/fixes/096_delete_invalid_moderation_reports.rb diff --git a/script/fixes/096_delete_invalid_moderation_reports.rb b/script/fixes/096_delete_invalid_moderation_reports.rb new file mode 100755 index 000000000..6bebbe196 --- /dev/null +++ b/script/fixes/096_delete_invalid_moderation_reports.rb @@ -0,0 +1,14 @@ +#!/usr/bin/env ruby + +require_relative "base" + +with_confirmation do + CurrentUser.scoped(User.system) do + ModerationReport.find_each do |report| + if report.invalid? && report.errors[:model] == ["must exist"] + puts "destroying modreport ##{report.id}" + report.destroy! + end + end + end +end From 893fa1e94869d8d2208afc0d42e547f13e3f3929 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 18:29:33 -0600 Subject: [PATCH 05/11] css: fix margins. Fix regression in d6b1302e0. Fixes the related tags list having extra space between tags because of default margins around the checkboxes. --- app/javascript/src/styles/base/010_reset.scss | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/javascript/src/styles/base/010_reset.scss b/app/javascript/src/styles/base/010_reset.scss index 30fa1c48f..b27494803 100644 --- a/app/javascript/src/styles/base/010_reset.scss +++ b/app/javascript/src/styles/base/010_reset.scss @@ -2,7 +2,7 @@ box-sizing: border-box; } -body, h1, h2, h3, h4, h5, h6, p, ul, ol, li, blockquote, dl, dd, menu { +body, h1, h2, h3, h4, h5, h6, p, ul, ol, li, blockquote, dl, dd, menu, input { margin: 0; } From f02f3fcc6f2f2828c50826037ebf8b421ef0d911 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 21:33:34 -0600 Subject: [PATCH 06/11] css: fix text sizes in desktop mode on mobile. Disable font boosting on mobile. By default, when desktop mode is enabled on mobile, mobile browsers will automagically increase the size of text. Usually they do so poorly, making things like headers smaller than body text, which breaks the layout. Fixes regression in d6b1302e0. --- app/javascript/src/styles/base/010_reset.scss | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/app/javascript/src/styles/base/010_reset.scss b/app/javascript/src/styles/base/010_reset.scss index b27494803..b928b2506 100644 --- a/app/javascript/src/styles/base/010_reset.scss +++ b/app/javascript/src/styles/base/010_reset.scss @@ -1,3 +1,11 @@ +html { + // Disable font boosting on mobile. By default, when desktop mode is enabled + // on mobile, mobile browsers will automagically increase the size of text. + // Usually they do so poorly, making things like headers smaller than body + // text, which breaks the layout. + text-size-adjust: none; +} + *, ::before, ::after { box-sizing: border-box; } From 0213f8d76afeee4b068929b1ff7720e7d1eec373 Mon Sep 17 00:00:00 2001 From: evazion Date: Sat, 22 Jan 2022 22:04:30 -0600 Subject: [PATCH 07/11] css: remove `image-rendering: smooth`. Contrary to its name, `image-rendering: smooth` resulted in pixelated edges when images were downscaled to fit the browser window. This only affected Firefox because other browsers don't support `smooth`. --- app/javascript/src/styles/base/020_base.scss | 1 - 1 file changed, 1 deletion(-) diff --git a/app/javascript/src/styles/base/020_base.scss b/app/javascript/src/styles/base/020_base.scss index 6593d41d8..6b072fc8c 100644 --- a/app/javascript/src/styles/base/020_base.scss +++ b/app/javascript/src/styles/base/020_base.scss @@ -50,7 +50,6 @@ fieldset { img { border: none; vertical-align: middle; - image-rendering: smooth; } input, select, textarea { From 5c97595c6a9746f0f4050862e65092ecd9dd71dd Mon Sep 17 00:00:00 2001 From: evazion Date: Mon, 24 Jan 2022 01:53:30 -0600 Subject: [PATCH 08/11] posts: fix post view counts not being recorded. Broken by the upgrade to webpacker-6.0.0.rc.6. Webpacker now defaults to loading the Javascript bundle with `