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

Primary key not returned when upserting specific attributes #79

Closed
abinoda opened this issue Sep 16, 2018 · 12 comments
Closed

Primary key not returned when upserting specific attributes #79

abinoda opened this issue Sep 16, 2018 · 12 comments

Comments

@abinoda
Copy link

abinoda commented Sep 16, 2018

This issue was introduced when I upgraded from 0.9.1 to 0.9.3. See the example belowuser.id is nil in the final object. Again, this code works fine in v0.9.1 but not 0.9.3.

class User < ApplicationRecord
  upsert_keys [:my_id]
end

user = User.new(
  name:   name,
  email:  email,
  my_id:  my_id
)

# Overwrite email field but not name
user.upsert!(attributes: [:name])

user.id.nil? # => true
@jesjos
Copy link
Owner

jesjos commented Sep 16, 2018

Hi! Thanks for reporting this!

I'm trying to reproduce this using a unit test, but it's going nowhere. See this commit: 3c28213

I expected it to (mis)behave the same way, but I can't reproduce the behavior.

Maybe there's something else about your User record or how the db table was setup that causes the problem? Or maybe there's some kind of version incompatilibility with Rails?

@abinoda
Copy link
Author

abinoda commented Sep 16, 2018

@jesjos Just left a comment 3c28213#r30519950

@abinoda
Copy link
Author

abinoda commented Sep 16, 2018

@jesjos I'll pull down the code and check out that test. Will get back to you.

@abinoda
Copy link
Author

abinoda commented Sep 16, 2018

@jesjos Ok, so I wasn't able to get that test to fail either, which got me curious. I tried upgrading my app to Rails 5.2 from 5.1.6 and the issue went away. So this issue is specific to Rails 5.1.

Now I'm trying to run the test suite for active_record_upsert with the Rails 5.1 Gemfile to try and isolate what the issue was but I'm getting tons of test failures.

What are your thoughts?

@jesjos
Copy link
Owner

jesjos commented Sep 16, 2018

I’m not in a position to try anything out right now, but I can try later today. It seems the travis builds aren’t building the correct rails versions. I need to take a look at that as well. I’ll get back to you.

@jesjos
Copy link
Owner

jesjos commented Sep 16, 2018

Can confirm our travis build matrix has not been working as intended, all tests have run on rails 5.2 :(

Seeing a lot of errors. Guess we'll have to start fixing them.

@abinoda
Copy link
Author

abinoda commented Sep 16, 2018

@jesjos Gotcha. Yeah I ran manually by changing Gemfile contents to eval_gemfile "#{__dir__}/Gemfile.rails-5-1", running bundle, then running the tests. Glad we discovered this though!

@jesjos
Copy link
Owner

jesjos commented Sep 16, 2018

Yeah, the way I do it is I use an environment variable:

$ BUNDLE_GEMFILE=Gemfile.rails-5-1 bundle install
$ BUNDLE_GEMFILE=Gemfile.rails-5-1 bundle exec rspec

@jesjos
Copy link
Owner

jesjos commented Sep 17, 2018

Working on this here: #79

I've got most tests working, some left.

@jesjos
Copy link
Owner

jesjos commented Sep 19, 2018

Closed in #82

@jesjos jesjos closed this as completed Sep 19, 2018
@olleolleolle
Copy link
Collaborator

Yay!

@jesjos Thanks for listening and building!

@abinoda Thanks so much for reporting with such clarity and detail! (Happy that your test suite helped this project, too!)

♥️ 💛 💚 💙 💜

@abinoda
Copy link
Author

abinoda commented Sep 19, 2018

@jesjos Thanks for resolving this! And thank you @jesjos @olleolleolle for maintaining this library. It's been critical for my project and is a much needed piece in the Ruby/Rails ecosystem.

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

No branches or pull requests

3 participants