No need for validation query unless relevant fields were changed #2905
Conversation
|
If you set the diff to hide white space changes, you'll see that the change here is very small. The function is now even longer and uglier with the additional nesting. 😆 This might be a nice place for someone to contribute a refactor. |
|
Thank you so much @nathanl! I have dropped some comments. We just need some tests too and we should be good to go! |
|
@josevalim Thanks for the feedback. I noticed that |
|
@nathanl the integration tests are now part of |
|
Hmm. I'm confused because |
|
@nathanl the integration tests are still shipped as part of ecto, because they are shared across adapters, but there is no adapter in ecto per se for us to run them against. |
unsafe_validate_unique/3 performs a query to ensure that the specified fields represent a unique combination of values in the repo. For example, perhaps the combination of city and state must be unique. But there's no need to perform this query if the changeset is not making any change to city or state. Fixes #2901
|
@josevalim Thanks for your feedback and explanation. I've updated this PR and also created #2908 and elixir-ecto/ecto_sql#79 to document what you explained about integration tests. Any more feedback? |
| assert_receive [function: :one, query: �to.Query{}] | ||
|
|
||
| unsafe_validate_unique(body_change, :title, DetectionRepo) | ||
| refute_receive [function: :one, query: �to.Query{}] |
There was a problem hiding this comment.
I was going to say we should break this apart but given everything is together for now, then it is best to leave it for another PR. :)
There was a problem hiding this comment.
@josevalim Here's a PR to break apart the test: #2909
|
❤️ 💚 💙 💛 💜 |
unsafe_validate_unique/3performs a query to ensure that the specifiedfields represent a unique combination of values in the repo. For
example, perhaps the combination of city and state must be unique.
But there's no need to perform this query if the changeset is not making
any change to city or state.
Fixes #2901