-
Notifications
You must be signed in to change notification settings - Fork 37
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
[SDPA-3128] Added property to exclude hits. #508
Conversation
@@ -83,7 +83,11 @@ export default { | |||
'title', | |||
'type', | |||
'url' | |||
] | |||
], | |||
exclude: { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I feel like this should be conditionally set based on the status of the nuxt-tide Grant sub-module. Is that required?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @GROwen I think this is depending on how we design the search index in backend and hard to tell just in front end. I feel the best way to control in front end is extensible whitelist. The exclude
is a blacklist, but still be ok here I think. We don't have the ability to extend the config for this default search page for now, so we can't move this setting into Grant module. Unless we add a API/hook to allow custom config merging into this search config.
But you need to test and make sure, without Grant
enabled in CMS, this exclude query is still working.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@tim-yao Yep the query will still return results with an absence of any docs of type: grant
in an index.
A whitelist won't be possible because of the additional requirement to check for non-null values - which complicates things. I also didn't want to go overboard with the flexibility of this solution because we don't know of any other requirements and it may all be made redundant by the UX discovery and refactor of the search implementation.
JIRA issue: https://digital-engagement.atlassian.net/browse/SDPA-3128
Changed