Problem/Motivation

It is common when building Drupal sites to create "small" content types that are used as pieces in pages, and that are never meant to be displayed on their own pages. It would thus be desirable to excluded them from Search.

Proposed resolution

To exclude node types from search results, site builders can make a small module with a query alter. They can look for queries with the 'search_node' query tag, and implement hook_query_TAG_alter() to alter the query.

To exclude node types from being added to the search index at all, it would be useful for the NodeSearch plugin to be configurable this way.

Remaining tasks

Make the NodeSearch plugin configurable so that it can exclude certain node types from the index.

User interface changes

New config option for NodeSearch will be added.

API changes

None.

Original report by asimov

The Drupal search module currently includes all node types in its search results. A feature request to add capabilities to exclude some node types. Please see http://drupal.org/node/82348#comment-161699 for more info.

Comments

pluess’s picture

Title:Feature to exclude node types from search results» There's more than just excluding node types

I think there's more than just excluding node types.

The real work gets donne in the node.module at node_update_index(). It simply takes every node, renders it and indexes the content. Via hook_nodeapi modules have the chance to provide extra information.

I see the following possibilities:

  1. Add a possibility to exclude certain node types from beeing indexed to the search.module and make the node.module skipping nodes of this types.
  2. Add a hook_index_view. The node can return whatever it want's to get indexed (maybe NULL). If isnt' availabe hook_view is called (as it is to day). This requires node_update_index in node.module to be patched.

Of course the two possibilities can be combined.

What do you think?

asimov’s picture

I agree it's probably best to have the option to prevent certain nodes from being indexed in the first place rather than just filtering them, although a combination is better since it also allows for exclusion of results that have previously been indexed.

I imagine the indexing prevention patch would apply to node_update_index() in the node.module, and the addition of filtering capabilities would need to be made within the search module inside search_index(), with a check for an allowed $type against the new hook_index_view that you describe?

yched’s picture

Title:There's more than just excluding node types» Exclude node types from search index
Version:4.7.5» 6.x-dev
Status:Active» Needs review

(New features go only in the dev release.)

Excluding content types is an important feature. Now that content types are so easy to add, and esp. with modules such as 'node as block', we must have a way to prevent 'internal', 'administrative' or 'cosmetic' content types from being indexed.

Attached patch presents a select box on the search settings page letting the admin choose the indexed content types.
Note that indexing is opt-out (new content types are indexed by default), so we store a list of excluded content types. However, having the user select the content types he wants to exclude makes a kind of convoluted brainer ('negative choice'), so the UI is opt-in style ('positive choice').

@plueless : a finer grained ('per field') indexing would be sweet as well, but let's take small steps :-). What that would require actually is a 'search index' rendering style (see http://drupal.org/node/144608 and http://www.lullabot.com/blog/trouble_nodes) - VERY interesting, but probably not for now...

yched’s picture

StatusFileSize
new7.56 KB

aaaand the patch.

yched’s picture

http://drupal.org/node/63028 was marked a duplicate

yched’s picture

asimov’s picture

I applied the patch to the latest version of HEAD, and tested story content in a story node and page content in a page node by selectively setting the search index to index one or the other. I also tried searching for the content in the excluded node type. It works great, thanks.

Gábor Hojtsy’s picture

The feature looks sensible...

- non_indexed_content_types should be 'search_excluded_content_types' or 'search_excluded_node_types', see the variable name prefix, and shorter naming
- content_indexing_settings should be shorter IMHO, why do we need the 'settings' there? this is a settings form, we know
- why do you need to add the validation with altering?
- the help text on the setting should note that existing search index data is wiped for excluded types

yched’s picture

StatusFileSize
new7.99 KB

Thanks for the review, Gabor.

non_indexed_content_types should be 'search_excluded_content_types' or 'search_excluded_node_types', see the variable name prefix, and shorter naming

True :-) done.

content_indexing_settings should be shorter IMHO, why do we need the 'settings' there? this is a settings form, we know

That was to stay inline with search_module's $form['indexing_settings'] fieldset. But sure, why not.

why do you need to add the validation with altering?
because the 'per module settings' forms are array_merged (search.module line 257), not array_merge_recursive, so if I add the #validate directly in node_search($op = 'admin'), either the node specific or search generic #validate gets overwritten.

the help text on the setting should note that existing search index data is wiped for excluded types
True - done.

Updated patch attached.

Gábor Hojtsy’s picture

validation: why not fix the original problem then? or add a field #validate key to validate that value?

yched’s picture

Actually, I think the whole hook_search('admin') op should be ditched, and handled by hook_form_alter (I think it's the general trend, isn't it ?)
The advanced settings on the search form are added that way, so what's the point ?

yched’s picture

StatusFileSize
new11.47 KB

Updated patch does the above (move hook_search('admin') to _form_alter), plus changes the selection widget for indexed content types from multiple selectbox to checkboxes (less error prone)

Please also note that the 'reset to defaults' button is problematic for this form, does not play well with the code used to wipe the search index - this is already the case without this patch, though...

Gábor Hojtsy’s picture

After you reset to defaults, the search index should be gradually built up again, no?

yched’s picture

when you hit 'reset to defaults' without changing any of the values in the form, the current search_admin_settings_validate() and my new node_search_admin_settings_validate() functions do not 'catch' the fact that the values _will_ change, and thus do not take the relevant action (search_wipe() ).

Not sure of the best way to fix that yet. As I stated above, this bug already exists in D5 and is not strictly related to this patch.

yched’s picture

StatusFileSize
new14.05 KB

Updated patch, fixes the issues with 'Reset to defaults' by :
- storing the old values of the relevant variables in validate
- letting system_theme_settings_submit() actually update the variables (be it new form values or back to defaults)
- only then take action in our own submit handlers, based on the comparison between the old values and the newly updated ones.

This patch is now 2-in-1 : new 'exclude content types form indexing' feature + fix existing problem with 'Back to defaults'. I can roll two different patches if you prefer.

yched’s picture

StatusFileSize
new14.3 KB

Small fix : $form_state['storage'] instead of $form_state['store']

yched’s picture

StatusFileSize
new13.91 KB

Rerolled after deletion API patch.
Just retested, everything works OK.

pwolanin’s picture

subscribing - patch still applies with offset.

Bevan’s picture

Cool! Just What I'm looking for (subscribing)

pwolanin’s picture

Status:Needs review» Needs work

Going through this in detail makes me feel as though this should be a property of each node type (i.e. a table column), perhaps, rather than stuffed into an array setting? In which case, each node type's add/edit form could have a checkbox, rather than having one large set of checkboxes on the search admin form.

In any case, the patch needs to add code to node_node_type() to handle changes in node type names.

yched’s picture

Status:Needs work» Needs review
StatusFileSize
new15.61 KB

One checkbox on each content type's settings page vs. checkboxes on search admin settings :

I guess this could be discussed. Changing the 'excluded types' settings might require rebuilding the index, so I felt it more adequate to leave this grouped with the other settings that wight do so (currently only on the search settings form).

New patch - rerolled against latest D6, and reacts to hook_node_types('update')

phillamb168’s picture

Hey there,

I've been thinking it'd be useful to also include a per-node opt-out option for searching. Have it be a checkbox flag similar to promote to front, and would (on cron) exclude any new/existing keywords under the node ID.

Would this be useful for anyone?

inforeto’s picture

subscribing

pwolanin’s picture

Version:6.x-dev» 5.x-dev

I think this is now for 7.x, but can be implemented to a reasonable degree in contrib in 6.x thanks to this patch: http://drupal.org/node/152493

yched’s picture

Version:5.x-dev» 7.x-dev

True - but I think your finger slipped :-)

asimov’s picture

A per-node-opt-out would be very handy, especially where you need to have the node in published status for certain users to access directly, but don't want others to find it when searching.

NancyDru’s picture

I like this option being implemented. Is there some way there could also be a hook_search_filter such that I could control whether or not to show my content type?

My case: I have already implemented hook_update_index to exclude my content type, but I am not totally happy with that choice. I'd like certain users to be able to search and see this content (I do implement a permission to "access xyz"). So I'd like to set up a "filter" to check that access after the search has selected items, but before they are shown, so that I can exclude the results for users who don't have the correct permission.

pwolanin’s picture

for 6.x you can use the node build_mode to accomplish this in a contrib module. In 5.x, I'm not sure it's possible.

NancyDru’s picture

Oh, you didn't have to tell me that - now I HAVE to do it!

jschneider’s picture

So is it my understanding this works for Drupal 6 and 7? and does not work on 5.1? If this is the case sorry for asking here but where does one obtain a copy of 6?

Thanks in advance!

yched’s picture

You do not want drupal 6 - drupal 6 is under development, not ready for public use.

You *might* want to adapt this patch to the drupal 5 branch (should be quite straightforward), *but* then you'll be running a forked version of Drupal core, which is really not advised...

WorldFallz’s picture

Does that mean this can't be implemented in 5.x as a contrib? Let me know so I don't waste my time trying, lol. Thanks.

NancyDru’s picture

Node_build_mode probably could not be back-ported - certainly not easily.

That, however, doesn't mean you can't do this in 5.x. I added additional checks to hook_view (above and beyond menu access checks) to determine if the current user is allowed to see the node. It may not be "pretty" but it seems to work, but I want to do some more testing before I put this in my contributed module.

WorldFallz’s picture

Actually-- seems someone has already done it!

Check out: http://drupal.org/project/search_block. So far from my testing it's correctly blocking items from being searched. It just doesn't seem to be changing the advanced search form which should be easily doable with an override. From what I can tell taking a quick look at the code, it's not attempting to do it. If I get it figured out for my site, i'll post a patch for it.

Bevan’s picture

I've been using Views Fast Search very successfully to reach the same end.

WorldFallz’s picture

Wow-- it never ceases to amaze me how much time i waste on things 1) either already implemented in a module or 2) already solved in a previous issue or forum thread. I"m not a lightweight searcher-- i typically spend hours and hours researching (both drupal.org and google) something before attempting to fix it myself. For whatever reason, I'm often not successful in finding the gems out there.

For anyone else needing this functionality now for the 5.x code the search_config module does ALL of this in a single module-- fixes the index and the adv search form. Brilliant.

Hopefully this will save someone else from wasting time on something that's already been done beautifully.

NancyDru’s picture

Those both look like promising solutions for most people. However my use case is that I want these nodes to be searchable, but only by those who are authorized to do so. Since the indexing is not dynamic, it falls back to my view code to disallow it. Excluding on a user-by-user basis from the advanced search form is easy. The problem is that I'd also like the search results to not show anything either.

WorldFallz’s picture

ahhh... interesting use case. Very valuable functionality too I would think. So in your case if a user doesn't have access to read those content types, they also shouldn't see them in search results. That is a bit different then what these modules do because in your case all the content types will still need to be indexed normally (for those that can search) but you'll have to throttle access control at the search or view stage based on permissions. What about a custom search module that uses an SQL WHERE clause to limit the results returned?

NancyDru’s picture

Unfortunately "type" for search is just "node" for everything, so this would require a lot of JOINing to get the real type.

Right now (at least until D6), I think the most "efficient" is to use hook_view to deny access. I'm not crazy about that because too much stuff might still show up in the search results list.

Leeteq’s picture

What is the status of these "needs" now that D6 is out ?
How to best achieve it under D6?
(I reckon D6 does not cater for all these variants out-of-the-box.)

How about role-based display? Does D6 core let ACL filter what is available to search?

NancyDru’s picture

The needs are still there but I have not had a chance to see what D6 brings to the table. Unfortunately, node.module's (IMHO) incorrect access checking doesn't help any.

davidtrainer’s picture

subscribing

moshe weitzman’s picture

anyone up for a reroll?

challa.kamal’s picture

Hey nice solution i think but not able to implement in my project . Please could you send the modified files rather than patch files. becoz iam not able to implement. please

Thanks inadvance

Bevan’s picture

floretan’s picture

StatusFileSize
new11.65 KB

Updated the patch for Drupal 7.

Todo: re-index nodes after change in excluded content type.

yched’s picture

Thx for re-rolling, flobruit.

IIRC, the "Todo: re-index nodes after change in excluded content type" part is already taken care of in node_search_admin_settings_submit(). Logic is : "Delete the indexed entries for newly opted-out content types, rebuild the whole index if there are newly opted-in content types"

robertDouglass’s picture

Rowanw’s picture

Status:Needs review» Needs work
StatusFileSize
new11.65 KB
new16.67 KB

I've created a patch with some better wording (in my opinion) for the settings form, screenshot attached.

Also, the submit buttons on the Search settings page come before the last two fieldsets, but I'm not sure how to address this.

spiffyd’s picture

1 hunk of the patch didn't work with Drupal 6.4. Another update?

canen’s picture

Sub.

WorldFallz’s picture

I took the patch in #39 for a test drive on a fresh checkout of head and received the following error when navigating to the search settings page:

Fatal error: Uncaught exception 'PDOException' with message 'SELECT COUNT(*) FROM {node} n WHERE n.type NOT IN(?) AND n.status = 1 - Array ( ) SQLSTATE[HY093]: Invalid parameter number: no parameters were bound' in C:\wamp\www\drupal7\drupal\includes\database\database.inc:365 Stack trace: #0 C:\wamp\www\drupal7\drupal\includes\database\database.inc(1154): DatabaseConnection->query('SELECT COUNT(*)...', Array, Array) #1 C:\wamp\www\drupal7\drupal\modules\node\node.module(1237): db_query('SELECT COUNT(*)...') #2 [internal function]: node_search('status') #3 C:\wamp\www\drupal7\drupal\includes\module.inc(445): call_user_func_array('node_search', Array) #4 C:\wamp\www\drupal7\drupal\modules\search\search.admin.inc(42): module_invoke('node', 'search', 'status') #5 [internal function]: search_admin_settings(Array) #6 C:\wamp\www\drupal7\drupal\includes\form.inc(360): call_user_func_array('search_admin_se...', Array) #7 [internal function]: drupal_retrieve_form('search_admin_se...', Array) #8 C:\wamp\www\drupal7\drupal\include in C:\wamp\www\drupal7\drupal\includes\database\database.inc on line 365

I've no idea what that means or where to begin troubleshooting.

catch’s picture

It means the patch needs updating for the new database layer which got in last night.

momendo’s picture

subscribe

deviantintegral’s picture

I'm the maintainer of Restricted Search, and am subscribing as I'd obviously not like to have to maintain a module for something that could be in core instead :). We're in the process of launching the site I wrote this for, but hopefully I'll be able to work on this patch once it's launched.

--Andrew

dreed47’s picture

Not sure of the status on this one but subscribing and a big +1 from me. I think this feature is critical in core. I've been trying to get the Restricted Search module to work for me on a Drupal 6.5 site and I've had to resort to hacking core node module (not at all happy about that).

vendeka’s picture

subscribing

momendo’s picture

FYI, the Search Config module has a D6 patch that already does node exclusion and other features. Maybe there is some overlap? It seems further along than Search Restrict and Restricted Search modules.

http://drupal.org/node/294696

Bevan’s picture

#286263: Make search indexing smart to handle nodes wth php redirects fixes one of the primary issues this issue was created to fix. I'm inclined to mark this as druplicate given that #294696: Port Drupal 6 version of Search Config and #286263: Make search indexing smart to handle nodes wth php redirects have everything covered.

yched’s picture

Bevan : this thread is about having the feature in Drupal core, where IMO it belongs.
Now, whether a core patch should be based on the patch here or from Search Config / Search Restrict / Restricted Search contrib modules can probably be discussed, I haven't really looked at those modules.
I don't intend to do more work on the issue myself, though :-).

Bevan’s picture

I didn't realize that was a contrib module.

naxoc’s picture

StatusFileSize
new2.63 KB
new36.37 KB

I have needed the possibility a few times now to exclude node types from search for some users, but still keep all content types in the search index so privileged users can do a full search.

This patch needs some work, but is a very simple shot at having exclusion of content types chosen in the search settings page (see the screenshot).

in my opinion we might need both the possibility to exclude node types entirely from the search index and the possibility to exclude node types from some searches. Explaining the difference to user that has to set this up is probably not going to be an easy job though...

akahn’s picture

Category:feature» task

Naxoc, your patch has to do with filtering search results, not with controlling search indexing. You should file a separate issue with your patch.

talino’s picture

I wanted to accomplish more or less the same thing and did something very simple which works just fine, although it doesn't remove any checkboxes in advanced search nor does it prevent content types from being indexed. I simply wanted to avoid confusing users with unneeded search results, and enclosed everything in my search-result.tpl.php file with this:

<?php if ($info_split['type'] != 'Profil') : // Hide Profil content-type (human-readable name) from search results ?>
  // The rest of the template
<?php endif; ?>

Hackish and not perfect, but it gets the job done without needing to add another module.

catch’s picture

Version:7.x-dev» 8.x-dev

Moving to D8.

jhodgdon’s picture

Category:task» feature
mcrittenden’s picture

Sub.

chezwel’s picture

subscribe

Observer123’s picture

sub

Bevan’s picture

Status:Needs work» Closed (won't fix)

This feature request is well over three years old, and a contrib module now exists which implements the feature: http://drupal.org/project/search_config.

I think efforts would be better spent on maturing that module, and then potentially moving it to core if it proves popular enough.

jhodgdon’s picture

Status:Closed (won't fix)» Active

Please leave this open. It is under consideration for Drupal 8.

jhodgdon’s picture

And by the way, http://drupal.org/project/search_by_page also has this functionality.

DanielR’s picture

It seems that's incorrect. I just checked search_config-6.x-1.6, and the README.txt file explains this module no longer attempts to prevent any nodes from being indexed, it just hides them from search results.

muhleder’s picture

Looking through the code of search_config, it intercepts the search module as indexing occurs and prevents the unwanted node types from being indexed, as opposed to letting them be indexed and then later removing them from the index.

Cyberwolf’s picture

Subscribing.

jbomb’s picture

The "Search Config" module was based on the following article: Hiding content from Drupal's search system.

It basically uses hook_db_rewrite_sql to append an exclusionary "where" clause to the search SQL generated in the node module's hook_search implementation.

kotnik’s picture

Subscribing

David Sparks’s picture

This may not the place for this, but if anyone's looking for just a small snippet of code to achieve what the issue's title suggests (rather than installing the Search by Page module - nice though it is):

<?php
 
function [my_module]_node_view($node, $view_mode, $langcode) {
  if (
$view_mode === 'search_index') {
   
$excluded_types = array(['my_type']);
    if (
in_array($node->type, $excluded_types)) {
     
$node->title = '';
     
$node->content = array();
    }
  }
}
?>

If the content's already been added to the {search_dataset} then you need to set 'reindex' <> 0 to force it to be reindexed. You could do this by calling node_search_reset() or by manually updating the DB.

ngstigator’s picture

subscribe

Gabriel Radic’s picture

For Drupal 6, none of the solutions I found worked for me.

I ended up with hacking node.module, which sucks, but it had to be done.

<?php
/**
 * Implementation of hook_update_index().
 */
function node_update_index() {
 
$limit = (int)variable_get('search_cron_limit', 100);

 
// Store the maximum possible comments per thread (used for ranking by reply count)
 
variable_set('node_cron_comments_scale', 1.0 / max(1, db_result(db_query('SELECT MAX(comment_count) FROM {node_comment_statistics}'))));
 
variable_set('node_cron_views_scale', 1.0 / max(1, db_result(db_query('SELECT MAX(totalcount) FROM {node_counter}'))));

 
// HACK: This was hacked to exclude sos_banners from the index.
 
$result = db_query_range("SELECT n.nid FROM {node} n LEFT JOIN {search_dataset} d ON d.type = 'node' AND d.sid = n.nid WHERE n.type NOT LIKE 'sos_banner' AND (d.sid IS NULL OR d.reindex <> 0) ORDER BY d.reindex ASC, n.nid ASC", 0, $limit);

  while (
$node = db_fetch_object($result)) {
 
// HACK: Mentioning indexed stuff in the logs.
   
watchdog('search', 'Indexing ' . l("node/". $node->nid, 'node/'.$node->nid.'/edit'));
   
_node_index_node($node);
  }
}
?>
Alan D.’s picture

Note that hook_query_node_access_alter() can be used to reduce the results nicely without losing all of the search index information for admin users!

The D7 of version of the Search Config module uses a series of permissions for this. For example, a site with the two content types, webform and page has the following permissions:

Search all content types
Allow users to search any content, overriding content type permissions.

Online Form: Search content of this type

Page: Search content of this type

If no permissions are set, no content is returned. The "Search all content types" is given to all users when the module is installed.

The relevant code is:

<?php
/**
 * Implements hook_permission().
 */
function search_config_permission() {
 
$permissions = array(
   
'search all content' => array(
     
'title' => t('Search all content types'),
     
'description' => t('Allow users to search any content, overriding content type permissions.'),
    ),
  );
 
$ct = array_map('check_plain', node_type_get_names());
  foreach (
$ct as $type => $label) {
   
$permissions["search $type content"] = array(
     
'title' => t('%type_name: Search content of this type', array('%type_name' => $label)),
    );
  }

  return
$permissions;
}

/**
 * Implements of hook_query_node_access_alter().
 */
function search_config_query_node_access_alter(QueryAlterableInterface $query) {
 
// A global override to prevent user 1 from being restricted.
 
global $user;
  if (
$user->uid == 1) {
    return;
  }

 
// Try and see if we are dealing with a node search
 
$search = FALSE;
 
$node = FALSE;
  foreach (
$query->getTables() as $alias => $table) {
    if (
$table['table'] == 'search_index') {
     
$search = $alias;
    }
    elseif (
$table['table'] == 'node') {
     
$node = $alias;
    }
  }

  if (
$node && $search) {
   
// Standard search
   
if (!user_access('search all content')) {
     
$excluded_content_types = array();
     
$ct = array_map('check_plain', node_type_get_names());
      foreach (
$ct as $type => $label) {
        if (!
user_access("search $type content")) {
         
$excluded_content_types[] = $type;
        }
      }

     
$query->condition($node . '.type', array($excluded_content_types), 'NOT IN');
    }
  }
}

// Also form alters for the search form and install code, etc.
?>

This code would probably be even more compact if in the node module.

Note: While it would be cool to get this functionality into core, I personally see Search Restrict module as the best place for this functionality, but after delays / stonewall from that project, I took up Search Config and refactored it to use this permission based solution.

pwolanin’s picture

Sometimes there are nodes they you never want in the index - seems like that's a common enough use case that the simple list of excluded node types is a reasonable feature for core.

Alan D.’s picture

Permissions work well. See search configuration module as an example. Note that the actual implementation within Custom Search of the query alter is much cleaner, but this uses (?) another setting somewhere.

Downside of permissions is that you get node permission settings randomly thrown into the permissions page and additional cruft here eventually leads to memory issues. We have two sites with 128M memory that run out of memory on the permissions all page.

So the settings based off #62 and the query access alter would be the way to go imho, with maybe a singular permission to search all content. This bypasses the need to flush the index if these settings change.

pwolanin’s picture

Well, those are very different use cases. I don't think core should handle the case essentially of node access (permission based access) - that should be handled by a node access module. It should handle the simple case of excluding some content types.

jhodgdon’s picture

Just to clarify, as a site builder who often needs this feature: The main use case I see for excluding node types from the index is when you create a small content type that is used to make a View but should never ever be shown by itself on its own page on the site. The content type needs to be visible (because you want everyone to see those views) but you don't want it in the search index. So node access permissions are not the right solution.

On the other hand, you could in principle use the Drupal permissions system to implement this feature request, separate from the main node access permissions. You could have a permission called "search [content_type_name]" for each content type, and then you could add to the search query something like condition(n.type, array(content types the person has permission to search)). But this seems overly complicated when you probably don't want *anyone* to search them and you might as well excluded them from indexing entirely to save time. Of course, that would mean an administrative view for VBO would also not be able to find these little content items...

ditcheva’s picture

I want to add my voice to the mix. Both to vote for this functionality to be added to core and to confirm @jhodgdon's clarification that content-type granularity for excluding nodes from search is the most common need for me. I'm not saying there aren't times when you want to exclude individual nodes, but excluding entire types should be a minimum requirement for seach. I also have many instances when I create content types to be included in views, and the individual nodes (whether they're small or very full) don't make sense by themselves without the context of the view they're in...

Whether this is done via the permissions screen or via the search admin UI page and the node module, I wanted to say I really would like to have this issue as well... It's true that doing it the permissions way would enable admins to still see the nodes via search, but you could just build an admin view that displays the nodes to an admin rather than having them relying on search if you really need to. I think I'd vote for doing it the second way...

Alan D.’s picture

I'd keep the nodes in the search index irrespective. Two main reasons

a) It would be a common use case to restrict content types per role

b) Without the search index, admins could not use views to search using search terms in views unless each and every field is searched by the admin views. (i.e. Views search terms uses search_index)

ditcheva’s picture

True... and I didn't think about b), but you're right about that.

So what's the consensus about getting this in core? Do we need to create a patch and set to 'needs review'?

jhodgdon’s picture

Those are good points.

So... Based on all of the above, the plan here would be:
- Index everything
- Have separate permissions in node search for "search content of all content types", and "search foo content" for each content type

Right?

ditcheva’s picture

Correct. That's what we're leaning towards now.

jhodgdon’s picture

Yes, the next steps would be:
a) Create an issue summary with this plan in it.
b) Write a patch (whoever is doing that: assign the issue to yourself). As another note, this would also affect the choices available in Advanced Search where I think you can restrict to content type? Maybe?
c) Several people review the patch
d) Hopefully one of the core committers commits it to core.

I would also point out in the issue summary that I don't think it is easy for a contrib module to make this happen, because as far as I know, there is not an easy way to detect that a search is taking place and modify the query.

So, another alternative would be to make sure there is a query tag of some sort on the search query, so that a contrib module could easily do things like this and possibly other things. That would also be a good feature and perhaps an even more useful way to provide this functionality.

Alan D.’s picture

"I don't think it is easy for a contrib module to make this happen"

Nope, search configuration does it already (assuming the same hooks are in Drupal 8.x). While it's primary focus was on the UI for 6.x, and these were carried through to 7.x, I have implemented a permission system that does this with a query alter + UI restrictions on the types. Actually, it allows a dual level restrictions, one at the UI level plus the core restrictions at the query level. It also provides a third, per node restrictions too with a singular join that ensures that there is minimal overhead in this.

I'm considering dropping the UI side of things, as custom search does many of these already, but that would lead to a direct conflict with Search restrict. So if core handles this, I'd happily retire that module (again).

General summary of it's approach:

Implement hook_query_node_access_alter() to restrict
Provides a series of permissions, "search all content", and "search content of %type"
Form alters out the additional types in the advanced search form

Search restrict

Implement hook_query_node_access_alter() to restrict (nearly identical if not identical)
Provides role options on the content type page itself to restrict

Personally, I prefer the permissions system but it is a matter of personal taste I guess.

pwolanin’s picture

@ jhodgdon - since indexing tends to be quite slow and bloat the DB, why not simply exclude certain node types from the index? That would seem to have might higher performance at both index and query time (since you don't need extra SQL conditions)

jhodgdon’s picture

RE #92 - how does a contrib module know that a particular query it is modifying is actually a Search query? Presumably it doesn't want to change node access for all queries.

RE #93 - this was addressed in #87.

Alan D.’s picture

In D7, search_config looks at the tables via hook_node_access_alter, but it would be better to have this tagged to make this easier.

<?php
function search_config_query_node_access_alter(QueryAlterableInterface $query) {
 
$search = FALSE;
 
$node = FALSE;
  foreach (
$query->getTables() as $alias => $table) {
    if (
$table['table'] == 'search_index') {
     
$search = $alias;
    }
    elseif (
$table['table'] == 'node') {
     
$node = $alias;
    }
  }

  if (
$node && $search) {
   
$query->condition($node . '.type', array($excluded_content_types), 'NOT IN');
  }
}
?>

I'd check out the other search modules too, there may be even a cleaner way without the tags :)

jhodgdon’s picture

Title:Exclude node types from search index» Exclude node types from searches

RE #95 - The problem with that method is that it will also exclude those content types from being used in Views that use the "search keywords" filter, or any other node query that happens to join the search index table, not just actual searches. Which is to say, Core really doesn't have a way to detect "this is a core node search query". There should be a query tag, or better yet, just build this functionality into core and have the node module put this condition in when a search is actually taken place.

Chasen’s picture

This would be very handy, but not just excluding whole content/node-types but individual nodes as well would be great.

klonos’s picture

I think we should postpone this on #2083717: Convert Search Results to Views (or close it as a duplicate or even make the other issue a META and this one one of its sub-issues).

jhodgdon’s picture

Issue summary:View changes

Coming back to this issue...

So there were two use cases discussed here:

a) You don't want to have certain node types added to the search index at all. This is actually a fairly common use case, and can be solved by adding configuration to the NodeSearch plugin to exclude certain node types from indexing. Or it could be solved by a contrib module providing its own replacement for the NodeSearch plugin.

b) You don't want users to see certain node types in search results, but you still want them in the index for purposes of building admin views that will let admins find this content by search term (as opposed to using something like a title contains filter, which doesn't search the entire content of the node). In this case, you could resolve it by doing a query alter hook pretty easily, so I don't think we need to cover it here. Note that this was not possible a year ago, until #1435834: Cannot alter search queries, tag added too late was fixed for Drupal 8.x.; it was actually fixed on #1366020: Overhaul SearchQuery; make search redirects use GET query params for keywords.

Updated summary.

jhodgdon’s picture

Version:8.0.x-dev» 8.1.x-dev

Since 8.0.x-beta1 has been released, our policy at this point is No feature requests until 8.1.x. See #2350615: [policy, no patch] What changes can be accepted during the Drupal 8 beta phase?. Sorry, it's just too late for 8.0.x at this point, so even if we had a viable patch, the core committers would not commit it. So unless we decide this is a Task or a Bug (and I don't think it is), we'll have to delay it.