Edgewall Software
Modify

Opened 9 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.3.x
Component: general Version:
Severity: normal Keywords: AJAX, patch
Cc: ejucovy@…, Thijs Triemstra, walty8@…
Release Notes:
API 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@…> 9 years ago.
7628-reporter-auto-complete.v1.patch (3.9 KB ) - added by walty <walty8@…> 2 years ago.
7628-reporter-auto-complete.v2.patch (4.7 KB ) - added by anonymous 2 years ago.
patch.v2.search_result.png (10.4 KB ) - added by anonymous 2 years ago.

Download all attachments as: .zip

Change History (31)

Changed 9 years ago by Jeff Hammel <jhammel@…>

comment:1 Changed 9 years ago by Remy Blank

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

comment:2 Changed 9 years ago by Remy Blank

Tentatively scheduling for the same milestone as #2456.

comment:3 Changed 9 years ago by Remy Blank

Milestone: 0.13

comment:4 in reply to:  2 ; Changed 9 years ago by 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

comment:5 Changed 7 years ago by anonymous

Would like to see this.

comment:6 in reply to:  4 Changed 7 years ago by Thijs Triemstra

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 Changed 2 years ago by 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.

comment:8 Changed 2 years ago by Ryan J Ollos

#8966 closed as a duplicate.

comment:9 Changed 2 years ago by 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?

comment:10 Changed 2 years ago by walty <walty8@…>

Cc: walty8@… added

comment:11 Changed 2 years ago by Ryan J Ollos

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

comment:12 Changed 2 years ago by 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.

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

Changed 2 years ago by walty <walty8@…>

comment:13 Changed 2 years ago by 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).
  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.

comment:14 in reply to:  13 ; Changed 2 years ago by Ryan J Ollos

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.

comment:15 in reply to:  14 Changed 2 years ago by 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.

comment:16 Changed 2 years ago by 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.

comment:17 in reply to:  14 Changed 2 years ago by 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.

comment:18 in reply to:  16 ; Changed 2 years ago by 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?

comment:19 in reply to:  18 ; Changed 2 years ago by 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.

Changed 2 years ago by anonymous

Changed 2 years ago by anonymous

Attachment: patch.v2.search_result.png added

comment:20 Changed 2 years ago by walty8@…

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.

comment:21 in reply to:  19 ; Changed 2 years ago by 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.

comment:22 in reply to:  21 ; Changed 2 years ago by 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?

comment:23 in reply to:  22 Changed 2 years ago by 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.

comment:24 Changed 18 months ago by 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.

comment:25 Changed 12 months ago by Ryan J Ollos

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

comment:26 Changed 11 months ago by 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?

comment:27 in reply to:  12 Changed 6 months ago by 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.

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.
The owner will be changed from (none) to anonymous.

Add Comment


E-mail address and name can be saved in the Preferences .

 
Note: See TracTickets for help on using tickets.