Download as
Browse Code »

Commit cad89415a89ad2054f173283e46968108414b3c4

Authored by Rodrigo Souto
1 parent 5a911ba4
Exists in master and in 22 other branches angular_poc, api-article-archived, article-readonly, backup-7-jan-2016, captcha_serpro_plugin, content-manager-hostspot, export-comment-paragraph, fix_event_date_issue, login-captcha, master_rails3, new_video_plugin, pg_search_rank, production, refactor_with_role, refactor_with_role_scope, remove_profile_cat_icons, resend_confirmation_email, staging, staging_rails3, tasks_keep_filter_params, theme-brasil-digital-from-staging, travis

Fixing blog archives block performance issue 

Introducing bug (http://noosfero.org/Development/ActionItem2705)
temporarily in order to avoid the performance issues associated with
the necessary loading of all posts to run display_to?. When we develop a
scoped way to retrieve this contents we then will be able to fix this
bug.

(ActionItem2706)
Showing 2 changed files with 32 additions and 25 deletions   Show diff stats
app/models/blog_archives_block.rb
  Diff comments   View file @cad8941
... ... @@ -21,18 +21,22 @@ class BlogArchivesBlock < Block
21 21 end
22 22  
23 23 def visible_posts(person)
24   - blog.posts.native_translations.select {|post| post.display_to?(person)}
  24 + #FIXME Performance issues with display_to. Must convert it to a scope.
  25 + # Checkout this page for further information: http://noosfero.org/Development/ActionItem2705
  26 + blog.posts.published.native_translations #.select {|post| post.display_to?(person)}
25 27 end
26 28  
27 29 def content(args={})
28 30 owner_blog = self.blog
29 31 return nil unless owner_blog
30 32 results = ''
31   - visible_posts(args[:person]).group_by {|i| i.published_at.year }.sort_by { |year,count| -year }.each do |year, results_by_year|
32   - results << content_tag('li', content_tag('strong', "#{year} (#{results_by_year.size})"))
  33 + posts = visible_posts(args[:person])
  34 + posts.count(:all, :group => 'EXTRACT(YEAR FROM published_at)').sort_by {|year, count| -year.to_i}.each do |year, count|
  35 + results << content_tag('li', content_tag('strong', "#{year} (#{count})"))
33 36 results << "<ul class='#{year}-archive'>"
34   - results_by_year.group_by{|i| [ ('%02d' % i.published_at.month()), gettext(MONTHS[i.published_at.month() - 1])]}.sort.reverse.each do |month, results_by_month|
35   - results << content_tag('li', link_to("#{month[1]} (#{results_by_month.size})", owner_blog.url.merge(:year => year, :month => month[0])))
  37 + posts.count(:all, :conditions => ['EXTRACT(YEAR FROM published_at)=?', year], :group => 'EXTRACT(MONTH FROM published_at)').sort_by {|month, count| -month.to_i}.each do |month, count|
  38 + month_name = gettext(MONTHS[month.to_i - 1])
  39 + results << content_tag('li', link_to("#{month_name} (#{count})", owner_blog.url.merge(:year => year, :month => month)))
36 40 end
37 41 results << "</ul>"
38 42 end
... ...
test/unit/blog_archives_block_test.rb
  Diff comments   View file @cad8941
... ... @@ -38,7 +38,7 @@ class BlogArchivesBlockTest &lt; ActiveSupport::TestCase
38 38 end
39 39 block = BlogArchivesBlock.new
40 40 block.stubs(:owner).returns(profile)
41   - assert_tag_in_string block.content, :tag => 'a', :content => 'January (10)', :attributes => {:href => /^http:\/\/.*\/flatline\/blog-one\?month=01&year=2008$/ }
  41 + assert_tag_in_string block.content, :tag => 'a', :content => 'January (10)', :attributes => {:href => /^http:\/\/.*\/flatline\/blog-one\?month=1&year=2008$/ }
42 42 end
43 43  
44 44 should 'order list of amount posts' do
... ... @@ -131,7 +131,7 @@ class BlogArchivesBlockTest &lt; ActiveSupport::TestCase
131 131 end
132 132 block = BlogArchivesBlock.new
133 133 block.stubs(:owner).returns(profile)
134   - assert_tag_in_string block.content, :tag => 'a', :content => 'January (2)', :attributes => {:href => /^http:\/\/.*\/flatline\/blog-one\?month=01&year=2008$/ }
  134 + assert_tag_in_string block.content, :tag => 'a', :content => 'January (2)', :attributes => {:href => /^http:\/\/.*\/flatline\/blog-one\?month=1&year=2008$/ }
135 135 end
136 136  
137 137 should 'not try to load a removed blog' do
... ... @@ -157,22 +157,25 @@ class BlogArchivesBlockTest &lt; ActiveSupport::TestCase
157 157 end
158 158 end
159 159  
160   - should 'not count articles if the user can\'t see them' do
161   - person = create_user('testuser').person
162   - blog = fast_create(Blog, :profile_id => profile.id, :path => 'blog_path')
163   - block = fast_create(BlogArchivesBlock)
164   -
165   - feed = mock()
166   - feed.stubs(:url).returns(blog.url)
167   - blog.stubs(:feed).returns(feed)
168   - block.stubs(:blog).returns(blog)
169   - block.stubs(:owner).returns(profile)
170   -
171   - public_post = fast_create(TextileArticle, :profile_id => profile.id, :parent_id => blog.id, :published => true, :published_at => Time.mktime(2012, 'jan'))
172   - private_post = fast_create(TextileArticle, :profile_id => profile.id, :parent_id => blog.id, :published => false, :published_at => Time.mktime(2012, 'jan'))
173   -
174   - assert_match /January \(1\)/, block.content({:person => person})
175   - assert_match /January \(1\)/, block.content()
176   - assert_match /January \(2\)/, block.content({:person => profile})
177   - end
  160 +#FIXME Performance issues with display_to. Must convert it to a scope.
  161 +# Checkout this page for further information: http://noosfero.org/Development/ActionItem2705
  162 +#
  163 +# should 'not count articles if the user can\'t see them' do
  164 +# person = create_user('testuser').person
  165 +# blog = fast_create(Blog, :profile_id => profile.id, :path => 'blog_path')
  166 +# block = fast_create(BlogArchivesBlock)
  167 +#
  168 +# feed = mock()
  169 +# feed.stubs(:url).returns(blog.url)
  170 +# blog.stubs(:feed).returns(feed)
  171 +# block.stubs(:blog).returns(blog)
  172 +# block.stubs(:owner).returns(profile)
  173 +#
  174 +# public_post = fast_create(TextileArticle, :profile_id => profile.id, :parent_id => blog.id, :published => true, :published_at => Time.mktime(2012, 'jan'))
  175 +# private_post = fast_create(TextileArticle, :profile_id => profile.id, :parent_id => blog.id, :published => false, :published_at => Time.mktime(2012, 'jan'))
  176 +#
  177 +# assert_match /January \(1\)/, block.content({:person => person})
  178 +# assert_match /January \(1\)/, block.content()
  179 +# assert_match /January \(2\)/, block.content({:person => profile})
  180 +# end
178 181 end
... ...