Validate ticket comment changes

[Ryan J Ollos]

Discussed in #12722.

_validate_ticket should probably be called because currently max_comment_size is not enforced for a comment edit: tags/trac-1.2/trac/ticket/web_ui.py#L1336​. _validate_ticket calls ITicketManipulator, which would allow additional rules to be enforced for ticket comment changes.

ICommentManipulator (comment:2:ticket:12722) might provide a nicer interface, but I'm unsure if it's necessary.

As suggested in #12642 for ticket workflow warnings, and similar to what is discussed in #11269, the warnings should be rendered near the comment edit box.

[Ryan J Ollos]

WIP changes in log:rjollos.git:t12725_validate_comment.

[Peter Suter]

Some plugins (​th:TracHoursPlugin, ​th:PullRequestsPlugin) implement ITicketManipulator mainly to manipulate / cause side-effects, not to validate. Will there still be a way for these plugins to avoid these side-effects on comment editing? (It looks like req.args.get('comment') might return None for comment editing, correct? That might conveniently keep them working as-is.)

[Ryan J Ollos]

Yes, req.args.get('comment') returns None for comment editing. I haven't checked, but it probably return an empty string for a ticket property change with no comment.

A plugin could also detect a comment edit by checking for 'edited_comment' in req.args.

[Ryan J Ollos]

I'll target next-major-release since there is impact on plugins.

[Ryan J Ollos]

Part of #11269 will also be addressed in this ticket. I chose to place the warnings directly above the preview so that it doesn't interfere with the ticket property changes box. Here is an example of the interference:

With warnings placed above the preview:

For consistency I added a warning above the New Ticket preview:

For consistency I moved the edit conflict warnings and added a warning box for the Ticket Change preview:

[Ryan J Ollos]

Proposed changes in log:rjollos.git:t12725_validate_comment.1. I will add test coverage before committing.

[Ryan J Ollos]

Replying to Peter Suter:

Some plugins (​th:TracHoursPlugin, ​th:PullRequestsPlugin) implement ITicketManipulator mainly to manipulate / cause side-effects, not to validate. Will there still be a way for these plugins to avoid these side-effects on comment editing? (It looks like req.args.get('comment') might return None for comment editing, correct? That might conveniently keep them working as-is.)

Documented in TracDev/PluginDevelopment/ExtensionPoints/trac.ticket.api.ITicketManipulator@7.

[Ryan J Ollos]

Committed to trunk in r15719.