From 30f3a3df77a63f09a291c70f26a8a4f534827cd2 Mon Sep 17 00:00:00 2001 From: Aaron Patterson Date: Thu, 1 Apr 2010 21:42:03 -0700 Subject: errors.rb needs to be declared as UTF-8 [#3941 state:resolved] Signed-off-by: wycats --- activemodel/lib/active_model/errors.rb | 2 ++ activemodel/test/cases/validations/i18n_validation_test.rb | 10 ++++++++++ 2 files changed, 12 insertions(+) diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 8d28040c32..64b28f6def 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- + require 'active_support/core_ext/array/wrap' require 'active_support/core_ext/string/inflections' require 'active_support/core_ext/object/blank' diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index 38844bb1fa..7dd82e711d 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -1,3 +1,5 @@ +# -*- coding: utf-8 -*- + require "cases/helper" require 'cases/tests_database' require 'models/person' @@ -41,6 +43,14 @@ class I18nValidationTest < ActiveModel::TestCase @person.errors.add_on_blank :title, 'custom' end + def test_full_message_encoding + I18n.backend.store_translations('en', :errors => { + :messages => { :too_short => '猫舌' }}) + Person.validates_length_of :title, :within => 3..5 + @person.valid? + assert_equal ['Title 猫舌'], @person.errors.full_messages + end + def test_errors_full_messages_translates_human_attribute_name_for_model_attributes @person.errors.add(:name, 'not found') Person.expects(:human_attribute_name).with(:name, :default => 'Name').returns("Person's name") -- cgit v1.2.3 From bc7da9b77d6347eeccefa2c735b2f236a08eea57 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 15:28:09 +0100 Subject: Consistency when using Relation constants --- activerecord/lib/active_record/relation/spawn_methods.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index a17de1bdbb..63246079cb 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -6,7 +6,7 @@ module ActiveRecord merged_relation = clone return merged_relation unless r - (ActiveRecord::Relation::ASSOCIATION_METHODS + ActiveRecord::Relation::MULTI_VALUE_METHODS).reject {|m| [:joins, :where].include?(m)}.each do |method| + (Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS).reject {|m| [:joins, :where].include?(m)}.each do |method| unless (value = r.send(:"#{method}_values")).blank? merged_relation.send(:"#{method}_values=", value) end @@ -26,7 +26,7 @@ module ActiveRecord merged_relation.where_values = merged_wheres - ActiveRecord::Relation::SINGLE_VALUE_METHODS.reject {|m| m == :lock}.each do |method| + Relation::SINGLE_VALUE_METHODS.reject {|m| m == :lock}.each do |method| unless (value = r.send(:"#{method}_value")).nil? merged_relation.send(:"#{method}_value=", value) end -- cgit v1.2.3 From b77dd218ce845f01753d02fcbc2605c9a5ee93e1 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 17:34:48 +0100 Subject: Add Relation extensions --- activerecord/CHANGELOG | 10 +++++++++ activerecord/lib/active_record/named_scope.rb | 7 ++---- activerecord/lib/active_record/relation.rb | 8 +++++-- .../lib/active_record/relation/query_methods.rb | 25 ++++++++++++++++++---- .../lib/active_record/relation/spawn_methods.rb | 11 ++++++---- activerecord/test/cases/relations_test.rb | 16 ++++++++++++++ activerecord/test/models/post.rb | 6 ++++++ 7 files changed, 68 insertions(+), 15 deletions(-) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index e379f4f967..e0625c3dbb 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,5 +1,15 @@ *Rails 3.0.0 [Beta 2] (pending)* +* Add Relation extensions. [Pratik Naik] + + users = User.where(:admin => true).extending(User::AdminPowers) + + latest_users = User.order('created_at DESC') do + def posts_count + Post.count(:user_id => to_a.map(&:id)) + end + end + * To prefix the table names of all models in a module, define self.table_name_prefix on the module. #4032 [Andrew White] * Silenced "SHOW FIELDS" and "SET SQL_AUTO_IS_NULL=0" statements from the MySQL driver to improve log signal to noise ration in development [DHH] diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 9abf979cd0..3456332227 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -133,19 +133,16 @@ module ActiveRecord delegate :scopes, :with_scope, :with_exclusive_scope, :scoped_methods, :scoped, :to => :klass def self.init(klass, options, &block) - relation = new(klass, klass.arel_table) + relation = new(klass, klass.arel_table, &block) scope = if options.is_a?(Hash) - klass.scoped.apply_finder_options(options.except(:extend)) + klass.scoped.apply_finder_options(options) else options ? klass.scoped.merge(options) : klass.scoped end relation = relation.merge(scope) - Array.wrap(options[:extend]).each {|extension| relation.send(:extend, extension) } if options.is_a?(Hash) - relation.send(:extend, Module.new(&block)) if block_given? - relation.current_scoped_methods_when_defined = klass.send(:current_scoped_methods) relation end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 8577ec58f7..dde20532af 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -4,7 +4,7 @@ module ActiveRecord class Relation JoinOperation = Struct.new(:relation, :join_class, :on) ASSOCIATION_METHODS = [:includes, :eager_load, :preload] - MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having] + MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :extends] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from] include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches @@ -13,8 +13,9 @@ module ActiveRecord delegate :insert, :to => :arel attr_reader :table, :klass + attr_accessor :extensions - def initialize(klass, table) + def initialize(klass, table, &block) @klass, @table = klass, table @implicit_readonly = nil @@ -22,6 +23,9 @@ module ActiveRecord SINGLE_VALUE_METHODS.each {|v| instance_variable_set(:"@#{v}_value", nil)} (ASSOCIATION_METHODS + MULTI_VALUE_METHODS).each {|v| instance_variable_set(:"@#{v}_values", [])} + @extensions = [] + + apply_modules(Module.new(&block)) if block_given? end def new(*args, &block) diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index e224781016..b5e8b7570a 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -10,8 +10,9 @@ module ActiveRecord next if [:where, :having].include?(query_method) class_eval <<-CEVAL - def #{query_method}(*args) + def #{query_method}(*args, &block) new_relation = clone + new_relation.send(:apply_modules, Module.new(&block)) if block_given? value = Array.wrap(args.flatten).reject {|x| x.blank? } new_relation.#{query_method}_values += value if value.present? new_relation @@ -21,8 +22,9 @@ module ActiveRecord [:where, :having].each do |query_method| class_eval <<-CEVAL - def #{query_method}(*args) + def #{query_method}(*args, &block) new_relation = clone + new_relation.send(:apply_modules, Module.new(&block)) if block_given? value = build_where(*args) new_relation.#{query_method}_values += [*value] if value.present? new_relation @@ -34,8 +36,9 @@ module ActiveRecord attr_accessor :"#{query_method}_value" class_eval <<-CEVAL - def #{query_method}(value = true) + def #{query_method}(value = true, &block) new_relation = clone + new_relation.send(:apply_modules, Module.new(&block)) if block_given? new_relation.#{query_method}_value = value new_relation end @@ -43,8 +46,16 @@ module ActiveRecord end end - def lock(locks = true) + def extending(*modules) + new_relation = clone + new_relation.send :apply_modules, *modules + new_relation + end + + def lock(locks = true, &block) relation = clone + relation.send(:apply_modules, Module.new(&block)) if block_given? + case locks when String, TrueClass, NilClass clone.tap {|new_relation| new_relation.lock_value = locks || true } @@ -191,6 +202,12 @@ module ActiveRecord private + def apply_modules(modules) + values = Array.wrap(modules) + @extensions += values if values.present? + values.each {|extension| extend(extension) } + end + def reverse_sql_order(order_query) order_query.to_s.split(/,/).each { |s| if s.match(/\s(asc|ASC)$/) diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 63246079cb..8fdd64afcc 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -7,9 +7,8 @@ module ActiveRecord return merged_relation unless r (Relation::ASSOCIATION_METHODS + Relation::MULTI_VALUE_METHODS).reject {|m| [:joins, :where].include?(m)}.each do |method| - unless (value = r.send(:"#{method}_values")).blank? - merged_relation.send(:"#{method}_values=", value) - end + value = r.send(:"#{method}_values") + merged_relation.send(:"#{method}_values=", value) if value.present? end merged_relation = merged_relation.joins(r.joins_values) @@ -34,6 +33,9 @@ module ActiveRecord merged_relation.lock_value = r.lock_value unless merged_relation.lock_value + # Apply scope extension modules + merged_relation.send :apply_modules, r.extensions + merged_relation end @@ -69,7 +71,7 @@ module ActiveRecord result end - VALID_FIND_OPTIONS = [ :conditions, :include, :joins, :limit, :offset, + VALID_FIND_OPTIONS = [ :conditions, :include, :joins, :limit, :offset, :extend, :order, :select, :readonly, :group, :having, :from, :lock ] def apply_finder_options(options) @@ -84,6 +86,7 @@ module ActiveRecord relation = relation.where(options[:conditions]) if options.has_key?(:conditions) relation = relation.includes(options[:include]) if options.has_key?(:include) + relation = relation.extending(options[:extend]) if options.has_key?(:extend) relation end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 1e345399f5..7b9e680c02 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -572,4 +572,20 @@ class RelationTest < ActiveRecord::TestCase assert_equal Post.all, all_posts.all end + def test_anonymous_extension + relation = Post.where(:author_id => 1).order('id ASC') do + def author + 'lifo' + end + end + + assert_equal "lifo", relation.author + assert_equal "lifo", relation.limit(1).author + end + + def test_named_extension + relation = Post.where(:author_id => 1).order('id ASC').extending(Post::NamedExtension) + assert_equal "lifo", relation.author + assert_equal "lifo", relation.limit(1).author + end end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 704313649a..d092c4bf09 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -1,4 +1,10 @@ class Post < ActiveRecord::Base + module NamedExtension + def author + 'lifo' + end + end + scope :containing_the_letter_a, where("body LIKE '%a%'") scope :ranked_by_comments, order("comments_count DESC") scope :limit_by, lambda {|l| limit(l) } -- cgit v1.2.3 From 83ebe6224f546d2154ad4ed72bb3f2c9b07de678 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 17:39:47 +0100 Subject: Oops :extends is not a MULTI_VALUE_METHOD --- activerecord/lib/active_record/relation.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index dde20532af..3ca27f06ea 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -4,7 +4,7 @@ module ActiveRecord class Relation JoinOperation = Struct.new(:relation, :join_class, :on) ASSOCIATION_METHODS = [:includes, :eager_load, :preload] - MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having, :extends] + MULTI_VALUE_METHODS = [:select, :group, :order, :joins, :where, :having] SINGLE_VALUE_METHODS = [:limit, :offset, :lock, :readonly, :create_with, :from] include FinderMethods, Calculations, SpawnMethods, QueryMethods, Batches -- cgit v1.2.3 From a0cdb0499eecd40ba18d33226767783e4a058847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 2 Apr 2010 19:13:47 +0200 Subject: Maintain the usage of :as consistent in the router. Whenever it's supplied, it changes the NAMED ROUTE. If you want to change the PATH, use :path instead. Example: resources :projects, :path => 'projetos' --- actionpack/lib/action_dispatch/routing/mapper.rb | 53 +++++++++++++----------- actionpack/test/dispatch/routing_test.rb | 51 +++++++++++++++++++---- 2 files changed, 71 insertions(+), 33 deletions(-) diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8a84afd315..4e9112bc04 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -194,6 +194,21 @@ module ActionDispatch self end + def mount(app, options = nil) + if options + path = options.delete(:at) + else + options = app + app, path = options.find { |k, v| k.respond_to?(:call) } + options.delete(app) if app + end + + raise "A rack application must be specified" unless path + + match(path, options.merge(:to => app, :anchor => false)) + self + end + def default_url_options=(options) @set.default_url_options = options end @@ -380,14 +395,13 @@ module ActionDispatch [:index, :create, :new, :show, :update, :destroy, :edit] end - attr_reader :plural, :singular, :options + attr_reader :controller, :path, :options def initialize(entities, options = {}) - @name = entities.to_s - @options = options - - @plural = @name.pluralize - @singular = @name.singularize + @name = entities.to_s + @path = options.delete(:path) || @name + @controller = options.delete(:controller) || @name.to_s.pluralize + @options = options end def default_actions @@ -417,8 +431,12 @@ module ActionDispatch options[:as] || @name end - def controller - options[:controller] || plural + def plural + name.to_s.pluralize + end + + def singular + name.to_s.singularize end def member_name @@ -509,7 +527,7 @@ module ActionDispatch resource = SingletonResource.new(resources.pop, options) - scope(:path => resource.name.to_s, :controller => resource.controller) do + scope(:path => resource.path, :controller => resource.controller) do with_scope_level(:resource, resource) do scope(:name_prefix => resource.name.to_s, :as => "") do @@ -539,7 +557,7 @@ module ActionDispatch resource = Resource.new(resources.pop, options) - scope(:path => resource.name.to_s, :controller => resource.controller) do + scope(:path => resource.path, :controller => resource.controller) do with_scope_level(:resources, resource) do yield if block_given? @@ -603,21 +621,6 @@ module ActionDispatch end end - def mount(app, options = nil) - if options - path = options.delete(:at) - else - options = app - app, path = options.find { |k, v| k.respond_to?(:call) } - options.delete(app) if app - end - - raise "A rack application must be specified" unless path - - match(path, options.merge(:to => app, :anchor => false)) - self - end - def match(*args) options = args.extract_options! diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index e58653cb8c..19538cb88b 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -58,8 +58,9 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest get 'admin/accounts' => "queenbee#accounts" end - scope 'es' do - resources :projects, :path_names => { :edit => 'cambiar' }, :as => 'projeto' + scope 'pt', :name_prefix => 'pt' do + resources :projects, :path_names => { :edit => 'editar' }, :path => 'projetos' + resource :admin, :path_names => { :new => 'novo' }, :path => 'administrador' end resources :projects, :controller => :project do @@ -74,10 +75,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest resource :avatar, :controller => :avatar end - resources :images do + resources :images, :as => :funny_images do post :revise, :on => :member end + resource :manager, :as => :super_manager do + post :fire + end + resources :people do nested do scope "/:access_token" do @@ -144,7 +149,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end namespace :forum do - resources :products, :as => '' do + resources :products, :path => '' do resources :questions end end @@ -430,15 +435,35 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end + def test_project_manager + with_test_routes do + get '/projects/1/manager' + assert_equal 'managers#show', @response.body + assert_equal '/projects/1/manager', project_super_manager_path(:project_id => '1') + + get '/projects/1/manager/new' + assert_equal 'managers#new', @response.body + assert_equal '/projects/1/manager/new', new_project_super_manager_path(:project_id => '1') + + post '/projects/1/manager/fire' + assert_equal 'managers#fire', @response.body + assert_equal '/projects/1/manager/fire', fire_project_super_manager_path(:project_id => '1') + end + end + def test_project_images with_test_routes do get '/projects/1/images' assert_equal 'images#index', @response.body - assert_equal '/projects/1/images', project_images_path(:project_id => '1') + assert_equal '/projects/1/images', project_funny_images_path(:project_id => '1') + + get '/projects/1/images/new' + assert_equal 'images#new', @response.body + assert_equal '/projects/1/images/new', new_project_funny_image_path(:project_id => '1') post '/projects/1/images/1/revise' assert_equal 'images#revise', @response.body - assert_equal '/projects/1/images/1/revise', revise_project_image_path(:project_id => '1', :id => '1') + assert_equal '/projects/1/images/1/revise', revise_project_funny_image_path(:project_id => '1', :id => '1') end end @@ -552,11 +577,21 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_path_names with_test_routes do - get '/es/projeto' + get '/pt/projetos' assert_equal 'projects#index', @response.body + assert_equal '/pt/projetos', pt_projects_path - get '/es/projeto/1/cambiar' + get '/pt/projetos/1/editar' assert_equal 'projects#edit', @response.body + assert_equal '/pt/projetos/1/editar', edit_pt_project_path(1) + + get '/pt/administrador' + assert_equal 'admins#show', @response.body + assert_equal '/pt/administrador', pt_admin_path + + get '/pt/administrador/novo' + assert_equal 'admins#new', @response.body + assert_equal '/pt/administrador/novo', new_pt_admin_path end end -- cgit v1.2.3 From 0be31f85639cf2f536c558819ef3ee45ba7d83a3 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 17:48:06 +0100 Subject: Scope#current_scoped_methods_when_defined is no longer needed --- activerecord/lib/active_record/named_scope.rb | 15 ++------------- 1 file changed, 2 insertions(+), 13 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 3456332227..4203e36239 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -128,8 +128,6 @@ module ActiveRecord end class Scope < Relation - attr_accessor :current_scoped_methods_when_defined - delegate :scopes, :with_scope, :with_exclusive_scope, :scoped_methods, :scoped, :to => :klass def self.init(klass, options, &block) @@ -141,10 +139,7 @@ module ActiveRecord options ? klass.scoped.merge(options) : klass.scoped end - relation = relation.merge(scope) - - relation.current_scoped_methods_when_defined = klass.send(:current_scoped_methods) - relation + relation.merge(scope) end def first(*args) @@ -178,13 +173,7 @@ module ActiveRecord def method_missing(method, *args, &block) if klass.respond_to?(method) - with_scope(self) do - if current_scoped_methods_when_defined && !scoped_methods.include?(current_scoped_methods_when_defined) && !scopes.include?(method) - with_scope(current_scoped_methods_when_defined) { klass.send(method, *args, &block) } - else - klass.send(method, *args, &block) - end - end + with_scope(self) { klass.send(method, *args, &block) } else super end -- cgit v1.2.3 From ee07950c03bf8aab703191d73165eb206f9f55ef Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 17:48:57 +0100 Subject: Scope#method_missing can safely rely on Relation#method_missing --- activerecord/lib/active_record/named_scope.rb | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 4203e36239..3efcff11ca 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -169,16 +169,6 @@ module ActiveRecord end end - private - - def method_missing(method, *args, &block) - if klass.respond_to?(method) - with_scope(self) { klass.send(method, *args, &block) } - else - super - end - end - end end -- cgit v1.2.3 From 62fe16932c9b7c3044017900114193e06814fd0c Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 17:58:13 +0100 Subject: Make Relation#first and Relation#last behave like named scope's --- activerecord/lib/active_record/named_scope.rb | 16 ---------------- .../lib/active_record/relation/finder_methods.rb | 20 ++++++++++++++++++-- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 3efcff11ca..be26b1f47f 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -142,22 +142,6 @@ module ActiveRecord relation.merge(scope) end - def first(*args) - if args.first.kind_of?(Integer) || (loaded? && !args.first.kind_of?(Hash)) - to_a.first(*args) - else - args.first.present? ? apply_finder_options(args.first).first : super - end - end - - def last(*args) - if args.first.kind_of?(Integer) || (loaded? && !args.first.kind_of?(Hash)) - to_a.last(*args) - else - args.first.present? ? apply_finder_options(args.first).last : super - end - end - def ==(other) case other when Scope diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 37aaac0894..a26f1c0ac8 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -106,13 +106,29 @@ module ActiveRecord # A convenience wrapper for find(:first, *args). You can pass in all the # same arguments to this method as you can to find(:first). def first(*args) - args.any? ? apply_finder_options(args.first).first : find_first + if args.any? + if args.first.kind_of?(Integer) || (loaded? && !args.first.kind_of?(Hash)) + to_a.first(*args) + else + apply_finder_options(args.first).first + end + else + find_first + end end # A convenience wrapper for find(:last, *args). You can pass in all the # same arguments to this method as you can to find(:last). def last(*args) - args.any? ? apply_finder_options(args.first).last : find_last + if args.any? + if args.first.kind_of?(Integer) || (loaded? && !args.first.kind_of?(Hash)) + to_a.last(*args) + else + apply_finder_options(args.first).last + end + else + find_last + end end # A convenience wrapper for find(:all, *args). You can pass in all the -- cgit v1.2.3 From cfa283201e079b4f700eb915490bcfa18451b11e Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 18:57:01 +0100 Subject: Goodbye ActiveRecord::NamedScope::Scope --- activerecord/lib/active_record/named_scope.rb | 52 +++++++--------------- activerecord/lib/active_record/relation.rb | 11 +++++ .../active_record/relation/predicate_builder.rb | 2 +- 3 files changed, 29 insertions(+), 36 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index be26b1f47f..d56f7afb74 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -25,7 +25,8 @@ module ActiveRecord # You can define a scope that applies to all finders using ActiveRecord::Base.default_scope. def scoped(options = {}, &block) if options.present? - Scope.init(self, options, &block) + relation = scoped.apply_finder_options(options) + block_given? ? relation.extending(Module.new(&block)) : relation else current_scoped_methods ? unscoped.merge(current_scoped_methods) : unscoped.clone end @@ -107,13 +108,22 @@ module ActiveRecord end scopes[name] = lambda do |parent_scope, *args| - Scope.init(parent_scope, case options - when Hash, Relation - options - when Proc - options.call(*args) - end, &block) + scope_options = case options + when Hash, Relation + options + when Proc + options.call(*args) + end + + relation = if scope_options.is_a?(Hash) + parent_scope.scoped.apply_finder_options(scope_options) + else + scope_options ? parent_scope.scoped.merge(scope_options) : parent_scope.scoped + end + + block_given? ? relation.extending(Module.new(&block)) : relation end + singleton_class.instance_eval do define_method name do |*args| scopes[name].call(self, *args) @@ -127,33 +137,5 @@ module ActiveRecord end end - class Scope < Relation - delegate :scopes, :with_scope, :with_exclusive_scope, :scoped_methods, :scoped, :to => :klass - - def self.init(klass, options, &block) - relation = new(klass, klass.arel_table, &block) - - scope = if options.is_a?(Hash) - klass.scoped.apply_finder_options(options) - else - options ? klass.scoped.merge(options) : klass.scoped - end - - relation.merge(scope) - end - - def ==(other) - case other - when Scope - to_sql == other.to_sql - when Relation - other == self - when Array - to_a == other.to_a - end - end - - end - end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3ca27f06ea..69d04d7375 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -311,11 +311,22 @@ module ActiveRecord @should_eager_load ||= (@eager_load_values.any? || (@includes_values.any? && references_eager_loaded_tables?)) end + def ==(other) + case other + when Relation + other.to_sql == to_sql + when Array + to_a == other.to_a + end + end + protected def method_missing(method, *args, &block) if Array.method_defined?(method) to_a.send(method, *args, &block) + elsif @klass.scopes[method] + merge(@klass.send(method, *args, &block)) elsif @klass.respond_to?(method) @klass.send(:with_scope, self) { @klass.send(method, *args, &block) } elsif arel.respond_to?(method) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 711df16bf1..717756418c 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -23,7 +23,7 @@ module ActiveRecord attribute = table[column] case value - when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::NamedScope::Scope + when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation values = value.to_a attribute.in(values) when Range -- cgit v1.2.3 From 13eb2c87e6b8296ca2ce2da36eb746e2b94a563b Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Fri, 2 Apr 2010 18:57:20 +0100 Subject: Make Relation#inspect less noisy --- activerecord/lib/active_record/relation.rb | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 69d04d7375..4e62187449 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -320,6 +320,10 @@ module ActiveRecord end end + def inspect + to_a.inspect + end + protected def method_missing(method, *args, &block) -- cgit v1.2.3 From d898a4ba425a201827f07a5bb11c8c6bf85159b8 Mon Sep 17 00:00:00 2001 From: Rolf Bjaanes Date: Fri, 2 Apr 2010 20:00:29 +0200 Subject: Raise exceptions instead of rendering error templates in test environment [#4315 state:resolved] MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: José Valim --- actionpack/lib/action_dispatch/railtie.rb | 3 ++- railties/lib/rails/application/configuration.rb | 2 +- .../generators/rails/app/templates/config/environments/test.rb.tt | 3 +++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 563df0f256..7ea9182e9c 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -6,7 +6,8 @@ module ActionDispatch config.action_dispatch = ActiveSupport::OrderedOptions.new config.action_dispatch.x_sendfile_header = "" config.action_dispatch.ip_spoofing_check = true - + config.action_dispatch.show_exceptions = true + # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| # TODO: This used to say unless defined?(Dispatcher). Find out why and fix. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index 44635ff4f6..5c7de616be 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -132,7 +132,7 @@ module Rails middleware.use('::Rack::Lock', :if => lambda { !allow_concurrency }) middleware.use('::Rack::Runtime') middleware.use('::Rails::Rack::Logger') - middleware.use('::ActionDispatch::ShowExceptions', lambda { consider_all_requests_local }) + middleware.use('::ActionDispatch::ShowExceptions', lambda { consider_all_requests_local }, :if => lambda { action_dispatch.show_exceptions }) middleware.use("::ActionDispatch::RemoteIp", lambda { action_dispatch.ip_spoofing_check }, lambda { action_dispatch.trusted_proxies }) middleware.use('::Rack::Sendfile', lambda { action_dispatch.x_sendfile_header }) middleware.use('::ActionDispatch::Callbacks', lambda { !cache_classes }) diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 0b87b241ec..beb28e2229 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -14,6 +14,9 @@ config.consider_all_requests_local = true config.action_controller.perform_caching = false + # Raise exceptions instead of rendering exception templates + config.action_dispatch.show_exceptions = false + # Disable request forgery protection in test environment config.action_controller.allow_forgery_protection = false -- cgit v1.2.3 From 997e22c2751c66f8bba31dcdf4d1054072156036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Fri, 2 Apr 2010 20:54:10 +0200 Subject: Add a test which ensures action_dispatch.show_exceptions is properly disabled. --- actionpack/lib/action_dispatch/railtie.rb | 2 +- actionpack/lib/action_dispatch/testing/integration.rb | 4 +--- railties/test/application/middleware_test.rb | 6 ++++++ 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 7ea9182e9c..004c254e55 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -7,7 +7,7 @@ module ActionDispatch config.action_dispatch.x_sendfile_header = "" config.action_dispatch.ip_spoofing_check = true config.action_dispatch.show_exceptions = true - + # Prepare dispatcher callbacks and run 'prepare' callbacks initializer "action_dispatch.prepare_dispatcher" do |app| # TODO: This used to say unless defined?(Dispatcher). Find out why and fix. diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 621d63c5e2..031fa1dfb4 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -263,9 +263,7 @@ module ActionDispatch "HTTP_HOST" => host, "REMOTE_ADDR" => remote_addr, "CONTENT_TYPE" => "application/x-www-form-urlencoded", - "HTTP_ACCEPT" => accept, - - "action_dispatch.show_exceptions" => false + "HTTP_ACCEPT" => accept } (rack_environment || {}).each do |key, value| diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 9a359d20b1..7f72881d55 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -52,6 +52,12 @@ module ApplicationTests assert !middleware.include?("ActionDispatch::Static") end + test "removes show exceptions if action_dispatch.show_exceptions is disabled" do + add_to_config "config.action_dispatch.show_exceptions = false" + boot! + assert !middleware.include?("ActionDispatch::ShowExceptions") + end + test "use middleware" do use_frameworks [] add_to_config "config.middleware.use Rack::Config" -- cgit v1.2.3 From 90e3343ae5f9daa1458fc37e220d89cdb41b3185 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 2 Apr 2010 16:43:15 -0300 Subject: utc_offset is no longer required on TimeZone and if it's not supplied we delegate to TZInfo --- .../lib/active_support/values/time_zone.rb | 79 +++++----------------- 1 file changed, 18 insertions(+), 61 deletions(-) diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 945cdd5278..b9766a236a 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -194,7 +194,7 @@ module ActiveSupport # offset is the number of seconds that this time zone is offset from UTC # (GMT). Seconds were chosen as the offset unit because that is the unit that # Ruby uses to represent time zone offsets (see Time#utc_offset). - def initialize(name, utc_offset, tzinfo = nil) + def initialize(name, utc_offset = nil, tzinfo = nil) @name = name @utc_offset = utc_offset @tzinfo = tzinfo @@ -202,8 +202,12 @@ module ActiveSupport end def utc_offset - @current_period ||= tzinfo.current_period - @current_period.utc_offset + if @utc_offset + @utc_offset + else + @current_period ||= tzinfo.current_period + @current_period.utc_offset + end end # Returns the offset of this time zone as a formatted string, of the @@ -305,71 +309,24 @@ module ActiveSupport tzinfo.period_for_local(time, dst) end - # TODO: Preload instead of lazy load for thread safety def tzinfo + @tzinfo ||= find_tzinfo + end + + # TODO: Preload instead of lazy load for thread safety + def find_tzinfo require 'tzinfo' unless defined?(::TZInfo) - @tzinfo ||= ::TZInfo::Timezone.get(MAPPING[name]) + ::TZInfo::Timezone.get(MAPPING[name]) end unless const_defined?(:ZONES) ZONES = [] ZONES_MAP = {} - [[-39_600, "International Date Line West", "Midway Island", "Samoa" ], - [-36_000, "Hawaii" ], - [-32_400, "Alaska" ], - [-28_800, "Pacific Time (US & Canada)", "Tijuana" ], - [-25_200, "Mountain Time (US & Canada)", "Chihuahua", "Mazatlan", - "Arizona" ], - [-21_600, "Central Time (US & Canada)", "Saskatchewan", "Guadalajara", - "Mexico City", "Monterrey", "Central America" ], - [-18_000, "Eastern Time (US & Canada)", "Indiana (East)", "Bogota", - "Lima", "Quito" ], - [-16_200, "Caracas" ], - [-14_400, "Atlantic Time (Canada)", "Georgetown", "La Paz", "Santiago" ], - [-12_600, "Newfoundland" ], - [-10_800, "Brasilia", "Buenos Aires", "Greenland" ], - [ -7_200, "Mid-Atlantic" ], - [ -3_600, "Azores", "Cape Verde Is." ], - [ 0, "Dublin", "Edinburgh", "Lisbon", "London", "Casablanca", - "Monrovia", "UTC" ], - [ 3_600, "Belgrade", "Bratislava", "Budapest", "Ljubljana", "Prague", - "Sarajevo", "Skopje", "Warsaw", "Zagreb", "Brussels", - "Copenhagen", "Madrid", "Paris", "Amsterdam", "Berlin", - "Bern", "Rome", "Stockholm", "Vienna", - "West Central Africa" ], - [ 7_200, "Bucharest", "Cairo", "Helsinki", "Kyiv", "Riga", "Sofia", - "Tallinn", "Vilnius", "Athens", "Istanbul", "Minsk", - "Jerusalem", "Harare", "Pretoria" ], - [ 10_800, "Moscow", "St. Petersburg", "Volgograd", "Kuwait", "Riyadh", - "Nairobi", "Baghdad" ], - [ 12_600, "Tehran" ], - [ 14_400, "Abu Dhabi", "Muscat", "Baku", "Tbilisi", "Yerevan" ], - [ 16_200, "Kabul" ], - [ 18_000, "Ekaterinburg", "Islamabad", "Karachi", "Tashkent" ], - [ 19_800, "Chennai", "Kolkata", "Mumbai", "New Delhi", "Sri Jayawardenepura" ], - [ 20_700, "Kathmandu" ], - [ 21_600, "Astana", "Dhaka", "Almaty", - "Novosibirsk" ], - [ 23_400, "Rangoon" ], - [ 25_200, "Bangkok", "Hanoi", "Jakarta", "Krasnoyarsk" ], - [ 28_800, "Beijing", "Chongqing", "Hong Kong", "Urumqi", - "Kuala Lumpur", "Singapore", "Taipei", "Perth", "Irkutsk", - "Ulaan Bataar" ], - [ 32_400, "Seoul", "Osaka", "Sapporo", "Tokyo", "Yakutsk" ], - [ 34_200, "Darwin", "Adelaide" ], - [ 36_000, "Canberra", "Melbourne", "Sydney", "Brisbane", "Hobart", - "Vladivostok", "Guam", "Port Moresby" ], - [ 39_600, "Magadan", "Solomon Is.", "New Caledonia" ], - [ 43_200, "Fiji", "Kamchatka", "Marshall Is.", "Auckland", - "Wellington" ], - [ 46_800, "Nuku'alofa" ]]. - each do |offset, *places| - places.each do |place| - place.freeze - zone = new(place, offset) - ZONES << zone - ZONES_MAP[place] = zone - end + MAPPING.each_key do |place| + place.freeze + zone = new(place) + ZONES << zone + ZONES_MAP[place] = zone end ZONES.sort! ZONES.freeze -- cgit v1.2.3 From 1b742ea9b100278dd818879a4162dab1a2c0e6d4 Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 2 Apr 2010 16:44:38 -0300 Subject: delegate unknown timezones to TZInfo --- activesupport/lib/active_support/values/time_zone.rb | 17 ++++++++++++----- activesupport/test/time_zone_test.rb | 8 ++++++++ 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index b9766a236a..03b324764b 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -310,13 +310,15 @@ module ActiveSupport end def tzinfo - @tzinfo ||= find_tzinfo + @tzinfo ||= TimeZone.find_tzinfo(name) end # TODO: Preload instead of lazy load for thread safety - def find_tzinfo + def self.find_tzinfo(name) require 'tzinfo' unless defined?(::TZInfo) - ::TZInfo::Timezone.get(MAPPING[name]) + ::TZInfo::Timezone.get(MAPPING[name] || name) + rescue TZInfo::InvalidTimezoneIdentifier + nil end unless const_defined?(:ZONES) @@ -330,7 +332,6 @@ module ActiveSupport end ZONES.sort! ZONES.freeze - ZONES_MAP.freeze US_ZONES = ZONES.find_all { |z| z.name =~ /US|Arizona|Indiana|Hawaii|Alaska/ } US_ZONES.freeze @@ -361,7 +362,7 @@ module ActiveSupport def [](arg) case arg when String - ZONES_MAP[arg] + ZONES_MAP[arg] ||= lookup(arg) when Numeric, ActiveSupport::Duration arg *= 3600 if arg.abs <= 13 all.find { |z| z.utc_offset == arg.to_i } @@ -375,6 +376,12 @@ module ActiveSupport def us_zones US_ZONES end + + private + + def lookup(name) + (tzinfo = find_tzinfo(name)) && create(tzinfo.name.freeze) + end end end end diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index ce43c6507d..68027f7c94 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -75,6 +75,14 @@ class TimeZoneTest < Test::Unit::TestCase end end + def test_unknown_timezones_delegation_to_tzinfo + zone = ActiveSupport::TimeZone['America/Montevideo'] + assert_equal ActiveSupport::TimeZone, zone.class + assert_equal zone.object_id, ActiveSupport::TimeZone['America/Montevideo'].object_id + assert_equal Time.utc(2010, 1, 31, 22), zone.utc_to_local(Time.utc(2010, 2)) # daylight saving offset -0200 + assert_equal Time.utc(2010, 3, 31, 21), zone.utc_to_local(Time.utc(2010, 4)) # standard offset -0300 + end + def test_today Time.stubs(:now).returns(Time.utc(2000, 1, 1, 4, 59, 59)) # 1 sec before midnight Jan 1 EST assert_equal Date.new(1999, 12, 31), ActiveSupport::TimeZone['Eastern Time (US & Canada)'].today -- cgit v1.2.3 From a1a352019847653c0001ee6b5efeda010c727f86 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 2 Apr 2010 14:44:56 -0700 Subject: CI: omit ruby-debug19 from Gemfile since the ruby source dep hoses permissions --- Gemfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Gemfile b/Gemfile index 348a1daaa7..d516abe111 100644 --- a/Gemfile +++ b/Gemfile @@ -11,7 +11,7 @@ group :mri do if RUBY_VERSION < '1.9' gem "system_timer" gem "ruby-debug", ">= 0.10.3" - elsif RUBY_VERSION < '1.9.2' + elsif RUBY_VERSION < '1.9.2' && !ENV['CI'] gem "ruby-debug19" end end -- cgit v1.2.3 From ee7605ecf0d36d16cddd885bb9df6ebddda8ff39 Mon Sep 17 00:00:00 2001 From: Jeremy Kemper Date: Fri, 2 Apr 2010 15:54:38 -0700 Subject: Key partial name cache on controller and object class *names* to avoid memory leaks in dev mode --- actionpack/lib/action_view/render/partials.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/actionpack/lib/action_view/render/partials.rb b/actionpack/lib/action_view/render/partials.rb index f04a89c1ac..4d23d55687 100644 --- a/actionpack/lib/action_view/render/partials.rb +++ b/actionpack/lib/action_view/render/partials.rb @@ -179,7 +179,7 @@ module ActionView def initialize(view_context, options, block) @view = view_context - @partial_names = PARTIAL_NAMES[@view.controller.class] + @partial_names = PARTIAL_NAMES[@view.controller.class.name] setup(options, block) end @@ -300,7 +300,7 @@ module ActionView end def partial_path(object = @object) - @partial_names[object.class] ||= begin + @partial_names[object.class.name] ||= begin object = object.to_model if object.respond_to?(:to_model) object.class.model_name.partial_path.dup.tap do |partial| -- cgit v1.2.3 From 47c99f901257937c47a47953745624dd8469ec4b Mon Sep 17 00:00:00 2001 From: Santiago Pastorino Date: Fri, 2 Apr 2010 11:50:30 -0300 Subject: Fixing index style [#4313 state:committed] Signed-off-by: wycats --- railties/lib/rails/generators/rails/app/templates/public/index.html | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/railties/lib/rails/generators/rails/app/templates/public/index.html b/railties/lib/rails/generators/rails/app/templates/public/index.html index 836da1b689..9fb304a66b 100644 --- a/railties/lib/rails/generators/rails/app/templates/public/index.html +++ b/railties/lib/rails/generators/rails/app/templates/public/index.html @@ -82,7 +82,8 @@ #about-content { background-color: #ffd; border: 1px solid #fc0; - margin-left: -11px; + margin-left: -55px; + margin-right: -10px; } #about-content table { margin-top: 10px; -- cgit v1.2.3 From 684e4d39d60d87273ea74f9a076b3ea308fc2ffe Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 3 Apr 2010 01:22:42 +0100 Subject: Remove unnecessary argument for creating scopes --- activerecord/lib/active_record/named_scope.rb | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index d56f7afb74..86a18f8b6b 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -107,7 +107,7 @@ module ActiveRecord "Overwriting existing method #{self.name}.#{name}." end - scopes[name] = lambda do |parent_scope, *args| + scopes[name] = lambda do |*args| scope_options = case options when Hash, Relation options @@ -116,9 +116,9 @@ module ActiveRecord end relation = if scope_options.is_a?(Hash) - parent_scope.scoped.apply_finder_options(scope_options) + scoped.apply_finder_options(scope_options) else - scope_options ? parent_scope.scoped.merge(scope_options) : parent_scope.scoped + scope_options ? scoped.merge(scope_options) : scoped end block_given? ? relation.extending(Module.new(&block)) : relation @@ -126,7 +126,7 @@ module ActiveRecord singleton_class.instance_eval do define_method name do |*args| - scopes[name].call(self, *args) + scopes[name].call(*args) end end end -- cgit v1.2.3 From 41a2ba652a6b4b4eb3d12b9a2d55cb5b1eb5581a Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 3 Apr 2010 01:35:10 +0100 Subject: Improve named scope lambda --- activerecord/lib/active_record/named_scope.rb | 17 ++++------------- 1 file changed, 4 insertions(+), 13 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 86a18f8b6b..50c57783b2 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -99,7 +99,7 @@ module ActiveRecord # # expected_options = { :conditions => { :colored => 'red' } } # assert_equal expected_options, Shirt.colored('red').proxy_options - def scope(name, options = {}, &block) + def scope(name, scope_options = {}, &block) name = name.to_sym if !scopes[name] && respond_to?(name, true) @@ -108,19 +108,10 @@ module ActiveRecord end scopes[name] = lambda do |*args| - scope_options = case options - when Hash, Relation - options - when Proc - options.call(*args) - end - - relation = if scope_options.is_a?(Hash) - scoped.apply_finder_options(scope_options) - else - scope_options ? scoped.merge(scope_options) : scoped - end + options = scope_options.is_a?(Proc) ? scope_options.call(*args) : scope_options + relation = scoped + relation = options.is_a?(Hash) ? relation.apply_finder_options(options) : scoped.merge(options) if options block_given? ? relation.extending(Module.new(&block)) : relation end -- cgit v1.2.3 From c6372d604952a8eef16ce73a06814aa143b94779 Mon Sep 17 00:00:00 2001 From: Pratik Naik Date: Sat, 3 Apr 2010 01:49:01 +0100 Subject: Improve scope docs --- activerecord/lib/active_record/named_scope.rb | 44 +++++++++------------------ 1 file changed, 14 insertions(+), 30 deletions(-) diff --git a/activerecord/lib/active_record/named_scope.rb b/activerecord/lib/active_record/named_scope.rb index 50c57783b2..632322b517 100644 --- a/activerecord/lib/active_record/named_scope.rb +++ b/activerecord/lib/active_record/named_scope.rb @@ -8,16 +8,15 @@ module ActiveRecord extend ActiveSupport::Concern module ClassMethods - # Returns a relation if invoked without any arguments. + # Returns an anonymous scope. # # posts = Post.scoped # posts.size # Fires "select count(*) from posts" and returns the count # posts.each {|p| puts p.name } # Fires "select * from posts" and loads post objects # - # Returns an anonymous named scope if any options are supplied. - # - # shirts = Shirt.scoped(:conditions => {:color => 'red'}) - # shirts = shirts.scoped(:include => :washing_instructions) + # fruits = Fruit.scoped + # fruits = fruits.where(:colour => 'red') if options[:red_only] + # fruits = fruits.limit(10) if limited? # # Anonymous \scopes tend to be useful when procedurally generating complex queries, where passing # intermediate values (scopes) around as first-class objects is convenient. @@ -37,21 +36,21 @@ module ActiveRecord end # Adds a class method for retrieving and querying objects. A scope represents a narrowing of a database query, - # such as :conditions => {:color => :red}, :select => 'shirts.*', :include => :washing_instructions. + # such as where(:color => :red).select('shirts.*').includes(:washing_instructions). # # class Shirt < ActiveRecord::Base - # scope :red, :conditions => {:color => 'red'} - # scope :dry_clean_only, :joins => :washing_instructions, :conditions => ['washing_instructions.dry_clean_only = ?', true] + # scope :red, where(:color => 'red') + # scope :dry_clean_only, joins(:washing_instructions).where('washing_instructions.dry_clean_only = ?', true) # end # # The above calls to scope define class methods Shirt.red and Shirt.dry_clean_only. Shirt.red, - # in effect, represents the query Shirt.find(:all, :conditions => {:color => 'red'}). + # in effect, represents the query Shirt.where(:color => 'red'). # # Unlike Shirt.find(...), however, the object returned by Shirt.red is not an Array; it resembles the association object - # constructed by a has_many declaration. For instance, you can invoke Shirt.red.find(:first), Shirt.red.count, - # Shirt.red.find(:all, :conditions => {:size => 'small'}). Also, just - # as with the association objects, named \scopes act like an Array, implementing Enumerable; Shirt.red.each(&block), - # Shirt.red.first, and Shirt.red.inject(memo, &block) all behave as if Shirt.red really was an Array. + # constructed by a has_many declaration. For instance, you can invoke Shirt.red.first, Shirt.red.count, + # Shirt.red.where(:size => 'small'). Also, just as with the association objects, named \scopes act like an Array, + # implementing Enumerable; Shirt.red.each(&block), Shirt.red.first, and Shirt.red.inject(memo, &block) + # all behave as if Shirt.red really was an Array. # # These named \scopes are composable. For instance, Shirt.red.dry_clean_only will produce all shirts that are both red and dry clean only. # Nested finds and calculations also work with these compositions: Shirt.red.dry_clean_only.count returns the number of garments @@ -70,9 +69,7 @@ module ActiveRecord # Named \scopes can also be procedural: # # class Shirt < ActiveRecord::Base - # scope :colored, lambda { |color| - # { :conditions => { :color => color } } - # } + # scope :colored, lambda {|color| where(:color => color) } # end # # In this example, Shirt.colored('puce') finds all puce shirts. @@ -80,25 +77,12 @@ module ActiveRecord # Named \scopes can also have extensions, just as with has_many declarations: # # class Shirt < ActiveRecord::Base - # scope :red, :conditions => {:color => 'red'} do + # scope :red, where(:color => 'red') do # def dom_id # 'red_shirts' # end # end # end - # - # - # For testing complex named \scopes, you can examine the scoping options using the - # proxy_options method on the proxy itself. - # - # class Shirt < ActiveRecord::Base - # scope :colored, lambda { |color| - # { :conditions => { :color => color } } - # } - # end - # - # expected_options = { :conditions => { :colored => 'red' } } - # assert_equal expected_options, Shirt.colored('red').proxy_options def scope(name, scope_options = {}, &block) name = name.to_sym -- cgit v1.2.3 From b29e89368841869c92d706addddd09f436a3ea2f Mon Sep 17 00:00:00 2001 From: wycats Date: Fri, 2 Apr 2010 19:21:19 -0700 Subject: Fix memory leak in dev mode --- actionpack/lib/action_view/template.rb | 9 +++++++++ actionpack/test/template/render_test.rb | 8 ++++++++ 2 files changed, 17 insertions(+) diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 8abc1633ff..3df2bd8eed 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -18,6 +18,14 @@ module ActionView attr_reader :source, :identifier, :handler, :virtual_path, :formats + def self.finalizer_for(method_name) + proc do + ActionView::CompiledTemplates.module_eval do + remove_possible_method method_name + end + end + end + def initialize(source, identifier, handler, details) @source = source @identifier = identifier @@ -98,6 +106,7 @@ module ActionView begin ActionView::CompiledTemplates.module_eval(source, identifier, line) + ObjectSpace.define_finalizer(self, self.class.finalizer_for(method_name)) method_name rescue Exception => e # errors from template code if logger = (view && view.logger) diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb index e54ebfbf8d..5f33c933db 100644 --- a/actionpack/test/template/render_test.rb +++ b/actionpack/test/template/render_test.rb @@ -281,6 +281,10 @@ class CachedViewRenderTest < ActiveSupport::TestCase assert_equal ActionView::FileSystemResolver, view_paths.first.class setup_view(view_paths) end + + def teardown + GC.start + end end class LazyViewRenderTest < ActiveSupport::TestCase @@ -294,4 +298,8 @@ class LazyViewRenderTest < ActiveSupport::TestCase assert_equal ActionView::FileSystemResolver.new(FIXTURE_LOAD_PATH), view_paths.first setup_view(view_paths) end + + def teardown + GC.start + end end -- cgit v1.2.3 From 386b7bfd9d78a6d8c8bc7cc4a310df806ad0ba57 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 2 Apr 2010 20:12:50 -0700 Subject: Remove an unused argument --- activerecord/lib/active_record/associations.rb | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7406daf837..40022a614a 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1493,14 +1493,13 @@ module ActiveRecord # The +extra_conditions+ parameter, which is not used within the main # Active Record codebase, is meant to allow plugins to define extra # finder conditions. - def configure_dependency_for_has_many(reflection, extra_conditions = nil) + def configure_dependency_for_has_many(reflection) if reflection.options.include?(:dependent) # Add polymorphic type if the :as option is present dependent_conditions = [] dependent_conditions << "#{reflection.primary_key_name} = \#{record.#{reflection.name}.send(:owner_quoted_id)}" dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as] dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.table_name) if reflection.options[:conditions] - dependent_conditions << extra_conditions if extra_conditions dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") dependent_conditions = dependent_conditions.gsub('@', '\@') case reflection.options[:dependent] -- cgit v1.2.3 From 13004d4f849682772060371273fda3312dd3b884 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Fri, 2 Apr 2010 22:33:57 -0700 Subject: Make the query built by has_many ...., :dependent => :____ lazy since all the information is not really available yet. --- activerecord/lib/active_record/associations.rb | 72 ++++++++------------------ activerecord/lib/active_record/reflection.rb | 10 ++++ 2 files changed, 32 insertions(+), 50 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 40022a614a..7d628005dd 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1495,13 +1495,6 @@ module ActiveRecord # finder conditions. def configure_dependency_for_has_many(reflection) if reflection.options.include?(:dependent) - # Add polymorphic type if the :as option is present - dependent_conditions = [] - dependent_conditions << "#{reflection.primary_key_name} = \#{record.#{reflection.name}.send(:owner_quoted_id)}" - dependent_conditions << "#{reflection.options[:as]}_type = '#{base_class.name}'" if reflection.options[:as] - dependent_conditions << sanitize_sql(reflection.options[:conditions], reflection.table_name) if reflection.options[:conditions] - dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") - dependent_conditions = dependent_conditions.gsub('@', '\@') case reflection.options[:dependent] when :destroy method_name = "has_many_dependent_destroy_for_#{reflection.name}".to_sym @@ -1510,51 +1503,30 @@ module ActiveRecord end before_destroy method_name when :delete_all - # before_destroy do |record| - # self.class.send(:delete_all_has_many_dependencies, - # record, - # "posts", - # Post, - # %@...@) # this is a string literal like %(...) - # end - # end - module_eval <<-CALLBACK - before_destroy do |record| - self.class.send(:delete_all_has_many_dependencies, - record, - "#{reflection.name}", - #{reflection.class_name}, - %@#{dependent_conditions}@) - end - CALLBACK + before_destroy do |record| + self.class.send(:delete_all_has_many_dependencies, + record, + reflection.name, + reflection.klass, + reflection.dependent_conditions(record, self.class)) + end when :nullify - # before_destroy do |record| - # self.class.send(:nullify_has_many_dependencies, - # record, - # "posts", - # Post, - # "user_id", - # %@...@) # this is a string literal like %(...) - # end - # end - module_eval <<-CALLBACK - before_destroy do |record| - self.class.send(:nullify_has_many_dependencies, - record, - "#{reflection.name}", - #{reflection.class_name}, - "#{reflection.primary_key_name}", - %@#{dependent_conditions}@) - end - CALLBACK - when :restrict - method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym - define_method(method_name) do - unless send(reflection.name).empty? - raise DeleteRestrictionError.new(reflection) - end + before_destroy do |record| + self.class.send(:nullify_has_many_dependencies, + record, + reflection.name, + reflection.klass, + reflection.primary_key_name, + reflection.dependent_conditions(record, self.class)) + end + when :restrict + method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym + define_method(method_name) do + unless send(reflection.name).empty? + raise DeleteRestrictionError.new(reflection) end - before_destroy method_name + end + before_destroy method_name else raise ArgumentError, "The :dependent option expects either :destroy, :delete_all, :nullify or :restrict (#{reflection.options[:dependent].inspect})" end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 5e8fc104cb..2a3f3f8713 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -277,6 +277,16 @@ module ActiveRecord !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many) end + def dependent_conditions(record, base_class) + dependent_conditions = [] + dependent_conditions << "#{primary_key_name} = #{record.send(name).send(:owner_quoted_id)}" + dependent_conditions << "#{options[:as]}_type = '#{base_class.name}'" if options[:as] + dependent_conditions << klass.send(:sanitize_sql, options[:conditions]) if options[:conditions] + dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") + dependent_conditions = dependent_conditions.gsub('@', '\@') + dependent_conditions + end + private def derive_class_name class_name = name.to_s.camelize -- cgit v1.2.3 From 3eb97531b8650db5cc7b9558cc3828c56a526b6a Mon Sep 17 00:00:00 2001 From: wycats Date: Sat, 3 Apr 2010 02:30:06 -0700 Subject: Refactored url_for in AV to have its own instances of the helpers instead of proxying back to the controller. This potentially allows for more standalone usage of AV. It also kicked up a lot of dust in the tests, which were mocking out controllers to get this behavior. By moving it to the view, it made a lot of the tests more standalone (a win) --- actionpack/lib/action_view/base.rb | 7 +- actionpack/lib/action_view/helpers.rb | 9 +- actionpack/lib/action_view/helpers/form_helper.rb | 1 + actionpack/lib/action_view/helpers/url_helper.rb | 52 ++- actionpack/lib/action_view/test_case.rb | 7 +- actionpack/test/abstract_unit.rb | 20 + .../test/template/active_model_helper_test.rb | 9 +- actionpack/test/template/asset_tag_helper_test.rb | 26 +- actionpack/test/template/erb/helper.rb | 11 +- actionpack/test/template/form_helper_test.rb | 18 +- actionpack/test/template/form_tag_helper_test.rb | 14 +- actionpack/test/template/prototype_helper_test.rb | 25 +- .../test/template/scriptaculous_helper_test.rb | 33 +- actionpack/test/template/url_helper_test.rb | 419 ++++++++++----------- 14 files changed, 338 insertions(+), 313 deletions(-) diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index a9b0715b2e..fde61e9df9 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -168,6 +168,8 @@ module ActionView #:nodoc: remove_method :helpers attr_reader :helpers + class_attribute :_router + class << self delegate :erb_trim_mode=, :to => 'ActionView::Template::Handlers::ERB' delegate :logger, :to => 'ActionController::Base', :allow_nil => true @@ -204,7 +206,10 @@ module ActionView #:nodoc: @assigns = assigns_for_first_render.each { |key, value| instance_variable_set("@#{key}", value) } @helpers = self.class.helpers || Module.new - @_controller = controller + if @_controller = controller + @_request = controller.request if controller.respond_to?(:request) + end + @_config = ActiveSupport::InheritableOptions.new(controller.config) if controller && controller.respond_to?(:config) @_content_for = Hash.new { |h,k| h[k] = ActiveSupport::SafeBuffer.new } @_virtual_path = nil diff --git a/actionpack/lib/action_view/helpers.rb b/actionpack/lib/action_view/helpers.rb index a50c180f63..ba3bdd0d18 100644 --- a/actionpack/lib/action_view/helpers.rb +++ b/actionpack/lib/action_view/helpers.rb @@ -29,16 +29,13 @@ module ActionView #:nodoc: autoload :TranslationHelper autoload :UrlHelper - def self.included(base) - base.extend(ClassMethods) - end + extend ActiveSupport::Concern - module ClassMethods - include SanitizeHelper::ClassMethods + included do + extend SanitizeHelper::ClassMethods end include ActiveSupport::Benchmarkable - include ActiveModelHelper include AssetTagHelper include AtomFeedHelper diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 89560d0b49..6a14f0be9c 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -96,6 +96,7 @@ module ActionView extend ActiveSupport::Concern include FormTagHelper + include UrlHelper # Creates a form and a scope around a specific model object that is used # as a base for questioning about values for the fields. diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index b23d5fcb68..1415966869 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -13,14 +13,15 @@ module ActionView extend ActiveSupport::Concern include ActionDispatch::Routing::UrlFor - include JavaScriptHelper + include TagHelper # Need to map default url options to controller one. - def default_url_options(*args) #:nodoc: - controller.send(:default_url_options, *args) - end - + # def default_url_options(*args) #:nodoc: + # controller.send(:default_url_options, *args) + # end + # def url_options + return super unless controller.respond_to?(:url_options) controller.url_options end @@ -97,7 +98,7 @@ module ActionView when Hash options = { :only_path => options[:host].nil? }.update(options.symbolize_keys) escape = options.key?(:escape) ? options.delete(:escape) : false - controller.send(:url_for, options) + super when :back escape = false controller.request.env["HTTP_REFERER"] || 'javascript:history.back()' @@ -119,13 +120,24 @@ module ActionView # # ==== Signatures # - # link_to(name, options = {}, html_options = nil) - # link_to(options = {}, html_options = nil) do + # link_to(body, url, html_options = {}) + # # url is a String; you can use URL helpers like + # # posts_path + # + # link_to(body, url_options = {}, html_options = {}) + # # url_options, except :confirm or :method, + # # is passed to url_for + # + # link_to(options = {}, html_options = {}) do + # # name + # end + # + # link_to(url, html_options = {}) do # # name # end # # ==== Options - # * :confirm => 'question?' - This will allow the unobtrusive JavaScript + # * :confirm => 'question?' - This will allow the unobtrusive JavaScript # driver to prompt with the question specified. If the user accepts, the link is # processed normally, otherwise no action is taken. # * :method => symbol of HTTP verb - This modifier will dynamically @@ -138,7 +150,11 @@ module ActionView # disabled clicking the link will have no effect. If you are relying on the # POST behavior, you should check for it in your controller's action by using # the request object's methods for post?, delete? or put?. - # * The +html_options+ will accept a hash of html attributes for the link tag. + # * :remote => true - This will allow the unobtrusive JavaScript + # driver to make an Ajax request to the URL in question instead of following + # the link. The drivers each provide mechanisms for listening for the + # completion of the Ajax request and performing JavaScript operations once + # they're complete # # ==== Examples # Because it relies on +url_for+, +link_to+ supports both older-style controller/action/id arguments @@ -220,8 +236,8 @@ module ActionView options = args[1] || {} html_options = args[2] - url = url_for(options) html_options = convert_options_to_data_attributes(options, html_options) + url = url_for(options) if html_options html_options = html_options.stringify_keys @@ -259,10 +275,10 @@ module ActionView # There are a few special +html_options+: # * :method - Specifies the anchor name to be appended to the path. # * :disabled - Specifies the anchor name to be appended to the path. - # * :confirm - This will use the unobtrusive JavaScript driver to + # * :confirm - This will use the unobtrusive JavaScript driver to # prompt with the question specified. If the user accepts, the link is # processed normally, otherwise no action is taken. - # * :remote - If set to true, will allow the Unobtrusive JavaScript drivers to control the + # * :remote - If set to true, will allow the Unobtrusive JavaScript drivers to control the # submit behaviour. By default this behaviour is an ajax submit. # # ==== Examples @@ -282,7 +298,7 @@ module ActionView # # " # # - # <%= button_to('Destroy', 'http://www.example.com', :confirm => 'Are you sure?', + # <%= button_to('Destroy', 'http://www.example.com', :confirm => 'Are you sure?', # :method => "delete", :remote => true, :disable_with => 'loading...') %> # # => "
# #
@@ -546,8 +562,14 @@ module ActionView # current_page?(:controller => 'library', :action => 'checkout') # # => false def current_page?(options) + unless request + raise "You cannot use helpers that need to determine the current " \ + "page unless your view context provides a Request object " \ + "in a #request method" + end + url_string = CGI.unescapeHTML(url_for(options)) - request = controller.request + # We ignore any extra parameters in the request_uri if the # submitted url doesn't have any either. This lets the function # work with things like ?order=asc diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 23b0c6e121..ddea9cfd92 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -142,8 +142,13 @@ module ActionView end end + def _router + @controller._router if @controller.respond_to?(:_router) + end + def method_missing(selector, *args) - if @controller._router.named_routes.helpers.include?(selector) + if @controller.respond_to?(:_router) && + @controller._router.named_routes.helpers.include?(selector) @controller.__send__(selector, *args) else super diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 143491a640..fe78b8ec1f 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -57,6 +57,18 @@ module RackTestUtils extend self end +module RenderERBUtils + def render_erb(string) + template = ActionView::Template.new( + string.strip, + "test template", + ActionView::Template::Handlers::ERB, + {}) + + template.render(self, {}).strip + end +end + module SetupOnce extend ActiveSupport::Concern @@ -225,6 +237,14 @@ class Rack::TestCase < ActionController::IntegrationTest end end +class ActionController::Base + def self.test_routes(&block) + router = ActionDispatch::Routing::RouteSet.new + router.draw(&block) + include router.url_helpers + end +end + class ::ApplicationController < ActionController::Base end diff --git a/actionpack/test/template/active_model_helper_test.rb b/actionpack/test/template/active_model_helper_test.rb index 1a5316a689..47eb620f7a 100644 --- a/actionpack/test/template/active_model_helper_test.rb +++ b/actionpack/test/template/active_model_helper_test.rb @@ -118,13 +118,12 @@ class ActiveModelHelperTest < ActionView::TestCase setup_user @response = ActionController::TestResponse.new + end - @controller = Object.new - def @controller.url_for(options) - options = options.symbolize_keys + def url_for(options) + options = options.symbolize_keys - [options[:action], options[:id].to_param].compact.join('/') - end + [options[:action], options[:id].to_param].compact.join('/') end def test_generic_input_tag diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index fbd504ae7d..223a430f92 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -34,9 +34,7 @@ class AssetTagHelperTest < ActionView::TestCase ) end - @controller = Class.new(BasicController) do - def url_for(*args) "http://www.example.com" end - end.new + @controller = BasicController.new @request = Class.new do def protocol() 'http://' end @@ -49,6 +47,10 @@ class AssetTagHelperTest < ActionView::TestCase ActionView::Helpers::AssetTagHelper::reset_javascript_include_default end + def url_for(*args) + "http://www.example.com" + end + def teardown config.perform_caching = false ENV.delete('RAILS_ASSET_ID') @@ -893,25 +895,19 @@ class AssetTagHelperNonVhostTest < ActionView::TestCase def setup super - @controller = Class.new(BasicController) do - def url_for(options) - "http://www.example.com/collaboration/hieraki" - end - end.new - + @controller = BasicController.new @controller.config.relative_url_root = "/collaboration/hieraki" - @request = Class.new do - def protocol - 'gopher://' - end - end.new - + @request = Struct.new(:protocol).new("gopher://") @controller.request = @request ActionView::Helpers::AssetTagHelper::reset_javascript_include_default end + def url_for(options) + "http://www.example.com/collaboration/hieraki" + end + def test_should_compute_proper_path assert_dom_equal(%(), auto_discovery_link_tag) assert_dom_equal(%(/collaboration/hieraki/javascripts/xmlhr.js), javascript_path("xmlhr")) diff --git a/actionpack/test/template/erb/helper.rb b/actionpack/test/template/erb/helper.rb index 7147178849..799f9e4036 100644 --- a/actionpack/test/template/erb/helper.rb +++ b/actionpack/test/template/erb/helper.rb @@ -1,20 +1,13 @@ module ERBTest class ViewContext - mock_controller = Class.new do - include SharedTestRoutes.url_helpers - end - + include SharedTestRoutes.url_helpers include ActionView::Helpers::TagHelper include ActionView::Helpers::JavaScriptHelper include ActionView::Helpers::FormHelper - attr_accessor :output_buffer + attr_accessor :output_buffer, :controller def protect_against_forgery?() false end - - define_method(:controller) do - mock_controller.new - end end class BlockTestCase < ActiveSupport::TestCase diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 7c5ccfd6ed..4af38e52dd 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -63,15 +63,15 @@ class FormHelperTest < ActionView::TestCase @post.body = "Back to the hill and over it again!" @post.secret = 1 @post.written_on = Date.new(2004, 6, 15) + end - @controller = Class.new do - attr_reader :url_for_options - def url_for(options) - @url_for_options = options - "http://www.example.com" - end + def url_for(object) + @url_for_options = object + if object.is_a?(Hash) + "http://www.example.com" + else + super end - @controller = @controller.new end def test_label @@ -1348,8 +1348,8 @@ class FormHelperTest < ActionView::TestCase def test_form_for_with_hash_url_option form_for(:post, @post, :url => {:controller => 'controller', :action => 'action'}) do |f| end - assert_equal 'controller', @controller.url_for_options[:controller] - assert_equal 'action', @controller.url_for_options[:action] + assert_equal 'controller', @url_for_options[:controller] + assert_equal 'action', @url_for_options[:action] end def test_form_for_with_record_url_option diff --git a/actionpack/test/template/form_tag_helper_test.rb b/actionpack/test/template/form_tag_helper_test.rb index 1f26840289..3b8760351e 100644 --- a/actionpack/test/template/form_tag_helper_test.rb +++ b/actionpack/test/template/form_tag_helper_test.rb @@ -8,11 +8,15 @@ class FormTagHelperTest < ActionView::TestCase def setup super - @controller = Class.new(BasicController) do - def url_for(options) - "http://www.example.com" - end - end.new + @controller = BasicController.new + end + + def url_for(options) + if options.is_a?(Hash) + "http://www.example.com" + else + super + end end VALID_HTML_ID = /^[A-Za-z][-_:.A-Za-z0-9]*$/ # see http://www.w3.org/TR/html4/types.html#type-name diff --git a/actionpack/test/template/prototype_helper_test.rb b/actionpack/test/template/prototype_helper_test.rb index 619293dc43..0ff37f44c2 100644 --- a/actionpack/test/template/prototype_helper_test.rb +++ b/actionpack/test/template/prototype_helper_test.rb @@ -47,19 +47,18 @@ class PrototypeHelperBaseTest < ActionView::TestCase def setup super @template = self - @controller = Class.new do - def url_for(options) - if options.is_a?(String) - options - else - url = "http://www.example.com/" - url << options[:action].to_s if options and options[:action] - url << "?a=#{options[:a]}" if options && options[:a] - url << "&b=#{options[:b]}" if options && options[:a] && options[:b] - url - end - end - end.new + end + + def url_for(options) + if options.is_a?(String) + options + else + url = "http://www.example.com/" + url << options[:action].to_s if options and options[:action] + url << "?a=#{options[:a]}" if options && options[:a] + url << "&b=#{options[:b]}" if options && options[:a] && options[:b] + url + end end protected diff --git a/actionpack/test/template/scriptaculous_helper_test.rb b/actionpack/test/template/scriptaculous_helper_test.rb index bebc3cb9f4..233012bfdd 100644 --- a/actionpack/test/template/scriptaculous_helper_test.rb +++ b/actionpack/test/template/scriptaculous_helper_test.rb @@ -3,21 +3,16 @@ require 'abstract_unit' class ScriptaculousHelperTest < ActionView::TestCase tests ActionView::Helpers::ScriptaculousHelper - def setup - super - @controller = Class.new do - def url_for(options) - url = "http://www.example.com/" - url << options[:action].to_s if options and options[:action] - url - end - end.new + def url_for(options) + url = "http://www.example.com/" + url << options[:action].to_s if options and options[:action] + url end - + def test_effect assert_equal "new Effect.Highlight(\"posts\",{});", visual_effect(:highlight, "posts") assert_equal "new Effect.Highlight(\"posts\",{});", visual_effect("highlight", :posts) - assert_equal "new Effect.Highlight(\"posts\",{});", visual_effect(:highlight, :posts) + assert_equal "new Effect.Highlight(\"posts\",{});", visual_effect(:highlight, :posts) assert_equal "new Effect.Fade(\"fademe\",{duration:4.0});", visual_effect(:fade, "fademe", :duration => 4.0) assert_equal "new Effect.Shake(element,{});", visual_effect(:shake) assert_equal "new Effect.DropOut(\"dropme\",{queue:'end'});", visual_effect(:drop_out, 'dropme', :queue => :end) @@ -43,7 +38,7 @@ class ScriptaculousHelperTest < ActionView::TestCase assert ve[2].include?("scope:'test'") assert ve[2].include?("position:'end'") end - + def test_toggle_effects assert_equal "Effect.toggle(\"posts\",'appear',{});", visual_effect(:toggle_appear, "posts") assert_equal "Effect.toggle(\"posts\",'slide',{});", visual_effect(:toggle_slide, "posts") @@ -52,26 +47,26 @@ class ScriptaculousHelperTest < ActionView::TestCase assert_equal "Effect.toggle(\"posts\",'slide',{});", visual_effect("toggle_slide", "posts") assert_equal "Effect.toggle(\"posts\",'blind',{});", visual_effect("toggle_blind", "posts") end - + def test_sortable_element - assert_dom_equal %(), + assert_dom_equal %(), sortable_element("mylist", :url => { :action => "order" }) - assert_equal %(), + assert_equal %(), sortable_element("mylist", :tag => "div", :constraint => "horizontal", :url => { :action => "order" }) - assert_dom_equal %||, + assert_dom_equal %||, sortable_element("mylist", :containment => ['list1','list2'], :constraint => "horizontal", :url => { :action => "order" }) - assert_dom_equal %(), + assert_dom_equal %(), sortable_element("mylist", :containment => 'list1', :constraint => "horizontal", :url => { :action => "order" }) end - + def test_draggable_element assert_dom_equal %(), draggable_element("product_13") assert_equal %(), draggable_element("product_13", :revert => true) end - + def test_drop_receiving_element assert_dom_equal %(), drop_receiving_element("droptarget1") diff --git a/actionpack/test/template/url_helper_test.rb b/actionpack/test/template/url_helper_test.rb index 87b2e59255..35e73fbf1e 100644 --- a/actionpack/test/template/url_helper_test.rb +++ b/actionpack/test/template/url_helper_test.rb @@ -3,58 +3,73 @@ require 'abstract_unit' require 'active_support/ordered_options' require 'controller/fake_controllers' -class UrlHelperTest < ActionView::TestCase - - def setup - super - @controller = Class.new(BasicController) do - attr_accessor :url - def url_for(options) - url - end - end +class UrlHelperTest < ActiveSupport::TestCase + + # In a few cases, the helper proxies to 'controller' + # or request. + # + # In those cases, we'll set up a simple mock + attr_accessor :controller, :request + + routes = ActionDispatch::Routing::RouteSet.new + routes.draw do + match "/" => "foo#bar" + match "/other" => "foo#other" + end + + include routes.url_helpers + + include ActionView::Helpers::UrlHelper + include ActionDispatch::Assertions::DomAssertions + include ActionView::Context + include RenderERBUtils + + # self.default_url_options = {:host => "www.example.com"} + + # TODO: This shouldn't be needed (see template.rb:53) + def assigns + {} + end - @controller = @controller.new - @request = @controller.request = ActionDispatch::TestRequest.new - @controller.url = "http://www.example.com" + def abcd(hash = {}) + hash_for(:a => :b, :c => :d).merge(hash) end + def hash_for(opts = {}) + {:controller => "foo", :action => "bar"}.merge(opts) + end + alias url_hash hash_for + def test_url_for_escapes_urls - @controller.url = "http://www.example.com?a=b&c=d" - assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd') - assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => true) - assert_equal "http://www.example.com?a=b&c=d", url_for(:a => 'b', :c => 'd', :escape => false) + assert_equal "/?a=b&c=d", url_for(abcd) + assert_equal "/?a=b&c=d", url_for(abcd(:escape => true)) + assert_equal "/?a=b&c=d", url_for(abcd(:escape => false)) end def test_url_for_escaping_is_safety_aware - assert url_for(:a => 'b', :c => 'd', :escape => true).html_safe?, "escaped urls should be html_safe?" - assert !url_for(:a => 'b', :c => 'd', :escape => false).html_safe?, "non-escaped urls shouldn't be safe" + assert url_for(abcd(:escape => true)).html_safe?, "escaped urls should be html_safe?" + assert !url_for(abcd(:escape => false)).html_safe?, "non-escaped urls should not be html_safe?" end def test_url_for_escapes_url_once - @controller.url = "http://www.example.com?a=b&c=d" - assert_equal "http://www.example.com?a=b&c=d", url_for("http://www.example.com?a=b&c=d") + assert_equal "/?a=b&c=d", url_for("/?a=b&c=d") end def test_url_for_with_back - @request.env['HTTP_REFERER'] = 'http://www.example.com/referer' + referer = 'http://www.example.com/referer' + @controller = Struct.new(:request).new(Struct.new(:env).new({"HTTP_REFERER" => referer})) + assert_equal 'http://www.example.com/referer', url_for(:back) end def test_url_for_with_back_and_no_referer - @request.env['HOST_NAME'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' + @controller = Struct.new(:request).new(Struct.new(:env).new({})) assert_equal 'javascript:history.back()', url_for(:back) end def test_url_for_from_hash_doesnt_escape_ampersand - @controller = TestController.new - @view = ActionView::Base.new - @view.controller = @controller - - path = @view.url_for(:controller => :cheeses, :foo => :bar, :baz => :quux) - - assert_equal '/cheeses?baz=quux&foo=bar', sort_query_string_params(path) + path = url_for(hash_for(:foo => :bar, :baz => :quux)) + assert_equal '/?baz=quux&foo=bar', sort_query_string_params(path) end # todo: missing test cases @@ -118,65 +133,54 @@ class UrlHelperTest < ActionView::TestCase end def test_link_tag_without_host_option - ActionController::Base.class_eval { attr_accessor :url } - url = {:controller => 'weblog', :action => 'show'} - @controller = ActionController::Base.new - @controller.request = ActionController::TestRequest.new - assert_dom_equal(%q{Test Link}, link_to('Test Link', url)) + assert_dom_equal(%q{Test Link}, link_to('Test Link', url_hash)) end def test_link_tag_with_host_option - ActionController::Base.class_eval { attr_accessor :url } - url = {:controller => 'weblog', :action => 'show', :host => 'www.example.com'} - @controller = ActionController::Base.new - @controller.request = ActionController::TestRequest.new - assert_dom_equal(%q{Test Link}, link_to('Test Link', url)) + hash = hash_for(:host => "www.example.com") + expected = %q{Test Link} + assert_dom_equal(expected, link_to('Test Link', hash)) end def test_link_tag_with_query - assert_dom_equal "Hello", link_to("Hello", "http://www.example.com?q1=v1&q2=v2") + expected = %{Hello} + assert_dom_equal expected, link_to("Hello", "http://www.example.com?q1=v1&q2=v2") end def test_link_tag_with_query_and_no_name - assert_dom_equal "http://www.example.com?q1=v1&q2=v2", link_to(nil, "http://www.example.com?q1=v1&q2=v2") + link = link_to(nil, "http://www.example.com?q1=v1&q2=v2") + expected = %{http://www.example.com?q1=v1&q2=v2} + assert_dom_equal expected, link end def test_link_tag_with_back - @request.env['HOST_NAME'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @request.env['HTTP_REFERER'] = 'http://www.example.com/referer' - assert_dom_equal "go back", link_to('go back', :back) + env = {"HTTP_REFERER" => "http://www.example.com/referer"} + @controller = Struct.new(:request).new(Struct.new(:env).new(env)) + expected = %{go back} + assert_dom_equal expected, link_to('go back', :back) end def test_link_tag_with_back_and_no_referer - @request.env['HOST_NAME'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - assert_dom_equal "go back", link_to('go back', :back) - end - - def test_link_tag_with_back - @request.env['HOST_NAME'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @request.env['HTTP_REFERER'] = 'http://www.example.com/referer' - assert_dom_equal "go back", link_to('go back', :back) - end - - def test_link_tag_with_back_and_no_referer - @request.env['HOST_NAME'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - assert_dom_equal "go back", link_to('go back', :back) + @controller = Struct.new(:request).new(Struct.new(:env).new({})) + link = link_to('go back', :back) + assert_dom_equal %{go back}, link end def test_link_tag_with_img - assert_dom_equal "\"Favicon\"", link_to(image_tag("/favicon.jpg"), "http://www.example.com") + link = link_to("".html_safe, "/") + expected = %{} + assert_dom_equal expected, link end def test_link_with_nil_html_options - assert_dom_equal "Hello", link_to("Hello", {:action => 'myaction'}, nil) + link = link_to("Hello", url_hash, nil) + assert_dom_equal %{Hello}, link end def test_link_tag_with_custom_onclick - assert_dom_equal "Hello", link_to("Hello", "http://www.example.com", :onclick => "alert('yay!')") + link = link_to("Hello", "http://www.example.com", :onclick => "alert('yay!')") + expected = %{Hello} + assert_dom_equal expected, link end def test_link_tag_with_javascript_confirm @@ -238,91 +242,106 @@ class UrlHelperTest < ActionView::TestCase end def test_link_tag_using_block_in_erb - output_buffer = link_to("http://example.com") { concat("Example site") } - assert_equal 'Example site', output_buffer + out = render_erb %{<%= link_to('/') do %>Example site<% end %>} + assert_equal 'Example site', out end def test_link_to_unless - assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog") - assert_dom_equal "Listing", link_to_unless(false, "Listing", :action => "list", :controller => "weblog") - assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) - assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { |name, options, html_options| - "#{name}" - } - assert_equal "Showing", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { |name| - "#{name}" - } - assert_equal "test", link_to_unless(true, "Showing", :action => "show", :controller => "weblog", :id => 1) { - "test" - } + assert_equal "Showing", link_to_unless(true, "Showing", url_hash) + + assert_dom_equal %{Listing}, + link_to_unless(false, "Listing", url_hash) + + assert_equal "Showing", link_to_unless(true, "Showing", url_hash) + + assert_equal "Showing", + link_to_unless(true, "Showing", url_hash) { |name| + "#{name}" + } + + assert_equal "Showing", + link_to_unless(true, "Showing", url_hash) { |name| + "#{name}" + } + + assert_equal "test", + link_to_unless(true, "Showing", url_hash) { + "test" + } end def test_link_to_if - assert_equal "Showing", link_to_if(false, "Showing", :action => "show", :controller => "weblog") - assert_dom_equal "Listing", link_to_if(true, "Listing", :action => "list", :controller => "weblog") - assert_equal "Showing", link_to_if(false, "Showing", :action => "show", :controller => "weblog", :id => 1) + assert_equal "Showing", link_to_if(false, "Showing", url_hash) + assert_dom_equal %{Listing}, link_to_if(true, "Listing", url_hash) + assert_equal "Showing", link_to_if(false, "Showing", url_hash) + end + + def request_for_url(url) + env = Rack::MockRequest.env_for("http://www.example.com#{url}") + ActionDispatch::Request.new(env) end def test_current_page_with_simple_url - @request.env['HTTP_HOST'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @controller.url = "http://www.example.com/weblog/show" - assert current_page?({ :action => "show", :controller => "weblog" }) - assert current_page?("http://www.example.com/weblog/show") + @request = request_for_url("/") + assert current_page?(url_hash) + assert current_page?("http://www.example.com/") end def test_current_page_ignoring_params - @request.env['HTTP_HOST'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @request.env['QUERY_STRING'] = 'order=desc&page=1' - @controller.url = "http://www.example.com/weblog/show?order=desc&page=1" - assert current_page?({ :action => "show", :controller => "weblog" }) - assert current_page?("http://www.example.com/weblog/show") + @request = request_for_url("/?order=desc&page=1") + + assert current_page?(url_hash) + assert current_page?("http://www.example.com/") end def test_current_page_with_params_that_match - @request.env['HTTP_HOST'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @request.env['QUERY_STRING'] = 'order=desc&page=1' - @controller.url = "http://www.example.com/weblog/show?order=desc&page=1" - assert current_page?({ :action => "show", :controller => "weblog", :order => "desc", :page => "1" }) - assert current_page?("http://www.example.com/weblog/show?order=desc&page=1") + @request = request_for_url("/?order=desc&page=1") + + assert current_page?(hash_for(:order => "desc", :page => "1")) + assert current_page?("http://www.example.com/?order=desc&page=1") end def test_link_unless_current - @request.env['HTTP_HOST'] = 'www.example.com' - @request.env['PATH_INFO'] = '/weblog/show' - @controller.url = "http://www.example.com/weblog/show" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show") - - @request.env['QUERY_STRING'] = 'order=desc' - @controller.url = "http://www.example.com/weblog/show" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show") - - @request.env['QUERY_STRING'] = 'order=desc&page=1' - @controller.url = "http://www.example.com/weblog/show?order=desc&page=1" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog", :order=>'desc', :page=>'1' }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1") - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=1") - - @request.env['QUERY_STRING'] = 'order=desc' - @controller.url = "http://www.example.com/weblog/show?order=asc" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=asc") - - @request.env['QUERY_STRING'] = 'order=desc&page=1' - @controller.url = "http://www.example.com/weblog/show?order=desc&page=2" - assert_equal "Showing", link_to_unless_current("Showing", { :action => "show", :controller => "weblog" }) - assert_equal "Showing", link_to_unless_current("Showing", "http://www.example.com/weblog/show?order=desc&page=2") - - @request.env['QUERY_STRING'] = '' - @controller.url = "http://www.example.com/weblog/list" - assert_equal "Listing", - link_to_unless_current("Listing", :action => "list", :controller => "weblog") - assert_equal "Listing", - link_to_unless_current("Listing", "http://www.example.com/weblog/list") + @request = request_for_url("/") + + assert_equal "Showing", + link_to_unless_current("Showing", url_hash) + assert_equal "Showing", + link_to_unless_current("Showing", "http://www.example.com/") + + @request = request_for_url("/?order=desc") + + assert_equal "Showing", + link_to_unless_current("Showing", url_hash) + assert_equal "Showing", + link_to_unless_current("Showing", "http://www.example.com/") + + @request = request_for_url("/?order=desc&page=1") + + assert_equal "Showing", + link_to_unless_current("Showing", hash_for(:order=>'desc', :page=>'1')) + assert_equal "Showing", + link_to_unless_current("Showing", "http://www.example.com/?order=desc&page=1") + + @request = request_for_url("/?order=desc") + + assert_equal %{Showing}, + link_to_unless_current("Showing", hash_for(:order => :asc)) + assert_equal %{Showing}, + link_to_unless_current("Showing", "http://www.example.com/?order=asc") + + @request = request_for_url("/?order=desc") + assert_equal %{Showing}, + link_to_unless_current("Showing", hash_for(:order => "desc", :page => 2)) + assert_equal %{Showing}, + link_to_unless_current("Showing", "http://www.example.com/?order=desc&page=2") + + @request = request_for_url("/show") + + assert_equal %{Listing}, + link_to_unless_current("Listing", url_hash) + assert_equal %{Listing}, + link_to_unless_current("Listing", "http://www.example.com/") end def test_mail_to @@ -352,7 +371,8 @@ class UrlHelperTest < ActionView::TestCase end def test_mail_to_with_img - assert_dom_equal %(), mail_to('feedback@example.com', ''.html_safe) + assert_dom_equal %(), + mail_to('feedback@example.com', ''.html_safe) end def test_mail_to_with_hex @@ -369,6 +389,7 @@ class UrlHelperTest < ActionView::TestCase assert_dom_equal "", mail_to("me@domain.com", nil, :encode => "javascript", :replace_at => "(at)", :replace_dot => "(dot)") end + # TODO: button_to looks at this ... why? def protect_against_forgery? false end @@ -383,6 +404,15 @@ end class UrlHelperControllerTest < ActionController::TestCase class UrlHelperController < ActionController::Base + test_routes do |map| + match 'url_helper_controller_test/url_helper/show_named_route', + :to => 'url_helper_controller_test/url_helper#show_named_route', + :as => :show_named_route + + map.connect ":controller/:action/:id" + # match "/:controller(/:action(/:id))" + end + def show_url_for render :inline => "<%= url_for :controller => 'url_helper_controller_test/url_helper', :action => 'show_url_for' %>" end @@ -406,17 +436,14 @@ class UrlHelperControllerTest < ActionController::TestCase end def test_named_route_url_shows_host_and_path - with_url_helper_routing do - get :show_named_route, :kind => 'url' - assert_equal 'http://test.host/url_helper_controller_test/url_helper/show_named_route', @response.body - end + get :show_named_route, :kind => 'url' + assert_equal 'http://test.host/url_helper_controller_test/url_helper/show_named_route', + @response.body end def test_named_route_path_shows_only_path - with_url_helper_routing do - get :show_named_route, :kind => 'path' - assert_equal '/url_helper_controller_test/url_helper/show_named_route', @response.body - end + get :show_named_route, :kind => 'path' + assert_equal '/url_helper_controller_test/url_helper/show_named_route', @response.body end def test_url_for_nil_returns_current_path @@ -431,24 +458,16 @@ class UrlHelperControllerTest < ActionController::TestCase end end - with_url_helper_routing do - get :show_named_route, :kind => 'url' - assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body - end + get :show_named_route, :kind => 'url' + assert_equal 'http://testtwo.host/url_helper_controller_test/url_helper/show_named_route', @response.body end - - protected - def with_url_helper_routing - with_routing do |set| - set.draw do |map| - match 'url_helper_controller_test/url_helper/show_named_route', :to => 'url_helper_controller_test/url_helper#show_named_route', :as => :show_named_route - end - yield - end - end end class TasksController < ActionController::Base + test_routes do + resources :tasks + end + def index render_default end @@ -468,36 +487,19 @@ class TasksController < ActionController::Base end class LinkToUnlessCurrentWithControllerTest < ActionController::TestCase - def setup - super - @controller = TasksController.new - end + tests TasksController def test_link_to_unless_current_to_current - with_restful_routing do - get :index - assert_equal "tasks\ntasks", @response.body - end + get :index + assert_equal "tasks\ntasks", @response.body end def test_link_to_unless_current_shows_link - with_restful_routing do - get :show, :id => 1 - assert_equal "tasks\n" + - "tasks", - @response.body - end + get :show, :id => 1 + assert_equal "tasks\n" + + "tasks", + @response.body end - - protected - def with_restful_routing - with_routing do |set| - set.draw do |map| - resources :tasks - end - yield - end - end end class Workshop @@ -537,6 +539,12 @@ class Session end class WorkshopsController < ActionController::Base + test_routes do + resources :workshops do + resources :sessions + end + end + def index @workshop = Workshop.new(nil) render :inline => "<%= url_for(@workshop) %>\n<%= link_to('Workshop', @workshop) %>" @@ -551,6 +559,12 @@ class WorkshopsController < ActionController::Base end class SessionsController < ActionController::Base + test_routes do + resources :workshops do + resources :sessions + end + end + def index @workshop = Workshop.new(params[:workshop_id]) @session = Session.new(nil) @@ -567,56 +581,31 @@ class SessionsController < ActionController::Base end class PolymorphicControllerTest < ActionController::TestCase - def setup - super - @response = ActionController::TestResponse.new - end - def test_new_resource @controller = WorkshopsController.new - with_restful_routing do - get :index - assert_equal "/workshops\nWorkshop", @response.body - end + get :index + assert_equal "/workshops\nWorkshop", @response.body end def test_existing_resource @controller = WorkshopsController.new - with_restful_routing do - get :show, :id => 1 - assert_equal "/workshops/1\nWorkshop", @response.body - end + get :show, :id => 1 + assert_equal "/workshops/1\nWorkshop", @response.body end def test_new_nested_resource @controller = SessionsController.new - with_restful_routing do - get :index, :workshop_id => 1 - assert_equal "/workshops/1/sessions\nSession", @response.body - end + get :index, :workshop_id => 1 + assert_equal "/workshops/1/sessions\nSession", @response.body end - + def test_existing_nested_resource @controller = SessionsController.new - - with_restful_routing do - get :show, :workshop_id => 1, :id => 1 - assert_equal "/workshops/1/sessions/1\nSession", @response.body - end + + get :show, :workshop_id => 1, :id => 1 + assert_equal "/workshops/1/sessions/1\nSession", @response.body end - - protected - def with_restful_routing - with_routing do |set| - set.draw do |map| - resources :workshops do - resources :sessions - end - end - yield - end - end end -- cgit v1.2.3 From 52ffaa182ea323af120dc5687d7547004167d0da Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Sat, 3 Apr 2010 09:19:56 -0700 Subject: Sanitize association conditions using the correct class --- .../lib/active_record/associations/through_association_scope.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association_scope.rb index 6f0f698f1e..1d2f323112 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association_scope.rb @@ -135,7 +135,7 @@ module ActiveRecord def build_through_conditions conditions = @reflection.through_reflection.options[:conditions] if conditions.is_a?(Hash) - interpolate_sql(sanitize_sql(conditions)).gsub( + interpolate_sql(@reflection.through_reflection.klass.send(:sanitize_sql, conditions)).gsub( @reflection.quoted_table_name, @reflection.through_reflection.quoted_table_name) elsif conditions -- cgit v1.2.3 From 6e18fa0375b7116328fbee920c074006962a52e8 Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Sat, 3 Apr 2010 09:21:44 -0700 Subject: Raise a StatementInvalid error when trying to build a condition with hash keys that do not correspond to columns. --- activerecord/lib/active_record/relation/predicate_builder.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/activerecord/lib/active_record/relation/predicate_builder.rb b/activerecord/lib/active_record/relation/predicate_builder.rb index 717756418c..d0efa2189d 100644 --- a/activerecord/lib/active_record/relation/predicate_builder.rb +++ b/activerecord/lib/active_record/relation/predicate_builder.rb @@ -20,7 +20,9 @@ module ActiveRecord table = Arel::Table.new(table_name, :engine => @engine) end - attribute = table[column] + unless attribute = table[column] + raise StatementInvalid, "No attribute named `#{column}` exists for table `#{table.name}`" + end case value when Array, ActiveRecord::Associations::AssociationCollection, ActiveRecord::Relation -- cgit v1.2.3 From 467d251c3dcbd3e4dd1e785a21d63535b795a64c Mon Sep 17 00:00:00 2001 From: Carl Lerche Date: Sat, 3 Apr 2010 09:54:15 -0700 Subject: Bring back +extra_conditions+. This effectively reverts 386b7bfd9d78a6d8c8bc7cc4a310df806ad0ba57 --- activerecord/lib/active_record/associations.rb | 6 +++--- activerecord/lib/active_record/reflection.rb | 3 ++- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 7d628005dd..20a8754b7c 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1493,7 +1493,7 @@ module ActiveRecord # The +extra_conditions+ parameter, which is not used within the main # Active Record codebase, is meant to allow plugins to define extra # finder conditions. - def configure_dependency_for_has_many(reflection) + def configure_dependency_for_has_many(reflection, extra_conditions = nil) if reflection.options.include?(:dependent) case reflection.options[:dependent] when :destroy @@ -1508,7 +1508,7 @@ module ActiveRecord record, reflection.name, reflection.klass, - reflection.dependent_conditions(record, self.class)) + reflection.dependent_conditions(record, self.class, extra_conditions)) end when :nullify before_destroy do |record| @@ -1517,7 +1517,7 @@ module ActiveRecord reflection.name, reflection.klass, reflection.primary_key_name, - reflection.dependent_conditions(record, self.class)) + reflection.dependent_conditions(record, self.class, extra_conditions)) end when :restrict method_name = "has_many_dependent_restrict_for_#{reflection.name}".to_sym diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 2a3f3f8713..0e48e229b3 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -277,11 +277,12 @@ module ActiveRecord !options[:validate].nil? ? options[:validate] : (options[:autosave] == true || macro == :has_many) end - def dependent_conditions(record, base_class) + def dependent_conditions(record, base_class, extra_conditions) dependent_conditions = [] dependent_conditions << "#{primary_key_name} = #{record.send(name).send(:owner_quoted_id)}" dependent_conditions << "#{options[:as]}_type = '#{base_class.name}'" if options[:as] dependent_conditions << klass.send(:sanitize_sql, options[:conditions]) if options[:conditions] + dependent_conditions << extra_conditions if extra_conditions dependent_conditions = dependent_conditions.collect {|where| "(#{where})" }.join(" AND ") dependent_conditions = dependent_conditions.gsub('@', '\@') dependent_conditions -- cgit v1.2.3