diff options
72 files changed, 1000 insertions, 766 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 840708cdc6..d67d563181 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -693,15 +693,8 @@ module ActionMailer #:nodoc: end def each_template(paths, name, &block) #:nodoc: - Array.wrap(paths).each do |path| - templates = lookup_context.find_all(name, path) - templates = templates.uniq_by { |t| t.formats } - - unless templates.empty? - templates.each(&block) - return - end - end + templates = lookup_context.find_all(name, Array.wrap(paths)) + templates.uniq_by { |t| t.formats }.each(&block) end def create_parts_from_responses(m, responses) #:nodoc: diff --git a/actionmailer/lib/action_mailer/old_api.rb b/actionmailer/lib/action_mailer/old_api.rb index a8d7454898..dedb447ced 100644 --- a/actionmailer/lib/action_mailer/old_api.rb +++ b/actionmailer/lib/action_mailer/old_api.rb @@ -201,7 +201,7 @@ module ActionMailer if String === @body @parts.unshift create_inline_part(@body) elsif @parts.empty? || @parts.all? { |p| p.content_disposition =~ /^attachment/ } - lookup_context.find_all(@template, @mailer_name).each do |template| + lookup_context.find_all(@template, [@mailer_name]).each do |template| self.formats = template.formats @parts << create_inline_part(render(:template => template), template.mime_type) end diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index b18d6f7817..78fadce6cb 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,8 +1,10 @@ *Rails 3.1.0 (unreleased)* -* brought back config.action_view.cache_template_loading, which allows to decide whether templates should be cached or not [Piotr Sarnacki] +* Template lookup now searches further up in the inheritance chain. [Artemave] -* url_for and named url helpers now accept :subdomain and :domain as options [Josh Kalderimis] +* Brought back config.action_view.cache_template_loading, which allows to decide whether templates should be cached or not. [Piotr Sarnacki] + +* url_for and named url helpers now accept :subdomain and :domain as options, [Josh Kalderimis] * The redirect route method now also accepts a hash of options which will only change the parts of the url in question, or an object which responds to call, allowing for redirects to be reused (check the documentation for examples). [Josh Kalderimis] diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index b7d1d8c2af..20becc0bb1 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -21,7 +21,7 @@ Gem::Specification.new do |s| s.add_dependency('activesupport', version) s.add_dependency('activemodel', version) - s.add_dependency('rack-cache', '~> 0.5.3') + s.add_dependency('rack-cache', '~> 1.0.0') s.add_dependency('builder', '~> 3.0.0') s.add_dependency('i18n', '~> 0.5.0') s.add_dependency('rack', '~> 1.2.1') diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index 606f7eedec..4ee54474cc 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -265,11 +265,11 @@ module AbstractController raise ArgumentError, "Layouts must be specified as a String, Symbol, false, or nil" when nil if name - _prefix = "layouts" unless _implied_layout_name =~ /\blayouts/ + _prefixes = _implied_layout_name =~ /\blayouts/ ? [] : ["layouts"] self.class_eval <<-RUBY, __FILE__, __LINE__ + 1 def _layout - if template_exists?("#{_implied_layout_name}", #{_prefix.inspect}) + if template_exists?("#{_implied_layout_name}", #{_prefixes.inspect}) "#{_implied_layout_name}" else super diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 91b75273fa..9b912ea988 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -60,6 +60,20 @@ module AbstractController end end end + + def parent_prefixes + @parent_prefixes ||= begin + parent_controller = superclass + prefixes = [] + + until parent_controller.abstract? + prefixes << parent_controller.controller_path + parent_controller = parent_controller.superclass + end + + prefixes + end + end end attr_writer :view_context_class @@ -114,9 +128,12 @@ module AbstractController view_context.render(options) end - # The prefix used in render "foo" shortcuts. - def _prefix - controller_path + # The prefixes used in render "foo" shortcuts. + def _prefixes + @_prefixes ||= begin + parent_prefixes = self.class.parent_prefixes + parent_prefixes.dup.unshift(controller_path) + end end private @@ -156,7 +173,7 @@ module AbstractController end if (options.keys & [:partial, :file, :template, :once]).empty? - options[:prefix] ||= _prefix + options[:prefixes] ||= _prefixes end options[:template] ||= (options[:action] || action_name).to_s diff --git a/actionpack/lib/action_controller/metal/implicit_render.rb b/actionpack/lib/action_controller/metal/implicit_render.rb index 282dcf66b3..cfa7004048 100644 --- a/actionpack/lib/action_controller/metal/implicit_render.rb +++ b/actionpack/lib/action_controller/metal/implicit_render.rb @@ -12,10 +12,10 @@ module ActionController def method_for_action(action_name) super || begin - if template_exists?(action_name.to_s, _prefix) + if template_exists?(action_name.to_s, _prefixes) "default_render" end end end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_controller/metal/responder.rb b/actionpack/lib/action_controller/metal/responder.rb index 38d32211cc..cffef29f83 100644 --- a/actionpack/lib/action_controller/metal/responder.rb +++ b/actionpack/lib/action_controller/metal/responder.rb @@ -115,7 +115,7 @@ module ActionController #:nodoc: # Main entry point for responder responsible to dispatch to the proper format. # def respond - method = :"to_#{format}" + method = "to_#{format}" respond_to?(method) ? send(method) : to_format end @@ -171,7 +171,7 @@ module ActionController #:nodoc: # Checks whether the resource responds to the current format or not. # def resourceful? - resource.respond_to?(:"to_#{format}") + resource.respond_to?("to_#{format}") end # Returns the resource location by retrieving it from the options or diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb index f1ea14fc29..ab8c6259c5 100644 --- a/actionpack/lib/action_view/base.rb +++ b/actionpack/lib/action_view/base.rb @@ -232,8 +232,8 @@ module ActionView #:nodoc: @controller_path ||= controller && controller.controller_path end - def controller_prefix - @controller_prefix ||= controller && controller._prefix + def controller_prefixes + @controller_prefixes ||= controller && controller._prefixes end ActiveSupport.run_load_hooks(:action_view, self) diff --git a/actionpack/lib/action_view/lookup_context.rb b/actionpack/lib/action_view/lookup_context.rb index d524c68450..1365048724 100644 --- a/actionpack/lib/action_view/lookup_context.rb +++ b/actionpack/lib/action_view/lookup_context.rb @@ -78,17 +78,17 @@ module ActionView @view_paths = ActionView::Base.process_view_paths(paths) end - def find(name, prefix = nil, partial = false, keys = []) - @view_paths.find(*args_for_lookup(name, prefix, partial, keys)) + def find(name, prefixes = [], partial = false, keys = []) + @view_paths.find(*args_for_lookup(name, prefixes, partial, keys)) end alias :find_template :find - def find_all(name, prefix = nil, partial = false, keys = []) - @view_paths.find_all(*args_for_lookup(name, prefix, partial, keys)) + def find_all(name, prefixes = [], partial = false, keys = []) + @view_paths.find_all(*args_for_lookup(name, prefixes, partial, keys)) end - def exists?(name, prefix = nil, partial = false, keys = []) - @view_paths.exists?(*args_for_lookup(name, prefix, partial, keys)) + def exists?(name, prefixes = [], partial = false, keys = []) + @view_paths.exists?(*args_for_lookup(name, prefixes, partial, keys)) end alias :template_exists? :exists? @@ -107,18 +107,26 @@ module ActionView protected - def args_for_lookup(name, prefix, partial, keys) #:nodoc: - name, prefix = normalize_name(name, prefix) - [name, prefix, partial || false, @details, details_key, keys] + def args_for_lookup(name, prefixes, partial, keys) #:nodoc: + name, prefixes = normalize_name(name, prefixes) + [name, prefixes, partial || false, @details, details_key, keys] end # Support legacy foo.erb names even though we now ignore .erb # as well as incorrectly putting part of the path in the template # name instead of the prefix. - def normalize_name(name, prefix) #:nodoc: + def normalize_name(name, prefixes) #:nodoc: name = name.to_s.gsub(handlers_regexp, '') parts = name.split('/') - return parts.pop, [prefix, *parts].compact.join("/") + name = parts.pop + + prefixes = if prefixes.blank? + [parts.join('/')] + else + prefixes.map { |prefix| [prefix, *parts].compact.join('/') } + end + + return name, prefixes end def default_handlers #:nodoc: @@ -237,4 +245,4 @@ module ActionView include Details include ViewPaths end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_view/path_set.rb b/actionpack/lib/action_view/path_set.rb index fa35120a0d..e3de3e1eac 100644 --- a/actionpack/lib/action_view/path_set.rb +++ b/actionpack/lib/action_view/path_set.rb @@ -11,13 +11,15 @@ module ActionView #:nodoc: end def find(*args) - find_all(*args).first || raise(MissingTemplate.new(self, "#{args[1]}/#{args[0]}", args[3], args[2])) + find_all(*args).first || raise(MissingTemplate.new(self, *args)) end - def find_all(*args) - each do |resolver| - templates = resolver.find_all(*args) - return templates unless templates.empty? + def find_all(path, prefixes = [], *args) + prefixes.each do |prefix| + each do |resolver| + templates = resolver.find_all(path, prefix, *args) + return templates unless templates.empty? + end end [] end diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb index 317479ad4c..3fdea23a4a 100644 --- a/actionpack/lib/action_view/renderer/partial_renderer.rb +++ b/actionpack/lib/action_view/renderer/partial_renderer.rb @@ -111,8 +111,8 @@ module ActionView end def find_template(path=@path, locals=@locals.keys) - prefix = @view.controller_prefix unless path.include?(?/) - @lookup_context.find_template(path, prefix, true, locals) + prefixes = path.include?(?/) ? [] : @view.controller_prefixes + @lookup_context.find_template(path, prefixes, true, locals) end def collection_with_template @@ -152,7 +152,7 @@ module ActionView object = object.to_model if object.respond_to?(:to_model) object.class.model_name.partial_path.dup.tap do |partial| - path = @view.controller_prefix + path = @view.controller_prefixes.first partial.insert(0, "#{File.dirname(path)}/") if partial.include?(?/) && path.include?(?/) end end diff --git a/actionpack/lib/action_view/renderer/template_renderer.rb b/actionpack/lib/action_view/renderer/template_renderer.rb index ece3f621b6..9ae1636131 100644 --- a/actionpack/lib/action_view/renderer/template_renderer.rb +++ b/actionpack/lib/action_view/renderer/template_renderer.rb @@ -22,14 +22,14 @@ module ActionView def render_once(options) paths, locals = options[:once], options[:locals] || {} layout, keys = options[:layout], locals.keys - prefix = options.fetch(:prefix, @view.controller_prefix) + prefixes = options.fetch(:prefixes, @view.controller_prefixes) raise "render :once expects a String or an Array to be given" unless paths render_with_layout(layout, locals) do contents = [] Array.wrap(paths).each do |path| - template = find_template(path, prefix, false, keys) + template = find_template(path, prefixes, false, keys) contents << render_template(template, nil, locals) if @rendered.add?(template) end contents.join("\n") @@ -43,13 +43,13 @@ module ActionView if options.key?(:text) Template::Text.new(options[:text], formats.try(:first)) elsif options.key?(:file) - with_fallbacks { find_template(options[:file], options[:prefix], false, keys) } + with_fallbacks { find_template(options[:file], options[:prefixes], false, keys) } elsif options.key?(:inline) handler = Template.handler_for_extension(options[:type] || "erb") Template.new(options[:inline], "inline template", handler, :locals => keys) elsif options.key?(:template) options[:template].respond_to?(:render) ? - options[:template] : find_template(options[:template], options[:prefix], false, keys) + options[:template] : find_template(options[:template], options[:prefixes], false, keys) end end diff --git a/actionpack/lib/action_view/template.rb b/actionpack/lib/action_view/template.rb index 0d8373ef14..80543a8b77 100644 --- a/actionpack/lib/action_view/template.rb +++ b/actionpack/lib/action_view/template.rb @@ -163,7 +163,7 @@ module ActionView name = pieces.pop partial = !!name.sub!(/^_/, "") lookup.disable_cache do - lookup.find_template(name, pieces.join('/'), partial, @locals) + lookup.find_template(name, [ pieces.join('/') ], partial, @locals) end end diff --git a/actionpack/lib/action_view/template/error.rb b/actionpack/lib/action_view/template/error.rb index ff256738a9..d7d98e1dd5 100644 --- a/actionpack/lib/action_view/template/error.rb +++ b/actionpack/lib/action_view/template/error.rb @@ -27,7 +27,7 @@ module ActionView class MissingTemplate < ActionViewError #:nodoc: attr_reader :path - def initialize(paths, path, details, partial) + def initialize(paths, path, prefixes, partial, details, *) @path = path display_paths = paths.compact.map{ |p| p.to_s.inspect }.join(", ") template_type = if partial @@ -38,7 +38,11 @@ module ActionView 'template' end - super("Missing #{template_type} #{path} with #{details.inspect} in view paths #{display_paths}") + searched_paths = prefixes.map { |prefix| [prefix, path].join("/") } + + out = "Missing #{template_type} #{searched_paths.join(", ")} with #{details.inspect}. Searched in:\n" + out += paths.compact.map { |p| " * #{p.to_s.inspect}\n" }.join + super out end end diff --git a/actionpack/lib/action_view/template/resolver.rb b/actionpack/lib/action_view/template/resolver.rb index 0dccc99d14..d23aa5ef85 100644 --- a/actionpack/lib/action_view/template/resolver.rb +++ b/actionpack/lib/action_view/template/resolver.rb @@ -47,7 +47,7 @@ module ActionView path end - # Hnadles templates caching. If a key is given and caching is on + # Handles templates caching. If a key is given and caching is on # always check the cache before hitting the resolver. Otherwise, # it always hits the resolver but check if the resolver is fresher # before returning it. diff --git a/actionpack/lib/action_view/testing/resolvers.rb b/actionpack/lib/action_view/testing/resolvers.rb index 55583096e0..5c5cab7c7d 100644 --- a/actionpack/lib/action_view/testing/resolvers.rb +++ b/actionpack/lib/action_view/testing/resolvers.rb @@ -13,7 +13,11 @@ module ActionView #:nodoc: @hash = hash end - private + def to_s + @hash.keys.join(', ') + end + + private def query(path, exts, formats) query = "" diff --git a/actionpack/test/abstract/abstract_controller_test.rb b/actionpack/test/abstract/abstract_controller_test.rb index 19855490b4..981c023d38 100644 --- a/actionpack/test/abstract/abstract_controller_test.rb +++ b/actionpack/test/abstract/abstract_controller_test.rb @@ -30,14 +30,14 @@ module AbstractController class RenderingController < AbstractController::Base include ::AbstractController::Rendering - def _prefix() end + def _prefixes + [] + end def render(options = {}) if options.is_a?(String) options = {:_template_name => options} end - - options[:_prefix] = _prefix super end @@ -116,8 +116,8 @@ module AbstractController name.underscore end - def _prefix - self.class.prefix + def _prefixes + [self.class.prefix] end end @@ -157,10 +157,10 @@ module AbstractController private def self.layout(formats) begin - find_template(name.underscore, {:formats => formats}, :_prefix => "layouts") + find_template(name.underscore, {:formats => formats}, :_prefixes => ["layouts"]) rescue ActionView::MissingTemplate begin - find_template("application", {:formats => formats}, :_prefix => "layouts") + find_template("application", {:formats => formats}, :_prefixes => ["layouts"]) rescue ActionView::MissingTemplate end end diff --git a/actionpack/test/abstract/render_test.rb b/actionpack/test/abstract/render_test.rb index 25dc8bd804..b9293d1241 100644 --- a/actionpack/test/abstract/render_test.rb +++ b/actionpack/test/abstract/render_test.rb @@ -6,8 +6,8 @@ module AbstractController class ControllerRenderer < AbstractController::Base include AbstractController::Rendering - def _prefix - "renderer" + def _prefixes + %w[renderer] end self.view_paths = [ActionView::FixtureResolver.new( diff --git a/actionpack/test/controller/mime_responds_test.rb b/actionpack/test/controller/mime_responds_test.rb index 98c9d43b93..fe14d24327 100644 --- a/actionpack/test/controller/mime_responds_test.rb +++ b/actionpack/test/controller/mime_responds_test.rb @@ -565,7 +565,7 @@ class RespondWithController < ActionController::Base def using_resource_with_action respond_with(resource, :action => :foo) do |format| - format.html { raise ActionView::MissingTemplate.new([], "foo/bar", {}, false) } + format.html { raise ActionView::MissingTemplate.new([], "bar", ["foo"], {}, false) } end end diff --git a/actionpack/test/controller/new_base/render_once_test.rb b/actionpack/test/controller/new_base/render_once_test.rb index 3035ed4ff2..175abf8a7e 100644 --- a/actionpack/test/controller/new_base/render_once_test.rb +++ b/actionpack/test/controller/new_base/render_once_test.rb @@ -18,8 +18,8 @@ module RenderTemplate self.view_paths = [RESOLVER] - def _prefix - "test" + def _prefixes + %w(test) end def multiple @@ -39,11 +39,11 @@ module RenderTemplate end def with_prefix - render :once => "result", :prefix => "other" + render :once => "result", :prefixes => %w(other) end def with_nil_prefix - render :once => "test/result", :prefix => nil + render :once => "test/result", :prefixes => [] end end diff --git a/actionpack/test/controller/new_base/render_partial_test.rb b/actionpack/test/controller/new_base/render_partial_test.rb index d800ea264d..83b0d039ad 100644 --- a/actionpack/test/controller/new_base/render_partial_test.rb +++ b/actionpack/test/controller/new_base/render_partial_test.rb @@ -9,7 +9,10 @@ module RenderPartial "render_partial/basic/basic.html.erb" => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'basic' %><%= @test_unchanged %>", "render_partial/basic/with_json.html.erb" => "<%= render 'with_json.json' %>", "render_partial/basic/_with_json.json.erb" => "<%= render 'final' %>", - "render_partial/basic/_final.json.erb" => "{ final: json }" + "render_partial/basic/_final.json.erb" => "{ final: json }", + "render_partial/basic/overriden.html.erb" => "<%= @test_unchanged = 'goodbye' %><%= render :partial => 'overriden' %><%= @test_unchanged %>", + "render_partial/basic/_overriden.html.erb" => "ParentPartial!", + "render_partial/child/_overriden.html.erb" => "OverridenPartial!" )] def html_with_json_inside_json @@ -20,7 +23,13 @@ module RenderPartial @test_unchanged = 'hello' render :action => "basic" end + + def overriden + @test_unchanged = 'hello' + end end + + class ChildController < BasicController; end class TestPartial < Rack::TestCase testing BasicController @@ -37,4 +46,18 @@ module RenderPartial end end + class TestInheritedPartial < Rack::TestCase + testing ChildController + + test "partial from parent controller gets picked if missing in child one" do + get :changing + assert_response("goodbyeBasicPartial!goodbye") + end + + test "partial from child controller gets picked" do + get :overriden + assert_response("goodbyeOverridenPartial!goodbye") + end + end + end diff --git a/actionpack/test/controller/new_base/render_test.rb b/actionpack/test/controller/new_base/render_test.rb index df97a2725b..d6062bfa8c 100644 --- a/actionpack/test/controller/new_base/render_test.rb +++ b/actionpack/test/controller/new_base/render_test.rb @@ -6,7 +6,11 @@ module Render "render/blank_render/index.html.erb" => "Hello world!", "render/blank_render/access_request.html.erb" => "The request: <%= request.method.to_s.upcase %>", "render/blank_render/access_action_name.html.erb" => "Action Name: <%= action_name %>", - "render/blank_render/access_controller_name.html.erb" => "Controller Name: <%= controller_name %>" + "render/blank_render/access_controller_name.html.erb" => "Controller Name: <%= controller_name %>", + "render/blank_render/overriden_with_own_view_paths_appended.html.erb" => "parent content", + "render/blank_render/overriden_with_own_view_paths_prepended.html.erb" => "parent content", + "render/blank_render/overriden.html.erb" => "parent content", + "render/child_render/overriden.html.erb" => "child content" )] def index @@ -21,6 +25,15 @@ module Render render :action => "access_action_name" end + def overriden_with_own_view_paths_appended + end + + def overriden_with_own_view_paths_prepended + end + + def overriden + end + private def secretz @@ -35,6 +48,11 @@ module Render end end + class ChildRenderController < BlankRenderController + append_view_path ActionView::FixtureResolver.new("render/child_render/overriden_with_own_view_paths_appended.html.erb" => "child content") + prepend_view_path ActionView::FixtureResolver.new("render/child_render/overriden_with_own_view_paths_prepended.html.erb" => "child content") + end + class RenderTest < Rack::TestCase test "render with blank" do with_routing do |set| @@ -94,4 +112,26 @@ module Render assert_body "Controller Name: blank_render" end end + + class TestViewInheritance < Rack::TestCase + test "Template from child controller gets picked over parent one" do + get "/render/child_render/overriden" + assert_body "child content" + end + + test "Template from child controller with custom view_paths prepended gets picked over parent one" do + get "/render/child_render/overriden_with_own_view_paths_prepended" + assert_body "child content" + end + + test "Template from child controller with custom view_paths appended gets picked over parent one" do + get "/render/child_render/overriden_with_own_view_paths_appended" + assert_body "child content" + end + + test "Template from parent controller gets picked if missing in child controller" do + get "/render/child_render/index" + assert_body "Hello world!" + end + end end diff --git a/actionpack/test/template/log_subscriber_test.rb b/actionpack/test/template/log_subscriber_test.rb index 435936b19f..8b8b005a1d 100644 --- a/actionpack/test/template/log_subscriber_test.rb +++ b/actionpack/test/template/log_subscriber_test.rb @@ -57,7 +57,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase end def test_render_partial_with_implicit_path - @view.stubs(:controller_prefix).returns("test") + @view.stubs(:controller_prefixes).returns(%w(test)) @view.render(Customer.new("david"), :greeting => "hi") wait @@ -74,7 +74,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase end def test_render_collection_with_implicit_path - @view.stubs(:controller_prefix).returns("test") + @view.stubs(:controller_prefixes).returns(%w(test)) @view.render([ Customer.new("david"), Customer.new("mary") ], :greeting => "hi") wait @@ -83,7 +83,7 @@ class AVLogSubscriberTest < ActiveSupport::TestCase end def test_render_collection_template_without_path - @view.stubs(:controller_prefix).returns("test") + @view.stubs(:controller_prefixes).returns(%w(test)) @view.render([ GoodCustomer.new("david"), Customer.new("mary") ], :greeting => "hi") wait diff --git a/actionpack/test/template/lookup_context_test.rb b/actionpack/test/template/lookup_context_test.rb index c9dd27cf2a..a629b3c046 100644 --- a/actionpack/test/template/lookup_context_test.rb +++ b/actionpack/test/template/lookup_context_test.rb @@ -80,18 +80,18 @@ class LookupContextTest < ActiveSupport::TestCase end test "find templates using the given view paths and configured details" do - template = @lookup_context.find("hello_world", "test") + template = @lookup_context.find("hello_world", %w(test)) assert_equal "Hello world!", template.source @lookup_context.locale = :da - template = @lookup_context.find("hello_world", "test") + template = @lookup_context.find("hello_world", %w(test)) assert_equal "Hey verden", template.source end test "found templates respects given formats if one cannot be found from template or handler" do ActionView::Template::Handlers::ERB.expects(:default_format).returns(nil) @lookup_context.formats = [:text] - template = @lookup_context.find("hello_world", "test") + template = @lookup_context.find("hello_world", %w(test)) assert_equal [:text], template.formats end @@ -137,44 +137,44 @@ class LookupContextTest < ActiveSupport::TestCase test "gives the key forward to the resolver, so it can be used as cache key" do @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now we are going to change the template, but it won't change the returned template # since we will hit the cache. @lookup_context.view_paths.first.hash["test/_foo.erb"] = "Bar" - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # This time we will change the locale. The updated template should be picked since # lookup_context generated a new key after we changed the locale. @lookup_context.locale = :da - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Bar", template.source # Now we will change back the locale and it will still pick the old template. # This is expected because lookup_context will reuse the previous key for :en locale. @lookup_context.locale = :en - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Finally, we can expire the cache. And the expected template will be used. @lookup_context.view_paths.first.clear_cache - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Bar", template.source end test "can disable the cache on demand" do @lookup_context.view_paths = ActionView::FixtureResolver.new("test/_foo.erb" => "Foo") - old_template = @lookup_context.find("foo", "test", true) + old_template = @lookup_context.find("foo", %w(test), true) - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal template, old_template assert @lookup_context.cache template = @lookup_context.disable_cache do assert !@lookup_context.cache - @lookup_context.find("foo", "test", true) + @lookup_context.find("foo", %w(test), true) end assert @lookup_context.cache @@ -182,11 +182,11 @@ class LookupContextTest < ActiveSupport::TestCase end test "data can be stored in cached templates" do - template = @lookup_context.find("hello_world", "test") + template = @lookup_context.find("hello_world", %w(test)) template.data["cached"] = "data" assert_equal "Hello world!", template.source - template = @lookup_context.find("hello_world", "test") + template = @lookup_context.find("hello_world", %w(test)) assert_equal "data", template.data["cached"] assert_equal "Hello world!", template.source end @@ -200,54 +200,54 @@ class LookupContextWithFalseCaching < ActiveSupport::TestCase end test "templates are always found in the resolver but timestamp is checked before being compiled" do - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now we are going to change the template, but it won't change the returned template # since the timestamp is the same. @resolver.hash["test/_foo.erb"][0] = "Bar" - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source # Now update the timestamp. @resolver.hash["test/_foo.erb"][1] = Time.now.utc - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Bar", template.source end test "if no template was found in the second lookup, with no cache, raise error" do - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source @resolver.hash.clear assert_raise ActionView::MissingTemplate do - @lookup_context.find("foo", "test", true) + @lookup_context.find("foo", %w(test), true) end end test "if no template was cached in the first lookup, retrieval should work in the second call" do @resolver.hash.clear assert_raise ActionView::MissingTemplate do - @lookup_context.find("foo", "test", true) + @lookup_context.find("foo", %w(test), true) end @resolver.hash["test/_foo.erb"] = ["Foo", Time.utc(2000)] - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "Foo", template.source end test "data can be stored as long as template was not updated" do - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) template.data["cached"] = "data" assert_equal "Foo", template.source - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_equal "data", template.data["cached"] assert_equal "Foo", template.source @resolver.hash["test/_foo.erb"][1] = Time.now.utc - template = @lookup_context.find("foo", "test", true) + template = @lookup_context.find("foo", %w(test), true) assert_nil template.data["cached"] assert_equal "Foo", template.source end -end
\ No newline at end of file +end diff --git a/actionpack/test/template/template_test.rb b/actionpack/test/template/template_test.rb index 2ec640c84c..3432a02c3c 100644 --- a/actionpack/test/template/template_test.rb +++ b/actionpack/test/template/template_test.rb @@ -95,14 +95,14 @@ class TestERBTemplate < ActiveSupport::TestCase def test_refresh_with_templates @template = new_template("Hello", :virtual_path => "test/foo/bar") @template.locals = [:key] - @context.lookup_context.expects(:find_template).with("bar", "test/foo", false, [:key]).returns("template") + @context.lookup_context.expects(:find_template).with("bar", %w(test/foo), false, [:key]).returns("template") assert_equal "template", @template.refresh(@context) end def test_refresh_with_partials @template = new_template("Hello", :virtual_path => "test/_foo") @template.locals = [:key] - @context.lookup_context.expects(:find_template).with("foo", "test", true, [:key]).returns("partial") + @context.lookup_context.expects(:find_template).with("foo", %w(test), true, [:key]).returns("partial") assert_equal "partial", @template.refresh(@context) end diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 33454123ce..67f88b4088 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -325,7 +325,7 @@ module ActiveModel options.symbolize_keys! @prefix, @suffix = options[:prefix] || '', options[:suffix] || '' @regex = /^(#{Regexp.escape(@prefix)})(.+?)(#{Regexp.escape(@suffix)})$/ - @method_missing_target = :"#{@prefix}attribute#{@suffix}" + @method_missing_target = "#{@prefix}attribute#{@suffix}" @method_name = "#{prefix}%s#{suffix}" end diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index aaa41f5ec6..7302a869c8 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -102,7 +102,7 @@ module ActiveModel define_callbacks(callback, options) types.each do |type| - send(:"_define_#{type}_model_callback", self, callback) + send("_define_#{type}_model_callback", self, callback) end end end diff --git a/activemodel/lib/active_model/secure_password.rb b/activemodel/lib/active_model/secure_password.rb index 52941942b8..7e8370a04c 100644 --- a/activemodel/lib/active_model/secure_password.rb +++ b/activemodel/lib/active_model/secure_password.rb @@ -37,22 +37,26 @@ module ActiveModel validates_confirmation_of :password validates_presence_of :password_digest + + include InstanceMethodsOnActivation end end - # Returns self if the password is correct, otherwise false. - def authenticate(unencrypted_password) - if BCrypt::Password.new(password_digest) == unencrypted_password - self - else - false + module InstanceMethodsOnActivation + # Returns self if the password is correct, otherwise false. + def authenticate(unencrypted_password) + if BCrypt::Password.new(password_digest) == unencrypted_password + self + else + false + end end - end - # Encrypts the password into the password_digest attribute. - def password=(unencrypted_password) - @password = unencrypted_password - self.password_digest = BCrypt::Password.create(unencrypted_password) + # Encrypts the password into the password_digest attribute. + def password=(unencrypted_password) + @password = unencrypted_password + self.password_digest = BCrypt::Password.create(unencrypted_password) + end end end end diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 6d905fe6fe..ecf7b6c210 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -127,8 +127,7 @@ module ActiveRecord association_proxy = parent_record.send(reflection_name) association_proxy.loaded association_proxy.target.push(*Array.wrap(associated_record)) - - association_proxy.__send__(:set_inverse_instance, associated_record, parent_record) + association_proxy.send(:set_inverse_instance, associated_record) end end @@ -148,16 +147,19 @@ module ActiveRecord def set_association_single_records(id_to_record_map, reflection_name, associated_records, key) seen_keys = {} associated_records.each do |associated_record| + seen_key = associated_record[key].to_s + #this is a has_one or belongs_to: there should only be one record. #Unfortunately we can't (in portable way) ask the database for #'all records where foo_id in (x,y,z), but please # only one row per distinct foo_id' so this where we enforce that - next if seen_keys[associated_record[key].to_s] - seen_keys[associated_record[key].to_s] = true - mapped_records = id_to_record_map[associated_record[key].to_s] + next if seen_keys.key? seen_key + + seen_keys[seen_key] = true + mapped_records = id_to_record_map[seen_key] mapped_records.each do |mapped_record| association_proxy = mapped_record.send("set_#{reflection_name}_target", associated_record) - association_proxy.__send__(:set_inverse_instance, associated_record, mapped_record) + association_proxy.send(:set_inverse_instance, associated_record) end end @@ -188,19 +190,17 @@ module ActiveRecord left = reflection.klass.arel_table - table_name = reflection.klass.quoted_table_name id_to_record_map, ids = construct_id_map(records) records.each {|record| record.send(reflection.name).loaded} options = reflection.options - conditions = "t0.#{reflection.primary_key_name} #{in_or_equals_for_ids(ids)}" - conditions << append_conditions(reflection, preload_options) - right = Arel::Table.new(options[:join_table]).alias('t0') - condition = left[reflection.klass.primary_key].eq( + + + join_condition = left[reflection.klass.primary_key].eq( right[reflection.association_foreign_key]) - join = left.create_join(right, left.create_on(condition)) + join = left.create_join(right, left.create_on(join_condition)) select = [ # FIXME: options[:select] is always nil in the tests. Do we really # need it? @@ -216,8 +216,16 @@ module ActiveRecord associated_records_proxy.joins_values = [join] associated_records_proxy.select_values = select + custom_conditions = append_conditions(reflection, preload_options) + all_associated_records = associated_records(ids) do |some_ids| - associated_records_proxy.where([conditions, some_ids]).to_a + method = in_or_equal(some_ids) + conditions = right[reflection.primary_key_name].send(*method) + conditions = custom_conditions.inject(conditions) do |ast, cond| + ast.and cond + end + + associated_records_proxy.where(conditions).to_a end set_association_collection_records(id_to_record_map, reflection.name, all_associated_records, 'the_parent_record_id') @@ -324,61 +332,54 @@ module ActiveRecord if klass = record.send(polymorph_type) klass_id = record.send(primary_key_name) if klass_id - id_map = klasses_and_ids[klass] ||= {} + id_map = klasses_and_ids[klass.constantize] ||= {} (id_map[klass_id.to_s] ||= []) << record end end end else - id_map = {} - records.each do |record| + id_map = records.group_by do |record| key = record.send(primary_key_name) - (id_map[key.to_s] ||= []) << record if key + key && key.to_s end - klasses_and_ids[reflection.klass.name] = id_map unless id_map.empty? + id_map.delete nil + klasses_and_ids[reflection.klass] = id_map unless id_map.empty? end - klasses_and_ids.each do |klass_name, _id_map| - klass = klass_name.constantize - - table_name = klass.quoted_table_name + klasses_and_ids.each do |klass, _id_map| + table = klass.arel_table primary_key = (reflection.options[:primary_key] || klass.primary_key).to_s - column_type = klass.columns.detect{|c| c.name == primary_key}.type - - ids = _id_map.keys.map do |id| - if column_type == :integer - id.to_i - elsif column_type == :float - id.to_f - else - id - end - end + method = in_or_equal(_id_map.keys) + conditions = table[primary_key].send(*method) - conditions = "#{table_name}.#{connection.quote_column_name(primary_key)} #{in_or_equals_for_ids(ids)}" - conditions << append_conditions(reflection, preload_options) + custom_conditions = append_conditions(reflection, preload_options) + conditions = custom_conditions.inject(conditions) do |ast, cond| + ast.and cond + end - associated_records = klass.unscoped.where([conditions, ids]).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a + associated_records = klass.unscoped.where(conditions).apply_finder_options(options.slice(:include, :select, :joins, :order)).to_a set_association_single_records(_id_map, reflection.name, associated_records, primary_key) end end def find_associated_records(ids, reflection, preload_options) - options = reflection.options - table_name = reflection.klass.quoted_table_name + options = reflection.options + table = reflection.klass.arel_table + + conditions = [] + + key = reflection.primary_key_name if interface = reflection.options[:as] - conditions = "#{reflection.klass.quoted_table_name}.#{connection.quote_column_name "#{interface}_id"} #{in_or_equals_for_ids(ids)} and #{reflection.klass.quoted_table_name}.#{connection.quote_column_name "#{interface}_type"} = '#{self.base_class.sti_name}'" - else - foreign_key = reflection.primary_key_name - conditions = "#{reflection.klass.quoted_table_name}.#{foreign_key} #{in_or_equals_for_ids(ids)}" + key = "#{interface}_id" + conditions << table["#{interface}_type"].eq(base_class.sti_name) end - conditions << append_conditions(reflection, preload_options) + conditions += append_conditions(reflection, preload_options) find_options = { - :select => preload_options[:select] || options[:select] || Arel::SqlLiteral.new("#{table_name}.*"), + :select => preload_options[:select] || options[:select] || table[Arel.star], :include => preload_options[:include] || options[:include], :joins => options[:joins], :group => preload_options[:group] || options[:group], @@ -386,19 +387,24 @@ module ActiveRecord } associated_records(ids) do |some_ids| - reflection.klass.scoped.apply_finder_options(find_options.merge(:conditions => [conditions, some_ids])).to_a + method = in_or_equal(some_ids) + where = conditions.inject(table[key].send(*method)) do |ast, cond| + ast.and cond + end + + reflection.klass.scoped.apply_finder_options(find_options.merge(:conditions => where)).to_a end end def append_conditions(reflection, preload_options) - sql = "" - sql << " AND (#{reflection.sanitized_conditions})" if reflection.sanitized_conditions - sql << " AND (#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions] - sql + [ + ("(#{reflection.sanitized_conditions})" if reflection.sanitized_conditions), + ("(#{sanitize_sql preload_options[:conditions]})" if preload_options[:conditions]), + ].compact.map { |x| Arel.sql x } end - def in_or_equals_for_ids(ids) - ids.size > 1 ? "IN (?)" : "= ?" + def in_or_equal(ids) + ids.length == 1 ? ['eq', ids.first] : ['in', ids] end # Some databases impose a limit on the number of ids in a list (in Oracle its 1000) diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 1056c51a3d..b49cf8de95 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -120,6 +120,8 @@ module ActiveRecord # So there is no need to eager load them. autoload :AssociationCollection, 'active_record/associations/association_collection' autoload :AssociationProxy, 'active_record/associations/association_proxy' + autoload :HasAssociation, 'active_record/associations/has_association' + autoload :ThroughAssociation, 'active_record/associations/through_association' autoload :BelongsToAssociation, 'active_record/associations/belongs_to_association' autoload :BelongsToPolymorphicAssociation, 'active_record/associations/belongs_to_polymorphic_association' autoload :HasAndBelongsToManyAssociation, 'active_record/associations/has_and_belongs_to_many_association' diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 7964f4fa2b..108e316672 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -18,6 +18,8 @@ module ActiveRecord # If you need to work on all current children, new and existing records, # +load_target+ and the +loaded+ flag are your friends. class AssociationCollection < AssociationProxy #:nodoc: + include HasAssociation + delegate :group, :order, :limit, :joins, :where, :preload, :eager_load, :includes, :from, :lock, :readonly, :having, :to => :scoped def select(select = nil) @@ -111,7 +113,7 @@ module ActiveRecord else build_record(attributes) do |record| block.call(record) if block_given? - set_belongs_to_association_for(record) + set_owner_attributes(record) end end end @@ -123,7 +125,7 @@ module ActiveRecord load_target if @owner.new_record? transaction do - flatten_deeper(records).each do |record| + records.flatten.each do |record| raise_on_type_mismatch(record) add_record_to_target_with_callbacks(record) do |r| result &&= insert_record(record) unless @owner.new_record? @@ -452,9 +454,7 @@ module ActiveRecord end records = @reflection.options[:uniq] ? uniq(records) : records - records.each do |record| - set_inverse_instance(record, @owner) - end + records.each { |record| set_inverse_instance(record) } records end @@ -468,7 +468,7 @@ module ActiveRecord @target << record end callback(:after_add, record) - set_inverse_instance(record, @owner) + set_inverse_instance(record) record end @@ -488,7 +488,7 @@ module ActiveRecord ensure_owner_is_persisted! transaction do - with_scope(:create => @scope[:create].merge(scoped.where_values_hash || {})) do + with_scope(:create => @scope[:create].merge(scoped.where_values_hash)) do build_record(attrs, &block) end end @@ -501,7 +501,7 @@ module ActiveRecord end def remove_records(*records) - records = flatten_deeper(records) + records = records.flatten records.each { |record| raise_on_type_mismatch(record) } transaction do diff --git a/activerecord/lib/active_record/associations/association_proxy.rb b/activerecord/lib/active_record/associations/association_proxy.rb index f4eceeed8c..6720d83199 100644 --- a/activerecord/lib/active_record/associations/association_proxy.rb +++ b/activerecord/lib/active_record/associations/association_proxy.rb @@ -4,17 +4,17 @@ module ActiveRecord module Associations # = Active Record Associations # - # This is the root class of all association proxies: + # This is the root class of all association proxies ('+ Foo' signifies an included module Foo): # # AssociationProxy # BelongsToAssociation - # HasOneAssociation # BelongsToPolymorphicAssociation - # AssociationCollection + # AssociationCollection + HasAssociation # HasAndBelongsToManyAssociation # HasManyAssociation - # HasManyThroughAssociation - # HasOneThroughAssociation + # HasManyThroughAssociation + ThroughAssociation + # HasOneAssociation + HasAssociation + # HasOneThroughAssociation + ThroughAssociation # # Association proxies in Active Record are middlemen between the object that # holds the association, known as the <tt>@owner</tt>, and the actual associated @@ -167,11 +167,6 @@ module ActiveRecord end protected - # Does the association have a <tt>:dependent</tt> option? - def dependent? - @reflection.options[:dependent] - end - def interpolate_sql(sql, record = nil) @owner.send(:interpolate_sql, sql, record) end @@ -181,20 +176,6 @@ module ActiveRecord @reflection.klass.send(:sanitize_sql, sql, table_name) end - # Assigns the ID of the owner to the corresponding foreign key in +record+. - # If the association is polymorphic the type of the owner is also set. - def set_belongs_to_association_for(record) - if @reflection.options[:as] - record["#{@reflection.options[:as]}_id"] = @owner.id if @owner.persisted? - record["#{@reflection.options[:as]}_type"] = @owner.class.base_class.name.to_s - else - if @owner.persisted? - primary_key = @reflection.options[:primary_key] || :id - record[@reflection.primary_key_name] = @owner.send(primary_key) - end - end - end - # Merges into +options+ the ones coming from the reflection. def merge_options_from_reflection!(options) options.reverse_merge!( @@ -232,6 +213,17 @@ module ActiveRecord {} end + def aliased_table + @reflection.klass.arel_table + end + + # Set the inverse association, if possible + def set_inverse_instance(record) + if record && invertible_for?(record) + record.send("set_#{inverse_reflection_for(record).name}_target", @owner) + end + end + private # Forwards any missing method call to the \target. def method_missing(method, *args) @@ -290,34 +282,21 @@ module ActiveRecord end end - if RUBY_VERSION < '1.9.2' - # Array#flatten has problems with recursive arrays before Ruby 1.9.2. - # Going one level deeper solves the majority of the problems. - def flatten_deeper(array) - array.collect { |element| (element.respond_to?(:flatten) && !element.is_a?(Hash)) ? element.flatten : element }.flatten - end - else - def flatten_deeper(array) - array.flatten - end - end - # Returns the ID of the owner, quoted if needed. def owner_quoted_id @owner.quoted_id end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.inverse_of - unless inverse_relationship.nil? - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + # Can be redefined by subclasses, notably polymorphic belongs_to + # The record parameter is necessary to support polymorphic inverses as we must check for + # the association in the specific class of the record. + def inverse_reflection_for(record) + @reflection.inverse_of end - # Override in subclasses - def we_can_set_the_inverse_on_this?(record) - false + # Is this association invertible? Can be redefined by subclasses. + def invertible_for?(record) + inverse_reflection_for(record) end end end diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index bbfe18f9fb..98c1c13524 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -32,7 +32,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record @@ -69,7 +69,7 @@ module ActiveRecord options ) if @owner[@reflection.primary_key_name] end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end @@ -83,8 +83,9 @@ module ActiveRecord # NOTE - for now, we're only supporting inverse setting from belongs_to back onto # has_one associations. - def we_can_set_the_inverse_on_this?(record) - @reflection.has_inverse? && @reflection.inverse_of.macro == :has_one + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def record_id(record) diff --git a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb index c580de7fbe..90eff7399c 100644 --- a/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_polymorphic_association.rb @@ -14,7 +14,7 @@ module ActiveRecord @updated = true end - set_inverse_instance(record, @owner) + set_inverse_instance(record) loaded record end @@ -38,23 +38,13 @@ module ActiveRecord private - # NOTE - for now, we're only supporting inverse setting from belongs_to back onto - # has_one associations. - def we_can_set_the_inverse_on_this?(record) - if @reflection.has_inverse? - inverse_association = @reflection.polymorphic_inverse_of(record.class) - inverse_association && inverse_association.macro == :has_one - else - false - end + def inverse_reflection_for(record) + @reflection.polymorphic_inverse_of(record.class) end - def set_inverse_instance(record, instance) - return if record.nil? || !we_can_set_the_inverse_on_this?(record) - inverse_relationship = @reflection.polymorphic_inverse_of(record.class) - if inverse_relationship - record.send(:"set_#{inverse_relationship.name}_target", instance) - end + def invertible_for?(record) + inverse = inverse_reflection_for(record) + inverse && inverse.macro == :has_one end def construct_find_scope @@ -71,7 +61,7 @@ module ActiveRecord :include => @reflection.options[:include] ) end - set_inverse_instance(target, @owner) + set_inverse_instance(target) target end diff --git a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb index a74d0a4ca8..bb6145656e 100644 --- a/activerecord/lib/active_record/associations/class_methods/join_dependency.rb +++ b/activerecord/lib/active_record/associations/class_methods/join_dependency.rb @@ -212,7 +212,7 @@ module ActiveRecord collection = record.send(join_part.reflection.name) collection.loaded collection.target.push(association) - collection.__send__(:set_inverse_instance, association, record) + collection.send(:set_inverse_instance, association) when :belongs_to set_target_and_inverse(join_part, association, record) else @@ -224,7 +224,7 @@ module ActiveRecord def set_target_and_inverse(join_part, association, record) association_proxy = record.send("set_#{join_part.reflection.name}_target", association) - association_proxy.__send__(:set_inverse_instance, association, record) + association_proxy.send(:set_inverse_instance, association) end end end diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index e17ac6f2cc..b1d454545f 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -75,10 +75,12 @@ module ActiveRecord "INNER JOIN #{@owner.connection.quote_table_name @reflection.options[:join_table]} ON #{@reflection.quoted_table_name}.#{@reflection.klass.primary_key} = #{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.association_foreign_key}" end - def construct_conditions - sql = "#{@owner.connection.quote_table_name @reflection.options[:join_table]}.#{@reflection.primary_key_name} = #{owner_quoted_id} " - sql << " AND (#{conditions})" if conditions - sql + def join_table + Arel::Table.new(@reflection.options[:join_table]) + end + + def construct_owner_conditions + super(join_table) end def construct_find_scope @@ -108,6 +110,10 @@ module ActiveRecord [] end end + + def invertible_for?(record) + false + end end end end diff --git a/activerecord/lib/active_record/associations/has_association.rb b/activerecord/lib/active_record/associations/has_association.rb new file mode 100644 index 0000000000..0ecdb696ea --- /dev/null +++ b/activerecord/lib/active_record/associations/has_association.rb @@ -0,0 +1,42 @@ +module ActiveRecord + module Associations + # Included in all has_* associations (i.e. everything except belongs_to) + module HasAssociation #:nodoc: + protected + # Sets the owner attributes on the given record + def set_owner_attributes(record) + if @owner.persisted? + construct_owner_attributes.each { |key, value| record[key] = value } + end + end + + # Returns a hash linking the owner to the association represented by the reflection + def construct_owner_attributes(reflection = @reflection) + attributes = {} + if reflection.macro == :belongs_to + attributes[reflection.association_primary_key] = @owner.send(reflection.primary_key_name) + else + attributes[reflection.primary_key_name] = @owner.send(reflection.active_record_primary_key) + + if reflection.options[:as] + attributes["#{reflection.options[:as]}_type"] = @owner.class.base_class.name + end + end + attributes + end + + # Builds an array of arel nodes from the owner attributes hash + def construct_owner_conditions(table = aliased_table, reflection = @reflection) + construct_owner_attributes(reflection).map do |attr, value| + table[attr].eq(value) + end + end + + def construct_conditions + conditions = construct_owner_conditions + conditions << Arel.sql(sql_conditions) if sql_conditions + aliased_table.create_and(conditions) + end + end + end +end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 4b05cf6e81..c3360463cd 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -54,7 +54,7 @@ module ActiveRecord end def insert_record(record, force = false, validate = true) - set_belongs_to_association_for(record) + set_owner_attributes(record) save_record(record, force, validate) end @@ -79,22 +79,6 @@ module ActiveRecord end end - def target_obsolete? - false - end - - def construct_conditions - if @reflection.options[:as] - sql = - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " + - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" - else - sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}" - end - sql << " AND (#{conditions})" if conditions - sql - end - def construct_find_scope { :conditions => construct_conditions, @@ -106,13 +90,7 @@ module ActiveRecord end def construct_create_scope - create_scoping = {} - set_belongs_to_association_for(create_scoping) - create_scoping - end - - def we_can_set_the_inverse_on_this?(record) - @reflection.inverse_of + construct_owner_attributes end end end diff --git a/activerecord/lib/active_record/associations/has_many_through_association.rb b/activerecord/lib/active_record/associations/has_many_through_association.rb index 5f4667b4d8..e2b008034e 100644 --- a/activerecord/lib/active_record/associations/has_many_through_association.rb +++ b/activerecord/lib/active_record/associations/has_many_through_association.rb @@ -1,17 +1,16 @@ -require "active_record/associations/through_association_scope" require 'active_support/core_ext/object/blank' module ActiveRecord # = Active Record Has Many Through Association module Associations class HasManyThroughAssociation < HasManyAssociation #:nodoc: - include ThroughAssociationScope + include ThroughAssociation alias_method :new, :build def destroy(*records) transaction do - delete_records(flatten_deeper(records)) + delete_records(records.flatten) super end end @@ -21,9 +20,13 @@ module ActiveRecord # have a size larger than zero, and you need to fetch that collection afterwards, it'll take one fewer # SELECT query if you use #length. def size - return @owner.send(:read_attribute, cached_counter_attribute_name) if has_cached_counter? - return @target.size if loaded? - return count + if has_cached_counter? + @owner.send(:read_attribute, cached_counter_attribute_name) + elsif loaded? + @target.size + else + count + end end protected @@ -51,9 +54,9 @@ module ActiveRecord # TODO - add dependent option support def delete_records(records) - klass = @reflection.through_reflection.klass + through_association = @owner.send(@reflection.through_reflection.name) records.each do |associate| - klass.delete_all(construct_join_attributes(associate)) + through_association.where(construct_join_attributes(associate)).delete_all end end @@ -63,16 +66,8 @@ module ActiveRecord scoped.all end - def has_cached_counter? - @owner.attribute_present?(cached_counter_attribute_name) - end - - def cached_counter_attribute_name - "#{@reflection.name}_count" - end - # NOTE - not sure that we can actually cope with inverses here - def we_can_set_the_inverse_on_this?(record) + def invertible_for?(record) false end end diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index c49fd6e66a..c32aaf986e 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -2,6 +2,8 @@ module ActiveRecord # = Active Record Belongs To Has One Association module Associations class HasOneAssociation < AssociationProxy #:nodoc: + include HasAssociation + def create(attrs = {}, replace_existing = true) new_record(replace_existing) do |reflection| attrs = merge_with_conditions(attrs) @@ -27,7 +29,7 @@ module ActiveRecord load_target unless @target.nil? || @target == obj - if dependent? && !dont_save + if @reflection.options[:dependent] && !dont_save case @reflection.options[:dependent] when :delete @target.delete if @target.persisted? @@ -49,11 +51,11 @@ module ActiveRecord @target = nil else raise_on_type_mismatch(obj) - set_belongs_to_association_for(obj) + set_owner_attributes(obj) @target = (AssociationProxy === obj ? obj.target : obj) end - set_inverse_instance(obj, @owner) + set_inverse_instance(obj) @loaded = true unless !@owner.persisted? || obj.nil? || dont_save @@ -79,26 +81,16 @@ module ActiveRecord the_target = with_scope(:find => @scope[:find]) do @reflection.klass.find(:first, options) end - set_inverse_instance(the_target, @owner) + set_inverse_instance(the_target) the_target end def construct_find_scope - if @reflection.options[:as] - sql = - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_id = #{owner_quoted_id} AND " + - "#{@reflection.quoted_table_name}.#{@reflection.options[:as]}_type = #{@owner.class.quote_value(@owner.class.base_class.name.to_s)}" - else - sql = "#{@reflection.quoted_table_name}.#{@reflection.primary_key_name} = #{owner_quoted_id}" - end - sql << " AND (#{conditions})" if conditions - { :conditions => sql } + { :conditions => construct_conditions } end def construct_create_scope - create_scoping = {} - set_belongs_to_association_for(create_scoping) - create_scoping + construct_owner_attributes end def new_record(replace_existing) @@ -115,17 +107,12 @@ module ActiveRecord else record[@reflection.primary_key_name] = @owner.id if @owner.persisted? self.target = record - set_inverse_instance(record, @owner) + set_inverse_instance(record) end record end - def we_can_set_the_inverse_on_this?(record) - inverse = @reflection.inverse_of - return !inverse.nil? - end - def merge_with_conditions(attrs={}) attrs ||= {} attrs.update(@reflection.options[:conditions]) if @reflection.options[:conditions].is_a?(Hash) diff --git a/activerecord/lib/active_record/associations/has_one_through_association.rb b/activerecord/lib/active_record/associations/has_one_through_association.rb index eb17935d6a..c9ae930e93 100644 --- a/activerecord/lib/active_record/associations/has_one_through_association.rb +++ b/activerecord/lib/active_record/associations/has_one_through_association.rb @@ -1,10 +1,8 @@ -require "active_record/associations/through_association_scope" - module ActiveRecord # = Active Record Has One Through Association module Associations class HasOneThroughAssociation < HasOneAssociation - include ThroughAssociationScope + include ThroughAssociation def replace(new_value) create_through_record(new_value) @@ -13,25 +11,26 @@ module ActiveRecord private - def create_through_record(new_value) #nodoc: - klass = @reflection.through_reflection.klass - - current_object = @owner.send(@reflection.through_reflection.name) + def create_through_record(new_value) + proxy = @owner.send(@reflection.through_reflection.name) || + @owner.send(:association_instance_get, @reflection.through_reflection.name) + record = proxy.target - if current_object - new_value ? current_object.update_attributes(construct_join_attributes(new_value)) : current_object.destroy + if record && !new_value + record.destroy elsif new_value - if @owner.new_record? - self.target = new_value - through_association = @owner.send(:association_instance_get, @reflection.through_reflection.name) - through_association.build(construct_join_attributes(new_value)) + attributes = construct_join_attributes(new_value) + + if record + record.update_attributes(attributes) + elsif @owner.new_record? + proxy.build(attributes) else - @owner.send(@reflection.through_reflection.name, klass.create(construct_join_attributes(new_value))) + proxy.create(attributes) end end end - private def find_target update_stale_state scoped.first diff --git a/activerecord/lib/active_record/associations/through_association_scope.rb b/activerecord/lib/active_record/associations/through_association.rb index e57de84f66..57718285f8 100644 --- a/activerecord/lib/active_record/associations/through_association_scope.rb +++ b/activerecord/lib/active_record/associations/through_association.rb @@ -1,7 +1,7 @@ module ActiveRecord - # = Active Record Through Association Scope + # = Active Record Through Association module Associations - module ThroughAssociationScope + module ThroughAssociation def scoped with_scope(@scope) do @@ -35,8 +35,12 @@ module ActiveRecord } end + # This scope affects the creation of the associated records (not the join records). At the + # moment we only support creating on a :through association when the source reflection is a + # belongs_to. Thus it's not necessary to set a foreign key on the associated record(s), so + # this scope has can legitimately be empty. def construct_create_scope - construct_owner_attributes(@reflection) + { } end def aliased_through_table @@ -47,35 +51,13 @@ module ActiveRecord @reflection.through_reflection.klass.arel_table end - # Build SQL conditions from attributes, qualified by table name. - def construct_conditions - table = aliased_through_table - conditions = construct_owner_attributes(@reflection.through_reflection).map do |attr, value| - table[attr].eq(value) - end - conditions << Arel.sql(sql_conditions) if sql_conditions - table.create_and(conditions) - end - - # Associate attributes pointing to owner - def construct_owner_attributes(reflection) - if as = reflection.options[:as] - { "#{as}_id" => @owner[reflection.active_record_primary_key], - "#{as}_type" => @owner.class.base_class.name } - elsif reflection.macro == :belongs_to - { reflection.klass.primary_key => @owner[reflection.primary_key_name] } - else - { reflection.primary_key_name => @owner[reflection.active_record_primary_key] } - end + def construct_owner_conditions + super(aliased_through_table, @reflection.through_reflection) end - def construct_from - @reflection.table_name - end - - def construct_select(custom_select = nil) - distinct = "DISTINCT #{@reflection.quoted_table_name}.*" if @reflection.options[:uniq] - custom_select || @reflection.options[:select] || distinct + def construct_select + @reflection.options[:select] || + @reflection.options[:uniq] && "DISTINCT #{@reflection.quoted_table_name}.*" end def construct_joins @@ -117,12 +99,10 @@ module ActiveRecord # TODO: revisit this to allow it for deletion, supposing dependent option is supported raise ActiveRecord::HasManyThroughCantAssociateThroughHasOneOrManyReflection.new(@owner, @reflection) if [:has_one, :has_many].include?(@reflection.source_reflection.macro) - join_attributes = construct_owner_attributes(@reflection.through_reflection) - - join_attributes.merge!( + join_attributes = { @reflection.source_reflection.primary_key_name => associate.send(@reflection.source_reflection.association_primary_key) - ) + } if @reflection.options[:source_type] join_attributes.merge!(@reflection.source_reflection.options[:foreign_type] => associate.class.base_class.name) diff --git a/activerecord/lib/active_record/attribute_methods/primary_key.rb b/activerecord/lib/active_record/attribute_methods/primary_key.rb index b891b2c50e..978cd7fbe3 100644 --- a/activerecord/lib/active_record/attribute_methods/primary_key.rb +++ b/activerecord/lib/active_record/attribute_methods/primary_key.rb @@ -14,11 +14,13 @@ module ActiveRecord # Defines the primary key field -- can be overridden in subclasses. Overwriting will negate any effect of the # primary_key_prefix_type setting, though. def primary_key - reset_primary_key + @primary_key ||= reset_primary_key end def reset_primary_key #:nodoc: - key = get_primary_key(base_class.name) + key = self == base_class ? get_primary_key(base_class.name) : + base_class.primary_key + set_primary_key(key) key end @@ -40,6 +42,9 @@ module ActiveRecord end end + attr_accessor :original_primary_key + attr_writer :primary_key + # Sets the name of the primary key column to use to the given value, # or (if the value is nil or false) to the value returned by the given # block. @@ -48,9 +53,11 @@ module ActiveRecord # set_primary_key "sysid" # end def set_primary_key(value = nil, &block) - define_attr_method :primary_key, value, &block + @primary_key ||= '' + self.original_primary_key = @primary_key + value &&= value.to_s + self.primary_key = block_given? ? instance_eval(&block) : value end - alias :primary_key= :set_primary_key end end end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 6aac25ba9b..396ab226a9 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1490,8 +1490,10 @@ MSG attributes.each do |k, v| if k.include?("(") multi_parameter_attributes << [ k, v ] + elsif respond_to?("#{k}=") + send("#{k}=", v) else - respond_to?(:"#{k}=") ? send(:"#{k}=", v) : raise(UnknownAttributeError, "unknown attribute: #{k}") + raise(UnknownAttributeError, "unknown attribute: #{k}") end end @@ -1628,7 +1630,7 @@ MSG # Returns the contents of the record as a nicely formatted string. def inspect attributes_as_nice_string = self.class.column_names.collect { |name| - if has_attribute?(name) || new_record? + if has_attribute?(name) "#{name}: #{attribute_for_inspect(name)}" end }.compact.join(", ") @@ -1816,7 +1818,7 @@ MSG if scope = self.class.send(:current_scoped_methods) create_with = scope.scope_for_create create_with.each { |att,value| - respond_to?(:"#{att}=") && send("#{att}=", value) + respond_to?("#{att}=") && send("#{att}=", value) } end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb index a7a12faac2..7489e88eef 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/quoting.rb @@ -33,8 +33,9 @@ module ActiveRecord when BigDecimal then value.to_s('F') when Numeric then value.to_s when Date, Time then "'#{quoted_date(value)}'" + when Symbol then "'#{quote_string(value.to_s)}'" else - "'#{quote_string(value.to_s)}'" + "'#{quote_string(value.to_yaml)}'" end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 9ac8fcb176..b05957981d 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -258,11 +258,11 @@ module ActiveRecord # Creates a record with values matching those of the instance attributes # and returns its id. def create - if self.id.nil? && connection.prefetch_primary_key?(self.class.table_name) + if id.nil? && connection.prefetch_primary_key?(self.class.table_name) self.id = connection.next_sequence_value(self.class.sequence_name) end - attributes_values = arel_attributes_values + attributes_values = arel_attributes_values(!id.nil?) new_id = if attributes_values.empty? self.class.unscoped.insert connection.empty_insert_statement_value @@ -282,7 +282,7 @@ module ActiveRecord # that instances loaded from the database would. def attributes_from_column_definition Hash[self.class.columns.map do |column| - [column.name, column.default] unless column.name == self.class.primary_key + [column.name, column.default] end] end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index d4c5c85594..20e983b5f7 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -166,13 +166,19 @@ module ActiveRecord if conditions || options.present? where(conditions).apply_finder_options(options.slice(:limit, :order)).update_all(updates) else + limit = nil + order = [] # Apply limit and order only if they're both present if @limit_value.present? == @order_values.present? - stmt = arel.compile_update(Arel::SqlLiteral.new(@klass.send(:sanitize_sql_for_assignment, updates))) - @klass.connection.update stmt.to_sql - else - except(:limit, :order).update_all(updates) + limit = arel.limit + order = arel.orders end + + stmt = arel.compile_update(Arel.sql(@klass.send(:sanitize_sql_for_assignment, updates))) + stmt.take limit + stmt.order(*order) + stmt.key = @klass.arel_table[@klass.primary_key] + @klass.connection.update stmt.to_sql end end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 443f318067..868f761a33 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -301,8 +301,8 @@ module ActiveRecord # Save the new record state and id of a record so it can be restored later if a transaction fails. def remember_transaction_record_state #:nodoc @_start_transaction_state ||= {} + @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) unless @_start_transaction_state.include?(:new_record) - @_start_transaction_state[:id] = id if has_attribute?(self.class.primary_key) @_start_transaction_state[:new_record] = @new_record end unless @_start_transaction_state.include?(:destroyed) @@ -329,7 +329,7 @@ module ActiveRecord @attributes = @attributes.dup if @attributes.frozen? @new_record = restore_state[:new_record] @destroyed = restore_state[:destroyed] - if restore_state[:id] + if restore_state.has_key?(:id) self.id = restore_state[:id] else @attributes.delete(self.class.primary_key) diff --git a/activerecord/test/cases/associations/eager_test.rb b/activerecord/test/cases/associations/eager_test.rb index 6efb0d57e4..ad7eb6e4e6 100644 --- a/activerecord/test/cases/associations/eager_test.rb +++ b/activerecord/test/cases/associations/eager_test.rb @@ -886,15 +886,15 @@ class EagerAssociationTest < ActiveRecord::TestCase end def test_preload_has_one_using_primary_key - expected = Firm.find(:first).account_using_primary_key - firm = Firm.find :first, :include => :account_using_primary_key + expected = accounts(:signals37) + firm = Firm.find :first, :include => :account_using_primary_key, :order => 'companies.id' assert_no_queries do assert_equal expected, firm.account_using_primary_key end end def test_include_has_one_using_primary_key - expected = Firm.find(1).account_using_primary_key + expected = accounts(:signals37) firm = Firm.find(:all, :include => :account_using_primary_key, :order => 'accounts.id').detect {|f| f.id == 1} assert_no_queries do assert_equal expected, firm.account_using_primary_key diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index 8214815bde..e3cbae4a84 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -105,7 +105,7 @@ class AttributeMethodsTest < ActiveRecord::TestCase def test_read_attributes_before_type_cast category = Category.new({:name=>"Test categoty", :type => nil}) - category_attrs = {"name"=>"Test categoty", "type" => nil, "categorizations_count" => nil} + category_attrs = {"name"=>"Test categoty", "id" => nil, "type" => nil, "categorizations_count" => nil} assert_equal category_attrs , category.attributes_before_type_cast end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e186b2aea7..594f3b80c6 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -703,7 +703,7 @@ class BasicsTest < ActiveRecord::TestCase duped_topic.reload # FIXME: I think this is poor behavior, and will fix it with #5686 - assert_equal({'a' => 'c'}.to_s, duped_topic.title) + assert_equal({'a' => 'c'}.to_yaml, duped_topic.title) end def test_dup_with_aggregate_of_same_name_as_attribute @@ -1073,10 +1073,14 @@ class BasicsTest < ActiveRecord::TestCase end def test_define_attr_method_with_block - k = Class.new( ActiveRecord::Base ) - k.primary_key = "id" - k.send(:define_attr_method, :primary_key) { "sys_" + original_primary_key } - assert_equal "sys_id", k.primary_key + k = Class.new( ActiveRecord::Base ) do + class << self + attr_accessor :foo_key + end + end + k.foo_key = "id" + k.send(:define_attr_method, :foo_key) { "sys_" + original_foo_key } + assert_equal "sys_id", k.foo_key end def test_set_table_name_with_value diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 31e4981a1d..c35590b84b 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -982,7 +982,7 @@ class FinderTest < ActiveRecord::TestCase def test_select_rows assert_equal( - [["1", nil, nil, "37signals"], + [["1", "1", nil, "37signals"], ["2", "1", "2", "Summit"], ["3", "1", "1", "Microsoft"]], Company.connection.select_rows("SELECT id, firm_id, client_of, name FROM companies WHERE id IN (1,2,3) ORDER BY id").map! {|i| i.map! {|j| j.to_s unless j.nil?}}) diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index 9ce163a00f..864a7a2acc 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -13,6 +13,7 @@ require 'models/category' require 'models/parrot' require 'models/pirate' require 'models/treasure' +require 'models/traffic_light' require 'models/matey' require 'models/ship' require 'models/book' @@ -24,7 +25,7 @@ class FixturesTest < ActiveRecord::TestCase self.use_instantiated_fixtures = true self.use_transactional_fixtures = false - fixtures :topics, :developers, :accounts, :tasks, :categories, :funny_jokes, :binaries + fixtures :topics, :developers, :accounts, :tasks, :categories, :funny_jokes, :binaries, :traffic_lights FIXTURES = %w( accounts binaries companies customers developers developers_projects entrants @@ -204,6 +205,10 @@ class FixturesTest < ActiveRecord::TestCase data.freeze assert_equal data, @flowers.data end + + def test_serialized_fixtures + assert_equal ["Green", "Red", "Orange"], traffic_lights(:uk).state + end end if Account.connection.respond_to?(:reset_pk_sequence!) diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index c3da9cdf53..9fac9373eb 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -208,7 +208,7 @@ class InheritanceTest < ActiveRecord::TestCase def test_eager_load_belongs_to_primary_key_quoting con = Account.connection - assert_sql(/\(#{con.quote_table_name('companies')}.#{con.quote_column_name('id')} = 1\)/) do + assert_sql(/#{con.quote_table_name('companies')}.#{con.quote_column_name('id')} = 1/) do Account.find(1, :include => :firm) end end @@ -219,6 +219,10 @@ class InheritanceTest < ActiveRecord::TestCase switch_to_default_inheritance_column end + def test_inherits_custom_primary_key + assert_equal Subscriber.primary_key, SpecialSubscriber.primary_key + end + def test_inheritance_without_mapping assert_kind_of SpecialSubscriber, SpecialSubscriber.find("webster132") assert_nothing_raised { s = SpecialSubscriber.new("name" => "And breaaaaathe!"); s.id = 'roger'; s.save } diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index 0ffd0e2ab3..e3ba65b4fa 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -14,7 +14,7 @@ class MethodScopingTest < ActiveRecord::TestCase def test_set_conditions Developer.send(:with_scope, :find => { :conditions => 'just a test...' }) do - assert_equal '(just a test...)', Developer.scoped.arel.send(:where_clauses).join(' AND ') + assert_match '(just a test...)', Developer.scoped.arel.to_sql end end @@ -275,7 +275,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do Developer.send(:with_scope, :find => { :limit => 10 }) do devs = Developer.scoped - assert_equal '(salary = 80000)', devs.arel.send(:where_clauses).join(' AND ') + assert_match '(salary = 80000)', devs.arel.to_sql assert_equal 10, devs.taken end end @@ -309,7 +309,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do Developer.send(:with_scope, :find => { :conditions => 'salary = 80000' }) do devs = Developer.scoped - assert_equal "(name = 'David') AND (salary = 80000)", devs.arel.send(:where_clauses).join(' AND ') + assert_match "(name = 'David') AND (salary = 80000)", devs.arel.to_sql assert_equal(1, Developer.count) end Developer.send(:with_scope, :find => { :conditions => "name = 'Maiha'" }) do @@ -322,7 +322,7 @@ class NestedScopingTest < ActiveRecord::TestCase Developer.send(:with_scope, :find => { :conditions => 'salary = 80000', :limit => 10 }) do Developer.send(:with_scope, :find => { :conditions => "name = 'David'" }) do devs = Developer.scoped - assert_equal "(salary = 80000) AND (name = 'David')", devs.arel.send(:where_clauses).join(' AND ') + assert_match "(salary = 80000) AND (name = 'David')", devs.arel.to_sql assert_equal 10, devs.taken end end diff --git a/activerecord/test/cases/quoting_test.rb b/activerecord/test/cases/quoting_test.rb index 2ef5b5a800..b87fb51d97 100644 --- a/activerecord/test/cases/quoting_test.rb +++ b/activerecord/test/cases/quoting_test.rb @@ -154,13 +154,13 @@ module ActiveRecord end def test_crazy_object - crazy = Class.new { def to_s; 'lol' end }.new + crazy = Class.new { def to_yaml; 'lol' end }.new assert_equal "'lol'", @quoter.quote(crazy, nil) assert_equal "'lol'", @quoter.quote(crazy, Object.new) end def test_crazy_object_calls_quote_string - crazy = Class.new { def to_s; 'lo\l' end }.new + crazy = Class.new { def to_yaml; 'lo\l' end }.new assert_equal "'lo\\\\l'", @quoter.quote(crazy, nil) assert_equal "'lo\\\\l'", @quoter.quote(crazy, Object.new) end diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index f113a9c516..e8672515fc 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -152,7 +152,7 @@ class NestedRelationScopingTest < ActiveRecord::TestCase Developer.where('salary = 80000').scoping do Developer.limit(10).scoping do devs = Developer.scoped - assert_equal '(salary = 80000)', devs.arel.send(:where_clauses).join(' AND ') + assert_match '(salary = 80000)', devs.arel.to_sql assert_equal 10, devs.taken end end diff --git a/activerecord/test/fixtures/companies.yml b/activerecord/test/fixtures/companies.yml index ffaa097686..a982d3921d 100644 --- a/activerecord/test/fixtures/companies.yml +++ b/activerecord/test/fixtures/companies.yml @@ -12,6 +12,7 @@ first_firm: type: Firm name: 37signals ruby_type: Firm + firm_id: 1 second_client: id: 3 diff --git a/activerecord/test/fixtures/traffic_lights.yml b/activerecord/test/fixtures/traffic_lights.yml new file mode 100644 index 0000000000..6dabd53474 --- /dev/null +++ b/activerecord/test/fixtures/traffic_lights.yml @@ -0,0 +1,6 @@ +uk: + location: UK + state: + - Green + - Red + - Orange diff --git a/activerecord/test/models/traffic_light.rb b/activerecord/test/models/traffic_light.rb new file mode 100644 index 0000000000..228f3f7bd4 --- /dev/null +++ b/activerecord/test/models/traffic_light.rb @@ -0,0 +1,3 @@ +class TrafficLight < ActiveRecord::Base + serialize :state, Array +end diff --git a/activesupport/lib/active_support/duration.rb b/activesupport/lib/active_support/duration.rb index de3ded1e1f..00c67a470d 100644 --- a/activesupport/lib/active_support/duration.rb +++ b/activesupport/lib/active_support/duration.rb @@ -80,6 +80,10 @@ module ActiveSupport parts.to_sentence(:locale => :en) end + def as_json(options = nil) #:nodoc: + to_i + end + protected def sum(sign, time = ::Time.current) #:nodoc: diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 51b28b6a43..c89b03e243 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -3,6 +3,27 @@ require 'test/unit' require 'active_support' module CallbacksTest + class Phone + include ActiveSupport::Callbacks + define_callbacks :save, :rescuable => true + + set_callback :save, :before, :before_save1 + set_callback :save, :after, :after_save1 + + def before_save1; self.history << :before; end + def after_save1; self.history << :after; end + + def save + self.send(:_run_save_callbacks) do + raise 'boom' + end + end + + def history + @history ||= [] + end + end + class Record include ActiveSupport::Callbacks @@ -338,6 +359,14 @@ module CallbacksTest end class CallbacksTest < Test::Unit::TestCase + def test_save_phone + phone = Phone.new + assert_raise RuntimeError do + phone.save + end + assert_equal [:before, :after], phone.history + end + def test_save_person person = Person.new assert_equal [], person.history @@ -573,5 +602,5 @@ module CallbacksTest ], writer.history end end - + end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index bb453b8d7f..6a01eeed6b 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -1,5 +1,6 @@ require 'abstract_unit' require 'active_support/time' +require 'active_support/json' class DurationTest < ActiveSupport::TestCase def test_is_a @@ -138,6 +139,10 @@ class DurationTest < ActiveSupport::TestCase assert_equal counter, 60 end + def test_to_json + assert_equal '172800', 2.days.to_json + end + protected def with_env_tz(new_tz = 'US/Eastern') old_tz, ENV['TZ'] = ENV['TZ'], new_tz diff --git a/railties/Rakefile b/railties/Rakefile index 5137bee761..1b469543cc 100755 --- a/railties/Rakefile +++ b/railties/Rakefile @@ -46,8 +46,8 @@ task :generate_guides do end task :update_ujs do - system "curl http://github.com/rails/prototype-ujs/raw/master/src/rails.js > lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js" - system "curl http://github.com/rails/jquery-ujs/raw/master/src/rails.js > lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js" + system "curl https://github.com/rails/prototype-ujs/raw/master/src/rails.js > lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js" + system "curl https://github.com/rails/jquery-ujs/raw/master/src/rails.js > lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js" end # Validate guides ------------------------------------------------------------------------- diff --git a/railties/guides/source/contributing_to_rails.textile b/railties/guides/source/contributing_to_rails.textile deleted file mode 100644 index 1a1f4e9858..0000000000 --- a/railties/guides/source/contributing_to_rails.textile +++ /dev/null @@ -1,311 +0,0 @@ -h2. Contributing to Rails - -This guide covers ways in which _you_ can become a part of the ongoing development of Rails. After reading it, you should be familiar with: - -* Using Lighthouse to report issues with Rails -* Cloning edge Rails and running the test suite -* Helping to resolve existing issues -* Contributing to the Rails documentation -* Contributing to the Rails code - -Rails is not "someone else's framework." Over the years, hundreds of people have contributed code ranging from a single character to massive architectural changes, all with the goal of making Rails better for everyone. Even if you don't feel up to writing code yet, there are a variety of other ways that you can contribute, from reporting issues to testing patches to contributing documentation. - -endprologue. - -h3. Reporting a Rails Issue - -Rails uses a "Lighthouse project":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/ to track issues (primarily bugs and contributions of new code). If you've found a bug in Rails, this is the place to start. You'll need to create a (free) Lighthouse account in order to comment on issues or to upload tests or patches. - -NOTE: Bugs in the most recent released version of Rails are likely to get the most attention. Also, the Rails core team is always interested in feedback from those who can take the time to test _edge Rails_ (the code for the version of Rails that is currently under development). Later in this Guide you'll find out how to get edge Rails for testing. - -h4. Creating a Bug Report - -If you've found a problem in Rails, you can start by "adding a new ticket":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/new to the Rails Lighthouse. At the minimum, your ticket needs a title and descriptive text. But that's only a minimum. You should include as much relevant information as possible. You need to at least post the code sample that has the issue. Even better is to include a unit test that shows how the expected behavior is not occurring. Your goal should be to make it easy for yourself - and others - to replicate the bug and figure out a fix. - -You shouldn't assign the bug to a particular core developer (through the *Who's Responsible* select list) unless you know for sure which developer will be handling any patch. The core team periodically reviews issues and assigns developers and milestones to them. - -You should set tags for your issue. Use the "bug" tag for a bug report, and add the "patch" tag if you are attaching a patch. Try to find some relevant tags from the existing tag list (which will appear as soon as you start typing in the *Choose some tags* textbox), rather than creating new tags. - -Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kind of bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to confirm it with a "I'm having this problem too" comment. - -h4. Special Treatment for Security Issues - -If you've found a security vulnerability in Rails, please do *not* report it via a Lighthouse ticket. Lighthouse tickets are public as soon as they are entered. Instead, you should use the dedicated email address "security@rubyonrails.org":mailto:security@rubyonrails.org to report any vulnerabilities. This alias is monitored and the core team will work with you to quickly and completely address any such vulnerabilities. - -WARNING: Just to emphasize the point, _please do not report security vulnerabilities on public Lighthouse tickets_. This will only expose your fellow Rails developers to needless risks. - -You should receive an acknowledgement and detailed response to any reported security issue within 48 hours. If you don't think you're getting adequate response from the security alias, refer to the "Rails security policy page":http://rubyonrails.org/security for direct emails for the current Rails security coordinators. - -h4. What About Feature Requests? - -Please don't put "feature request" tickets into Lighthouse. If there's a new feature that you want to see added to Rails, you'll need to write the code yourself - or convince someone else to partner with you to write the code. Later in this guide you'll find detailed instructions for proposing a patch to Rails. If you enter a wishlist item in Lighthouse with no code, you can expect it to be marked "invalid" as soon as it's reviewed. - -h3. Running the Rails Test Suite - -To move on from submitting bugs to helping resolve existing issues or contributing your own code to Rails, you _must_ be able to run the Rails test suite. In this section of the guide you'll learn how to set up the tests on your own computer. - -h4. Install git - -Rails uses git for source code control. You won’t be able to do anything without the Rails source code, and this is a prerequisite. The "git homepage":http://git-scm.com/ has installation instructions. If you’re on OS X, use the "Git for OS X":http://code.google.com/p/git-osx-installer/ installer. If you're unfamiliar with git, there are a variety of resources on the net that will help you learn more: - -* "Everyday Git":http://www.kernel.org/pub/software/scm/git/docs/everyday.html will teach you just enough about git to get by. -* The "PeepCode screencast":https://peepcode.com/products/git on git ($9) is easier to follow. -* "GitHub":http://github.com/guides/home offers links to a variety of git resources. -* "Pro Git":http://progit.org/book/ is an entire book about git with a Creative Commons license. - -h4. Get the Rails Source Code - -Don’t fork the main Rails repository. Instead, you want to clone it to your own computer. Navigate to the folder where you want the source code (it will create its own /rails subdirectory) and run: - -<shell> -git clone git://github.com/rails/rails.git -cd rails -</shell> - -h4. Set up and Run the Tests - -All of the Rails tests must pass with any code you submit, otherwise you have no chance of getting code accepted. This means you need to be able to run the tests. First, you need to install all Rails dependencies with bundler: - -NOTE: Ensure you install bundler v1.0 - -<shell> -gem install bundler -bundle install --without db -</shell> - -The second command will install all dependencies, except MySQL and PostgreSQL. We will come back at these soon. With dependencies installed, you can run the whole Rails test suite with: - -<shell> -rake test -</shell> - -You can also run tests for an specific framework, like Action Pack, by going into its directory and executing the same command: - -<shell> -cd actionpack -rake test -</shell> - -h4. Testing Active Record - -By default, when you run Active Record tests, it will execute the test suite three times, one for each of the main databases: SQLite3, MySQL and PostgreSQL. If you are adding a feature that is not specific to the database, you can run the test suite (or just one file) for just one of them. Here is an example for SQLite3: - -<shell> -cd activerecord -rake test_sqlite3 -rake test_sqlite3 TEST=test/cases/validations_test.rb -</shell> - -If you want to use another database, as MySQL, you need to create a user named +rails+ with privileges on the test databases. - -<shell> -mysql> GRANT ALL PRIVILEGES ON activerecord_unittest.* - to 'rails'@'localhost'; -mysql> GRANT ALL PRIVILEGES ON activerecord_unittest2.* - to 'rails'@'localhost'; -</shell> - -Now you'll have to install Active Record dependencies. This step is a little tricky because just running +bundle install+ without the +--without db+ parameter won't get those dependencies installed. It turns out that bundler remembers the +--without db+ parameter between calls so you'll have to manually override this. (See the "+bundle_install+ man page":http://gembundler.com/man/bundle-install.1.html for details) - -The easiest way to do this is to remove bundler's config file and then run +install+ again: - -<shell> -rm .bundle/config -bundle install -</shell> - -INFO: If you don't feel comfortable deleting bundler's config file, you can achieve the same effect by manually removing the "+BUNDLE_WITHOUT: db+" line on +.bundle/config+. - -Finally, enter this from the +activerecord+ directory to create the test databases: - -<shell> -rake mysql:build_databases -</shell> - -NOTE: Using the rake task to create the test databases ensures they have the correct character set and collation. - -If you’re using another database, check the files under +activerecord/test/connections+ in the Rails source code for default connection information. You can edit these files if you _must_ on your machine to provide different credentials, but obviously you should not push any such changes back to Rails. - -You can now run tests as you did for +sqlite3+: - -<shell> -rake test_mysql -</shell> - -You can also replace +mysql+ with +postgresql+, +jdbcmysql+, +jdbcsqlite3+ or +jdbcpostgresql+. Check out the file +activerecord/RUNNING_UNIT_TESTS+ for information on running more targeted database tests, or the file +ci/ci_build.rb+ to see the test suite that the Rails continuous integration server runs. - -NOTE: If you're working with Active Record code, you _must_ ensure that the tests pass for at least MySQL, PostgreSQL, and SQLite 3. Subtle differences between the various Active Record database adapters have been behind the rejection of many patches that looked OK when tested only against MySQL. - -h4. Older versions of Rails - -If you want to work add a fix to older versions of Rails, you'll need to set up and switch to your own local tracking branch. Here is an example to switch to Rails 2.3 branch: - -<shell> -git branch --track 2-3-stable origin/2-3-stable -git checkout 2-3-stable -</shell> - -TIP: You may want to "put your git branch name in your shell prompt":http://github.com/guides/put-your-git-branch-name-in-your-shell-prompt to make it easier to remember which version of the code you're working with. - -h3. Helping to Resolve Existing Issues - -As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the "open tickets":https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets?q=state%3Aopen list in Lighthouse, you'll find hundreds of issues already requiring attention. What can you do for these? Quite a bit, actually: - -h4. Verifying Bug Reports - -For starters, it helps to just verify bug reports. Can you reproduce the reported issue on your own computer? If so, you can add a comment to the ticket saying that you're seeing the same thing. - -If something is very vague, can you help squish it down into something specific? Maybe you can provide additional information to help reproduce a bug, or eliminate needless steps that aren't required to help demonstrate the problem. - -If you find a bug report without a test, it's very useful to contribute a failing test. This is also a great way to get started exploring the Rails source: looking at the existing test files will teach you how to write more tests for Rails. New tests are best contributed in the form of a patch, as explained later on in the "Contributing to the Rails Code" section. - -Anything you can do to make bug reports more succinct or easier to reproduce is a help to folks trying to write code to fix those bugs - whether you end up writing the code yourself or not. - -h4. Testing Patches - -You can also help out by examining patches that have been submitted to Rails via Lighthouse. To apply someone's changes you need to first create a branch of the Rails source code: - -<shell> -git checkout -b testing_branch -</shell> - -Then you can apply their patch: - -<shell> -git apply their-patch-file.diff -</shell> - -After applying a patch, test it out! Here are some things to think about: - -* Does the patch actually work? -* Are you happy with the tests? Can you follow what they're testing? Are there any tests missing? -* Does the documentation still seem right to you? -* Do you like the implementation? Can you think of a nicer or faster way to implement a part of their change? - -Once you're happy that the patch contains a good change, comment on the Lighthouse ticket indicating your approval. Your comment should indicate that you like the change and what you like about it. Something like: - -<blockquote> -I like the way you've restructured that code in generate_finder_sql, much nicer. The tests look good too. -</blockquote> - -If your comment simply says "+1", then odds are that other reviewers aren't going to take it too seriously. Show that you took the time to review the patch. Once three people have approved it, add the "verified" tag. This will bring it to the attention of a core team member who will review the changes looking for the same kinds of things. - -h3. Contributing to the Rails Documentation - -Another area where you can help out if you're not yet ready to take the plunge to writing Rails core code is with Rails documentation. You can help with the Rails Guides or the Rails API documentation. - -TIP: "docrails":http://github.com/lifo/docrails/tree/master is the documentation branch for Rails with an *open commit policy*, it has public write access. Documentation changes made as part of the "docrails":http://github.com/lifo/docrails/tree/master project are merged back to the Rails master code from time to time. Check out the "original announcement":http://weblog.rubyonrails.org/2008/5/2/help-improve-rails-documentation-on-git-branch for more details. - -h4. The Rails Guides - -The "Rails Guides":http://guides.rubyonrails.org/ are a set of online resources that are designed to make people productive with Rails and to understand how all of the pieces fit together. These guides (including this one!) are written as part of the "docrails":http://github.com/lifo/docrails/tree/master project. If you have an idea for a new guide, or improvements for an existing guide, you can refer to the "contribution page":contribute.html for instructions on getting involved. - -h4. The Rails API Documentation - -The "Rails API documentation":http://api.rubyonrails.org/ is automatically generated from the Rails source code via "RDoc":http://rdoc.rubyforge.org/. If you find some part of the documentation to be incomplete, confusing, or just plain wrong, you can step in and fix it. - -To contribute an update to the API documentation, you can contact "lifo":http://github.com/lifo on GitHub and ask for commit rights to the docrails repository and push your changes to the docrails repository. Please follow the "docrails RDoc conventions":http://wiki.github.com/lifo/docrails/rails-api-documentation-conventions when contributing the changes. - -h3. The Rails Wiki - -The "Rails wiki":http://wiki.rubyonrails.org/ is a collection of user-generated and freely-editable information about Rails. It covers everything from getting started to FAQs to how-tos and popular plugins. To contribute to the wiki, just find some useful information that isn't there already and add it. There are style guidelines to help keep the wiki a coherent resources; see the section on "contributing to the wiki":http://wiki.rubyonrails.org/#contributing_to_the_wiki for more details. - -h3. Contributing to the Rails Code - -When you're ready to take the plunge, one of the most helpful ways to contribute to Rails is to actually submit source code. Here's a step-by-step listing of the things you need to do to make this a successful experience. - -h4. Learn the Language and the Framework - -Learn at least _something_ about Ruby and Rails. If you don’t understand the syntax of the language, common Ruby idioms, and the code that already exists in Rails, you’re unlikely to be able to build a good patch (that is, one that will get accepted). You don’t have to know every in-and-out of the language and the framework; some of the Rails code is fiendishly complex. But Rails is probably not appropriate as the first place that you ever write Ruby code. You should at least understand (though not necessarily memorize) "The Ruby Programming Language":http://www.amazon.com/gp/product/0596516177?ie=UTF8&linkCode=as2&camp=1789&creative=390957&creativeASIN=0596516177 and have browsed the Rails source code. - -h4. Fork the Rails Source Code - -Fork Rails. You’re not going to put your patches right into the master branch, OK? This is where you need that copy of Rails that you cloned earlier. Think of a name for your new branch and run - -<shell> -git checkout -b my_new_branch -</shell> - -It doesn’t really matter what name you use, because this branch will only exist on your local computer. - -h4. Write Your Code - -Now get busy and add your code to Rails (or edit the existing code). You’re on your branch now, so you can write whatever you want (you can check to make sure you’re on the right branch with +git branch -a+). But if you’re planning to submit your change back for inclusion in Rails, keep a few things in mind: - -* Get the code right -* Use Rails idioms and helpers -* Include tests that fail without your code, and pass with it -* Update the documentation - -h4. Follow the Coding Conventions - -Rails follows a simple set of coding style conventions. - -* Two spaces, no tabs -* Prefer +&&+/+||+ over +and+/+or+ -* +MyClass.my_method(my_arg)+ not +my_method( my_arg )+ or +my_method my_arg+ -* Follow the conventions you see used in the source already - -h4. Sanity Check - -You should not be the only person who looks at the code before you submit it. You know at least one other Rails developer, right? Show them what you’re doing and ask for feedback. Doing this in private before you push a patch out publicly is the “smoke test” for a patch: if you can’t convince one other developer of the beauty of your code, you’re unlikely to convince the core team either. - -You might also want to check out the "RailsBridge BugMash":http://wiki.railsbridge.org/projects/railsbridge/wiki/BugMash as a way to get involved in a group effort to improve Rails. This can help you get started and help check your code when you're writing your first patches. - -h4. Commit Your Changes - -When you're happy with the code on your computer, you need to commit the changes to git: - -<shell> -git commit -a -m "Here is a commit message" -</shell> - -h4. Update Rails - -Update your copy of Rails. It’s pretty likely that other changes to core Rails have happened while you were working. Go get them: - -<shell> -git checkout master -git pull -</shell> - -Now reapply your patch on top of the latest changes: - -<shell> -git checkout my_new_branch -git rebase master -</shell> - -No conflicts? Tests still pass? Change still seems reasonable to you? Then move on. - -h4. Create a Patch - -Now you can create a patch file to share with other developers (and with the Rails core team). Still in your branch, run - -<shell> -git commit -a -git format-patch master --stdout > my_new_patch.diff -</shell> - -Sanity check the results of this operation: open the diff file in your text editor of choice and make sure that no unintended changes crept in. - -h4. Create a Lighthouse Ticket - -Now create a ticket with your patch. Go to the "new ticket":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/new page at Lighthouse. Fill in a reasonable title and description, remember to attach your patch file, and tag the ticket with the ‘patch’ tag and whatever other subject area tags make sense. - -h4. Get Some Feedback - -Now you need to get other people to look at your patch, just as you've looked at other people's patches. You can use the rubyonrails-core mailing list or the #rails-contrib channel on IRC freenode for this. You might also try just talking to Rails developers that you know. - -h4. Iterate as Necessary - -It’s entirely possible that the feedback you get will suggest changes. Don’t get discouraged: the whole point of contributing to an active open source project is to tap into community knowledge. If people are encouraging you to tweak your code, then it’s worth making the tweaks and resubmitting. If the feedback is that your code doesn’t belong in the core, you might still think about releasing it as a plugin. - -And then...think about your next contribution! - -h3. Changelog - -* April 6, 2010: Fixed document to validate XHTML 1.0 Strict. "Jaime Iniesta":http://jaimeiniesta.com -* August 1, 2009: Updates/amplifications by "Mike Gunderloy":credits.html#mgunderloy -* March 2, 2009: Initial draft by "Mike Gunderloy":credits.html#mgunderloy - diff --git a/railties/guides/source/contributing_to_ruby_on_rails.textile b/railties/guides/source/contributing_to_ruby_on_rails.textile new file mode 100644 index 0000000000..484c36aeea --- /dev/null +++ b/railties/guides/source/contributing_to_ruby_on_rails.textile @@ -0,0 +1,370 @@ +h2. Contributing to Ruby on Rails + +This guide covers ways in which _you_ can become a part of the ongoing development of Ruby on Rails. After reading it, you should be familiar with: + +* Using Lighthouse to report issues +* Cloning master and running the test suite +* Helping to resolve existing issues +* Contributing to the Ruby on Rails documentation +* Contributing to the Ruby on Rails code + +Ruby on Rails is not "someone else's framework." Over the years, hundreds of people have contributed to Ruby on Rails ranging from a single character to massive architectural changes or significant documentation. All with the goal of making Ruby on Rails better for everyone. Even if you don't feel up to writing code or documentation yet, there are a variety of other ways that you can contribute, from reporting issues to testing patches. + +endprologue. + +h3. Reporting an Issue + +Ruby on Rails uses a "Lighthouse project":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/ to track issues (primarily bugs and contributions of new code). If you've found a bug in Ruby on Rails, this is the place to start. You'll need to create a (free) Lighthouse account in order to comment on issues or to upload patches. + +NOTE: Bugs in the most recent released version of Ruby on Rails are likely to get the most attention. Also, the Rails core team is always interested in feedback from those who can take the time to test _edge Rails_ (the code for the version of Rails that is currently under development). Later in this guide you'll find out how to get edge Rails for testing. + +h4. Creating a Bug Report + +If you've found a problem in Ruby on Rails which is not a security risk do a search in Lighthouse in case it was already reported. If you find no ticket addressing it you can "add a new one":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/new. (See the next section for reporting security issues.) + +At the minimum, your ticket needs a title and descriptive text. But that's only a minimum. You should include as much relevant information as possible. You need to at least post the code sample that has the issue. Even better is to include a unit test that shows how the expected behavior is not occurring. Your goal should be to make it easy for yourself - and others - to replicate the bug and figure out a fix. + +You shouldn't assign the bug to a particular core developer unless you know for sure which developer will be handling that issue. The core team periodically reviews issues and assigns developers and milestones to them. + +You should set tags for your issue. Use the "bug" tag for a bug report, and add the "patch" tag if you are attaching a patch. Try to find some relevant tags from the existing tag list (which will appear as soon as you start typing in the "Choose some tags" textbox), rather than creating new tags. + +Then don't get your hopes up. Unless you have a "Code Red, Mission Critical, The World is Coming to an End" kind of bug, you're creating this ticket in the hope that others with the same problem will be able to collaborate with you on solving it. Do not expect that the ticket automatically will see any activity or that others will jump to fix it. Creating a ticket like this is mostly to help yourself start on the path of fixing the problem and for others to confirm it with a "I'm having this problem too" comment. + +h4. Special Treatment for Security Issues + +WARNING: Please do not report security vulnerabilities on public Lighthouse tickets. The "Rails security policy page":http://rubyonrails.org/security details the procedure to follow for security issues. + +h4. What About Feature Requests? + +Please don't put "feature request" tickets into Lighthouse. If there's a new feature that you want to see added to Ruby on Rails, you'll need to write the code yourself - or convince someone else to partner with you to write the code. Later in this guide you'll find detailed instructions for proposing a patch to Ruby on Rails. If you enter a wishlist item in Lighthouse with no code, you can expect it to be marked "invalid" as soon as it's reviewed. + +h3. Running the Test Suite + +To move on from submitting bugs to helping resolve existing issues or contributing your own code to Ruby on Rails, you _must_ be able to run its test suite. In this section of the guide you'll learn how to set up the tests on your own computer. + +h4. Install git + +Ruby on Rails uses git for source code control. The "git homepage":http://git-scm.com/ has installation instructions. There are a variety of resources on the net that will help you get familiar with git: + +* "Everyday Git":http://www.kernel.org/pub/software/scm/git/docs/everyday.html will teach you just enough about git to get by. +* The "PeepCode screencast":https://peepcode.com/products/git on git ($9) is easier to follow. +* "GitHub":http://github.com/guides/home offers links to a variety of git resources. +* "Pro Git":http://progit.org/book/ is an entire book about git with a Creative Commons license. + +h4. Clone the Ruby on Rails Repository + +Navigate to the folder where you want the Ruby on Rails source code (it will create its own +rails+ subdirectory) and run: + +<shell> +git clone git://github.com/rails/rails.git +cd rails +</shell> + +h4. Set up and Run the Tests + +The test suite must pass with any submitted code. No matter whether you are writing a new patch, or evaluating someone else's, you need to be able to run the tests. + +Install first libxml2 and libxslt together with their development files for Nokogiri. In Ubuntu that's + +<shell> +sudo aptitude install libxml2 libxml2-dev libxslt1-dev +</shell> + +Also, SQLite3 and its development files for the +sqlite3-ruby+ gem, in Ubuntu you're done with + +<shell> +sudo aptitude install sqlite3 libsqlite3-dev +</shell> + +Get a recent version of "Bundler":http://gembundler.com/: + +<shell> +gem install bundler +</shell> + +and run: + +<shell> +bundle install --without db +</shell> + +This command will install all dependencies except the MySQL and PostgreSQL Ruby drivers. We will come back at these soon. With dependencies installed, you can run the test suite with: + +<shell> +rake test +</shell> + +You can also run tests for an specific framework, like Action Pack, by going into its directory and executing the same command: + +<shell> +cd actionpack +rake test +</shell> + +h4. Warnings + +The test suite runs with warnings enabled. Ideally Ruby on Rails should issue no warning, but there may be a few, and also some from third-party libraries. Please ignore (or fix!) them if any, and submit patches that do not issue new warnings. + +As of this writing they are specially noisy with Ruby 1.9. If you are sure about what you are doing and would like to have a more clear output, there's a way to override the flag: + +<shell> +RUBYOPT=-W0 rake test +</shell> + +h4. Testing Active Record + +The test suite of Active Record attempts to run four times, once for SQLite3, once for each of the two MySQL gems (+mysql+ and +mysql2+), and once for PostgreSQL. We are going to see now how to setup the environment for them. + +WARNING: If you're working with Active Record code, you _must_ ensure that the tests pass for at least MySQL, PostgreSQL, and SQLite3. Subtle differences between the various adapters have been behind the rejection of many patches that looked OK when tested only against MySQL. + +h5. SQLite3 + +The gem +sqlite3-ruby+ does not belong to the "db" group indeed, if you followed the instructions above you're ready. This is how you run the Active Record test suite only for SQLite3: + +<shell> +cd activerecord +rake test_sqlite3 +</shell> + +h5. MySQL and PostgreSQL + +To be able to run the suite for MySQL and PostgreSQL we need their gems. Install first the servers, their client libraries, and their development files. In Ubuntu just run + +<shell> +sudo aptitude install mysql-server libmysqlclient15-dev +sudo aptitude install postgresql postgresql-client postgresql-contrib libpq-dev +</shell> + +After that run: + +<shell> +rm .bundle/config +bundle install +</shell> + +We need first to delete +.bundle/config+ because Bundler remembers in that file that we didn't want to install the "db" group (alternatively you can edit the file). + +In order to be able to run the test suite against MySQL you need to create a user named +rails+ with privileges on the test databases: + +<shell> +mysql> GRANT ALL PRIVILEGES ON activerecord_unittest.* + to 'rails'@'localhost'; +mysql> GRANT ALL PRIVILEGES ON activerecord_unittest2.* + to 'rails'@'localhost'; +</shell> + +and create the test databases: + +<shell> +cd activerecord +rake mysql:build_databases +</shell> + +PostgreSQL's authentication works differently. A simple way to setup the development environment for example is to run with your development account + +<shell> +sudo -u postgres createuser --superuser $USER +</shell> + +and after that create the test databases with + +<shell> +cd activerecord +rake postgresql:build_databases +</shell> + +NOTE: Using the rake task to create the test databases ensures they have the correct character set and collation. + +If you’re using another database, check the files under +activerecord/test/connections+ for default connection information. You can edit these files if you _must_ on your machine to provide different credentials, but obviously you should not push any such changes back to Rails. + +You can now run tests as you did for +sqlite3+, the tasks are + +<shell> +test_mysql +test_mysql2 +test_postgresql +</shell> + +respectively. As we mentioned before + +<shell> +rake test +</shell> + +will now run the four of them in turn. + +You can also invoke +test_jdbcmysql+, +test_jdbcsqlite3+ or +test_jdbcpostgresql+. Check out the file +activerecord/RUNNING_UNIT_TESTS+ for information on running more targeted database tests, or the file +ci/ci_build.rb+ to see the test suite that the continuous integration server runs. + +h4. Older versions of Ruby on Rails + +If you want to add a fix to older versions of Ruby on Rails, you'll need to set up and switch to your own local tracking branch. Here is an example to switch to the 2-3-stable branch: + +<shell> +git branch --track 2-3-stable origin/2-3-stable +git checkout 2-3-stable +</shell> + +TIP: You may want to "put your git branch name in your shell prompt":http://qugstart.com/blog/git-and-svn/add-colored-git-branch-name-to-your-shell-prompt/ to make it easier to remember which version of the code you're working with. + +h3. Helping to Resolve Existing Issues + +As a next step beyond reporting issues, you can help the core team resolve existing issues. If you check the "open tickets":https://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets?q=state%3Aopen list in Lighthouse, you'll find lots of issues already requiring attention. What can you do for these? Quite a bit, actually: + +h4. Verifying Bug Reports + +For starters, it helps to just verify bug reports. Can you reproduce the reported issue on your own computer? If so, you can add a comment to the ticket saying that you're seeing the same thing. + +If something is very vague, can you help squish it down into something specific? Maybe you can provide additional information to help reproduce a bug, or eliminate needless steps that aren't required to help demonstrate the problem. + +If you find a bug report without a test, it's very useful to contribute a failing test. This is also a great way to get started exploring the source code: looking at the existing test files will teach you how to write more tests. New tests are best contributed in the form of a patch, as explained later on in the "Contributing to the Rails Code" section. + +Anything you can do to make bug reports more succinct or easier to reproduce is a help to folks trying to write code to fix those bugs - whether you end up writing the code yourself or not. + +h4. Testing Patches + +You can also help out by examining patches that have been submitted to Ruby on Rails via Lighthouse. To apply someone's changes you need to first create a dedicated branch: + +<shell> +git checkout -b testing_branch +</shell> + +Then you can apply their patch: + +<shell> +git apply their-patch-file.diff +</shell> + +After applying a patch, test it out! Here are some things to think about: + +* Does the patch actually work? +* Are you happy with the tests? Can you follow what they're testing? Are there any tests missing? +* Does it have proper documentation coverage? Should documentation elsewhere be updated? +* Do you like the implementation? Can you think of a nicer or faster way to implement a part of their change? + +Once you're happy that the patch contains a good change, comment on the Lighthouse ticket indicating your approval. Your comment should indicate that you like the change and what you like about it. Something like: + +<blockquote> +I like the way you've restructured that code in generate_finder_sql, much nicer. The tests look good too. +</blockquote> + +If your comment simply says "+1", then odds are that other reviewers aren't going to take it too seriously. Show that you took the time to review the patch. Once three people have approved it, add the "verified" tag. This will bring it to the attention of a core team member who will review the changes looking for the same kinds of things. + +h3. Contributing to the Rails Documentation + +Ruby on Rails has two main sets of documentation: The guides help you to learn Ruby on Rails, and the API is a reference. + +You can create a ticket in Lighthouse to fix or expand documentation. However, if you're confident about your changes you can push them yourself directly via "docrails":http://github.com/lifo/docrails/tree/master. docrails is a branch with an *open commit policy* and public write access. Commits to docrails are still reviewed, but that happens after they are pushed. docrails is merged with master regularly, so you are effectively editing the Ruby on Rails documentation. + +When working with documentation, please take into account the "API Documentation Guidelines":api_documentation_guidelines.html and the "Ruby on Rails Guides Guidelines":ruby_on_rails_guides_guidelines.html. + +NOTE: As explained above, ordinary code patches should have proper documentation coverage. docrails is only used for isolated documentation improvements. + +WARNING: docrails has a very strict policy: no code can be touched whatsoever, no matter how trivial or small the change. Only RDoc and guides can be edited via docrails. + +If you have an idea for a new guide you can refer to the "contribution page":contribute.html for instructions on getting involved. + +h3. Contributing to the Rails Code + +h4. Clone the Rails Repository + +The first thing you need to do to be able to contribute code is to clone the repository: + +<shell> +git clone git://github.com/rails/rails.git +</shell> + +and create a dedicated branch: + +<shell> +cd rails +git checkout -b my_new_branch +</shell> + +It doesn’t really matter what name you use, because this branch will only exist on your local computer. + +h4. Write Your Code + +Now get busy and add or edit code. You’re on your branch now, so you can write whatever you want (you can check to make sure you’re on the right branch with +git branch -a+). But if you’re planning to submit your change back for inclusion in Rails, keep a few things in mind: + +* Get the code right +* Use Rails idioms and helpers +* Include tests that fail without your code, and pass with it +* Update the documentation, the surrounding one, examples elsewhere, guides, whatever is affected by your contribution + +h4. Follow the Coding Conventions + +Rails follows a simple set of coding style conventions. + +* Two spaces, no tabs +* Prefer +&&+/+||+ over +and+/+or+ +* +MyClass.my_method(my_arg)+ not +my_method( my_arg )+ or +my_method my_arg+ +* Follow the conventions you see used in the source already + +h4. Sanity Check + +You should not be the only person who looks at the code before you submit it. You know at least one other Rails developer, right? Show them what you’re doing and ask for feedback. Doing this in private before you push a patch out publicly is the “smoke test” for a patch: if you can’t convince one other developer of the beauty of your code, you’re unlikely to convince the core team either. + +You might also want to check out the "RailsBridge BugMash":http://wiki.railsbridge.org/projects/railsbridge/wiki/BugMash as a way to get involved in a group effort to improve Rails. This can help you get started and help check your code when you're writing your first patches. + +h4. Commit Your Changes + +When you're happy with the code on your computer, you need to commit the changes to git: + +<shell> +git commit -a -m "Here is a commit message" +</shell> + +h4. Update master + +It’s pretty likely that other changes to master have happened while you were working. Go get them: + +<shell> +git checkout master +git pull +</shell> + +Now reapply your patch on top of the latest changes: + +<shell> +git checkout my_new_branch +git rebase master +</shell> + +No conflicts? Tests still pass? Change still seems reasonable to you? Then move on. + +h4. Create a Patch + +Now you can create a patch file to share with other developers (and with the core team). Still in your branch, run + +<shell> +git commit -a +git format-patch master --stdout > my_new_patch.diff +</shell> + +Sanity check the results of this operation: open the diff file in your text editor of choice and make sure that no unintended changes crept in. + +h4. Create a Lighthouse Ticket + +Now create a ticket with your patch. Go to the "new ticket":http://rails.lighthouseapp.com/projects/8994-ruby-on-rails/tickets/new page at Lighthouse. Fill in a reasonable title and description, remember to attach your patch file, and tag the ticket with the ‘patch’ tag and whatever other subject area tags make sense. + +h4. Get Some Feedback + +Now you need to get other people to look at your patch, just as you've looked at other people's patches. You can use the rubyonrails-core mailing list or the #rails-contrib channel on IRC freenode for this. You might also try just talking to Rails developers that you know. + +h4. Iterate as Necessary + +It’s entirely possible that the feedback you get will suggest changes. Don’t get discouraged: the whole point of contributing to an active open source project is to tap into community knowledge. If people are encouraging you to tweak your code, then it’s worth making the tweaks and resubmitting. If the feedback is that your code doesn’t belong in the core, you might still think about releasing it as a plugin. + +And then...think about your next contribution! + +h3. Rails Contributors + +All contributions, either via master or docrails, get credit in "Rails Contributors":http://contributors.rubyonrails.org. + +h3. Changelog + +* December 28, 2010: Complete revision by "Xavier Noria":credits.html#fxn +* April 6, 2010: Fixed document to validate XHTML 1.0 Strict. "Jaime Iniesta":http://jaimeiniesta.com +* August 1, 2009: Updates/amplifications by "Mike Gunderloy":credits.html#mgunderloy +* March 2, 2009: Initial draft by "Mike Gunderloy":credits.html#mgunderloy + diff --git a/railties/guides/source/credits.html.erb b/railties/guides/source/credits.html.erb index 825e042628..8c2f1ffeb6 100644 --- a/railties/guides/source/credits.html.erb +++ b/railties/guides/source/credits.html.erb @@ -20,7 +20,7 @@ Ruby on Rails Guides: Credits <% end %> <%= author('Xavier Noria', 'fxn', 'fxn.png') do %> - Xavier has been into Rails since 2005, he is currently a Rails consultant. Xavier is Rails committer and enjoys combining his passion for Rails and his past life as a proofreader of math textbooks. Oh, he also <a href="http://twitter.com/fxn">tweets</a> and can be found everywhere as "fxn". + Xavier Noria has been into Ruby on Rails since 2005. He is a Rails committer and enjoys combining his passion for Rails and his past life as a proofreader of math textbooks. Xavier is currently a Ruby on Rails consultant. Oh, he also <a href="http://twitter.com/fxn">tweets</a> and can be found everywhere as "fxn". <% end %> <h3 class="section">Rails Guides Designers</h3> @@ -52,7 +52,7 @@ Ruby on Rails Guides: Credits <% end %> <%= author('James Miller', 'bensie') do %> - James Miller is a software developer for <a href="http://www.jk-tech.com">JK Tech</a> in San Diego, CA. Find me on GitHub, Gmail, Twitter, and Freenode as "bensie". + James Miller is a software developer for <a href="http://www.jk-tech.com">JK Tech</a> in San Diego, CA. You can find James on GitHub, Gmail, Twitter, and Freenode as "bensie". <% end %> <%= author('Emilio Tagua', 'miloops') do %> diff --git a/railties/guides/source/index.html.erb b/railties/guides/source/index.html.erb index 771cfbb794..933bf66b3f 100644 --- a/railties/guides/source/index.html.erb +++ b/railties/guides/source/index.html.erb @@ -151,10 +151,10 @@ Ruby on Rails Guides <% end %> </dl> -<h3>Contributing to Rails</h3> +<h3>Contributing to Ruby on Rails</h3> <dl> - <%= guide("Contributing to Rails", 'contributing_to_rails.html') do %> + <%= guide("Contributing to Ruby on Rails", 'contributing_to_ruby_on_rails.html') do %> <p>Rails is not "somebody else's framework." This guide covers a variety of ways that you can get involved in the ongoing development of Rails.</p> <% end %> diff --git a/railties/guides/source/layout.html.erb b/railties/guides/source/layout.html.erb index dbb48124d0..91dab18b21 100644 --- a/railties/guides/source/layout.html.erb +++ b/railties/guides/source/layout.html.erb @@ -75,8 +75,8 @@ <dd><a href="rails_on_rack.html">Rails on Rack</a></dd> <dd><a href="generators.html">Creating and Customizing Rails Generators</a></dd> - <dt>Contributing to Rails</dt> - <dd><a href="contributing_to_rails.html">Contributing to Rails</a></dd> + <dt>Contributing to Ruby on Rails</dt> + <dd><a href="contributing_to_ruby_on_rails.html">Contributing to Ruby on Rails</a></dd> <dd><a href="api_documentation_guidelines.html">API Documentation Guidelines</a></dd> <dd><a href="ruby_on_rails_guides_guidelines.html">Ruby on Rails Guides Guidelines</a></dd> diff --git a/railties/lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js b/railties/lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js index 43456a77f6..668cffa73a 100644 --- a/railties/lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js +++ b/railties/lib/rails/generators/rails/app/templates/public/javascripts/jquery_ujs.js @@ -1,3 +1,12 @@ +/* + * jquery-ujs + * + * http://github.com/rails/jquery-ujs/blob/master/src/rails.js + * + * This rails.js file supports jQuery 1.4.3 and 1.4.4 . + * + */ + jQuery(function ($) { var csrf_token = $('meta[name=csrf-token]').attr('content'), csrf_param = $('meta[name=csrf-param]').attr('content'); @@ -7,9 +16,6 @@ jQuery(function ($) { * Triggers a custom event on an element and returns the event result * this is used to get around not being able to ensure callbacks are placed * at the end of the chain. - * - * TODO: deprecate with jQuery 1.4.2 release, in favor of subscribing to our - * own events and placing ourselves at the end of the chain. */ triggerAndReturn: function (name, data) { var event = new $.Event(name); @@ -19,26 +25,33 @@ jQuery(function ($) { }, /** - * Handles execution of remote calls firing overridable events along the way + * Handles execution of remote calls. Provides following callbacks: + * + * - ajax:beforeSend - is executed before firing ajax call + * - ajax:success - is executed when status is success + * - ajax:complete - is executed when the request finishes, whether in failure or success + * - ajax:error - is execute in case of error */ callRemote: function () { var el = this, method = el.attr('method') || el.attr('data-method') || 'GET', url = el.attr('action') || el.attr('href'), - dataType = el.attr('data-type') || 'script'; + dataType = el.attr('data-type') || ($.ajaxSettings && $.ajaxSettings.dataType); if (url === undefined) { - throw "No URL specified for remote call (action or href must be present)."; + throw "No URL specified for remote call (action or href must be present)."; } else { - if (el.triggerAndReturn('ajax:before')) { - var data = el.is('form') ? el.serializeArray() : []; + var $this = $(this), data = el.is('form') ? el.serializeArray() : []; + $.ajax({ url: url, data: data, dataType: dataType, type: method.toUpperCase(), beforeSend: function (xhr) { - el.trigger('ajax:loading', xhr); + if ($this.triggerHandler('ajax:beforeSend') === false) { + return false; + } }, success: function (data, status, xhr) { el.trigger('ajax:success', [data, status, xhr]); @@ -47,20 +60,17 @@ jQuery(function ($) { el.trigger('ajax:complete', xhr); }, error: function (xhr, status, error) { - el.trigger('ajax:failure', [xhr, status, error]); + el.trigger('ajax:error', [xhr, status, error]); } }); - } - - el.trigger('ajax:after'); } } }); /** - * confirmation handler + * confirmation handler */ - $('a[data-confirm],input[data-confirm]').live('click', function () { + $('body').delegate('a[data-confirm], button[data-confirm], input[data-confirm]', 'click.rails', function () { var el = $(this); if (el.triggerAndReturn('confirm')) { if (!confirm(el.attr('data-confirm'))) { @@ -70,28 +80,34 @@ jQuery(function ($) { }); + /** * remote handlers */ - $('form[data-remote]').live('submit', function (e) { + $('form[data-remote]').live('submit.rails', function (e) { $(this).callRemote(); e.preventDefault(); }); - $('a[data-remote],input[data-remote]').live('click', function (e) { + $('a[data-remote],input[data-remote]').live('click.rails', function (e) { $(this).callRemote(); e.preventDefault(); }); - $('a[data-method]:not([data-remote])').live('click', function (e){ + /** + * <%= link_to "Delete", user_path(@user), :method => :delete, :confirm => "Are you sure?" %> + * + * <a href="/users/5" data-confirm="Are you sure?" data-method="delete" rel="nofollow">Delete</a> + */ + $('a[data-method]:not([data-remote])').live('click.rails', function (e){ var link = $(this), href = link.attr('href'), method = link.attr('data-method'), form = $('<form method="post" action="'+href+'"></form>'), metadata_input = '<input name="_method" value="'+method+'" type="hidden" />'; - if (csrf_param != null && csrf_token != null) { - metadata_input += '<input name="'+csrf_param+'" value="'+csrf_token+'" type="hidden" />'; + if (csrf_param !== undefined && csrf_token !== undefined) { + metadata_input += '<input name="'+csrf_param+'" value="'+csrf_token+'" type="hidden" />'; } form.hide() @@ -105,9 +121,9 @@ jQuery(function ($) { /** * disable-with handlers */ - var disable_with_input_selector = 'input[data-disable-with]'; - var disable_with_form_remote_selector = 'form[data-remote]:has(' + disable_with_input_selector + ')'; - var disable_with_form_not_remote_selector = 'form:not([data-remote]):has(' + disable_with_input_selector + ')'; + var disable_with_input_selector = 'input[data-disable-with]', + disable_with_form_remote_selector = 'form[data-remote]:has(' + disable_with_input_selector + ')', + disable_with_form_not_remote_selector = 'form:not([data-remote]):has(' + disable_with_input_selector + ')'; var disable_with_input_function = function () { $(this).find(disable_with_input_selector).each(function () { @@ -118,10 +134,10 @@ jQuery(function ($) { }); }; - $(disable_with_form_remote_selector).live('ajax:before', disable_with_input_function); - $(disable_with_form_not_remote_selector).live('submit', disable_with_input_function); + $(disable_with_form_remote_selector).live('ajax:before.rails', disable_with_input_function); + $(disable_with_form_not_remote_selector).live('submit.rails', disable_with_input_function); - $(disable_with_form_remote_selector).live('ajax:complete', function () { + $(disable_with_form_remote_selector).live('ajax:complete.rails', function () { $(this).find(disable_with_input_selector).each(function () { var input = $(this); input.removeAttr('disabled') @@ -129,4 +145,10 @@ jQuery(function ($) { }); }); + var jqueryVersion = $().jquery; + + if (!( (jqueryVersion === '1.4.3') || (jqueryVersion === '1.4.4'))){ + alert('This rails.js does not support the jQuery version you are using. Please read documentation.'); + } + }); diff --git a/railties/lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js b/railties/lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js index fc69f93c58..4c18cb0c3e 100644 --- a/railties/lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js +++ b/railties/lib/rails/generators/rails/app/templates/public/javascripts/prototype_ujs.js @@ -95,9 +95,10 @@ parameters: params, evalScripts: true, - onComplete: function(request) { element.fire("ajax:complete", request); }, - onSuccess: function(request) { element.fire("ajax:success", request); }, - onFailure: function(request) { element.fire("ajax:failure", request); } + onCreate: function(response) { element.fire("ajax:create", response); }, + onComplete: function(response) { element.fire("ajax:complete", response); }, + onSuccess: function(response) { element.fire("ajax:success", response); }, + onFailure: function(response) { element.fire("ajax:failure", response); } }); element.fire("ajax:after"); @@ -114,7 +115,7 @@ csrf_token = $$('meta[name=csrf-token]')[0]; var form = new Element('form', { method: "POST", action: url, style: "display: none;" }); - element.parentNode.insert(form); + $(element.parentNode).insert(form); if (method !== 'post') { insertHiddenField(form, '_method', method); @@ -127,52 +128,65 @@ form.submit(); } + function disableFormElements(form) { + form.select('input[type=submit][data-disable-with]').each(function(input) { + input.store('rails:original-value', input.getValue()); + input.setValue(input.readAttribute('data-disable-with')).disable(); + }); + } + + function enableFormElements(form) { + form.select('input[type=submit][data-disable-with]').each(function(input) { + input.setValue(input.retrieve('rails:original-value')).enable(); + }); + } - document.on("click", "*[data-confirm]", function(event, element) { + function allowAction(element) { var message = element.readAttribute('data-confirm'); - if (!confirm(message)) event.stop(); - }); + return !message || confirm(message); + } - document.on("click", "a[data-remote]", function(event, element) { - if (event.stopped) return; - handleRemote(element); - event.stop(); - }); + document.on('click', 'a[data-confirm], a[data-remote], a[data-method]', function(event, link) { + if (!allowAction(link)) { + event.stop(); + return false; + } - document.on("click", "a[data-method]", function(event, element) { - if (event.stopped) return; - handleMethod(element); - event.stop(); + if (link.readAttribute('data-remote')) { + handleRemote(link); + event.stop(); + } else if (link.readAttribute('data-method')) { + handleMethod(link); + event.stop(); + } }); - document.on("click", "form input[type=submit]", function(event, button) { + document.on("click", "form input[type=submit], form button[type=submit], form button:not([type])", function(event, button) { // register the pressed submit button event.findElement('form').store('rails:submit-button', button.name || false); }); document.on("submit", function(event) { - var form = event.findElement(), - message = form.readAttribute('data-confirm'); + var form = event.findElement(); - if (message && !confirm(message)) { + if (!allowAction(form)) { event.stop(); return false; } - form.select('input[type=submit][data-disable-with]').each(function(input) { - input.store('rails:original-value', input.getValue()); - input.disable().setValue(input.readAttribute('data-disable-with')); - }); - if (form.readAttribute('data-remote')) { handleRemote(form); event.stop(); + } else { + disableFormElements(form); } }); - document.on("ajax:after", "form", function(event, form) { - form.select('input[type=submit][data-disable-with]').each(function(input) { - input.setValue(input.retrieve('rails:original-value')).enable(); - }); + document.on('ajax:create', 'form', function(event, form) { + if (form == event.findElement()) disableFormElements(form); + }); + + document.on('ajax:complete', 'form', function(event, form) { + if (form == event.findElement()) enableFormElements(form); }); })(); |