-
Notifications
You must be signed in to change notification settings - Fork 124
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
Issue-138 Add Eager Load Option #139
Conversation
This is a very valuable feature. Can it be released, please, @rafaelfranca? |
Hello @rafaelfranca . Should I do something else here or this is ok the way it is now? Thanks in advance |
Hey @rafacoello, it seems you forgot to deprecate the locator with only one argument, can you add that? |
156c150
to
b3c1b9d
Compare
Hey @rafaelfranca Sorry. I miss-understood the first time. Please let me know if that way is Ok for the deprecation. Thanks! |
lib/global_id/locator.rb
Outdated
return unless gid && find_allowed?(gid.model_class, options[:only]) | ||
|
||
if locator_for(gid).method(:locate).arity == 1 | ||
warn "#{Kernel.caller.first} warning: method locate(gid) is deprecated. Calling `locate(gid)' is deprecated. Please use `locate(gid, options)' instead." |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
warn "#{Kernel.caller.first} warning: method locate(gid) is deprecated. Calling `locate(gid)' is deprecated. Please use `locate(gid, options)' instead." | |
ActiveSupport::Deprecation.warn "Calling `locate(gid)' is deprecated. Please use `locate(gid, options)' instead." |
Also see https://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html#breaking-changes
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hey @bdewater Done. Thanks!
fe4ab58
to
bca0e65
Compare
+ added eager load to default locators (in locate and locate_many methods) with backwards compatibility + added tests cases with includes method in use. + created a new stub class for mocking the includes behavior ~ modified Person::Child class to have a relationship called parent.
bca0e65
to
98e02be
Compare
~ Modified Person::Child class to have a relationship called parent.