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

don't remove empty string #28

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

exoconnor
Copy link

This is a mildly breaking change, but is also what I assumed default behavior would be, curious what others think.

With current cast behaviour, if an empty string is sent as a param, that string gets removed, due to default empty values behaviour https://hexdocs.pm/ecto/Ecto.Changeset.html#module-empty-values

If I want to clear a text field, I typically send a json object of the form

{ field-to-clear: ""}

This is getting changed to nil, and I can't clear fields.

I could set default empty string, but there's only one update route, and other actions will send an object of form

{ other-field: 55 }

And with default empty string, this unrelated update zeroes out my text field.

This update is all that's necessary to get my narrow use case working, I haven't dug too hard into potential consequences elsewhere.

Requires ecto version bump to pass tests.

Also leaving internet as soon as I post this, so not going to be a good conversationalist for a couple days.

Connor Wilson added 2 commits August 23, 2018 13:29
With current cast behaviour, if an empty string is sent as a param, that string gets removed. Setting nil to be the only empty value allows empty string to go in the changeset.

Requires ecto version bump to pass tests.
@vic
Copy link
Owner

vic commented Sep 11, 2018

Looks like something reasonable to me. Could you please also add a test to your PR?

I'm just not using this package right now. So I hope someone can give it a try and see if we should bump a major version or something.

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

Successfully merging this pull request may close these issues.

None yet

2 participants