Upsert

[paulcsmith]

Closes #298

Needs

• Document the macro upsert_lookup_columns
• Document how to use upsert and upsert!
• Create in luckyframework/website to add section in the guides Document Avram#upsert website#716
• Extract a #save method that accepts a block so we don't have duplicate code

[jkthorne]

So this does not use on conflict. Is that going to be added in a later PR?

[paulcsmith]

@wontruefree Yep! Ideally it will use on conflict, but that'll require adding new methods to the query builder so figured we can at least get this out there in the meantime, then improve it later

[confact]

@paulcsmith A idea i have for find_or_create is to maybe add posibility to use a block.
then it use the parameters to find it and if not find call the block to add those fields. It makes more sense in my head using that.

as example:

email = "hakan@example.com"
SaveUser.find_or_create(email: email) do |u|
  u.email = email
  u.username = "confact"
  u.role = :user 
  u.name = params["name"]
end

Edit: changed to an more luckilized example instead of ruby on rails

[paulcsmith]

@confact I like that API, but for a couple of reasons it is tricky to get type-safe and to work with param parsing. But I like the idea. I tried a few APIs before and couldn't get something that worked, but I'll think on it and see if we can do something closer to that style.

[stephendolan]

@jwoertink I've merged in master to this branch and resolved the conflicts/updates necessary. What are the next steps to get this merge-able?

I'm happy to take some guidance and attempt to tackle finishing this one up myself, too!

[jwoertink]

@stephendolan Overall this looks good. I left a few suggestions to fix. The only major thing is we don't have the block forms of these methods. So we can either add those in, or roll with this as is and add the block forms as a separate PR. I'm good either way.

[stephendolan]

@jwoertink I opted not to introduce all the block variants in this PR so that we can get it cleaned out of the queue and at least have the basics available for folks in the next release.

If we merge this as-is, I can follow up with an issue to document the need for those additional overloads.

[matthewmcgarvey]

I feel iffy about this. Here's some things going through my mind, they might or might not be worth anything, just what I'm thinking:

• As I commented, unique_columns is weird to me because the call site provides data that isn't used in the fetch. It feels disjointed
• It's introducing two methods/macros that have to be used together
• We are depending on the query object that we generated

Because we separate out queries and operations I'm hesitant to believe this is the best way to go so I looked into Elixir's Ecto ORM. They don't have a find_or_create method or an upsert. Instead, they provide options on their Repo.insert function to handle conflicts.

https://hexdocs.pm/ecto/Ecto.Repo.html#c:insert/2

Can we talk before merging this?

[jwoertink]

Yeah, I'm ok with holding off on this.

[stephendolan]

I'm definitely cool with a different approach, as long as this doesn't sit for another year untouched 😆 .

Having just implemented this manually in two apps, it feels like an area where it'll be easy for end-users of Lucky to make mistakes until we provide the capability.

[confact]

Yea, I think it is good to have something like this. I am doing find and check if nil to create now in many places, so it would surely save a lot of code.

But the current flow is confusing to me. I am not sure what it is using to find it and what it will update. Is it some of them or all of them?

If we find a good way to handle that in a good way, it would be good. But I understand the issue @paulcsmith discussed with the parameters and block, so a ruby on a rails-like method might be hard.

Not sure exactly how to fix it. I think it is a good call to wait out and think it through.

Working on updating this

[jwoertink]

This is looking nice! Just a few questions, but I dig the direction.

[jwoertink]

I like how this reads.

[paulcsmith]

This was moved to SaveOperation#save so we don't need to remember to call it in multiple places

[jwoertink]

This is great stuff! Overall, the implementation seems fairly simple which is nice. Just a few small wording suggestions, but if you're cool with it, merge away!