Fullname filter for views

+1

Anybody reviewed this patch?

Going to do this right now.

Very nice start! The patch looks almost complete, a few thing needs to be done. Here is my review:

+++ b/includes/name_handler_filter_fullname.incundefined
@@ -0,0 +1,86 @@
+    foreach($words as $word) {
+      $table = $this->table;
+      $field_prefix = $this->field;
+      $this->query->add_where_expression($this->options['group'], "$table.$field_prefix" ."_given LIKE $placeholder OR $table.$field_prefix" ."_middle LIKE $placeholder OR $table.$field_prefix" ."_family LIKE $placeholder ", array($placeholder => $word . '%'));

I've got wrong queries is being generated. E.g. When searching for 'Vadim S' following conditions are added, which aren't looking right:

WHERE (( (crm_core_contact_type.disabled = '0') AND (field_data_contact_name.contact_name_given LIKE 'vadim%' OR field_data_contact_name.contact_name_middle LIKE 'vadim%' OR field_data_contact_name.contact_name_family LIKE 'vadim%' ) AND (field_data_contact_name.contact_name_given LIKE 'vadim%' OR field_data_contact_name.contact_name_middle LIKE 'vadim%' OR field_data_contact_name.contact_name_family LIKE 'vadim%' ) ))

This issue cause we are using same placeholder for different words, so it is better to use unique placeholder. Or we need to call it inside the loop.

+++ b/includes/name_handler_filter_fullname.incundefined
@@ -0,0 +1,86 @@
+    }
+    ¶

Space at empty line

+++ b/name.moduleundefined
@@ -1236,3 +1236,13 @@ function name_clean_name_component($name, $data_key = 'user_data', $letters = 'a
+  list($module, $api) = func_get_args();
+  if ($module == "views" && $api == "views_default") {
+    return array("version" => "3.0");
+  }

Why do we need to make this check? I've got both parameters are equal to 'views', hence nothing is returned. Just need to return version here.

+++ b/name.views.incundefined
@@ -0,0 +1,23 @@
+function name_views_data_alter(&$data) { ¶

Space at the end of line.

+++ b/includes/name_handler_filter_fullname.incundefined
@@ -0,0 +1,86 @@
+    }

Space at empty line.

Also I am experiencing weird behavior, which is brought by the Views module, I guess. Labels of the filter are named as one the field label for one of the bundle, however I have 3 different bundles, which have different labels for this field. May be it is caused by how the views work, not sure, it is something to check. May be we need to programmatically rename label to be more generic.

Also show all, show none is not working.

I have updated following things in a patch:

1. Replaced operator to one which allows to use these options: contains, has any word has all words.
2. Updated how query() works, now it concats all fields into one sting and then performs search depending on the operator. I am not sure which way is more good for performance, though.
3. Renamed name of the filter and cleaned up some code.

The same patch as above with spaces at the end of line removed.

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,115 @@
+/**
+ * Field handler to provide simple renderer that allows linking to a node.

Please replace "node" with "entity" word as we can add field not only to nodes.

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,115 @@
+      $placeholder =  $this->placeholder();
+      $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '%' . $word . '%'));
+    }

Should be:
$where->where("$fulltext_field LIKE $placeholder", array($placeholder => '%' . db_like($word) . '%'));

Truly to say, I was not able to test this patch properly.

What I have done:
1. Added Name field to article with default settings.
2. Created several articles with Name field submitted.
3. Created a view, table display, title and name fields

Now as I understand I should create a filter for Fulltext search in name, but can't find an option. Could you please point me a place in settings where I can do that.

In the code I see that we replace handlers for standard fields to make them to search in fulltext field, but I can't make this functionality work.

Thanks for review. This filter will appear when you will clear the cache.

Actually we do not replace handler, cause there is not any filter handler for the name field, so we add the filter. But there are a filters for the it's component.

This filter is called as a name field label.

@dealancer yes, you are right. My bad. Clearing caches helps :)

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,115 @@
+  function exposed_validate(&$form, &$form_state) {
+    if (!isset($this->options['expose']['identifier'])) {
+      return;
+    }
+
+    $key = $this->options['expose']['identifier'];
+  }

Why do we need exposed_validate() method? Looks like its code doesn't do anything.

+++ b/name.views.incundefined
@@ -0,0 +1,22 @@
+
+function name_views_data_alter(&$data) {
+  foreach(field_info_fields() as $field_name => $field) {
+    if($field['module'] == 'name') {
+      $name_fields[$field_name] = $field;
+    }
+  }
+  foreach($data as $field_name => $field) {
+    foreach(array_keys($name_fields) as $name) {
+      if(isset($field[$name])) {
+        $data[$field_name][$name]['filter'] = array(
+          'field' => $name,
+          'table' => key($name_fields[$name]['storage']['details']['sql'][FIELD_LOAD_CURRENT]),
+          'handler' => 'name_handler_filter_name_fulltext',
+          'field_name' => $name,
+          'allow_empty' => TRUE,
+        );
+      }
+    }
+  }

Needs doxygen comments. Formatting problem. Missing spaces after "if" and "foreach".

Actually we need to add check to exposed_validate if operator is selected. Or to set default operator.

Here is a new patch iteration with fixed issues described in comments above.

+++ b/name.views.incundefined
@@ -0,0 +1,25 @@
+/**
+ * Implements hook_views_data_alter().
+ */

Can we extend this comment and add "Add new views name fulltext handler for every name field."

+++ b/name.views.incundefined
@@ -0,0 +1,25 @@
+  foreach (field_info_fields() as $field_name => $field) {
+    if($field['module'] == 'name') {

Formatting issue space after "if"

Everything else I am happy with! Great work!

Please reroll the patch so I will mark it RTBC.

+++ b/includes/name_handler_filter_name_fulltext.inc
@@ -0,0 +1,105 @@
+  function option_definition() {
+    $options = parent::option_definition();
+
+    $options['operator']['default'] = 'contains';
+
+    return $options;

Remove the extra lines.

+++ b/includes/name_handler_filter_name_fulltext.inc
@@ -0,0 +1,105 @@
+   * Build strings from the operators() for 'select' options

Add period at end of comment.

+++ b/includes/name_handler_filter_name_fulltext.inc
@@ -0,0 +1,105 @@
+  function operator_options($which = 'title') {
+    $options = array();
+    foreach ($this->operators() as $id => $info) {
+      $options[$id] = $info[$which];
+    }
+
+    return $options;

Could remove this extra line.

+++ b/includes/name_handler_filter_name_fulltext.inc
@@ -0,0 +1,105 @@
+   * Provide a simple textfield for equality

Add period at end of comment.

+++ b/name.module
@@ -1074,3 +1074,10 @@ function name_process_name_component($name, $data_key = 'user_data', $letters =
+  return array("version" => "3.0");

I'd use single quotes - not essential.

I think Yuriy covered my other issues :)

I'm just having a look for the first time.

False matches (from a human perspective)

I have not tested the patch, but looking at these two lines, suggest that partial matches will occur within a string:

$fulltext_field = "CONCAT({$field}_title, ' ', {$field}_given, ' ', {$field}_middle, ' ', {$field}_family, ' ', {$field}_generational, ' ', {$field}_credentials)";

    foreach($words as $word) {
      $placeholder =  $this->placeholder();
      $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '%' . db_like($word) . '%'));
    }

So searching for the name "cdefg" using "cd", there would be a hit on "abcd" too.

Normally I would use a conditional OR, IE:

LOWER(name) LIKE LOWER('abc%') OR LOWER(name) LIKE LOWER('% abc%')

But I think that this can be simplified to just "% :placeholder%%" if we change the CONCAT to:

$fulltext_field = "CONCAT(' ', {$field}_title, ' ', {$field}_given, ' ', {$field}_middle, ' ', {$field}_family, ' ', {$field}_generational, ' ', {$field}_credentials)";

And modify the condition to:

    foreach($words as $word) {
      $placeholder =  $this->placeholder();
      $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '% ' . db_like($word) . '%'));
    }

PostgreSQL

Don't use this much, and I haven't even got this installed, but I know that LIKE is case sensitive.

So pushing the above changes one step further:

$fulltext_field = "LOWER(CONCAT(' ', {$field}_title, ' ', {$field}_given, ' ', {$field}_middle, ' ', {$field}_family, ' ', {$field}_generational, ' ', {$field}_credentials))";

The search terms are already converted:

$words = explode(' ', decode_entities(drupal_strtolower($this->value)));

Really nice work everyone!

Thanks for review guys, here is new iteration of patch. How it is ok ;)

The same patch with space at the end of line fixed.

@Alan D. thanks for a nice notice re LIKE in postgres. We can fix it fast as you suggested.

Regarding the human search, it was exactly as in first iteration of patch, which @kevee made, but I forgot to do same thing when was updating patch with a new operators. I think it will be easy to update. Thanks for a comment!

I think @Alan D. has very good point about converting words to lowercase. In the meanwhile I would suggest to take native views as example i.e. no conversion at all. See http://drupalcode.org/project/views.git/blob/refs/heads/7.x-3.x:/handler...

So lets not do strtolower() but also trim words:

<?php
$words = preg_split('/ /', $words, -1, PREG_SPLIT_NO_EMPTY);
foreach($words as $word) {
   $placeholder =  $this->placeholder();
   $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '%' . db_like(trim($word, " ,!?")) . '%'));
}
?>

In this way even for PostgreSQL behavior will be the same for usual strings and our name field fulltext field. Search will be case sensitive.

Well, they used case transformation in 6.x-3.x (http://drupalcode.org/project/views.git/blob/refs/heads/6.x-3.x:/handler...) but no it 7.x-3.x.

What about making all concat string lower, I could not find how to do this actually, cause there is no such commands in this list: http://drupal.org/node/773090. It means that it could work for couple databases but not all, so it is better don't use strtolower.

@ygerasimov, agreed with you, that we don't need to transform sting to lower, however trimming " ,!?" is not very helpful, cause it is a name, which is typically does not contain such characters, but let's use. Actually trimming from commas is good idea :) Also following change will be helpful:

    $words = $phrase ? array($words) : preg_split('/ /', $this->value, -1, PREG_SPLIT_NO_EMPTY);

@Alan D. and @ygerasimov, please, reviewnew generation of patch. It contains human search and trimming improvements.

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,110 @@
+    $words = $phrase ? array($words) : preg_split('/ /', $this->value, -1, PREG_SPLIT_NO_EMPTY);

$phrase is undefined. Please keep it simple:

$words = preg_split('/ /', $this->value, -1, PREG_SPLIT_NO_EMPTY);

Case insensitive searches

Right, a small step in my D7 learning curve:

@see DatabaseConnection_pgsql::mapConditionOperator()

http://api.drupal.org/api/drupal/includes--database--pgsql--database.inc...

I think that we should do this, as every string search in core modules use condition($a, $b, 'LIKE') and is the expected behavior.

-      $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '%' . db_like($word) . '%'));
+      $where->condition($fulltext_field, '% ' . db_like($word) . '%', 'LIKE');

Special characters

We can not trim these off. This module has been used in many non-name based usages, such as a container for course information that can have special codes.

Explode

I think that we can keep this simple for the moment, we can add a quote aware split latter if required.

> I think that we should do this, as every string search in core modules use condition($a, $b, 'LIKE') and is the expected behavior.

Unfortunately condition works for the fields only, but we are having expression. Also using $where variable we can control if we want to add condition with AND or OR operator. This is very similar to how default string filter works.

> We can not trim these off. This module has been used in many non-name based usages, such as a container for course information that can have special codes.

Really, so it is not a problem. Reverted.

> I think that we can keep this simple for the moment, we can add a quote aware split latter if required.

Well, the reason I used it is automatically timing the words with a spaces. The code actually was taken from the string filter in views 7.x-3.x so will work fine.

Here is a patch, and I think it is ready for commit now!

Now it is. Updated the way of spiting words.

New issue, I have not enabled a couple of the components and these are NULL values in the database. So CONCAT() fails on MySQL unless every field is populated.

Going back to SQL-92, it should be save using the COALESCE() function. Note that CONCAT() is not from these old standards.

-    $fulltext_field = "CONCAT(' ', {$field}_title, ' ', {$field}_given, ' ', {$field}_middle, ' ', {$field}_family, ' ', {$field}_generational, ' ', {$field}_credentials)";
+    $fulltext_field = "LOWER(CONCAT(' ', COALESCE({$field}_title, ''), ' ', COALESCE({$field}_given, ''), ' ', COALESCE({$field}_middle, ''), ' ', COALESCE({$field}_family, ''), ' ', COALESCE({$field}_generational, ''), ' ', COALESCE({$field}_credentials, '')))";

Sorry, drop the lower:

+    $fulltext_field = "CONCAT(' ', COALESCE({$field}_title, ''), ' ', COALESCE({$field}_given, ''), ' ', COALESCE({$field}_middle, ''), ' ', COALESCE({$field}_family, ''), ' ', COALESCE({$field}_generational, ''), ' ', COALESCE({$field}_credentials, ''))";

I think that we should do this, as every string search in core modules use condition($a, $b, 'LIKE') and is the expected behavior.

Unfortunately condition works for the fields only, but we are having expression. Also using $where variable we can control if we want to add condition with AND or OR operator. This is very similar to how default string filter works.

Can you expand one this? I tried a simple un-grouped filter and it worked. An AND group with inner OR then tested with an inner AND and they all worked. (As far as I can tell, they both end up in the same DB object parameter, it is just that the condition() one just has more fields and is rendered slightly differently)

How do you trigger an error with this? And yes, the part that is being overridden here follows views, but as per a change last year, the others do not:

 
   function op_starts($field) {
-    $placeholder = $this->placeholder();
-    $this->query->add_where_expression($this->options['group'], "$field LIKE $placeholder", array($placeholder => db_like($this->value) . '%'));
+    $this->query->add_where($this->options['group'], $field, db_like($this->value) . '%', 'LIKE');
   }
 

As from the views issue #1425184: views_handler_filter_string and case sensitivity: any word or all words search, it looks like that was a bug that was not addressed in #1029534: PostgreSQL case sensitivity in filter.

@Allan D.

Right, COALESCE is from old ANSI SQL, so we need to use it.

According to http://drupal.org/node/1029534 we need to fix following part of code to allow cross db work, replace add_where_expression with add_where:

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,110 @@
+    $this->query->add_where_expression($this->options['group'], "$fulltext_field LIKE $placeholder", array($placeholder => '% ' . db_like($this->value) . '%'));

And yeah we need to deal with this. We could replace where to condition, so LIKE should be processed the same way in different DBs:

+++ b/includes/name_handler_filter_name_fulltext.incundefined
@@ -0,0 +1,110 @@
+      $placeholder =  $this->placeholder();
+      $where->where("$fulltext_field LIKE $placeholder", array($placeholder => '% ' . db_like($word) . '%'));

Looks like I have excpected issue with add_where, but not condition. Let me try it.

Well no, I am still having issues with both condition and add_where.

So following code

  function op_word($fulltext_field) {
    $where = $this->operator == 'word' ? db_or() : db_and();

    // Don't filter on empty strings.
    if (empty($this->value)) {
      return;
    }

    $words = preg_split('/ /', $this->value, -1, PREG_SPLIT_NO_EMPTY);
    foreach($words as $word) {
      $where->condition($fulltext_field, '% ' . db_like($word) . '%', 'LIKE');

    }

    $this->query->add_where($this->options['group'], $where);
  }
}

Generates query like this

WHERE (( (crm_core_contact_type.disabled = '0') AND( (CONCATCOALESCEfield_data_contact_name.contact_name_titleCOALESCEfield_data_contact_name.contact_name_givenCOALESCEfield_data_contact_name.contact_name_middleCOALESCEfield_data_contact_name.contact_name_familyCOALESCEfield_data_contact_name.contact_name_generationalCOALESCEfield_data_contact_name.contact_name_credentials LIKE '% Vadim%' ESCAPE '\\') )))

@Allan D. Can you provide the working code of what you described in comment #32 please.

Not sure if this was it or not, I was playing around with querying individual components rather than the entire concat(), but the performance was really really bad:

IE: Best results but terrible performance:

$condition
foreach ($words as $word) {
  $or = db_or()
  foreach {$components as $component) {
    $or->condition($component, $word, 'LIKE');
  }
  $condition->condition($or)
}

Looking back into my local history, I think that this was the last working example, or at least this was the last time that I saved on that day. It still had the LOWER(), but I can not see that this would save the query from being stripped of the brackets as per your example. Note that I was only testing different word / allwords combo.

/**
 * @file
 * Field handler to provide simple renderer that allows linking to a entity.
 */
class name_handler_filter_name_fulltext extends views_handler_filter {
  var $always_multiple = TRUE;

  function option_definition() {
    $options = parent::option_definition();
    $options['operator']['default'] = 'contains';
    return $options;
  }

  /**
   * This kind of construct makes it relatively easy for a child class to add or
   * remove functionality by overriding this function and adding/removing items
   * from this array.
   */
  function operators() {
    return array(
       'contains' => array(
        'title' => t('Contains'),
        'short' => t('contains'),
        'method' => 'op_contains',
        'values' => 1,
      ),
      'word' => array(
        'title' => t('Contains any word'),
        'short' => t('has word'),
        'method' => 'op_word',
        'values' => 1,
      ),
      'allwords' => array(
        'title' => t('Contains all words'),
        'short' => t('has all'),
        'method' => 'op_word',
        'values' => 1,
      ),
    );
  }


  /**
   * Build strings from the operators() for 'select' options.
   */
  function operator_options($which = 'title') {
    $options = array();
    foreach ($this->operators() as $id => $info) {
      $options[$id] = $info[$which];
    }
    return $options;
  }

  /**
   * Provide a simple textfield for equality.
   */
  function value_form(&$form, &$form_state) {
    $form['value'] = array(
      '#type' => 'textfield',
      '#size' => 15,
      '#default_value' => $this->value,
      '#attributes' => array('title' => t('Enter the name you wish to search for.')),
      '#title' => empty($form_state['exposed']) ? t('Value') : '',
    );
  }

  /**
   * Add this filter to the query.
   *
   * Due to the nature of fapi, the value and the operator have an unintended
   * level of indirection. You will find them in $this->operator and
   * $this->value respectively.
   */
  function query() {
    $name_table = $this->ensure_my_table();
    $field = "$this->table_alias.$this->real_field";
    $fulltext_field = "LOWER(CONCAT(' ', COALESCE({$field}_title, ''), ' ', COALESCE({$field}_given, ''), ' ', COALESCE({$field}_middle, ''), ' ', COALESCE({$field}_family, ''), ' ', COALESCE({$field}_generational, ''), ' ', COALESCE({$field}_credentials, '')))";

    $info = $this->operators();
    if (!empty($info[$this->operator]['method'])) {
      $this->{$info[$this->operator]['method']}($fulltext_field);
    }
  }

  function op_contains($fulltext_field) {
    $placeholder = $this->placeholder();
    $this->query->add_where_expression($this->options['group'], "$fulltext_field LIKE $placeholder", array($placeholder => '%' . db_like($this->value) . '%'));
  }

  function op_word($fulltext_field) {
    $where = $this->operator == 'word' ? db_or() : db_and();

    $words = explode(' ', decode_entities(drupal_strtolower($this->value)));
    foreach($words as $word) {
      $placeholder =  $this->placeholder();
      $where->condition($fulltext_field, '% ' . db_like($word) . '%', 'LIKE');
    }
    $this->query->add_where($this->options['group'], $where);
  }
}

Checked once again, adding complex field to condition() or add_where() is not working.

This is how add_where_expression works:

http://drupalcontrib.org/api/drupal/contributions!views!plugins!views_pl...

This is how add_where works:

http://drupalcontrib.org/api/drupal/contributions%21views%21plugins%21vi...

Here follows to magic functions which, are executed after add_where or add_where_expression to build a query. We need to investigate them to find a correct solution:

http://drupalcontrib.org/api/drupal/contributions%21views%21plugins%21vi...
http://drupalcontrib.org/api/drupal/drupal%21includes%21database%21query...
http://drupalcontrib.org/api/drupal/drupal%21includes%21database%21query...

As we can see from DatabaseCondition::compile (http://drupalcontrib.org/api/drupal/drupal%21includes%21database%21query...) if operator is null (e.g. we added it via where(), which was cause we called add_where_expression()) then there is no operator replacement happens:

      if (empty($condition['operator'])) {
        // This condition is a literal string, so let it through as is.
        $condition_fragments[] = ' (' . $condition['field'] . ') ';
        $arguments += $condition['value'];
      }

otherwise if we set operator manually then it processed depending on a database version

          // For simplicity, we treat all operators as the same data structure.
          // In the typical degenerate case, this won't get changed.
          $operator_defaults = array(
            'prefix' => '', 
            'postfix' => '', 
            'delimiter' => '', 
            'operator' => $condition['operator'], 
            'use_value' => TRUE,
          );
          $operator = $connection->mapConditionOperator($condition['operator']);
          if (!isset($operator)) {
            $operator = $this->mapConditionOperator($condition['operator']);
          }
          $operator += $operator_defaults;

The reason is of we have modified version of field if we add complex field is in following code:

$condition_fragments[] = ' (' . $connection->escapeField($condition['field']) . ' ' . $operator['operator'] . ' ' . $operator['prefix'] . implode($operator['delimiter'], $placeholders) . $operator['postfix'] . ') ';

escapeField() does this job, see http://drupalcontrib.org/api/drupal/drupal%21includes%21database%21datab...

The only possible way how we can do is to map LIKE operator manually.

Which bit did you try again? Did you modify the code at all?

The following would need changing: (untested)

  function op_contains($fulltext_field) {
    // Not sure if we can $this->query->condition() here, so...
    $where = db_and();
    $where->condition($fulltext_field, '% ' . db_like($this->value) . '%', 'LIKE');
    $this->query->add_where($this->options['group'], $where);
  }

Correct, the code you have mentioned is not working. The fulletext filed is parsed to CONCATCOALESCEfield_data_contact_name.contact_name_titleCOALESCEfield_data_contact_name.contact_name_givenCOALESCEfield_data_contact_name.contact_name_middleCOALESCEfield_data_contact_name.contact_name_familyCOALESCEfield_data_contact_name.contact_name_generationalCOALESCEfield_data_contact_name.contact_name_credentials by escapeField() function.

Hence we need to use add_where_expression and lower both fulltext field and search terms.

Here is a new patch!

@Alan D., could you review the patch, please? Also please see my reply here http://drupal.org/node/1425184#comment-5615552

Lost my test setup. If someone reviews I'll commit this :)

Hi,

I have reviewed dealancer's code. The search filter works with name components (phrase) when choosing operator of contains or contains any for full name search, i.e. John Smith

Thanks for all the hard work guys. This has been committed to 7.x-1.x.

Reopen if there are issues with the existing code, or create a new thread for additional features / bugs (Like lower thingee, but I'm ignoring this for the moment).

Reopened for back-porting to D6.

Marked #1518158: Fullname views' filter for Drupal6 as a duplicate.

Patches welcome.

Actually our query construction with COALESCE is not working so good. It could not find both names and surnames like "Barack Obama". because there is no middle name, so "Barack Obama" != "Barack Obama". If middle name is skipped extra space is added.

The solution is to use CASE function.

I think there is additional space in line 92 of the patch #42 in '% '
array($placeholder => '% ' . db_like($value) . '%'));
array($placeholder => '%' . db_like($value) . '%'));

The issue with regards to not being able to search both first and last names is crippling and makes this filter unusable.

Should a new issue be opened for that or can this be addressed here?

Okay, so I have fiddled about a little bit more, and it does work if you change the operator to "Contains any word" or "Contains all words".

This isn't exactly obvious when first using the filter, especially when it is a "full name" filter, but it only works out of the box when using only part of a full name.

Flagging as a duplicate of #2090789: Fulltext Filter does not match surname which seems to be the same and I can't replicate.

Also Views Global combine filter is an option here, add the given, middle and family components to that and expose it.