#[changeset_for] now skips optional fields when None
#47
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Review? @mfpiccolo |
Closed
|
This is great! Good work 👍 |
787b04b to
eec068c
Compare
My original thoughts on how to implement this involved boxing and sticking in a `Vec`. Unfortunately, that requires implementing `AsChangeset` for `UserChanges` and not `&UserChanges`, which I don't want to do by default. If you actually want to assign `NULL`, you can still do so by doing `update(table).set(column.eq(None))`. In the future we might introduce an additional type to make it possible to assign null through codegen. With this change in implementation, I've removed the impl of `Changeset` for `Vec<T>`, as it feels redundant with a tuple containing options. I've done a best effort to make sure we generate valid SQL in the various cases. There is still one error case, when the resulting changeset has 0 fields to update. This will be corrected in another PR, as I'm still considering whether we should do magic to return the same thing as we would otherwise, or whether it's an error case. Fixes #26
eec068c to
197e2b8
Compare
|
Looks great. 👍 Well done! |
sgrif
added a commit
that referenced
this pull request
Jan 19, 2016
This allows users to opt into the behavior of `changeset_for` that was present prior to #47. I still feel that the default behavior of skipping optional fields is the most common case, as explicitly wanting to set a column to `NULL` is a pretty niche case to begin with, and much more frequently the field would be `None` on the struct because you've populated it from a web form or API endpoint that doesn't specify all fields.
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.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
My original thoughts on how to implement this involved boxing and
sticking in a
Vec. Unfortunately, that requires implementingAsChangesetforUserChangesand not&UserChanges, which I don'twant to do by default.
If you actually want to assign
NULL, you can still do so by doingupdate(table).set(column.eq(None)). In the future we might introducean additional type to make it possible to assign null through codegen.
With this change in implementation, I've removed the impl of
Changesetfor
Vec<T>, as it feels redundant with a tuple containing options.I've done a best effort to make sure we generate valid SQL in the
various cases. There is still one error case, when the resulting
changeset has 0 fields to update. This will be corrected in another PR,
as I'm still considering whether we should do magic to return the same
thing as we would otherwise, or whether it's an error case.
Fixes #26