Ability to override Search API page with Panels variant

Just pointing out that pontus_nilsson's search_api_page_panels works out of the box and does exactly what he describes.
It would be *great* to see this shipped as a submodule of search_api_page.

Thanks.
J.

Thanks for posting this! Sounds like something that could be useful, so it's great you're publishing your code.

Adding it as a submodule to this module would be an idea, yes. However, as I don't know much about Panels and therefore don't really understand the code, I hesitate to do that. I don't want to be responsible for maintaining code I don't understand. (Probably, the submodule would be unnecessary anyways – it seems this would only slightly change our page callback and add a single hook and the plugins, so it could also be included in the module directly without adding significant complexity.)
So maybe Pontus should just make the sandbox a full project? Or if he would agree to keep maintaining the code, we could also add it to this module directly, if you say the code works well and think that this would make sense.

I figured a submodule would be best in order not to introduce an unnecessary dependency of search_api_pages on panels, but if there are ways to avoid that: great.

I totally get your point about not wanting to maintain panels code if that's not your thing. But I also feel that this addition would be a natural fit, *and* having it as yet-another-module feels kind of sad.

I'm not a deep panels expert but I'd be willing to help integrate and maintain this, if you and pontus_nilsson agree...

J.

There is one change I would like to introduce to search_api_page_view so that the settings can for the page can be passed to the function instead of read from the page.

Yes, I already saw that in your sandbox, and that's totally fine. And also one large Pro for not making this a new module – copying the whole function code for just this change is a bit overkill, but otherwise of course necessary.

So, yes, would be great if you could make a patch for this! Thanks a lot!
And as you say, this of course won't introduce any dependency. It will just work if Panels is there, and be ignored otherwise.

Like this? I've not tested it yet though.

This improved version fixes a typo on the search_api_page_ctools_plugin_directory() function, and adds a new search box pane.

FYI the patch (and sandbox) adds a pane / CTools content type for the search results from a given search page definition - I'm not sure of the intended use case but it doesn't solve the original request.

I moved the search box plugin into #2425179: CTools plugin for search box, so this patch no longer has that. That said, there are now two copies of search_api_page_ctools_plugin_directory(), so depending upon which one is committed first the other will need to be rerolled.

Thanks a lot for the initiative, Damien!
Could someone please test and verify this is correctly working?

Also, two little remarks:

1. +++ b/plugins/content_types/search_api_page_result.inc
   @@ -0,0 +1,157 @@
   +
   +if (module_exists('search_api_page')) {
   +  /**
   

   +++ b/plugins/tasks/search_api_page.inc
   @@ -0,0 +1,221 @@
   +  if (!module_exists('search_api_page')) {
   +    return;
   +  }
   

   I'm not completely sure, but I guess this isn't necessary anymore when it's directly included in this module?

2. +++ b/plugins/tasks/search_api_page.inc
   @@ -0,0 +1,221 @@
   +  array_unshift($args, $type);
   +
   +  return call_user_func_array($function, $args);
   

   If the keys contains slashes, wouldn't that then pass the part after the first slash as $overridden_options? I guess we should here manually pass $type, $keys?

3. +++ b/search_api_page.pages.inc
   @@ -11,13 +11,17 @@
   +  // Merge in overriden options.
   +  $page->options = array_merge($page->options, $override_options);
   

   Isn't it a bit unnecessarily "dangerous" to directly set the overridden options on the page object? If it is then saved somewhere later in the page request, the overridden options would be saved, too.
   I can see that too many changes in other parts would be necessary to just use a new variable, but we should at least restore the original option values after the search (even though it's tricky, with the rendering only happening later).

The last one of those is fixed in the attached, along with some other minor problems. For the former, I'd first need confirmation whether I'm right there.
Fixing the comment standards in the patch required a bit of guesswork, though, as I have for the most part no idea what's going on, or what the correct phrasing is. It would be great if you, Damien, or something else who understands the code, could add appropriate @param and @return tags.

And finally, could someone please test (and maybe review)?

I removed the two module_exists() calls because they shouldn't be necessary, wrapped some variable checks in !empty(), and removed the unnecessary "return NULL;" that was added.

removed the unnecessary "return NULL;" that was added.

I'm of course aware that PHP will return NULL by default, but I find it helpful to try and have uniform return points in a function/method: if a value is returned in some places, I prefer explicitly returning NULL in the others.

Also, what about my second comment in #11? Wouldn't $function($type, $keys) make more sense?

Updated to per #13.

The "search_api_page_result" pane tried to call non-existing search_api_page_panels_view function and the entire Task plugin had wrong function names (copy paste errors from the sandbox I guess). - I have provided an interdiff and a new patch. But it's actually working fine for me, now :)

/ Kallehauge

Good to hear it basically works, thanks for all the feedback and the work on the patch!
Here is an updated patch with a few style fixes and the suggested check for the menu item existence. Would have been great to also complete the doc blocks (e.g., docs for the $type parameter), but since I don't really know what that is/does, I couldn't really fix it. Not too bad, though, the patch would be RTBC for me.

Could someone just check if it still works (with the latest HEAD – just committed #2425179: CTools plugin for search box which "interferes" a bit with this one)? Then I can finally commit it.

@drunken monkey: You forgot to add the patch ;-)

Ah, §$&#!
Sorry, and thanks for pointing it out!

#19 works well with 7.x-1.2
Thanks a lot!

Excellent, great to hear. Thanks for testing!
Committed.
Thanks again to everyone who worked on this!

Woot! One less patch to add to all of my installs :)

I applied the patch, but now I have a problem.
Where can I add the search_api_page to my panel.
I could not find an object in the "whole panel thing".
Obviously, I understood something wrong.
I hint, where to find my search_api_page would be great.

Greetings
Berthold

@BLausch: Try downloading the current -dev release and clearing the caches, then it should appear.

@DamienMcKenna
You are absolutely right. It works.
Thank You very much
Berthold Lausch

Is this patch still relevant?

Thanks!