diff options
18 files changed, 157 insertions, 46 deletions
@@ -47,7 +47,6 @@ platforms :mri do end platforms :ruby do - gem 'json' gem 'yajl-ruby' gem 'nokogiri', '>= 1.4.5' diff --git a/actionmailer/CHANGELOG.md b/actionmailer/CHANGELOG.md index 68b34805ff..ebce32bf11 100644 --- a/actionmailer/CHANGELOG.md +++ b/actionmailer/CHANGELOG.md @@ -1,5 +1,10 @@ ## Rails 4.0.0 (unreleased) ## +* Do not render views when mail() isn't called. + Fix #7761 + + *Yves Senn* + * Support `Mailer.deliver_foo(*args)` as a synonym for `Mailer.foo(*args).deliver`. This makes it easy to write e.g. `Mailer.expects(:deliver_foo)` when testing code that calls diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index fee4a64248..4d1c697502 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -510,7 +510,19 @@ module ActionMailer def process(*args) #:nodoc: lookup_context.skip_default_locale! - super + + generated_mail = super + unless generated_mail + @_message = NullMail.new + end + end + + class NullMail #:nodoc: + def body; '' end + + def method_missing(*args) + nil + end end def mailer_name diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 1cb3ce63fe..b30ec2ddc9 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -499,6 +499,12 @@ class BaseTest < ActiveSupport::TestCase end end + test 'the view is not rendered when mail was never called' do + mail = BaseMailer.without_mail_call + assert_equal('', mail.body.to_s.strip) + mail.deliver + end + # Before and After hooks class MyObserver diff --git a/actionmailer/test/fixtures/base_mailer/without_mail_call.erb b/actionmailer/test/fixtures/base_mailer/without_mail_call.erb new file mode 100644 index 0000000000..290379d5fb --- /dev/null +++ b/actionmailer/test/fixtures/base_mailer/without_mail_call.erb @@ -0,0 +1 @@ +<% raise 'the template should not be rendered' %>
\ No newline at end of file diff --git a/actionmailer/test/mailers/base_mailer.rb b/actionmailer/test/mailers/base_mailer.rb index f25d9b9aff..52342bc59e 100644 --- a/actionmailer/test/mailers/base_mailer.rb +++ b/actionmailer/test/mailers/base_mailer.rb @@ -115,4 +115,7 @@ class BaseMailer < ActionMailer::Base def email_with_translations mail body: render("email_with_translations", formats: [:html]) end + + def without_mail_call + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 815902a129..4916777ce7 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,33 @@ ## Rails 4.0.0 (unreleased) ## +* The `create_table` method raises an `ArgumentError` when the primary key column is redefined. + Fix #6378 + + *Yves Senn* + +* `ActiveRecord::AttributeMethods#[]` raises `ActiveModel::MissingAttributeError` + error if the given attribute is missing. Fixes #5433. + + class Person < ActiveRecord::Base + belongs_to :company + end + + # Before: + person = Person.select('id').first + person[:name] # => nil + person.name # => ActiveModel::MissingAttributeError: missing_attribute: name + person[:company_id] # => nil + person.company # => nil + + # After: + person = Person.select('id').first + person[:name] # => ActiveModel::MissingAttributeError: missing_attribute: name + person.name # => ActiveModel::MissingAttributeError: missing_attribute: name + person[:company_id] # => ActiveModel::MissingAttributeError: missing_attribute: company_id + person.company # => ActiveModel::MissingAttributeError: missing_attribute: company_id + + *Francesco Rodriguez* + * Small binary fields use the `VARBINARY` MySQL type, instead of `TINYBLOB`. *Victor Costan* @@ -51,7 +79,7 @@ *Scott Willson* -* Fix bug where sum(expression) returns string '0' for no matching records +* Fix bug where sum(expression) returns string '0' for no matching records. Fixes #7439 *Tim Macfarlane* diff --git a/activerecord/lib/active_record/attribute_methods.rb b/activerecord/lib/active_record/attribute_methods.rb index 9a4fcdfda3..101c641877 100644 --- a/activerecord/lib/active_record/attribute_methods.rb +++ b/activerecord/lib/active_record/attribute_methods.rb @@ -266,16 +266,22 @@ module ActiveRecord # Returns the value of the attribute identified by <tt>attr_name</tt> after it has been typecast (for example, # "2004-12-12" in a data column is cast to a date object, like Date.new(2004, 12, 12)). - # (Alias for the protected <tt>read_attribute</tt> method). + # (Alias for the protected <tt>read_attribute</tt> method). It raises an <tt>ActiveModel::MissingAttributeError</tt> + # error if the identified attribute is missing. # # class Person < ActiveRecord::Base + # belongs_to :organization # end # # person = Person.new(name: 'Francesco', age: '22' # person[:name] # => "Francesco" # person[:age] # => 22 + # + # person = Person.select('id').first + # person[:name] # => ActiveModel::MissingAttributeError: missing attribute: name + # person[:organization_id] # => ActiveModel::MissingAttributeError: missing attribute: organization_id def [](attr_name) - read_attribute(attr_name) + read_attribute(attr_name) { |n| missing_attribute(n, caller) } end # Updates the attribute identified by <tt>attr_name</tt> with the specified +value+. diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index dca355aa93..0f6b177b62 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -234,6 +234,10 @@ module ActiveRecord name = name.to_s type = type.to_sym + if primary_key_column_name == name + raise ArgumentError, "you can't redefine the primary key column '#{name}'. To define a custom primary key, pass { id: false } to create_table." + end + column = self[name] || new_column_definition(@base, name, type) limit = options.fetch(:limit) do @@ -302,6 +306,11 @@ module ActiveRecord definition end + def primary_key_column_name + primary_key_column = columns.detect { |c| c.type == :primary_key } + primary_key_column && primary_key_column.name + end + def native @base.native_database_types end diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 4d5cb72c67..c5a859475f 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -519,24 +519,22 @@ module ActiveRecord def copy_table(from, to, options = {}) #:nodoc: from_primary_key = primary_key(from) - options[:primary_key] = from_primary_key if from_primary_key != 'id' - unless options[:primary_key] - options[:id] = columns(from).detect{|c| c.name == 'id'}.present? && from_primary_key == 'id' - end + options[:id] = false create_table(to, options) do |definition| @definition = definition + @definition.primary_key(from_primary_key) if from_primary_key.present? columns(from).each do |column| column_name = options[:rename] ? (options[:rename][column.name] || options[:rename][column.name.to_sym] || column.name) : column.name + next if column_name == from_primary_key @definition.column(column_name, column.type, :limit => column.limit, :default => column.default, :precision => column.precision, :scale => column.scale, :null => column.null) end - @definition.primary_key(from_primary_key) if from_primary_key yield @definition if block_given? end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index d5ee98382d..5c5417a0e1 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -342,7 +342,9 @@ module ActiveRecord end def call(env) - ActiveRecord::Migration.check_pending! + ActiveRecord::Base.logger.quietly do + ActiveRecord::Migration.check_pending! + end @app.call(env) end end diff --git a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb index 74288a98d1..d03d1dd94c 100644 --- a/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/copy_table_test.rb @@ -32,6 +32,11 @@ class CopyTableTest < ActiveRecord::TestCase end end + def test_copy_table_allows_to_pass_options_to_create_table + @connection.create_table('blocker_table') + test_copy_table('customers', 'blocker_table', force: true) + end + def test_copy_table_with_index test_copy_table('comments', 'comments_with_index') do @connection.add_index('comments_with_index', ['post_id', 'type']) @@ -43,7 +48,9 @@ class CopyTableTest < ActiveRecord::TestCase end def test_copy_table_without_primary_key - test_copy_table('developers_projects', 'programmers_projects') + test_copy_table('developers_projects', 'programmers_projects') do + assert_nil @connection.primary_key('programmers_projects') + end end def test_copy_table_with_id_col_that_is_not_primary_key diff --git a/activerecord/test/cases/attribute_methods_test.rb b/activerecord/test/cases/attribute_methods_test.rb index c2b58fd7d1..8b82b79219 100644 --- a/activerecord/test/cases/attribute_methods_test.rb +++ b/activerecord/test/cases/attribute_methods_test.rb @@ -287,6 +287,12 @@ class AttributeMethodsTest < ActiveRecord::TestCase assert_equal "Don't change the topic", topic[:title] end + def test_read_attribute_raises_missing_attribute_error_when_not_exists + computer = Computer.select('id').first + assert_raises(ActiveModel::MissingAttributeError) { computer[:developer] } + assert_raises(ActiveModel::MissingAttributeError) { computer[:extendedWarranty] } + end + def test_read_attribute_when_false topic = topics(:first) topic.approved = false diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 17c1634444..86451289e7 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -132,6 +132,26 @@ module ActiveRecord assert_equal %w(foo testingid), connection.columns(:testings).map(&:name).sort end + def test_create_table_raises_when_redefining_primary_key_column + error = assert_raise(ArgumentError) do + connection.create_table :testings do |t| + t.column :id, :string + end + end + + assert_equal "you can't redefine the primary key column 'id'. To define a custom primary key, pass { id: false } to create_table.", error.message + end + + def test_create_table_raises_when_redefining_custom_primary_key_column + error = assert_raise(ArgumentError) do + connection.create_table :testings, primary_key: :testing_id do |t| + t.column :testing_id, :string + end + end + + assert_equal "you can't redefine the primary key column 'testing_id'. To define a custom primary key, pass { id: false } to create_table.", error.message + end + def test_create_table_with_timestamps_should_create_datetime_columns connection.create_table table_name do |t| t.timestamps diff --git a/activesupport/test/core_ext/module/qualified_const_test.rb b/activesupport/test/core_ext/module/qualified_const_test.rb index 343a848a42..37c9228a64 100644 --- a/activesupport/test/core_ext/module/qualified_const_test.rb +++ b/activesupport/test/core_ext/module/qualified_const_test.rb @@ -89,13 +89,20 @@ class QualifiedConstTest < ActiveSupport::TestCase end test "reject absolute paths" do - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")} - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_defined?("::X::Y")} - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")} - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X")} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_get("::X::Y")} - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)} - assert_raise(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X", nil)} + assert_raise_with_message(NameError, "wrong constant name ::X") { Object.qualified_const_set("::X::Y", nil)} + end + + private + + def assert_raise_with_message(expected_exception, expected_message, &block) + exception = assert_raise(expected_exception, &block) + assert_equal expected_message, exception.message end end diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index e5bc806397..670a04e5df 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -147,7 +147,8 @@ class DependenciesTest < ActiveSupport::TestCase def test_circular_autoloading_detection with_autoloading_fixtures do - assert_raise(RuntimeError, "Circular dependency detected while autoloading constant Circular1") { Circular1 } + e = assert_raise(RuntimeError) { Circular1 } + assert_equal "Circular dependency detected while autoloading constant Circular1", e.message end end diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index ec1f1d4df1..f5db76f217 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -900,7 +900,7 @@ end } ``` -The keys of the `:addresses_attributes` hash are unimportant, they need merely be different for each address. +The keys of the `:addresses_attributes` hash are unimportant, they need merely be different for each address. If the associated object is already saved, `fields_for` autogenerates a hidden input with the `id` of the saved record. You can disable this by passing `:include_id => false` to `fields_for`. You may wish to do this if the autogenerated input is placed in a location where an input tag is not valid HTML or when using an ORM where children do not have an id. diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 5a53d03423..bb736eb670 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -39,7 +39,7 @@ to vanilla JavaScript as well. As an example, here's some CoffeeScript code that makes an Ajax request using the jQuery library: -``` +```coffeescript $.ajax(url: "/test").done (html) -> $("#results").append html ``` @@ -63,35 +63,35 @@ demonstrate other ways. Here's the simplest way to write JavaScript. You may see it referred to as 'inline JavaScript': -``` +```html <a href="#" onclick="alert('Hello, world.')">Here</a> ``` When clicked, the alert will trigger. Here's the problem: what happens when we have lots of JavaScript we want to execute on a click? -``` +```html <a href="#" onclick="function fib(n){return n<2?n:fib(n-1)+fib(n-2);};alert('fib of 15 is: ' + fib(15) + '.');">Calculate</a> ``` Awkward, right? We could pull the function definition out of the click handler, and turn it into CoffeeScript: -``` +```coffeescript fib = (n) -> (if n < 2 then n else fib(n - 1) + fib(n - 2)) ``` And then on our page: -``` +```html <a href="#" onclick="alert('fib of 15 is: ' + fib(15) + '.');">Calculate</a> ``` That's a little bit better, but what about multiple links that have the same effect? -``` +```html <a href="#" onclick="alert('fib of 16 is: ' + fib(16) + '.');">Calculate</a> <a href="#" onclick="alert('fib of 17 is: ' + fib(17) + '.');">Calculate</a> <a href="#" onclick="alert('fib of 18 is: ' + fib(18) + '.');">Calculate</a> @@ -101,7 +101,7 @@ Not very DRY, eh? We can fix this by using events instead. We'll add a `data-*` attribute to our link, and then bind a handler to the click event of every link that has that attribute: -``` +```coffeescript fib = (n) -> (if n < 2 then n else fib(n - 1) + fib(n - 2)) @@ -149,7 +149,7 @@ attributes, and attaches appropriate handlers. is a helper that assists with writing forms. `form_for` takes a `:remote` option. It works like this: -``` +```erb <%= form_for(@post, remote: true) do |f| %> ... <% end %> @@ -157,7 +157,7 @@ option. It works like this: This will generate the following HTML: -``` +```html <form accept-charset="UTF-8" action="/posts" class="new_post" data-remote="true" id="new_post" method="post"> ... </form> @@ -170,14 +170,12 @@ You probably don't want to just sit there with a filled out `<form>`, though. You probably want to do something upon a successful submission. To do that, bind to the `ajax:success` event. On failure, use `ajax:error`. Check it out: -``` -<script> +```coffeescript $(document).ready -> $("#new_post").on("ajax:success", (e, data, status, xhr) -> $("#new_post").append xhr.responseText ).bind "ajax:error", (e, xhr, status, error) -> $("#new_post").append "<p>ERROR</p>" -</script> ``` Obviously, you'll want to be a bit more sophisticated than that, but it's a @@ -189,7 +187,7 @@ start. is very similar to `form_for`. It has a `:remote` option that you can use like this: -``` +```erb <%= form_tag('/posts', remote: true) %> ``` @@ -202,13 +200,13 @@ details. is a helper that assists with generating links. It has a `:remote` option you can use like this: -``` +```erb <%= link_to "first post", @post, remote: true %> ``` which generates -``` +```html <a href="/posts/1" data-remote="true">a post</a> ``` @@ -216,11 +214,13 @@ You can bind to the same Ajax events as `form_for`. Here's an example. Let's assume that we have a resource `/fib/:n` that calculates the `n`th Fibonacci number. We would generate some HTML like this: +```erb <%= link_to "Calculate", "/fib/15", remote: true, data: { fib: 15 } %> +``` and write some CoffeeScript like this: -``` +```coffeescript $(document).ready -> $("a[data-fib]").on "ajax:success", (e, data, status, xhr) -> count = $(this).data("fib") @@ -231,14 +231,15 @@ $(document).ready -> [`button_to`](http://api.rubyonrails.org/classes/ActionView/Helpers/UrlHelper.html#method-i-button_to) is a helper that helps you create buttons. It has a `:remote` option that you can call like this: -``` +```erb <%= button_to "A post", @post, remote: true %> ``` this generates -``` +```html <form action="/posts/1" class="button_to" data-remote="true" method="post"> + <div><input type="submit" value="A post"></div> </form> ``` @@ -257,7 +258,7 @@ Imagine you have a series of users that you would like to display and provide a form on that same page to create a new user. The index action of your controller looks like this: -``` +```ruby class UsersController < ApplicationController def index @users = User.all @@ -268,7 +269,7 @@ class UsersController < ApplicationController The index view (`app/views/users/index.html.erb`) contains: -``` +```erb <b>Users</b> <ul id="users"> @@ -288,7 +289,7 @@ The index view (`app/views/users/index.html.erb`) contains: The `app/views/users/_user.html.erb` partial contains the following: -``` +```erb <li><%= user.name %></li> ``` @@ -301,7 +302,7 @@ users controller as an Ajax request, looking for JavaScript. In order to service that request, the create action of your controller would look like this: -``` +```ruby # app/controllers/users_controller.rb # ...... def create @@ -325,7 +326,7 @@ respond to your Ajax request. You then have a corresponding `app/views/users/create.js.erb` view file that generates the actual JavaScript code that will be sent and executed on the client side. -``` +```erb $("<%= escape_javascript(render @user) %>").appendTo("#users"); ``` @@ -352,7 +353,7 @@ and put `//= require turbolinks` in your CoffeeScript manifest, which is usually If you want to disable Turbolinks for certain links, add a `data-no-turbolink` attribute to the tag: -``` +```html <a href="..." data-no-turbolink>No turbolinks here</a>. ``` @@ -361,7 +362,7 @@ attribute to the tag: When writing CoffeeScript, you'll often want to do some sort of processing upon page load. With jQuery, you'd write something like this: -``` +```coffeescript $(document).ready -> alert "page has loaded!" ``` @@ -370,7 +371,7 @@ However, because Turbolinks overrides the normal page loading process, the event that this relies on will not be fired. If you have code that looks like this, you must change your code to do this instead: -``` +```coffeescript $(document).on "page:change", -> alert "page has loaded!" ``` |