ecto
ecto copied to clipboard
Add new functions to Repo behaviour
This PR is not suitable for merging right away because of a few reasons (detailed below), but it serves as a template for what I think should be included in Ecto.
Rationale
Currently there is no way to recursively preload associations in deeply nested changesets. Preloading association in deeply nested changesets is important if one is working with nested forms inside LiveView. LiveView expects forms built from changesets (not ecto structs!) and it expects associations to be preloaded so that the HTML components can iterate over the value's "children".
One could think of dropping changesets from LiveView, but changesets are really convenient to validate data and to make sure that errors are rendered correctly in HTML forms.
Nothing inside this PR refers to LiveView, of course, but it's just the motivating example that led me to write it.
Changes implemented in this PR
This PR implements two new public callbacks in the Repo
behaviour, namely:
-
@spec preload_in_result({:ok, Ecto.Schema.t()} | {:error, Ecto.Changeset.t()}, list(), list()) :: {:ok, struct} | {:error, changeset}
which is optimized for piping results returned by functions such asRepo.insert()
andRepo.update()
-
@spec preload_in_changeset(changeset, list(), list())
to preload associations inside a changeset (the function above calls this one when given a{:error, changeset}
tuple)
The implementation is not very efficient in the sense that it probably generates more queries than is really required, but that can be optimized by walking the nested changesets twice (once to get the IDs of all the parents to get the children in a single query and then to attribute each child to the correct parent based on the ID; changesets without IDs can't have children already in the database, and don't need to be queryeed - I already do this optimization).
Testing
I'm trying to test this with the TestRepo
which uses a custom adapter (which I don't understand at all, I've never looked into adapter code), but it seems like the TestRepo
is not generating IDs correctly when multiple changesets are inserted (?).
Questions
- Is this considered to be in scope for Ecto? I believe it should be considered in scope because it's non-trivial functionality which might be useful for a number of projects, but probably too trivial to be in its own package.
- How can I make the test adapter generate IDs correctly? Should I instead test with a new adapter, even if I have to import something just for testing?