Edgewall Software
Modify

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#10643 closed enhancement (fixed)

Allow auto-query-linking of custom text fields

Reported by: ethan.jucovy@… Owned by: Ethan Jucovy <ethan.jucovy@…>
Priority: normal Milestone: 1.0
Component: ticket system Version:
Severity: normal Keywords:
Cc:
Release Notes:

TracTicketsCustomFields support two new formats for text fields, reference (query-able value) and list (list of query-able values, like keywords)

API Changes:

Description

When viewing a ticket, Trac creates "autoquery links" for all of the built-in ticket fields in the .properties table. This lets the user click on the text of (e.g.) the "priority" value to land on a Custom Query page showing all open tickets with the same priority as the ticket he was just viewing.

This autoquery-linking behavior is active for built-in fields. It is also active for custom fields, depending on the type of the field. It is enabled for custom "select", "checkbox" and "radio" fields, but it is not enabled for custom "text" or "textarea" fields.

Per discussion on trac-users it can be useful to enable autoquery-linking for custom "text" fields in some scenarios. Currently Trac provides no built in way to do this: it can only be done by implementing a plugin that provides a Genshi stream filter.

The attached patch allows users to turn on autoquery-linking for custom "text" and "textarea" fields. Per discussion on #7562 and #1791, this configuration does not introduce a new Option. Instead, it reuses the [ticket-custom] FIELDNAME.format field introduced in [7563], because it does not make sense for a field to be both auto-linked and wiki-formatted.

Two new .format values are introduced. To turn on standard autoquery-linking for a custom field, the user does this:

[ticket-custom]
foobar = text
foobar.label = The foo dazzle
foobar.value = ""
foobar.format = querylink

The user can also turn on "keyword-style" autoquery-linking for a custom field. This makes a separate query-link for each word in the field's value, in the same way that the built-in "keyword" field does:

[ticket-custom]
foobar = text
foobar.label = The foo dazzle
foobar.value = ""
foobar.format = querylinkwords

Attachments (5)

querylink_customfields.diff (1.4 KB) - added by ethan.jucovy@… 3 years ago.
querylink_customfields.2.diff (1.8 KB) - added by ethan.jucovy@… 3 years ago.
10643_format_reference_with_tests.diff (5.5 KB) - added by Ethan Jucovy <ethan.jucovy@…> 2 years ago.
10643_format_reference_genshi_stream_tests.diff (7.9 KB) - added by Ethan Jucovy <ethan.jucovy@…> 2 years ago.
use genshi streams to make better test assertions
T10643_format_reference_re_split_tests.patch (9.4 KB) - added by psuter 2 years ago.

Download all attachments as: .zip

Change History (21)

Changed 3 years ago by ethan.jucovy@…

comment:1 follow-up: Changed 3 years ago by ethan.jucovy@…

  • Component changed from general to ticket system

The Trac tests are showing me seven failures on a fresh checkout of trunk@11017, so the tests don't pass with this patch applied, but the test output is identical with or without the patch applied.

Looking at the patches applied for the two related features (#7562 and #1791; [7499], [7560], [7563]) I don't see any tests or documentation for autoquery-linking or for [ticket-custom] FIELD.format so I wasn't sure where to put new docs or tests for this patch. I'd be happy to add documentation and tests (for #7562 and #1791 as well as the present ticket, if they're not already there somewhere) if someone could point me to the best place to add them.

Lastly, I'm not very happy with the names I used (format=querylink and format=querylinkwords) but I couldn't think of anything more descriptive.

comment:2 Changed 3 years ago by ethan.jucovy@…

Also, I'm not sure whether it makes any sense to enable this for "textarea" fields. It seems like the consensus in #7562 was against it.

But it seems like it doesn't hurt either (after all the user must explicitly enable the behavior for his custom text-based fields) and I think the principle of least surprise suggests that having consistent configuration across the field types is more important than forcibly disabling the behavior for "textarea" fields.

comment:3 in reply to: ↑ 1 ; follow-up: Changed 3 years ago by psuter

Thanks for the patch. In a superficial test it seems to work, and I didn't see any failing unit tests.

I'd be happy to add documentation and tests (for #7562 and #1791 as well as the present ticket, if they're not already there somewhere) if someone could point me to the best place to add them.

Documentation is or should go here I think: source:trunk/trac/wiki/default-pages/TracTicketsCustomFields

Lastly, I'm not very happy with the names I used (format=querylink and format=querylinkwords) but I couldn't think of anything more descriptive.

We might want to consider a less specific name. Some alternatives:

querylinkquerlinkwords
itemlist
keywordkeywords
keywordkeywordlist
termterms
termtermlist

This would allow other features to be triggered by the same format. E.g. if we want to allow configuring custom fields for batch modify "keywords mode" (comment:80:ticket:525), this would be useful for the same kind of keywords/list fields.

I'm not sure whether it makes any sense to enable this for "textarea" fields.

Doesn't seem very useful. I would lean to no. (At least until someone actually needs it.)

Changed 3 years ago by ethan.jucovy@…

comment:4 in reply to: ↑ 3 ; follow-ups: Changed 3 years ago by ethan.jucovy@…

Replying to psuter:

I didn't see any failing unit tests.

Apologies for going offtopic, but is there documentation on how to run the tests? I ran pip install genshi babel pytz twill followed by python /path/to/trac/checkout/trac/test.py and got some test failures on a fresh checkout.

Documentation is or should go here I think: source:trunk/trac/wiki/default-pages/TracTicketsCustomFields

Thanks. I've attached a new version of the patch which includes updated documentation in that file.

Lastly, I'm not very happy with the names I used (format=querylink and format=querylinkwords) but I couldn't think of anything more descriptive.

We might want to consider a less specific name. This would allow other features to be triggered by the same format.

Good point. In my new patch I went with "item"/"list" — mostly because they're easiest to distinguish visually. Bikeshedding … "list" makes a lot of sense, but "item" isn't very self-documenting at all.

I tried looking around for any established nouns that refer to "the set of possible values for built in fields like component/version/milestone/etc" but didn't see anything. I also looked at the GenericTrac page to see if there were any such nouns there .. again no luck.

I'm not sure whether it makes any sense to enable this for "textarea" fields.

Doesn't seem very useful. I would lean to no. (At least until someone actually needs it.)

Fair enough. No longer enabled for "textarea" fields in my new patch.

comment:5 in reply to: ↑ 4 ; follow-up: Changed 3 years ago by rblank

  • Milestone set to 0.13

Replying to ethan.jucovy@…:

Apologies for going offtopic, but is there documentation on how to run the tests? I ran pip install genshi babel pytz twill followed by python /path/to/trac/checkout/trac/test.py and got some test failures on a fresh checkout.

I usually set the PWD to the Trac checkout, activate the virtualenv, then run:

PYTHONPATH=. python trac/test.py

Thanks. I've attached a new version of the patch which includes updated documentation in that file.

The documentation "master" is actually the wiki here, and we periodically update the files in the repository from the wiki. I have already made a copy of TracTicketsCustomFields to 0.13/TracTicketsCustomFields, so you can edit the latter as soon as your patch to trac/ticket/web_ui.py is applied.

Nice patch, BTW, and nice argumentation above!

comment:6 in reply to: ↑ 4 Changed 3 years ago by psuter

Replying to ethan.jucovy@…:

is there documentation on how to run the tests?

TracDev/UnitTests

I tried looking around for any established nouns that refer to "the set of possible values for built in fields like component/version/milestone/etc" but didn't see anything.

Maybe "enum"?

Might be misleading here, as these custom field "enum values" are not stored in the enum DB table (like ticket type, resolution, priority and severity, but not component/milestone/version.)

comment:7 in reply to: ↑ 5 Changed 3 years ago by psuter

Replying to rblank:

The documentation "master" is actually the wiki here

Oops, sorry for spreading wrong information. Is there a wiki page on that? (Hm now I remember comment:15:ticket:10284.)

Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

comment:8 follow-up: Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

I uploaded a new version of the patch. The changes are:

  1. It now includes functional tests for the new features, as well as a test for the already-existing .format=wiki feature.
  2. The new format options are now called "reference" and "list". I think "reference" is my favorite name so far, because it describes how the text will be formatted both literally (it is turned into a hyperlink to a query that references the text) and also conceptually (the text is assumed to be formally referencing some concept or attribute known to the system's users)

comment:9 in reply to: ↑ 8 Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

Replying to Ethan Jucovy <ethan.jucovy@…>:

The new format options are now called "reference" and "list". I think "reference" is my favorite name so far, because it describes how the text will be formatted both literally (it is turned into a hyperlink to a query that references the text) and also conceptually (the text is assumed to be formally referencing some concept or attribute known to the system's users)

(If "reference" sticks around then maybe the multi-option should be named "referencelist".)

comment:10 follow-up: Changed 2 years ago by psuter

Strange, I get this error in your new test:

======================================================================
FAIL: Test for features added in http://trac.edgewall.org/ticket/1791
----------------------------------------------------------------------
Traceback (most recent call last):
  File "trac\ticket\tests\functional.py", line 30
1, in runTest
    </td>''')
  File "trac\tests\functional\better_twill.py", l
ine 204, in better_find
    raise twill.errors.TwillAssertionError(*args)
TwillAssertionError: ('no match to \'<td headers="h_newfield" colspan="3">\n
          <p>\n\n  <a class="missing wiki" href="/wiki/HelloWorld" rel="nofollow
">HelloWorld\\?</a> bananas<br />\n\n</p>\n\n        </td>\'', 'testenv\\trac\\log\\TestTicketCustomFieldTextFormats.html')

----------------------------------------------------------------------

although the log\\TestTicketCustomFieldTextFormats.html file does seem to contain that snippet:

        <td headers="h_newfield" colspan="3">
              <p>

<a class="missing wiki" href="/wiki/HelloWorld" rel="nofollow">HelloWorld?</a> bananas<br />

</p>

        </td>

Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

use genshi streams to make better test assertions

comment:11 in reply to: ↑ 10 Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

Replying to psuter:

Strange, I get this error in your new test although the log\\TestTicketCustomFieldTextFormats.html file does seem to contain that snippet

When I was originally writing the tests I was seeing a lot of failures that ended up coming down to subtle differences in whitespace between my expected and actual output. The tests ended up passing for me so I'm not sure if this is the cause of the failures you are seeing. But just in case, I've uploaded a new patch with better tests — it passes the HTML output through Genshi and makes assertions by looking at particular xpath selections. This should be less brittle. I'm not sure if this fixes the failures you see though, since they were already passing for me..

comment:12 follow-up: Changed 2 years ago by psuter

It seems it was indeed a whitespace issue (specifically a \r\n line ending issue).

Your Genshi approach looks interesting, but maybe it's not needed: Does the attached regular-expression-based whitespace-matching work for you?

It also splits up your test into multiple smaller tests, a bit more in the style of the others in that file.

(I don't think testing that textarea does not support these new formats is necessary.)

comment:13 in reply to: ↑ 12 Changed 2 years ago by Ethan Jucovy <ethan.jucovy@…>

Replying to psuter:

Your Genshi approach looks interesting, but maybe it's not needed: Does the attached regular-expression-based whitespace-matching work for you?

Thanks — yes, the tests in your attached patch pass for me.

comment:14 Changed 2 years ago by psuter

  • Resolution set to fixed
  • Status changed from new to closed

Committed in [11034] (with small TracDev/CodingStyle cleanup).

comment:15 Changed 2 years ago by psuter

  • Owner set to Ethan Jucovy <ethan.jucovy@…>

comment:16 Changed 2 years ago by cboos

  • Release Notes modified (diff)

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed The owner will remain Ethan Jucovy <ethan.jucovy@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Ethan Jucovy <ethan.jucovy@…> to the specified user.
Author


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

 
Note: See TracTickets for help on using tickets.