-
Notifications
You must be signed in to change notification settings - Fork 257
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
Custom facet value nil exception #1256
Custom facet value nil exception #1256
Conversation
@@ -282,7 +282,7 @@ def facet_value_to_fq_string(facet_field, value) | |||
|
|||
case | |||
when (facet_config and facet_config.query) | |||
facet_config.query[value][:fq] | |||
facet_config.query[value].blank? ? '-*' : facet_config.query[value][:fq] |
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.
Is -*
actually a Solr query? I think you might mean -*:*
?
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.
Do you think this can this be simplified any?
facet_config.query[value].blank? ? '-*' : facet_config.query[value][:fq]
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.
- yup -: is what i meant will fix
- i thought that was pretty succint, any suggestions?
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.
Sorry, I wasn't thinking about succinctness, more about clarity and understandability. Maybe something like this?
if facet_config.query[value]
facet_config.query[value][:fq]
else
# exclude all documents for the bogus facet query key
'-*:*'
end
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.
yeah, ok, more readable to humans, will update
Thanks. Want to squash your commits? |
e0e1942
to
d1005c2
Compare
Commits squashed |
Can you update the commit message to describe this change a little better? |
…r requests a custom facet query value that does not exist (fixes projectblacklight#1255)
d1005c2
to
c119f0c
Compare
👍 |
Thanks. Quick question, will this get cherry-picked into v6.x or does that need a separate request? |
Yes, I merged it into master in #1257. In the future, it would be better if you targeted pull requests at |
ok. good to know, thanks. |
Proposed fix for #1255 including 2 new tests.