Sort autocomplete of issue tags descending by usage count *in the issue metadata form*

The handler used on Drupal.org is from Search API: http://cgit.drupalcode.org/search_api/tree/contrib/search_api_views/incl... That callback looks similar to core's.

Ordering by usage would indeed slow down matches:

mysql> explain SELECT t.tid, t.name FROM taxonomy_term_data t WHERE t.vid = 9 AND t.name LIKE '%need%' LIMIT 10;
+----+-------------+-------+------+------------------------+----------+---------+-------+-------+--------------------------+
| id | select_type | table | type | possible_keys          | key      | key_len | ref   | rows  | Extra                    |
+----+-------------+-------+------+------------------------+----------+---------+-------+-------+--------------------------+
|  1 | SIMPLE      | t     | ref  | vid_name,taxonomy_tree | vid_name | 4       | const | 11397 | Using where; Using index |
+----+-------------+-------+------+------------------------+----------+---------+-------+-------+--------------------------+
1 row in set (0.00 sec)

mysql> explain SELECT t.tid, t.name, count(1) c FROM taxonomy_term_data t LEFT JOIN taxonomy_index i ON i.tid = t.tid WHERE t.vid = 9 AND t.name LIKE '%need%' GROUP BY t.tid ORDER BY c DESC LIMIT 10;
+----+-------------+-------+------+------------------------+-----------+---------+--------------+-------+-----------------------------------------------------------+
| id | select_type | table | type | possible_keys          | key       | key_len | ref          | rows  | Extra                                                     |
+----+-------------+-------+------+------------------------+-----------+---------+--------------+-------+-----------------------------------------------------------+
|  1 | SIMPLE      | t     | ref  | vid_name,taxonomy_tree | vid_name  | 4       | const        | 11397 | Using where; Using index; Using temporary; Using filesort |
|  1 | SIMPLE      | i     | ref  | term_node              | term_node | 4       | drupal.t.tid |     7 | Using index                                               |
+----+-------------+-------+------+------------------------+-----------+---------+--------------+-------+-----------------------------------------------------------+

Drupal.org has 19,135 issue tags, sorting among that many looks doable, 20k isn't actually that much compared to some of the other things Drupal.org does to the DB. It would be okay for us, still answering queries quickly, maybe with some noticeable latency, a fraction of a second. (Sorting by name has effectively zero performance impact compared to the current query.)

this would also help what #2013222: Add "Issue tasks" to project issues and correlate tasks with handbook documentation set out to do (in part)

is this a candidate for a "quick easy win" to get on the d.o priorities list of issues?

Would it also help to add in a sort on i.nid (to get a kind of 'last used') in the mix? Some kinda of secondary sort on 'freshness'.

+1 for this issue regardless:)

One option is to add a configuration option in the views UI settings page admin/structure/views/settings/advanced (defaulting to the current non-sorted by usage order).
and then use that config in https://api.drupal.org/api/views/includes%21ajax.inc/function/views_ajax...

or, project issue tracking module could implement a hook_menu_alter() for
'admin/views/ajax/autocomplete/taxonomy'
'page callback' => 'views_ajax_autocomplete_taxonomy',
to be some other callback function that had the sort we want

I think this would be a great first step. I imagine it has some performance impact, but there should also be ways to mitigate that as much as possible?

Definitely hoping this ends up on the short list!

So, fixing this in drupal 8 core is probably not going to happen if adding sort settings gets categorized as a feature request.
Fixing it in drupal 7 core... would be weird if it can't be fixed in d8 first.

So I think that leaves us with doing a hook menu alter something that is specific to d.o project issue tracking.
Can someone provide some more specific guidance on how to implement this?

The menu change should be fairly simple:

- use hook_menu() to define a custom path, say, project_issue/autocomplete/TID. Copy taxonomy module's hook_menu() item for that
- for the callback, again, copy what taxonomy module has for its normal autocomplete, adding the sort to the query.

Next problem is getting the form to use this path instead. IIRC taxonomy autocomplete widget does something weird and unlike normal FormAPI autocomplete -- I seem to recall an issue about tidying that up. This may or may not have an impact here ;)

One way might be to use form alteration. A heavier (but maybe simpler alternative) would be to just make a new widget.

That said...

This is the query from taxonomy_autocomplete():

    $query = db_select('taxonomy_term_data', 't');
    $query->addTag('translatable');
    $query->addTag('term_access');

    // Do not select already entered terms.
    if (!empty($tags_typed)) {
      $query->condition('t.name', $tags_typed, 'NOT IN');
    }
    // Select rows that match by term name.
    $tags_return = $query
      ->fields('t', array('tid', 'name'))
      ->condition('t.vid', $vids)
      ->condition('t.name', '%' . db_like($tag_last) . '%', 'LIKE')
      ->range(0, 10)
      ->execute()
      ->fetchAllKeyed();

It doesn't get the vocab in its metadata, which is a shame, but it's tagged. So an implementation of hook_query_TAG_alter() could check the t.vid condition for the project issue tags vocab (assuming that's a separate vocab? I don't know) and add the sort order to the query.

That's much less code -- just one function. However, will the alter hook have a performance cost?

The handler used on Drupal.org is from Search API: http://cgit.drupalcode.org/search_api/tree/contrib/search_api_views/incl... That callback looks similar to core's.

I think one alternative solution that might avoid expensive queries is adding exact matches at the top of the list, at least?

I wonder if some js delay like in #2333053: JavaScript for #type => 'machine_name' registers key presses on 'source' slowly later, when label has spaces, special or international characters in it might help here also. (waiting for people to pause while typing before trying to get the results)

Per discussion with YesCT in #durpal-contirbute, I will try to come up with custom module according the #10 comment, which might be ported to d.o.

Maybe a d.o dev environment already exists that we can use to test this?

or one might need to be created https://www.drupal.org/node/1018084 "Develop on our server (preferred)"

I started the build of tags-drupal.redesign.devdrupal.org for this issue.

Hi, I'm sorry for delay, didn't have time to look into it last week.

First I was going to modify 'project_issue' module, but it does not have tags field for Issues. "Issue tags" field was created in drupalorg_project, so I have improved this module.

I have added menu item (taxonomy/autocomplete/taxonomy_vocabulary_9) with callback function "drupalorg_project_issue_tag_autocomplete", which is based on "taxonomy_autocomplete".
This menu item affects only taxonomy/autocomplete/taxonomy_vocabulary_9, so other arguments in "taxonomy/autocomplete/%/%" still works with core taxonomy_autocomplete.

Execution times:
- for "Usability" = 0.05158805847168
- for "Needs" =0.031492948532104

Patch is attached.

It is uploaded to tags-drupal.redesign.devdrupal.org, so you can verify how it works.

I've just realized, that I had to add "do-not-test" to the patch name - it will fail tests, because the patch is for https://www.drupal.org/project/drupalorg

Moving to Drupal.org customizations

I am guessing this needs review by drumm.

The handler used on Drupal.org is from Search API: http://cgit.drupalcode.org/search_api/tree/contrib/search_api_views/incl... That callback has a different menu path.

Hi drumm,

Could you explain more? I don't understand why are you referring to Search API?
thanks

Sorry, I mixed this up with the autocomplete for the advanced issue queue search. It would be good to update to match, but that doesn't have to be part of this issue.

Made an issue for that other autocomplete
#2398379: Sort autocomplete of issue tags descending by usage count *on advanced search page*

The implementation of this does belong in Project Issue module. Drupalorg currently doesn't do any issue-tags-related customization; and I expect this would be a welcome feature for anyone using Project Issue.

To make the menu alter generic, we need to see which vocabularies are autocomplete issue tags. The algorithm will be something like:

foreach (vocabulary as $vocabulary) {
  if (variable_get('project_issue_taxonomy_vocabulary_issue_queue_' . $vocabulary->vid, 0) === 'autocomplete') {
    // Override the menu item for $vocabulary's machine name.
  }
}

@drumm
I was going to do it in Project Issue, but that module does not have tags field out of the box. I see, that it was added in drupal.org manually.

I commented this above.
https://www.drupal.org/node/2362065#comment-9390563

First I was going to modify 'project_issue' module, but it does not have tags field for Issues. "Issue tags" field was created in drupalorg_project, so I have improved this module.

Identifying this as a potential good issue for Sprint Weekend. See discussion at #2407325: preparing for a sprint, would be good to have a list of candidate issues. Adding sprint queue tag.
If this is worked on during the sprint, please add the tag SprintWeekend2015. Add a comment when you start work saying what you are about to do, so we can coordinate.

While Project Issue doesn't make a tag vocabulary out of the box (maybe it should, that's another issue), it does have the configuration and customization for tag vocabularies. For example, project_issue_form_taxonomy_form_vocabulary_alter() adds the configuration to edit vocabulary pages. Grepping for project_issue_taxonomy_vocabulary_issue_queue_ shows the various things project issue does for the vocabulary(/ies).

See #25 for implementation details for this issue.

For D7 core, adding an extra "taxonomy_autocomplete" tag could help, because then another module could customize the results.

FYI for core I opened a new issue: #2409171: Add a specific tag to the taxonomy_autocomplete query

Also, there was already an existing issue but it was focused on altering the output: #2069967: Add an alter hook for taxonomy_autocomplete results

Removed tag by mistake

I am at a global sprint looking at this issue, along with Alimac. YesCT says I need to request access to the Dev environment located at:
https://tags-drupal.redesign.devdrupal.org/
per the steps listed at: https://www.drupal.org/node/1018084

It looks like the next thing to do is to take Patch #17 and try to make it work on project_issue instead of drupalorg.

I logged into the server: tags-drupal.redesign.devdrupal.org

In /var/www/dev/tags-drupal.redesign.devdrupal.org/htdocs/sites/all/modules/drupalorg, I checked out a new branch titled '2362065-17' and committed all pending changes (included changes for this issue and Critical Block). Then, I checked out branch '7.x-3.x' to emulate site behavior prior to any of these changes.

After this, I cleared caches, entered 'usability' into the Issue Tags field on page: https://tags-drupal.redesign.devdrupal.org/node/2380883

We observed that 'Usability' did not appear at the top of the tag list. In fact, 'Usability' did not appear at all.

Then, I changed directories to /var/www/dev/tags-drupal.redesign.devdrupal.org/htdocs/sites/all/modules/project_issue, I committed existing minor changes to project-issue.info to a new branch titled '2362065-stuff', and then I checked out another new branch titled '2362065-34'.

Then, I pasted both hunks from Patch #17 into the new branch titled '2362065-34'. I adjusted the function callback name to match the new project_issue module.

After this, I cleared caches, entered 'usability' into the Issue Tags field on page: https://tags-drupal.redesign.devdrupal.org/node/2380883

We observed that 'Usability' did appear at the top of the tag list.

The resulting Patch #34 is attached.

I'm done for the day. Next, think about a better name for the callback function project_issue_issue....

Also, see if the performance impact has already been tested. There was some performance related code that was in the drupalorg 2362065-17 branch that I did not carry over to project_issue. Does this still need performance evaluation?

I'm not sure why that wont apply with git. I dont see any reason.
it did apply with patch -p1 (see https://www.drupal.org/patch/apply )
And I make a patch with git diff (which is the same way the previous patch was made...)
Maybe next time we should do a scp instead of copy and paste from the server to a local file.

I thought about the function name 'project_issue_issue_tag_autocomplete', and I think it is fine. I also caught a debugging variable declaration ($time_start) that I originally missed, but now it is out. Re-posting the new patch. I think this is ready to go.

taxonomy_vocabulary_9 should not be hard-coded. #25 has some pseudocode for how to know which vocabulary/ies are autocomplete tags for issues.

added info from the comments to the summary about how to work on the issue.

Wait, we have an "official" topical tag list? /rubs eyes

Could we formalize that list, cache it, and have an ultra-fast autocomplete of preferred tags? It could even be two-step, returning the fast preferred results first and then the deeper search later.

> Could we formalize that list, cache it, and have an ultra-fast autocomplete of preferred tags?

Simplest way to do that would be to add a boolean field to the tags taxonomy: yay for D7 and fields on everything! :)

This is the menu alter recommendation from #25 only.

This should remove the vocabulary hardcoding completely. I haven't tested this yet.

An AJAX HTTP error occurred.
HTTP Result Code: 200
Debugging information follows.
Path: https://tags-drupal.redesign.devdrupal.org/taxonomy/autocomplete/taxonomy_vocabulary_9
StatusText: OK
ResponseText: Taxonomy field taxonomy_vocabulary_Undefine index, display notices, bxslider, usab not found.

hm.

#43 works after a cache clear. Now we must put in the optimizations mentioned in #40-41.

Let's get this in and optimize in #2418255: Formalize and optimize autocomplete of issue tags *in the issue metadata form*.

#43 works after a cache clear.

Then it needs a hook_update_N() to clear the appropriate cache.

I have never used a specific cache clear function. I hope this is the right one for this situation.

looks good to me, let's see what drumm (or others say if they want to look also)

Looks okay, committed.

Now deployed.

So. Much. Win.

Thank you!

Oops, that tag was from a test that I apparently didn't remove properly. I'm already skewing the numbers :P

Unfortunately, on many issues, this is somehow causing "9/" to be prefixed to the first tag in the list of tags. I've seen it happen, but so have webchick and Alex Pott. Or at least, this is the most likely suspect.

Is this on the issue tags list or the autocomplete? Can you give an example?

On this very issue, type ", DX" at the end of the issue tags, wait for it to autocomplete, select "DX (Developer Experience)" and BAM, "9/" is prefixed to the list of tags.

At least it is self-documenting.

:D

+++ b/project_issue.module
@@ -2795,3 +2806,106 @@ function project_issue_default_component_allowed_values($field, $instance, $enti
+    foreach ($field['settings']['allowed_values'] as $tree) {
...
+    // Do not select already entered terms.
...
+    $prefix = count($tags_typed) ? drupal_implode_tags($tags_typed) . ', ' : '';
+
+    foreach ($result as $tid => $name) {
+      $n = $name;
+      // Term names containing commas or quotes must be wrapped in quotes.
+      if (strpos($name, ',') !== FALSE || strpos($name, '"') !== FALSE) {
+        $n = '"' . str_replace('"', '""', $name) . '"';
+      }
+      $term_matches[$prefix . $n] = check_plain($name);
+    }
+  }

I don't know what the point of this prefix is.

Another problem that seems to have been introduced by this: some tags don't autocomplete. Like "Contributed project blocker", which is definitely a valid tag: https://www.drupal.org/project/issues/search/drupal?project_issue_follow.... Note that on *that* page, it does autocomplete, just not at /node/add/project-issue/drupal.

The test site has the same bug so we could have seen this coming.

And another instance of where the "9/" prefix problem occurred: #2424309: Expose more language container parameters, to allow services to be language-aware efficiently.

If anyone has time to work on this in the short-term, the test site is up-to-date with #49 with cleared caches, and does in fact reproduce the problem.

@jhodgdon also noticed this and filed #2424399: Tag auto-complete field has a problem, which I've just marked as a duplicate.

I'm guessing the 9 is coming from taxonomy_vocabulary_9 somehow?

I'm rolling this back for now: https://bitbucket.org/drupalorg-infrastructure/drupal.org/commits/05c439....

When fixing this, please do not include another hook_update_N(). For Drupal.org deployments, we have the jobs set up so specific cache clears are a checkbox on deployment, so it isn't necessary to script them.

That rollback is deployed now. I'll leave the commits in project_issue as-is, unless another issue in this project wants to be deployed before this is fixed.

I think I found the bug, it is that func_get_args() didn't expect the new argument I added. Fixing...

Test manually at https://tags-drupal.redesign.devdrupal.org/node/2380883 bacon/bacon. Needs tests.

To anyone wanting to write the tests, you will need to do the following:

1. Create a taxonomy named project_issue_taxonomy_vocabulary_issue_queue_1. It is important that thetid of the taxonomy actually is '1'.
2. Create a field named taxonomy_vocabulary_1 (not field_taxonomy_vocabulary_1).
3. Tag some content with terms from the taxonomy. You need to do this in such a way that certain searches should favor the most popular terms.
4. Hit taxonomy/autocomplete/taxonomy_vocabulary_1/searchterm for the results.

There are some hardcoded assumptions in the patch which is the reason for how 'highly-tuned' it is. I came along after those but we could consider making it a little more flexible.

This test is not quite done.

let's see the test results anyway.

@cilefen so what is next here?
[edit]
oh, I guess the @todo's in the patch. :)

@YesCT Exactly. The bug is fixed AFAIK.

I'll finish the tests for this over the weekend.

Tests attached.

However, I this patch still has some hardcoding going on and could use a bit more work.

The original menu route item for taxonomy autocomplete looks like this:

  $items['taxonomy/autocomplete'] = array(
    'title' => 'Autocomplete taxonomy',
    'page callback' => 'taxonomy_autocomplete',
    'access arguments' => array('access content'),
    'type' => MENU_CALLBACK,
    'file' => 'taxonomy.pages.inc',
  );

The real path to get results for something like this would be a request to /taxonomy/autocomplete/[field_name]/[terms]

The patch that was committed does this in a menu alter:

  foreach (taxonomy_get_vocabularies() as $vid => $vocabulary) {
    if (variable_get('project_issue_taxonomy_vocabulary_issue_queue_' . $vid, 0) === 'autocomplete') {
      $items['taxonomy/autocomplete/taxonomy_vocabulary_' . $vid] = array(
        'title' => 'Issue tags Autocomplete',
        'page callback' => 'project_issue_issue_tag_autocomplete',
        'access arguments' => array('access content'),
        'page arguments' => array((string) $vid),
        'type' => MENU_CALLBACK,
      );
    }
  }

This does not look right to me. Instead of replacing the menu route from the taxonomy module, we're adding one that's more "specific", but we're relying on the field name to start with taxonomy_vocabulary_. Maybe on d.o. thats how the field name exists, but not every site using the Project Issue module.

I think we should instead just completely override the original menu route from taxonomy module with our own. Inside it, we loop through each taxonomy that's part of that field and check if that taxonomy has the special variable project_issue_taxonomy_vocabulary_issue_queue_[vid] set from this module. If so, then we alter the query to take usage into account.

Can someone back me on that? If so I'll work on it.

Disregard last patch (interdiff still valid tho)

I agree 100%. But unless you or I can make those changes early this week, should we hold off improvements for a follow-up? It works now so if we can get basic tests passing, it will be good for upcoming sprints. @YesCT - thoughts?

1. +++ b/project_issue.module
   @@ -176,15 +176,14 @@ function project_issue_menu() {
   - *
   - * A project_issue node requires a parent project, and also that a "component"
   - * of that project is specified.  In order to have the pull-down for the
   - * component be properly populated, we hook 'node/add/project-issue' and
   - * have the user select a project there, and then a form on
   - * 'node/add/project-issue/<projectname>' has the component widget properly
   - * constructed.
     */
    function project_issue_menu_alter(&$items) {
   +  // A project_issue node requires a parent project, and also that a "component"
   +  // of that project is specified.  In order to have the pull-down for the
   +  // component be properly populated, we hook 'node/add/project-issue' and
   +  // have the user select a project there, and then a form on
   +  // 'node/add/project-issue/<projectname>' has the component widget properly
   +  // constructed.
   

   this change does not seem related to this issue.

2. +++ b/project_issue.module
   @@ -192,6 +191,11 @@ function project_issue_menu_alter(&$items) {
   +  // with the naming scheme "taxonomy_vocabulary_[vid]" where the vocabulary
   +  //they reference has project issue enabled. This allows us to override the sorting
   +  // for the data returned.
   

   line wrapping off on this. I'll fix it.

Also.. I think we should be able to show that the added tests would have caught the problem.
Hoping that this tests only patch shows that and fails.

Looks okay, committing.

Now deployed.

wow!

something I didn't notice before, but seems fine, is that usage is per project. For example, in Core, usability is high, but in this queue it is not.

opps.

So. Much. Win.

Thanks!

We should now do the same thing on the advanced search tag field
#2398379: Sort autocomplete of issue tags descending by usage count *on advanced search page*

@bkosborne Regarding #82, should we handle that in a new follow-up or in #2418255: Formalize and optimize autocomplete of issue tags *in the issue metadata form*?