-
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
Merge types during Ecto.Changeset.merge/2
#4426
Conversation
This commit adds support for merging types when calling `Ecto.Changeset.merge/2`. Closes elixir-ecto#4424
lib/ecto/changeset.ex
Outdated
@@ -1523,10 +1523,10 @@ defmodule Ecto.Changeset do | |||
|
|||
defp cast_merge(cs1, cs2) do | |||
new_params = (cs1.params || cs2.params) && Map.merge(cs1.params || %{}, cs2.params || %{}) | |||
new_types = (cs1.types || cs2.types) && Map.merge(cs1.types || %{}, cs2.types || %{}) |
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.
From the Ecto.Changeset.t/1
typespec, types
can be nil
, so I implemented fallbacks with ||
. However, in the context of Ecto.Changeset.merge/2
, I'm not sure if types
can actually be nil
, since an (ArgumentError) different :data when merging changesets
error is raised if the data
fields differ.
Could someone please confirm if it's safe to simply Map.merge(cs1.types, cs2.types)
for a cleaner solution?
Ecto.Changeset.merge/2
@@ -1523,10 +1523,10 @@ defmodule Ecto.Changeset do | |||
|
|||
defp cast_merge(cs1, cs2) do | |||
new_params = (cs1.params || cs2.params) && Map.merge(cs1.params || %{}, cs2.params || %{}) | |||
new_types = Map.merge(cs1.types, cs2.types) |
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.
It seems the assumption is that some people were initializing the changeset directly without the proper API. I don't think we need to support that.
💚 💙 💜 💛 ❤️ |
This commit adds support for merging types when calling
Ecto.Changeset.merge/2
.Closes #4424