Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 14 additions & 10 deletions app/controllers/comments_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def create_thread
@comment_thread = CommentThread.new(title: title, post: @post)
@comment = Comment.new(post: @post, content: body, user: current_user, comment_thread: @comment_thread)

pings = check_for_pings @comment_thread, body
pings = check_for_pings(@comment_thread, body)

success = ActiveRecord::Base.transaction do
@comment_thread.save!
Expand All @@ -49,7 +49,7 @@ def create_thread
ThreadFollower.create(user: tf.user, comment_thread: @comment_thread)
end

apply_pings pings
apply_pings(pings)
else
flash[:danger] = "Could not create comment thread: #{(@comment_thread.errors.full_messages \
+ @comment.errors.full_messages).join(', ')}"
Expand All @@ -59,15 +59,15 @@ def create_thread

def create
body = params[:content]
pings = check_for_pings @comment_thread, body
pings = check_for_pings(@comment_thread, body)

@comment = Comment.new(post: @post, content: body, user: current_user,
comment_thread: @comment_thread, has_reference: false)

status = @comment.save

if status
apply_pings pings
apply_pings(pings)
@comment_thread.thread_follower.each do |follower|
next if follower.user_id == current_user.id
next if pings.include? follower.user_id
Expand Down Expand Up @@ -98,17 +98,18 @@ def update
@post = @comment.post
@comment_thread = @comment.comment_thread
before = @comment.content
before_pings = check_for_pings @comment_thread, before
before_pings = check_for_pings(@comment_thread, before)
if @comment.update comment_params
unless current_user.id == @comment.user_id
audit('comment_update', @comment, "from <<#{before}>>\nto <<#{@comment.content}>>")
end

after_pings = check_for_pings @comment_thread, @comment.content
after_pings = check_for_pings(@comment_thread, @comment.content)
apply_pings(after_pings - before_pings - @comment_thread.thread_follower.to_a)

render json: { status: 'success',
comment: render_to_string(partial: 'comments/comment', locals: { comment: @comment }) }
comment: render_to_string(partial: 'comments/comment',
locals: { comment: @comment, pingable: after_pings }) }
else
render json: { status: 'failed',
message: "Comment failed to save (#{@comment.errors.full_messages.join(', ')})" },
Expand Down Expand Up @@ -272,8 +273,7 @@ def post_follow

def pingable
thread = params[:id] == '-1' ? CommentThread.new(post_id: params[:post]) : CommentThread.find(params[:id])
ids = helpers.get_pingable(thread)
users = User.where(id: ids)
users = User.where(id: thread.pingable)
render json: users.to_h { |u| [u.username, u.id] }
end

Expand Down Expand Up @@ -372,12 +372,16 @@ def check_if_target_post_locked
check_if_locked(Post.find(params[:post_id]))
end

# @param thread [CommentThread] thread to extract pings for
# @param content [String] content to extract pings from
# @return [Array<Integer>] list of pinged user ids
def check_for_pings(thread, content)
pingable = helpers.get_pingable(thread)
pingable = thread.pingable
matches = content.scan(/@#(\d+)/)
matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i)
end

# @param pings [Array<Integer>] list of pinged user ids
def apply_pings(pings)
pings.each do |p|
user = User.where(id: p).first
Expand Down
30 changes: 1 addition & 29 deletions app/helpers/comments_helper.rb
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ def render_pings(content, pingable: nil)
was_pung = pingable.present? && pingable.include?(user.id)
classes = "ping #{'me' if user.same_as?(current_user)} #{'unpingable' unless was_pung}"
user_link user, class: classes, dir: 'ltr',
title: was_pung ? '' : 'This user was not notified because they have not participated in this thread.'
title: was_pung ? '' : I18n.t('comments.warnings.unrelated_user_not_pinged')
end
end.html_safe
end
Expand Down Expand Up @@ -149,34 +149,6 @@ def rate_limited_error_msg(user, post)
I18n.t('comments.errors.rate_limited', count: comments_count)
end

##
# Get a list of user IDs who should be pingable in a specified comment thread. This combines the post author, answer
# authors, recent history event authors, recent comment authors on the post (in any thread), and all thread followers.
# @param thread [CommentThread]
# @return [Array<Integer>]
def get_pingable(thread)
post = thread.post

# post author +
# answer authors +
# last 500 history event users +
# last 500 comment authors +
# all thread followers
query = <<~END_SQL
SELECT posts.user_id FROM posts WHERE posts.id = #{post.id}
UNION DISTINCT
SELECT DISTINCT posts.user_id FROM posts WHERE posts.parent_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT ph.user_id FROM post_histories ph WHERE ph.post_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT comments.user_id FROM comments WHERE comments.post_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT tf.user_id FROM thread_followers tf WHERE tf.comment_thread_id = #{thread.id || '-1'}
END_SQL

ActiveRecord::Base.connection.execute(query).to_a.flatten
end

##
# Is the specified user comment rate limited for the specified post?
# @param user [User] The user to check.
Expand Down
6 changes: 6 additions & 0 deletions app/models/comment.rb
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,12 @@ def content_length
end
end

def pings
pingable = thread.pingable
matches = content.scan(/@#(\d+)/)
matches.flatten.select { |m| pingable.include?(m.to_i) }.map(&:to_i)
end

private

def create_follower
Expand Down
27 changes: 25 additions & 2 deletions app/models/comment_thread.rb
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ class CommentThread < ApplicationRecord

after_create :create_follower

def self.post_followed?(post, user)
ThreadFollower.where(post: post, user: user).any?
end

def read_only?
locked? || archived? || deleted?
end
Expand All @@ -26,8 +30,27 @@ def can_access?(user)
post.can_access?(user)
end

def self.post_followed?(post, user)
ThreadFollower.where(post: post, user: user).any?
# Gets a list of user IDs who should be pingable in the thread.
# @return [Array<Integer>]
def pingable
# post author +
# answer authors +
# last 500 history event users +
# last 500 comment authors +
# all thread followers
query = <<~END_SQL
SELECT posts.user_id FROM posts WHERE posts.id = #{post.id}
UNION DISTINCT
SELECT DISTINCT posts.user_id FROM posts WHERE posts.parent_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT ph.user_id FROM post_histories ph WHERE ph.post_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT comments.user_id FROM comments WHERE comments.post_id = #{post.id}
UNION DISTINCT
SELECT DISTINCT tf.user_id FROM thread_followers tf WHERE tf.comment_thread_id = #{id || '-1'}
END_SQL

ActiveRecord::Base.connection.execute(query).to_a.flatten
end

private
Expand Down
2 changes: 1 addition & 1 deletion app/views/comment_threads/_expanded.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@

<% max_shown_comments = 5 %>
<% comment_id ||= defined?(comment_id) ? comment_id : nil %>
<% pingable = get_pingable(thread) %>
<% pingable = thread.pingable %>
<% shown_comments_count = [max_shown_comments, thread.reply_count].min %>

<%
Expand Down
2 changes: 1 addition & 1 deletion app/views/flags/_flag.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
<%= render 'posts/type_agnostic', show_type_tag: true, show_category_tag: true, post: flag.post %>
</div>
<% elsif flag.post_type == 'Comment' %>
<%= render 'comments/comment', comment: flag.post, with_post_link: true %>
<%= render 'comments/comment', comment: flag.post, with_post_link: true, pingable: flag.post.comment_thread.pingable %>
<% end %>
<div class="widget--body">
<p>
Expand Down
10 changes: 9 additions & 1 deletion app/views/moderator/recent_comments.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,16 @@
communities; consider using activity logs on user profiles instead.
</p>

<%
thread_pingables = @comments.map(&:comment_thread).to_set.to_h do |thread|
[thread, thread.pingable]
end
%>

<% @comments.each do |comment| %>
<%= render 'comments/comment', comment: comment, with_post_link: true %>
<%= render 'comments/comment', comment: comment,
pingable: thread_pingables[comment.comment_thread],
with_post_link: true %>
<% end %>

<div class="has-padding-top-4">
Expand Down
3 changes: 3 additions & 0 deletions config/locales/strings/en.comments.yml
Original file line number Diff line number Diff line change
Expand Up @@ -34,3 +34,6 @@ en:
Start new comment thread
reply_to_thread: >
Reply to this thread
warnings:
unrelated_user_not_pinged: >
This user was not notified because they have not participated in this thread.