don't remove empty string #28
Open
Conversation
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.
|
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
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
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
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.