Fix that #exists? can blow up with ThrowResult exception #6521

Merged
merged 1 commit into from Jun 11, 2012

Conversation

Projects
None yet
2 participants
Contributor

Empact commented May 28, 2012

FinderMethods#construct_limited_ids_condition will raise ThrowResult if the limited reflection comes back empty. The other callers of #construct_limited_ids_condition handle this exception (more specifically, the callers of construct_relation_for*), but #exists? didn't until now.

The included test hits this condition, and after the tests runs green.

Contributor

Empact commented Jun 8, 2012

Can somebody take a look at this? I ran into when running an #exists? against a paginated collection from kaminari in a view.

Owner

rafaelfranca commented Jun 11, 2012

Seems good. Could you squash the commits?

@Empact Empact Fix that #exists? raises ThrowResult when called with an empty limited
reflection.

ActiveRecord::FinderMethods#construct_limited_ids_condition will raise
ThrowResult if the limited reflection comes back empty. The other callers
of #construct_limited_ids_condition handle this exception (more specifically,
the callers of construct_relation_for*), but #exists? didn't until now.
340a93f
Contributor

Empact commented Jun 11, 2012

Done. Also added the complete explanation to the commit message.

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 11, 2012

@rafaelfranca rafaelfranca Merge pull request #6521 from Empact/throw-result
Fix that #exists? can blow up with ThrowResult exception
f48e767

@rafaelfranca rafaelfranca merged commit f48e767 into rails:master Jun 11, 2012

Contributor

Empact commented Jun 11, 2012

Thanks - would you also take a look at #6523? It's now squashed and such as well.

@rafaelfranca rafaelfranca added a commit that referenced this pull request Jun 11, 2012

@rafaelfranca rafaelfranca Merge pull request #6521 from Empact/throw-result
Fix that #exists? can blow up with ThrowResult exception
Conflicts:
	activerecord/lib/active_record/relation/finder_methods.rb
bbec883
Owner

rafaelfranca commented Jun 11, 2012

I'll look at it. But I need to discuss with someone with a better knowledge of the active record first.

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