Repository navigation
Ecto parity - #148
Closed
nduitz wants to merge 11 commits into
Closed
Ecto parity#148nduitz wants to merge 11 commits into
nduitz wants to merge 11 commits into
Conversation
This was referenced Oct 22, 2026
nduitz
commented
Oct 23, 2026
| address: "address" | ||
| } | ||
| ], | ||
| contexts2: nil, |
Contributor
Author
There was a problem hiding this comment.
since nil is now an invalid error like ecto handles it, this test would fail with it. I added the other test for this behaviour
nduitz
commented
Oct 23, 2026
Comment on lines
+435
to
+445
| # Not in Ecto: Ecto checks this as part of "cast embeds_many changing schemas". | ||
| test "cast embeds_many keeps the data of a row that it matches by id" do | ||
| posts = [%Post{id: 1, title: "first", position: 5}] | ||
| params = %{"posts" => [post(%{"id" => "1", "title" => "new"})]} | ||
|
|
||
| changeset = cast(%Author{posts: posts}, params, :posts) | ||
|
|
||
| assert [%Post{id: 1, title: "new", position: 5}] = | ||
| Changeset.apply_changes(changeset).posts | ||
| end | ||
|
|
Contributor
Author
There was a problem hiding this comment.
this test was added as prove the implementation works. it is removed later since "cast embeds_many changing schemas" proves the exact same thing, but at this point the mentioned test still had other failures that I only addressed later
ecto_parity_test.exs mirrors test/ecto/changeset/embedded_test.exs from Ecto v3.12.1 for polymorphic embeds. form_parity_test.exs compares the forms of embeds_one and embeds_many with the forms of the polymorphic helpers for the same params. The goal is output parity: for the same input, a polymorphic embed gives the same output as an Ecto embed. Tests for Ecto features that polymorphic embeds do not have are left out. Most tests fail on purpose. Each failure shows a difference from Ecto that a later commit addresses.
Before, a string or a list for a single embed, or a string, nil or a
list with values that are not maps for a list of embeds, crashed with a
CaseClauseError or a FunctionClauseError. Now the cast adds the same
error as Ecto: {"is invalid", [validation: :embed, type: :map]} or
type {:array, :map}.
nil for a list of embeds is now invalid, as in Ecto. Before, the cast
ignored it. The test "supports lists of polymorphic embeds" sent
contexts2: nil without a reason, so the line is removed. The new test
"nil for a list of embeds is invalid" checks the error for the regular
and the polymorphic schema.
With required: true, the cast now: - adds the field to changeset.required - adds "can't be blank" when the value is nil or an empty list after the cast, also for an existing empty list - puts a change to nil when the params set an existing embed to nil - adds no "can't be blank" when the field already has an error
The cast looked for the row id with params[:id]. Params from a form have string keys, so no row matched. Each existing row became a new :insert, and the row lost all data that the params did not set. Now a row matches when it has the same type and each primary key, cast to its type, is equal to the param value. This is the rule that Ecto uses, so "1" matches the integer id 1.
Ecto raises an ArgumentError when on_replace has a value that the embed does not accept. The polymorphic embed raised a RuntimeError. The messages do not change.
The cast crashed with a CaseClauseError for an embedded schema with an autogenerated integer id, because Ecto 3.12 returns a 3-tuple from __schema__(:autogenerate_id). Now the cast keeps the id nil, as Ecto does. dump/3 raises the same ArgumentError as Ecto when the id is still nil on insert.
nduitz
force-pushed
the
ecto-parity
branch
3 times, most recently
from
October 23, 2026 12:33
e4b885d to
322096b
Compare
cast_polymorphic_embed/3 now gives the same changeset output as cast_embed/3: - Each child is an Ecto.Changeset in changes. Before, a valid child was a struct. - A list of embeds gets a :replace changeset for each row of the data that the params do not keep. Rows that the load options kept as nil or plain maps match no params and are dropped without a :replace changeset. - A valid child with action :update and without changes is not a change. When the params do not contain the field, the data does not go into changes. When the changes of the field hold changesets, the field gets an :embed type in changeset.types, so that apply_changes/1, get_field/2 and traverse_errors/2 handle them as for an Ecto embed. Else the field keeps its PolymorphicEmbed type: on insert, Ecto builds changesets from an embed type with one schema module only, and a struct that put_change/3 set before the cast must stay a struct. The type then keeps the changeset function as on_cast for the forms. The cast still generates the binary id of a new child, in the data of the child changeset, so that apply_changes/1 and a later insert of that struct have the id. The changes of the child stay as in Ecto. A prepare_changes/2 function turns the child changesets into structs before the repo writes the parent, and puts the schema type back. It also generates missing ids of structs that the repo puts into the changes on insert, and of data without a change, because Ecto runs the prepare functions of an ordinary embed before it puts its data into the changes. On a parent insert, a child update counts as an insert, as in Ecto. The ordinary Ecto embeds of a child get the same preparation as Ecto gives to the embeds of a schema: their prepare functions run, and their ids and timestamps are generated. PolymorphicEmbed.Embedded is a copy of the logic of Ecto.Embedded.prepare/4, because that function is not public API. As in Ecto, the prepare functions of a child must not make it invalid: then the insert or update raises an ArgumentError. On insert, the data of a child goes into its changes, as in Ecto, so that ordinary embeds and polymorphic fields in the data of a child also get their ids. A struct on insert gets the same preparation as a new child changeset. Ecto reads the prepare functions of a changeset before it runs them. A cast inside a prepare function of the parent therefore prepares the embeds at once, so that the returned struct also holds structs. PolymorphicEmbed.HTML.Helpers.to_form/4 now reads the child changesets from changes, so that the forms keep the errors of invalid children. Changes to existing tests: - Four tests read the id as a field of the struct in changes. They now assert the output of Ecto embeds. - "validates lists of polymorphic embeds" now expects the :replace changeset for the data row without id, before the new row. CHANGELOG.md lists the breaking changes of this branch.
PolymorphicEmbed.HTML.Helpers.to_form/4 now follows the rules that phoenix_ecto uses for embeds_one and embeds_many: - A child form uses the child changeset from changes, with its own params and its own action. Before, row 0 of a list got the params of all rows, and each changeset got the parent action. Structs in the changes, for example after the repo prepared them or after put_change/3, become changesets in the same way as the data. - The form errors follow the parent action, and valid? does not change. - form.hidden contains the primary key of the data, as in phoenix_ecto, and the type field. - Without a change, a form is built with the changeset function that the cast stores as on_cast in the field type. - A new Ecto embed has no id before the insert, so the form of a new child hides the id that the cast generated: in form.data, in the data of form.source and in form.hidden. Only a cast child has a params map, so a child that the caller gives as a struct keeps its id. Two existing tests asserted the old output. The list test now expects the value of the first row, as for the regular embed. The nil embed test now expects the errors of the new SMS changeset, as for an Ecto embed. All form parity tests pass.
phoenix_ecto takes the parent action of the child forms from the form. The polymorphic form helper took it from the changeset argument. The two differ when a form is built with to_form(changeset, action: :validate) and the helper gets the changeset without an action: the child forms then had no action and showed no errors. The new form parity tests build the form with action: :validate, once with form.source and once with the changeset without action.
Reviews of the change that keeps the child changesets in the changes found these flows. The tests check that: - an insert keeps embeds of more than one type that are only in the data, and generates their ids - the form helper builds forms when the changes hold structs, also after the repo prepared them - polymorphic embeds inside an ordinary embed of a child, and that ordinary embed, get ids on insert - a polymorphic embed that is only in the data gets its id on insert, inside an ordinary embed and inside a polymorphic child - an ordinary embed in a polymorphic child gets its id when a parent insert updates the child - a struct that put_change/3 set before the cast stays usable for apply_changes/1, get_field/2 and required: true - lists with entries that the load kept as maps or set to nil can be replaced - the struct that apply_changes/1 returns can be inserted, with ids - a child that its prepare function makes invalid stops the insert - merge/2 keeps the structs when the cast changeset is the second argument - a cast inside a prepare function of the parent gives structs with ids
When the cast puts changesets into the changes of a polymorphic field, the field has an Ecto embed type with one schema module. Ecto's put_change/3, update_change/3 and change/2 then check each child against that module, so they raise for children of another type. Ecto has no place where polymorphic_embed can change that check. put_polymorphic_embed/3 works as Ecto.Changeset.put_embed/4, but it finds the matching data row by module and primary key, per element. So a list can hold more than one type, and a single embed can change its type. It accepts structs, changesets, maps and keyword lists. The type of a map comes from its type field, from the data row with the same primary key, or from the only type of the field. A new child gets its id at once, as in the cast. The changelog lists the new function, and the limit of Ecto's functions as a breaking change. The function keeps the changeset function that an earlier cast stored for the forms, for example a with: function. An empty list replaces data that holds only entries from the load options. The tests also check that ordinary embeds in the data of a put child get their ids on insert, and that the form keeps the id of a struct that the caller gives. As Ecto.Changeset.put_embed/4 does, the function gives the error "is invalid" for a value that the field does not accept, for example nil for a list of embeds, and keeps the data and the changes. A put inside a prepare function of the parent prepares the embeds at once, as the cast does.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Hey again,
with these two PRs I tried to fix some issues we ran into. But the more I dug into it, the more it became clear to me that these problems arise from polymorphic_embed behaving differently than ecto/phoenix.
#132
#145
So this PR tries to implement parity with the output of casting embeds in Ecto / rendering forms via inputs_for.
I hope this direction is appreciated.
I use this in our production app now. We had some workarounds in place to make polymorphic_embed work for us, e.g. casting polymorphic embeds sometimes returns data as a changeset, sometimes as a struct. We could fall back to
Ecto.Changeset.get_fieldnow, because the output is now normalized in all situations.This work includes a lot of breaking changes, because return values now differ, I tried to list them all.
This branch only focuses on output parity. Ecto has some features polymorphic_embed does not have yet. I'll submit a follow-up PR to add those features.