Solr-powered user directory

Updating the issue with a suggested design comp.

Some criteria here:

- The search should include results where any field on the user profile matches the search query
- We should support partial name searching (eg, a search for "Johns" would include results for "Johnson."

Marking #1972630: Solr-powered User directory as a duplicate.

User entities are not indexed by default with just Apache Solr Search Integration. Something like Apachesolr User contrib module adds in the indexing of user's, but currently says minimally maintained. The module also adds facets for all fields on a user entity as well, if that's something needed at a later date.

http://drupal.org/project/search_api does users out of the box. Why would Commons go with Apache Solr? :)

Here's an updated comp showing how we're likely to present these. Note the ability to filter the directory based on who you are following.

This also includes "trusted contact" functionality for which I'll file a new meta issue shortly.

ooo i see a notification thingy on the screenshot next to the name "Jeff Noyes" showing "(1)", what feature is that? Is it part of commons 3.3 because i do not have it on 3.2.

@the604, see #1975198: [Meta] Trusted Contacts & Private messaging for details on that.

Update: I have the feature "working" on my local dev site. It will require a patch to Commons Events, and some updates to the Custom Search module to allow for a second Solr search page to be used in the search dropdown (to replace the core User search option).

Will post a patch here shortly.

The mockup in #7 looks great. I'm only missing some filters on profile fields.

Would love to have this in Commons 3.

jpontani - great to see this. Have you released the updated feature yet? If not, any idea when you plan on posting?

Also, what dev version are you currently using? I'm not able to patch any of your patches from: #1975198: [Meta] Trusted Contacts & Private messaging, in any version of Commons... although maybe I'm doing something wrong.

Thanks!

Not yet, I was going to do it last night but realized I was missing some key components to the feature. I'm working on it today, and should have something by tomorrow night (Friday) at the latest.

Great news! What dev build are you working off of? I would really like to apply your patches from http://drupal.org/node/1975198 but it keeps failing w/my local copies of commons dev.

This should be a good baseline feature to be able to add more to later.

Hi Joseph,

thanks for the patch. I've applied it and I see a feature appearing.
I've enabled the feature but I don't see anything happen, except for the 'Users' dropdown in the search box.

When I try to search users, I get redirected to the Site search.

Can you explain what it should do, and how we can see if it is working?

That's my fault, forgot to add that in the patch.

Try this patch instead, it should change the default action of the dropdown 'Users' in the search form to use the directory.

Hmm, might be me, but I don't see a tab 'People' appearing. I've enabled the feature, enabled the User entity in the SOLR settings and re-indexen my content. The feature is in its default state and I've cleared my caches.

Can you give any help on how to get this patch working?

Also; the paths in the patch look incorrect.

Should 'modules/commons_search_solr/modules/commons_search_solr_user' be changed into 'profiles/commons/modules/contrib/commons_search_solr_user'?

The 'People' tab isn't appearing because I had the menu item's weight set incorrectly.

As for the paths in the patch, they are correct. This patch is for the commons_search feature directly, so the paths are relative to commons_search.

I attached a new patch with the correct menu weight, and a forced revert of the menu links on install, this should get the People tab on the main menu to show up.

Thanks, jpontani!

I realize this patch is in progress, so here are some areas that can be revised.

A) The "People" menu link is the leftmost link with its current weight - Per the designs, it should be to the right of "Events."
B) In my testing, user entities weren't enabled for indexing by default. Let's enable this.
C) The search text field on the "People" page appears to be a username autocomplete, but it should be a "contains"-type search. Per comment #1,

- The search should include results where any field on the user profile matches the search query
- We should support partial name searching (eg, a search for "Johns" would include results for "Johnson."

D) Can we provide some more friendly empty text, or even better, fallback to a non-Solr version of this page when users haven't been indexed?

E) Remaining tweaks to the view to match the designs.

A, B, C, and D should be taken care of in this patch.

A: I just do a query for the Events weight, add one to it, and use that as the weight for the new menu link.
B: User's should be set to be indexed upon enabling this module now.
C: This is now the Realname field, but it does require user's have their realname set for the results to be accurate
D: Falls back to the base view that is embedded if users haven't been indexed yet.

Will add the remaining aspects to the view tomorrow (Wednesday).

Setting to needs review for these 4 aspects.

Thanks, jpontani!

C: This is now the Realname field, but it does require user's have their realname set for the results to be accurate

Note that searching against only Realname is not sufficient, because:

- The search should include results where any field on the user profile matches the search query

Complete rework of how I was doing it. This method embeds a view per each result, and that view has the necessary output items (name, picture, groups, etc), and the view has an attachment for Following/Not following flag.

This patch also doesn't rely on any exposed views form to do the 'search', it uses the Solr search form, so will return results that match the search term, regardless of where it matches (ie: will search all fields on the user for the terms, addressing #22).

This patch also uses a custom theme function to output in a 'grid' (table based for now), with adjustable grid size (in code).

To get rid of the warning messages, need the fix from #1788322: Undefined index: id (may need a new patch for 7.x-1.0 release, not sure if it applies, I did it manually).

Thanks, jpontani.

I wonder about the performance impact, specifically in render time, that we'll incur from rendering a single view for each result that Solr sends back. Would Solr send back the entire result set or just the results that would display in a single page?

The Solr page itself (/people) is limited to paging results 12 at a time, so the most views being rendered on that page would be capped at twelve (excluding other views being injected later from custom code). Solr only sends back a limited dataset per 'page'.

Updated patch, forgot to update the feature info, and add a dependency.

Hi all,

I've tested the patch against 3.2 (the reason I had the errors in #18 were probably caused because I used the patch on Commons 3.0).

The patch applies nicely and I'm seeing the People tab.

Some issues though:

- The People tab misses the icon.
- If you use the search box drop-down to search on users, it still jumps back to the Site option on the People directory. By the way; shouldn't 'Users' be 'People'?
- search/user/test is still active?
- I'm getting an error about the Search not accessible: "Search is temporarily unavailable. If the problem persists, please contact the site administrator.". The search works though and the server on admin/config/search/apachesolr/settings is green.
- Some styling issues, see screenshot.

1: the error above
2: shouldn't this be like #7?
3: Contributed to and Follow doesn't work as expected, see #7.

Also, every time I clear the cache, the People tab gets disabled and the feature overridden. Reverting the feature puts the People tab back.

I'm seeing this issue when I click the 'following' button and type in Jeff manually:

And when I uncheck it, the search function doesn't appear to do anything. It just keeps showing the same users.

Also, I've updated the drupal-org make file to use the recent versions of views_field_view and apachesolr_user modules. This should eliminate some problems people have been having reviewing this module.

http://drupalcode.org/project/commons.git/commit/bb135dc

A) I tested by accident in an environment where the Solr service was down and the People view showed everyone along with a message about search being down. Nice fallback behavior!

However, we also see all people on the site when a search query returns no results, such as http://localhost/dev/acquia/commons_nightly/commons/latest-2/people/3453.... I would expect to see empty text in the event that a search query shows no results.

B) I verified that the search did include terms inside various fields on the user profile (eg Body field). However, the search did not meet the requirement from #1 of

- We should support partial name searching (eg, a search for "Johns" would include results for "Johnson."

For example, a search such as "people/dharm?flagged=0" didn't turn up Dharmesh Mistry. I'm open to addressing this in a followup issue if that becomes the one remaining outlier.

C) The "People" menu item wasn't created for me.

D) Let's change "Apply" to "Search"

E) Can we make the "Following" checkbox submit asynchronously? If not, that's OK. Could also be a followup issue.

E) Let's change "Users" to "People" in the search dropdown and anywhere else it appears in the community member UI.

On request :

/**
 * Build the documents before sending them to Solr.
 *
 * Supports all types of
 * hook_apachesolr_index_document_build_' . $entity_type($documents[$id], $entity, $env_id);
 *
 * The function is the follow-up for apachesolr_update_index but then for
 * specific entity types
 *
 * @param ApacheSolrDocument $document
 * @param object $entity
 * @param string $env_id
 *   The machine name of the environment.
 */
function commons_search_apachesolr_index_document_build_user(ApacheSolrDocument $document, $entity, $env_id) {
  // Index field_main_image as a separate field
  if ($entity->type == 'user') {
    $username = 'blabla_up_to_25chars';
    $document->setMultiValue('tos_username', $username);
}

function commons_search_apachesolr_query_alter(DrupalSolrQueryInterface $query) {
  // Search in partial usernames.
  $query->addParam('qf', 'tos_username');
}

Thanks, Nick_vh!

@jpontani - the pseudocode in #32 is meant to show how we might address #31B.

Nick also pointed to http://drupal.org/project/apache_solr_search_view_modes, which could help reduce some of the custom code we have in Commons around altering the presentation of search results.

Additionally, Nick pointed out that the default behavior of Solr results is to render the results using the Drupal theme layer, and so presentation of search results would be consistent with the way that the entities in the result set are themed in non-Solr listings, but our use of Rich Snippets module alters the presentation of Solr-based results.

For this reason, I think we should consider removing Rich Snippets module. If it would eliminate the need to use individual views for each user per page of search results, it seems worth removing Rich Snippets as part of this issue.

If not, let's evaluate that in a followup issue.

japerry is going to take this on.

For this reason, I think we should consider removing Rich Snippets module. If it would eliminate the need to use individual views for each user per page of search results, it seems worth removing Rich Snippets as part of this issue.

If not, let's evaluate that in a followup issue.

So, we may find ourselves at a point where we chose between a template file for rendering user teasers, or a Panelizer config.

Still listing this as needs work -- as we need to do some theming love. But this patch does quite a few things:

A) When solr is down or no results are returned and the query is empty, the page will show all users. When a search query is entered and no results are found, or if filters don't allow results to be shown (like following), it'll show the no results found help box

B) Partial name searching, from the beginning, works. Discussed middle-end name searches, and it really is difficult to do, apparently google doesn't do it either so neither will we.

C) The "People" menu -should- auto create, to the right. Double check the feature, and manually revert, but the code is in the install to revert the feature. Not sure why its not working for some people.. There is no icon uploaded yet, I'm guessing that'll come with theming.

D) Apply button was changed to search

E) Follow checkbox auto-submits when checked

F) People is listed in the search instead of users.

G) Views requirement and rendering has been removed. We're directly grabbing a user and displaying it in a new custom template that can be overridden by themes. This also should slightly improve performance because we aren't embedding a view to grab OG group memberships.

H) Rich snippets doesn't really have anything to do with us now that we're calling the grid theme directly. We should tackle that later, but its fine to keep it enabled for now.

I) Use the 'search_results' user view mode, which works great and doesn't require us to use the solr_view_modes module or displaysuite

Thanks for the new patch. I've applied it and I'm noticing the following:

- The dropdown in the search box on top still shows Site on the People search page. Shouldn't this be People?
- I'm not seeing pagers, I see all (thousands of) people on one page which makes the page really slow.
- I'm not seeing any facets?
- I'm getting the following notice when indexing:

Notice: Undefined property: stdClass::$type in _node_extract_type() (line 371 of /modules/node/node.module).
Notice: Undefined property: stdClass::$nid in node_build_content() (line 1366 of /modules/node/node.module).
Search is temporarily unavailable. If the problem persists, please contact the site administrator.

- I'm getting the following notices on the People page:

Notice: Undefined offset: 397 in theme_commons_search_solr_user_grid() (line 217 of commons_search_solr_user/commons_search_solr_user.module).
Notice: Undefined offset: 397 in theme_commons_search_solr_user_grid() (line 218 of commons_search_solr_user/commons_search_solr_user.module).

For what it's worth, I've tested on a real site, not a clean install. So I'm not sure if the errors above are all caused by the patch?

Thanks BarisW for the review, some responses/clarifications:

- The dropdown is site-wide and will always show site first, I don't think there is any provisions for people to show up first here.

- Ahh pagers. I saw that on the main page as well, but forgot to mention it last night. its going to be a problem for the default people page, since we bypass solr alltogether and do a massive query.

- I don't think we have any facets we're using for user search. Ezra, ideas?

- The undefined node notices are from facets showing up when they shouldn't be. To fix this issue, go into your solr configuration for the people page (admin/config/search/apachesolr/search-pages/user_search/edit) goto the advanced options and select 'show search box' for behavior on empty search.

- The last bit of notices, I haven't seen, but has to do with the grid filler for unfilled rows. The error after looking at the code doesn't glare out at me so I'm not quite sure whats up yet. But I also feel like there is already some grid theme formatter, so maybe can get rid of all this code anyway.

Thanks again for the review! I'll look into getting a pager in so its more scalable

Thanks, japerry for the patch and BarisW for the review!

I read the patch but still need to do functional testing today. Here's some feedback. Feel free to say "Fixed A from #38" and so on.

Assigning back to japerry - I assume switching that to jpontani was unintentional.

+++ b/modules/commons_search_solr_user/commons_search_solr_user.installundefined
@@ -0,0 +1,92 @@
+  $user_search_page = apachesolr_search_page_load('user_search');
+  if (empty($user_search_page)) {
+    // Couldn't find the user results page, so we create one.
+    $user_search_page = array(
+      'page_id' => 'user_search',
+      'label' => 'User Search',
+      'description' => 'User Search',
+      'search_path' => 'people',
+      'env_id' => 'solr',
+      'page_title' => t('People'),
+      'settings' => array(
+        'fq' => array('entity_type:user'),
+        'apachesolr_search_custom_enable' => TRUE,
+        'apachesolr_search_search_type' => 'custom',
+        'apachesolr_search_per_page' => 12,
+        'apachesolr_search_browse' => 'none',
+        'apachesolr_search_spellcheck' => TRUE,
+        'apachesolr_search_search_box' => TRUE,
+      ),
+    );
+  }
+  if ($user_search_page !== FALSE) {
+    $user_search_page['search_path'] = 'people';
+    $user_search_page['page_title'] = t('People');
+    $user_search_page['settings']['apachesolr_search_per_page'] = 12;
+    $user_search_page['settings']['apachesolr_search_search_box'] = TRUE;
+    $user_search_page['settings']['apachesolr_search_browse'] = 'results';
+    $user_search_page['settings']['apachesolr_search_spellcheck'] = FALSE;
+    apachesolr_search_page_save($user_search_page);

Solr pages are exportable. See Commons Search Solr as an example. Is there a reason we're saving this page in hook_enable() rather than exporting it with Features, which would allow us to detect and revert overrides?

B) I notice you're adding a new template for user profiles that's specific to Solr search results. Would it be possible to instead use a single template for user entity teasers, wherever they're displayed?

When applying #35 I get the following errors, and apparently the site search box loses the dropdown.

Notice: Undefined index: #type in template_preprocess_search_block_form() (line 1070 of /Users/ezra/Developer/htdocs/dev/acquia/commons_nightly/commons/latest-2/modules/search/search.module).
Warning: array_merge(): Argument #1 is not an array in _form_set_class() (line 4162 of /Users/ezra/Developer/htdocs/dev/acquia/commons_nightly/commons/latest-2/includes/form.inc).
Notice: Undefined index: #type in template_preprocess_search_block_form() (line 1070 of /Users/ezra/Developer/htdocs/dev/acquia/commons_nightly/commons/latest-2/modules/search/search.module).
Notice: Undefined index: class in custom_search_form_alter() (line 103 of /Users/ezra/Developer/htdocs/dev/acquia/commons_nightly/commons/latest-2/profiles/commons/modules/contrib/custom_search/custom_search.module).

Chatted with japerry in IRC: I had the master branch of custom_search module checkout out, rather than 7.x-1.x. Checking out 7.x-1.x of Custom Search resolved the issues in #39.

okay I've made a few more changes, including adding a pager, and creating a tpl as part of the user profile. There are no user 'teasers' yet (the view display doesn't exist) so I'm just using the search results. Although users are entities, they aren't quite nodes yet ;-)

You'll have to apply both patches for it to work. And it still needs theming love.

Thanks, japerry! I'll take another look at this.

There are no user 'teasers' yet (the view display doesn't exist) so I'm just using the search results. Although users are entities, they aren't quite nodes yet ;-)

However, they are full entities :).

Per hook_user_view, users have a display mode. Is there a particular reason why we can't make use of that?

This is looking good so far. I tested in the same environment #41 and will next test on a fresh install/upgrade.

A few points of revision.

A) Let's have the current search query reflected in the search box:

B) If I search "Site" for "Noyes" from the site search box, I'm presented with some interesting results, per this screenshot:

C) In the above screenshot, you can see that there is a tab for "Users" -- This tab takes the search to a user search page that appears to present user results with Rich Snippets presentation (shown in below screenshot).

Is there a reason we can't override or theme the existing ApacheSolr search/user page?

[edit] D) I forgot to call out in the text that in the screenshot for A, we're also apparently missing the peoples' names.

Committed a change to Commons Events that will fix B. The fix will require disabling Rich Snippets, which requires Commons Search to remove it as a dependency, per discussion.

http://drupalcode.org/project/commons_events.git/commitdiff/446d843

Okay whew, this patch makes a bunch of changes to the search results with users and nodes.

Note: You need to have an updated version of commons_events. There is a simple bug in it that caused node_load to fire on all entities, not just nodes. Coupled with this patch, search results should look a little more useful now.

Also, to eliminate this error on the events page, do this:

http://drupal.org/node/1946132

I'll add it to the make file.

+++ b/commons_search.installundefined
@@ -0,0 +1,8 @@
\ No newline at end of file

Newline

+++ b/commons_search.moduleundefined
@@ -32,10 +33,12 @@ function commons_search_form_alter(&$form, &$form_state, $form_id) {
+        $site_key = 'o-solr';

where does the o-solr come from. Why is it o-solr? Should this be documented?

+++ b/commons_search.moduleundefined
@@ -45,10 +48,11 @@ function commons_search_form_alter(&$form, &$form_state, $form_id) {
     if ((strrpos($node, 'node/') !== FALSE || strrpos($node, 'group/') !== FALSE)
-      && is_numeric(substr($node, strrpos($node, '/')+1))) {
+      && is_numeric(substr($node, strrpos($node, '/') + 1))

add some comment here what you are trying to do. Have a hard time understanding what you are doing here.

+++ b/commons_search.moduleundefined
@@ -58,12 +62,11 @@ function commons_search_form_alter(&$form, &$form_state, $form_id) {
+    if (arg(0) == 'search') {
       // If we've already searched, refill the search box with the current
       // search keywords.
-      $form['search_block_form']['#default_value'] = check_plain($_REQUEST['keys']);
+      $form['search_block_form']['#default_value'] = check_plain(arg(2));

take arg() in a variable so you don't need to invoke it twice. Not sure if D7 still has the args() function.

+++ b/commons_search.moduleundefined
@@ -87,10 +91,12 @@ function commons_search_search_form_submit($form, &$form_state) {
+      $form_state['redirect'] = 'search/user/' . $keys;

This will fail if the search page is manually updated by some administrator. You might want to get the search page path from the search page object. Load it by unique ID and get the path from there.

+++ b/modules/commons_search_solr/commons_search_solr.moduleundefined
@@ -442,3 +442,14 @@ function commons_search_solr_modules_enabled($modules) {
\ No newline at end of file

newline

+++ b/modules/commons_search_solr_user/commons_search_solr_user.apachesolr_search_defaults.incundefined
@@ -0,0 +1,37 @@
+  $searcher->label = 'User Search';
+  $searcher->description = 'User Search';

does this needs to to be wrapped in t()?

+++ b/modules/commons_search_solr_user/commons_search_solr_user.infoundefined
@@ -0,0 +1,16 @@
\ No newline at end of file

newline

+++ b/modules/commons_search_solr_user/commons_search_solr_user.installundefined
@@ -0,0 +1,92 @@
+  // We disable core user search in place of Solr user search.

s/in place/with

+++ b/modules/commons_search_solr_user/commons_search_solr_user.installundefined
@@ -0,0 +1,92 @@
+      'page_id' => 'user_search',
+      'label' => 'User Search',
+      'description' => 'User Search',
+      'search_path' => 'people',
+      'env_id' => 'solr',
+      'page_title' => t('People'),

Perhaps get this from a common function?

+++ b/modules/commons_search_solr_user/commons_search_solr_user.moduleundefined
@@ -0,0 +1,251 @@
+  $document->addField('tes_firstname', $entity->field_name_first[LANGUAGE_NONE][0]['value']);
+  $document->addField('tes_lastname', $entity->field_name_last[LANGUAGE_NONE][0]['value']);

check if this field exists?

+++ b/modules/commons_search_solr_user/commons_search_solr_user.moduleundefined
@@ -0,0 +1,251 @@
+    // We set the title to be blank so it doesn't disply 'Search results'.

typo :)

Commons_search is not really part of this review. But I noticed when I made a one line adjustment to it, my editor reformatted it to drupal standards. (not sure why newline is missed.. but yah). Anywho, code snippets 2-5 aren't relevant to this patch, its just spacing changes to comply with standards. I don't disagree with the comments, I just don't have an answer for them =)

Comment 6, I've looked to see what others who've exported this feature have done, and none have indicated that they're using t() on those fields. Its quite possible that the exporter has a bug, but for now I suggest not changing it, and if it should be using a t(), it should be brought up as an issue in the apachesolr module.

Comment 10 -- we're reverting the feature and that code shouldn't exist. I've pulled it out in favor of using the feature.

Other comments -- implemented the typos and line spaces. Thanks for the review!

re-rolled to not include the user search as an enabled module (we've replaced it with /people)

Aside from themeing, the remaining work here is to make sure all components are in default states when installed. Assigning to me to work on that.

And post DrupalCon, assigning back to japerry.

Wow this took all day to debug.

In short, caching sucks. And the menu item module should be pointing to the commons_apache_solr_user module instead of menu.

After a long day of debugging the features module, looking at various fixes, etc. I noticed that the problem is every time we do a feature revert, it'd do the user page at the same time as the menu item without dumping menu cache. The problem with this on install is without the 'people' menu existing, the validation of the main-menu link will fail, and won't get executed.

When we use drush to do a features revert, it does both at the same time, clearing the menu item and failing this validation. The UI works because the menu cache dumps upon new page loads, and we're only selecting the menu_link, not all the feature, to be reverted.

So what I've done is added caching callbacks in the .install file so the menu actually gets dumped between feature reverts. You can see this in the solr branch of commons_search.

every time we do a feature revert, it'd do the user page at the same time as the menu item without dumping menu cache. The problem with this on install is without the 'people' menu existing, the validation of the main-menu link will fail, and won't get executed.

Nice troubleshooting!
Taking a look at your new solr branch:

  //We must clear the menu cache before trying to set a link, otherwise it will fail.
  drupal_static_reset('apachesolr_search_load_all_search_pages');
  variable_set('menu_rebuild_needed', TRUE);
  menu_cache_clear_all();
  drupal_flush_all_caches();

  $revert = array(
    'commons_search_solr_user' => array('menu_links'),
  );
  features_revert($revert);
  menu_cache_clear_all();
  drupal_flush_all_caches();

Some concerns here:

This seems like quite a few cache clears that could be relatively expensive, and are at least partially redundant. For example, drupal_flush_all_caches() calls menu_rebuild() which calls menu_cache_clear_all(), yet we explicitly call each of those functions twice in this code. Clearing cache in this way can be quite expensive, so it seems worth exploring a more targeted fix.

I checked with mpotter in #drupal-features about whether there is a way to control the order that components of a feature are reverted and it appears that an API doesn't existing strictly for this purpose. However, there are some Features API hooks that seem worth exploring and could help us avoid some of aggressive cache clears.

Reading through features.api.php suggests a few that are worth looking into:

hook_pre_features_enable_feature
hook_pre_features_revert

Also, friendly reminder to be mindful of Drupal coding standards around code comments :).

The reason why the cache clear was required is because the 'customize' flag isn't usually set for features. The idea is that features shouldn't be customizable by the user. When menu_links aren't customizable, the process above is required for the menu_links system to not delete the link because the parent menu item (in this case apache solr user page) doesn't exist yet.

When we force a menu_rebuild and cache_clear (which does the actual rebuilding), the link will save properly. However this is expensive.

With patch #8 (not 6.. thats for drupal 6) from #927576: Menu links not set as customized, revert when menu rebuilt we can use the 'customized' key in the menu_link and not have the feature show overridden. I think for now this is a decent fix.

https://drupal.org/files/menu_links_customized-927576-8.patch

On a fresh installation of Commons with Features Rc1 and the patch specified in #54, I've disabled Commons Search Core, enabled Commons Search Solr and Solr User, and both features are Overidden.

After reverting the features and queueing content for indexing, I search for a person's name and get a 404 at people/noyes.

I rewrote the module's theme implementation. This should allow for more flexible styling in both commons_origins and any other theme that wants to take advantage of it.

http://drupalcode.org/project/commons_search.git/commitdiff/f108f20c1f92...

Putting back ezra's status.

Looks like japerry made another commit last night:

http://drupalcode.org/project/commons_search.git/commitdiff/e24b0ad?hp=f...

However, I believe the status is still "needs work" here because:

cache_clear_all('*', 'cache_menu', TRUE);

We need to use a Drupal API function to clear the menu cache, such as menu_cache_clear() or menu_cache_clear_all() in order to clear the cache properly.

Another thing that is of great concern to me is the processing going on in commons_search_solr_user_process_search_results(). This sort data processing should not be happening in a preprocess function. The fact that it is happening in a process function makes it even worse. I have gone in and adds a simplification to the code and processing to share what is happening in commons_search_solr_user_apachesolr_search_page_alter(), but it is still less than optimal.

It's clear that we're experiencing some difficulty getting the apachesolr_search_page to revert automatically upon initial installation of the feature.

Rather than pursuing agressive cache clears and saving to the database, I propose that we instead:

- Document the need to revert the Commons Search Solr* features after enabling
- Consider merging Commons Search Solr, and Commons Search Solr User into a single module, which would greatly simplify exporting our Solr configuration, as a followup issue: #2009148: Consider re-architecting Commons Search Solr.
- Pursue any potential issues with the CTools export-revertability of ApcheSolr Search pages in the ApacheSolr search module.

I've updated the 7.x-3.x-solr feature branch to remove the agressive cache clears so that we can achieve the immediate goal of this issue, and attached a diff against the 7.x-3.x branch.

I think this is ready to go from a theming end as long as there are no drastic changes to the layout in the meantime. The theme changes can be found in the issue-1921982-solr-user-directory branch.

Looks like the last thing is figuring out the flags. Right now I'm going with pulling them out of the search results once we get them. Same with the search landing page, which doesn't use solr because nothing is being searched for.

We aren't doing a flag facet because its per user per piece of content. This makes indexing a bit of a pain. If we were flagging content globally it'd be a different story. For now just doing the flag after, and changing the pager show it doesn't show different results.

Marking as "needs work" per #63.

I take back what I said about not messing with the solr index. There is no way to intercept the solr index pager after the query is built. Therefore the only way to get the pager to work correctly when doing a solr search is to have the flag ids in the query itself. The only issue now with that is the followed tag won't be instant for searches, since it relies on solr reindexing that content.

However, we do a separate db_search query instead of using EFQ views still for the landing page. This doesn't touch solr, so its not really relevant. If you look at the code, you'll see the switch for flags where no results (no query) is present. All of the queries use the user_view hook, eliminating the need to duplicate code.

Pagers have been tested in all the other use cases:
1) People landing page, 113 people, no following
2) People landing page, 10 people, no following
3) People landing page, 113 people, following 1
4) People landing page, 113 people, following 13
5) People landing page, 10 people, 2 following
6) Solr Search result, 24 people found (not following)
7) Solr Search result, 4 people found (following)
8) Solr Search result, 12 people found (following)

The pager works in all of these use cases. Should be ready for review.

Thanks, japerry & Zarabadoo!

This is coming along nicely! Here are some areas where it needs work:

Functional:

I did a devel generate of 200 users.

I searched for "Lisa Rex in quotes.

A) I got 2 pages of results which definitely do not contain "Lisa Rex" in them, whereas I would expect only Lisa Rex's profile to come up.

B) The search bar contains unfriendly text from sanitization (%22).

Screenshot of A& B:

C) Clicking the "People" link (aka, a search with no keywords) shows no results, when I would expect it to show all users on the site.

D) When the "Flagged" query string is not present, I get the error:

Notice: Undefined index: flagged in commons_search_solr_user_apachesolr_query_alter() (line 35 of/Users/ezra/Developer/htdocs/acquia/commons-builds/latest-5/profiles/commons/modules/contrib/commons_search/modules/commons_search_solr_user/commons_search_solr_user.module).

E) We're displaying "Report user" in the user directory. Let's remove that from the user directory since it's not in the designs and is unlikely to be a top 20% task when searching the user directory and is more consistent with not displaying "Report" links prominently at all times.

Themeing:

We're not matching the designs in 2 important ways:
G) We have a sentence format "Send this user a private message" link, whereas the designs have use the Trusted=Pending=>Message button.
H) We're not presenting the results as a grid and as a result about half of the user cards are whitespace.

Code (mostly related to code comments):

I)

+/**
+ * Submit handler that handles only the redirection to our Solr user directory.
+ */
+function commons_search_solr_user_search_submit($form, &$form_state) {
+  if ($form_state['values']['custom_search_types'] == 's-user') {
+    $user_search_page = apachesolr_search_page_load('user_search');
+    $keys = '/' . check_plain($form_state['values']['search_block_form']);
+    $form_state['redirect'] = $user_search_page['search_path'] . $keys;
+    drupal_goto($form_state['redirect']);
+  }
+}

What's the reason for using drupal_goto() in addition to setting $form_state['redirect']. Ideally we'd be able to use $form_state['redirect'], but if not, we should have a code comment explaining the need for drupal_goto(), since it's unusual to do drupa_goto() in a form submit handler.

J)

+/**
+ * Implements hook_module_implements_alter().
+ */
+function commons_search_solr_user_module_implements_alter(&$modules, $hook) {
+  switch ($hook) {
+    case 'form_alter':
+      // Move this module to be last for form alterations.

We should explain why this is necessary in a code comment.

K)

+
+    // We must clear the menu cache before trying to set a link, otherwise it will fail.
+    drupal_static_reset('apachesolr_search_load_all_search_pages');

Let's be more specific about what it means that this will fail and why that's the case.

L)

<?php
/**
+ * Implements hook_apachesolr_search_page_alter().
+ */
+function commons_search_solr_user_apachesolr_search_page_alter(&$build, $search_page) {

Let's explain what we're doing here in a code comment.

Thanks, japerry & Zarabadoo!

This is coming along nicely! Here are some areas where it needs work:

Functional:

I did a devel generate of 200 users.

I searched for "Lisa Rex in quotes.

A) I got 2 pages of results which definitely do not contain "Lisa Rex" in them, whereas I would expect only Lisa Rex's profile to come up.

B) The search bar contains unfriendly text from sanitization (%22).

Screenshot of A& B:

C) Clicking the "People" link (aka, a search with no keywords) shows no results, when I would expect it to show all users on the site.

D) When the "Flagged" query string is not present, I get the error:

Notice: Undefined index: flagged in commons_search_solr_user_apachesolr_query_alter() (line 35 of/Users/ezra/Developer/htdocs/acquia/commons-builds/latest-5/profiles/commons/modules/contrib/commons_search/modules/commons_search_solr_user/commons_search_solr_user.module).

E) We're displaying "Report user" in the user directory. Let's remove that from the user directory since it's not in the designs and is unlikely to be a top 20% task when searching the user directory and is more consistent with not displaying "Report" links prominently at all times.

Themeing:

We're not matching the designs in 2 important ways:
G) We have a sentence format "Send this user a private message" link, whereas the designs have use the Trusted=Pending=>Message button.
H) We're not presenting the results as a grid and as a result about half of the user cards are whitespace.

Code (mostly related to code comments):

I)

+/**
+ * Submit handler that handles only the redirection to our Solr user directory.
+ */
+function commons_search_solr_user_search_submit($form, &$form_state) {
+  if ($form_state['values']['custom_search_types'] == 's-user') {
+    $user_search_page = apachesolr_search_page_load('user_search');
+    $keys = '/' . check_plain($form_state['values']['search_block_form']);
+    $form_state['redirect'] = $user_search_page['search_path'] . $keys;
+    drupal_goto($form_state['redirect']);
+  }
+}

What's the reason for using drupal_goto() in addition to setting $form_state['redirect']? Ideally we'd be able to use $form_state['redirect'], but if not, we should have a code comment explaining the need for drupal_goto(), since it's unusual to do drupal_goto() in a form submit handler.

J)

+/**
+ * Implements hook_module_implements_alter().
+ */
+function commons_search_solr_user_module_implements_alter(&$modules, $hook) {
+  switch ($hook) {
+    case 'form_alter':
+      // Move this module to be last for form alterations.

We should explain why this is necessary in a code comment.

K)

+
+    // We must clear the menu cache before trying to set a link, otherwise it will fail.
+    drupal_static_reset('apachesolr_search_load_all_search_pages');

Let's be more specific about what it means that this will fail and why that's the case.

L)

/**
+ * Implements hook_apachesolr_search_page_alter().
+ */
+function commons_search_solr_user_apachesolr_search_page_alter(&$build, $search_page) {

Let's explain what we're doing here in a code comment.

Updated code (in the solr branch) based on recent review. Should fix/take into consideration of the points above, with exception of theming the grid on results and private messaging.

Nice fixes!

However
A) I'm still seeing the problem from 67A
B) We get a JavaScript parse error as a result of sanitizing the search query in the search box. japerry pointed out the syntax error in IRC:
C) When on the "people" page with a search query entered, if I change the dropdown to "Site" and enter a new search query, submitting the form redirects me back to the "People" page with the previous search query still present.

Zarabadoo pointed out that searching from the frontpage results in being redirected back to the frontpage.

The latest version in the solr branch should fix 70c and 71. pretty sure 70a is an environment issue since no one else can reproduce the issue.

70b is because of this issue #2012210: Javascript exception when quotes used in search box -- I've added a patch to the issue and the make file so we can at least ship this, and then come back to custom search js later.

I was able to reproduce 67 A & B.

67 G is reliant on https://drupal.org/node/2012026 being done first. Once that is done I can make sure it looks like a button and positioned appropriately. I am making some preparations for that now.

67 H I am not exactly sure what to do with. The stacking is something I discussed with noyes and it seemed logical from the sense that we are performing a search and we had conflicting styles existing search results. It seemed like the least complicated and messy approach to do it this way.

I am sure we can make search results in a grid, but it does not blend well with the normal search results and we will still have a girth of horizontal space there. So we just end up having two interfaces for similar search functionality and do not really fix the core problem. One possibility is that we can possibly make better use of the space during a query. We have a whole description field that can be used along with social networking links. If you are searching for someone, I can see this information being useful in determining if show a person is and more interests. The initial splash of the grid can stay the same and act as a scannable page of faces. With the following option clicked, I find this a bit handier than the user admin panel that we currently have. (Likely a personal preference.)

Sorry if this sounds a bit nonsensical. This is a raw brain dump of a possible directions ans solution. I do not really consider excess whitespace to be a deal breaker, but there is possibly more we can do with it.

With the fix to trusted contacts and discussions with Al about the theming, I think we can mark this RTBC! Any specific solr issues can be filed after as their own issue.

Added! awesome work everyone on this feature :-)

http://drupalcode.org/project/commons_search.git/commit/1344644

Also we are using this JS patch for custom_search in the make file.

http://drupalcode.org/project/commons.git/commit/048de49

Lets not forget the theme changes: http://drupalcode.org/project/commons_origins.git/commit/c681409

There's a warning message when searching now, due to a function return array being passed directly to drupal_render, which expects a referenced array in order to modify it.

Strict warning: Only variables should be passed by reference in commons_search_solr_apachesolr_process_results() (line 486 of /Users/joe/Desktop/code/commons_search/modules/commons_search_solr/commons_search_solr.module).

The issue lies in the commons_search_solr_apachesolr_process_results function, and should be something like:

function commons_search_solr_apachesolr_process_results(&$results, DrupalSolrQueryInterface $query) {
  foreach($results AS $rkey => $result) {
    if ($results[$rkey]['fields']['entity_type'] == 'node') {
      $node = node_view(node_load($results[$rkey]['fields']['entity_id']), 'teaser');
      $results[$rkey]['snippet'] = drupal_render($node);
    }
    if ($results[$rkey]['fields']['entity_type'] == 'user') {
      $user = user_view(user_load($results[$rkey]['fields']['entity_id']), 'search_results');
      $results[$rkey]['snippet'] = drupal_render($user);
    }
  }
}

So this warning only shows up when using PHP 5.4. Officially we aren't supporting 5.4 just yet, so leaving this as fixed.

Minor fix to get the feature back to normal (currently, it keeps being overridden).

Per comment #54, it's intentional that the "Customized" flag is exported as 1.

Thanks for pointing out that the feature is overidden - Please file a new issue so that we can triage that issue and let this 80+ thread about the initial implementation remain closed :).