Skip to content
This repository
Browse code

Be sure to use the @finder_sql in the has_many association's #find me…

…thod, even if explicit conditions have not been given.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@1412 5ecf4fe2-1ee6-0310-87b1-e25e094e27de
  • Loading branch information...
commit 37a370d8d4e3e1a5bf707ffac9cd188c07572248 1 parent e0537ac
Jamis Buck jamis authored
6 activerecord/lib/active_record/associations/has_many_association.rb
@@ -66,9 +66,11 @@ def find(*args)
66 66 load_target.select { |record| ids.include?(record.id) }
67 67 end
68 68 else
69   - if options[:conditions] = sanitize_sql(options[:conditions])
70   - options[:conditions] = "#{@finder_sql} AND #{options[:conditions]}"
  69 + conditions = "#{@finder_sql}"
  70 + if sanitized_conditions = sanitize_sql(options[:conditions])
  71 + conditions << " AND #{sanitized_conditions}"
71 72 end
  73 + options[:conditions] = conditions
72 74
73 75 if options[:order] && @options[:order]
74 76 options[:order] = "#{options[:order]}, #{@options[:order]}"
18 activerecord/test/associations_test.rb
@@ -540,9 +540,10 @@ def test_destroy_all
540 540 end
541 541
542 542 def test_dependence
543   - assert_equal 2, Client.find_all.size
544   - Firm.find_first.destroy
545   - assert Client.find_all.empty?
  543 + firm = companies(:first_firm)
  544 + assert_equal 2, firm.clients.size
  545 + firm.destroy
  546 + assert Client.find(:all, :conditions => "firm_id=#{firm.id}").empty?
546 547 end
547 548
548 549 def test_destroy_dependent_when_deleted_from_association
@@ -567,14 +568,14 @@ def test_three_levels_of_dependence
567 568
568 569 uses_transaction :test_dependence_with_transaction_support_on_failure
569 570 def test_dependence_with_transaction_support_on_failure
570   - assert_equal 2, Client.find_all.length
571   - firm = Firm.find_first
  571 + firm = companies(:first_firm)
572 572 clients = firm.clients
  573 + assert_equal 2, clients.length
573 574 clients.last.instance_eval { def before_destroy() raise "Trigger rollback" end }
574 575
575 576 firm.destroy rescue "do nothing"
576 577
577   - assert_equal 2, Client.find_all.length
  578 + assert_equal 2, Client.find(:all, :conditions => "firm_id=#{firm.id}").size
578 579 end
579 580
580 581 def test_dependence_on_account
@@ -590,6 +591,11 @@ def test_included_in_collection
590 591 def test_adding_array_and_collection
591 592 assert_nothing_raised { Firm.find_first.clients + Firm.find_all.last.clients }
592 593 end
  594 +
  595 + def test_find_all_without_conditions
  596 + firm = companies(:first_firm)
  597 + assert_equal 2, firm.clients.find(:all).length
  598 + end
593 599 end
594 600
595 601 class BelongsToAssociationsTest < Test::Unit::TestCase
3  activerecord/test/base_test.rb
@@ -283,8 +283,9 @@ def test_destroy_all
283 283 end
284 284
285 285 def test_destroy_many
  286 + assert_equal 3, Client.count
286 287 Client.destroy([2, 3])
287   - assert_equal 0, Client.count
  288 + assert_equal 1, Client.count
288 289 end
289 290
290 291 def test_delete_many
8 activerecord/test/deprecated_associations_test.rb
@@ -60,14 +60,14 @@ def test_has_many_queries
60 60 end
61 61
62 62 def test_has_many_dependence
63   - assert_equal 2, Client.find_all.length
  63 + assert_equal 3, Client.find_all.length
64 64 Firm.find_first.destroy
65   - assert_equal 0, Client.find_all.length
  65 + assert_equal 1, Client.find_all.length
66 66 end
67 67
68 68 uses_transaction :test_has_many_dependence_with_transaction_support_on_failure
69 69 def test_has_many_dependence_with_transaction_support_on_failure
70   - assert_equal 2, Client.find_all.length
  70 + assert_equal 3, Client.find_all.length
71 71
72 72 firm = Firm.find_first
73 73 clients = firm.clients
@@ -75,7 +75,7 @@ def test_has_many_dependence_with_transaction_support_on_failure
75 75
76 76 firm.destroy rescue "do nothing"
77 77
78   - assert_equal 2, Client.find_all.length
  78 + assert_equal 3, Client.find_all.length
79 79 end
80 80
81 81 def test_has_one_dependence
14 activerecord/test/fixtures/companies.yml
@@ -19,3 +19,17 @@ second_client:
19 19 client_of: 1
20 20 name: Microsoft
21 21 ruby_type: Client
  22 +
  23 +another_firm:
  24 + id: 4
  25 + type: Firm
  26 + name: Flamboyant Software
  27 + ruby_type: Firm
  28 +
  29 +another_client:
  30 + id: 5
  31 + type: Client
  32 + firm_id: 4
  33 + client_of: 4
  34 + name: Ex Nihilo
  35 + ruby_type: Client
10 activerecord/test/inheritance_test.rb
@@ -48,9 +48,9 @@ def test_alt_inheritance_save
48 48 end
49 49
50 50 def test_inheritance_condition
51   - assert_equal 3, Company.find_all.length
52   - assert_equal 1, Firm.find_all.length
53   - assert_equal 2, Client.find_all.length
  51 + assert_equal 5, Company.find_all.length
  52 + assert_equal 2, Firm.find_all.length
  53 + assert_equal 3, Client.find_all.length
54 54 end
55 55
56 56 def test_alt_inheritance_condition
@@ -82,7 +82,7 @@ def test_alt_update_all_within_inheritance
82 82 def test_destroy_all_within_inheritance
83 83 Client.destroy_all
84 84 assert_equal 0, Client.find_all.length
85   - assert_equal 1, Firm.find_all.length
  85 + assert_equal 2, Firm.find_all.length
86 86 end
87 87
88 88 def test_alt_destroy_all_within_inheritance
@@ -132,4 +132,4 @@ def switch_to_alt_inheritance_column
132 132
133 133 def Company.inheritance_column() "ruby_type" end
134 134 end
135   -end
  135 +end

0 comments on commit 37a370d

Please sign in to comment.
Something went wrong with that request. Please try again.