disabled types should be removed from search index

I have also just come across this method of controlling what gets indexed.
Here is my take on this. The SQL only retrieves nodes that have been indexed.
I don’t think the watchdog message is necessary, but I do think this should be managed from a setting, it would just give admins more control.

I also don’t think the search_config module should wait for core to implement this, I consider this to be a critical feature.

/**
 * Implementation of hook_update_index()
 */
function search_config_update_index() {
  // This hook is only called when the search module is enabled
  // The function_exists check can be removed, but there is no harm in leaving it in
  if (function_exists('search_wipe')) {
    $remove = variable_get('search_config_disable_type', array());
    foreach ($remove as $type => $value) {
      if ($remove[$type]) {
        $result = db_query("SELECT n.nid FROM {node} n INNER JOIN {search_dataset} s ON n.nid=s.sid WHERE n.type = '%s'", $type);
        while ($nid = db_result($result)) {
          search_wipe($nid, 'node');
        }
      }
    }
  }
}

I'm about to implement this and I'm looking for some feedback on where exactly I should but the setting.

1. Have a checkbox on the search settings page to automatically remove content types from the search index once they are removed from the display.
2. Make the settings independent of the search and place in on each content type page.

1 is easier but can cause some issues where an admin only wants to remove the node type from the display but not the search index. 2 is more flexible but means that if you do want to remove a content type from the search index and search form at the same time it takes one extra step.

I'm leaning towards 2 at the moment. Any preference?

I vote for option 2, if possible with a tick box under the index node settings called something like "Same as Search Form Node Types". That could reduce the extra admin you were referring to.

To avoid confusion the 2 different node type settings need to be clearly labeled. My suggestions are “Search form Node Types” and “Search index Node types”.

There is one thing we need to take into consideration here. If a node type is set to not be indexed then it should also be removed from the form.

There is one thing we need to take into consideration here. If a node type is set to not be indexed then it should also be removed from the form.

That was the idea. Will see how it turns out.

...settings called something like "Same as Search Form Node Types...".

I'm not sure where you are referring to here.

Here is a patch of what I was thinking.

This looks good, thanks. I'll have more in depth look when I get home. Which version of the module is this patch against?

5.x-1.2

NaX,

I've committed your version of the patch in http://drupal.org/cvs?commit=81182. Thanks a lot. I'm sure there is more to be done. The approach I was taking was different (altering the content type form) but either way works for now.

I really would like some testing done on this to see if any issues pop up. The installation of Drupal I have here is really minimal (still recovering from a HD crash) so no content to speak of for testing. You would be surprised to know that I rarely use this module so it's good to see other people using it and making contributions :).

The version I'm developing against is the 5.x-1.x-dev version, there should be a package soon, if not you can use the CVS version. If after testing everything is OK I'll update the documentation and make a new release.

Thanks again.

I am currently testing this patch on a site that has a lot of nodes.
I first tested the patch by running the cron manually with the devel module showing redirects and SQL queries.
It all seems to be fine, but I will keep monitoring things.

The one thing that mariuss said I don’t understand.

Although far from ideal (performance issues and skewed word count)

What is the problem with skewed word count? The performance issue is not an issue anymore with the modified SQL.