Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MONGOID-5104 Add AR-style dirty attributes #5092

Merged
merged 3 commits into from
Oct 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
43 changes: 43 additions & 0 deletions docs/release-notes/mongoid-7.5.txt
Original file line number Diff line number Diff line change
Expand Up @@ -117,3 +117,46 @@ Embedded associations (``embeds_one`` and ``embeds_many``):
+---------------------------------------+---------------------------------------+
| Parent :after_save | Parent :after_save |
+---------------------------------------+---------------------------------------+


``Changeable`` Module Behavior Made Compatible With ``ActiveModel::Ditry``
--------------------------------------------------------------------------

When updating documents, it is now possible to get updated attribute values
in ``after_*`` callbacks. This is following with ActiveRecord/ActiveModel
behavior.

.. code-block:: ruby

class Cat
include Mongoid::Document

field :age, type: Integer

after_save do
p self
p attribute_was(:age)
end
end

a = Cat.create!
a.age = 2
a.save!

Mongoid 7.5 output:

.. code-block:: ruby

#<Cat _id: 60aef1652c97a617438dc9bb, age: 2>
2


Mongoid 7.4 output:

.. code-block:: ruby

#<Cat _id: 60aef1652c97a617438dc9bb, age: 2>
nil

Notice that in 7.4 ``attribute_was(:age)`` returns an old attribute value,
while in 7.5 ``attribute_was(:age)`` returns the new values,
4 changes: 2 additions & 2 deletions lib/mongoid/association/referenced/counter_cache.rb
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,8 @@ def self.define_callbacks!(association)
if record = __send__(name)
foreign_key = association.foreign_key

if attribute_changed?(foreign_key)
original, current = attribute_change(foreign_key)
if send("#{foreign_key}_previously_changed?")
original, current = send("#{foreign_key}_previous_change")

unless original.nil?
record.class.with(persistence_context) do |_class|
Expand Down
4 changes: 2 additions & 2 deletions lib/mongoid/association/referenced/syncable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -67,8 +67,8 @@ def remove_inverse_keys(association)
#
# @return [ Object ] The updated values.
def update_inverse_keys(association)
if changes.has_key?(association.foreign_key)
old, new = changes[association.foreign_key]
if previous_changes.has_key?(association.foreign_key)
old, new = previous_changes[association.foreign_key]
adds, subs = new - (old || []), (old || []) - new

# If we are autosaving we don't want a duplicate to get added - the
Expand Down
2 changes: 1 addition & 1 deletion lib/mongoid/association/relatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -242,7 +242,7 @@ def inverse_type_setter
#
# @return [ String ] The foreign key check.
def foreign_key_check
@foreign_key_check ||= "#{foreign_key}_changed?" if (stores_foreign_key? && foreign_key)
@foreign_key_check ||= "#{foreign_key}_previously_changed?" if (stores_foreign_key? && foreign_key)
end

# Create an association proxy object using the owner and target.
Expand Down
7 changes: 4 additions & 3 deletions lib/mongoid/persistable/creatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,19 +102,20 @@ def post_process_insert
def prepare_insert(options = {})
return self if performing_validations?(options) &&
invalid?(options[:context] || :create)
result = run_callbacks(:save, with_children: false) do
run_callbacks(:save, with_children: false) do
run_callbacks(:create, with_children: false) do
run_callbacks(:persist_parent, with_children: false) do
_mongoid_run_child_callbacks(:save) do
_mongoid_run_child_callbacks(:create) do
yield(self)
result = yield(self)
post_process_insert
post_process_persist(result, options)
end
end
end
end
end
post_process_persist(result, options) and self
self
end

module ClassMethods
Expand Down
6 changes: 3 additions & 3 deletions lib/mongoid/persistable/updatable.rb
Original file line number Diff line number Diff line change
Expand Up @@ -96,19 +96,19 @@ def prepare_update(options = {})
return false if performing_validations?(options) &&
invalid?(options[:context] || :update)
process_flagged_destroys
result = run_callbacks(:save, with_children: false) do
run_callbacks(:save, with_children: false) do
run_callbacks(:update, with_children: false) do
run_callbacks(:persist_parent, with_children: false) do
_mongoid_run_child_callbacks(:save) do
_mongoid_run_child_callbacks(:update) do
yield(self)
result = yield(self)
post_process_persist(result, options)
true
end
end
end
end
end
post_process_persist(result, options) and result
end

# Update the document in the database.
Expand Down
2 changes: 0 additions & 2 deletions spec/integration/callbacks_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -217,8 +217,6 @@
let!(:obj) { Emission.create!(frequency: 1) }

it 'is set to the new value' do
pending 'MONGOID-5104'

obj.frequency = 2
obj.save!

Expand Down
4 changes: 2 additions & 2 deletions spec/mongoid/association/referenced/belongs_to_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2019,8 +2019,8 @@ class BelongingObject; include Mongoid::Document; end

describe '#foreign_key_check' do

it 'returns the foreign_key followed by "_changed?"' do
expect(association.foreign_key_check).to eq('owner_object_id_changed?')
it 'returns the foreign_key followed by "_previously_changed?"' do
expect(association.foreign_key_check).to eq('owner_object_id_previously_changed?')
end
end

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1008,8 +1008,8 @@ class OtherHasManyRightObject; end

describe '#foreign_key_check' do

it 'returns the foreign_key followed by "_changed?"' do
expect(association.foreign_key_check).to eq('has_many_right_object_ids_changed?')
it 'returns the foreign_key followed by "_previously_changed?"' do
expect(association.foreign_key_check).to eq('has_many_right_object_ids_previously_changed?')
end
end

Expand Down
7 changes: 0 additions & 7 deletions spec/mongoid/attributes/nested_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4395,13 +4395,6 @@ class BandWithAllowDestroyedRecords < Band

context "when nesting multiple levels and parent is timestamped" do

around do |example|
original_relations = Location.relations
Location.embedded_in :address, touch: true
example.run
Location.relations = original_relations
end

after do
Address.reset_callbacks(:save)
end
Expand Down
26 changes: 18 additions & 8 deletions spec/mongoid/changeable_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -1649,21 +1649,26 @@

before do
Acolyte.set_callback(:save, :after, if: :callback_test?) do |doc|
doc[:changed_in_callback] = doc.changes.dup
doc[:changed_in_after_callback] = doc.changes.dup
end

Acolyte.set_callback(:save, :before, if: :callback_test?) do |doc|
doc[:changed_in_before_callback] = doc.changes.dup
end
end

after do
Acolyte._save_callbacks.select do |callback|
callback.kind == :after
[:before, :after].include?(callback.kind)
end.each do |callback|
Acolyte._save_callbacks.delete(callback)
end
end

it "retains the changes until after all callbacks" do
it "does not retain the changes until after all callbacks" do
acolyte.update_attribute(:status, "testing")
expect(acolyte.changed_in_callback).to eq({ "status" => [ nil, "testing" ] })
expect(acolyte.changed_in_before_callback).to eq({"status"=>[nil, "testing"]})
expect(acolyte.changed_in_after_callback).to eq({ })
end
end

Expand All @@ -1675,21 +1680,26 @@

before do
Acolyte.set_callback(:save, :after, if: :callback_test?) do |doc|
doc[:changed_in_callback] = doc.changes.dup
doc[:changed_after_in_callback] = doc.changes.dup
end

Acolyte.set_callback(:save, :before, if: :callback_test?) do |doc|
doc[:changed_before_in_callback] = doc.changes.dup
end
end

after do
Acolyte._save_callbacks.select do |callback|
callback.kind == :after
[:before, :after].include?(callback.kind)
end.each do |callback|
Acolyte._save_callbacks.delete(callback)
end
end

it "retains the changes until after all callbacks" do
it "does not retain the changes until after all callbacks" do
acolyte.save
expect(acolyte.changed_in_callback["name"]).to eq([ nil, "callback-test" ])
expect(acolyte.changed_before_in_callback["name"]).to eq([ nil, "callback-test" ])
expect(acolyte.changed_after_in_callback["name"]).to be_nil
end
end
end
Expand Down