Skip to content

Commit 13044a8

Browse files
authored
MONGOID-4160 Don't persist document if associated document is invalid with optional: false (#5303)
* MONGOID-4160 what happens if you use validations on autosave? * Update lib/mongoid/association/referenced/auto_save.rb * MONGOID-4160 attempt to reject invalid foreign keys * MONGOID-4160 if it's not persisted * MONGOID-4160 undo changes * MONGOID-4160 don't persist document if associated document is invalid * MONGOID-4160 release notes * MONGOID-4160 update docs
1 parent 74383da commit 13044a8

File tree

3 files changed

+154
-1
lines changed

3 files changed

+154
-1
lines changed

docs/release-notes/mongoid-8.0.txt

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -352,3 +352,59 @@ Removed ``Array#update_values`` and ``Hash#update_values`` methods
352352

353353
The previously deprecated ``Array#update_values`` and ``Hash#update_values``
354354
methods have been removed in Mongoid 8.
355+
356+
357+
No Longer Persisting Documents with Invalid ``belongs_to`` Associations
358+
-----------------------------------------------------------------------
359+
360+
In Mongoid 8, if an invalid document is assigned to a ``belongs_to`` association,
361+
and the base document is saved, if the ``belongs_to`` association had the
362+
option ``optional: false`` or ``optional`` is unset and the global flag
363+
``require_belongs_to_by_default`` is true, neither the document nor the
364+
associated document will be persisted. For example, given the following
365+
models:
366+
367+
.. code::
368+
369+
class Parent
370+
include Mongoid::Document
371+
has_one :child
372+
field :name
373+
validates :name, presence: true
374+
end
375+
376+
class Child
377+
include Mongoid::Document
378+
379+
belongs_to :parent, autosave: true, validate: false, optional: false
380+
end
381+
382+
child = Child.new
383+
parent = Parent.new
384+
child.parent = parent # parent is invalid, it does not have a name
385+
child.save
386+
387+
In this case, both the child and the parent will not be persisted.
388+
389+
.. note::
390+
If ``save!`` were called, a validation error would be raised.
391+
392+
If optional is false, then the Child will be persisted with a parent_id, but the
393+
parent will not be persisted:
394+
395+
.. code::
396+
397+
child = Child.new
398+
parent = Parent.new
399+
child.parent = parent # parent is invalid, it does not have a name
400+
child.save
401+
402+
p Child.first
403+
# => <Child _id: 629a50b0d1327aad89d214d2, parent_id: BSON::ObjectId('629a50b0d1327aad89d214d3')>
404+
p Parent.first
405+
# => nil
406+
407+
If you want the functionality of neither document being persisted in Mongoid 7 or 8
408+
and earlier, the ``validate: true`` option can be set on the association. If
409+
you want the functionality of only the Child being persisted, the ``validate:
410+
false`` option can be set on the association.

lib/mongoid/association/referenced/belongs_to.rb

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ def setup_instance_methods!
153153
create_foreign_key_field!
154154
setup_index!
155155
define_touchable!
156-
@owner_class.validates_associated(name) if validate?
156+
@owner_class.validates_associated(name) if validate? || require_association?
157157
@owner_class.validates(name, presence: true) if require_association?
158158
end
159159

spec/mongoid/association/referenced/has_one/proxy_spec.rb

Lines changed: 97 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1148,4 +1148,101 @@
11481148
expect(Vehicle.aliased_fields["driver"]).to eq("driver_id")
11491149
end
11501150
end
1151+
1152+
context "when the document is not persisted and the association is invalid" do
1153+
1154+
before do
1155+
class HomParent
1156+
include Mongoid::Document
1157+
has_one :child, class_name: "HomChild"
1158+
field :name
1159+
validates :name, presence: true
1160+
end
1161+
1162+
class HomChild; include Mongoid::Document; end
1163+
1164+
belongs_to
1165+
child.parent = parent
1166+
child.save
1167+
end
1168+
1169+
after do
1170+
Object.send(:remove_const, :HomParent)
1171+
Object.send(:remove_const, :HomChild)
1172+
end
1173+
1174+
let(:belongs_to) do
1175+
HomChild.belongs_to :parent, autosave: true, validate: false, optional: optional
1176+
end
1177+
1178+
let(:child) { HomChild.new }
1179+
let(:parent) { HomParent.new }
1180+
1181+
1182+
context "when belongs_to_required_by_default is true" do
1183+
config_override :belongs_to_required_by_default, true
1184+
1185+
context "when optional is true" do
1186+
let(:optional) { true }
1187+
1188+
it "persists the child with the parent_id" do
1189+
expect(HomChild.first.parent_id).to eq(parent._id)
1190+
expect(HomParent.count).to eq(0)
1191+
end
1192+
end
1193+
1194+
context "when optional is false" do
1195+
let(:optional) { false }
1196+
1197+
it "doesn't persist the parent or the child" do
1198+
expect(HomChild.count).to eq(0)
1199+
expect(HomParent.count).to eq(0)
1200+
end
1201+
end
1202+
1203+
context "when optional is not set" do
1204+
let(:belongs_to) do
1205+
HomChild.belongs_to :parent, autosave: true, validate: false
1206+
end
1207+
1208+
it "doesn't persist the parent or the child" do
1209+
expect(HomChild.count).to eq(0)
1210+
expect(HomParent.count).to eq(0)
1211+
end
1212+
end
1213+
end
1214+
1215+
context "when belongs_to_required_by_default is false" do
1216+
config_override :belongs_to_required_by_default, false
1217+
1218+
context "when optional is true" do
1219+
let(:optional) { true }
1220+
1221+
it "persists the child with the parent_id" do
1222+
expect(HomChild.first.parent_id).to eq(parent._id)
1223+
expect(HomParent.count).to eq(0)
1224+
end
1225+
end
1226+
1227+
context "when optional is false" do
1228+
let(:optional) { false }
1229+
1230+
it "doesn't persist the parent or the child" do
1231+
expect(HomChild.count).to eq(0)
1232+
expect(HomParent.count).to eq(0)
1233+
end
1234+
end
1235+
1236+
context "when optional is not set" do
1237+
let(:belongs_to) do
1238+
HomChild.belongs_to :parent, autosave: true, validate: false
1239+
end
1240+
1241+
it "persists the child with the parent_id" do
1242+
expect(HomChild.first.parent_id).to eq(parent._id)
1243+
expect(HomParent.count).to eq(0)
1244+
end
1245+
end
1246+
end
1247+
end
11511248
end

0 commit comments

Comments
 (0)