Extension point interface for ticket's comment

[t2y <tetsuya.morimoto@…>]

I'm testing a full-text search plugin for trac now.

I found ITicketChangeListener doesn't handle when I edit a ticket's comment, because ticket.modify_comment() method has no extension point. It means there is no way to update the index for full-text search when a comment is edited.

I need a new extension point interface for ticket's comment.

[Ryan J Ollos]

I didn't follow the conversation in #11148 too closely, but the IResourceChangeListener might support your use-case.

[t2y <tetsuya.morimoto@…>]

Thank you for good information. I'll try!

[t2y <tetsuya.morimoto@…>]

I've misunderstood #11148 is not implemented yet …

BTW, why don't you use this for ticket cross-reference?

​th:TracTicketReferencePlugin

[Jun Omae]

We can handle modify and delete ticket's comment to use IRequestFilter and Request.add_redirect_listener. Of course, that's not elegant….

tracadvsearchplugin.diff​ is untested patch for ​th:TracAdvancedSearchPlugin.

I think we should add modified_ticket_comment and deleted_ticket_comment methods to ITicketChangeListener because it seems that we need more time to fix #11148. Thoughts?

[Ryan J Ollos]

Replying to jomae:

I think we should add modified_ticket_comment and deleted_ticket_comment methods to ITicketChangeListener because it seems that we need more time to fix #11148. Thoughts?

I think it makes sense to add the methods to ITicketChangeListener in 1.0.x. To have consistency in naming with ticket_created, ticket_change and ticket_deleted, the names ticket_comment_modified and ticket_comment_deleted seem better to me (or even comment_modified, comment_deleted; as long as the verb goes last).

In the case of deleting a comment though, we are really deleting a change, which includes ticket properties: Ticket.delete_change is called by TicketDeleter. When editing a comment, Ticket.modify_comment is called, but perhaps we should make the interface more generic for cases that properties might be edited through a plugin. So should the methods be ticket_change_modified and ticket_change_deleted?

[Olemis Lang <olemis+trac@…>]

Replying to rjollos:

Replying to jomae:

I think we should add modified_ticket_comment and deleted_ticket_comment methods to ITicketChangeListener because it seems that we need more time to fix #11148. Thoughts?

I think it makes sense to add the methods to ITicketChangeListener in 1.0.x. To have consistency in naming with ticket_created, ticket_change and ticket_deleted, the names ticket_comment_modified and ticket_comment_deleted seem better to me (or even comment_modified, comment_deleted; as long as the verb goes last).

In the case of deleting a comment though, we are really deleting a change, which includes ticket properties: Ticket.delete_change is called by TicketDeleter. When editing a comment, Ticket.modify_comment is called, but perhaps we should make the interface more generic for cases that properties might be edited through a plugin. So should the methods be ticket_change_modified and ticket_change_deleted?

I'd rather prefer to integrate this into the changes proposed for #11148 , and thereby prefer comment_modified because :

• there are other ways to modify a ticket (e.g. BH ticket relations …)
• comments may be leveraged as resources bound to other resources i.e. like attachments and scattered change event notification interfaces will just over complicate things

[Jun Omae]

I think it makes sense to add the methods to ITicketChangeListener in 1.0.x. To have consistency in naming with ticket_created, ticket_change and ticket_deleted, the names ticket_comment_modified and ticket_comment_deleted seem better to me (or even comment_modified, comment_deleted; as long as the verb goes last).

Indeed. But I think ticket_change_deleted is better cause of Ticket.delete_change(). Proposed changes can be found in jomae.git@t11377.

In the case of deleting a comment though, we are really deleting a change, which includes ticket properties: Ticket.delete_change is called by TicketDeleter. When editing a comment, Ticket.modify_comment is called, but perhaps we should make the interface more generic for cases that properties might be edited through a plugin. So should the methods be ticket_change_modified and ticket_change_deleted?

No. The methods doesn't provide a way to edit the properties of ticket. The interface is a post-hook is called after a ticket is changed. If the enhancement is reasonable, we should add or improve an interface for the edit of properties, e.g. ITicketManipulator.prepare_ticket() and #11477.

[Ryan J Ollos]

Replying to jomae:

Indeed. But I think ticket_change_deleted is better cause of Ticket.delete_change(). Proposed changes can be found in jomae.git@t11377.

I tested the change and it seems good. I rebased on 1.0-stable before testing. It would be great to commit it.

[Jun Omae]

Replying to rjollos:

I tested the change and it seems good. I rebased on 1.0-stable before testing. It would be great to commit it.

Thanks for the testing. I'll push it with unit tests. Unit tests would be added to trac/ticket/tests/model.py.

[Jun Omae]

I refreshed the branch with unit tests, jomae.git@t11377.1.

[Jun Omae]

Committed in [12956] and merged to trunk in [12957]. Thanks.

[Ryan J Ollos]

We might want to move the Release Notes in this ticket to API Changes since it's a developer-facing change.

[Jun Omae]

Indeed. Thanks for pointing out.