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

SCI & criteria support #37

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

SCI & criteria support #37

wants to merge 58 commits into from

Conversation

tomasc
Copy link
Contributor

@tomasc tomasc commented May 5, 2018

The search now respects SCI.

From the included spec:

class MyDoc
  include Mongoid::Document
  include Mongoid::FullTextSearch

  field :title
  fulltext_search_in :title
end

class MyInheritedDoc < MyDoc
end
MyDoc.fulltext_search() # => will return both MyDoc as well as MyInheritedDoc documents
MyInheritedDoc.fulltext_search() # => will return only MyInheritedDoc documents

It is also possible to pre-empt the search with Monogid criteria:

MyDoc.where(value: 10).fulltext_search()

Please note that this will obviously not work in case an index is shared by multiple classes (that are not connected through inheritance), since a criteria applies only to one class.

@tomasc
Copy link
Contributor Author

tomasc commented May 5, 2018

builds upon #35 , will rebased once that one is merged

@tomasc tomasc mentioned this pull request May 5, 2018
@tomasc
Copy link
Contributor Author

tomasc commented May 8, 2018

@dblock here we keep failing miserably on Mongoid 3 (but not 3.1). No idea why, again …

@dblock
Copy link
Contributor

dblock commented May 8, 2018

I can reproduce this locally and it looks like a clear bug introduced in this PR. We could drop support for 3.0, but I'd like to understand what's going on.

With Mongoid 3.

Both ExternalArtist and ExternalArtwork are using the same index, mongoid_fulltext.artworks_and_artists.

I am supposed to get back 2 objects, an external artist and artwork via ExternalArtwork.fulltext_search. The correct result is:

[#<ExternalArtist _id: 5af21a8f9b0b58e214000001, _type: nil, full_name: "Pablo Picasso">,
 #<ExternalArtwork _id: 5af21a8f9b0b58e214000002, _type: nil, title: "Portrait of Picasso">]

In Mongoid 3 the query is {"ngram"=>"picasso", "class"=>{"$in"=>["ExternalArtwork"]}} which is incorrect.

The code in document_type_filters does return {} unless fields['_type'].present?, in Mongoid 3 fields['_type'] returns #<Mongoid::Fields::Standard:...>", and in Mongoid > 3.1 that's nil. I am not sure what that means ;)

@dblock
Copy link
Contributor

dblock commented May 9, 2018

I tried fields['_type'].default_val.call instead, and that passed all tests, which tells me you're missing a test here for a descendant type, that is never not nil in the entire test suite.

@dblock
Copy link
Contributor

dblock commented May 9, 2018

This is simpler and works for all versions:

return {} if descendants.none?

@dblock
Copy link
Contributor

dblock commented May 10, 2018

@tomasc Got a moment to wrap this up?

@tomasc
Copy link
Contributor Author

tomasc commented Jul 11, 2018

@dblock will try to get to that this week. In the end I ended up not using the gem, but yes, let's wrap this up.

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.

None yet

2 participants