It would be good for user fields to have autocomplete fields for usernames.

Files: 
CommentFileSizeAuthor
#39 2110315-39--empty_entity_filters.patch3.1 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]
#37 interdiff-36-32.diff3.92 KBufku
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch interdiff-36-32_0.diff. Unable to apply patch. See the log in the details link for more information.
[ View ]
#36 2110315-36--views_taxonomy_term_filter.patch3.99 KBufku
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]
#33 2110315-33--empty_entity_filters.patch1.01 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]
#32 2110315-32--views_taxonomy_term_filter.patch2.38 KBufku
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]
#25 2110315-25--views_taxonomy_term_filter.patch1.53 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 356 pass(es).
[ View ]
#24 2110315-24--views_taxonomy_term_filter.patch977 bytesdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 356 pass(es).
[ View ]
#18 2110315-18--views_taxonomy_term_filter.patch4.73 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#11 2110315-no-taxonomy-select.diff14.83 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#9 2110315-9--views_entity_filters.patch19.79 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#8 2110315.diff19.48 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#7 2110315-7--views_entity_filters.patch21.09 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#6 2110315.diff20.86 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#6 2110315-interdiff.txt21.63 KBdrumm
#5 2110315-5--views_user_filter.patch8.77 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#5 2110315-5--views_user_filter--interdiff.txt4.09 KBdrunken monkey
#4 2110315.diff7.97 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#3 2110315-3--views_user_filter.patch7.93 KBdrunken monkey
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#2 2110315.diff5.93 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]
#1 2110315.diff5.91 KBdrumm
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Comments

drumm’s picture

Status:Active» Needs review
StatusFileSize
new5.91 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

And here is a patch for that. It is based on the equivalent filter in Views and I've tested it with Search API DB.

drumm’s picture

StatusFileSize
new5.93 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Improved condition added in search_api_views.views.inc so this works for list types.

drunken monkey’s picture

StatusFileSize
new7.93 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Thanks a lot for the suggestion and the patch! Having a dedicated filter for users makes sense, of course.
However, there were several errors in your patch regarding use cases other than the one on d.o – for instance, non-exposed filters and exposed operators. Also, the code style and comments didn't completely follow the coding standards.
A revised patch is attached, please test/review!

While working on this, I also noticed an unrelated bug in our base filter handler: #2111273: Fix Javascript states for exposed filter operator. (Probably won't concern the d.o use case, though.)

drumm’s picture

StatusFileSize
new7.97 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Yes, I did indeed go through this somewhat quickly. Thanks for all the improvements. They all look good.

When testing, I did get a notice from admin_summary() when the default value is empty. $this->value is an empty string rather than array. Attached is a patch wrapping that line in if (!empty($value)).

drunken monkey’s picture

StatusFileSize
new4.09 KB
new8.77 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Ah, you're right, thanks!
However, I guess it'd be even better to map the UIDs to the names in the summary. I also noticed that that conversion didn't take place for exposed filters with defaults in the live preview. It's really tiring how many cases you have to accomodate in Views …
Anyways, improved patch attached.

drumm’s picture

Title:Specialized filter for users» Specialized filter for users and terms
StatusFileSize
new21.63 KB
new20.86 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

As suggested in #2111899: Specialized filter for taxonomy terms, this moves common code to a parent class, SearchApiViewsHandlerFilterEntity. SearchApiViewsHandlerFilterUser is especially simplified. And I can use this to go back and simplify http://drupalcode.org/project/project_issue.git/blob/refs/heads/7.x-2.x:... which is for project names.

Outstanding from #2111899: Specialized filter for taxonomy terms:

Also, I don't know whether you realize this, but even with the patch all taxonomy term fields not using autocomplete will use the SearchApiViewsHandlerFilterOptions handler class, since those term reference fields will have an options list. Since your filter also allows selection via a select list, this is probably unnecessary now, especially since #1832334: Fix performance issues of Views options filter handler for huge options lists will probably get rid of the options list caching. I'll notify the people there of this issue, let's see what their opinion is.

So a bunch of code in SearchApiViewsHandlerFilterTaxonomyTerm can likely still be removed, which would be great.

drunken monkey’s picture

StatusFileSize
new21.09 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Thanks a lot for creating this new patch with a common base class! As you say, this also adds additional value for other contrib modules, and it definitely makes the code cleaner.

Actually, I would have thought even more code could be generalized – e.g., if you take the ID and label column from the entity_keys entity info setting, you could likely make most of the label <=> ID conversion code common. Just the vocabulary for terms and the anonymous user would have to be added, but the base class could already take that into account. (Also, users don't have the label set as an entity key, but rather use a label callback, so that would also have to be added manually somewhere. Maybe the two columns could be retrieved in init(), making overriding easy. Or we could use a dedicated method, of course.)

Speaking of anonymous users, though: Why does the code just use the literal string "Anonymous" instead of the anonymous variable with a translated default? Isn't that very confusing for sites in other languages, or who override the name? Or am I missing some behind-the-scenes magic?

So a bunch of code in SearchApiViewsHandlerFilterTaxonomyTerm can likely still be removed, which would be great.

I don't know, how would you mean we could do that? Do you mean removing the option to have a select list from the filter handler? That could of course make sense, yes …
Though, as said, providing this functionality in the base class, and thus also offering it for users (which could make sense on sites with a small set of possible authors), could also be a nice feature. And even for tagging term fields, a select list could make sense if the site is small.
Also, the code in the options filter handler doesn't allow you to use a select list without hierarchy, and also doesn't support autocomplete. So I'd say using this new handler for all taxonomy term references would probably be the better option.
The only "feature" the options handler has compared to the new one is caching of the options list, which could be a very small performance gain. (But by the way, how many of the issue fields are term references? If several are, a performance comparison between the two options might make sense – we are searching for performance data, and d.o would be perfect for that.)

Further comments/nitpicking:

  1. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +    $options['limit'] = array('default' => TRUE, 'bool' => TRUE);

    I don't see any config form for "limit". How is that set, and what does it do?

  2. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +    if ($this->options['limit']) {
    +      // We only do this when the form is displayed.
    +      if (empty($this->definition['vocabulary'])) {
    +        $first_vocabulary = reset($vocabularies);
    +        $this->definition['vocabulary'] = $first_vocabulary->machine_name;
    +      }
    +
    +      if (empty($this->definition['vocabulary'])) {
    +        $form['vocabulary'] = array(
    +          '#type' => 'radios',
    +          '#title' => t('Vocabulary'),
    +          '#options' => $options,
    +          '#description' => t('Select which vocabulary to show terms for in the regular options.'),
    +          '#default_value' => $this->definition['vocabulary'],
    +        );
    +      }
    +    }

    I don't understand this bit at all. Apart from the fact that I don't understand the "limit" option – doesn't the first if block mean that the second one will only be executed if there are no vocabularies defined – making the whole element useless?

  3. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +      '#dependency' => array('radio:options[type]' => array('select')),

    As far as I know, and according to #1595022: Convert dependency to states, #dependency shouldn't be used anymore in favor of #states (though I admit the syntax for that is more complicated).

  4. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +    if ($missing && !empty($this->options['error_message'])) {
    +      form_error($form, format_plural(count($missing), 'Unable to find term: @terms', 'Unable to find terms: @terms', array('@terms' => implode(', ', array_keys($missing)))));
    +    }
    +    elseif ($missing && empty($this->options['error_message'])) {
    +      $tids = array(0);
    +    }
    +

    A nested if would make the code considerably more readable here.

  5. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +    elseif ($missing && empty($this->options['error_message'])) {
    +      $tids = array(0);
    +    }

    Is that really the best behavior? Shouldn't we just add 0, instead of replacing all found TIDs with just that one?
    If the operator is "=", this will show an empty result – which is probably exactly the expected behavior. However, for "<>", all other entered terms will be ignored, too!

  6. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +    $form['error_message'] = array(
    +      '#type' => 'checkbox',
    +      '#title' => t('Display error message'),
    +      '#default_value' => !empty($this->options['error_message']),
    +    );

    This should have some description.

  7. +++ b/contrib/search_api_views/includes/handler_filter_taxonomy_term.inc
    @@ -0,0 +1,310 @@
    +      $form['value'] = array(
    +        '#type' => 'select',
    +        '#title' => $this->options['limit'] ? t('Select terms from vocabulary @voc', array('@voc' => $vocabulary->name)) : t('Select terms'),
    +        '#multiple' => TRUE,
    +        '#options' => $options,
    +        '#size' => min(9, count($options)),
    +        '#default_value' => $default_value,
    +      );

    Completely setting $form['value'] here overwrites/removes general improvements like the #states for the "empty" operators and removing the title in the exposed form (though Views admittedly does that automatically anyways).

    Also, the taxonomy term filter should also (like the user filter) only use ids_to_strings() to set the default value if it actually used – i.e., not if the exposed form has already been submitted. It's probably only a minor performance improvement, but still better than nothing.

All of the comments I knew how to fix are contained in the attached patch, along with some documentation fixes/beautification. (I guess we don't really need interdiffs, you'll probably also use a dedicated branch, right? Otherwise, I can of course continue providing them.)

drumm’s picture

StatusFileSize
new19.48 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Actually, I would have thought even more code could be generalized – e.g., if you take the ID and label column from the entity_keys entity info setting, you could likely make most of the label <=> ID conversion code common. Just the vocabulary for terms and the anonymous user would have to be added, but the base class could already take that into account. (Also, users don't have the label set as an entity key, but rather use a label callback, so that would also have to be added manually somewhere. Maybe the two columns could be retrieved in init(), making overriding easy. Or we could use a dedicated method, of course.)

I tried going down this path a bit today. For validate_entity_strings(), it seems like the way to go would be using EntityFieldQuery, this would build in any standard access checking into the query in a generic way. That only returns IDs, so we would have to load the entities and match up the arrays to get to any remaining $missing for a nice error message. And there would need to be an alter step where taxonomy term could add in its vid restriction.

ids_to_strings() is already pretty simple. It wouldn't change the complexity much to make it more generic.

Do you mean removing the option to have a select list from the filter handler? That could of course make sense

Ah, I misunderstood your comments in #2111899: Specialized filter for taxonomy terms. I'm not using the select list on Drupal.org, issues tags is the only taxonomy for issues. The select list has been along for the ride from being copied out of Views's views_handler_filter_term_node_tid. #options is keyed by tid there, so that's why #default_value is overwritten. Having that as an option list managed by SearchApiViewsHandlerFilterOptions seems like it may be better, if possible.

Otherwise, I made some progress on everything else you mentioned:

Replaced hard-coded anonymous with variable_get().
Now assuming there is a single vocabulary with every taxonomy_term index, so it will be set and limit is discarded.
Moved setting #default_value up to the parent class.

drunken monkey’s picture

StatusFileSize
new19.79 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Great, thanks!
What I noticed, though, is that you're using reduce_value_options() in the term filter, where it isn't available. I've now added it there, it's pretty trivial anyways.

Do you still want to work on the two mentioned issues (making ID <=> label conversions more generic and either removing or generalizing the "select list" option), or would you prefer we just ensure everything works now and commit it?
Especially the former would be hard to do later, though, so we'd have to be sure we don't want that.

drumm’s picture

Patch looks good. I would of course like to concentrate on committing it.

I don't see a huge benefit to making ID <=> label conversions more generic. Maybe it will emerge as search_api_views adds more entity types, but that's a separate issue.

I'm okay with leaving the taxonomy select list as-is or removing it. Removing would be a big simplification, and it isn't needed for Drupal.org. But I don't want to rip it out if it is generally good for Search API.

drumm’s picture

StatusFileSize
new14.83 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Attached is a version with no taxonomy select, if we want to run with that.

drunken monkey’s picture

I don't see a huge benefit to making ID <=> label conversions more generic. Maybe it will emerge as search_api_views adds more entity types, but that's a separate issue.

I guess you're right. I thought that it would be harder to implement, then, as contrib modules might rely on the generic class already – but since the method is abstract now, all subclasses must implement it from scratch anyways. If we later on add a generic implementation there, subclasses won't notice it unless they choose to use it.
So, nothing against leaving it like this for now.

As for the select/autocomplete option, I don't really know either. Maybe someone who's using this will weigh in. Otherwise, I'll just commit one of them next Monday or so and hope for the best. ;)

In any case, thanks a lot again for your work!

tvn’s picture

Any chance you could commit this earlier? Today or tomorrow? This issue is kind of blocking other our before-launch to-dos.

drunken monkey’s picture

Status:Needs review» Fixed

Damn, I'm sorry, I didn't check the issues on the weekend. Please send me a mail in such situations – for d.o I'm more than willing to make an exception to the "no nagging about issues via mail" rule.
Anyways, I committed the patch in #9 now – having the select list available, too, might be useful for some users, and it gives us the option to always use this filter handler instead of the options list-based one for taxonomy terms.

Thanks again for your work, Neil, and sorry I blocked you guys!

tvn’s picture

No worries, and thanks a lot for all your help with this issue!

s_leu’s picture

Status:Fixed» Active

I have some problems with this change with an exposed filter of type "All parent terms (indexed)". After updating from search api 1.4 to the latest recommended release, the filter suddenly stopped to work.

Debugging the code in SearchApiViewsHandlerFilterTaxonomyTerm::value_form() shows that the base for loading the terms, $this->definition['vocabulary'], is empty. That worked well before i updated. Seems like this new term filter is buggy.

drumm’s picture

Issue tags:-Drupal.org D7

this does not affect Drupal.org

drunken monkey’s picture

Status:Active» Needs review
StatusFileSize
new4.73 KB
PASSED: [[SimpleTest]]: [MySQL] 298 pass(es).
[ View ]

Oh, damn, I can't believe I overlooked that … Thanks a lot for reporting this!

Does the attached patch fix this for you? It attempts to retrieve the field information of the containing entity for the parent and parents_all properties. If that's not available, no vocabulary is saved for the filter and the functionality gets reduced accordingly (only a plain select list available). Later, we could also implement our own taxonomy term callback for that case (i.e., use all vocabularies) – but that's not necessary right away, I think. We'd have to see if having a term reference without a taxonomy defined (or defined some way we can't detect it) is really a use case.

In any case, thanks again for reporting, and sorry I committed this so hastily, breaking your site!

drunken monkey’s picture

Assigned:drumm» Unassigned
drumm’s picture

#18 looks okay from reading through the patch file.

s_leu’s picture

Status:Needs review» Needs work

Can confirm the patch in #18 fixes the problem with the empty $definition['vocabulary']. Unfortunately there is still something different from the behavior of the corresponding filter in search api 7.x-1.4.

I'm using the filter as exposed and the form should also work when there is no value for that filter submitted. This was working fine in 1.4. But In the current version there is something wrong in SearchApiViewsHandlerFilterTaxonomyTerm::validate_entity_strings() now. I just tried to make a few changes quickly to see if i could make it work and came up with this:

<?php
 
protected function validate_entity_strings(array &$form, array $values) {
    if (empty(
$values)) {
      return array();
    }

   
$tids = array();
   
$names = array();
   
$missing = array();
    foreach (
$values as $value) {
     
$missing[strtolower($value)] = TRUE;
     
$names[] = $value;
    }

    if (!
$names) {
      return
FALSE;
    }

   
$query = db_select('taxonomy_term_data', 'td');
   
$query->innerJoin('taxonomy_vocabulary', 'tv', 'td.vid = tv.vid');
   
$query->fields('td');
   
$query->condition('td.tid', $names);
    if (!empty(
$this->definition['vocabulary'])) {
     
$query->condition('tv.machine_name', $this->definition['vocabulary']);
    }
   
$query->addTag('term_access');
   
$result = $query->execute();
    foreach (
$result as $term) {
      unset(
$missing[strtolower($term->tid)]);
     
$tids[] = $term->tid;
    }

    if (
$missing) {
      if (!empty(
$this->options['error_message'])) {
       
form_error($form, format_plural(count($missing), 'Unable to find term: @terms', 'Unable to find terms: @terms', array('@terms' => implode(', ', array_keys($missing)))));
      }
      else {
       
// Add a bogus TID which will show an empty result for a positive filter
        // and be ignored for an excluding one.
       
$tids[] = 0;
      }
    }

    return
$tids;
  }
?>

That way it is working as expected again. Sorry i don't have time to make a correct patch. This is for sure not the correct solution but it might let you see what is going on actually. The main problem is that the current code in this method expects $values to be an array of term names. What i get is an array of tids though.

Would be great if you could have a look at this again.

drunken monkey’s picture

Good to know the patch works – I now committed it to fix at least that part.

For the new problem: could you please describe in more detail what you're doing, what settings you're using and what goes wrong?

s_leu’s picture

Ok so here is what i am trying to do. I have a "All parent terms (indexed)" filter that is exposed on a view that is items of a certain search index. The field is part of that index and enabled as well in the fields of the index.

Due to exposure of the filter i need it to accept empty values and the empty behavior should be to return all results for the view. Unfortunately what happens is that no results are shown, the autocomplete field is marked red and the message "Unable to find term:" gets displayed.

It is caused by the code in SearchApiViewsHandlerFilterTaxonomyTerm::validate_entity_strings() . That function expects the $values argument to be an array of term names. But instead what i get there is an array of tids which of course will cause that function to return nothing. Please let me know if you need any more details.

drunken monkey’s picture

Issue summary:View changes
Status:Needs work» Needs review
StatusFileSize
new977 bytes
PASSED: [[SimpleTest]]: [MySQL] 356 pass(es).
[ View ]

Sorry, I can't reproduce this. Everything seems to work fine. Maybe try to get to the bottom of why validate_entity_strings() receives IDs instead of names, which really shouldn't happen. However, I don't see why this should even be relevant to the problem you describe – if you have issues with empty form input, then the array passed to validate_entity_strings() will be empty anyways.

One problem I noticed, however (even though I've made sure this is solved before committing) is that the default value of the "Value" field in the admin UI will again not always be properly set – i.e., IDs instead of names.
Views never ceases to shock me – it seems $form_state['input'] is sometimes even populated for new forms! I really haven't got any clue left on how to properly determine when the default value will be used, so the attached patch would change this to have the IDs be always transformed to names. The performance improvement was probably negligible anyways, since setting a default value for an exposed filter isn't often done anyways, I'd guess.

drunken monkey’s picture

StatusFileSize
new1.53 KB
PASSED: [[SimpleTest]]: [MySQL] 356 pass(es).
[ View ]

Ah, now I stumbled across it by accident. Would have been important to know you have "Allow multiple selections" disabled.
The attached patch should fix this, please test!

s_leu’s picture

Status:Needs review» Needs work

Thanks for your effort drunken monkey. I just checked your patch and can confirm that it solved the problem with the empty value of the field. Unfortunately when i try to select a term using autocompletion, the view doesn't display any results at all now.

It just displays the message "Unable to find term: 123". And yes, i have "Allow multiple selections" disabled on that filter.

Hope it helps

drunken monkey’s picture

Good to hear that part works, thanks for testing!
I cannot reproduce the new error you mention, though. It's with the same setup and field as before, and the filter exposed? With the patch, everything really seems to work now for me.
Did you maybe enter a term from another vocabulary, or something?

Colin @ PCMarket’s picture

I came across this issue when trying to solve an issue that was introduced after updating to 7.x-1.9 and was wondering if you could advise if this issue may be related to the issue as i've detailed in https://drupal.org/node/2141741

thanks!

drunken monkey’s picture

Have you tried with the latest dev version of the Search API, too?

Colin @ PCMarket’s picture

Yes I am running 7.x-1.9+33-dev

drunken monkey’s picture

That doesn't seem to be related to this issue at all. I'll comment over in #2141741: Use case with taxonomy/term/% issue with search api 7.x-1.9and facet api.

@ s_leu: Could you reproduce your problem with a newly configured view? If so, please provide details and reproducible steps. Otherwise, we should finally close this issue.

ufku’s picture

Status:Needs work» Needs review
StatusFileSize
new2.38 KB
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]

Here is my observation for term fields:

1- "Term reference" fields:
- handled by "SearchApiViewsHandlerFilterOptions".
- terms are provided as checkboxes in filter configuration.
- no autocomplete option.

2- "Entity Reference"(Taxonomy term) fields:
- handled by "SearchApiViewsHandlerFilterTaxonomyTerm".
- terms (all terms in the site!) are provided in a multiple-select in filter configuration.
- no autocomplete option.

Attached patch:
- makes "Term reference" fields to be handled by "SearchApiViewsHandlerFilterTaxonomyTerm"
- correctly prepopulates "vocabulary" option for "Entity Reference" so extra options(autocomplete,etc.) work as expected.

ps: Not sure if there is an open issue on it, but i think providing a vocabulary selection UI would be nice besides/instead of prepopulating it in hook_views_data().

drunken monkey’s picture

StatusFileSize
new1.01 KB
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]

Thanks for the patch, the suggestion makes sense.
The decision to use the options filter for non-autocomplete term reference fields was a deliberate one, so I'd first need to know whether there is larger consensus that these should be handled by the term-specific filter instead.
Also, if this is the case, we still should just move the if for taxonomy terms up in the chain (probably along with users, for consistency), instead of moving the one for options down. Otherwise, options also become less preferable than, e.g., dates.

We could also consider a different approach, of just adding autocomplete to the options filter as well – should be well possible and could also be handy in other situations. However, it would of course be more work, so I understand if no-one wants to do that. (We'd probably have to abstract our entity filter handler again to also handle other autocomplete-able values.)

+++ b/contrib/search_api_views/search_api_views.views.inc
@@ -221,9 +216,20 @@ function _search_api_views_add_handlers($id, array $field, EntityMetadataWrapper
+        $table[$id]['filter']['vocabulary'] = key($field_info['settings']['handler_settings']['target_bundles']);

Will the 'target_bundles' key always only contain a single value? Otherwise, we should only do this when there is only a single value. Or change $table[$id]['filter']['vocabulary'] to an array, but that would of course be a larger change.

Also, should this maybe also be fixed for the search_api_views_taxonomy_term contextual filter for "Entity reference" fields? It seems that would have the same problem.

Attached is also a re-roll of #25 – however, part of this already seems to have been fixed by #2198791: Indexed Content: Author as Exposed Filter: empty field treated as zero (Anonymous), so I'd first need to know if anyone still has problems with this in the latest version of this module?

ufku’s picture

Will the 'target_bundles' key always only contain a single value? Otherwise, we should only do this when there is only a single value. Or change $table[$id]['filter']['vocabulary'] to an array, but that would of course be a larger change.

Target bundles may contain multiple values. It's unlikely but possible. Limiting to single valued target_bundles would cover most of the sites i guess. There is also another scenario where the handler(term source) is the views module, which may be considered later together with the multiple bundles.

Also, should this maybe also be fixed for the search_api_views_taxonomy_term contextual filter for "Entity reference" fields? It seems that would have the same problem.

Yes, the same vocabulary discovery logic should be added there too.

drunken monkey’s picture

Status:Needs review» Needs work

OK, then please add these two fixes, too.

ufku’s picture

Status:Needs work» Needs review
StatusFileSize
new3.99 KB
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]
new3.99 KB
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]

- Moved the term-if up keeping the options-if in its place as you suggested.

- Moved the vocabulary discovery code to a function.

- Fixed the two issues.

ufku’s picture

StatusFileSize
new3.92 KB
FAILED: [[SimpleTest]]: [MySQL] Unable to apply patch interdiff-36-32_0.diff. Unable to apply patch. See the log in the details link for more information.
[ View ]

Fixed interdiff.

Status:Needs review» Needs work

The last submitted patch, 37: interdiff-36-32.diff, failed testing.

drunken monkey’s picture

Status:Needs work» Needs review
StatusFileSize
new3.1 KB
PASSED: [[SimpleTest]]: [MySQL] 370 pass(es).
[ View ]

Thanks, looks good. Just had to fix the comments a bit.
However, as said, I don't want to change the handler type of non-autocomplete term reference fields without enough consensus on the issue, as this will probably break at least some, if not a lot of, views containing such filters. Does the rest of the fix make sense without that? Or will term-typed entity reference fields then still not be recognized? In the former case, I could commit the attached patch – for the if-reordering I'd need some more feedback here.

ufku’s picture

This works for Entity reference fields.

Term reference fields however needs the if-reordering. I understand your concerns about breaking existing configurations but in my opinion if there were many people using the term filter as options list some of them would have already come here and request dropdown and autocomplete widgets because that's the way a term field works in normal views. I think we should fix this. The sooner the better.

  • drunken monkey committed cdbd157 on 7.x-1.x authored by ufku
    Follow-up to #2110315 by ufku, drunken monkey: Fixed Views field handler...
drunken monkey’s picture

Status:Needs review» Fixed

I understand your concerns about breaking existing configurations but in my opinion if there were many people using the term filter as options list some of them would have already come here and request dropdown and autocomplete widgets because that's the way a term field works in normal views.

On the other hand, you are the first to complain about it the other way.
I have committed the patch from #39 now. To avoid overloading this issue even more, please create a new one for the re-ordering issue, and we can have a "vote" there.

Status:Fixed» Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.