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

Pass the context through to the search service to restore function… #2075

Merged
merged 2 commits into from Feb 27, 2019

Conversation

Projects
None yet
3 participants
@cbeer
Copy link
Member

commented Feb 25, 2019

…al compatibility with Blacklight 7.

Some uses of search builders expect access to controller methods (e.g. current user, can? from cancan, etc). Instead of making consumers always override the search service, we can provide the context from the controller as a convenience.

@jcoyne

This comment has been minimized.

Copy link
Member

commented Feb 25, 2019

This seems to totally bust the interface so I'm not keen. Blacklight-access-controls passes an Ability instance, would that be sufficient?

@cbeer

This comment has been minimized.

Copy link
Member Author

commented Feb 25, 2019

I don't see how blacklight-access-controls could possibly work right now; as far as I can tell, it's doing the thing that's compatible with Blacklight 6, and that 7 completely removes the ability to do without providing a different SearchService and overriding the Blacklight::Catalog#search_service method.

@cbeer cbeer referenced this pull request Feb 26, 2019

Open

Update Spotlight to Blacklight 7 + Bootstrap 4 #2137

2 of 2 tasks complete
@jcoyne

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

@cbeer yes, it takes away the ability to configure the search builder....but that aside, could we just pass the Ability as a parameter to a SearchBuilder. That seems to be the part of the controller that we really need to work with in the SearchBuilder, right?

@cbeer

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

How are you thinking?

@jcoyne

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

I'm wondering, that if instead of passing controller: in, can we do ability: instead?

search_service_class.new(config: blacklight_config, user_params: search_state.to_h, ability: current_ability)
@cbeer

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

I guess, but Blacklight doesn't have a dependency on cancan, so it seems a little weird to me to bake that functionality in?

@jcoyne

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

@cbeer could we encapsulate the fields that get passed to the search builder into a method that can be overridden as needed?

@cbeer

This comment has been minimized.

Copy link
Member Author

commented Feb 27, 2019

Like bf13e21?

@cbeer cbeer changed the title Pass the controller through to the search service to restore function… Pass the context through to the search service to restore function… Feb 27, 2019

@jcoyne

This comment has been minimized.

Copy link
Member

commented Feb 27, 2019

Could we do this?

search_service_class.new(config: blacklight_config, user_params: search_state.to_h, **search_service_context)

@cbeer cbeer force-pushed the search-service-controller-context branch from 7887ed7 to f416117 Feb 27, 2019

cbeer added some commits Feb 25, 2019

Allow the implementor to supply context information to the search ser…
…vice, which restores functional compatibility with Blacklight 6 search builders.

@cbeer cbeer force-pushed the search-service-controller-context branch from f416117 to c35f439 Feb 27, 2019

@jcoyne

jcoyne approved these changes Feb 27, 2019

@jcoyne jcoyne merged commit 037633c into master Feb 27, 2019

2 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details

@jcoyne jcoyne deleted the search-service-controller-context branch Feb 27, 2019

@jrochkind jrochkind added this to the 7.1 milestone Apr 30, 2019

@jrochkind

This comment has been minimized.

Copy link
Member

commented May 1, 2019

Not sure if there are docs on how to use this feature somewhere already. So after figuring it out from a code read, I wrote a short (for me haha) blog post.

https://bibwild.wordpress.com/2019/04/30/blacklight-7-current_user-or-other-request-context-in-searchbuilder-solr-query-builder/

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.