Great work on this!

Here are some questions I have and issues I spotted, though:

  • Why does search_api_extended_processors_views_plugins() call search_api_views_views_plugins() and use its return value as a base? From what I can see, this will just return the cache plugin again that Search API already defines. I think, just dropping the first three statements in that function would just work as well.
  • search_api_admin_element_compare() is defined in search_api.admin.inc, so you need to load that file to use that function.
  • I guess the install.inc file could be removed? The information in api.php also seems to be outdated (the normal processors are used, you're not using your own hooks anymore). Same for the processor class and interface definitions.
  • With the current code, it is apparently only possible to add additional processors to a query, not to remove processors that would otherwise be used. I think with a few small changes both should be possible:
    • Make the default for the extended_processors option NULL (and maybe rename to something like overridden processors).
    • In the query's preExecute() and postExecute() methods, don't call the index's methods if the option is set and instead use the processors there.
    • In the form, also provide a way to switch overriding on/off explicitly, defaulting to "off" of course. (Should probably be in the form functions in admin.inc if possible, not in the Views query class.

Other than that, it looks great, and already seems to work brilliantly. Thanks a lot!

CommentFileSizeAuthor
#3 2581151-3--search_id.patch630 bytesdrunken monkey
Support from Acquia helps fund testing for Drupal Acquia logo

Comments

drunken monkey created an issue. See original summary.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
Version: » 7.x-1.x-dev
Status: Active » Needs review

Code cleanup and "Override base Processors": done in latest 7.x-1.0-beta3 release.

drunken monkey’s picture

drunken monkey
he/him
Primary language German
Location Vienna, Austria
CreditAttribution: drunken monkey as a volunteer commented
FileSize
2581151-3--search_id.patch630 bytes

Didn't have time to look at this again, yet, but noticed (since I still have the module installed on my test site) that this breaks the search IDs of Views searches. You can check this yourself, it has to do with how the Search API Views query class checks for whether to apply its own search ID or not.
Patch to fix this attached. Not ideal or very clean, but at least it works.

thePanz’s picture

thePanz CreditAttribution: thePanz at Liip for FREITAG lab. AG commented
Status: Needs review » Closed (cannot reproduce)

Hi @DrunkenMonkey: I already fixed such issue.
Could you checkout the latest 7.x-1.x branch?

This is the actual code of such function

  /**
   * {@inheritdoc}
   */
  public function __construct(SearchApiIndex $index, array $options = array()) {
    parent::__construct($index, $options);
    $this->options = array_merge($options, array(
      // Override the 'search id' parameter, since our parent set it to
      // "SearchApiQuery": this override is required for search_api_facetapi to
      // work correctly.
      'search id' => __CLASS__,
      // Add the extended_processors settings.
      'extended_processors' => array(),
      'extended_processors_override' => FALSE,
    ));
  }