From d07e3bd16b6055a3ba50703b0b02103d7ea5ffab Mon Sep 17 00:00:00 2001 From: MoisesMachado Date: Thu, 3 Jul 2008 00:47:09 +0000 Subject: [PATCH] ActionItem509: implemented pagination infrastructure only left put views helpers --- app/controllers/public/search_controller.rb | 24 +++++++++++------------- app/models/category_finder.rb | 12 +++++++----- app/models/environment_finder.rb | 27 ++++++++++++++++----------- app/views/search/products.rhtml | 2 ++ config/environment.rb | 1 + test/functional/search_controller_test.rb | 10 ++++++++++ test/unit/category_finder_test.rb | 18 ++++++++++++++++++ test/unit/environment_finder_test.rb | 22 ++++++++++++++++++++++ 8 files changed, 87 insertions(+), 29 deletions(-) diff --git a/app/controllers/public/search_controller.rb b/app/controllers/public/search_controller.rb index 04d3dac..df65f08 100644 --- a/app/controllers/public/search_controller.rb +++ b/app/controllers/public/search_controller.rb @@ -89,20 +89,20 @@ class SearchController < ApplicationController def load_product_categories_menu(asset) @results[asset].uniq! @categories_menu = ProductCategory.menu_categories(@product_category, environment).map do |cat| - hits = @finder.count(:products, @filtered_query, calculate_find_options(asset, nil, cat, @region, params[:radius])) + hits = @finder.count(:products, @filtered_query, calculate_find_options(asset, nil, nil, cat, @region, params[:radius])) childs = [] # REFACTOR DUPLICATED CODE inner loop doing the same thing that outter loop childs = cat.children.map do |child| - child_hits = @finder.count(:products, @filtered_query, calculate_find_options(asset, nil, child, @region, params[:radius])) + child_hits = @finder.count(:products, @filtered_query, calculate_find_options(asset, nil, nil, child, @region, params[:radius])) [child, child_hits] end.select{|child, child_hits| child_hits > 0 } [cat, hits, childs] end.select{|cat, hits| hits > 0 } end - def calculate_find_options(asset, limit, product_category, region, radius) + def calculate_find_options(asset, limit, page, product_category, region, radius) - result = { :limit => limit, :product_category => product_category} + result = { :product_category => product_category, :per_page => limit, :page => page } if [:enterprises, :people].include?(asset) && region result.merge!(:within => radius, :region => region.id) end @@ -110,6 +110,12 @@ class SearchController < ApplicationController result end + # limit the number of results per page + # TODO: dont hardcore like this + def limit + 10 + end + public include SearchHelper @@ -125,9 +131,6 @@ class SearchController < ApplicationController [ :events, N_('Events') ] ] - # TODO don't hardcode like this >:-( - LIST_LIMIT = 10 - def complete_region # FIXME this logic should be in the model @regions = Region.find(:all, :conditions => [ '(name like ? or name like ?) and lat is not null and lng is not null', '%' + params[:region][:name] + '%', '%' + params[:region][:name].capitalize + '%' ]) @@ -145,16 +148,11 @@ class SearchController < ApplicationController # how many assets we are searching for? number_of_result_assets = @searching.values.select{|v| v}.size - # apply limit when searching for only one type of asset - limit = (number_of_result_assets == 1) ? nil: LIST_LIMIT - # apply limit to all searches -# limit = nil - @results = {} @names = {} SEARCH_IN.select { |key,description| @searching[key] }.each do |key, description| - @results[key] = @finder.find(key, @filtered_query, calculate_find_options(key, limit, @product_category, @region, params[:radius])) + @results[key] = @finder.find(key, @filtered_query, calculate_find_options(key, limit, params[:page], @product_category, @region, params[:radius])) @names[key] = gettext(description) end diff --git a/app/models/category_finder.rb b/app/models/category_finder.rb index fcb3abc..e815068 100644 --- a/app/models/category_finder.rb +++ b/app/models/category_finder.rb @@ -7,8 +7,6 @@ class CategoryFinder attr_reader :category_id - - def find(asset, query='', options={}) @region = Region.find_by_id(options.delete(:region)) if options.has_key?(:region) if @region && options[:within] @@ -17,10 +15,12 @@ class CategoryFinder options.delete(:within) end + options = {:page => 1, :per_page => options.delete(:limit)}.merge(options) if query.blank? - asset_class(asset).find(:all, options_for_find(asset_class(asset), {:order => "created_at desc, #{asset_table(asset)}.id desc"}.merge(options))) + asset_class(asset).paginate(:all, options_for_find(asset_class(asset), {:order => "created_at desc, #{asset_table(asset)}.id desc"}.merge(options))) else - asset_class(asset).find_by_contents(query, {}, options_for_find(asset_class(asset), options)).uniq + ferret_options = {:page => options.delete(:page), :per_page => options.delete(:per_page)} + asset_class(asset).find_by_contents(query, ferret_options, options_for_find(asset_class(asset), options)).uniq end end @@ -33,8 +33,10 @@ class CategoryFinder end def count(asset, query='', options={}) + # because will_paginate needs a page + options = {:page => 1}.merge(options) if query.blank? - find(asset, query, options).size + find(asset, query, options).total_entries else find(asset, query, options).total_hits end diff --git a/app/models/environment_finder.rb b/app/models/environment_finder.rb index cab280f..13cfdca 100644 --- a/app/models/environment_finder.rb +++ b/app/models/environment_finder.rb @@ -15,22 +15,25 @@ class EnvironmentFinder product_category = options.delete(:product_category) product_category_ids = product_category.map_traversal(&:id) if product_category + options = {:page => 1, :per_page => options.delete(:limit)}.merge(options) if query.blank? - if product_category && asset == :products - @environment.send(asset).find(:all, options.merge({:order => 'created_at desc, id desc', :conditions => ['product_category_id in (?)', product_category_ids]})) - elsif product_category && asset == :enterprises - @environment.send(asset).find(:all, options.merge(:order => 'profiles.created_at desc, profiles.id desc', :include => 'products', :conditions => ['products.product_category_id in (?)', product_category_ids])) - else - @environment.send(asset).find( :all, options.merge( {:order => 'created_at desc, id desc'} ) ) - end + options = {:order => 'created_at desc, id desc'}.merge(options) + if product_category && asset == :products + @environment.send(asset).paginate(:all, options.merge(:conditions => ['product_category_id in (?)', product_category_ids])) + elsif product_category && asset == :enterprises + @environment.send(asset).paginate(:all, options.merge(:order => 'profiles.created_at desc, profiles.id desc', :include => 'products', :conditions => ['products.product_category_id in (?)', product_category_ids])) + else + @environment.send(asset).paginate(:all, options) + end else + ferret_options = {:page => options.delete(:page), :per_page => options.delete(:per_page)} if product_category && asset == :products # SECURITY no risk of SQL injection, since product_category_ids comes from trusted source - @environment.send(asset).find_by_contents(query, {}, options.merge({:conditions => 'product_category_id in (%s)' % product_category_ids.join(',') })) + @environment.send(asset).find_by_contents(query, ferret_options, options.merge({:conditions => 'product_category_id in (%s)' % product_category_ids.join(',') })) elsif product_category && asset == :enterprises - @environment.send(asset).find_by_contents(query + " +extra_data_for_index:#{product_category.name}", {}, options) + @environment.send(asset).find_by_contents(query + " +extra_data_for_index:#{product_category.name}", ferret_options, options) else - @environment.send(asset).find_by_contents(query, {}, options) + @environment.send(asset).find_by_contents(query, ferret_options, options) end end end @@ -44,9 +47,11 @@ class EnvironmentFinder end def count(asset, query = '', options = {}) + # because will_paginate needs a page + options = {:page => 1}.merge(options) if query.blank? # SLOW - find(asset, query, options).size + find(asset, query, options).total_entries else find(asset, query, options).total_hits end diff --git a/app/views/search/products.rhtml b/app/views/search/products.rhtml index 623c188..6d6fdd9 100644 --- a/app/views/search/products.rhtml +++ b/app/views/search/products.rhtml @@ -29,4 +29,6 @@ +<%= will_paginate @results[:products], :id => 'products-pagination' %> +
diff --git a/config/environment.rb b/config/environment.rb index 7429f04..5c42ff8 100644 --- a/config/environment.rb +++ b/config/environment.rb @@ -103,6 +103,7 @@ require 'acts_as_having_settings' require 'acts_as_having_image' require 'hacked_after_create' require 'sqlite_extension' +require 'will_paginate' # load a local configuration if present, but not under test environment. if ENV['RAILS_ENV'] != 'test' diff --git a/test/functional/search_controller_test.rb b/test/functional/search_controller_test.rb index 337c4aa..9512f07 100644 --- a/test/functional/search_controller_test.rb +++ b/test/functional/search_controller_test.rb @@ -283,6 +283,16 @@ class SearchControllerTest < Test::Unit::TestCase assert_equal [prod1], assigns(:results)[:products] end + should 'paginate enterprise listing' do + @controller.expects(:limit).returns(1) + ent1 = Enterprise.create!(:name => 'teste 1', :identifier => 'teste_1') + ent2 = Enterprise.create!(:name => 'teste 2', :identifier => 'teste_2') + + get :assets, :asset => 'enterprises', :page => '2' + + assert_equal [ent1], assigns(:results)[:enterprises] # older on page 2 + end + should 'display search results' do ent = Enterprise.create!(:name => 'display enterprise', :identifier => 'teste1') product = ent.products.create!(:name => 'display product') diff --git a/test/unit/category_finder_test.rb b/test/unit/category_finder_test.rb index f8f80fc..bb089d4 100644 --- a/test/unit/category_finder_test.rb +++ b/test/unit/category_finder_test.rb @@ -151,6 +151,24 @@ class CategoryFinderTest < ActiveSupport::TestCase assert_includes recent, ent2 assert_not_includes recent, ent1 end + + should 'paginate the list of more enterprises than limit' do + ent1 = Enterprise.create!(:name => 'teste1', :identifier => 'teste1', :category_ids => [@category.id]) + ent2 = Enterprise.create!(:name => 'teste2', :identifier => 'teste2', :category_ids => [@category.id]) + + assert_equal [ent2], @finder.find('enterprises', nil, :per_page => 1, :page => 1) + assert_equal [ent1], @finder.find('enterprises', nil, :per_page => 1, :page => 2) + end + + should 'paginate the list of more enterprises than limit with query' do + ent1 = Enterprise.create!(:name => 'teste 1', :identifier => 'teste1', :category_ids => [@category.id]) + ent2 = Enterprise.create!(:name => 'teste 2', :identifier => 'teste2', :category_ids => [@category.id]) + + p1 = @finder.find('enterprises', 'teste', :per_page => 1, :page => 1) + p2 = @finder.find('enterprises', 'teste', :per_page => 1, :page => 2) + + assert (p1 == [ent1] && p2 == [ent2]) || (p1 == [ent2] && p2 == [ent1]) # consistent paging + end should 'count enterprises' do count = @finder.count('enterprises') diff --git a/test/unit/environment_finder_test.rb b/test/unit/environment_finder_test.rb index 642ae4a..f359ad3 100644 --- a/test/unit/environment_finder_test.rb +++ b/test/unit/environment_finder_test.rb @@ -49,6 +49,27 @@ class EnvironmentFinderTest < ActiveSupport::TestCase recent = finder.recent('enterprises', 1) assert_includes recent, ent2 # newer assert_not_includes recent, ent1 # older + end + + should 'paginate the list of more enterprises than limit' do + finder = EnvironmentFinder.new(Environment.default) + ent1 = Enterprise.create!(:name => 'teste1', :identifier => 'teste1') + ent2 = Enterprise.create!(:name => 'teste2', :identifier => 'teste2') + + assert_equal [ent2], finder.find('enterprises', nil, :per_page => 1, :page => 1) #newer + assert_equal [ent1], finder.find('enterprises', nil, :per_page => 1, :page => 2) #older + end + + should 'paginate the list of more enterprises than limit with query' do + finder = EnvironmentFinder.new(Environment.default) + + ent1 = Enterprise.create!(:name => 'teste 1', :identifier => 'teste1') + ent2 = Enterprise.create!(:name => 'teste 2', :identifier => 'teste2') + + p1 = finder.find('enterprises', 'teste', :per_page => 1, :page => 1) + p2 = finder.find('enterprises', 'teste', :per_page => 1, :page => 2) + + assert (p1 == [ent1] && p2 == [ent2]) || (p1 == [ent2] && p2 == [ent1]) end should 'count enterprises' do @@ -57,6 +78,7 @@ class EnvironmentFinderTest < ActiveSupport::TestCase Enterprise.create!(:name => 'teste1', :identifier => 'teste1') assert_equal count+1, finder.count('enterprises') end + should 'count people' do finder = EnvironmentFinder.new(Environment.default) count = finder.count('people') -- libgit2 0.21.2