Skip to content
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

🧹 Fix spec to reflect new order of processing #55

Merged
merged 1 commit into from
Oct 5, 2023

Conversation

jeremyf
Copy link
Contributor

@jeremyf jeremyf commented Oct 4, 2023

I encourage you to read the code comments, reproduced here:

Yes there's a duplicate for add_access_controls_to_solr_params; but
that does not appear to be causing a problem like the duplication and
order of the now removed additional :add_advanced_parse_q_to_solr,
:add_advanced_search_to_solr filters. Those existed in their current
position and at the end of the array.

When we had those duplicates, the :add_advanced_parse_q_to_solr
obliterated the join logic for files.

<2023-10-04 Wed> Oh how I wish I wrote a bit more back in the day.
But alas, I didn't. So I must rebuild that context. Looking at the
":add_advanced_parse_q_to_solr obliterated the join logic" fragment,
I'm assuming that what I meant was "I think that the
'show_works_or_works_that_contain_files' processor chain should come
after the 'add_advanced_parse_q_to_solr' and
'add_advanced_search_to_solr' processor chains.

This assumption is bolstered by the implementation of
Hyrax::CollectionMemberSearchBuilderDecorator which introduces the
show_works_or_works_that_contain_files method. See
https://github.com/samvera/hyku/blob/07fde572f9152d513b13f71cae90dd4fdfbfba6c/app/search_builders/hyrax/collection_member_search_builder_decorator.rb#L16-L34

Related to:

I encourage you to read the code comments, reproduced here:

> Yes there's a duplicate for add_access_controls_to_solr_params; but
> that does not appear to be causing a problem like the duplication and
> order of the now removed additional :add_advanced_parse_q_to_solr,
> :add_advanced_search_to_solr filters.  Those existed in their current
> position and at the end of the array.
>
> When we had those duplicates, the :add_advanced_parse_q_to_solr
> obliterated the join logic for files.
>
> <2023-10-04 Wed> Oh how I wish I wrote a bit more back in the day.
> But alas, I didn't.  So I must rebuild that context.  Looking at the
> ":add_advanced_parse_q_to_solr obliterated the join logic" fragment,
> I'm assuming that what I meant was "I think that the
> 'show_works_or_works_that_contain_files' processor chain should come
> after the 'add_advanced_parse_q_to_solr' and
> 'add_advanced_search_to_solr' processor chains.
>
> This assumption is bolstered by the implementation of
> `Hyrax::CollectionMemberSearchBuilderDecorator` which introduces the
> `show_works_or_works_that_contain_files` method.  See
> https://github.com/samvera/hyku/blob/07fde572f9152d513b13f71cae90dd4fdfbfba6c/app/search_builders/hyrax/collection_member_search_builder_decorator.rb#L16-L34

Related to:

- https://github.com/scientist-softserv/adventist-dl/issues/538
@jeremyf jeremyf force-pushed the fix-i-hope-the-adv-search-builder-order-spec branch from 6ebdefb to 84a3ede Compare October 4, 2023 20:51
@jeremyf jeremyf merged commit 79ce65d into main Oct 5, 2023
3 of 7 checks passed
@jeremyf jeremyf deleted the fix-i-hope-the-adv-search-builder-order-spec branch October 5, 2023 02:09
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant