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_name is deprecated in Rails 3.1 #9

Closed
wants to merge 3 commits into from
Closed

primary_key_name is deprecated in Rails 3.1 #9

wants to merge 3 commits into from

Conversation

joelmoss
Copy link
Contributor

@joelmoss joelmoss commented May 6, 2011

primary_key_name is deprecated in Rails 3 and shouts warnings in 3.1. Replaced with foreign_key

@@ -191,7 +191,7 @@ module Shoulda # :nodoc:
end

def foreign_key
reflection.primary_key_name
reflection.foreign_key

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be better to see whether the Rails Version was actually 3.1? - that way it would still work for 3.0

i.e.

      if defined?(Rails) && Rails::VERSION::MAJOR >= 3 && Rails::VERSION::MINOR >= 1
        reflection.foreign_key
      else
        reflection.primary_key_name
      end

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@omarqureshi Should be good to go now, thx.

@plentz
Copy link

plentz commented May 19, 2011

+1

@haines
Copy link

haines commented May 19, 2011

Definitely keen for a fix for these warnings.

Rather than test the Rails version, why not

def foreign_key
  if reflection.respond_to?(:foreign_key)
    reflection.foreign_key
  else
    reflection.primary_key_name
  end
end

(or even a one-liner using ? :)

@omarqureshi
Copy link

even better!

@jasonm
Copy link
Contributor

jasonm commented May 19, 2011

@jasonm
Copy link
Contributor

jasonm commented May 20, 2011

Thanks, all.

Merged in with commit 6b4035c

@jasonm jasonm closed this May 20, 2011
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

Successfully merging this pull request may close these issues.

None yet

5 participants