-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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-5782 Ensure embedded documents are also marked timeless #5912
base: master
Are you sure you want to change the base?
MONGOID-5782 Ensure embedded documents are also marked timeless #5912
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great sleuthing! This looks good. 👍
Noting for posterity: as @DarshanaVenkatesh and I discussed elsewhere, this solution does not properly handle deeply nested children. There is a bug that we discovered in the callback code that results in callbacks on nested children being invoked twice, which defeats the timeless code because the first invocation resets timeless on the nested children, allowing the second one to actually update the timestamps, which propagates the change back up to the parent(s). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(Updating this to officially revoke my approval of the PR, per our discussions.)
MONGOID-5782
When a parent doc is set to timeless, this should also keep the embedded document as timeless. This is a bug fix as previously, the updated_at time would get set to current.