aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
-rw-r--r--actioncable/lib/action_cable/connection/base.rb2
-rw-r--r--actioncable/lib/action_cable/server/broadcasting.rb2
-rw-r--r--actionview/CHANGELOG.md2
-rw-r--r--activerecord/CHANGELOG.md21
-rw-r--r--activerecord/lib/active_record/callbacks.rb2
-rw-r--r--activerecord/lib/active_record/relation/finder_methods.rb30
-rw-r--r--activerecord/lib/active_record/relation/query_methods.rb2
-rw-r--r--activerecord/lib/active_record/sanitization.rb2
-rw-r--r--activerecord/lib/active_record/validations.rb2
-rw-r--r--activerecord/test/cases/finder_test.rb46
-rw-r--r--guides/source/action_mailer_basics.md2
-rw-r--r--guides/source/action_view_overview.md2
-rw-r--r--guides/source/active_model_basics.md2
-rw-r--r--guides/source/engines.md2
-rw-r--r--guides/source/layouts_and_rendering.md2
-rw-r--r--guides/source/upgrading_ruby_on_rails.md2
16 files changed, 92 insertions, 31 deletions
diff --git a/actioncable/lib/action_cable/connection/base.rb b/actioncable/lib/action_cable/connection/base.rb
index e23789978c..1acef93025 100644
--- a/actioncable/lib/action_cable/connection/base.rb
+++ b/actioncable/lib/action_cable/connection/base.rb
@@ -35,7 +35,7 @@ module ActionCable
#
# First, we declare that this connection can be identified by its current_user. This allows us later to be able to find all connections
# established for that current_user (and potentially disconnect them if the user was removed from an account). You can declare as many
- # identification indexes as you like. Declaring an identification means that a attr_accessor is automatically set for that key.
+ # identification indexes as you like. Declaring an identification means that an attr_accessor is automatically set for that key.
#
# Second, we rely on the fact that the WebSocket connection is established with the cookies from the domain being sent along. This makes
# it easy to use signed cookies that were set when logging in via a web interface to authorize the WebSocket connection.
diff --git a/actioncable/lib/action_cable/server/broadcasting.rb b/actioncable/lib/action_cable/server/broadcasting.rb
index 7e8aef45f4..b87232671b 100644
--- a/actioncable/lib/action_cable/server/broadcasting.rb
+++ b/actioncable/lib/action_cable/server/broadcasting.rb
@@ -23,7 +23,7 @@ module ActionCable
broadcaster_for(broadcasting).broadcast(message)
end
- # Returns a broadcaster for a named <tt>broadcasting</tt> that can be reused. Useful when you have a object that
+ # Returns a broadcaster for a named <tt>broadcasting</tt> that can be reused. Useful when you have an object that
# may need multiple spots to transmit to a specific broadcasting over and over.
def broadcaster_for(broadcasting)
Broadcaster.new(self, broadcasting)
diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md
index 256b90784a..bebe78c360 100644
--- a/actionview/CHANGELOG.md
+++ b/actionview/CHANGELOG.md
@@ -68,7 +68,7 @@
*Vasiliy Ermolovich*
-* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising a error
+* Add a `hidden_field` on the `collection_radio_buttons` to avoid raising an error
when the only input on the form is the `collection_radio_buttons`.
*Mauro George*
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md
index 53538f4157..f13a80195a 100644
--- a/activerecord/CHANGELOG.md
+++ b/activerecord/CHANGELOG.md
@@ -1,3 +1,24 @@
+* Rework `ActiveRecord::Relation#last`
+
+ 1. Never perform additional SQL on loaded relation
+ 2. Use SQL reverse order instead of loading relation if relation doesn't have limit
+ 3. Deprecated relation loading when SQL order can not be automatically reversed
+
+ Topic.order("title").load.last(3)
+ # before: SELECT ...
+ # after: No SQL
+
+ Topic.order("title").last
+ # before: SELECT * FROM `topics`
+ # after: SELECT * FROM `topics` ORDER BY `topics`.`title` DESC LIMIT 1
+
+ Topic.order("coalesce(author, title)").last
+ # before: SELECT * FROM `topics`
+ # after: Deprecation Warning for irreversible order
+
+ *Bogdan Gusiev*
+
+
* Allow `joins` to be unscoped.
Closes #13775.
diff --git a/activerecord/lib/active_record/callbacks.rb b/activerecord/lib/active_record/callbacks.rb
index 854f9776a3..1f1b11eb68 100644
--- a/activerecord/lib/active_record/callbacks.rb
+++ b/activerecord/lib/active_record/callbacks.rb
@@ -179,7 +179,7 @@ module ActiveRecord
#
# If the +before_validation+ callback throws +:abort+, the process will be
# aborted and {ActiveRecord::Base#save}[rdoc-ref:Persistence#save] will return +false+.
- # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise a ActiveRecord::RecordInvalid exception.
+ # If {ActiveRecord::Base#save!}[rdoc-ref:Persistence#save!] is called it will raise an ActiveRecord::RecordInvalid exception.
# Nothing will be appended to the errors object.
#
# == Canceling callbacks
diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb
index a4280c5f33..8b114c6bdf 100644
--- a/activerecord/lib/active_record/relation/finder_methods.rb
+++ b/activerecord/lib/active_record/relation/finder_methods.rb
@@ -145,15 +145,21 @@ module ActiveRecord
#
# [#<Person id:4>, #<Person id:3>, #<Person id:2>]
def last(limit = nil)
- if limit
- if order_values.empty? && primary_key
- order(arel_attribute(primary_key).desc).limit(limit).reverse
- else
- to_a.last(limit)
- end
- else
- find_last
- end
+ return find_last(limit) if loaded? || limit_value
+
+ result = limit(limit || 1)
+ result.order!(arel_attribute(primary_key)) if order_values.empty? && primary_key
+ result = result.reverse_order!
+
+ limit ? result.reverse : result.first
+ rescue ActiveRecord::IrreversibleOrderError
+ ActiveSupport::Deprecation.warn(<<-WARNING.squish)
+ Finding a last element by loading the relation when SQL ORDER
+ can not be reversed is deprecated.
+ Rails 5.1 will raise ActiveRecord::IrreversibleOrderError in this case.
+ Please call `to_a.last` if you still want to load the relation.
+ WARNING
+ find_last(limit)
end
# Same as #last but raises ActiveRecord::RecordNotFound if no record
@@ -330,7 +336,7 @@ module ActiveRecord
end
# This method is called whenever no records are found with either a single
- # id or multiple ids and raises a ActiveRecord::RecordNotFound exception.
+ # id or multiple ids and raises an ActiveRecord::RecordNotFound exception.
#
# The error message is different depending on whether a single id or
# multiple ids are provided. If multiple ids are provided, then the number
@@ -578,5 +584,9 @@ module ActiveRecord
find_nth_with_limit(index, limit)
end
end
+
+ def find_last(limit)
+ limit ? to_a.last(limit) : to_a.last
+ end
end
end
diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb
index 91bfa4d131..91d486e902 100644
--- a/activerecord/lib/active_record/relation/query_methods.rb
+++ b/activerecord/lib/active_record/relation/query_methods.rb
@@ -1112,6 +1112,8 @@ module ActiveRecord
order_query.flat_map do |o|
case o
+ when Arel::Attribute
+ o.desc
when Arel::Nodes::Ordering
o.reverse
when String
diff --git a/activerecord/lib/active_record/sanitization.rb b/activerecord/lib/active_record/sanitization.rb
index 2bfc5ff7ae..a9e1fd0dad 100644
--- a/activerecord/lib/active_record/sanitization.rb
+++ b/activerecord/lib/active_record/sanitization.rb
@@ -60,7 +60,7 @@ module ActiveRecord
end
# Accepts an array, or string of SQL conditions and sanitizes
- # them into a valid SQL fragment for a ORDER clause.
+ # them into a valid SQL fragment for an ORDER clause.
#
# sanitize_sql_for_order(["field(id, ?)", [1,3,2]])
# # => "field(id, 1,3,2)"
diff --git a/activerecord/lib/active_record/validations.rb b/activerecord/lib/active_record/validations.rb
index 6677e6dc5f..ecaf04e39e 100644
--- a/activerecord/lib/active_record/validations.rb
+++ b/activerecord/lib/active_record/validations.rb
@@ -45,7 +45,7 @@ module ActiveRecord
end
# Attempts to save the record just like {ActiveRecord::Base#save}[rdoc-ref:Base#save] but
- # will raise a ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid.
+ # will raise an ActiveRecord::RecordInvalid exception instead of returning +false+ if the record is not valid.
def save!(options={})
perform_validations(options) ? super : raise_validation_error
end
diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb
index 75a74c052d..5b41630cd8 100644
--- a/activerecord/test/cases/finder_test.rb
+++ b/activerecord/test/cases/finder_test.rb
@@ -101,7 +101,7 @@ class FinderTest < ActiveRecord::TestCase
def test_find_with_ids_where_and_limit
# Please note that Topic 1 is the only not approved so
- # if it were among the first 3 it would raise a ActiveRecord::RecordNotFound
+ # if it were among the first 3 it would raise an ActiveRecord::RecordNotFound
records = Topic.where(approved: true).limit(3).find([3,2,5,1,4])
assert_equal 3, records.size
assert_equal 'The Third Topic of the day', records[0].title
@@ -516,16 +516,44 @@ class FinderTest < ActiveRecord::TestCase
assert_equal Topic.order("title").to_a.last(2), Topic.order("title").last(2)
end
- def test_last_with_integer_and_order_should_not_use_sql_limit
- query = assert_sql { Topic.order("title").last(5).entries }
- assert_equal 1, query.length
- assert_no_match(/LIMIT/, query.first)
+ def test_last_with_integer_and_order_should_use_sql_limit
+ relation = Topic.order("title")
+ assert_queries(1) { relation.last(5) }
+ assert !relation.loaded?
end
- def test_last_with_integer_and_reorder_should_not_use_sql_limit
- query = assert_sql { Topic.reorder("title").last(5).entries }
- assert_equal 1, query.length
- assert_no_match(/LIMIT/, query.first)
+ def test_last_with_integer_and_reorder_should_use_sql_limit
+ relation = Topic.reorder("title")
+ assert_queries(1) { relation.last(5) }
+ assert !relation.loaded?
+ end
+
+ def test_last_on_loaded_relation_should_not_use_sql
+ relation = Topic.limit(10).load
+ assert_no_queries do
+ relation.last
+ relation.last(2)
+ end
+ end
+
+ def test_last_with_irreversible_order
+ assert_deprecated do
+ Topic.order("coalesce(author_name, title)").last
+ end
+ end
+
+ def test_last_on_relation_with_limit_and_offset
+ post = posts('sti_comments')
+
+ comments = post.comments.order(id: :asc)
+ assert_equal comments.limit(2).to_a.last, comments.limit(2).last
+ assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2)
+ assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3)
+
+ comments = comments.offset(1)
+ assert_equal comments.limit(2).to_a.last, comments.limit(2).last
+ assert_equal comments.limit(2).to_a.last(2), comments.limit(2).last(2)
+ assert_equal comments.limit(2).to_a.last(3), comments.limit(2).last(3)
end
def test_take_and_first_and_last_with_integer_should_return_an_array
diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md
index cb9b6af214..91ea4efb55 100644
--- a/guides/source/action_mailer_basics.md
+++ b/guides/source/action_mailer_basics.md
@@ -222,7 +222,7 @@ class SendWeeklySummary
end
```
-The method `welcome_email` returns a `ActionMailer::MessageDelivery` object which
+The method `welcome_email` returns an `ActionMailer::MessageDelivery` object which
can then just be told `deliver_now` or `deliver_later` to send itself out. The
`ActionMailer::MessageDelivery` object is just a wrapper around a `Mail::Message`. If
you want to inspect, alter or do anything else with the `Mail::Message` object you can
diff --git a/guides/source/action_view_overview.md b/guides/source/action_view_overview.md
index 543937f8e5..5e6eae1071 100644
--- a/guides/source/action_view_overview.md
+++ b/guides/source/action_view_overview.md
@@ -1524,7 +1524,7 @@ Localized Views
Action View has the ability to render different templates depending on the current locale.
-For example, suppose you have a `ArticlesController` with a show action. By default, calling this action will render `app/views/articles/show.html.erb`. But if you set `I18n.locale = :de`, then `app/views/articles/show.de.html.erb` will be rendered instead. If the localized template isn't present, the undecorated version will be used. This means you're not required to provide localized views for all cases, but they will be preferred and used if available.
+For example, suppose you have an `ArticlesController` with a show action. By default, calling this action will render `app/views/articles/show.html.erb`. But if you set `I18n.locale = :de`, then `app/views/articles/show.de.html.erb` will be rendered instead. If the localized template isn't present, the undecorated version will be used. This means you're not required to provide localized views for all cases, but they will be preferred and used if available.
You can use the same technique to localize the rescue files in your public directory. For example, setting `I18n.locale = :de` and creating `public/500.de.html` and `public/404.de.html` would allow you to have localized rescue pages.
diff --git a/guides/source/active_model_basics.md b/guides/source/active_model_basics.md
index c05e20aceb..a8199e5d02 100644
--- a/guides/source/active_model_basics.md
+++ b/guides/source/active_model_basics.md
@@ -319,7 +319,7 @@ person.serializable_hash # => {"name"=>"Bob"}
#### ActiveModel::Serializers
-Rails provides a `ActiveModel::Serializers::JSON` serializer.
+Rails provides an `ActiveModel::Serializers::JSON` serializer.
This module automatically include the `ActiveModel::Serialization`.
##### ActiveModel::Serializers::JSON
diff --git a/guides/source/engines.md b/guides/source/engines.md
index 415def8367..db50ad278f 100644
--- a/guides/source/engines.md
+++ b/guides/source/engines.md
@@ -461,7 +461,7 @@ model, a comment controller and then modify the articles scaffold to display
comments and allow people to create new ones.
From the application root, run the model generator. Tell it to generate a
-`Comment` model, with the related table having two columns: a `article_id` integer
+`Comment` model, with the related table having two columns: an `article_id` integer
and `text` text column.
```bash
diff --git a/guides/source/layouts_and_rendering.md b/guides/source/layouts_and_rendering.md
index 6946eb81eb..83173e8d75 100644
--- a/guides/source/layouts_and_rendering.md
+++ b/guides/source/layouts_and_rendering.md
@@ -555,7 +555,7 @@ class Admin::ProductsController < AdminController
end
```
-The lookup order for a `admin/products#index` action will be:
+The lookup order for an `admin/products#index` action will be:
* `app/views/admin/products/`
* `app/views/admin/`
diff --git a/guides/source/upgrading_ruby_on_rails.md b/guides/source/upgrading_ruby_on_rails.md
index cef83e4264..0dfa4f1cb8 100644
--- a/guides/source/upgrading_ruby_on_rails.md
+++ b/guides/source/upgrading_ruby_on_rails.md
@@ -183,7 +183,7 @@ the logs. In the next version, these errors will no longer be suppressed.
Instead, the errors will propagate normally just like in other Active
Record callbacks.
-When you define a `after_rollback` or `after_commit` callback, you
+When you define an `after_rollback` or `after_commit` callback, you
will receive a deprecation warning about this upcoming change. When
you are ready, you can opt into the new behavior and remove the
deprecation warning by adding following configuration to your