Skip to content

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