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

BlockJoin improvements for Rails applications #23

Open
wants to merge 20 commits into
base: master
Choose a base branch
from
Open

BlockJoin improvements for Rails applications #23

wants to merge 20 commits into from

Conversation

DougKeller
Copy link

@DougKeller DougKeller commented Mar 28, 2019

Thanks for your PR - because this isn't part of the official branch yet, we chose to try out your changes for our Solr needs.

That said, there were a few changes/improvements we had to make to get it up and running for our app. Unfortunately these changes haven't been tested due to time constraints, but I wanted to put up this PR for you to consider some things that may have been missed.

Changes:

  • Prevent indexing of a child doc without indexing its parent.
    • Because child/parent docs are expected to be indexed in sequence, indexing a child without its parent will break the index for that child/parent pair.
  • Accept Rails collection proxies has a valid child_documents value
    • CollectionProxy instances are enumerable, so it'll behave the same as an Array
  • Escape the parent filter values when performing a BlockJoinQuery.
    • Our models had colons in their class names due to being nested within modules. Not escaping these colons was causing solr query parsing issues.
  • When performing a BlockJoin facet, only filter on the parent type
    • When the json facet was specifying the allParents filter to be the full all_parents_filter, facet values were coming back incorrectly. Changing this to only filter on type:parent fixed the counts.
  • Fixed a bug with FieldJsonFacet not setting name correctly
  • Implicitly include child documents in the query result when performing a BJQ
  • If a hit returns _childDocuments_ in its field list, serialize each _childDocuments_ value as a Sunspot::Search::Hit
  • Add support for sorting on child document fields
    • order_by :some_field, :desc, block_join: on_child(Child) { with(...) }

@DougKeller
Copy link
Author

We ultimately ended up going a different route than BJQ for our app due to issues with keeping parent/child docs in sync, so I have deleted our fork of this branch. I'll still leave this PR open in case there's anything valuable in it, however it's unmaintained and isn't totally bug-free.

@giovannelli
Copy link
Member

Hi @DougKeller thank you for the PR, we'll review it. I'm sure there some valuable stuff here.

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