Merge branch 'develop' into MoshiKoi/1025/remove-special-case-notifying-author-of-threads

Author: trichoplax

app/controllers/comments_controller.rb

@@ -21,11 +21,7 @@ class CommentsController < ApplicationController
   def create_thread
     title = params[:title]
     unless title.present?
-      title = if params[:body].length > 100
-                "#{params[:body][0..100]}..."
-              else
-                params[:body]
-              end
+      title = helpers.generate_thread_title(params[:body])
     end
 
     body = params[:body]

app/helpers/comments_helper.rb

@@ -1,5 +1,19 @@
 # Helpers related to comments.
 module CommentsHelper
+  # Generates a comment thread title from its body
+  # @param body [String] coment thread body
+  # @return [String] generated title
+  def generate_thread_title(body)
+    body = strip_markdown(body)
+    body = body.gsub(/^>.+?$/, '') # also remove leading blockquotes
+
+    if body.length > 100
+      "#{body[0..100]}..."
+    else
+      body
+    end
+  end
+
   ##
   # Get a link to the specified comment, accounting for deleted comments.
   # @param comment [Comment]
@@ -20,26 +34,47 @@ def comment_user_link(comment)
     user_link(comment.user, { host: comment.community.host })
   end
 
+  # Gets a list of pinged users for a given content
+  # @param content [String] content to get pinged users from
+  # @return [Hash{String => User}] list of pinged users
+  def pinged_users(content)
+    user_ids = content.scan(/@#(\d+)/).map { |g| g[0].to_i }
+    User.where(id: user_ids).to_a.to_h { |u| [u.id, u] }
+  end
+
   ##
-  # Process a comment and convert ping-strings (i.e. @#1234) into links.
-  # @param comment [String] The text of the comment to process.
-  # @param pingable [Array<Integer>, nil] A list of user IDs that should be pingable in this comment. Any user IDs not
-  #   present in the list will be displayed as 'unpingable'.
+  # Converts all ping-strings (i.e. @#1234) into links.
+  # @param content [String] content to convert ping-strings for
+  # @param pingable [Array<Integer>, nil] A list of user IDs. Any user ID not present will be displayed as 'unpingable'.
   # @return [ActiveSupport::SafeBuffer]
-  def render_pings(comment, pingable: nil)
-    comment.gsub(/@#\d+/) do |id|
-      u = User.where(id: id[2..-1].to_i).first
-      if u.nil?
-        id
+  def render_pings(content, pingable: nil)
+    users = pinged_users(content)
+
+    content.gsub(/@#(\d+)/) do |ping|
+      user = users[Regexp.last_match(1).to_i]
+      if user.nil?
+        ping
       else
-        was_pung = pingable.present? && pingable.include?(u.id)
-        classes = "ping #{u.id == current_user&.id ? 'me' : ''} #{was_pung ? '' : 'unpingable'}"
-        user_link u, class: classes, dir: 'ltr',
+        was_pung = pingable.present? && pingable.include?(user.id)
+        classes = "ping #{user.same_as?(current_user) ? 'me' : ''} #{was_pung ? '' : 'unpingable'}"
+        user_link user, class: classes, dir: 'ltr',
                   title: was_pung ? '' : 'This user was not notified because they have not participated in this thread.'
       end
     end.html_safe
   end
 
+  # Converts all ping-strings (i.e. @#1234) in content into usernames for use in text-only contexts
+  # @param content [String] content to convert ping-strings for
+  # @return [String] processed content
+  def render_pings_text(content)
+    users = pinged_users(content)
+
+    content.gsub(/@#(\d+)/) do |ping|
+      user = users[Regexp.last_match(1).to_i]
+      user.nil? ? ping : "@#{rtl_safe_username(user)}"
+    end
+  end
+
   ##
   # Process comment text and convert helper links (like [help] and [flags]) into real links.
   # @param comment_text [String] The text of the comment to process.

app/models/concerns/identity.rb

@@ -3,7 +3,7 @@ module Identity
 
   included do
     # Is this record the same as a given other record?
-    # @param other [ApplicationRecord] record to compare with
+    # @param other [Class, nil] record to compare with
     # @return [Boolean] check result
     def same_as?(other)
       instance_of?(other.class) && id == other.id

app/views/comment_threads/_collapsed.html.erb

@@ -21,6 +21,6 @@
   <% elsif thread.locked? %>
     <i class="fas fa-lock fa-fw" title="Locked thread" aria-label="Locked thread"></i>
   <% end %>
-  <%= link_to thread.title, comment_thread_path(thread), class: 'js--comment-link', data: { thread: thread.id }, role: 'button' %>
+  <%= link_to render_pings_text(thread.title), comment_thread_path(thread), class: 'js--comment-link', data: { thread: thread.id }, role: 'button' %>
   (<%= pluralize(thread.reply_count, 'comment') %>)
 </div>

app/views/comment_threads/_expanded.html.erb

@@ -51,7 +51,7 @@
       <% elsif thread.locked? %>
         <i class="fas fa-lock fa-fw" title="Locked. This thread has been locked and cannot be modified."></i>
       <% end %>
-      <span class="js-thread-title"><%= thread.title %></span>
+      <span class="js-thread-title"><%= render_pings_text(thread.title) %></span>
     </div>
     <% skipped_deleted = 0 %>
     <% comments = thread.comments %>

test/models/concerns/identity_test.rb

@@ -36,4 +36,14 @@ def initialize(id)
 
     assert_not first.same_as?(second)
   end
+
+  test 'same_as? should not fail if the compared model is nil' do
+    first = @klass1.new(42)
+
+    assert_nothing_raised do
+      first.same_as?(nil)
+    end
+
+    assert_not first.same_as?(nil)
+  end
 end