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

Phoenix.HTML.Form.field_value/3 does not return then change when given a Ecto.Changeset #71

Closed
ThijsWouters opened this issue Dec 19, 2016 · 6 comments

Comments

@ThijsWouters
Copy link

When I create a form with a Ecto.Changeset with changes, I would expect that the changes are taken into account when the form is populated. Right now only the params are taken into account.

changeset = %Model{number: 1}
            |> Ecto.Changeset.cast(%{number: 2}, [:number])
            |> Ecto.Changeset.put_change(:number, 3)

Phoenix.HTML.FormData.to_form(changeset)
|> Phoenix.HTML.Form.field_value(:number) # => 2, I would expect this to be 3
ThijsWouters added a commit to ThijsWouters/phoenix_ecto that referenced this issue Dec 20, 2016
@ThijsWouters
Copy link
Author

ThijsWouters commented Dec 20, 2016

No, that did not fix it. I added a test to describe the behaviour I am after. Correct me if I am wrong:

  • Look in the changes,
  • if nothing is there, look in the params,
  • if nothing is there, look in the data,
  • if nothing is there, return the default value or nil.

I'll see if I can make the changes.

@ThijsWouters
Copy link
Author

ThijsWouters commented Dec 20, 2016

I can't fix it in this repo. This behaviour needs a change in phoenixframework/phoenix_html.

The precedence now is: params -> default -> changes -> data.

This feels wrong, especially that the default value is taken before the changes or actual data.

I think the precedence should be: changes -> params -> data -> default.

@josevalim
Copy link
Member

@ThijsWouters can you please check Ecto master?

You are correct except by the default which should have higher preference than the data because the default often uses a value computed on top of the data.

@ThijsWouters
Copy link
Author

@josevalim Taking the default over the data does not make sense to me.

Let's say we have a transaction with a date. We want a default date of today.

So you say that when we start to edit the transaction, the date field is not filled with the previously chosen date, but with the current date?

@josevalim
Copy link
Member

The default is used for cases where you want to use a value that was computed on top of the data value, hence you want it to have higher precedence:

input_value(form, :name, String.upcase(data.name))

If you really want a value if nothing else was set then you can do this:

input_value(form, :name) || "fallback"

Maybe it is a matter of giving it a better name than default, such as "computed".

Changing it would also be backwards incompatible, so finding a better way to describe is the best way to go.

Did the rest work as expected on master?

@ThijsWouters
Copy link
Author

It does work as expected on master, thanks.

default is indeed a confusing name for something like this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants