Commit 518ec8308de685eb394b4a7005b9fe43032d7b49
Exists in
master
and in
23 other branches
Merge commit 'refs/merge-requests/233' of git://gitorious.org/noosfero/noosfero …
…into merge-requests/233 Conflicts: test/integration/routing_test.rb
Showing
19 changed files
with
109 additions
and
75 deletions
Show diff stats
app/controllers/public/not_found_controller.rb
app/controllers/public/profile_controller.rb
| ... | ... | @@ -49,36 +49,36 @@ class ProfileController < PublicController |
| 49 | 49 | |
| 50 | 50 | def communities |
| 51 | 51 | if is_cache_expired?(profile.communities_cache_key(params)) |
| 52 | - @communities = profile.communities.paginate(:per_page => per_page, :page => params[:npage]) | |
| 52 | + @communities = profile.communities.includes(relations_to_include).paginate(:per_page => per_page, :page => params[:npage]) | |
| 53 | 53 | end |
| 54 | 54 | end |
| 55 | 55 | |
| 56 | 56 | def enterprises |
| 57 | - @enterprises = profile.enterprises | |
| 57 | + @enterprises = profile.enterprises.includes(relations_to_include) | |
| 58 | 58 | end |
| 59 | 59 | |
| 60 | 60 | def friends |
| 61 | 61 | if is_cache_expired?(profile.friends_cache_key(params)) |
| 62 | - @friends = profile.friends.paginate(:per_page => per_page, :page => params[:npage]) | |
| 62 | + @friends = profile.friends.includes(relations_to_include).paginate(:per_page => per_page, :page => params[:npage]) | |
| 63 | 63 | end |
| 64 | 64 | end |
| 65 | 65 | |
| 66 | 66 | def members |
| 67 | 67 | if is_cache_expired?(profile.members_cache_key(params)) |
| 68 | - @members = profile.members.paginate(:per_page => members_per_page, :page => params[:npage]) | |
| 68 | + @members = profile.members.includes(relations_to_include).paginate(:per_page => members_per_page, :page => params[:npage]) | |
| 69 | 69 | end |
| 70 | 70 | end |
| 71 | 71 | |
| 72 | 72 | def fans |
| 73 | - @fans = profile.fans | |
| 73 | + @fans = profile.fans.includes(relations_to_include) | |
| 74 | 74 | end |
| 75 | 75 | |
| 76 | 76 | def favorite_enterprises |
| 77 | - @favorite_enterprises = profile.favorite_enterprises | |
| 77 | + @favorite_enterprises = profile.favorite_enterprises.includes(relations_to_include) | |
| 78 | 78 | end |
| 79 | 79 | |
| 80 | 80 | def sitemap |
| 81 | - @articles = profile.top_level_articles | |
| 81 | + @articles = profile.top_level_articles.includes([:profile, :parent]) | |
| 82 | 82 | end |
| 83 | 83 | |
| 84 | 84 | def join |
| ... | ... | @@ -264,7 +264,7 @@ class ProfileController < PublicController |
| 264 | 264 | |
| 265 | 265 | def profile_info |
| 266 | 266 | begin |
| 267 | - @block = profile.blocks.find(params[:block_id]) | |
| 267 | + @block = profile.blocks.find(params[:block_id]).includes(:box) | |
| 268 | 268 | rescue |
| 269 | 269 | render :text => _('Profile information could not be loaded') |
| 270 | 270 | end |
| ... | ... | @@ -378,4 +378,8 @@ class ProfileController < PublicController |
| 378 | 378 | @can_edit_profile ||= user && user.has_permission?('edit_profile', profile) |
| 379 | 379 | end |
| 380 | 380 | helper_method :can_edit_profile |
| 381 | + | |
| 382 | + def relations_to_include | |
| 383 | + [:image, :domains, :preferred_domain, :environment] | |
| 384 | + end | |
| 381 | 385 | end | ... | ... |
app/controllers/public/search_controller.rb
| ... | ... | @@ -46,7 +46,7 @@ class SearchController < PublicController |
| 46 | 46 | if !@empty_query |
| 47 | 47 | full_text_search ['public:true'] |
| 48 | 48 | else |
| 49 | - @results[@asset] = @environment.people.visible.send(@filter).paginate(paginate_options) | |
| 49 | + @results[@asset] = visible_profiles(Person).send(@filter).paginate(paginate_options) | |
| 50 | 50 | end |
| 51 | 51 | end |
| 52 | 52 | |
| ... | ... | @@ -76,7 +76,7 @@ class SearchController < PublicController |
| 76 | 76 | full_text_search ['public:true'] |
| 77 | 77 | else |
| 78 | 78 | @filter_title = _('Enterprises from network') |
| 79 | - @results[@asset] = @environment.enterprises.visible.paginate(paginate_options) | |
| 79 | + @results[@asset] = visible_profiles(Enterprise, [{:products => :product_category}]).paginate(paginate_options) | |
| 80 | 80 | end |
| 81 | 81 | end |
| 82 | 82 | |
| ... | ... | @@ -84,7 +84,7 @@ class SearchController < PublicController |
| 84 | 84 | if !@empty_query |
| 85 | 85 | full_text_search ['public:true'] |
| 86 | 86 | else |
| 87 | - @results[@asset] = @environment.communities.visible.send(@filter).paginate(paginate_options) | |
| 87 | + @results[@asset] = visible_profiles(Community).send(@filter).paginate(paginate_options) | |
| 88 | 88 | end |
| 89 | 89 | end |
| 90 | 90 | |
| ... | ... | @@ -310,4 +310,12 @@ class SearchController < PublicController |
| 310 | 310 | @all_facets = ret[:all_facets] |
| 311 | 311 | end |
| 312 | 312 | |
| 313 | + private | |
| 314 | + | |
| 315 | + def visible_profiles(klass, *extra_relations) | |
| 316 | + relations = [:image, :domains, :environment, :preferred_domain] | |
| 317 | + relations += extra_relations | |
| 318 | + @environment.send(klass.name.underscore.pluralize).visible.includes(relations) | |
| 319 | + end | |
| 320 | + | |
| 313 | 321 | end | ... | ... |
app/helpers/boxes_helper.rb
| ... | ... | @@ -66,7 +66,7 @@ module BoxesHelper |
| 66 | 66 | |
| 67 | 67 | def display_box_content(box, main_content) |
| 68 | 68 | context = { :article => @page, :request_path => request.path, :locale => locale } |
| 69 | - box_decorator.select_blocks(box.blocks, context).map { |item| display_block(item, main_content) }.join("\n") + box_decorator.block_target(box) | |
| 69 | + box_decorator.select_blocks(box.blocks.includes(:box), context).map { |item| display_block(item, main_content) }.join("\n") + box_decorator.block_target(box) | |
| 70 | 70 | end |
| 71 | 71 | |
| 72 | 72 | def select_blocks(arr, context) | ... | ... |
app/models/article.rb
| ... | ... | @@ -179,37 +179,23 @@ class Article < ActiveRecord::Base |
| 179 | 179 | end |
| 180 | 180 | |
| 181 | 181 | named_scope :more_popular, :order => 'hits DESC' |
| 182 | - | |
| 183 | - # retrieves the latest +limit+ articles, sorted from the most recent to the | |
| 184 | - # oldest. | |
| 185 | - # | |
| 186 | - # Only includes articles where advertise == true | |
| 187 | - def self.recent(limit = nil, extra_conditions = {}) | |
| 188 | - # FIXME this method is a horrible hack | |
| 189 | - options = { :page => 1, :per_page => limit, | |
| 190 | - :conditions => [ | |
| 191 | - "advertise = ? AND | |
| 192 | - published = ? AND | |
| 193 | - profiles.visible = ? AND | |
| 194 | - profiles.public_profile = ? AND | |
| 195 | - ((articles.type != ? and articles.type != ? and articles.type != ?) OR articles.type is NULL)", true, true, true, true, 'UploadedFile', 'RssFeed', 'Blog' | |
| 196 | - ], | |
| 197 | - :include => 'profile', | |
| 198 | - :order => 'articles.published_at desc, articles.id desc' | |
| 199 | - } | |
| 200 | - if ( scoped_methods && scoped_methods.last && | |
| 201 | - scoped_methods.last[:find] && | |
| 202 | - scoped_methods.last[:find][:joins] && | |
| 203 | - scoped_methods.last[:find][:joins].index('profiles') ) | |
| 204 | - options.delete(:include) | |
| 205 | - end | |
| 206 | - if extra_conditions == {} | |
| 207 | - self.paginate(options) | |
| 208 | - else | |
| 209 | - with_scope :find => {:conditions => extra_conditions} do | |
| 210 | - self.paginate(options) | |
| 211 | - end | |
| 182 | + named_scope :relevant_as_recent, :conditions => ["(articles.type != 'UploadedFile' and articles.type != 'RssFeed' and articles.type != 'Blog') OR articles.type is NULL"] | |
| 183 | + | |
| 184 | + def self.recent(limit = nil, extra_conditions = {}, pagination = true) | |
| 185 | + result = scoped({:conditions => extra_conditions}). | |
| 186 | + public. | |
| 187 | + relevant_as_recent. | |
| 188 | + limit(limit). | |
| 189 | + order(['articles.published_at desc', 'articles.id desc']) | |
| 190 | + | |
| 191 | + if !( scoped_methods && scoped_methods.last && | |
| 192 | + scoped_methods.last[:find] && | |
| 193 | + scoped_methods.last[:find][:joins] && | |
| 194 | + scoped_methods.last[:find][:joins].index('profiles') ) | |
| 195 | + result = result.includes(:profile) | |
| 212 | 196 | end |
| 197 | + | |
| 198 | + pagination ? result.paginate({:page => 1, :per_page => limit}) : result | |
| 213 | 199 | end |
| 214 | 200 | |
| 215 | 201 | # produces the HTML code that is to be displayed as this article's contents. | ... | ... |
app/models/enterprise.rb
| ... | ... | @@ -17,7 +17,7 @@ class Enterprise < Organization |
| 17 | 17 | after_save_reindex [:products], :with => :delayed_job |
| 18 | 18 | extra_data_for_index :product_categories |
| 19 | 19 | def product_categories |
| 20 | - products.map{|p| p.category_full_name}.compact | |
| 20 | + products.includes(:product_category).map{|p| p.category_full_name}.compact | |
| 21 | 21 | end |
| 22 | 22 | |
| 23 | 23 | N_('Organization website'); N_('Historic and current context'); N_('Activities short description'); N_('City'); N_('State'); N_('Country'); N_('ZIP code') | ... | ... |
app/models/environment.rb
| ... | ... | @@ -591,8 +591,8 @@ class Environment < ActiveRecord::Base |
| 591 | 591 | end |
| 592 | 592 | |
| 593 | 593 | has_many :articles, :through => :profiles |
| 594 | - def recent_documents(limit = 10) | |
| 595 | - self.articles.recent(limit) | |
| 594 | + def recent_documents(limit = 10, options = {}, pagination = true) | |
| 595 | + self.articles.recent(limit, options, pagination) | |
| 596 | 596 | end |
| 597 | 597 | |
| 598 | 598 | has_many :events, :through => :profiles, :source => :articles, :class_name => 'Event' | ... | ... |
app/models/person.rb
| ... | ... | @@ -248,7 +248,7 @@ class Person < Profile |
| 248 | 248 | |
| 249 | 249 | def is_admin?(environment = nil) |
| 250 | 250 | environment ||= self.environment |
| 251 | - role_assignments.select { |ra| ra.resource == environment }.map{|ra|ra.role.permissions}.any? do |ps| | |
| 251 | + role_assignments.includes([:role, :resource]).select { |ra| ra.resource == environment }.map{|ra|ra.role.permissions}.any? do |ps| | |
| 252 | 252 | ps.any? do |p| |
| 253 | 253 | ActiveRecord::Base::PERMISSIONS['Environment'].keys.include?(p) |
| 254 | 254 | end | ... | ... |
app/models/profile.rb
| ... | ... | @@ -398,8 +398,8 @@ class Profile < ActiveRecord::Base |
| 398 | 398 | # |
| 399 | 399 | # +limit+ is the maximum number of documents to be returned. It defaults to |
| 400 | 400 | # 10. |
| 401 | - def recent_documents(limit = 10, options = {}) | |
| 402 | - self.articles.recent(limit, options) | |
| 401 | + def recent_documents(limit = 10, options = {}, pagination = true) | |
| 402 | + self.articles.recent(limit, options, pagination) | |
| 403 | 403 | end |
| 404 | 404 | |
| 405 | 405 | def last_articles(limit = 10, options = {}) | ... | ... |
app/models/profile_list_block.rb
| ... | ... | @@ -14,12 +14,13 @@ class ProfileListBlock < Block |
| 14 | 14 | |
| 15 | 15 | def profile_list |
| 16 | 16 | result = nil |
| 17 | + visible_profiles = profiles.visible.includes([:image,:domains,:preferred_domain,:environment]) | |
| 17 | 18 | if !prioritize_profiles_with_image |
| 18 | - result = profiles.visible.all(:limit => limit, :order => 'updated_at DESC').sort_by{ rand } | |
| 19 | - elsif profiles.visible.with_image.count >= limit | |
| 20 | - result = profiles.visible.with_image.all(:limit => limit * 5, :order => 'updated_at DESC').sort_by{ rand } | |
| 19 | + result = visible_profiles.all(:limit => limit, :order => 'updated_at DESC').sort_by{ rand } | |
| 20 | + elsif visible_profiles.with_image.count >= limit | |
| 21 | + result = visible_profiles.with_image.all(:limit => limit * 5, :order => 'updated_at DESC').sort_by{ rand } | |
| 21 | 22 | else |
| 22 | - result = profiles.visible.with_image.sort_by{ rand } + profiles.visible.without_image.all(:limit => limit * 5, :order => 'updated_at DESC').sort_by{ rand } | |
| 23 | + result = visible_profiles.with_image.sort_by{ rand } + visible_profiles.without_image.all(:limit => limit * 5, :order => 'updated_at DESC').sort_by{ rand } | |
| 23 | 24 | end |
| 24 | 25 | result.slice(0..limit-1) |
| 25 | 26 | end | ... | ... |
app/models/recent_documents_block.rb
| ... | ... | @@ -16,11 +16,9 @@ class RecentDocumentsBlock < Block |
| 16 | 16 | |
| 17 | 17 | include ActionController::UrlWriter |
| 18 | 18 | def content(args={}) |
| 19 | - docs = self.limit.nil? ? owner.recent_documents : owner.recent_documents(self.limit) | |
| 20 | - | |
| 19 | + docs = self.limit.nil? ? owner.recent_documents(nil, {}, false) : owner.recent_documents(self.limit, {}, false) | |
| 21 | 20 | block_title(title) + |
| 22 | 21 | content_tag('ul', docs.map {|item| content_tag('li', link_to(h(item.title), item.url))}.join("\n")) |
| 23 | - | |
| 24 | 22 | end |
| 25 | 23 | |
| 26 | 24 | def footer | ... | ... |
app/views/profile/_person_profile.rhtml
| ... | ... | @@ -37,7 +37,7 @@ |
| 37 | 37 | <tr> |
| 38 | 38 | <th colspan='2'><%= __('Enterprises') %></th> |
| 39 | 39 | </tr> |
| 40 | - <% profile.enterprises.each do |item| %> | |
| 40 | + <% profile.enterprises.includes(:environment,:domains, :preferred_domain).each do |item| %> | |
| 41 | 41 | <tr> |
| 42 | 42 | <td></td> |
| 43 | 43 | <td><%= button 'menu-enterprise', item.name, item.url %></td> | ... | ... |
config/routes.rb
| ... | ... | @@ -23,13 +23,13 @@ ActionController::Routing::Routes.draw do |map| |
| 23 | 23 | map.connect '', :controller => "home", :conditions => { :if => lambda { |env| !Domain.hosting_profile_at(env[:host]) } } |
| 24 | 24 | map.home 'site/:action', :controller => 'home' |
| 25 | 25 | |
| 26 | - map.connect 'images/*stuff', :controller => 'not_found', :action => 'index' | |
| 27 | - map.connect 'stylesheets/*stuff', :controller => 'not_found', :action => 'index' | |
| 28 | - map.connect 'designs/*stuff', :controller => 'not_found', :action => 'index' | |
| 29 | - map.connect 'articles/*stuff', :controller => 'not_found', :action => 'index' | |
| 30 | - map.connect 'javascripts/*stuff', :controller => 'not_found', :action => 'index' | |
| 31 | - map.connect 'thumbnails/*stuff', :controller => 'not_found', :action => 'index' | |
| 32 | - map.connect 'user_themes/*stuff', :controller => 'not_found', :action => 'index' | |
| 26 | + map.connect 'images/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 27 | + map.connect 'stylesheets/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 28 | + map.connect 'designs/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 29 | + map.connect 'articles/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 30 | + map.connect 'javascripts/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 31 | + map.connect 'thumbnails/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 32 | + map.connect 'user_themes/*stuff', :controller => 'not_found', :action => 'nothing' | |
| 33 | 33 | |
| 34 | 34 | # online documentation |
| 35 | 35 | map.doc 'doc', :controller => 'doc', :action => 'index' | ... | ... |
lib/acts_as_having_boxes.rb
test/functional/profile_design_controller_test.rb
| ... | ... | @@ -322,11 +322,14 @@ class ProfileDesignControllerTest < ActionController::TestCase |
| 322 | 322 | should 'be able to save FeedReaderBlock configurations' do |
| 323 | 323 | @box1.blocks << FeedReaderBlock.new(:address => 'feed address') |
| 324 | 324 | holder.blocks(true) |
| 325 | + block = @box1.blocks.last | |
| 325 | 326 | |
| 326 | - post :save, :profile => 'designtestuser', :id => @box1.blocks[-1].id, :block => {:address => 'new feed address', :limit => '20'} | |
| 327 | + post :save, :profile => 'designtestuser', :id => block.id, :block => {:address => 'new feed address', :limit => '20'} | |
| 327 | 328 | |
| 328 | - assert_equal 'new feed address', @box1.blocks[-1].address | |
| 329 | - assert_equal 20, @box1.blocks[-1].limit | |
| 329 | + block.reload | |
| 330 | + | |
| 331 | + assert_equal 'new feed address', block.address | |
| 332 | + assert_equal 20, block.limit | |
| 330 | 333 | end |
| 331 | 334 | |
| 332 | 335 | should 'require login' do | ... | ... |
test/integration/routing_test.rb
| ... | ... | @@ -228,4 +228,32 @@ class RoutingTest < ActionController::IntegrationTest |
| 228 | 228 | assert_routing('/admin/plugin/foo/admin_bar/play/1', {:controller => 'foo_plugin_admin_bar', :action => 'play', :id => '1'}) |
| 229 | 229 | end |
| 230 | 230 | |
| 231 | + def test_not_found_images_on_nothing | |
| 232 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/images/aksdhf') | |
| 233 | + end | |
| 234 | + | |
| 235 | + def test_not_found_stylesheets_on_nothing | |
| 236 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/stylesheets/aksdhf') | |
| 237 | + end | |
| 238 | + | |
| 239 | + def test_not_found_designs_on_nothing | |
| 240 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/designs/aksdhf') | |
| 241 | + end | |
| 242 | + | |
| 243 | + def test_not_found_articles_on_nothing | |
| 244 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/articles/aksdhf') | |
| 245 | + end | |
| 246 | + | |
| 247 | + def test_not_found_javascripts_on_nothing | |
| 248 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/javascripts/aksdhf') | |
| 249 | + end | |
| 250 | + | |
| 251 | + def test_not_found_thumbnails_on_nothing | |
| 252 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/thumbnails/aksdhf') | |
| 253 | + end | |
| 254 | + | |
| 255 | + def test_not_found_user_themes_on_nothing | |
| 256 | + assert_recognizes({:controller => 'not_found', :action => 'nothing', :stuff => ['aksdhf']}, '/user_themes/aksdhf') | |
| 257 | + end | |
| 258 | + | |
| 231 | 259 | end | ... | ... |
test/unit/boxes_helper_test.rb
| ... | ... | @@ -46,7 +46,8 @@ class BoxesHelperTest < ActiveSupport::TestCase |
| 46 | 46 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] |
| 47 | 47 | b.display = 'never'; b.save! |
| 48 | 48 | box = b.box |
| 49 | - box.expects(:blocks).returns([b]) | |
| 49 | + box.blocks = [b] | |
| 50 | + box.save! | |
| 50 | 51 | expects(:display_block).with(b, '') |
| 51 | 52 | expects(:request).returns(request) |
| 52 | 53 | stubs(:block_target).returns('') |
| ... | ... | @@ -64,7 +65,8 @@ class BoxesHelperTest < ActiveSupport::TestCase |
| 64 | 65 | b = p.blocks.select{|bk| !bk.kind_of?(MainBlock) }[0] |
| 65 | 66 | b.display = 'never'; b.save! |
| 66 | 67 | box = b.box |
| 67 | - box.expects(:blocks).returns([b]) | |
| 68 | + box.blocks = [b] | |
| 69 | + box.save! | |
| 68 | 70 | expects(:display_block).with(b, '').never |
| 69 | 71 | expects(:request).returns(request) |
| 70 | 72 | stubs(:block_target).returns('') |
| ... | ... | @@ -106,9 +108,7 @@ class BoxesHelperTest < ActiveSupport::TestCase |
| 106 | 108 | |
| 107 | 109 | should 'fill context with the article, request_path and locale' do |
| 108 | 110 | request = mock() |
| 109 | - box = mock() | |
| 110 | - | |
| 111 | - box.expects(:blocks).returns([]) | |
| 111 | + box = Box.create!(:owner => fast_create(Profile)) | |
| 112 | 112 | request.expects(:path).returns('/') |
| 113 | 113 | expects(:request).returns(request) |
| 114 | 114 | expects(:locale).returns('en') | ... | ... |
test/unit/enterprise_test.rb
| ... | ... | @@ -482,9 +482,11 @@ class EnterpriseTest < ActiveSupport::TestCase |
| 482 | 482 | |
| 483 | 483 | should 'reindex products with full category name after save' do |
| 484 | 484 | product = mock |
| 485 | + products = mock | |
| 485 | 486 | product.expects(:category_full_name) |
| 486 | - Enterprise.any_instance.stubs(:products).returns([product]) | |
| 487 | - Enterprise.expects(:solr_batch_add).with(includes(product)) | |
| 487 | + products.stubs(:includes).returns([product]) | |
| 488 | + Enterprise.any_instance.stubs(:products).returns(products) | |
| 489 | + Enterprise.expects(:solr_batch_add).with([products]) | |
| 488 | 490 | ent = fast_create(Enterprise) |
| 489 | 491 | ent.save! |
| 490 | 492 | end | ... | ... |
vendor/plugins/access_control/lib/acts_as_accessor.rb
| ... | ... | @@ -4,7 +4,7 @@ class ActiveRecord::Base |
| 4 | 4 | |
| 5 | 5 | def has_permission?(permission, resource = nil) |
| 6 | 6 | return true if resource == self |
| 7 | - role_assignments.any? {|ra| ra.has_permission?(permission, resource)} | |
| 7 | + role_assignments.includes([:resource,:role]).any? {|ra| ra.has_permission?(permission, resource)} | |
| 8 | 8 | end |
| 9 | 9 | |
| 10 | 10 | def define_roles(roles, resource) | ... | ... |