Skip to content

Commit

Permalink
Make sure nested through associations are read only
Browse files Browse the repository at this point in the history
  • Loading branch information
jonleighton committed Oct 15, 2010
1 parent d619e39 commit edc176d
Show file tree
Hide file tree
Showing 6 changed files with 82 additions and 9 deletions.
6 changes: 6 additions & 0 deletions activerecord/lib/active_record/associations.rb
Expand Up @@ -64,6 +64,12 @@ def initialize(owner, reflection)
super("Cannot dissociate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to delete the has_many :through record associating them.") super("Cannot dissociate new records through '#{owner.class.name}##{reflection.name}' on '#{reflection.source_reflection.class_name rescue nil}##{reflection.source_reflection.name rescue nil}'. Both records must have an id in order to delete the has_many :through record associating them.")
end end
end end

class HasManyThroughNestedAssociationsAreReadonly < ActiveRecordError #:nodoc
def initialize(owner, reflection)
super("Cannot modify association '#{owner.class.name}##{reflection.name}' because it goes through more than one other association.")
end
end


class HasAndBelongsToManyAssociationWithPrimaryKeyError < ActiveRecordError #:nodoc: class HasAndBelongsToManyAssociationWithPrimaryKeyError < ActiveRecordError #:nodoc:
def initialize(reflection) def initialize(reflection)
Expand Down
Expand Up @@ -8,6 +8,11 @@ module Associations
class HasManyThroughAssociation < HasManyAssociation #:nodoc: class HasManyThroughAssociation < HasManyAssociation #:nodoc:
include ThroughAssociationScope include ThroughAssociationScope


def build(attributes = {}, &block)
ensure_not_nested
super
end

alias_method :new, :build alias_method :new, :build


def create!(attrs = nil) def create!(attrs = nil)
Expand Down Expand Up @@ -37,6 +42,7 @@ def size


protected protected
def create_record(attrs, force = true) def create_record(attrs, force = true)
ensure_not_nested
ensure_owner_is_not_new ensure_owner_is_not_new


transaction do transaction do
Expand All @@ -60,6 +66,8 @@ def construct_find_options!(options)
end end


def insert_record(record, force = true, validate = true) def insert_record(record, force = true, validate = true)
ensure_not_nested

if record.new_record? if record.new_record?
if force if force
record.save! record.save!
Expand All @@ -75,6 +83,8 @@ def insert_record(record, force = true, validate = true)


# TODO - add dependent option support # TODO - add dependent option support
def delete_records(records) def delete_records(records)
ensure_not_nested

klass = @reflection.through_reflection.klass klass = @reflection.through_reflection.klass
records.each do |associate| records.each do |associate|
klass.delete_all(construct_join_attributes(associate)) klass.delete_all(construct_join_attributes(associate))
Expand Down
Expand Up @@ -14,6 +14,8 @@ def replace(new_value)
private private


def create_through_record(new_value) #nodoc: def create_through_record(new_value) #nodoc:
ensure_not_nested

klass = @reflection.through_reflection.klass klass = @reflection.through_reflection.klass


current_object = @owner.send(@reflection.through_reflection.name) current_object = @owner.send(@reflection.through_reflection.name)
Expand Down
Expand Up @@ -8,15 +8,18 @@ module ThroughAssociationScope
protected protected


def construct_scope def construct_scope
{ :create => construct_owner_attributes(@reflection), scope = {}
:find => { :conditions => construct_conditions, scope[:find] = {
:joins => construct_joins, :conditions => construct_conditions,
:include => @reflection.options[:include] || @reflection.source_reflection.options[:include], :joins => construct_joins,
:select => construct_select, :include => @reflection.options[:include] || @reflection.source_reflection.options[:include],
:order => @reflection.options[:order], :select => construct_select,
:limit => @reflection.options[:limit], :order => @reflection.options[:order],
:readonly => @reflection.options[:readonly], :limit => @reflection.options[:limit],
} } :readonly => @reflection.options[:readonly]
}
scope[:create] = construct_owner_attributes(@reflection) unless @reflection.nested?
scope
end end


# Build SQL conditions from attributes, qualified by table name. # Build SQL conditions from attributes, qualified by table name.
Expand Down Expand Up @@ -299,6 +302,12 @@ def build_sti_condition
end end


alias_method :sql_conditions, :conditions alias_method :sql_conditions, :conditions

def ensure_not_nested
if @reflection.nested?
raise HasManyThroughNestedAssociationsAreReadonly.new(@owner, @reflection)
end
end
end end
end end
end end
4 changes: 4 additions & 0 deletions activerecord/lib/active_record/reflection.rb
Expand Up @@ -395,6 +395,10 @@ def through_reflection_chain
chain chain
end end
end end

def nested?
through_reflection_chain.length > 2
end


# Gets an array of possible <tt>:through</tt> source reflection names: # Gets an array of possible <tt>:through</tt> source reflection names:
# #
Expand Down
Expand Up @@ -363,6 +363,48 @@ def test_has_many_through_with_sti_on_through_reflection
assert !scope.where("comments.type" => "SpecialComment").empty? assert !scope.where("comments.type" => "SpecialComment").empty?
assert !scope.where("comments.type" => "SubSpecialComment").empty? assert !scope.where("comments.type" => "SubSpecialComment").empty?
end end

def test_nested_has_many_through_writers_should_raise_error
david = authors(:david)
subscriber = subscribers(:first)

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers = [subscriber]
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscriber_ids = [subscriber.id]
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers << subscriber
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers.delete(subscriber)
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers.clear
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers.build
end

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
david.subscribers.create
end
end

def test_nested_has_one_through_writers_should_raise_error
groucho = members(:groucho)
founding = member_types(:founding)

assert_raises(ActiveRecord::HasManyThroughNestedAssociationsAreReadonly) do
groucho.nested_member_type = founding
end
end


private private


Expand Down

0 comments on commit edc176d

Please sign in to comment.