[#3512] Support facebook-style query params in cases where a facet pivot is configured #3514
+41
−5
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…vot is configured each_with_object ignores the return value of the supplied block; you need to mutate the passed object instead. Prior to this change, the ramifications of bug are that: * FilterField#permitted_params always includes empty hashes (the original, unmutated object we passed to each_with_object) for a facet pivot field. * When Blacklight::Parameters::deep_merge_permitted_params comes across empty hashes, it discards a present hash in favor of the empty one. * As the permitted params for all facet fields are merged together, the presence of even one facet pivot field leads us to discard all other permitted facet field keys. * We have empty hashes, rather than hashes with permitted facet keys going into Blacklight::Parameters#deep_unmangle_params! * Blacklight::Parameters#deep_unmangle_params! can find no permitted parameters that it needs to unmangle, so the facebook params never get unmangled, causing errors and unexpected behavior elsewhere in the stack. * Facebook-style query params don't work if a facet pivot is configured. Co-authored-by: Christina Chortaria <christinach@users.noreply.github.com>
sandbergja
force-pushed
the
pivot-facet-facebook-params
branch
from
3f84146 to
beec8f5
Compare
sandbergja
commented
Feb 21, 2025
| @@ -803,7 +803,7 @@ def export_as_mock | |||
| end | |||
|
|
|||
| it "is empty for empty result sets", :integration do | |||
| get :page_links, params: { f: { "format" => 'empty-result-set' }, counter: 1 } | |||
| get :page_links, params: { f: { "format" => ['empty-result-set'] }, counter: 1 } | |||
There was a problem hiding this comment.
Correcting the structure of the params
maxkadel
approved these changes
Feb 24, 2025
There was a problem hiding this comment.
Thanks @sandbergja! Looks good. I appreciate all the tests, and the debugging - looks like it was pretty tricky to figure out what was going on!
|
ooh this is prob a good candidate for a backport. Bad one! Thank you for finding and fixing! |
sandbergja
added a commit
that referenced
this pull request
Feb 24, 2025
…vot is configured (#3514) each_with_object ignores the return value of the supplied block; you need to mutate the passed object instead. Prior to this change, the ramifications of bug are that: * FilterField#permitted_params always includes empty hashes (the original, unmutated object we passed to each_with_object) for a facet pivot field. * When Blacklight::Parameters::deep_merge_permitted_params comes across empty hashes, it discards a present hash in favor of the empty one. * As the permitted params for all facet fields are merged together, the presence of even one facet pivot field leads us to discard all other permitted facet field keys. * We have empty hashes, rather than hashes with permitted facet keys going into Blacklight::Parameters#deep_unmangle_params! * Blacklight::Parameters#deep_unmangle_params! can find no permitted parameters that it needs to unmangle, so the facebook params never get unmangled, causing errors and unexpected behavior elsewhere in the stack. * Facebook-style query params don't work if a facet pivot is configured. Co-authored-by: Christina Chortaria <christinach@users.noreply.github.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
each_with_objectignores the return value of the supplied block; you need to mutate the passed object instead.Prior to this change, the ramifications of bug were that:
FilterField#permitted_paramsalways includes empty hashes (the original, unmutated object we passed toeach_with_object) for a facet pivot field.Blacklight::Parameters::deep_merge_permitted_paramscomes across empty hashes, it discards a present hash in favor of the empty one.Blacklight::Parameters#deep_unmangle_params!Blacklight::Parameters#deep_unmangle_params!can find no permitted parameters that it needs to unmangle, so the facebook params never get unmangled, causing errors and unexpected behavior elsewhere in the stack.Closes #3512