Skip to content

MONGOID-5206 fix bug where embedded document is not re-embedded #5115

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 30, 2021
Merged

MONGOID-5206 fix bug where embedded document is not re-embedded #5115

merged 3 commits into from
Dec 30, 2021

Conversation

neilshweky
Copy link
Contributor

No description provided.

neilshweky
neilshweky commented Dec 29, 2021
View reviewed changes
lib/mongoid/association/embedded/embeds_one/proxy.rb
@@ -53,6 +53,7 @@ def substitute(replacement)
# The associated object will be replaced by the below update if non-nil, so only
# run the callbacks and state-changing code by passing persist: false in that case.
_target.destroy(persist: !replacement) if persistable?
_target.new_record = true
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maybe this should be inside the destroy?

Copy link
Contributor Author

@neilshweky neilshweky Dec 29, 2021 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A little explanation on why this fixes the bug... Say we have three assignments:

canvas.palette = palette
canvas.palette = nil 
canvas.palette = palette

Where canvas embeds_one palette.

Previously, what was happening was, on the first assignment, palette was considered a "new record" (new_record?=true) and thus palette was being inserted into the database. However, on the third assignment, where trying to reassign the palette, palette is no longer considered a new record, because it had been inserted previously. This is not exactly accurate, because the second assignment ultimately removed the palette from the database, so it needs to be reinserted. Since the palette's new_record is false, Mongoid ends up "updating" the document, which doesn't reinsert it into the database.

The change I introduce here, respecifies palette as a "new record" when it gets removed from the database, so if it is reassigned, it will be reinserted into the database.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a great explanation, can you please copy it into a comment in the source so that we can refer to it in the future when someone looks at this code?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sure!

p-mongo
p-mongo approved these changes Dec 30, 2021
View reviewed changes
Copy link
Contributor

@p-mongo p-mongo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍 Thank you!

@neilshweky neilshweky merged commit bf28262 into mongodb:master Dec 30, 2021
p-mongo pushed a commit to johnnyshields/mongoid that referenced this pull request Jan 3, 2022
@p
Merge branch 'master' into pr-5109
ea91374 
* master:
  Fix doc syntax
  RUBY-2783 Require bson 4-stable for Mongoid as bson master is 5.0 and not compatible with driver (mongodb#5113)
  MONGOID-5208 fix error on reloading nil embedded document (mongodb#5116)
  MONGOID-5206 fix bug where embedded document is not re-embedded (mongodb#5115)
  MONGOID-5207 Add Ruby 3.1 to GH Actions (mongodb#5114)
  MONGOID-5207 Use YAML.safe_load in specs (mongodb#5112)
  MONGOID-5199 Reloadable#reload_embedded_document doesn't respect session (mongodb#5105)
  Fix MONGOID-5198 Calling children_changed? on a deep cyclical data structure will cause semi-infinite looping (mongodb#5104)
p-mongo pushed a commit to johnnyshields/mongoid that referenced this pull request Jan 3, 2022
@p
Merge branch 'master' into pr-5061
1069e27 
* master:
  MONGOID-5173 Specs should use bang (!) methods (without describe/context change) (mongodb#5109)
  Fix doc syntax
  RUBY-2783 Require bson 4-stable for Mongoid as bson master is 5.0 and not compatible with driver (mongodb#5113)
  MONGOID-5208 fix error on reloading nil embedded document (mongodb#5116)
  MONGOID-5206 fix bug where embedded document is not re-embedded (mongodb#5115)
  MONGOID-5207 Add Ruby 3.1 to GH Actions (mongodb#5114)
  MONGOID-5207 Use YAML.safe_load in specs (mongodb#5112)
  MONGOID-5199 Reloadable#reload_embedded_document doesn't respect session (mongodb#5105)
  Fix MONGOID-5198 Calling children_changed? on a deep cyclical data structure will cause semi-infinite looping (mongodb#5104)
p-mongo pushed a commit that referenced this pull request Jan 7, 2022
@neilshweky @p
MONGOID-5206 fix bug where embedded document is not re-embedded (#5115)
90ed313 
* MONGOID-5206 fix bug where embedded document is not re-embedded

* MONGOID-5206 update test description

* MONGOID-5206 add comment explaining change
p-mongo pushed a commit to johnnyshields/mongoid that referenced this pull request Jan 21, 2022
@p
Merge branch 'master' into pr-5050
f4f582c 
* master: (48 commits)
  MONGOID-5098 Bug fix: Range.mongoize should mongoize its members (mongodb#5108)
  MONGOID-5212 Support for Decimal128 type (mongodb#5125)
  MONGOID-5213 Document changes to BigDecimal type and addition of global flag (mongodb#5126)
  MONGOID-5209 Document "Custom Field Options" functionality + various docs improvements (mongodb#5120)
  MONGOID-5193 rails 7 support  (mongodb#5122)
  Fix default topology name - should be standalone (mongodb#5130)
  Update MRSS (mongodb#5129)
  MONGOID-5207 Test Ruby 3.1 on Evergreen (mongodb#5123)
  MONGOID-5186 .with_scope should restore previous scope (mongodb#5127)
  MONGOID-5161 - [Documentation Only] Clarify the policy for working with . and $ named fields which were introduced in MongoDB 5.0 (mongodb#5051)
  MONGOID-5173 Specs should use bang (!) methods (without describe/context change) (mongodb#5109)
  Fix doc syntax
  RUBY-2783 Require bson 4-stable for Mongoid as bson master is 5.0 and not compatible with driver (mongodb#5113)
  MONGOID-5208 fix error on reloading nil embedded document (mongodb#5116)
  MONGOID-5206 fix bug where embedded document is not re-embedded (mongodb#5115)
  MONGOID-5207 Add Ruby 3.1 to GH Actions (mongodb#5114)
  MONGOID-5207 Use YAML.safe_load in specs (mongodb#5112)
  MONGOID-5199 Reloadable#reload_embedded_document doesn't respect session (mongodb#5105)
  Fix MONGOID-5198 Calling children_changed? on a deep cyclical data structure will cause semi-infinite looping (mongodb#5104)
  MONGOID-5203 Add all available auth_mech options to Configuration documentation (mongodb#5103)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@p-mongo p-mongo p-mongo approved these changes

@comandeo comandeo Awaiting requested review from comandeo

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@neilshweky @p-mongo