Permalink
Browse files

Make default scope with complex conditions (ie, conditions on joined

tables) work with update_all. With delete_all we raise an exception.
Fix delete_all on mysql when you try to update a column that mysql finds
ambiguous.
  • Loading branch information...
1 parent 549da0d commit 296603b101362330550d466183ef38a9645602b3 @derekkraan derekkraan committed Dec 4, 2012
@@ -95,7 +95,7 @@ def delete_records(records, method)
if method == :delete_all
update_counter(-scope.delete_all)
else
- update_counter(-scope.update_all(reflection.foreign_key => nil))
+ update_counter(-scope.update_all("#{reflection.table_name}.#{reflection.foreign_key}" => nil))
end
end
end
@@ -155,6 +155,18 @@ def quote_table_name(name)
quote_column_name(name)
end
+ # Override to return the quoted table name for assignment. Defaults to
+ # table quoting.
+ #
+ # This works for mysql and mysql2 where table.column can be used to
+ # resolve ambiguity.
+ #
+ # We override this in the sqlite and postgresql adaptors to use only
+ # the column name (as per syntax requirements).
+ def quote_table_name_for_assignment(name)
+ quote_table_name(name)
+ end
+
# Returns a bind substitution value given a +column+ and list of current
# +binds+
def substitute_at(column, index)
@@ -496,6 +496,10 @@ def quote_column_name(name) #:nodoc:
PGconn.quote_ident(name.to_s)
end
+ def quote_table_name_for_assignment(name)
+ quote_column_name(name.to_s.gsub(/.*\./, ''))
+ end
+
# Quote date/time values for use in SQL input. Includes microseconds
# if the value is a Time responding to usec.
def quoted_date(value) #:nodoc:
@@ -186,6 +186,10 @@ def quote_column_name(name) #:nodoc:
%Q("#{name.to_s.gsub('"', '""')}")
end
+ def quote_table_name_for_assignment(name)
+ quote_column_name(name.to_s.gsub(/.*\./, ''))
+ end
+
# Quote date/time values for use in SQL input. Includes microseconds
# if the value is a Time responding to usec.
def quoted_date(value) #:nodoc:
@@ -283,7 +283,7 @@ def update_all(updates, conditions = nil, options = {})
stmt.table(table)
stmt.key = table[primary_key]
- if joins_values.any?
+ if with_default_scope.joins_values.any?
@klass.connection.join_to_update(stmt, arel)
else
stmt.take(arel.limit)
@@ -404,6 +404,7 @@ def destroy(id)
# +after_destroy+ callbacks, use the +destroy_all+ method instead.
def delete_all(conditions = nil)
raise ActiveRecordError.new("delete_all doesn't support limit scope") if self.limit_value
+ raise ActiveRecordError.new("delete_all doesn't support conditions on joined tables in delete statements.") if with_default_scope.joins_values.any?
IdentityMap.repository[symbolized_base_class] = {} if IdentityMap.enabled?
if conditions
@@ -102,7 +102,7 @@ def sanitize_sql_hash_for_conditions(attrs, default_table_name = self.table_name
# # => "status = NULL , group_id = 1"
def sanitize_sql_hash_for_assignment(attrs)
attrs.map do |attr, value|
- "#{connection.quote_column_name(attr)} = #{quote_bound_value(value)}"
+ "#{connection.quote_table_name_for_assignment(attr)} = #{quote_bound_value(value)}"
end.join(', ')
end
@@ -8,6 +8,8 @@
require 'models/category'
require 'models/post'
require 'models/author'
+require 'models/book'
+require 'models/review'
require 'models/essay'
require 'models/comment'
require 'models/person'
@@ -102,7 +104,8 @@ def test_should_generate_valid_sql
class HasManyAssociationsTest < ActiveRecord::TestCase
fixtures :accounts, :categories, :companies, :developers, :projects,
:developers_projects, :topics, :authors, :comments,
- :people, :posts, :readers, :taggings, :cars, :essays
+ :people, :posts, :readers, :taggings, :cars, :essays,
+ :books, :reviews
def setup
Client.destroyed_client_ids.clear
@@ -1749,4 +1752,10 @@ def test_replace_returns_target
assert_equal [bulb1, bulb3], car.bulbs
assert_equal [bulb1, bulb3], result
end
+
+ def test_delete_records_with_complex_joins
+ david = authors(:david).becomes(AuthorWithPositiveReview)
+ david.books_with_positive_reviews = []
+ assert_equal [], david.books_with_positive_reviews
+ end
end
@@ -161,6 +161,23 @@ def test_ensure_that_method_scoping_is_correctly_restored
assert !Developer.scoped.where_values.include?("name = 'Jamis'")
end
+
+ def test_default_scope_filters_on_joins
+ assert_equal 1, DeveloperFilteredOnJoins.all.count
+ assert_equal DeveloperFilteredOnJoins.all.first, developers(:david).becomes(DeveloperFilteredOnJoins)
+ end
+
+ def test_update_all_default_scope_filters_on_joins
+ DeveloperFilteredOnJoins.update_all(:salary => 65000)
+ assert_equal 65000, Developer.find(developers(:david).id).salary
+
+ # has not changed jamis
+ assert_not_equal 65000, Developer.find(developers(:jamis).id).salary
+ end
+
+ def test_delete_all_default_scope_filters_on_joins
+ assert_raise(ActiveRecord::ActiveRecordError) { DeveloperFilteredOnJoins.delete_all }
+ end
end
class NestedRelationScopingTest < ActiveRecord::TestCase
@@ -0,0 +1,14 @@
+positive_review:
+ author_id: 2
+ book_id: 1
+ positive: true
+
+david_positive_review:
+ author_id: 1
+ book_id: 2
+ positive: true
+
+negative_review:
+ author_id: 3
+ book_id: 1
+ positive: false
@@ -104,6 +104,7 @@ def testing_proxy_target
has_many :tags_with_primary_key, :through => :posts
has_many :books
+ has_many :books_with_positive_reviews, :class_name => 'BookPositiveReview'
has_many :subscriptions, :through => :books
has_many :subscribers, :through => :subscriptions, :order => "subscribers.nick" # through has_many :through (on through reflection)
has_many :distinct_subscribers, :through => :subscriptions, :source => :subscriber, :select => "DISTINCT subscribers.*", :order => "subscribers.nick"
@@ -140,6 +141,8 @@ def testing_proxy_target
has_many :posts_with_default_include, :class_name => 'PostWithDefaultInclude'
has_many :comments_on_posts_with_default_include, :through => :posts_with_default_include, :source => :comments
+ has_many :reviews
+
scope :relation_include_posts, includes(:posts)
scope :relation_include_tags, includes(:tags)
@@ -198,3 +201,9 @@ class AuthorFavorite < ActiveRecord::Base
belongs_to :author
belongs_to :favorite_author, :class_name => "Author"
end
+
+class AuthorWithPositiveReview < Author
+ def self.has_written_positive_reviews
+ joins(:reviews).where(:reviews => {:positive => true})
+ end
+end
@@ -6,4 +6,14 @@ class Book < ActiveRecord::Base
has_many :subscriptions
has_many :subscribers, :through => :subscriptions
+
+ has_many :reviews
+end
+
+class BookPositiveReview < Book
+ default_scope { positive_reviews }
+
+ def self.positive_reviews
+ joins(:reviews).where(:reviews => {:positive => true})
+ end
end
@@ -106,6 +106,15 @@ class DeveloperWithIncludes < ActiveRecord::Base
default_scope includes(:audit_logs)
end
+class DeveloperFilteredOnJoins < ActiveRecord::Base
+ self.table_name = 'developers'
+ has_and_belongs_to_many :projects, :foreign_key => 'developer_id', :join_table => 'developers_projects', :order => 'projects.id'
+
+ def self.default_scope
+ joins(:projects).where(:projects => { :name => 'Active Controller' })
+ end
+end
+
class DeveloperOrderedBySalary < ActiveRecord::Base
self.table_name = 'developers'
default_scope :order => 'salary DESC'
@@ -0,0 +1,4 @@
+class Review < ActiveRecord::Base
+ belongs_to :author
+ belongs_to :book
+end
@@ -554,6 +554,12 @@ def create_table(*args, &block)
t.integer :lock_version, :default => 0
end
+ create_table :reviews, :force => true do |t|
+ t.integer :author_id
+ t.integer :book_id
+ t.boolean :positive
+ end
+
create_table :shape_expressions, :force => true do |t|
t.string :paint_type
t.integer :paint_id

0 comments on commit 296603b

Please sign in to comment.