Currently which searcher a block is associated with is not exported with the block itself. This is somewhat problematic as it seems there is then no way to set what the searcher should be once it has been imported into another site without manually going into the block_current_search table and adding a row.

My guess is that the searcher was separated out into another table as you may want to export a search block without necessarily tying it to any index in particular which makes a certain amount of sense but if that is so that relationship itself should also be exportable or at the very least configurable from within drupal.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

cpliakas’s picture

My guess is that the searcher was separated out into another table as you may want to export a search block without necessarily tying it to any index in particular

Yes, this is correct. The goal for all Facet API configurations (whether it is achieved or not) is to be able to port settings to whatever backend you are using.

Do you have any ideas as to the best way of doing this? The settings are tied to blocks, because my thinking is that the settings should be exported with block configurations separate from the current search block configuration. Let's talk about this further.

Thanks for bringing this up,
Chris

Dean Reilly’s picture

I think a good first step would be to allow the end user to change the associated searcher. As far as I can tell there's no reason for it to be uneditable.

I'd agree the settings should be exported separately as their own objects. The way the map array is generated will have to be rewritten a bit for this.

Looking forward, the current way blocks (be it facets, sorts or current searches) are associated with searches doesn't seem powerful enough. As far as I can tell if I have a page with two displays powered by the same index the blocks don't know which one to use as their source of information (I could be wrong about this). What would be nice is a system where I could target individual searches on a page.

Secondly, I'm not sure if there's any reason to force these blocks to be associated with only one index at a time. It would be nice to be able to configure one set of display settings the way I like and then associate it with as many indices (or specific search queries) as I have - each association generating a new block.

Finally, more of an after thought than anything else, if we start giving specific searches on a page unique ids so they can be identified like this we can process the searches at any stage and simply store the results in memory. This solves the problems where facet blocks are being called before the search is executed. In this scenario the facet block would cause the search to execute the results of which would then be stored until whatever code down the road needs them too.

I imagine some of this stuff falls outside the purview of the facet api module.

I'd be happy to help writing the code for some of this if I know it matches where you see the module heading.

cpliakas’s picture

Hi Dean.

Couple of thoughts...

I think a good first step would be to allow the end user to change the associated searcher.

You actually can change the searcher via the block settings. See http://drupal.org/files/current-search-07-block-visibility.jpg.

As far as I can tell if I have a page with two displays powered by the same index the blocks don't know which one to use as their source of information

I'm not 100% sure what you mean, but the backend in responsible for determining what a "searcher" is. In Apache Solr Search Integration it is an environment, so the same blocks can be used across pages. Search API sets searcher per page, or display, so that module approaches it a bit differently. Any further clarification would be helpful.

Secondly, I'm not sure if there's any reason to force these blocks to be associated with only one index at a time.

Well, what happens if two searchers are active on the same page? Which one does the block use? Especially with the Current Search Blocks module, it causes issues to know which data to pull. I am willing to discuss ideas about how to do this for 2.x, but it is not an easy problem to solve (for me anyways :-)).

if we start giving specific searches on a page unique ids so they can be identified like this we can process the searches at any stage and simply store the results in memory.

Agreed. Regardless of the technique there just needs to be a unified API to execute a search. This is definitely outside of the scope of Facet API (although Facet API could provide a bridge like it does with some othercontextaul information i.e. total results, search path, etc), but if core search we to handle only search pages, theming, and API functions for executing searches, then modules like Search API and Apache Solr Search Integration could build on a common framework and then do all of the complex stuff they do in between. That is something I am actually working on, although not publicly at this time since I am more or less playing around with ideas. I'm also not sure there is any community support for modifying core search in this way.

Regarding writing code, I would love to brainstorm some ideas with you and get you contributing. Code is always welcome given there is some consensus and direction of what is being done.

Thanks,
Chris

Dean Reilly’s picture

You actually can change the searcher via the block settings.

Sorry, I somehow managed to completely miss that. I had a quick look through but it also seems this isn't documented anywhere and I'm not sure how intuitive it is to have some settings in one place and the rest in another (although I can understand the reasoning). So maybe the writing of some help docs and messages to the user would be a good place for me to start.

I'm not 100% sure what you mean, but the backend in responsible for determining what a "searcher" is. In Apache Solr Search Integration it is an environment, so the same blocks can be used across pages. Search API sets searcher per page, or display, so that module approaches it a bit differently. Any further clarification would be helpful.

I was just wondering if I had a search page which draws results from a searcher, a solr core for instance, and on that page I also have a content reccomendation block that also uses that same solr core, I effectively have two search queries being exectued against the core. Does the current search block (or the facet blocks) have any mechanism of knowing which of the two searches I would want them to display information about?

Well, what happens if two searchers are active on the same page? Which one does the block use? Especially with the Current Search Blocks module, it causes issues to know which data to pull. I am willing to discuss ideas about how to do this for 2.x, but it is not an easy problem to solve (for me anyways :-)).

I guess this is a similar problem to what I was trying to describe in the last paragraph. The solution I envisaged was that there would still be multiple blocks but they would share a common set of configuration (as it seems that the current search blocks will for the most part stay the same regardless of what searcher they are using). However, having thought about it a bit more this might be an overengineered solution creating a fair bit more complexity in configuration and code and not saving very much time for the end user.
On the other hand this approach might make more sense when looked at from what's being exported. If I wanted to have the same current search block available for three different searchers I effectively have to export the same code three times under different names which doesn't feel very clean.
I'm not sure what to do about this one - what do you think?

Regardless of the technique there just needs to be a unified API to execute a search.

Yes, this would be superb, and it seems to be a niche that Search API is filling more and more.

Having thought about this idea of being able to identify searches uniquely, it occurs to me that this may not be something that would apply to just search data but really any data request (especially as Drupal's data sources become more and more diverse). When phrased like that it seems like resource caching might fall outside the purview of what Drupal should be handling.

In terms of implementation, sometihng like this would require the code responsibly for executing the data request to be decoupled so we would start to see a coding pattern similar to MVC. I haven't had a chance to look through the current search and facets code yet to see how this might be realised but that's something I'm going to try and do over the next few days.

Cheers,
Dean

cpliakas’s picture

I was just wondering if I had a search page which draws results from a searcher, a solr core for instance, and on that page I also have a content reccomendation block that also uses that same solr core, I effectively have two search queries being exectued against the core.

Actually, Facet API will consider this to be a single search because Apache Solr Search Integration shouldn't mark the More Like This query as an actual search since it isn't a facet-able query.

I effectively have to export the same code three times under different names which doesn't feel very clean.

Nope, not clean at all. I am wondering if there is a way to allow a single block to be active for multiple searchers. Again, the problem arises if these searchers are active on the same page. Then the system wouldn't know which one to use. Therefore I see it as a necessary evil to tie one block to one searcher. I could be convinced otherwise, though!

cpliakas’s picture

Status: Active » Closed (works as designed)

Although this solution is probably not as clean as we would like, I am going to respectfully mark this as "works as designed". Maybe we could pull out a couple of items here and post some feature requests to work on for the 7.x-2.x branch? I want to punt any code / UI cleanups to that branch so we can release a 1.0 that doesn't have larger pending changes on the horizon.

rerooting’s picture

Component: Custom Search Blocks » Code
Issue summary: View changes

I'm running into this issue as well - however now that we have configurable search results blocks - can't we just move that configuration over to that which has it's own exportables api implementation?

It would make this a lot cleaner! And easier to export. And if you are using panels, like me, you'd never have to visit the block configuration. Just sayin'!

Jon Pugh’s picture

Status: Closed (works as designed) » Active

Hey Chris,
This is a tricky situation. We are forced to use hook_install()/hook_update_N() to run db_insert() to set current search block config automatically.

The Features Extra block exporting ability doesn't pick up on this either.

I have a proposal for you: variables. If we used variables to save which block works for which searcher, we could export it easily with strongarm, and you would remove the need to maintain your own database table.

Is there any reason you can see that using a simple variable wouldn't work for this?

Something like current_search_searcher_DELTA: search_api@index_id, perhaps?

Just a thought!

Thanks,
Jon

peri22’s picture

Status: Active » Needs review
FileSize
5.1 KB

Hi,

I put this table to a variable as Jon Pugh suggested, so now it is exportable and cleaner a bit.
I made an update hook too.
Please check it.

Thanks,
Peri22

Status: Needs review » Needs work

The last submitted patch, 9: facetapi-cur-search-block-not-exported-1469002.patch, failed testing.

peri22’s picture

Status: Needs work » Needs review
FileSize
4.74 KB

sorry I forgot a dpm() in my code :)

Status: Needs review » Needs work

The last submitted patch, 11: facetapi-cur-search-block-not-exported-1469002_2.patch, failed testing.

peri22’s picture

Status: Needs work » Needs review
FileSize
4.94 KB

Status: Needs review » Needs work

The last submitted patch, 13: facetapi-cur-search-block-not-exported-1469002_3.patch, failed testing.

peri22’s picture

Version: 7.x-1.0-rc4 » 7.x-1.5
Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 13: facetapi-cur-search-block-not-exported-1469002_3.patch, failed testing.

rodrigoaguilera’s picture

First you should make sure that the patch applies against the last dev version once is commited you can make parches for older versions

Status: Needs work » Needs review

Status: Needs review » Needs work

The last submitted patch, 13: facetapi-cur-search-block-not-exported-1469002_3.patch, failed testing.

The last submitted patch, 9: facetapi-cur-search-block-not-exported-1469002.patch, failed testing.

peri22’s picture

Status: Needs work » Needs review

sorry, the version number was the problem.
The second patch is good: https://www.drupal.org/node/1469002#comment-9911313

rodrigoaguilera’s picture

If that is the good patch can you make it apply against latest dev so the tests get green?

rodrigoaguilera’s picture

Version: 7.x-1.5 » 7.x-1.x-dev
Status: Needs review » Reviewed & tested by the community

Ok
Sorry I misinterpreted which one was the second patch, #11 is the good one

I reviewed the code and everything looks good.

I did manual testing including the update function and it works as expected.

Great work!

chrisolof’s picture

So I've gone through this thread here - tried the patch in #11 - and I'm not convinced breaking that setting into a variable (or variables) is the right approach.

I think a better approach is to add the "searcher" setting into the ctools exportable. That immediately solves the export issue.

cpliakas mentions the root of the architectural decision that causes this issue was the desire to keep current search settings distinct from any particular search api backend or index, but then goes on to make a strong case for tying the two. It seems the one-to-one current search block to searcher rule keeps things in order when more than one search is present on a particular page.

I get the idea that the searcher setting is a site-specific item that has little to do with the meat of the current search block. But if you look at other major ctools-exportable things out there they put the same sort of stuff right into the export. Take views for instance. In your view you can define a menu item. To do this you have to choose a menu for the item to live in - which is totally site-specific and has nothing really to do with the meat of the view. If I export the view and import it into another site, the menu for my menu item may not exist. But no biggie - if it doesn't exist the obsolete menu item setting will drop out on the view edit screen before I can even save it. And the same goes for current search blocks - if I import one into a site that has different indexes it's no biggie - the obsolete searcher setting drops out and I'm forced to make a valid searcher choice on the edit screen prior to saving.

Attached is a patch that takes the ctools-exportable approach for the "searcher" setting. It treats a current search block's "searcher" as just another setting and as such exports it with the rest. It also moves the searcher setting back to the page with the rest of the current search block's exportable settings.

berliner’s picture

Status: Reviewed & tested by the community » Needs review

After looking over this discussion, I would favor the approach in #27. I have tested the patch and it's working as advertised.
Settings export using features works immediately and solves the original problem.

I think the approach outlined in #27 is cleaner than the previous one, no need for additional variables, and it follows the same logic as a lot of other ctools exportables.
Also, having the searcher setting directly on the current search block configuration page is a lot more intuitive. I had found myself looking for how to change that in the UI for a little while too.

Setting this back to "Needs review" because there are 2 working patches now that use fundamentally different approaches, so some more feedback would be great.

jantoine’s picture

I too wasted much time trying to figure out how to associate a searcher with a current search block. I think it makes more sense to keep all the configuration together. After reading through the thread, I also agree that #27 is the better approach (and it even comes with an update hook)! I think this is RTBC, but I'll hold off for a few more reviews.

jmdeleon’s picture

I used the patch in #27 to get current facets blocks working correctly in a multi-search-core setup using the Sarnia module. I hope this can be considered a positive review in favor of RTBC! :)

berliner’s picture

Status: Needs review » Reviewed & tested by the community

That's 3 reviews in 9 months, all of them positive.

drupalfan2’s picture

Is this patch still necessary for latest Drupal 7 version?

joseph.olstad’s picture

patch still applies cleanly.

 ╰❯ $ patch -p1 --dry-run < facetapi-exportable-current-search-searcher-1469002-0.patch 
checking file contrib/current_search/current_search.block.inc
checking file contrib/current_search/current_search.current_search.inc
checking file contrib/current_search/current_search.install
checking file contrib/current_search/current_search.js
checking file contrib/current_search/plugins/export_ui/current_search_export_ui.class.php
Hunk #1 succeeded at 196 (offset 2 lines).
Hunk #2 succeeded at 528 (offset 2 lines).
Hunk #3 succeeded at 615 (offset 2 lines).
checking file tests/facetapi_test.current_search.inc

I just queued up php 7.4 , 7.3 tests

joseph.olstad’s picture

passes php 7.4 tests

might put this in next release, if/whenever that is.

joseph.olstad’s picture

ok, the above patch (#27) still applies cleanly. We are about to tag a maintenance release for PHP 8.1 and PHP 8.2 support fixes, I will commit this AFTER tagging 7.x-1.10 shortly.

So this will sit in 7.x-1.x-dev for a while.

So in best scenario this will arrive in a tagged release for 7.x-1.11 if all goes well sometime after 7.x-1.10 gets some installs.

joseph.olstad’s picture

Actually, there's still some PHP 8.2 deprecated fixes needed. I'll postpone this commit on those.

joseph.olstad’s picture

Status: Reviewed & tested by the community » Needs review

I fixed the known PHP 8.2 issues, re-triggering tests above
#3401917: Dynamic properties are deprecated in PHP 8.2

joseph.olstad’s picture

Just published 7.x-1.10

If I haven't pushed #27 in by April 1st 2024, please remind me and I will push #27 in.