Proposal: Make `Ecto.Changeset.validate_change` call the validator for `nil` values

[Mário Guimarães]

Hi,

if `nil` is a valid value to change a field to, then `Ecto.Changeset.validate_change/3,4` should call the validator function when changing a field to `nil`.

What do you think?

Thanks

Mário

[José Valim]

This is documented behaviour and changing this would be a breaking change. That is already a red flag.

And given Ecto is now stable API, that is basically a no-no.

Plus you typically don't want to validate nil values, which is why it behaves as is. If you want to validate `nil` values, then you can roll your own, you don't need to use validate_change.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

[Mitchell Henke]

To add to this, I've written a patch to support this feature: https://github.com/elixir-ecto/ecto/compare/master...mitchellhenke:validate_change_nil?expand=1

I'm happy to submit the patch, but I was not sure to the extent that this would break existing implementations or if it would be accepted :)

[Mário Guimarães]

José 

This change can be supported without breaking the API: just add a new option. 

Also, since the validate_change function is used by most validation functions, the other solution you propose,  that of rolling our own nil validation, won't work,  because it would not be leveraged by the other validations. 

A new option is the best solution, I believe. 

Thanks

Mário 

--
You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.
To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGnRm4KQTe4_gxhT%2B2omN3A3xbY0ZOQq10GoFUkDTw6JRe1WRg%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.

[José Valim]

Ecto does not need to support everyone's use case out of the box. People can write their own, publish a lib, fork Ecto, etc.

We have no plans to add this to Ecto. Thanks.

José Valim

www.plataformatec.com.br

Skype: jv.ptec

Founder and Director of R&D

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAF7CYk72ZB5h7hgztXUSKMBgCuyFXQYsc3pFe0ghC3pRdmX3oQ%40mail.gmail.com.

[Mário Guimarães]

So Ecto is closed except for bug fixing, is that so? 

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGnRm4L97jU4TSrCKdg5LR-mr5aHVc8SNc2284p%2BUd9-QNR5Ow%40mail.gmail.com.

[José Valim]

Generally speaking yes. But maintainers can add some exceptions if they feel it is necessary.

But most should expect Ecto is what it is.

--

[Mário Guimarães]

I really doubt if there is a simpler change than this one I proposed, which makes me think that hardly any other change common from outside the maintainers close circle will get accepted.

I really regret such decision for Ecto, and it's community,  because I like this library and wanted to see it evolve continously.

In fact, it's a strange decision as we all know that in computing nothing is ever actually finished, unless it means it is dead and no one cares. 

All the best

--

You received this message because you are subscribed to the Google Groups "elixir-ecto" group.
To unsubscribe from this group and stop receiving emails from it, send an email to elixir-ecto...@googlegroups.com.

To view this discussion on the web visit https://groups.google.com/d/msgid/elixir-ecto/CAGnRm4%2BV8cSZmT1N8EkgBgzL5%2BaVbPCPvtYE%2BZue0T2%2B51CRow%40mail.gmail.com.

[José Valim]

This is a simple change but it can be easily achievable without making it part of Ecto.

A complex feature may actually have more chance of being made part of Ecto, because otherwise users have no option. But, no guarantees.

Also, Ecto is not finished. The focus is rather on maintenance.

In fact, it's a strange decision as we all know that in computing nothing is ever actually finished, unless it means it is dead and no one cares. 

I don’t think this is generally known. I don’t even think it is generally agreeable.

[Mário Guimarães]

In this case there is no simple option because too many other validation functions are dependent on `validate_change`, so anything done by the users would need to be applied to every other validation, which is something error-prone done at user's side...

[José Valim]

Can you please explain to me which of the existing validations should run on nil? They will all fail on nil. I can’t compute String.length on nil. Unless you are proposing that all of the validation code should be changed to also consider nil, then this change is the opposite of simple.

[José Valim]

If you want something to fail when not there. Then use validated_required.

Given this and the issues tracker discussion, it seems you are using Ecto very differently than what it is intended. I would recommend a discussion on the forum about your use case. I have personally invested a lot of time into this, so I won’t be able to help your further.

[Mário Guimarães]

I think I have understood why validate_change does not run on nil. Correct me if I am wrong, but it seems that all validation functions only validate non-empty changes, so when the change is to an "empty" value (nil or something else that translates to nil as per empty_values options), there is nothing to validate at all.

In case this thought is correct, for me it is fine, but it is a design decision that needs to be clearly documented. Therefore, I suggest for this to be included in the documentation of Ecto.Changeset.

This is something that I did not see explained anywhere.

I come to think often that Ecto's documentation would benefit greatly if design decisions that have influenced its API were documented more clearly.

[José Valim]

A pull request is welcome.

[José Valim]

Also a bit of feedback for future interactions. Instead of assuming the library is wrong and needs new features, please ask for the use cases and please be open to discussion. Especially feedback from maintainers. I have been giving evidence validations were working as desired since our first interaction today.

I know it is weird for me to basically say “listen to me”, since I am the maintainer in this case, but this is truly an advice that I follow when contributing to other projects.

The time I spent on our conversation could have been used to largely improve the docs. But maintainers are time constrained and now I won’t have the time to do it.

Since this is not the first time this happens between us, it is clear we cannot communicate well. I like to say that when there is a communication issue, both sides are to blame, and I believe it here. However, I most optimize how I use my time, for my own sanity, so please note I will keep our interactions short in the future.

[Mário Guimarães]

I understand, by email it can be difficult to communicate clearly, and I am always open to discussion.

If you are overwhelmed, please consider delegating the discussion to other maintainers, if you can do that.

Some of the issues I open could be avoided if the maintainers where more responsive on Slack (the channel I mostly use),

and there it seems that only micmus is responsive (the others that respond to me do not seem to be maintainers).

I generally open the issue when no one there gives a satisfactory answer.

Don't feel in need to quickly give an answer to me for an issue. I can easily wait for the next day for an answer, but it is you who seem to be in urge to close discussions.

Have a good night of rest.

[José Valim]

Some of the issues I open could be avoided if the maintainers where more responsive on Slack (the channel I mostly use),

and there it seems that only micmus is responsive (the others that respond to me do not seem to be maintainers).

I want to be very clear: It is not the responsibility of the maintainers to be responsive to your enquiries. We mostly work on it on our free time. No one is responsible to allocate their free time to make sure you have you have answers in reasonable time. You are not entitled to any of our focus.

If you want to have discussions, then please open up in proper media, such as the Elixir Forum, and then wait accordingly, as everybody else. Maybe a maintainer will reply, maybe somebody else, maybe nobody. The issues tracker is not the place of discussions.

Instead of telling how the whole Ecto team should behave, please consider a moment of introspection and see how you can help this small group. Several hints have been given on this thread already and I have also emailed you privately not long ago.

It is my responsibility to keep the issues tracker tidy and I will do it on whatever pace I find adequate for myself.

Please consider this thread closed. If you persist on the topic, I will ban your current e-mail address from the mailing list.

Have a good night,