Support term query type for not equal to field [#2083481]

Hi all,

I have a faceted search configured and working great. I have a requirement to enable one facet to exclude all items that match the selected term.

For example, if I have a colors facet and I click "red", the behavior for my requirement would be to exclude all items tagged red. It seems the default behavior of the SearchApiFacetapiTerm query type is to add a query condition to include all items tagged red.

I've solved this temporarily by adding a small conditional in the addFacetFilter function to add a query condition that looks like this:

$query_filter->condition($field, $filter, '<>');

I am curious if it makes sense to extend this query type to support this type of behavior OR if I am missing something and this is in fact supported in some other way.

Thanks.

Nick

Comment	File	Size	Author
#5	2083481-5--facets_exclude_option.patch	5.02 KB	drunken monkey
#5
#3	search_api-support_negating_terms-2083481-3.patch	2.05 KB	nickgs
#3

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Comment #1

drunken monkey

he/him

German

Vienna, Austria

CreditAttribution: drunken monkey commented 6 September 2013 at 16:29

I don't think it's supported at the moment, but we could definitely think about adding it. Maybe as one or two additional options for the "Operator" option. I don't expect it's a very common use case, but might still be handy from time to time, and simple enough to add (I think/hope).

If you'd provide a patch, we could add that feature.

Comment #2

nickgs CreditAttribution: nickgs commented 6 September 2013 at 17:32

Assigned:

Unassigned

» nickgs

Cool, thanks for the quick response. I am not an expert in the architecture of this module stack but I will give this a crack.

Thanks.

Nick

Comment #3

nickgs CreditAttribution: nickgs commented 6 September 2013 at 19:57

File	Size
search_api-support_negating_terms-2083481-3.patch	2.05 KB

Ok here is my first attempt at this feature. I took the approach of adding a new option to the settings form of the SearchApiFacetapiAdapter class. I then check for this setting within the addFacetFilter function and format the query condition accordingly.

One concern I have is how I am adding the form field with this check:

if(is_array($facet['query types']) && in_array('term', $facet['query types'])) {
  //form field here. 
}

I feel like there may be a better approach here but this seems to work well for this type of functionality.

Thanks.

Nick

Comment #4

nickgs CreditAttribution: nickgs commented 6 September 2013 at 19:58

Status:

Active

» Needs review

Comment #5

drunken monkey

he/him

German

Vienna, Austria

CreditAttribution: drunken monkey commented 7 September 2013 at 10:00

File	Size
2083481-5--facets_exclude_option.patch	5.02 KB

Great, thanks a lot for the patch!
It generally looks fine, there's just a few notes for you:

+++ b/contrib/search_api_facetapi/plugins/facetapi/adapter.inc
@@ -250,5 +250,21 @@ class SearchApiFacetapiAdapter extends FacetapiAdapter {
+     $form['global']['negate_results'] = array(
+        '#type' => 'select',
+        '#title' => t('Negate Results'),

I think calling this option "Exclude", and making it a checkbox, like for Views contextual filters, is clearer.

+++ b/contrib/search_api_facetapi/plugins/facetapi/query_type_term.inc
@@ -94,7 +94,16 @@ class SearchApiFacetapiTerm extends FacetapiQueryType implements FacetapiQueryTy
+      if(isset($settings->settings['negate_results']) && $settings->settings['negate_results'] == 1) {

You can just use !empty() here, that's shorter and easier to read. You should usually use that if you want an array key to default to FALSE if it's not present.

+++ b/contrib/search_api_facetapi/plugins/facetapi/query_type_term.inc
@@ -94,7 +94,16 @@ class SearchApiFacetapiTerm extends FacetapiQueryType implements FacetapiQueryTy
+      if(isset($settings->settings['negate_results']) && $settings->settings['negate_results'] == 1) {
+        //add not equal condition.
+        $query_filter->condition($field, $filter, "<>");
+      }
+      else {
+        //add inclusive condition.
+        $query_filter->condition($field, $filter);
+      }

You don't need an if…else statement here, just use a ternary operator for passing the operator:
$query_filter->condition($field, $filter, $exclude ? '<>' : '=');

Also, the other parts of this method should be changed too, at least the one for the "missing" facet ($filter === '!').

Furthermore, please keep to the coding standards for comments – comments should always form complete sentences and end with a period.

I've attached a revised patch which should fix those shortcomings. Please test/review! (I'm sorry, it contains a few small unconnected changes, since I just happened to spot those.)

I feel like there may be a better approach here but this seems to work well for this type of functionality.

I see what you mean, this definitely doesn't feel right, but I can't find a better way either.

Comment #6

nickgs CreditAttribution: nickgs commented 7 September 2013 at 18:33

Status:

Needs review

» Reviewed & tested by the community

Awesome, thanks for the review and improvements. I've tested and it looks to be working well. Nice use of the ternary operator, cleans up the code a bit!

I also think you are correct about the naming of the field, "exclude" sounds clearer. Naming things is always a challenge... :)

Updating status to RTBC.

Thanks.

Nick