Commit 39e3c25c9690586f2e1b0cf4d7ad527d299d4355

Authored by Rodrigo Souto
Committed by Antonio Terceiro
1 parent 268f901c

Blog paginate only with the available post for the user

  * Posts unavailable for the user weren't displayed but were
    considered on pagination.
  * Used a direct sql to increase the performance.
  * Removing last and first_day_of_month methods and using the rails
    builtin methods in_the_beginning_of_month and in_the_end_of_month.

P.S.: done against the master branch.

(ActionItem1557)
app/controllers/public/content_viewer_controller.rb
@@ -78,7 +78,14 @@ class ContentViewerController < ApplicationController @@ -78,7 +78,14 @@ class ContentViewerController < ApplicationController
78 end 78 end
79 79
80 if @page.blog? 80 if @page.blog?
81 - @page.filter = {:year => params[:year], :month => params[:month]} 81 + posts = if params[:year] and params[:month]
  82 + filter_date = DateTime.parse("#{params[:year]}-#{params[:month]}-01")
  83 + @page.posts.by_range(filter_date..filter_date.at_end_of_month)
  84 + else
  85 + @page.posts
  86 + end
  87 +
  88 + @posts = posts.paginate({ :page => params[:npage], :per_page => @page.posts_per_page }.merge(Article.display_filter(user, profile)))
82 end 89 end
83 90
84 if @page.folder? && @page.view_as == 'image_gallery' 91 if @page.folder? && @page.view_as == 'image_gallery'
app/controllers/public/events_controller.rb
@@ -13,7 +13,7 @@ class EventsController < PublicController @@ -13,7 +13,7 @@ class EventsController < PublicController
13 @events_of_the_day = profile.events.by_day(@selected_day) 13 @events_of_the_day = profile.events.by_day(@selected_day)
14 end 14 end
15 15
16 - events = profile.events.by_range(Event.first_day_of_month(date - 1.month)..Event.last_day_of_month(date + 1.month)) 16 + events = profile.events.by_range((date - 1.month).at_beginning_of_month..(date + 1.month).at_end_of_month)
17 17
18 @calendar = populate_calendar(date, events) 18 @calendar = populate_calendar(date, events)
19 @previous_calendar = populate_calendar(date - 1.month, events) 19 @previous_calendar = populate_calendar(date - 1.month, events)
app/controllers/public/search_controller.rb
@@ -102,7 +102,7 @@ class SearchController < PublicController @@ -102,7 +102,7 @@ class SearchController < PublicController
102 102
103 if month || year 103 if month || year
104 date = Date.new(year.to_i, month.to_i, 1) 104 date = Date.new(year.to_i, month.to_i, 1)
105 - result[:date_range] = (date - 1.month)..Event.last_day_of_month(date + 1.month) 105 + result[:date_range] = (date - 1.month)..(date + 1.month).at_end_of_month
106 end 106 end
107 107
108 result 108 result
app/helpers/blog_helper.rb
@@ -14,7 +14,7 @@ module BlogHelper @@ -14,7 +14,7 @@ module BlogHelper
14 _('Configure blog') 14 _('Configure blog')
15 end 15 end
16 16
17 - def list_posts(user, articles, format = 'full') 17 + def list_posts(articles, format = 'full')
18 pagination = will_paginate(articles, { 18 pagination = will_paginate(articles, {
19 :param_name => 'npage', 19 :param_name => 'npage',
20 :prev_label => _('« Newer posts'), 20 :prev_label => _('« Newer posts'),
@@ -25,18 +25,16 @@ module BlogHelper @@ -25,18 +25,16 @@ module BlogHelper
25 articles.each_with_index{ |art,i| 25 articles.each_with_index{ |art,i|
26 css_add = [ 'position-'+(i+1).to_s() ] 26 css_add = [ 'position-'+(i+1).to_s() ]
27 position = (i%2 == 0) ? 'odd-post' : 'even-post' 27 position = (i%2 == 0) ? 'odd-post' : 'even-post'
28 - if art.published? || (user==art.profile)  
29 - css_add << 'first' if i == 0  
30 - css_add << 'last' if i == (artic_len-1)  
31 - css_add << 'not-published' if !art.published?  
32 - css_add << position + '-inner'  
33 - content << content_tag('div',  
34 - content_tag('div',  
35 - display_post(art, format) + '<br style="clear:both"/>',  
36 - :class => 'blog-post ' + css_add.join(' '),  
37 - :id => "post-#{art.id}"), :class => position  
38 - )  
39 - end 28 + css_add << 'first' if i == 0
  29 + css_add << 'last' if i == (artic_len-1)
  30 + css_add << 'not-published' if !art.published?
  31 + css_add << position + '-inner'
  32 + content << content_tag('div',
  33 + content_tag('div',
  34 + display_post(art, format) + '<br style="clear:both"/>',
  35 + :class => 'blog-post ' + css_add.join(' '),
  36 + :id => "post-#{art.id}"), :class => position
  37 + )
40 } 38 }
41 content.join("\n<hr class='sep-posts'/>\n") + (pagination or '') 39 content.join("\n<hr class='sep-posts'/>\n") + (pagination or '')
42 end 40 end
app/models/article.rb
@@ -38,6 +38,12 @@ class Article &lt; ActiveRecord::Base @@ -38,6 +38,12 @@ class Article &lt; ActiveRecord::Base
38 {:include => 'categories', :conditions => { 'categories.id' => category.id }} 38 {:include => 'categories', :conditions => { 'categories.id' => category.id }}
39 } 39 }
40 40
  41 + named_scope :by_range, lambda { |range| {
  42 + :conditions => [
  43 + 'published_at BETWEEN :start_date AND :end_date', { :start_date => range.first, :end_date => range.last }
  44 + ]
  45 + }}
  46 +
41 URL_FORMAT = /\A(http|https):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,5}(([0-9]{1,5})?\/.*)?\Z/ix 47 URL_FORMAT = /\A(http|https):\/\/[a-z0-9]+([\-\.]{1}[a-z0-9]+)*\.[a-z]{2,5}(([0-9]{1,5})?\/.*)?\Z/ix
42 48
43 validates_format_of :external_link, :with => URL_FORMAT, :if => lambda { |article| !article.external_link.blank? } 49 validates_format_of :external_link, :with => URL_FORMAT, :if => lambda { |article| !article.external_link.blank? }
@@ -246,19 +252,28 @@ class Article &lt; ActiveRecord::Base @@ -246,19 +252,28 @@ class Article &lt; ActiveRecord::Base
246 named_scope :folders, :conditions => { :type => ['Folder', 'Blog'] } 252 named_scope :folders, :conditions => { :type => ['Folder', 'Blog'] }
247 named_scope :images, :conditions => { :is_image => true } 253 named_scope :images, :conditions => { :is_image => true }
248 254
  255 + def self.display_filter(user, profile)
  256 + return {:conditions => ['published = ?', true]} if !user
  257 + {:conditions => [" articles.published = ? OR
  258 + articles.last_changed_by_id = ? OR
  259 + articles.profile_id = ? OR
  260 + ?",
  261 + true, user.id, user.id, user.has_permission?(:view_private_content, profile)] }
  262 + end
  263 +
249 def display_unpublished_article_to?(user) 264 def display_unpublished_article_to?(user)
250 - self.author == user || allow_view_private_content?(user) || user == self.profile ||  
251 - user.is_admin?(self.profile.environment) || user.is_admin?(self.profile) 265 + user == author || allow_view_private_content?(user) || user == profile ||
  266 + user.is_admin?(profile.environment) || user.is_admin?(profile)
252 end 267 end
253 268
254 - def display_to?(user)  
255 - if self.published?  
256 - self.profile.display_info_to?(user) 269 + def display_to?(user = nil)
  270 + if published?
  271 + profile.display_info_to?(user)
257 else 272 else
258 - if user.nil? 273 + if !user
259 false 274 false
260 else 275 else
261 - self.display_unpublished_article_to?(user) 276 + display_unpublished_article_to?(user)
262 end 277 end
263 end 278 end
264 end 279 end
app/models/blog.rb
1 class Blog < Folder 1 class Blog < Folder
2 2
3 - has_many :posts, :class_name => 'Article', :foreign_key => 'parent_id', :source => :children, :conditions => [ 'type != ?', 'RssFeed' ], :order => 'published_at DESC, id DESC' 3 + has_many :posts, :class_name => 'Article', :foreign_key => 'parent_id', :source => :children, :conditions => [ 'articles.type != ?', 'RssFeed' ], :order => 'published_at DESC, id DESC'
4 4
5 attr_accessor :feed_attrs 5 attr_accessor :feed_attrs
6 - attr_accessor :filter  
7 6
8 after_create do |blog| 7 after_create do |blog|
9 blog.children << RssFeed.new(:name => 'feed', :profile => blog.profile) 8 blog.children << RssFeed.new(:name => 'feed', :profile => blog.profile)
@@ -23,7 +22,9 @@ class Blog &lt; Folder @@ -23,7 +22,9 @@ class Blog &lt; Folder
23 # FIXME isn't this too much including just to be able to generate some HTML? 22 # FIXME isn't this too much including just to be able to generate some HTML?
24 include ActionView::Helpers::TagHelper 23 include ActionView::Helpers::TagHelper
25 def to_html(options = {}) 24 def to_html(options = {})
26 - posts_list(options[:page]) 25 + lambda do
  26 + render :file => 'content_viewer/blog_page'
  27 + end
27 end 28 end
28 29
29 def folder? 30 def folder?
@@ -49,19 +50,6 @@ class Blog &lt; Folder @@ -49,19 +50,6 @@ class Blog &lt; Folder
49 self.feed 50 self.feed
50 end 51 end
51 52
52 - def posts_list(npage)  
53 - article = self  
54 - children = if filter and filter[:year] and filter[:month]  
55 - filter_date = DateTime.parse("#{filter[:year]}-#{filter[:month]}-01")  
56 - posts.paginate :page => npage, :per_page => posts_per_page, :conditions => [ 'published_at between ? and ?', filter_date, filter_date + 1.month - 1.day ]  
57 - else  
58 - posts.paginate :page => npage, :per_page => posts_per_page  
59 - end  
60 - lambda do  
61 - render :file => 'content_viewer/blog_page', :locals => {:article => article, :children => children}  
62 - end  
63 - end  
64 -  
65 has_one :external_feed, :foreign_key => 'blog_id', :dependent => :destroy 53 has_one :external_feed, :foreign_key => 'blog_id', :dependent => :destroy
66 54
67 attr_accessor :external_feed_data 55 attr_accessor :external_feed_data
app/models/event.rb
@@ -64,17 +64,6 @@ class Event &lt; Article @@ -64,17 +64,6 @@ class Event &lt; Article
64 first_day..last_day 64 first_day..last_day
65 end 65 end
66 66
67 - def self.first_day_of_month(date)  
68 - date ||= Date.today  
69 - Date.new(date.year, date.month, 1)  
70 - end  
71 -  
72 - def self.last_day_of_month(date)  
73 - date ||= Date.today  
74 - date >>= 1  
75 - Date.new(date.year, date.month, 1) - 1.day  
76 - end  
77 -  
78 def date_range 67 def date_range
79 start_date..(end_date||start_date) 68 start_date..(end_date||start_date)
80 end 69 end
app/views/content_viewer/blog_page.rhtml
1 -<% add_rss_feed_to_head(article.name, article.feed.url) if article.blog? && article.feed %> 1 +<% add_rss_feed_to_head(@page.name, @page.feed.url) if @page.blog? && @page.feed %>
2 2
3 -<%= content_tag('em', _('(external feed was not loaded yet)'), :id => 'external-feed-info', :class => 'metadata') if article.blog? && article.external_feed && article.external_feed.enabled && article.external_feed.fetched_at.nil? %> 3 +<%= content_tag('em', _('(external feed was not loaded yet)'), :id => 'external-feed-info', :class => 'metadata') if @page.blog? && @page.external_feed && @page.external_feed.enabled && @page.external_feed.fetched_at.nil? %>
4 4
5 <div> 5 <div>
6 <div class='blog-description'> 6 <div class='blog-description'>
7 - <%= article.body %> 7 + <%= @page.body %>
8 </div> 8 </div>
9 </div> 9 </div>
10 <hr class="pre-posts"/> 10 <hr class="pre-posts"/>
11 <div class="blog-posts"> 11 <div class="blog-posts">
12 - <%= (children.compact.empty? ? content_tag('em', _('(no posts)')) : list_posts(user, children, article.visualization_format)) %> 12 + <%= (@posts.compact.empty? ? content_tag('em', _('(no posts)')) : list_posts(@posts, @page.visualization_format)) %>
13 </div> 13 </div>
test/functional/content_viewer_controller_test.rb
@@ -603,10 +603,27 @@ class ContentViewerControllerTest &lt; Test::Unit::TestCase @@ -603,10 +603,27 @@ class ContentViewerControllerTest &lt; Test::Unit::TestCase
603 assert_response :missing 603 assert_response :missing
604 end 604 end
605 605
  606 + should 'list unpublished posts to owner with a different class' do
  607 + login_as('testinguser')
  608 + blog = Blog.create!(:name => 'A blog test', :profile => profile)
  609 + blog.posts << TextileArticle.create!(:name => 'Post', :profile => profile, :parent => blog, :published => false)
  610 +
  611 + get :view_page, :profile => profile.identifier, :page => [blog.path]
  612 + assert_tag :tag => 'div', :attributes => {:class => /not-published/}
  613 + end
  614 +
  615 + should 'not list unpublished posts to a not logged person' do
  616 + blog = Blog.create!(:name => 'A blog test', :profile => profile)
  617 + blog.posts << TextileArticle.create!(:name => 'Post', :profile => profile, :parent => blog, :published => false)
  618 +
  619 + get :view_page, :profile => profile.identifier, :page => [blog.path]
  620 + assert_no_tag :tag => 'a', :content => "Post"
  621 + end
  622 +
606 should 'display pagination links of blog' do 623 should 'display pagination links of blog' do
607 blog = Blog.create!(:name => 'A blog test', :profile => profile, :posts_per_page => 5) 624 blog = Blog.create!(:name => 'A blog test', :profile => profile, :posts_per_page => 5)
608 for n in 1..10 625 for n in 1..10
609 - blog.children << TextileArticle.create!(:name => "Post #{n}", :profile => profile, :parent => blog) 626 + blog.posts << TextileArticle.create!(:name => "Post #{n}", :profile => profile, :parent => blog)
610 end 627 end
611 assert_equal 10, blog.posts.size 628 assert_equal 10, blog.posts.size
612 629
@@ -647,10 +664,20 @@ class ContentViewerControllerTest &lt; Test::Unit::TestCase @@ -647,10 +664,20 @@ class ContentViewerControllerTest &lt; Test::Unit::TestCase
647 end 664 end
648 665
649 should 'set year and month filter from URL params' do 666 should 'set year and month filter from URL params' do
650 - profile.articles << Blog.new(:name => 'A blog test', :profile => profile) 667 + blog = Blog.create!(:name => "blog", :profile => profile)
  668 + profile.articles << blog
  669 +
  670 + past_post = TextileArticle.create!(:name => "past post", :profile => profile, :parent => blog, :created_at => blog.created_at - 1.year)
  671 + actual_post = TextileArticle.create!(:name => "actual post", :profile => profile, :parent => blog)
  672 + blog.children << past_post
  673 + blog.children << actual_post
  674 +
651 year, month = profile.blog.created_at.year.to_s, '%02d' % profile.blog.created_at.month 675 year, month = profile.blog.created_at.year.to_s, '%02d' % profile.blog.created_at.month
  676 +
652 get :view_page, :profile => profile.identifier, :page => [profile.blog.path], :year => year, :month => month 677 get :view_page, :profile => profile.identifier, :page => [profile.blog.path], :year => year, :month => month
653 - assert_equal({ :year => year.to_s, :month => month.to_s }, assigns(:page).filter) 678 +
  679 + assert_no_tag :tag => 'a', :content => past_post.title
  680 + assert_tag :tag => 'a', :content => actual_post.title
654 end 681 end
655 682
656 should 'give link to create new article inside folder when view child of folder' do 683 should 'give link to create new article inside folder when view child of folder' do
test/integration/performance_test.rb
@@ -28,6 +28,10 @@ class PerformanceTest &lt; ActionController::IntegrationTest @@ -28,6 +28,10 @@ class PerformanceTest &lt; ActionController::IntegrationTest
28 person2 = create_profile('person2') 28 person2 = create_profile('person2')
29 create_posts(person2, 100) 29 create_posts(person2, 100)
30 30
  31 + get '/person0/blog'
  32 + get '/person1/blog'
  33 + get '/person2/blog'
  34 +
31 # no posts 35 # no posts
32 time0 = (Benchmark.measure { 10.times { get '/person0/blog' } }) 36 time0 = (Benchmark.measure { 10.times { get '/person0/blog' } })
33 37
test/unit/article_test.rb
@@ -1193,4 +1193,22 @@ class ArticleTest &lt; Test::Unit::TestCase @@ -1193,4 +1193,22 @@ class ArticleTest &lt; Test::Unit::TestCase
1193 assert_equal 6, ActionTrackerNotification.count 1193 assert_equal 6, ActionTrackerNotification.count
1194 end 1194 end
1195 1195
  1196 + should 'found articles with published date between a range' do
  1197 + start_date = DateTime.parse('2010-07-06')
  1198 + end_date = DateTime.parse('2010-08-02')
  1199 +
  1200 + article_found1 = fast_create(Article, :published_at => start_date)
  1201 + article_found2 = fast_create(Article, :published_at => end_date)
  1202 + article_not_found = fast_create(Article, :published_at => end_date + 1.month)
  1203 +
  1204 + assert_includes Article.by_range(start_date..end_date), article_found1
  1205 + assert_includes Article.by_range(start_date..end_date), article_found2
  1206 + assert_not_includes Article.by_range(start_date..end_date), article_not_found
  1207 + end
  1208 +
  1209 + should 'calculate first/end day of a month' do
  1210 + assert_equal 1, (DateTime.parse('2010-07-06')).at_beginning_of_month.day
  1211 + assert_equal 31, (DateTime.parse('2010-07-06')).at_end_of_month.day
  1212 + end
  1213 +
1196 end 1214 end
test/unit/blog_helper_test.rb
@@ -27,28 +27,7 @@ class BlogHelperTest &lt; Test::Unit::TestCase @@ -27,28 +27,7 @@ class BlogHelperTest &lt; Test::Unit::TestCase
27 expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", :class => 'blog-post position-1 first last odd-post-inner', :id => "post-#{published_post.id}").returns('POST') 27 expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", :class => 'blog-post position-1 first last odd-post-inner', :id => "post-#{published_post.id}").returns('POST')
28 expects(:content_tag).with('div', 'POST', {:class => 'odd-post'}).returns('RESULT') 28 expects(:content_tag).with('div', 'POST', {:class => 'odd-post'}).returns('RESULT')
29 29
30 - assert_equal 'RESULT', list_posts(profile, blog.posts)  
31 - end  
32 -  
33 - should 'list unpublished posts to owner with a different class' do  
34 - blog.children << unpublished_post = TextileArticle.create!(:name => 'Post', :profile => profile, :parent => blog, :published => false)  
35 -  
36 - expects(:display_post).with(anything, anything).returns('POST')  
37 - expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", :class => 'blog-post position-1 first last not-published odd-post-inner', :id => "post-#{unpublished_post.id}").returns('POST')  
38 - expects(:content_tag).with('div', 'POST', {:class => 'odd-post'}).returns('RESULT')  
39 - assert_equal 'RESULT', list_posts(profile, blog.posts)  
40 - end  
41 -  
42 - should 'not list unpublished posts to not owner' do  
43 - blog.children << unpublished_post = TextileArticle.create!(:name => 'First post', :profile => profile, :parent => blog, :published => false)  
44 -  
45 - blog.children << published_post = TextileArticle.create!(:name => 'Second post', :profile => profile, :parent => blog, :published => true)  
46 -  
47 - expects(:display_post).with(anything, anything).returns('POST')  
48 - expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", has_entries(:id => "post-#{unpublished_post.id}")).never  
49 - expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", has_entries(:id => "post-#{published_post.id}")).returns('POST')  
50 - expects(:content_tag).with('div', 'POST', {:class => 'odd-post'}).returns('RESULT')  
51 - assert_equal 'RESULT', list_posts(nil, blog.posts) 30 + assert_equal 'RESULT', list_posts(blog.posts)
52 end 31 end
53 32
54 should 'list even/odd posts with a different class' do 33 should 'list even/odd posts with a different class' do
@@ -64,7 +43,7 @@ class BlogHelperTest &lt; Test::Unit::TestCase @@ -64,7 +43,7 @@ class BlogHelperTest &lt; Test::Unit::TestCase
64 expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", :class => 'blog-post position-2 last even-post-inner', :id => "post-#{older_post.id}").returns('POST 2') 43 expects(:content_tag).with('div', "POST<br style=\"clear:both\"/>", :class => 'blog-post position-2 last even-post-inner', :id => "post-#{older_post.id}").returns('POST 2')
65 expects(:content_tag).with('div', "POST 2", :class => 'even-post').returns('EVEN-POST') 44 expects(:content_tag).with('div', "POST 2", :class => 'even-post').returns('EVEN-POST')
66 45
67 - assert_equal "ODD-POST\n<hr class='sep-posts'/>\nEVEN-POST", list_posts(nil, blog.posts) 46 + assert_equal "ODD-POST\n<hr class='sep-posts'/>\nEVEN-POST", list_posts(blog.posts)
68 end 47 end
69 48
70 49
test/unit/blog_test.rb
@@ -82,13 +82,6 @@ class BlogTest &lt; ActiveSupport::TestCase @@ -82,13 +82,6 @@ class BlogTest &lt; ActiveSupport::TestCase
82 assert_equal [newer, older], blog.posts 82 assert_equal [newer, older], blog.posts
83 end 83 end
84 84
85 - should 'has filter' do  
86 - p = create_user('testuser').person  
87 - blog = Blog.create!(:profile => p, :name => 'Blog test')  
88 - blog.filter = {:param => 'value'}  
89 - assert_equal 'value', blog.filter[:param]  
90 - end  
91 -  
92 should 'has one external feed' do 85 should 'has one external feed' do
93 p = create_user('testuser').person 86 p = create_user('testuser').person
94 blog = fast_create(Blog, :profile_id => p.id, :name => 'Blog test') 87 blog = fast_create(Blog, :profile_id => p.id, :name => 'Blog test')
test/unit/content_viewer_helper_test.rb
@@ -57,32 +57,10 @@ class ContentViewerHelperTest &lt; Test::Unit::TestCase @@ -57,32 +57,10 @@ class ContentViewerHelperTest &lt; Test::Unit::TestCase
57 should 'not list feed article' do 57 should 'not list feed article' do
58 profile.articles << Blog.new(:name => 'Blog test', :profile => profile) 58 profile.articles << Blog.new(:name => 'Blog test', :profile => profile)
59 assert_includes profile.blog.children.map{|i| i.class}, RssFeed 59 assert_includes profile.blog.children.map{|i| i.class}, RssFeed
60 - result = list_posts(nil, profile.blog.posts) 60 + result = list_posts(profile.blog.posts)
61 assert_no_match /feed/, result 61 assert_no_match /feed/, result
62 end 62 end
63 63
64 - should 'filter blog posts by date' do  
65 - blog = Blog.create!(:name => 'Blog test', :profile => profile)  
66 -  
67 - nov = TextileArticle.create!(:name => 'November post', :parent => blog, :profile => profile)  
68 - nov.update_attributes!(:published_at => DateTime.parse('2008-11-15'))  
69 -  
70 - sep = TextileArticle.create!(:name => 'September post', :parent => blog, :profile => profile)  
71 - sep.update_attribute(:published_at, DateTime.parse('2008-09-10'))  
72 -  
73 - blog.reload  
74 - blog.filter = {:year => 2008, :month => 11}  
75 - assert blog.save!  
76 -  
77 - self.stubs(:params).returns({:npage => nil})  
78 -  
79 - expects(:render).with(:file => 'content_viewer/blog_page', :locals => {:article => blog, :children => [nov]}).returns("BLI")  
80 -  
81 - result = article_to_html(blog)  
82 -  
83 - assert_equal 'BLI', result  
84 - end  
85 -  
86 end 64 end
87 65
88 def show_date(date) 66 def show_date(date)