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.
This commit is contained in:
parent
4774b670de
commit
15b3cce6ee
3 changed files with 28 additions and 15 deletions
|
|
@ -16,7 +16,7 @@ class Post < ActiveRecord::Base
|
||||||
xml_attr :public
|
xml_attr :public
|
||||||
xml_attr :created_at
|
xml_attr :created_at
|
||||||
|
|
||||||
has_many :comments, :dependent => :destroy
|
has_many :comments, :order => 'created_at', :dependent => :destroy
|
||||||
|
|
||||||
has_many :aspect_visibilities
|
has_many :aspect_visibilities
|
||||||
has_many :aspects, :through => :aspect_visibilities
|
has_many :aspects, :through => :aspect_visibilities
|
||||||
|
|
@ -109,11 +109,6 @@ class Post < ActiveRecord::Base
|
||||||
I18n.t('notifier.a_post_you_shared')
|
I18n.t('notifier.a_post_you_shared')
|
||||||
end
|
end
|
||||||
|
|
||||||
# @return [Array<Comment>]
|
|
||||||
def last_three_comments
|
|
||||||
self.comments.order('created_at DESC').limit(3).includes(:author => :profile).reverse
|
|
||||||
end
|
|
||||||
|
|
||||||
# @return [Integer]
|
# @return [Integer]
|
||||||
def update_comments_counter
|
def update_comments_counter
|
||||||
self.class.where(:id => self.id).
|
self.class.where(:id => self.id).
|
||||||
|
|
|
||||||
|
|
@ -9,9 +9,9 @@
|
||||||
|
|
||||||
%ul.comments{:class => ('loaded' if post.comments.size <= 3)}
|
%ul.comments{:class => ('loaded' if post.comments.size <= 3)}
|
||||||
-if post.comments.size > 3 && !comments_expanded
|
-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
|
-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)
|
- unless commenting_disabled?(post)
|
||||||
.new_comment_form_wrapper{:class => comment_form_wrapper_class(post)}
|
.new_comment_form_wrapper{:class => comment_form_wrapper_class(post)}
|
||||||
|
|
|
||||||
|
|
@ -106,15 +106,33 @@ describe Post do
|
||||||
end
|
end
|
||||||
end
|
end
|
||||||
|
|
||||||
describe '#last_three_comments' do
|
describe '#comments' do
|
||||||
it 'returns the last three comments of a post' do
|
it 'returns the comments of a post in created_at order' do
|
||||||
post = bob.post :status_message, :text => "hello", :to => 'all'
|
post = bob.post :status_message, :text => "hello", :to => 'all'
|
||||||
created_at = Time.now - 100
|
created_at = Time.now - 100
|
||||||
comments = [alice, eve, bob, alice].map do |u|
|
|
||||||
created_at = created_at + 10
|
# Posts are created out of time order.
|
||||||
u.comment("hey", :post => post, :created_at => created_at)
|
# i.e. id order is not created_at order
|
||||||
end
|
alice.comment 'comment a', :post => post, :created_at => created_at + 10
|
||||||
post.last_three_comments.map{|c| c.id}.should == comments[1,3].map{|c| c.id}
|
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
|
||||||
end
|
end
|
||||||
|
|
||||||
|
|
|
||||||
Loading…
Reference in a new issue