Use nil? instead of blank? to check dynamic finder result #8204

Merged
merged 1 commit into from Nov 13, 2012

Projects

None yet

3 participants

@nikitug
nikitug commented Nov 13, 2012

It's safe to use nil? instead of blank? because it's impossible to get an array on finder with bang; all_by fi

Fixes #7238

@carlosantoniodasilva

It needs a changelog entry. Also, I think it'll be good to have the tests on master as well, so that we can ensure it works perfectly fine there. Thanks!

@carlosantoniodasilva

Sorry, just noticed the other pull request for master 😄, thanks!

@carlosantoniodasilva

I think it'll need a rebase - again due to the changelog 😄

@carlosantoniodasilva

Can you also do like this, removing the nothing raised thing? https://github.com/rails/rails/pull/8202/files#r2112067 Thanks.

@nikitug nikitug Use nil? instead of blank? to check dynamic finder result
It's safe to use `nil?` instead of `blank?` because it's impossible to get an array on finder with bang;
`all_by` finder matches against regex without bang: `when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/`.

Fixes #7238
b56376b
@rafaelfranca rafaelfranca merged commit b56376b into rails:3-2-stable Nov 13, 2012
@rafaelfranca
Member

Thanks

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment