From 15b3cce6ee6b3052c960eea2561be1ed4fa6618a Mon Sep 17 00:00:00 2001
From: Pistos <github.pistos@purepistos.net>
Date: Tue, 11 Oct 2011 16:06:24 -0400
Subject: [PATCH] Refactored ordering of post comments; moved it down to model
 level for greater reach (DRYer code).

Leaving the #order call in #last_three_comments caused the generated SQL to
have two conflicting ORDER BY components, which caused the query to return
invalid results.  I removed the problem by removing #last_three_comments which
I consider a premature optimization.
---
 app/models/post.rb                     |  7 +-----
 app/views/comments/_comments.html.haml |  4 ++--
 spec/models/post_spec.rb               | 32 ++++++++++++++++++++------
 3 files changed, 28 insertions(+), 15 deletions(-)

diff --git a/app/models/post.rb b/app/models/post.rb
index 878d0690d..b579c370b 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -16,7 +16,7 @@ class Post < ActiveRecord::Base
   xml_attr :public
   xml_attr :created_at
 
-  has_many :comments, :dependent => :destroy
+  has_many :comments, :order => 'created_at', :dependent => :destroy
 
   has_many :aspect_visibilities
   has_many :aspects, :through => :aspect_visibilities
@@ -109,11 +109,6 @@ class Post < ActiveRecord::Base
     I18n.t('notifier.a_post_you_shared')
   end
 
-  # @return [Array<Comment>]
-  def last_three_comments
-    self.comments.order('created_at DESC').limit(3).includes(:author => :profile).reverse
-  end
-
   # @return [Integer]
   def update_comments_counter
     self.class.where(:id => self.id).
diff --git a/app/views/comments/_comments.html.haml b/app/views/comments/_comments.html.haml
index 2acdfb71e..35433f7b8 100644
--- a/app/views/comments/_comments.html.haml
+++ b/app/views/comments/_comments.html.haml
@@ -9,9 +9,9 @@
 
   %ul.comments{:class => ('loaded' if post.comments.size <= 3)}
     -if post.comments.size > 3 && !comments_expanded
-      = render :partial => 'comments/comment', :collection => post.last_three_comments, :locals => {:post => post}
+      = render :partial => 'comments/comment', :collection => post.comments.last(3), :locals => {:post => post}
     -else
-      = render :partial => 'comments/comment', :collection => post.comments.order('created_at'), :locals => {:post => post}
+      = render :partial => 'comments/comment', :collection => post.comments, :locals => {:post => post}
 
   - unless commenting_disabled?(post)
     .new_comment_form_wrapper{:class => comment_form_wrapper_class(post)}
diff --git a/spec/models/post_spec.rb b/spec/models/post_spec.rb
index 354427b8c..06aba38a8 100644
--- a/spec/models/post_spec.rb
+++ b/spec/models/post_spec.rb
@@ -106,15 +106,33 @@ describe Post do
     end
   end
 
-  describe '#last_three_comments' do
-    it 'returns the last three comments of a post' do
+  describe '#comments' do
+    it 'returns the comments of a post in created_at order' do
       post = bob.post :status_message, :text => "hello", :to => 'all'
       created_at = Time.now - 100
-      comments = [alice, eve, bob, alice].map do |u|
-        created_at = created_at + 10
-        u.comment("hey", :post => post, :created_at => created_at)
-      end
-      post.last_three_comments.map{|c| c.id}.should == comments[1,3].map{|c| c.id}
+
+      # Posts are created out of time order.
+      # i.e. id order is not created_at order
+      alice.comment 'comment a', :post => post, :created_at => created_at + 10
+      eve.comment   'comment d', :post => post, :created_at => created_at + 50
+      bob.comment   'comment b', :post => post, :created_at => created_at + 30
+      alice.comment 'comment e', :post => post, :created_at => created_at + 90
+      eve.comment   'comment c', :post => post, :created_at => created_at + 40
+
+      post.comments.map(&:text).should == [
+        'comment a',
+        'comment b',
+        'comment c',
+        'comment d',
+        'comment e',
+      ]
+      post.comments.map(&:author).should == [
+        alice.person,
+        bob.person,
+        eve.person,
+        eve.person,
+        alice.person,
+      ]
     end
   end