Having keywords in reports be followable

[Hans Zauber <hans@…>]

In reports (such as {8} - Active Tickets, Mine First), milestones are followable, that is, one may click a milestone's name which appears as a link. However, keywords, reporters or owners are not followable.

I have tried patching the code, but it seems like this is needed both in query.py and in report.py (depends on the query type), and in either case putting a link instead of a string is neither elegant nor doing the job, as html sanitizers eat the links.

I think it can't hurt adding this functionality. With very basic help, I am willing to write this functionality myself and send a patch.

[Ryan J Ollos]

Replying to Hans Zauber <hans@…>:

In reports (such as {8} - Active Tickets, Mine First), milestones are followable, that is, one may click a milestone's name which appears as a link. However, keywords, reporters or owners are not followable.

It's intuitive that milestones entries in the table should direct to their /milestone/<name> pages. Where do you propose that keywords, reporters and owners entries direct?

[Hans Zauber <hans@…>]

• the keyword performance will direct to /query?status=!closed&keywords=~performance
• the reporter hansz will direct to /query?status=!closed&reporter=hansz
• the owner hansz will direct to /query?status=!closed&owner=hansz

These are not "made up"; these are the links that exist anyway from the ticket itself, by clicking a keyword, a reporter name, or an owner name.

[Ryan J Ollos]

Okay. I understand that you'd like the keywords, reports and owners to be rendered in the same way they are rendered in the ticket properties box. That seems reasonable and I'd be willing to work with you on a patch.

You might start by looking at TicketModule._prepare_fields​, which does the work of preparing the rendered fields for the ticket properties box. We'd have to make sure it could be done in a way that performance of page rendering is not significantly impacted.

Note if the milestone field is not changed to be a query link as well, we would have an inconsistency in that most links would be query links, but milestone links would be resource links.

[Hans Zauber <hans@…>]

Note if the milestone field is not changed to be a query link as well, we would have an inconsistency in that most links would be query links, but milestone links would be resource links.

That's true. However, we already have that inconsistency…

I'll have a look at _prepare_fields, keeping performance in mind.

[Ryan J Ollos]

Replying to Hans Zauber <hans@…>:

That's true. However, we already have that inconsistency…

I'm referring to just the report table, and I don't see any inconsistency at the moment. As far as I can see all of the links in the report table are resource links; we don't have a mixture of resource links and query links in the report table.

I just think that a user will find it odd to click on some links and be taken to the Query page, and be taken to a resource page when clicking on other links. We might just change the milestone resource link to a query link for consistency, but should see what others think about the issue.

[Remy Blank]

Replying to rjollos:

I just think that a user will find it odd to click on some links and be taken to the Query page, and be taken to a resource page when clicking on other links.

FWIW, I agree that this would be confusing. Linking from one query to another query also doesn't have clear semantics: does it restrict the current query, or does it create a new query with only that constraint?

We might just change the milestone resource link to a query link for consistency, but should see what others think about the issue.

I would prefer not doing that. If we had user-visible pages (i.e. non-admin pages) for components, versions, etc. I would link to those. But we currently don't, so I wouldn't linkify these fields.

All in all, I'm -1 on this ticket.

[Ryan J Ollos]

Replying to rblank:

FWIW, I agree that this would be confusing. Linking from one query to another query also doesn't have clear semantics: does it restrict the current query, or does it create a new query with only that constraint?

Some good points. At this point I don't think I favor making the change to Trac either.

I would prefer not doing that. If we had user-visible pages (i.e. non-admin pages) for components, versions, etc. I would link to those. But we currently don't, so I wouldn't linkify these fields.

I will keep in mind and consider the possibility of adding resource links on the report page for components and versions while working on #1233.

[Hans Zauber <hans@…>]

I am not sure rblank's arguments convince me; but you absolutely know Trac better than I do. Anyway, I'll roll that in my mind, and maybe add my own patch to my own trac copy.

My last argument is that: our project is not component-based or milestone-based (we do not have milestones at all) - it is keyword-based. Whether that's a good idea or not is a different question, but given that, easy navigation via keywords is very convenient, and worth having some inconsistency.

[Ryan J Ollos]

Either way I'm happy to help you work through creating the patch if any questions come up on your end, and you could always roll it into a plugin and even publish it on trac-hacks if it's not accepted into the Trac core.

[Hans Zauber <hans@…>]

Thanks.

The main issue I'm fronting is that it seems like all relevant methods (like _query_link) are owned by TicketModule, and are not "global" utilities; I guess that ReportModule should use TicketModule's methods; do you usually duplicate methods when you need it? In that case, I guess it involves duplicating ticketlink_query and some other stuff as well, and it doesn't feel like the right way.

[Ryan J Ollos]

Replying to Hans Zauber <hans@…>:

The main issue I'm fronting is that it seems like all relevant methods (like _query_link) are owned by TicketModule, and are not "global" utilities; I guess that ReportModule should use TicketModule's methods; do you usually duplicate methods when you need it? In that case, I guess it involves duplicating ticketlink_query and some other stuff as well, and it doesn't feel like the right way.

I've been troubled by similar things while working on plugins. There's a lot of code that would be useful to plugins, but this isn't realized until working on a plugin. Trac could be improved by plugin developers suggesting patches to the Trac core to expose what they need, but it really doesn't help the plugin developer with their immediate task at hand, so I can see how this likely won't be a priority. Even as a developer of both Trac and plugins, I've only made some minor attempts to improve Trac to allow utilizing more code by plugins (e.g. #11169).

In this case I'd probably end up duplicating the code if I was working on a plugin, but if I was working on Trac I might try to extract _query_link and _query_link_words and make them functions in trac.ticket.query. I'd post the proposed changes and ask the other devs for feedback.

So you could take a crack at refactoring to extract the methods you need, but then in order to utilize those changes you'd have to run 1.0-stable or trunk, or wait for them to be incorporated into a release. If you produced a patch and the other devs are onboard with the changes you suggest I'd try to get them incorporated fairly soon.

[Christian Boos]

Replying to rjollos:

… I might try to extract _query_link and _query_link_words and make them functions in trac.ticket.query. I'd post the proposed changes and ask the other devs for feedback.

One of the problems of making more of the API visible as public API is that you end up having to "maintain" that interface over time, reducing the possibilities to reimplement things differently. So one has really to think carefully about this before exposing too much details. The above methods are probably good candidates, though.

So you could take a crack at refactoring to extract the methods you need, but then in order to utilize those changes you'd have to run 1.0-stable or trunk, or wait for them to be incorporated into a release.

If those changes are reviewed, tested, that shouldn't be more risky than using your own private code in a plugin ;-) We try our best to ensure that 1.0-stable, and even trunk, are always safe to use.

[Ryan J Ollos]

I extracted _query_link and _query_link_words to functions in repos:rjollos.git:t11246. I'm not entirely convinced that these changes are worth making, but at least we can see what the changes might look like.

[hans@…]

What do you think? Is there any chance that these changes will actually appear in trunk?

[Ryan J Ollos]

The changes that I've staged don't offer any value to Trac and would just complicate the codebase, so I can't see pushing them. However, you could copy the two functions in the prepared changes and use them in your plugin.

I would at least like to push [707d0e51/rjollos.git] though, and maybe another look at the code will reveal a more elegant solution.

[Ryan J Ollos]

Minor change committed to trunk in [12393].

Sorry we couldn't do better with regard to exposing functions or methods in the API to help with your plugin. In any case, it doesn't seem like it will be overly burdensome to just put the code in your plugin.