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

Add :defaults_to_struct option to embeds_one #4410

Merged
merged 10 commits into from
May 7, 2024
16 changes: 15 additions & 1 deletion lib/ecto/schema.ex
Original file line number Diff line number Diff line change
Expand Up @@ -1648,6 +1648,12 @@ defmodule Ecto.Schema do
selecting the whole struct in a query, such as `from p in Post, select: p`.
Defaults to `true`.

* `:defaults_to_struct` - When true, the field will default to the initialized
struct instead of nil, the same you would get from something like `%Order.Item{}`.
One important thing is that if the underlying data is explicitly nil when loading
the schema, it will still be loaded as nil, similar to how `:default` works in fields.
Defaults to `false`.

## Examples

defmodule Order do
Expand Down Expand Up @@ -2223,11 +2229,19 @@ defmodule Ecto.Schema do
Module.put_attribute(mod, :ecto_changeset_fields, {name, {:assoc, struct}})
end

@valid_embeds_one_options [:on_replace, :source, :load_in_query]
@valid_embeds_one_options [:on_replace, :source, :load_in_query, :defaults_to_struct]

@doc false
def __embeds_one__(mod, name, schema, opts) when is_atom(schema) do
check_options!(opts, @valid_embeds_one_options, "embeds_one/3")

opts =
if Keyword.get(opts, :defaults_to_struct) do
Keyword.put(opts, :default, schema.__schema__(:loaded))
else
opts
end

embed(mod, :one, name, schema, opts)
end

Expand Down
15 changes: 15 additions & 0 deletions test/ecto/embedded_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,15 @@ defmodule Ecto.EmbeddedTest do
end
end

defmodule Settings do
use Ecto.Schema

embedded_schema do
field :dark_mode, :boolean, default: false
embeds_one :default_post, Post, defaults_to_struct: true
end
end

test "__schema__" do
assert Author.__schema__(:embeds) ==
[:profile, :post, :posts]
Expand Down Expand Up @@ -63,6 +72,12 @@ defmodule Ecto.EmbeddedTest do
assert %UUIDSchema{uuid: ^uuid, authors: [%Author{}]} =
Ecto.embedded_load(UUIDSchema, %{"uuid" => uuid, "authors" => [%{}]}, :json)

assert %Settings{dark_mode: false, default_post: %Post{}} =
Ecto.embedded_load(Settings, %{}, :json)
Copy link
Member

Choose a reason for hiding this comment

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

What happens if I just do %Settings{}? Is the default still nil? Do we want to force it to be %Post{} even then?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It behaves exactly like :default on a field. So it will set to %Post{}.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

assert %Settings{dark_mode: false, default_post: %Post{}} = %Settings{}

Copy link
Member

Choose a reason for hiding this comment

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

Let's add a test for this scenario and I believe we are good to go!

Copy link
Member

Choose a reason for hiding this comment

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

I was about to approve but I think we're still missing this test

assert %Settings{dark_mode: false, default_post: %Post{}} = %Settings{}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is being tested through the load_embedded function, which relies on the struct default. I did it this way because it was how it was already been done. Of course the load is broader and also involves calling the field's load function, but I think that with both the case for non defined and explicit nil we cover all the cases. However, if you think we still need that test I can add it no problem.


assert %Settings{dark_mode: false, default_post: nil} =
Ecto.embedded_load(Settings, %{"default_post" => nil}, :json)

assert_raise ArgumentError,
~s[cannot load `"ABC"` as type Ecto.UUID for field `uuid` in schema Ecto.EmbeddedTest.UUIDSchema],
fn ->
Expand Down
Loading