Search Content by Author

I support the RTBC. The only argument against this approach is that there is no way to update the index for an author's nodes when that author changes his name. This, however, is no different than the inclusion of taxonomy terms in the search index... there is no way to update them either, so it is a larger problem that should be handled separately. This patch will make search much more useful.

This patch raises the question of how best to rebuild the search index. I think it would be unfair to simply wipe people's search index as a matter of upgrading to D6 (although that would certainly solve the problem). Perhaps it should be an update that they can opt out of with ample information?

The issue is the already indexed content that needs to be re-indexed for this feature to take hold. We have the same issue (though it is less likely to affect many people) with http://drupal.org/node/155262.

I think I'm in favor of an update that calls search_wipe_index and some documentation in the upgrade docs that points this out.

I'd put the update in search.install so that people can most easily opt out of it.

I've rerolled this patch relative to the base directory. I'm not sure what to do about the search index issue , but I think the ability to search by author should be included. I'm surpised that it wasn't in the past. +1

Shouldn't we theme('username', $node)?

@robert, I think that Steven's link patch is going to force a re-index. See #146466.

Re-rolling with slight changes to adhere to coding standards.

@Doug: thanks for the re-roll, and yes I think you're right about theme('username')

No, I take that back... theme_username isn't right here. Keeping as RTBC.

Oh, and the reindexing of the site due to Steven's patch would be a great way to handle the addition of this feature. In fact, if this patch, #155262 and Steven's link patch all went in it would be overall a nice incremental improvement to the search index.

In the future we need to have a hook that lets modules send back a set of nids which need reindexing. One monolithic query in node_update_index doesn't cut it any more.

I'm leaning towards agreeing that theme_username isn't needed here, but you didn't explain yourself. I think it might not be needed because all it does is wrap the user name in an HTML anchor, and since we're just doing search indexing here, all that the extra anchor would do is affects the words relevancy. You're already putting it inside a STRONG which does the same thing.

However, what about sites that override theme_username?

Sites that override theme_username usually change the presentation (add an icon, or styling) but don't change the name itself. I think that we have the name in fine form as it is and can use it to link content to the author in the view of the search engine.

I've occasionally used theme_username to replace the user name with the user's full name. So I'm not convinced this shouldn't be themed. But I do believe that we should index some user name. And if we decide later that it should be the themed name, this can be changed.

So, +1 for the current patch.

I haven't checked out the patch, but -1 on not using theme('username') everywhere a username is displayed. I think this should be marked 'needs work' but don't want to set it as I haven't installed the patch, just read comments.

In a lot of sites I've done, we have a Drupal core username and then a Display name using another module. I override theme_username to show a Profile module field's display name, Usernode CCK's field display name or a display name from LDAP. What we really need is a third argument in theme_username that shows name as a link or not IMO or an arbitrary $style string.

I created a simple patch for theme_username at http://drupal.org/node/156266 just now.

Blake, can you reroll with theme_username?

@Blake, now please re-roll while adhering to coding standards (see #30785 or compare your latest patch to the one I re-rolled).

No longer applies, but would be a nice addition in Drupal 7.

Rerolled for D7.

Applies cleanly and works as advertised.

Before applying the patch, I generate a bunch of stuff with devel_generate (first users, then content). I then mark the search index for re-indexing, run cron, and try to search for one of the users that have associated content - which returns nothing.
After applying the patch, I mark the search index for re-indexing, run cron, and try to search for the very same user, this time returning the nodes associated with it! Success! :D

Oh, yes, there's of course no upgrade path, but the search will still work as normal (according to my testing) until the site is re-indexed, so I don't really feel one is needed for the code to go in. Keeping the RTBC.

Do we want a note somewhere to mention the reindexing? Only places I can think of are CHANGELOG.txt and a fake search_update() with a drupal_set_message() though.

Ah, you're right. Not sure about whether a search_update_X is needed, but this patch has a draft for a CHANGELOG.txt entry. Setting this back to CNR until we've decided on the CHANGELOG.txt text and whether to use a search_update_X to inform upgraders.

Patch broken by changes to CHANGELOG.txt and the node.module hunk applied with offset. Re-rolled. Also, as there have been no comments on the CHANGELOG.txt text, I'm setting this back to RTBC.

Agree with RTBC.

First, I'm not a fan of using theme('username') here -- it leads to behavior that is unpredictable and that could lead to inconsistent or skewed index (i.e. when switching a theme that puts more or different information in the index).

Second, it would be good if we extended the search module's tests.

This didn't make the Drupal 7 feature request deadline... moving on to Drupal 8.

This is actually a duplicate of #233476: Add search by node creation date and the author

Is there a final code here? I really need this function in D7.