Merge pull request #86 from srgpqt/master

refactor Comment#ordered_for_story_or_thread_for_user method

app/controllers/comments_controller.rb

@@ -237,7 +237,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,

app/controllers/stories_controller.rb

@@ -124,8 +124,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 {
@@ -153,8 +152,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

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,73 +294,53 @@ class Comment < ActiveRecord::Base
     comment
   end
 
-  def self.ordered_for_story_or_thread_for_user(story_id, thread_id, 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|
-      (parents[c.parent_comment_id.to_i] ||= []).push c
-    end
+  def self.arrange_for_user(user)
+    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)

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
@@ -392,9 +393,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

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