Edgewall Software
Modify

Opened 13 years ago

Last modified 6 months ago

#7628 new enhancement

[PATCH] (suggested) autocomplete users on ticket forms

Reported by: Jeff Hammel <jhammel@…> Owned by:
Priority: normal Milestone: next-dev-1.7.x
Component: general Version:
Severity: normal Keywords: AJAX, patch
Cc: ejucovy@…, Thijs Triemstra, walty8@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

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.

Attachments (4)

autocompleteusers_vs_r7535.diff (26.6 KB ) - added by Jeff Hammel <jhammel@…> 13 years ago.
7628-reporter-auto-complete.v1.patch (3.9 KB ) - added by walty <walty8@…> 6 years ago.
7628-reporter-auto-complete.v2.patch (4.7 KB ) - added by anonymous 6 years ago.
patch.v2.search_result.png (10.4 KB ) - added by anonymous 6 years ago.

Download all attachments as: .zip

Change History (34)

by Jeff Hammel <jhammel@…>, 13 years ago

comment:1 by Remy Blank, 13 years ago

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

comment:2 by Remy Blank, 13 years ago

Tentatively scheduling for the same milestone as #2456.

comment:3 by Remy Blank, 13 years ago

Milestone: 0.13

in reply to:  2 ; comment:4 by Jeff Hammel <jhammel@…>, 13 years ago

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

comment:5 by anonymous, 12 years ago

Would like to see this.

in reply to:  4 comment:6 by Thijs Triemstra, 11 years ago

Cc: Thijs Triemstra added

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.

comment:7 by Ryan J Ollos, 6 years ago

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.

comment:8 by Ryan J Ollos, 6 years ago

#8966 closed as a duplicate.

comment:9 by walty <walty8@…>, 6 years ago

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?

comment:10 by walty <walty8@…>, 6 years ago

Cc: walty8@… added

comment:11 by Ryan J Ollos, 6 years ago

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

comment:12 by Ryan J Ollos, 6 years ago

#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.

Last edited 6 years ago by Ryan J Ollos (previous) (diff)

by walty <walty8@…>, 6 years ago

comment:13 by walty <walty8@…>, 6 years ago

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).
  1. 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.
  1. 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.

in reply to:  13 ; comment:14 by Ryan J Ollos, 6 years ago

Replying to walty <walty8@gmail.com>:

  1. 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.

  1. 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.

in reply to:  14 comment:15 by walty <walty8@…>, 6 years ago

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.

comment:16 by Ryan J Ollos, 6 years ago

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.

in reply to:  14 comment:17 by Ryan J Ollos, 6 years ago

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.

in reply to:  16 ; comment:18 by walty <walty8@…>, 6 years ago

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?

in reply to:  18 ; comment:19 by Ryan J Ollos, 6 years ago

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.

by anonymous, 6 years ago

by anonymous, 6 years ago

Attachment: patch.v2.search_result.png added

comment:20 by walty8@…, 6 years ago

sorry for the late, just uploaded the new patch.

the update includes:

  1. removed posixpath
  1. if show_full_names defined, full name will be displayed and searchable
  1. if show_email_addresses defined, email address will be displayed and searchable
  1. 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.

in reply to:  19 ; comment:21 by walty8@…, 6 years ago

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.

in reply to:  21 ; comment:22 by Ryan J Ollos, 6 years ago

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?

in reply to:  22 comment:23 by walty <walty8@…>, 6 years ago

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.

comment:24 by walty8@…, 6 years ago

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.

comment:25 by Ryan J Ollos, 5 years ago

Milestone: next-major-releasesnext-dev-1.3.x

comment:26 by Ryan J Ollos, 5 years ago

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?

in reply to:  12 comment:27 by Ryan J Ollos, 5 years ago

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.

comment:28 by Ryan J Ollos, 2 years ago

Milestone: next-dev-1.3.xnext-dev-1.5.x

Milestone renamed

comment:29 by Ryan J Ollos, 11 months ago

Milestone: next-dev-1.5.xnext-dev-1.7.x

Milestone renamed

comment:30 by Ryan J Ollos, 6 months ago

#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.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as new The ticket will remain with no owner.
The ticket will be disowned.
as The resolution will be set. Next status will be 'closed'.
The owner will be changed from (none) to anonymous. Next status will be 'assigned'.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.