diff --git a/lib/view_model/active_record/association_manipulation.rb b/lib/view_model/active_record/association_manipulation.rb index 24e8855c..0b480904 100644 --- a/lib/view_model/active_record/association_manipulation.rb +++ b/lib/view_model/active_record/association_manipulation.rb @@ -52,10 +52,9 @@ def load_associated(association_name, scope: nil, eager_include: true, serialize def replace_associated(association_name, update_hash, references: {}, deserialize_context: self.class.new_deserialize_context) association_data = self.class._association_data(association_name) - # TODO: do we need this to be a hard constraint? Is it useful? - # unless association_data.external? - # raise Viewmodel::DeserializationError::InternalAssociationWrite.new(association_name) - # end + unless association_data.external? + raise ViewModel::DeserializationError::InternalAssociationWrite.new(association_name, self.to_reference) + end if association_data.referenced? is_fupdate = @@ -110,6 +109,10 @@ def append_associated(association_name, subtree_hash_or_hashes, references: {}, direct_reflection = association_data.direct_reflection raise ArgumentError.new("Cannot append to single association '#{association_name}'") unless association_data.collection? + unless association_data.external? + raise ViewModel::DeserializationError::InternalAssociationWrite.new(association_name, self.to_reference) + end + ViewModel::Utils.wrap_one_or_many(subtree_hash_or_hashes) do |subtree_hashes| model_class.transaction do ViewModel::Callbacks.wrap_deserialize(self, deserialize_context: deserialize_context) do |hook_control| @@ -223,6 +226,10 @@ def delete_associated(association_name, associated_id, type: nil, deserialize_co association_data = self.class._association_data(association_name) direct_reflection = association_data.direct_reflection + unless association_data.external? + raise ViewModel::DeserializationError::InternalAssociationWrite.new(association_name, self.to_reference) + end + unless association_data.collection? raise ArgumentError.new("Cannot remove element from single association '#{association_name}'") end diff --git a/lib/view_model/active_record/update_data.rb b/lib/view_model/active_record/update_data.rb index 4b90ee52..5891d923 100644 --- a/lib/view_model/active_record/update_data.rb +++ b/lib/view_model/active_record/update_data.rb @@ -680,11 +680,11 @@ def parse(hash_data, valid_reference_keys) when AssociationData association_data = member_data - # TODO: is this a desirable constraint? If so, this implementation - # won't work, because replace_associated uses this update machinery - # if association_data.external? - # raise ViewModel::DeserializationError::ExternalAssociationWrite.new(name) - # end + # FIXME: replace_associated uses this update machinery, this won't work + # until we mark it. + if association_data.external? + raise ViewModel::DeserializationError::ExternalAssociationWrite.new(name) + end case when value.nil? diff --git a/lib/view_model/deserialization_error.rb b/lib/view_model/deserialization_error.rb index f5720639..5360f2bf 100644 --- a/lib/view_model/deserialization_error.rb +++ b/lib/view_model/deserialization_error.rb @@ -161,9 +161,6 @@ def meta end end - # FIXME: these two errors aren't used, because we haven't decided if it's a - # useful constraint. Use or remove before merging. - # Attempted to write to an external association during a deserialization class ExternalAssociationWrite < InvalidRequest attr_reader :association diff --git a/test/unit/view_model/active_record/has_many_test.rb b/test/unit/view_model/active_record/has_many_test.rb index cbe0fce3..bd9c33db 100644 --- a/test/unit/view_model/active_record/has_many_test.rb +++ b/test/unit/view_model/active_record/has_many_test.rb @@ -1087,6 +1087,10 @@ def new_model end describe 'with association manipulation' do + def subject_association_features + super.merge(external: true) + end + it 'appends a child' do view = create_viewmodel!