From cca3f98765659db1c888b61fe9990897b614e5de Mon Sep 17 00:00:00 2001 From: evazion Date: Sun, 19 Jan 2020 14:18:54 -0600 Subject: [PATCH] forum: remove ability to merge forum topics. * Rarely used (only used ~15 times in total, not used at all since 2015-2016). * Merging topics didn't properly bump the new topic. * Merging topics didn't log a modaction when the old topic was deleted. * Merging topics broke the old topic. Moving all the posts from one topic to another leaves the old topic with zero posts. This normally can't happen and it causes exceptions when you try to view the empty topic. * It was technically possible to merge a topic with itself. This would break the response_count. * It was technically possible for a mod to merge a topic into an admin-only topic. --- app/controllers/forum_topics_controller.rb | 14 ++--------- app/models/forum_topic.rb | 6 ----- .../forum_topics/_secondary_links.html.erb | 1 - app/views/forum_topics/new_merge.html.erb | 23 ------------------- config/routes.rb | 2 -- test/unit/forum_topic_test.rb | 13 ----------- 6 files changed, 2 insertions(+), 57 deletions(-) delete mode 100644 app/views/forum_topics/new_merge.html.erb diff --git a/app/controllers/forum_topics_controller.rb b/app/controllers/forum_topics_controller.rb index b91ac50fe..404ed6a22 100644 --- a/app/controllers/forum_topics_controller.rb +++ b/app/controllers/forum_topics_controller.rb @@ -2,10 +2,9 @@ class ForumTopicsController < ApplicationController respond_to :html, :xml, :json respond_to :atom, only: [:index, :show] before_action :member_only, :except => [:index, :show] - before_action :moderator_only, :only => [:new_merge, :create_merge] before_action :normalize_search, :only => :index - before_action :load_topic, :only => [:edit, :show, :update, :destroy, :undelete, :new_merge, :create_merge] - before_action :check_min_level, :only => [:show, :edit, :update, :new_merge, :create_merge, :destroy, :undelete] + before_action :load_topic, :only => [:edit, :show, :update, :destroy, :undelete] + before_action :check_min_level, :only => [:show, :edit, :update, :destroy, :undelete] skip_before_action :api_check def new @@ -79,15 +78,6 @@ class ForumTopicsController < ApplicationController redirect_to forum_topics_path, :notice => "All topics marked as read" end - def new_merge - end - - def create_merge - @merged_topic = ForumTopic.find(params[:merged_id]) - @forum_topic.merge(@merged_topic) - redirect_to forum_topic_path(@merged_topic) - end - private def normalize_search diff --git a/app/models/forum_topic.rb b/app/models/forum_topic.rb index c2666fe7e..8d35b984c 100644 --- a/app/models/forum_topic.rb +++ b/app/models/forum_topic.rb @@ -157,12 +157,6 @@ class ForumTopic < ApplicationRecord (response_count / Danbooru.config.posts_per_page.to_f).ceil end - def merge(topic) - ForumPost.where(:id => self.posts.map(&:id)).update_all(:topic_id => topic.id) - topic.update(response_count: topic.response_count + self.posts.length, updater_id: CurrentUser.id) - self.update_columns(:response_count => 0, :is_deleted => true, :updater_id => CurrentUser.id) - end - def delete! update(is_deleted: true) end diff --git a/app/views/forum_topics/_secondary_links.html.erb b/app/views/forum_topics/_secondary_links.html.erb index 715f0b694..135b9edf4 100644 --- a/app/views/forum_topics/_secondary_links.html.erb +++ b/app/views/forum_topics/_secondary_links.html.erb @@ -36,7 +36,6 @@ <% else %> <%= subnav_link_to "Sticky", forum_topic_path(@forum_topic, :forum_topic => {:is_sticky => true}), :method => :put, :data => {:confirm => "Are you sure you want to sticky this forum topic?"} %> <% end %> - <%= subnav_link_to "Merge", new_merge_forum_topic_path(@forum_topic) %> <% end %> <% end %> <% end %> diff --git a/app/views/forum_topics/new_merge.html.erb b/app/views/forum_topics/new_merge.html.erb deleted file mode 100644 index 5bee2080e..000000000 --- a/app/views/forum_topics/new_merge.html.erb +++ /dev/null @@ -1,23 +0,0 @@ -
-
-

Merge Forum Topic

- -

Merge <%= @forum_topic.title %> into:

- - <%= form_tag(create_merge_forum_topic_path(@forum_topic)) do %> -
- -
- <%= submit_tag "Merge" %> - <% end %> -
-
- -<%= render "secondary_links" %> - -<% content_for(:page_title) do %> - Merge Forum Topic - <%= Danbooru.config.app_name %> -<% end %> diff --git a/config/routes.rb b/config/routes.rb index 84b1af4dd..8864fd385 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -140,8 +140,6 @@ Rails.application.routes.draw do resources :forum_topics do member do post :undelete - get :new_merge - post :create_merge end collection do post :mark_all_as_read diff --git a/test/unit/forum_topic_test.rb b/test/unit/forum_topic_test.rb index 46fd4c97d..1dc61b35c 100644 --- a/test/unit/forum_topic_test.rb +++ b/test/unit/forum_topic_test.rb @@ -111,19 +111,6 @@ class ForumTopicTest < ActiveSupport::TestCase end end - context "#merge" do - setup do - @topic2 = create(:forum_topic, title: "yyy", creator: @user) - FactoryBot.create(:forum_post, :topic_id => @topic.id, :body => "xxx") - FactoryBot.create(:forum_post, :topic_id => @topic2.id, :body => "xxx") - end - - should "merge all the posts in one topic into the other" do - @topic.merge(@topic2) - assert_equal(2, @topic2.posts.count) - end - end - context "constructed with nested attributes for its original post" do should "create a matching forum post" do assert_difference(["ForumTopic.count", "ForumPost.count"], 1) do