Skip to content

Use configured searchFields in search 'get result' #6023

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

Open
wants to merge 1 commit into
base: next
Choose a base branch
from

Conversation

willstott101
Copy link

@willstott101 willstott101 commented Dec 12, 2017

Configuring the searchFields is a core part of the search module configuration. get result was not respecting that and had the default hard-coded. As all it does is pass the value through to the search object method (where it is an optional argument), I simply removed the reference.

Failing case: https://jsfiddle.net/15xd2e41/1/ (with 'name' in the source objects)
Working case: https://jsfiddle.net/ozcwbcf3/1/ (with the default 'title' in the source objects)

Configuring the lookupFields is a core part of the search module configuration. `get result` was not respecting that and had the default hard-coded. As all it does is pass the value through to the `search object` method (where it is an optional argument), I simply removed the reference.

Failing case: https://jsfiddle.net/15xd2e41/1/
Working case: https://jsfiddle.net/ozcwbcf3/1/
@willstott101 willstott101 changed the title Use configured lookupFields in search 'get result' Use configured searchFields in search 'get result' Dec 15, 2017
@jlukic
Copy link
Member

jlukic commented Feb 19, 2018

get result is also used internally to lookup the correct matching from inject.id, this could create false matches on other fields.

@willstott101
Copy link
Author

Thanks for the review!

I don't see where get.result is used in that file at all. It looks to be only a public API method, as far as I can tell.

inject.id uses it's own related inject.result which seems to be unrelated to me.

I'm not sure what I've missed, but I'd love to get this PR up to scratch if I can.

If it is required internally then perhaps we could add a searchFields argument to this method which passes through to search.object, and use that argument whenever it's required?

@lubber-de

This comment was marked as spam.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants