Pass ecto models to multiple_select

[NilsLattek]

I am currently playing around with the phoenix framework and I like the similarities to rails!
One thing that I think is still missing is the proper handling of many to many relationships. Regarding the changes required in the ecto framework there is already a ticket: elixir-ecto/ecto#183

I think another change would be to allow the multiple_select helper to work with ecto models. So that we can do something like this:

# in the controller:
# all_users = Repo.all(User)
<%= multiple_select f, :users, @all_users %>

And the form helper could fetch all currently assigned users using the value_from(form, field) method and set the "selected" property on the option elements. In Rails there is the http://api.rubyonrails.org/classes/ActionView/Helpers/FormOptionsHelper.html#method-i-collection_check_boxes to handle this case, but I think it is much nicer to use a multiple select so you can do autocomplete input fields.

[josevalim]

Just filter the data. In your controller:

users_select = Repo.all from u in User, select: {u.name, u.id}

There is no way Ecto nor Phoenix can guess which fields we should use. Better to be explicit. You can also use a Enum.map later:

<%= multiple_select f, :users, Enum.map(@all_users, &{&1.name, &1.id}) %>

[NilsLattek]

Thanks for your quick reply. The other form helpers like text_input set their value automatically from the form model. With multiple_select I have to pass in the current values manually (and transform them again) else the options won't be selected:

<%= multiple_select f, :users, @all_users, value: Enum.map(@changeset.model.users, fn(u) -> u.id end) %>

But when the form is rendered because of an error, the value property should be filled in with the current changeset.params.users attribute not changeset.model.users. At least this is the way the text_input helper works. This is handled in the private value_from method.
It looks like a lot of duplicate code if I do this in all forms manually.

In Rails the helper assumes some default names for the fields, but it is possible to override these:

collection_check_boxes(:post, :author_ids, Author.all, :id, :name_with_initial)

[josevalim]

But when the form is rendered because of an error, the value property should be filled in with the current changeset.params.users attribute not changeset.model.users. At least this is the way the text_input helper works. This is handled in the private value_from method.

I see the issue now. The issue is that we are not correctly marking the proper fields as chosen when the data comes through the form.

In any case, I would still not support the :id, :name_with_initial version. With Phoenix.Ecto, it is much easier to do it at the query level and get only the data you need, instead of all Author data.

In Rails the helper assumes some default names for the fields, but it is possible to override these:

I know. :D We wrote it in simple_form before it was merged into Rails ;)

[NilsLattek]

I see the issue now. The issue is that we are not correctly marking the proper fields as chosen when the data comes through the form.

Correct! And we have to differentiate between rendering the form through the editmethod or the update method when the form validation failed:

• In the first case we have to transform a list of currently selected Users into a list of ids for the value field in order to mark the options as selected.
• In the second case we already have a list of user ids in the changeset.params.users property and would use these. So that we can show the error messages and the selected values from the user.

I hope this is understandable :-D If not I can try to make a demo project...
These points are just for the value property which holds the selected entries/ids.
For the other property holding all users I agree, it is fine do to this at the query level.

When Ecto supports inserting rows in the mapping table for many-to-many relationships this form feature would make it easier to handle these cases.

I know. :D We wrote it in simple_form before it was merged into Rails ;)

Haha right! If I remember correctly I read somewhere that you were part of the Rails core team, nice job man! 👍

[josevalim]

And we have to differentiate between rendering the form through the editmethod or the update method when the form validation failed:

You can use the :default option. It will use whatever you have given only if the params are not set. So you don't need to do the dance between models and params.

[josevalim]

I actually, I have just verified this is working as expected. You just want to use the default option when giving the default values from the model. :)

[NilsLattek]

Oh right the default option, did not see that. Sorry! However I am not sure if I have to use values or value.
I thought values is a list with all available rows and value is just the selected ids. In the example at https://hexdocs.pm/phoenix_html/Phoenix.HTML.Form.html#multiple_select/4 it uses values (values: [1]) for the selected ones, maybe an error in the documentation?

So I am currently experimenting with the following code:

<%= multiple_select f, :users,
            # fetched with a query select: {u.name, u.id}
            @all_users,
            # select the options in case of an error else it is nil and the defaults are used
            value: (@changeset.params || %{}) |> Map.get("users"),
            # or uses values?
            #values: (@changeset.params || %{}) |> Map.get("users"),
            # when value(s) is null use the mapping from the model (open the form for the first time)
            default: Enum.map(@changeset.model.users, &(&1.id)),
            class: "form-control" %>

This will not select the rows when opening the form for the first time, when the value property is nil (without a submit). It is working when I use values, but then it is not working when rendering an error. Little confused here :-D

Thanks for your help!

[NilsLattek]

I have just tried out your latest refactoring and I think the problem is that

{default, opts}  = Keyword.pop(opts, :default, [])
{multiple, opts} = Keyword.pop(opts, :value, default)

will return nil for multiple when value: nil is set in the template. Because the default value will only work when the keyword-list has no key value.

$ m = [value: nil]
$ Keyword.pop(m, :value, %{})
-> {nil, []}
$ Keyword.pop(m, :unknown, %{})
-> {%{}, [value: nil]}

[josevalim]

No, you just need :default:

<%= multiple_select f, :users,
        # fetched with a query select: {u.name, u.id}
        @all_users,
        default: Enum.map(@changeset.model.users, &(&1.id)) %>

[NilsLattek]

Haha okay I did make it more complicated :-D
Well there is still one difference, but I can live with that:
When the form validation fails it is still selecting the original value in the select and not the new value.
Let's say we have a name field which is mandatory, a description field and the multi select.
When I leave the name field empty, fill out the description and select a couple of values in the select and hit submit.
It will render an error, because the name is empty. It will also fill in my description text which I have entered in the form, so that I do not have to enter it again. But it will not select the values again with the help from @changeset.params.users.
It's a small thing I know, it's just not consistent with the other fields. But I can ignore that case for now ;-) Thanks for your time!

[josevalim]

No, you are right, we need to fix this.

[NilsLattek]

Ok, let me know if I can help or test something. I will take a look at the code as well.