Allow auto-query-linking of custom text fields

[ethan.jucovy@…]

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

[ethan.jucovy@…]

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.

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

[Peter Suter]

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:

querylink	querlinkwords
item	list
keyword	keywords
keyword	keywordlist
term	terms
term	termlist

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

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

[Remy Blank]

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!

[Peter Suter]

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

[Peter Suter]

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

[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)

[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".)

[Peter Suter]

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>

[Ethan Jucovy <ethan.jucovy@…>]

use genshi streams to make better test assertions

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

[Peter Suter]

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

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

[Peter Suter]

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