Merge pull request #1877 from codidact/0valt/1876/thread-rename

Disallow renaming of threads to their current titles

Author: Oaphi

app/assets/javascripts/comments.js

@@ -255,6 +255,57 @@ $(() => {
     });
   });
 
+  /**
+   * @param {Element} target
+   */
+  const checkThreadRename = (target) => {
+    if (target instanceof HTMLInputElement) {
+      const $tgt = $(target);
+      const threadID = $tgt.data('thread');
+      const $form = $tgt.closest(`form[data-thread=${threadID}]`);
+      const $submitter = $form.find('.js-rename-thread');
+
+      const newStripped = QPixel.MD.stripMarkdown($tgt.val(), {
+        removeLeadingQuote: true
+      });
+
+      if (newStripped === $tgt.data('old')) {
+        $submitter.attr('disabled', 'true');
+      }
+    }
+  };
+
+  document.querySelectorAll('.js-thread-title-field').forEach((el) => {
+    checkThreadRename(el);
+  });
+
+  $(document).on('keyup change paste', '.js-thread-title-field', (ev) => {
+    checkThreadRename(ev.target);
+  });
+
+  $(document).on('click', '.js-rename-thread', async (ev) => {
+    ev.preventDefault();
+
+    const $tgt = $(ev.target);
+    const threadID = $tgt.data('thread');
+    const form = $tgt.closest(`form[data-thread=${threadID}]`).get(0);
+
+    if (form instanceof HTMLFormElement) {
+      const { value: newTitle, dataset = {} } = form.elements['title'] ?? {};
+      const { old } = dataset;
+
+      const newStripped = QPixel.MD.stripMarkdown(newTitle, {
+        removeLeadingQuote: true
+      });
+
+      if (newStripped === old) {
+        return;
+      }
+
+      form.submit();
+    }
+  });
+
   $(document).on('click', '.js-lock-thread', async (ev) => {
     ev.preventDefault();
 

app/controllers/application_controller.rb

@@ -351,6 +351,10 @@ def current_user
     helpers.current_user
   end
 
+  def system_user
+    helpers.system_user
+  end
+
   def user_signed_in?
     helpers.user_signed_in?
   end

app/controllers/comments_controller.rb

@@ -26,6 +26,7 @@ class CommentsController < ApplicationController
   before_action :check_unrestrict_access, only: [:thread_unrestrict]
   before_action :check_if_target_post_locked, only: [:create, :post_follow]
   before_action :check_if_parent_post_locked, only: [:update, :destroy]
+  before_action :verify_moderator, only: [:thread_followers]
 
   def create_thread
     title = params[:title]
@@ -203,8 +204,6 @@ def thread_content
   end
 
   def thread_followers
-    return not_found! unless current_user&.at_least_moderator?
-
     @followers = ThreadFollower.where(comment_thread: @comment_thread).joins(:user, user: :community_user)
                                .includes(:user, user: [:community_user, :avatar_attachment])
     respond_to do |format|
@@ -222,19 +221,24 @@ def thread_rename
 
     orig_title = @comment_thread.title
     title = helpers.strip_markdown(params[:title], strip_leading_quote: true)
+
+    if orig_title == title
+      flash[:danger] = I18n.t('comments.errors.no_rename_thread_to_current_title')
+      redirect_to comment_thread_path(@comment_thread.id)
+      return
+    end
+
     status = @comment_thread.update(title: title)
 
     if status
       # Comment is owned by System so regular users can't delete it. Without
       # this record, the title would be attributed to the thread creator,
       # which can be abused.
-      log_msg =
-        Comment.new(post: @post,
-                    content:
-                      "Thread renamed from \\\"#{orig_title}\\\" to \\\"#{title}\\\" by @##{current_user.id}",
-                    user: User.find(-1),
-                    comment_thread: @comment_thread,
-                    has_reference: false)
+      log_msg = Comment.new(post: @post,
+                            content: "Thread renamed from \"#{orig_title}\" to \"#{title}\" by @##{current_user.id}",
+                            user: helpers.system_user,
+                            comment_thread: @comment_thread,
+                            has_reference: false)
       comment_status = log_msg.save
     end
 

app/controllers/users_controller.rb

@@ -111,7 +111,7 @@ def filter_json(filter)
 
   def filters_json
     system_filters = Rails.cache.fetch 'default_system_filters', expires_in: 1.day do
-      User.find(-1).filters.to_h { |filter| [filter.name, filter_json(filter)] }
+      system_user.filters.to_h { |filter| [filter.name, filter_json(filter)] }
     end
 
     if user_signed_in?

app/helpers/application_helper.rb

@@ -336,6 +336,12 @@ def current_user
     @current_user
   end
 
+  # Gets the special System user
+  # @return [User, nil]
+  def system_user
+    User.find(-1)
+  end
+
   ##
   # Is there a user signed in on this request?
   # @return [Boolean]

app/views/comments/_rename_thread_modal.html.erb

@@ -6,7 +6,9 @@
 "%>
 
 <div class="modal is-with-backdrop is-small js--rename-thread-<%= thread.id %>">
-  <%= form_tag rename_comment_thread_path(thread.id), class: 'modal--container' do %>
+  <%= form_tag rename_comment_thread_path(thread.id),
+               class: 'modal--container',
+               data: { thread: thread.id } do %>
     <div class="modal--header">
       <button type="button"
               class="button is-close-button modal--header-button"
@@ -22,6 +24,7 @@
                          data: { post: thread.post.id,
                                  thread: thread.id,
                                  character_count: ".js-character-count-thread-title-#{thread.id}",
+                                 old: thread.title,
                                  markdown: 'strip' } %>
       <%= render 'shared/char_count',
                  type: "thread-title-#{thread.id}",
@@ -30,7 +33,9 @@
                  max: maximum_thread_title_length %>
     </div>
     <div class="modal--footer">
-      <%= submit_tag 'Update thread', class: 'button is-muted is-filled' %>
+      <%= submit_tag 'Update thread',
+                     class: 'button is-muted is-filled js-rename-thread',
+                     data: { thread: thread.id } %>
       <button type="button"
               class="button is-muted"
               data-modal=".js--rename-thread-<%= thread.id %>">Cancel</button>

config/locales/strings/en.comments.yml

@@ -31,6 +31,8 @@ en:
         Something went wrong when trying to undelete the comment.
       rename_thread_generic: >
         Failed to rename the thread.
+      no_rename_thread_to_current_title: >
+        Cannot rename a thread to its current title.
       title_presence: >
         can't be empty after Markdown is removed.
       comment_not_posted: >

test/controllers/comments/rename_test.rb

@@ -9,11 +9,12 @@ class CommentsControllerTest < ActionController::TestCase
     sign_in users(:deleter)
 
     try_rename_thread(comment_threads(:normal))
+    @thread = assigns(:comment_thread)
 
     assert_response(:found)
-    assert_not_nil assigns(:comment_thread)
-    assert_redirected_to comment_thread_path(assigns(:comment_thread))
-    assert_equal 'new thread title', assigns(:comment_thread).title
+    assert_not_nil(@thread)
+    assert_redirected_to comment_thread_path(@thread)
+    assert_equal 'new thread title', @thread.title
   end
 
   test 'non-moderators should not be able to rename restricted threads' do
@@ -26,12 +27,26 @@ class CommentsControllerTest < ActionController::TestCase
       @thread = assigns(:comment_thread)
 
       assert_response(:found)
-      assert_not_nil @thread
+      assert_not_nil(@thread)
       assert_redirected_to comment_thread_path(@thread)
       assert_equal before_title, @thread.title
     end
   end
 
+  test 'should not allow renaming to the current title' do
+    sign_in users(:admin)
+
+    thread = comment_threads(:normal)
+
+    try_rename_thread(thread, title: thread.title)
+    @thread = assigns(:comment_thread)
+
+    assert_response(:found)
+    assert_not_nil(@thread)
+    assert_equal flash[:danger], I18n.t('comments.errors.no_rename_thread_to_current_title')
+    assert_equal thread.title, @thread.title
+  end
+
   test 'should correctly handle invalid thread titles' do
     sign_in users(:admin)
 
@@ -40,7 +55,7 @@ class CommentsControllerTest < ActionController::TestCase
       @thread = assigns(:comment_thread)
 
       assert_response(:found)
-      assert_not_nil @thread
+      assert_not_nil(@thread)
       assert_not @thread.valid?
       assert_not_nil flash[:danger]
     end