[PATCH] (suggested) autocomplete users on ticket forms

[Jeff Hammel <jhammel@…>]

I've written a plugin, ​th:AutocompleteUsersPlugin , that uses AJAX to complete users in the owner and CC fields for new and existing tickets (for the latter, at least for the default workflow). The attached patch is the plugin in patch form, fairly unmodified. Performance should degrade gracefully for no JS or when restrict_owner is true.

The autocompleter completes from known users who have the TICKET_MODIFY permission, mirroring the performance of the select widget when restrict_owner is true and populates the fields with the user name, though (obfuscated) email and full names are shown in the drop down and are included in the completion algorithm.

I have included autocomplete.js from ​http://www.dyve.net/jquery/?autocomplete which is an updated version of the jquery plugin that source:trunk/trac/htdocs/js/suggest.js is based on. The update is needed to decouple what is displayed in the drop-down from what is populated to the input field. suggest.js could be updated with this as well as the only place it is used, source:trunk/trac/versioncontrol/web_ui/changeset.py​ , if this is desired.

A new RequestHandler handles requests to /users to respond to AJAX request. This handler could be made to do more if requests were descriminated upon req.get_header('X-Requested-With') == 'XMLHttpRequest', or the whole data could be added into the JS on the server side (which is not possible with the existing suggest.js).

Several new javascript files are included, in addition to autocomplete.js. These could be consolidated and/or generated on the server side if that is more desirable.

[Remy Blank]

This is related to #2456, which proposed an API for user management.

[Remy Blank]

Tentatively scheduling for the same milestone as #2456.

[Jeff Hammel <jhammel@…>]

Replying to rblank:

Tentatively scheduling for the same milestone as #2456.

Please let me know upon investigation if any additional work is desired for this patch, as I'd be happy to do it (including replacing the one existing piece of legacy autocomplete code). I'm also very excited about #2456, as it is something that I've wanted for awhile

[anonymous]

Would like to see this.

[Thijs Triemstra]

Replying to Jeff Hammel <jhammel@…>:

Please let me know upon investigation if any additional work is desired for this patch, as I'd be happy to do it (including replacing the one existing piece of legacy autocomplete code). I'm also very excited about #2456, as it is something that I've wanted for awhile

This needs a review.

[Ryan J Ollos]

Replacing suggest.js with jQuery UI's autocomplete would be a good start for anyone wishing to work on this ticket. The replacement of suggest.js could be done in another (new) ticket.

[Ryan J Ollos]

#8966 closed as a duplicate.

[walty <walty8@…>]

So, if I attempt to work on this ticket, I better create another new ticket and requesting for 'replacing suggest.js with jQuery UI', is that correct?

[Ryan J Ollos]

Yeah, I created #12169 for replacing suggest.js.

[Ryan J Ollos]

#2968 closed as a duplicate. After autocomplete is implemented we can use a text field for owner when restrict_owner is True. #1901 has a similar request for the milestone field that could be solved by autocompleting a text field for milestone.

[walty <walty8@…>]

the patch would do auto-suggest for the reporter field (if the reporter field is available)

pls note that:

1. I did not check the existence of reporter field in JS, because if the jquery got an empty set, there would be no effect of event binding. So there is no need to copy the logic for reporter field permission checking (administrator or anonymous).

2. I tried to add the unit tests to trac/ticket/tests/web_ui.py, my code is something like this:

       def test_new_ticket_reporter_suggest(self):
           req = self._create_request(args={'path_info': 'newticket', 'term': 'u'}, is_xhr = True)
           resp = self.ticket_module.process_request(req)
   
   

   however, the following error shows up:

   Traceback (most recent call last):
     File "/tracDev/trac-trunk/trac/ticket/tests/web_ui.py", line 197, in test_new_ticket_reporter_suggest
       resp = self.ticket_module.process_request(req)
     File "trac/ticket/web_ui.py", line 179, in process_request
       return self._process_newticket_request(req)
     File "trac/ticket/web_ui.py", line 563, in _process_newticket_request
       req.send(to_json(names), 'application/json', 200)
   AttributeError: 'Mock' object has no attribute 'send'
   

   Finally I just removed the unit tests.

3. I tried to run the pylint to check the new code, but there is a bunch of errors and warnings. Is there some configuration I could use for the pylint checking next time?

thanks.

[Ryan J Ollos]

Replying to walty <walty8@gmail.com>:

2. I tried to add the unit tests to trac/ticket/tests/web_ui.py, my code is something like this:

The send method needs to be added to the Mock, similar to the redirect method. We should probably just add a MockRequest object to test.py, but I haven't gotten around to proposing that yet.

3. I tried to run the pylint to check the new code, but there is a bunch of errors and warnings. Is there some configuration I could use for the pylint checking next time?

I haven't used PyLint directly. I've only used IDE integrations. However, I'd suggest trying make pylint from the top-level directory: tags/trac-1.1.6/Makefile@:364-371#L355​.

Thanks for the patch, I should have time to review it tomorrow. The use of posixpath seems to be copy and paste from #12169 and probably isn't needed here since we are working with a username, not a path.

[walty <walty8@…>]

Thanks for the patch, I should have time to review it tomorrow. The use of posixpath seems to be copy and paste from #12169 and probably isn't needed here since we are working with a username, not a path.

Yes, I did not check that one throughly. I just thought the result returned from the jquery plugin always contain two parts, that could be split by posixpath. I would fix that one along with other comments if any.

thanks.

[Ryan J Ollos]

The patch looks pretty good. We'll want to search on both username and full name when [trac] show_full_names is enabled (#7339). You could show full name (username) in the autocomplete listing, but then just insert the full name when the field is autocompleted. The value submitted for the field should be the username (comment:106:ticket:7339).

We might want to search on email addresses as well when [trac] show_email_addresses is True or the user has EMAIL_VIEW for the resource. I'm unsure. The logic that considers the enabled options would be similar to Chrome.format_author​.

Two features from the examples that we might consider using are ​combobox and ​max height. It could be worthwhile to experiment with those after the changes get a bit further along. I like the idea of just implementing this for the Reporter field, then when we feel the changes are ready expand the patch to include the CC and Assign-to fields.

This would be a great feature to have in Trac 1.2; keep up the good progress! Later we'll want to autocomplete other fields such as the username in the wiki form. That can be another ticket though.

[Ryan J Ollos]

Replying to Ryan J Ollos:

I haven't used PyLint directly. I've only used IDE integrations. However, I'd suggest trying make pylint from the top-level directory: tags/trac-1.1.6/Makefile@:364-371#L355​.

I tried make pylint and found the output to be overwhelming ⇒ #12218.

[walty <walty8@…>]

Replying to Ryan J Ollos:

The patch looks pretty good. We'll want to search on both username and full name when [trac] show_full_names is enabled (#7339). You could show full name (username) in the autocomplete listing, but then just insert the full name when the field is autocompleted. The value submitted for the field should be the username (comment:106:ticket:7339).

We might want to search on email addresses as well when [trac] show_email_addresses is True or the user has EMAIL_VIEW for the resource. I'm unsure. The logic that considers the enabled options would be similar to Chrome.format_author​.

Two features from the examples that we might consider using are ​combobox and ​max height. It could be worthwhile to experiment with those after the changes get a bit further along. I like the idea of just implementing this for the Reporter field, then when we feel the changes are ready expand the patch to include the CC and Assign-to fields.

This would be a great feature to have in Trac 1.2; keep up the good progress! Later we'll want to autocomplete other fields such as the username in the wiki form. That can be another ticket though.

hi, while the feature of show_full_names is relatively easy to understand, I don't quite understand how the feature of show_email_addresses works.

I tried to fill in my email address in the preference, and enabled it in trac.ini, there is nothing to change in the ticket view.

After digging into the source code (trac/web/chrome.py), I found that it mentioned that option would be removed by 1.3.1

1020             # show_email_address is deprecated: will be removed in 1.3.1
1021             'show_email_addresses': show_email_addresses,

May be we just consider full name for now?

[Ryan J Ollos]

Replying to walty <walty8@gmail.com>:

hi, while the feature of show_full_names is relatively easy to understand, I don't quite understand how the feature of show_email_addresses works.

I tried to fill in my email address in the preference, and enabled it in trac.ini, there is nothing to change in the ticket view.

You can see the effect of show_email_addresses by granting TICKET_CREATE and TICKET_VIEW to anonymous and entering an email address in the author field when creating a ticket as anonymous. The email address should be obfuscated when show_email_addresses is False.

After digging into the source code (trac/web/chrome.py), I found that it mentioned that option would be removed by 1.3.1

1020             # show_email_address is deprecated: will be removed in 1.3.1
1021             'show_email_addresses': show_email_addresses,

The option isn't being removed, rather the value will no longer passed in the data dictionary that is available when rendering templates. The email obfuscation behavior is now encapsulated in Chrome.format_author and it shouldn't be necessary to use show_email_addresses directly in templates.

May be we just consider full name for now?

Yeah, I think that is fine for the initial version of the patch, and maybe sufficient for the changes we commit in this ticket. Later on we can discuss whether we need to search on email addresses as well.

[walty8@…]

sorry for the late, just uploaded the new patch.

the update includes:

1. removed posixpath

2. if show_full_names defined, full name will be displayed and searchable

3. if show_email_addresses defined, email address will be displayed and searchable

4. limit the maximum height

pls ntoe that ​combobox feature is not implemented, because the author mentioned the sample is not complete and only used for demo purpose.

[walty8@…]

Replying to Ryan J Ollos:

Replying to walty <walty8@gmail.com>:

hi, while the feature of show_full_names is relatively easy to understand, I don't quite understand how the feature of show_email_addresses works.

btw, it's interesting that though my email is obfuscated in most places, it's displayed in public in this reply. It's not a big deal for me, but I wonder if it's some sort of expected behavior.

[Ryan J Ollos]

Replying to walty8@gmail.com:

btw, it's interesting that though my email is obfuscated in most places, it's displayed in public in this reply. It's not a big deal for me, but I wonder if it's some sort of expected behavior.

Yeah, it looks like that's probably a defect. It might be related to the work in #7339. Would you mind creating a ticket and assigning it to milestone:1.2?

[walty <walty8@…>]

Replying to Ryan J Ollos:

Replying to walty8@gmail.com:

btw, it's interesting that though my email is obfuscated in most places, it's displayed in public in this reply. It's not a big deal for me, but I wonder if it's some sort of expected behavior.

Yeah, it looks like that's probably a defect. It might be related to the work in #7339. Would you mind creating a ticket and assigning it to milestone:1.2?

No problem, just created ticket #12249.

[walty8@…]

I just converted the patch into a commit of git hub, ​df25c79e. The code is exactly the same, it's just easier to look with.

[Ryan J Ollos]

After #7339, users may have an inclination to search for "Real name" rather than "userid". For example, the timeline can be filtered by author. The timeline shows events by "Real name" (when [trac] show_full_names is True), so it's logical to put "Real name" in the author box to filter. However, this will lead to no results. Instead, users must know, or lookup, the "userid" corresponding to "Real name".

I considered that we might lookup "Real name" from "userid", however maybe a better solution is through the implementation of autocomplete in this ticket, which would allow the user to autocomplete "Real name" and submit would send "userid". Does that sound like the right solution, or should we consider implementing a lookup?

[Ryan J Ollos]

Replying to Ryan J Ollos:

#2968 closed as a duplicate. After autocomplete is implemented we can use a text field for owner when restrict_owner is True.

See also comment:1:ticket:7628 for suggested change in behavior of [ticket] restrict_owner on query and component admin pages.

[Ryan J Ollos]

Milestone renamed

[Ryan J Ollos]

Milestone renamed

[Ryan J Ollos]

#4632 closed as a duplicate, requesting we address the CC field. The issue requests a drop-down, but a more modern solution would be autocompleting the field.