ecto icon indicating copy to clipboard operation
ecto copied to clipboard

empty_values does not play well with default values in schemaless changesets

Open costaraphael opened this issue 1 year ago • 3 comments

Elixir version

1.14.0

Database and Version

PostgreSQL 14

Ecto Versions

3.10.2

Database Adapter and Versions (postgrex, myxql, etc)

0.17.1

Current behavior

There is no way to use the empty_values feature with schemaless changesets while providing default values for the fields:

types = %{title: :string}
data = %{title: "My base title"}

{data, types}
|> Ecto.Changeset.cast(%{title: ""}, [:title])
|> Ecto.Changeset.apply_action(:validate)
# {:ok, %{title: nil}}

Expected behavior

The documentation for Ecto.Changeset.cast/4 has the following example regarding the behavior of empty_values:

# Using default
iex> params = %{title: "", topics: []}
iex> changeset = cast(post, params, [:title, :topics])
iex> changeset.params
%{topics: []}

# Changing default
iex> params = %{title: "", topics: []}
iex> changeset = cast(post, params, [:topics], empty_values: [[], nil])
iex> changeset.params
%{title: ""}

# Augmenting default
iex> params = %{title: "", topics: []}
iex> changeset =
...>   cast(post, params, [:topics], empty_values: [[], nil] ++ Ecto.Changeset.empty_values())
iex> changeset.params
%{}

Which leads to an understanding that giving a param that matches any of the empty values is the same as not providing that param at all, so I was expecting something kinda like this to happen:

types = %{title: :string}
data = %{title: "My base title"}

{data, types}
|> Ecto.Changeset.cast(%{title: ""}, [:title])
|> Ecto.Changeset.apply_action(:validate)
# {:ok, %{title: "My base title"}}

But the actual behavior is described correctly further up in the docs:

Empty values are always replaced by the default value of the respective field.

Which means it only really works for full schema changesets (since those can have default values defined on their fields), and for everything else a param set to an empty value is the same as setting the field to nil.

I feel like we need to either update the code example in the docs to match the current behavior, or update the behavior to match the code example in the docs (unless there's some alternative I'm not seeing here 😅).

If the decided approach is to just update the docs, it would be great to also have a way to define default values for schemaless changesets (maybe data could be the defaults in this case, or allow a triple {data, types, defaults}).

I'd be happy to provide a PR for this, just figured it would be better to open an issue to discuss first 🙂

costaraphael avatar Mar 29 '24 17:03 costaraphael

Thanks for the report.

empty_values means the values are treated as nil. This is not specific to schemaless changesets either. Passing title: "" to a regular schema will also erase its default value, no? If you want to filter empty attributes, then you should filter them before calling cast.

We should update the docs, PR welcome!

josevalim avatar Mar 29 '24 21:03 josevalim

This is not specific to schemaless changesets either. Passing title: "" to a regular schema will also erase its default value, no?

Passing title: "" using a regular schema will replace the value in data by the default value of the field (or nil if the field doesn't have a default).

It's just that there's no such "default values" mechanism for schemaless changesets, which would be very handy for parsing untrusted data.

I can start a discussion regarding this on the mailing list, though. It feels out of the scope for this issue.

We should update the docs, PR welcome!

I'll send one over at some point tomorrow!

costaraphael avatar Mar 29 '24 23:03 costaraphael

You are correct. The only option I can think right now is to do a post-pass on the data converting nils back to the default value, sorry. :( I still don't think it is worth supporting the triplet format for this use case. :(

josevalim avatar Mar 30 '24 18:03 josevalim