New ticket_change class: commit

[jsalaz@…]

We have had an issue on our instance of TRAC where a developer typed a commit message awkwardly, and (because he had an edit button), attempted to fix the wording.

The problem he had was that upon clicking submit, the page would refresh but the changes would not be applied, yet if he would click edit again, his changes would be present there.

While I do consider this a bug, I respect the fact that as it stands, it would be way too hard and unnecessary to change, because you would cause an instance where a commit message says one thing, and the TRAC comment says another.

I am proposing a new type of value for use in the ticket_change table, "field" column, named "commit".

This would be functionally identical to the comment 'field' type, except it will show the reply button (maybe), but remove the edit button from view. It should only be set when using the commit ticket updater plugin, and similar methods.

[Remy Blank]

Replying to jsalaz@…:

It should only be set when using the commit ticket updater plugin, and similar methods.

It took me a while to figure out what you meant. The "ticket updater plugin" finally gave me the hint :)

Indeed, the ticket updater plugin adds a WikiProcessor that takes the commit message directly from the changeset itself. The reason for also having the message in the ticket comment is for ticket notification e-mails to also have something legible (we don't expand wiki text in e-mails yet).

Adding a specific field type to the ticket system just for that feels hackish, and adding yet another special case in the ticket code wouldn't be reasonable. At some point, we do want to expand wiki text when rendering ticket notification e-mails, so we won't include the changeset message in the ticket comment anymore. This should avoid the confusion your developer experienced.

In your situation, the correct thing to do would be to edit the changeset message in your version control system, if supported (SVN does), and to call trac-admin $ENV changeset modified. The ticket comment will then pick up the change automatically.

Suggesting "wontfix".

[Jason <jsalaz@…>]

I had a similar thought process with myself, but here's the thing;

I understand why you folks have the commit message inline. That's fine. It makes sense.

What's so special about it? It's identical to comment except it can't be edited. I imagine that there are a log of places you're going to have to drop "commit" into the SQL, but if it's worth doing, why not? It's confusing because the developer is presented an edit button, and he was absent-minded to forget that it wasn't something he typed into the ticket, but instead the checkin he made.

If an edit button is visible, there's an expectation that he can edit it. Disabling it based off the existence of the WikiProcessor is completely unreasonable, and given that commit messages do happen automatically by a special process, the first half of this ticket (how to put it in there in the first place) is, in my mind, feasible. (Then again, who am I? Just some user.)

What about some middle ground? Instead make the field be called comment-commit and update queries to use "WHERE field LIKE comment%" in the SQL (and similar structs if you use a non-raw SQL method). One character change (potentially) is a lot more reasonable than shoe-horning a new field name in.

I wouldn't expect this to be done before 0.13, fyi. The scope of such is too high for a minor release.

[jsalaz@…]

A little bit more cohesive reasoning, I wasn't entirely awake at 7AM this morning…

Yes, in the context of changing the message, modifying and resync'ing (I use that term generally) TRAC against SVN is the correct thing to do, and we should, but that's not my report, chiefly because then this wouldn't be a bug report.

My problem is that TRAC is presenting a feature to a user that won't work or isn't applicable the majority of the time. In his own words, he only tried to edit the message because he felt that it should have been worded better, and appeared to be given the ability to do so. Yes, they should know that commits aren't stored in TRAC, and instead or stored in SVN itself, referenced by TRAC, but in this case, they just didn't think of it that way.

Yes, you could add additional details above and below the WikiProcessor, but that's not really the flow either, and I imagine it doesn't happen often. That's why it would be nice to remove the "feature" that shouldn't be used. TRAC references the commit, and that's all it has to do, especially since it's an automatically generated message. Besides, checkin details like that shouldn't be open to be edited by anyone, accidental or otherwise.

[Christian Boos]

We can certainly think about adding some kind of "read-only" flag to automatically generated ticket change comments.