search

appbaseio / reactivecore

Core architecture of reactive UI libraries
Apache License 2.0
33 stars 28 forks source link

Suggestion filtering hides synonyms #29

Open focusaurus opened 5 years ago

focusaurus commented 5 years ago

I've enabled synonym support in my elasticsearch, which means, for example, end users might search for "trousers" and get back results that do not contain "trousers" but do contain "pants". I think that getSuggestions is hiding these results. Have you put any thought into this scenario? I think I understand what the filtering here is trying to do, which is handle when the user has changed their search term by the time results come back ( right? ), but in my case it's hiding results I'd still like to see.

siddharthlatest commented 5 years ago

@focusaurus This is a good point. The filtering is there to get distinct suggestions, however the way it currently is being set, it would not show synonyms. I don't have an answer atm on what would be the best way to handle this, open to suggestions.

siddharthlatest commented 5 years ago

@focusaurus This should actually be possible, can you try using https://opensource.appbase.io/reactive-manual/advanced/customsuggestions.html?

willopez commented 5 years ago

@siddharthlatest I have implemented your suggestion, and the synonyms still get filtered out. And that is because, getSuggestions will have already been called by the time custom suggestions rendering executes. This line is redundant, https://github.com/appbaseio/reactivecore/blob/master/src/utils/suggestions.js#L30, Elasticsearch will have already search for a match, and it's not taking into consideration synonyms, which Elasticsearch does.

siddharthlatest commented 5 years ago

@willopez Is the above comment still relevant? With v3.0, we pass rawData which would contains the unparsed suggestions. I saw the related PR here, but the currentValue based match is an important filter to remove duplicates (when your data may not contain distinct values) and in most scenarios is a good default. Removing that isn't a good idea.