[BUG] Automatically setting belongs_to fields when nil

[jcelliott]

Versions Used Kaffy: 0.9.4 Phoenix: 1.6.15 Elixir: 1.14.2

What's actually happening?

I have a schema like this:

schema "issues" do
    ...
    belongs_to(:closed_by, User)
    ...
end

And my admin has this:

def form_fields(_) do
    [
        closed_by_id: nil,
    ]
end

This correctly renders the field as a select with all the Users as options. Until this issue is closed the closed_by field should remain nil. However, when the form loads it defaults to the first option (first User alphabetically), and without explicitly choosing a value in the select the preloaded value gets sent in the update changeset.

What should happen instead?

The field should remain nil unless explicitly changed.

Is there a way to automatically include a default nil value in the select options? It looks like I could provide a custom choices value to the field, prepending a nil value to my list of users, but that doesn't seem like a great option.

I would be happy to try to implement a fix if this is something that other people would like to see as well.

[KristerV]

can confirm that this created a bunch of issues in my database. just plain broke some functionality of my app. took a whole day to figure out it was Kaffy.

the solution I think is to always have a nil value as first option, even for belongs_to fields. maybe unless null: false in the database itself, but the changeset would catch that anyway.

@jcelliott what workaround did you use for this? overriding each field seems like a massive pain.

edit: since i have a custom Kaffy already (related to #281) i went ahead and debugged this issue and came up with this solution: https://github.com/KristerV/kaffy/commit/398641c89d4e1b57bf51a50f9b690169fb49444e if this is the right way to go about things i can make a PR. but honestly i didn't delve too deep into other usecases.

[jcelliott]

@KristerV I just prepended a nil value in my admin config for the module for the fields that I absolutely needed it on. It's definitely a pain. Your solution does seem like a better option, I'll probably use that. Doesn't seem like it will cause any issues because like you said, the changeset should catch any problems with that. Also, nil seems like the right default. If someone has special logic for what the default should be in their app, that should be implemented with custom logic in their admin config.

[aesmail]

@jcelliott @KristerV thank you both for this. A fix has been implemented based on @KristerV's work and will be available in the next release soon.