From 5988038071f52598931319767f32a4b1fdd9f818 Mon Sep 17 00:00:00 2001 From: Serge Paquet Date: Tue, 7 Jan 2014 17:58:22 -0500 Subject: [PATCH 1/2] make Comment#ordered_for_story_or_thread_for_user work on query scope --- app/controllers/comments_controller.rb | 8 +++++++- app/controllers/stories_controller.rb | 10 ++++++---- app/models/comment.rb | 13 ++++--------- app/models/story.rb | 9 +++++---- script/mail_new_activity | 2 +- 5 files changed, 23 insertions(+), 19 deletions(-) diff --git a/app/controllers/comments_controller.rb b/app/controllers/comments_controller.rb index f6b389a..a3415ab 100644 --- a/app/controllers/comments_controller.rb +++ b/app/controllers/comments_controller.rb @@ -232,7 +232,13 @@ class CommentsController < ApplicationController end @threads = @showing_user.recent_threads(20).map{|r| - cs = Comment.ordered_for_story_or_thread_for_user(nil, r, @showing_user) + cs = Comment.where( + :thread_id => r + ).includes( + :user, :story + ).arrange_for_user( + @showing_user + ) if @user && (@showing_user.id == @user.id) @votes = Vote.comment_votes_by_user_for_story_hash(@user.id, diff --git a/app/controllers/stories_controller.rb b/app/controllers/stories_controller.rb index 325fde2..76a3db5 100644 --- a/app/controllers/stories_controller.rb +++ b/app/controllers/stories_controller.rb @@ -126,8 +126,7 @@ class StoriesController < ApplicationController @short_url = @story.short_id_url - @comments = Comment.ordered_for_story_or_thread_for_user(@story.id, nil, - @user) + @comments = @story.comments.includes(:user).arrange_for_user(@user) respond_to do |format| format.html { @@ -155,8 +154,11 @@ class StoriesController < ApplicationController return redirect_to @story.comments_url end - @comments = Comment.ordered_for_story_or_thread_for_user(@story.id, - @showing_comment.thread_id, @user ? @user : nil) + @comments = @story.comments + if @showing_comment.thread_id + @comments = @comments.where(:thread_id => @showing_comment.thread_id) + end + @comments = @comments.includes(:user).arrange_for_user(@user) @comments.each do |c,x| if c.id == @showing_comment.id diff --git a/app/models/comment.rb b/app/models/comment.rb index f72553d..983f01f 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -1,6 +1,7 @@ class Comment < ActiveRecord::Base belongs_to :user - belongs_to :story + belongs_to :story, + :inverse_of => :comments has_many :votes, :dependent => :delete_all belongs_to :parent_comment, @@ -293,16 +294,10 @@ class Comment < ActiveRecord::Base comment end - def self.ordered_for_story_or_thread_for_user(story_id, thread_id, user) + def self.arrange_for_user(user) parents = {} - if thread_id - cs = [ "thread_id = ?", thread_id ] - else - cs = [ "story_id = ?", story_id ] - end - - Comment.where(*cs).order("confidence DESC").includes(:user).each do |c| + self.order("confidence DESC").each do |c| (parents[c.parent_comment_id.to_i] ||= []).push c end diff --git a/app/models/story.rb b/app/models/story.rb index b13657a..b0a8eae 100644 --- a/app/models/story.rb +++ b/app/models/story.rb @@ -2,7 +2,8 @@ class Story < ActiveRecord::Base belongs_to :user has_many :taggings, :autosave => true - has_many :comments + has_many :comments, + :inverse_of => :story has_many :tags, :through => :taggings validates_length_of :title, :in => 3..150 @@ -381,9 +382,9 @@ class Story < ActiveRecord::Base end def update_comments_count! + comments = self.comments.arrange_for_user(nil) + # calculate count after removing deleted comments and threads - self.update_column :comments_count, - Comment.ordered_for_story_or_thread_for_user(self.id, nil, nil).select{|c| - !c.is_gone? }.count + self.update_column :comments_count, comments.count{|c| !c.is_gone? } end end diff --git a/script/mail_new_activity b/script/mail_new_activity index 7c75d28..fc0ec36 100755 --- a/script/mail_new_activity +++ b/script/mail_new_activity @@ -153,7 +153,7 @@ last_comment_id, false, false).order(:id).each do |c| thread = [] indent_level = 0 - Comment.ordered_for_story_or_thread_for_user(nil, c.thread_id, + Comment.where(:thread_id => c.thread_id).arrange_for_user( nil).reverse.each do |cc| if indent_level > 0 && cc.indent_level < indent_level thread.unshift cc From e2c266af4b22e21b14de4dde2a164a391e686191 Mon Sep 17 00:00:00 2001 From: Serge Paquet Date: Tue, 7 Jan 2014 18:07:10 -0500 Subject: [PATCH 2/2] simplify Comment#arrange_for_user method --- app/models/comment.rb | 78 ++++++++++++++++++------------------------- 1 file changed, 32 insertions(+), 46 deletions(-) diff --git a/app/models/comment.rb b/app/models/comment.rb index 983f01f..77696ad 100644 --- a/app/models/comment.rb +++ b/app/models/comment.rb @@ -295,66 +295,52 @@ class Comment < ActiveRecord::Base end def self.arrange_for_user(user) - parents = {} - - self.order("confidence DESC").each do |c| - (parents[c.parent_comment_id.to_i] ||= []).push c - end + parents = self.order("confidence DESC").group_by(&:parent_comment_id) # top-down list of comments, regardless of indent level ordered = [] - recursor = lambda{|comment,level| - if comment - comment.indent_level = level - ordered.push comment - end + ancestors = [nil] # nil sentinel so indent_level starts at 1 without add op. + subtree = parents[nil] - # for each comment that is a child of this one, recurse with it - (parents[comment ? comment.id : 0] || []).each do |child| - recursor.call(child, level + 1) - end - } - recursor.call(nil, 0) + while subtree + if (node = subtree.shift) + children = parents[node.id] - # for deleted comments, if they have no children, they can be removed from - # the tree. otherwise they have to stay and a "[deleted]" stub will be - # shown - new_ordered = [] - ordered.each_with_index do |c,x| - if c.is_gone? - if ordered[x + 1] && (ordered[x + 1].indent_level > c.indent_level) - # we have child comments, so we must stay - elsif user && (user.is_moderator? || c.user_id == user.id) + # for deleted comments, if they have no children, they can be removed + # from the tree. otherwise they have to stay and a "[deleted]" stub + # will be shown + if !node.is_gone? + # not deleted or moderated + elsif children + # we have child comments + elsif user && (user.is_moderator? || node.user_id == user.id) # admins and authors should be able to see their deleted comments else # drop this one next end - end - new_ordered.push c + node.indent_level = ancestors.length + ordered << node + + # no children to recurse + next unless children + + # for moderated threads, remove the entire sub-tree at the moderation + # point + next if node.is_moderated? + + # drill down a level + ancestors << subtree + subtree = children + else + # climb back out + subtree = ancestors.pop + end end - # for moderated threads, remove the entire sub-tree at the moderation point - do_reject = false - deleted_indent_level = 0 - new_ordered.reject!{|c| - if do_reject && (c.indent_level > deleted_indent_level) - true - else - if c.is_moderated? - do_reject = true - deleted_indent_level = c.indent_level - else - do_reject = false - end - - false - end - } - - new_ordered + ordered end def is_editable_by_user?(user)