diff options
| -rw-r--r-- | activerecord/CHANGELOG.md | 7 | ||||
| -rw-r--r-- | activerecord/lib/active_record/relation/finder_methods.rb | 17 | ||||
| -rw-r--r-- | activerecord/test/cases/base_test.rb | 3 | ||||
| -rw-r--r-- | activerecord/test/cases/finder_test.rb | 75 |
4 files changed, 100 insertions, 2 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2c5b670469..dc5d2632dc 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,10 @@ +* Order the result of `find(ids)` to match the passed array, if the relation + has no explicit order defined. + + Fixes #20338. + + *Miguel Grazziotin*, *Matthew Draper* + * Omit default limit values in dumped schema. It's tidier, and if the defaults change in the future, we can address that via Migration API Versioning. diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 435cef901b..19244bcf95 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -442,6 +442,8 @@ module ActiveRecord end def find_some(ids) + return find_some_ordered(ids) unless order_values.present? + result = where(primary_key => ids).to_a expected_size = @@ -463,6 +465,21 @@ module ActiveRecord end end + def find_some_ordered(ids) + ids = ids.slice(offset_value || 0, limit_value || ids.size) || [] + + result = except(:limit, :offset).where(primary_key => ids).to_a + + if result.size == ids.size + pk_type = @klass.type_for_attribute(primary_key) + + records_by_id = result.index_by(&:id) + ids.map { |id| records_by_id.fetch(pk_type.cast(id)) } + else + raise_record_not_found_exception!(ids, result.size, ids.size) + end + end + def find_take if loaded? @records.first diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index e628254b44..79791af187 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -525,7 +525,8 @@ class BasicsTest < ActiveRecord::TestCase end def test_find_by_slug_with_array - assert_equal Topic.find(['1-meowmeow', '2-hello']), Topic.find([1, 2]) + assert_equal Topic.find([1, 2]), Topic.find(['1-meowmeow', '2-hello']) + assert_equal 'The Second Topic of the day', Topic.find(['2-hello', '1-meowmeow']).first.title end def test_find_by_slug_with_range diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 73f5312eba..9f90ab79a1 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -48,6 +48,75 @@ class FinderTest < ActiveRecord::TestCase end end + def test_find_with_ids_returning_ordered + records = Topic.find([4,2,5]) + assert_equal 'The Fourth Topic of the day', records[0].title + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + + records = Topic.find(4,2,5) + assert_equal 'The Fourth Topic of the day', records[0].title + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + + records = Topic.find(['4','2','5']) + assert_equal 'The Fourth Topic of the day', records[0].title + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + + records = Topic.find('4','2','5') + assert_equal 'The Fourth Topic of the day', records[0].title + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + end + + def test_find_with_ids_and_order_clause + # The order clause takes precedence over the informed ids + records = Topic.order(:author_name).find([5,3,1]) + assert_equal 'The Third Topic of the day', records[0].title + assert_equal 'The First Topic', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + + records = Topic.order(:id).find([5,3,1]) + assert_equal 'The First Topic', records[0].title + assert_equal 'The Third Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + end + + def test_find_with_ids_with_limit_and_order_clause + # The order clause takes precedence over the informed ids + records = Topic.limit(2).order(:id).find([5,3,1]) + assert_equal 2, records.size + assert_equal 'The First Topic', records[0].title + assert_equal 'The Third Topic of the day', records[1].title + end + + def test_find_with_ids_and_limit + records = Topic.limit(3).find([3,2,5,1,4]) + assert_equal 3, records.size + assert_equal 'The Third Topic of the day', records[0].title + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + end + + 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 + 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 + assert_equal 'The Second Topic of the day', records[1].title + assert_equal 'The Fifth Topic of the day', records[2].title + end + + def test_find_with_ids_and_offset + records = Topic.offset(2).find([3,2,5,1,4]) + assert_equal 3, records.size + assert_equal 'The Fifth Topic of the day', records[0].title + assert_equal 'The First Topic', records[1].title + assert_equal 'The Fourth Topic of the day', records[2].title + end + def test_find_passing_active_record_object_is_deprecated assert_deprecated do Topic.find(Topic.last) @@ -195,7 +264,9 @@ class FinderTest < ActiveRecord::TestCase def test_find_by_ids_with_limit_and_offset assert_equal 2, Entrant.limit(2).find([1,3,2]).size - assert_equal 1, Entrant.limit(3).offset(2).find([1,3,2]).size + entrants = Entrant.limit(3).offset(2).find([1,3,2]) + assert_equal 1, entrants.size + assert_equal 'Ruby Guru', entrants.first.name # Also test an edge case: If you have 11 results, and you set a # limit of 3 and offset of 9, then you should find that there @@ -203,6 +274,8 @@ class FinderTest < ActiveRecord::TestCase devs = Developer.all last_devs = Developer.limit(3).offset(9).find devs.map(&:id) assert_equal 2, last_devs.size + assert_equal 'fixture_10', last_devs[0].name + assert_equal 'Jamis', last_devs[1].name end def test_find_with_large_number |