Skip to content

Conversation

avit
Copy link
Contributor

@avit avit commented Aug 13, 2014

When merging search criteria, it's necessary to use the same join dependency with a shared alias tracker. This can be achieved by passing a context option when building a Search object.

I've added the option and exposed Context#alias_tracker.

@jonatack
Copy link
Contributor

Thanks @avit :-) can you update so that the tests pass for Rails 3.0 and squash your commits?

@jonatack
Copy link
Contributor

uninitialized constant ActiveRecord::Associations::AliasTracker

Rails 3.0 doesn't seem to be finding your new method.

@avit
Copy link
Contributor Author

avit commented Aug 14, 2014

Ah, thank goodness for CI. I'll move things around.

I will also expose "join_sources" on the context. This will eventually enable something for chaining like search.or(search) which is essentially what we're doing in our application (searching by multiple filters).

When merging search criteria, it's necessary to use the same join
dependency with a shared alias tracker. This can be achieved by passing
a context option when building Search objects.

Rails 3.0: use Model.joins(context.join_associations) to merge relations
with JoinAssociations.

Since Rails 3.1: can use Model.joins(context.join_sources) to merge
relations with Arel::Join nodes.

Since Rails 4.1: only context.join_sources is available as Arel,
context.join_associations stops being supported.
@avit
Copy link
Contributor Author

avit commented Aug 15, 2014

I reworked this to correctly support the different approaches used in each ActiveRecord version:

  • 3.0 doesn't have AliasTracker so we can skip it there. It's not critical to have. All later versions define it.
  • 3.0 doesn't have join_sources, and doesn't support using Arel::Join objects directly anyway: relation.joins(arel_outer_join) will output "INNER JOIN OUTER JOIN". Instead, 3.0 should use join_associations, which gets applied correctly.
  • 3.1 and later has join_sources which works correctly and seems preferable.
  • 4.1 no longer has join_associations, so by then you should be using join_sources.

There are some conditionals in the specs to reflect the version support.

jonatack added a commit that referenced this pull request Aug 15, 2014
Support merging searches using shared context
@jonatack jonatack merged commit cd581f8 into activerecord-hackery:master Aug 15, 2014
@jonatack
Copy link
Contributor

Thanks @avit, nice work 💚

Could you document this for everyone in the readme or wiki?

@avit
Copy link
Contributor Author

avit commented Aug 15, 2014

Thanks for the quick merge! The details are in the wiki: Merging Searches

@jonatack
Copy link
Contributor

Great! 🎉

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.

2 participants