Fix Changeset.changed?/2 for assocs#4544
Conversation
Setting assoc to nil actually errors as it is not handled in changed?/2, while settin existing assoc to belongs_to assoc returns changed?/2 as false.
It now returns true whenever `fetch_change? != :error`. Previously, it returned
false when assoc was set to existing struct (as `changeset.changes == %{}`) and
errored when set to nil (due to `nil.changes != %{}` check in `relation_changed?`.
Fixes elixir-ecto#4543.
e74f9ee to
2fce190
Compare
|
We should fix it to handle |
|
Hi @josevalim thank you for looking into this. Note that handling |
|
Yes, we need to check more cases. And potentialy make it recursive. But simply checking if the field is set is not enough. |
|
Ping! :) |
|
Sorry for the delay. I've been tied up with other priorities — I will try to move this forward till the end of this month... |
|
@josevalim I am having some difficulty coming up with test cases that would fail with already removes profile assoc changeset (as it would contain We could probably manipulate changeset struct directly, but we should probably test for scenarios that can results from using Changeset functions? |
It now returns true whenever
fetch_change? != :error. Previously, it returnedfalse when assoc was set to existing struct (as
changeset.changes == %{}) anderrored when set to nil (due to
nil.changes != %{}check inrelation_changed?.Fixes #4543.