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

Allow merge:true to merge references with non-normalized objects, and vice-versa. #7778

Merged
merged 5 commits into from Mar 3, 2021

Conversation

benjamn
Copy link
Member

@benjamn benjamn commented Mar 2, 2021

Previously, you could allow InMemoryCache to merge non-normalized data using the merge: true configuration (see #6714 and #7070), but the merged objects had to be consistently non-normalized, since attempting to merge a normalized Reference with a non-normalized object would give preference to the reference, ignoring the object.

This PR fixes #7642 in both directions:

  • merging an incoming non-normalized object with an existing Reference now updates the normalized data behind the reference, leaving the field holding the Reference, and
  • merging an incoming Reference with an existing non-normalized object updates the normalized data behind the reference (as above), but, for any fields that overlap, the normalized field data takes precedence over the non-normalized field data.

In both directions, the new behavior matches what would happen if the normalized Reference was a non-normalized object instead.

Copy link
Contributor

@jcreighton jcreighton left a comment

Choose a reason for hiding this comment

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

🎉

Until now, we have only supported store.merge(id, object), which updates
the existing entity identified by id, letting new fields from the provided
object take precedence when fields overlap.

This commit makes it possible to call store.merge(object, id), which also
updates the object identified by id using fields from the provided object,
but preserves fields already in the store when there is overlap.
@benjamn benjamn force-pushed the 7642-allow-merging-into-existing-refs branch from 268c594 to 2553695 Compare March 3, 2021 20:28
@benjamn benjamn merged commit 6560394 into release-3.4 Mar 3, 2021
@benjamn benjamn deleted the 7642-allow-merging-into-existing-refs branch March 3, 2021 20:33
@hwillson hwillson added this to Done in Release 3.4 Mar 16, 2021
@hwillson hwillson removed this from the MM-2021-06 milestone Jul 29, 2021
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants