Edgewall Software
Modify

Opened 21 years ago

Closed 15 years ago

Last modified 23 months ago

#454 closed enhancement (fixed)

[patch]Edit ticket comments

Reported by: tim.alsop@… Owned by: Remy Blank
Priority: high Milestone: 0.12
Component: ticket system Version: 0.7
Severity: critical Keywords: edit ticket comments SPAM tracobject
Cc: david.hopwood@…, mankoff@…, louis@…, chris@…, pkou@…, tiger@…, lycos42@…, shishz@…, steve.webster@…, robin-trac@…, m@…, mnpettig@…, Axel.Thimm@…, ufs@…, martin.delisle@…, ianmayo@…, dbmarquardt@…, OllyBetts, blake@…, olivier@…, zsphinx@…, gary.wilson@…, jribenfors@…, kirean@…, jrgeller@…, nick@…, ryano@… Branch:
Release Notes:
API Changes:
Internal Changes:

Description

It is common for users to make mistakes when entering comments - when this happens, do we close the whole ticket and re-enter the comments correctly, then continue - surely not ! What would be more acceptable is a capability to edit comments in a ticket as long as appropriate permissions are provided (not everybody should be able to edit comments).

Attachments (8)

comment_edit_delete.patch (11.6 KB ) - added by David Sorber <baranovich@…> 16 years ago.
comment_edit.patch (10.7 KB ) - added by David Sorber <baranovich@…> 16 years ago.
comment-edit-11.patch (11.3 KB ) - added by jrgeller@… 16 years ago.
Comment_edit patch that applies cleanly to 0.11
comment-edit-11-5.patch (11.5 KB ) - added by Sebastian Krysmanski <sebastian@…> 15 years ago.
Patch against Trac 0.11.5 plus buttons displayed side by side
454-save-edit-history-r8561.patch (6.2 KB ) - added by Remy Blank 15 years ago.
Save comment edit history in _comment{n} pseudo-fields.
454-save-edit-history-r8561.2.patch (6.5 KB ) - added by Christian Boos 15 years ago.
rebased version, on top of ticket-edit-minor-fixes-r8561.diff
ticket-edit-minor-fixes-r8561.diff (7.6 KB ) - added by Christian Boos 15 years ago.
several minor enhancements on top of r8538, take 2
comments-edit-11-6.patch (11.4 KB ) - added by andrew.macheret@… 15 years ago.
Fixed the 11.5 patch to work for 11.6

Download all attachments as: .zip

Change History (208)

comment:1 by anonymous, 21 years ago

It should also be possible to edit/change the ticket description. It might also be useful to delete comments from a ticket so that a corrected comment can be added …

comment:2 by anonymous, 21 years ago

There should be some kind of history for edits and removals, something like the wikipages maybe?

comment:3 by daniel, 21 years ago

Priority: normallow
Summary: comment edit neededEdit ticket comments

Allowing editing of any comment has a couple of major drawbacks:

  • Easy to abuse. Simply erase everyones comments (script it for more havoc).
  • Comments are not tied to a user or session. Too complicated.

Allowing an admin (TICKET_ADMIN perm) to delete comments should be quite sufficient. It's ok to make mistakes, and it's ok to resubmit a corrected comment.

The communication with the humans of the project still works. ;)

comment:4 by anonymous, 20 years ago

I still think that not being able to edit comments is a major hassle. What if the Ticket already has comments that you want to keep around?

To minimize abuse, perhaps only allow the author of the ticket, or TICKET_ADMIN, the ability to edit a given Ticket.

I don't think the ability to edit a comment is as crucial. It's just the editing of Tickets themselves that's really a problem.

comment:5 by josh aka anonymous above, 20 years ago

Sorry, and when I said "I still".. I did not mean that I am involved with any of the above discussion… I just came back looking to see if anyone else wanted this feature, and added my desire to the list. :)

comment:6 by bkc@…, 20 years ago

I would like to be able to edit my comments on a ticket. I'm a big-boy, I have the TRAC_ADMIN permission, I think I should be able to edit my comments on tickets.

I forgot to indicate #!sql colorizing for a code fragment in a ticket comment and I want to go back and change it.

So, here's another vote for this feature, especially for TRAC_ADMIN

comment:7 by cboos@…, 20 years ago

Yes, nearly the same for me (I left countless wrong #!diff markups instead of #!text/x-diff all over Trac…). Something like what I proposed for editing attachments in #948 is needed here: the TICKET_ADMIN or the creator of the comment should be able to modify/delete it.

comment:8 by louis@…, 20 years ago

Cc: louis@… added

I'm interested in this too.

comment:9 by anonymous, 20 years ago

Cc: chris@… added

comment:10 by Christian Boos, 20 years ago

Owner: changed from Jonas Borgström to Christian Boos
Priority: lownormal
Severity: normalenhancement
Status: newassigned

#1180 has been marked as duplicate of this ticket

comment:11 by Florent Guillaume <fg@…>, 20 years ago

Cc: fg@… added

comment:12 by anonymous, 20 years ago

Cc: jforcier@… added

comment:13 by anonymous, 20 years ago

Cc: pkou@… added

comment:14 by d.mills@…, 20 years ago

I'm not too sure about deleting comments being that good an idea, mabey a moderate down options (like what's used on www.osnews.com) would be more appropriate.

Like that abuse can be screened, but stuff can't just disapear.

comment:15 by anonymous, 20 years ago

I think a TICKET_ADMIN permission, to be able to edit all comments, would be helpful. Or a default permission for any user to edit his or her own comments.

For example, I added the wrong "closed by changeset [111]" and I had to append another comment, instead of just editing the comment. That was kind of annoying.

I noticed something else, I renamed a module and the earlier comments continued to refer to the old module name. They did not update as I had expected.

comment:16 by Christian Boos, 20 years ago

Well, in the long term, I would suggest a default setup in which the ticket description, as well as the ticket comments, are editable by anyone, in true Wiki spirit.

Of course, this implies proper history management, such a possibility to revert changes, in order to control ticket butchers :) (e.g. see the tortured change history of #126 …)

comment:17 by anonymous, 19 years ago

Cc: tiger@… added

Authorized person should be able to change wrong comments and to cleanup and clarify history. Otherwise there is a lot of crap gets accumulated in comments which is totally useless and obscures overall picture.

comment:18 by fg@…, 19 years ago

Something that happens all the time for us is that a user reports a python traceback in a comment, but without the proper {{{ }}}. As an administrator I want to edit the comments and fix the formatting.

comment:19 by Gunnar Wagenknecht <gunnar@…>, 19 years ago

Cc: gunnar@… added

comment:20 by Christian Boos, 19 years ago

Note that by editing the comment, one would loose the old content. Is that OK?

If not, one possibility would be to do the following:

  • edit the old comment, save it
  • this creates a brand new comment, referring to the old
  • to old comment is marked as deleted, it would be still listed, but with an empty content. Only the "TICKET_ADMIN" would have the possibility to view the old content (and eventually to undelete it?)

comment:21 by Joe Wreschnig <piman at sacredchao.net>, 19 years ago

One of our tickets got spammed. I expect this to happen significantly more in the future, because well, that's how spam works. It'd be really nice to be able to remove/elide it without database twiddling.

comment:22 by Christian Boos, 19 years ago

Priority: normalhigh

In the case of spam, it would make sense to delete the whole comment.

But concerning the "normal" case, I would still be interested in getting some feedback to the question I asked two comments above, before starting to implement something.

comment:23 by Jeff Forcier (jforcier at strozllc dot com), 19 years ago

I'd be fine with an edit simply replacing the old content, believing that the case of a ticket admin wanting to make honest, necessary changes to a comment would be much more common than some evil, evil ticket admin wanting to screw with things.

On the please-don't-lose-any-data side of things, I think the best route would be a comment change history as in wiki pages or source files. Assuming that's a little too complex, your suggestion (mark old comment as deleted, created the 'edit' as a new one w/ pointer to old) would work pretty well, provided the 'deleted' comments don't take up much room vertically—otherwise a ticket with lots of comment edits would start to look really messy.

Perhaps replace the comment text in-place, but put small links in the bottom of the comment that would bring up the previous version(s) of the edit? That gets dangerously close to needing a history, though.

comment:24 by chris@…, 19 years ago

The main reason I want this is to delete spam comments. I spent an hour in sql yesterday deleting a ton of spam comments on the Growl trac, having a button or some checkboxes to delete them all would be nice, since I'm the only one with access to actually remove spam comments right now.

Giving other devs the ability to remove spam comments would be great.

comment:25 by anonymous, 19 years ago

Cc: louis@… chris@… fg@… jforcier@… pkou@… tiger@… gunnar@… removed
Owner: changed from Christian Boos to anonymous
Priority: highlowest
Status: assignednew

comment:26 by Matthew Good, 19 years ago

Cc: louis@… chris@… fg@… jforcier@… pkou@… tiger@… gunnar@… added
Priority: lowestnormal

This is not a test system. Do not abuse it.

comment:27 by Christian Boos, 19 years ago

Milestone: 1.0
Owner: changed from anonymous to Christian Boos
Priority: normalhigh

comment:28 by anonymous, 19 years ago

I'm starting to look at Trac for issue tracking on a system that has SLA requirements including response time to customer issues. From my perspective, any change that basically allowed a revisionist view of history would not allow Trac to be a valuable tool to demonstrate to our customers that their issues are being responded to in a timely and appropriate fashion.

Having worked with other tracking systems in the past, I'm painfully aware of many of the issues related to mal-formed comments and "wrong" comments (e.g., "Fixed") making the narrative less-than-readable.

I don't have a significant issue with being able to edit comments, as long as all history is preserved and easily viewable.

comment:29 by chris@…, 19 years ago

Whereas my main issue with not being able to is that my admins are unable to remove spam in a timely manner.

comment:30 by Joe Wreschnig, 19 years ago

If people with administration permissions are going to do malicious things, whether it's delete real tickets or make it look like things were fixed "too fast", a full history isn't going to stop them. Using Trac to prevent "revisionist history" is stupid, when someone could just go in and muck with the database manually if you were intent on fleecing someone (I'm not saying you are, I'm saying that holding up "Trac logs everything" to the customer as proof you're not is worthless).

DELETE_COMMENT_FOREVER_FOR_REAL and DELETE_TICKET_FOREVER_FOR_REAL is a feature that you need to deal with spam, period. Hiding it doesn't cut it, it still lowers your search engine rankings and raises the spammer's (and so makes the spamming effective, and so people keep spamming). You need to delete it forever. And ticket spam is a real problem that is here, now, and growing, as opposed to "oh, we might lose a valuable comment."

Please, just allow comment deletion. Screw history.

comment:31 by lycos42@…, 19 years ago

Cc: lycos42@… added
Milestone: 1.00.9.1

Same here, spam is getting everywhere, we should really have this feature, or at least another method to limit spam without using meta possibilities (catchpas, spam detection system(spamlookup))

It's a major problem.

comment:32 by anonymous, 19 years ago

Cc: shishz@… added

comment:33 by jacob@…, 19 years ago

Cc: jacob@… added

Not to pile on, but we just got hit by a spambot that looks like it's deliberatly targetting Trac (it hit every *open* ticket, but none of the closed ones). Spam bad, mmmmkay?

comment:34 by lycos42@…, 19 years ago

Same here, our trac installation has just been targeted again :-( …

Is there any "unofficial way" to edit/block spam actually ?

comment:35 by chris@…, 19 years ago

Ya, we just got hit again too. I'm probably just going to disable anon access to everything since going into the database is just getting annoying.

comment:36 by Christopher Lenz, 19 years ago

Milestone: 0.9.10.9.2

comment:37 by anonymous, 19 years ago

Deleting tickets is simply not good enough. I'm evaluating Trac as bug tracking application for our project, but without some functional spam blocking for ticket comments I don't see how it could remain useful in the long run.

We are currently using a forum for bug/issue reporting, and we see hundreds of spambot hits per day occasionally. There is no way to handle this by simply deleting posts - they must be prevented. Our solution is to block posts with hyperlinks - plain URLs are fine, but hyperlinks are not. So far, this works perfectly.

From our experience, I'm pretty sure that any Trac installation will have to shut down or disable ticket comments sooner or later, as long as there is no spam blocking provided.

comment:38 by fg@…, 19 years ago

Cc: fg@… removed

comment:39 by Christian Boos, 19 years ago

This ticket is not about spam prevention, see #1145 for that.

Editing/Deleting ticket comments is also useful beyond spam healing.

comment:40 by Gunnar Wagenknecht <gunnar@…>, 19 years ago

I think that moderating/editing/deleting comments should never be done for spam prevention. There are much better options (for example, don't allow anonymous posting).

When I usually comment on an issue in any ticket system I always expect that my ticket content will never be modified by somebody else. It's like sending an email to a mailing list and then allowing the list moderator to change the content of the mail.

Unfortunately, there are some ticket systems, which introduced this capability. The system in our corporate environment also allows. However, all system limit the editing capability to the creator of the specific comment/note or an admin. With each change a log is generated in the ticket history covering the editor's name.

There a lot large communities out there using Bugzilla. All community members have been able to work around the missing editing cabability since their beginning so honestly, I don't understand people using this as a reason for not using Track or Bugzilla.

Anyway, if such a capability is implemented it should be completely configurable, i.e. allowing the admin to define if anybody, only the admin, only the ticket owner or only the comment creator should be able to edit comments or if this capability should be completly disabled. Preferable the default should be disabled ;)

It should be also noted in the ticket history and/or next to the comment that this comment was edited on xxx by user yyy or that the comment was deleted.

I don't think that it's necessary to keep the old comment content around although this would be a nice option to correct accidental edits/deletes.

comment:41 by chris@…, 19 years ago

So if you don't allow anonymous posting, how do public projects allow their users to post tickets to trac?

comment:42 by vyt@…, 19 years ago

Cc: vyt@… added

comment:43 by Gunnar Wagenknecht <gunnar@…>, 19 years ago

So if you don't allow anonymous posting, how do public projects allow their users to post tickets to trac?

That's pretty simple. Allow them to register with their email address first. Doesn't provide Track such a feature?

comment:44 by Christian Boos, 19 years ago

Status: newassigned

Also, some installations create a guest user (with rights similar to those of anonymous), and publish the password somewhere, on their Mailing List or simply on the Wiki.

That's quite effective too, and doesn't require much effort on the admin side.

Speaking of the ticket itself, I'll try to come up with a prototype during those xmas holidays…

comment:45 by Christopher Lenz, 19 years ago

Also, some installations create a guest user (with rights similar to those of anonymous), and publish the password somewhere, on their Mailing List or simply on the Wiki.

The problem with this approach is that all users going through the guest account basically share one session, and thus cannot set their mail address and name so that it is used when posting tickets. Therefore I think this approach should be considered a workaround, not a real solution.

comment:46 by Christopher Lenz, 19 years ago

Milestone: 0.9.31.0

(Moving non-trivial enhancements to next milestone)

comment:47 by anonymous, 19 years ago

Cc: steve.webster@… added

comment:48 by Christopher Lenz, 19 years ago

Milestone: 1.00.10

comment:49 by anonymous, 19 years ago

Cc: robin-trac@… added

comment:50 by anonymous, 19 years ago

Cc: louis@mymindset.co.uk, chris@growl.info, jforcier@strozllc.com, pkou@ua.fm, tiger@tadol.net, gunnar@wagenknecht.org, lycos42@free.fr, shishz@hotpop.com,jacob@jacobian.org, vyt@vzljot.ru, steve.webster@featurecreep.com;robin-trac@robinbowes.com → louis@mymindset.co.uk, chris@growl.info, jforcier@strozllc.com, pkou@ua.fm, tiger@tadol.net, gunnar@wagenknecht.org, lycos42@free.fr, shishz@hotpop.com,jacob@jacobian.org, vyt@vzljot.ru, steve.webster@featurecreep.com,robin-trac@robinbowes.com

comment:51 by dkocher@…, 19 years ago

Tickets get [spammed http://trac.cyberduck.ch/ticket/69]. It should really be possible to just delete these comments .

comment:52 by Markus Tacker <m@…>, 19 years ago

Cc: m@… added
Keywords: SPAM added

comment:53 by anonymous, 19 years ago

Cc: mnpettig@… added

Also interested in seeing this feature added. Too many occasions where it's necessary and it's a major adoption hurdle for some.

comment:54 by anonymous, 19 years ago

originally i expected a comment or description be a normal wiki page (as it uses wiki syntax too), having a history. this would be the best i could imagine. we are also "signing a contract" with a partner, and it would help a lot having the history.

comment:55 by Emmanuel Blot, 19 years ago

About last comment: you have history, as all comments are kept.

I agree with the original comment from daniel (05/24/04): comments could be edited by the admin (to remove spam, etc.), not by users. If you want Wiki history feature, simply use a wiki page and add a link from a ticket to the Wiki page. Comments should be preserved and be shown with no specific action from the user, as with many other bug tracking tools. Keep it simple.

comment:56 by chris@…, 19 years ago

As of right now, anyone with access to sql can just remove it. This is probably the same person who is the admin. I see no point in leaving a history when it's just spam being deleted. If you can trust your trac-admin, who can you trust?

comment:57 by jacob@…, 19 years ago

It's not that simple, Chris — ticket spammers are also changing ticket properties, so just deleting the comment doesn't fix the incorrect info. There needs to be a way to revert an entire set of chances at once.

I can't belive that this ticket is still open — this is a really serious problem!

comment:58 by chris@…, 19 years ago

I guess I just assumed that when I said deleted, the entire changeset would be removed. But yes, I agree with you Jacob.

comment:59 by Gunnar Wagenknecht <gunnar@…>, 19 years ago

Chris, Jacob, editing ticket comments is NOT a solution for any spam problem. There are several tracking systems out there without a way to edit ticket comments. They never had any issues with spam. The solution is pretty simple: don't allow anonymous editing of tickets. (See also my comment from 12/16/05 03:05:51.)

comment:60 by chris@…, 19 years ago

Anonymous editing is part of why we chose Trac over other ticket systems.

comment:61 by jacob@…, 19 years ago

Gunnar: of course disallowing anonymous ticket editing would solve the problem, but it would cause an even bigger problem. If people have to create an account to file a ticket, some of them simply won't file the ticket.

I'd prefer spam to having bugs in my code that I don't know about.

I haven't looked at the Trac code too closely, but it seems to me that this can't be all that hard. Am I missing something? Is there some reason that this ticket has been open for a year without even an optional patch?

comment:62 by Christian Boos, 19 years ago

Status: assignednew

No, it shouldn't be that hard to implement. I once wanted to do it, but got distracted by other stuff… If you want to have a try, here are some tips:

  • removal of the comment should be a two step procedure, similar to the Delete this version of the Wiki UI.
  • when deleting a comment, one should also delete all the ticket changes that were done at the same time. The tricky thing is that all the subsequent ticket changes have to be checked so that their old_value field contain the appropriate value (i.e. the value before the deleted change)

E.g.

  1. comment added, version changed from 1.0 ⇒ 2.0, priority from low ⇒ high
  2. version changed from 2.0 ⇒ 3.0
  3. component changed from engine ⇒ doc
  4. priority from high ⇒ normal

After the removal of comment 1., we should have:

  1. version changed from 1.0 ⇒ 3.0
  2. component changed from engine ⇒ doc unmodified
  3. priority from low ⇒ normal

Hope this helps. The ticket is still on my list, but if someone wants to do it before I find time to do it, he's more than welcome :)

comment:63 by andrei@…, 19 years ago

Not a fix, but temporary work-around. It looks like in most cases all these spam entries are in some aisan language. As a result following SQL allows you to remove them easily

delete from ticket_change where lower( author ) = upper( author );

comment:64 by anonymous, 19 years ago

Cc: mankoff@… added; louis@… removed

comment:65 by anonymous, 19 years ago

Cc: mankoff@… louis@… added; mankoff@… removed

comment:66 by anonymous, 19 years ago

Cc: jforcier@… removed

comment:67 by anonymous, 19 years ago

Cc: casey@… added

comment:68 by Christian Boos, 19 years ago

Milestone: 0.100.11
Priority: highnormal

Post-poning this. I think it also depends on #2703 in some ways.

In the meantime, for people interested in cleaning up a spammed ticket page, I'd suggest having a look at the TicketDeletePlugin.

comment:69 by Axel.Thimm@…, 19 years ago

Cc: Axel.Thimm@… added

Does anyone have good step-by-step instructions on how to remove these spam comments on the database level?

Thanks.

comment:70 by chris@…, 19 years ago

This is what I've been sending to other folks:

So here we go. These all require being in your trac-env/db folder (or however you have it setup, you want to be in the folder with the db)

  • Deleting comments

1) Run this: sqlite trac.db "SELECT * FROM ticket_change WHERE ticket=406"

Change the ticket number to the one you want to affect.

2) You'll get something like this back:

406|1134596968|tick|comment
This is at least .9, but probably needs to be moved to 1.00

3) Copy the second set of numbers, in this case the 1134596968. This is the time you'll need for the next command

4) sqlite trac.db "DELETE FROM ticket_change WHERE ticket=418 and time=1136229618"

Change the ticket number and the time to what you need. Viola, it's just gone.

  • Deleting a ticket (I think, make a test ticket before doing this on a live one)

1) sqlite trac.db "SELECT * FROM ticket WHERE id=549"

2) sqlite trac.db "DELETE FROM ticket WHERE id=549"

Hope this helps. For Growl, the spam got to be too much and I just disabled nondevs ticket rights. It's sad, and not what I wanted, but oh well.

comment:71 by Chris Ashworth, 19 years ago

Although it won't be sufficient when the spam gets really bad, I've been dealing with comment spam as follows:

I wrote two scripts, gettracdb and puttracdb to download (and upload) the trac.db file to my local machine. I grabbed SQLite Database Browser to edit the database. When I get spammed, I grab the database, open it, go to "Browse Data", select ticket_change, delete the offending rows, and then upload the database back onto my server.

Not a long-term solution, but relatively painless for small volumes.

comment:72 by anonymous, 19 years ago

Cc: dkirov@… added

comment:73 by anonymous, 19 years ago

Cc: trac@… added

i also support verson tracking of ticket edits

comment:74 by anonymous, 18 years ago

Cc: gunnar@… removed

comment:75 by Christian Boos, 18 years ago

#3609 marked as duplicate.

comment:76 by Pedro Algarvio, aka, s0undt3ch <ufs@…>, 18 years ago

Cc: ufs@… added

For those worried about spam, wether wiki related or ticket related, I posted a message to the trac-users ML.

Still, I'd like to be able to at least edit my own tickets on other trac envs, and have full edit/delete power on my own trac envs.

comment:77 by anonymous, 18 years ago

Cc: stuff@… added

comment:78 by anonymous, 18 years ago

Cc: chris.alex.thomas@… added

on an unreleated topic.

sorry for interrupting everyones sleep, wouldnt passing the comment through something like SpamAssassin and listening to the results before posting the comment to the database be a way to prevent spam? or if not SpamAssassin, another system, maybe the system thunderbird uses, which IMHO doesnt make any mistakes (AFAIK it hasnt marked one right email as spam for me and I used it for like 4 years)

since these systems already exist, it seems like if one of them marks your comment as spam, something is wrong with it, a fallback solution should be made where the ticket is marked as "moderate" and an administrator can allow/deny it.

sorry to make the comment here, I know cboos says this ticket aint nothing to do with spam, but a lot of people are talking about it and there is my 2c

chris

in reply to:  78 comment:79 by Christian Boos, 18 years ago

Severity: normalmajor

Replying to anonymous:

on an unreleated topic….

Have a look at the SpamFilter.

sorry to make the comment here, I know cboos says this ticket aint nothing to do with spam,

np, but I didn't say that… In comment:39, I said that editing ticket comments had its use beyond repairing spam damage. Of course, being able to edit/delete comments that made through the spam filter is also useful.

More related to the original topic, I made some notes in the TracDev/Proposals/DataModel about a way to store changes to properties themselves. That way, the old comments could eventually be kept for people wanting to be able to audit all changes that ever happened.

I'm not sure the above will make it for 0.11, though. What could be done for 0.11 is support for editing ticket comments without versioning the old comment content. And that should probably be a configurable feature disabled by default.

comment:80 by anonymous, 18 years ago

Cc: dkirov@… removed

removing my mail from CC (requires some text, otherwise it is accepted as spam)

comment:81 by Martin Delisle <martin.delisle@…>, 18 years ago

Cc: martin.delisle@… added
Keywords: SPAM removed

comment:82 by Martin Delisle <martin.delisle@…>, 18 years ago

Keywords: SPAM added

comment:83 by maz <mzizka@…>, 18 years ago

Going back to the original request (not spam-related): I've often found a need to edit a comment some other users made, usually in order to correct the formatting.

Yes, it's true that other ticketing systems do not allow comment editing, but in many cases their formatting rules are also a lot simpler, so there are fewer misses. Perhaps an option for an alternative format in ticket comments and description would take care of many of these editing needs.

comment:84 by Christian Boos, 18 years ago

Milestone: 0.110.12

Not for 0.11, as we need a better data model for storing the comment changes.

comment:85 by anonymous, 18 years ago

I struggled with this same issue and found a workaround. There are two ways to edit Trac ticket comments directly using the sqlite in a root shell window.

To edit a comment on ticket 42:

$ cd /path/to/trac/project/db/
$ sqlite trac.db

sqlite> SELECT time,author,field 
  FROM ticket_change 
  WHERE ticket = 42;

1172336464|bob|summary
1172337066|bob|owner
1172337215|bill|comment
1172338123|alice|comment

sqlite> UPDATE ticket_change 
    SET newvalue = 'This is the new text of bill''s comment.' 
    WHERE ticket = 42 
    AND time = 1172337215;

sqlite> .exit

Note that in the sqlite console shell any single quotes in a string value must be doubled-up (ala. Python), or you can enclose the entire string value double-quotes, or tripled-double-quotes (another Pythonism)

The sqlite console shell does not allow literal newline or escaped newline characters in a string value, so to edit a multi-line comment you need to dump the entire database to a text file, edit the dump file, and restore the database like so:

$ cp trac.db trac.db.backup
$ sqlite trac.db .dump > dump.sql
$ vi dump.sql (search for the comment text and edit it)
$ sqlite newtrac.db < dump.sql
$ cp newtrac.db trac.db

Also note the trac.db file might not be readable with sqlite3 which is different than plain ol' sqlite which uses a different internal file format.

comment:86 by trisweb, 18 years ago

I was appalled when I found out I could not edit or delete comments on tickets. This should be obvious and not argued about; if you can't roll back the entire ticket yet, then at least give a way to edit/delete comments. So simple. Just do it.

I can't believe people have to go into the database to manually do this… so dumb. Stop making it more complicated than it needs to be, just implement it now.

comment:87 by robinbowes, 18 years ago

Well, I would be quite so arrogant as to demand that this be done NOW, but I do share your sentiment that it a fairly obvious defect in trac, and wonder why anyone would think it's not a sensible, high-priority enhancement.

comment:88 by andrei@…, 18 years ago

Maybe we can "re-ignite" this issue. Maybe if number of replies to this ticket will double of triple, Trac people will pay more attention to it (though it has quite a lot of replies as of now)?

comment:88 by anonymous, 18 years ago

There is th:TicketDeletePlugin that allows deletion of tickets and comments. That plugin may be integrated into the core at some point. So, the need for comment editing is not as important now unless you need to correct typos or something.

in reply to:  86 comment:89 by Emmanuel Blot, 18 years ago

Replying to trisweb:

if you can't roll back the entire ticket yet, then at least give a way to edit/delete comments.

Actually, this is not fully true: th:wiki:TicketDeletePlugin does the job: it allows to delete a whole ticket or any field/comment. It does not perform edition though.

So simple. Just do it.

Really? Well, Trac is an open source project, and developers welcome patches.

comment:90 by anonymous, 18 years ago

yeah, I commented in this thread nearly 3 years ago.

I run a closed Trac system. I'm admin, I should be able to edit any comment.

My interest in this has nothing to do with spam, and I'm concerned that this ticket has "SPAM" as a keyword. The original request has nothing to do with spam.

I think tying this ticket to spam fighting is way off track.

:-(

comment:91 by Emmanuel Blot, 18 years ago

Replying to robinbowes:

wonder why anyone would think it's not a sensible, high-priority enhancement.

So many things to implement…

OTOH, Subversion does not allow to delete things…

Moreover, deleting comments may leave orpheneaous links for other locations that pointing at them, so this feature may not be as straightforward as it might look like.

Comment deletion woud still be a nice feature to have for sure. There are many other things that would be nice, still.

comment:92 by bkc, 18 years ago

The title of this ticket is "Edit Ticket Comments".

Why delete? I just want to fix a typo in a comment. :-(

comment:93 by anonymous, 18 years ago

If this would be a useful feature to you, either write the code or pay someone to write it. This is open source software after all. Otherwise stop whining.

comment:94 by anonymous, 18 years ago

I think its marked spam because its killing the open trac systems. Well those of them who have not already implemented mod_security or something to stop it. Either way, bots are on the prowl for trac ticket comment spam.

I would think if this were a huge deal somebody would just make a patch and submit it. You can't demand things when it comes to stuff like this without paying somebody to do your bidding. I personally don't think its a huge deal but maybe thats just because I know how to stop it. But I will admit, it would be very nice feature enhancment. Other than that, I love trac! You simply can't beat it.

comment:95 by anonymous, 18 years ago

Why don't you try using the Trac plugin which is available for download - the plugin allows you to role back ticket comments. I find it very useful when a ticket has been updated and mistake made.

in reply to:  95 comment:96 by anonymous, 18 years ago

Replying to anonymous:

Why don't you try using the Trac plugin which is available for download - the plugin allows you to role back ticket comments. I find it very useful when a ticket has been updated and mistake made.

Because that resolves only the administrative portion of the request. It does not resolve normal user editing abilities, such as correcting typos and whatnot.

This is not just about spam.

comment:97 by anonymous, 18 years ago

I'm not trying to pile on, but this issue is probably going to keep me from using trac. I'm not expecting anyone to scream oh noes! and run off to do this feature on my account, but I do want to register another vote here. This kind of thing is important, especially since there isn't even a preview for looking at what your wiki stuff is going to look like.

I'll poke around and if its not that hard, maybe I'll submit a patch. But until then, I just submit a vote.

in reply to:  97 ; comment:98 by Emmanuel Blot, 18 years ago

Replying to anonymous:

This kind of thing is important, especially since there isn't even a preview for looking at what your wiki stuff is going to look like.

Sorry, but what are you refering to? You can preview your comments before submitting your changes.

in reply to:  98 comment:99 by anonymous, 18 years ago

Replying to eblot:

Replying to anonymous:

This kind of thing is important, especially since there isn't even a preview for looking at what your wiki stuff is going to look like.

Sorry, but what are you refering to? You can preview your comments before submitting your changes.

Doh! I thought I took that part out, after noticing the 'preview' button. Please disregard that part of the comment.

Hey, if I could edit my comment, I could have taken that part out.

comment:100 by anonymous, 18 years ago

I mark the above comment as the single most important comment made in this thread, it's more important than all of the blabbing by everyone else, yet it is the most simple.

I cannot find another example which proves the importance of this feature in so few words

"Hey, if I could edit my comment, I could have taken that part out"

how better to say it than that.

in reply to:  100 ; comment:101 by chris@…, 18 years ago

Replying to anonymous:

I mark the above comment as the single most important comment made in this thread, it's more important than all of the blabbing by everyone else, yet it is the most simple.

I cannot find another example which proves the importance of this feature in so few words

"Hey, if I could edit my comment, I could have taken that part out"

how better to say it than that.

There is a difference between allowing users to edit comments and allowing admins to edit comments. I'd prefer the latter.

in reply to:  101 ; comment:102 by anonymous, 18 years ago

Replying to chris@growl:

There is a difference between allowing users to edit comments and allowing admins to edit comments. I'd prefer the latter.

Sure, but that's just a matter of policy, i.e. once you've added the capability to edit comments then you can use the permission system to restrict that to whichever set of users you wish.

Personally, I think admins and the original author should be able to edit their comments.

R.

comment:103 by anonymous, 18 years ago

I'll pile on here. My organization uses trac to manage product requirements, and enabling user comments would be very helpful. I've also a number of requests to see diffs in ticket descriptions — something that's enabled in the Ticket Delete plugin (http://trac-hacks.org/wiki/TicketDeletePlugin), but only available in the admin view. It would be incredibly helpful to enable that diff view to general users…

in reply to:  103 comment:104 by Christian Boos, 18 years ago

Replying to anonymous:

It would be incredibly helpful to enable [ticket description] diff view to general users…

0.11 has already that feature, see #2945.

in reply to:  102 comment:105 by anonymous, 18 years ago

Replying to anonymous:

Replying to chris@growl:

Another vote for this feaure. We could use it now - if not sooner. Thanks!

comment:106 by anonymous, 18 years ago

Cc: stuff@… removed

removing my mail from CC (requires some text, otherwise bayes detects spam)

comment:107 by basic, 18 years ago

needing this now… entered #8 instead of [8] in a comment. doh!

comment:108 by red, 18 years ago

I would like the ability to edit comments. I found it surprising that 'wiki' software doesn't include this capability. Comment owners, and ticket admins should have the ability.Perhaps add a lock ability so that ticket admins can enable/disable a ticket's comments from being edited by anyone.

comment:109 by Matthew Good, 18 years ago

Please avoid replying simply to say "me too". This simply adds a lot of noise that makes it harder to follow real discussion on the topic. If you're really interested in this feature please consider helping work on a patch.

comment:110 by robin-trac@…, 18 years ago

Yeah, heaven forbid that the users of this tool might express their interest in having a particular feature implemented.

If you're not careful, support for this feature might reach critical mass and you might have to do something about it instead of arguing that we (the users) don't really need that feature and that you know best.

Sheesh.

(Sorry, bad day today)

comment:111 by Christian Boos, 18 years ago

Milestone: 0.120.11

Critical mass is certainly reached here. One of the problem is that the user base is half-split about the need for versioning the comment changes. I think this is not doable with the current code base and needs to wait for a major redesign (hopefully 0.12). But the first option, i.e. not versioning those changes, would certainly be doable for 0.11. That behavior should however be made optional for those not wanting the comments to be mutable without some kind of auditing process.

[ticket]
editable_comments = yes

Even if set, this feature should be reserved to TICKET_ADMIN or the user who made the comment.

comment:112 by robin-trac@…, 18 years ago

Keywords: edit ticket comments added

cboos, I agree entirely with the approach you are proposing.

I would help with implementation (I'm not just a serial whinger) but I'm afraid I don't grok python (I could help you if trac were written in perl!)

R.

in reply to:  112 ; comment:113 by Christian Boos, 18 years ago

Replying to robin-trac@robinbowes.com:

I would help with implementation (I'm not just a serial whinger) but I'm afraid I don't grok python (I could help you if trac were written in perl!)

Trac is a good excuse to learn Python ;-) It certainly worked for me (and manu (eblot) IIRC)

comment:114 by anonymous, 18 years ago

Cc: Heribert.Schuetz.extern@… added

in reply to:  113 comment:115 by Emmanuel Blot, 18 years ago

Replying to cboos:

Trac is a good excuse to learn Python ;-) It certainly worked for me (and manu (eblot) IIRC)

[off-topic]: yeah you're right, not mentionning that Trac is well written and easy to learn.

comment:116 by Ian Mayo, 18 years ago

Cc: ianmayo@… added

Add another interested party.

comment:117 by Benjamin Plaquevent <benjamin.plaquevent@…>, 18 years ago

Cc: benjamin.plaquevent@… added

I'm interrested in tickets comments editing too.

In my opinion, the reason why many of us ask for this feature doesn't really matter. The fact is that lot of us would appreciate…

I think ADMIN_COMMENTS and EDIT_COMMENTS permissions could be a good solution:

  • users with ADMIN_COMMENTS permission can edit/delete any comments
  • users with EDIT_COMMENTS permission can only edit their own comments
  • others changes are impossible or they are moderated by users with ADMIN_COMMENTS permission

I agree with Gunnar Wagenknecht:

It should be also noted in the ticket history and/or next to the comment that this comment was edited on xxx by user yyy or that the comment was deleted.

I don't think that it's necessary to keep the old comment content around although this would be a nice option to correct accidental edits/deletes.

in reply to:  117 comment:118 by Emmanuel Blot, 18 years ago

Replying to Benjamin Plaquevent <benjamin.plaquevent@net-spirit.com>:

I think ADMIN_COMMENTS and EDIT_COMMENTS permissions could be a good solution:

That would be COMMENT_ADMIN and COMMENT_EDIT (OR TICKET_COMMENT_ADMIN, TICKET_COMMENT_EDIT).

comment:119 by anonymous, 17 years ago

3 years to resolve a simple and necessary feature like this? This is insane!

in reply to:  119 comment:120 by Emmanuel Blot, 17 years ago

Replying to anonymous:

3 years to resolve a simple and necessary feature like this? This is insane!

Stop moaning and please provide a patch: trac is open source, isn't it?

in reply to:  119 comment:121 by Christian Boos, 17 years ago

Keywords: tracobject added

Replying to anonymous:

3 years to resolve a simple and necessary feature like this? This is insane!

Simple: not really, see comment:111

Necessary: I agree

Insane: your take ;-)

For milestone:0.12 and versioned edits, I think we should make comments first-class obj^H^H^H resources. The only little drawback I see is some possible risk of resource contention on the 'message' related tables, as they'll get used by any subsystem that will enable commenting on their resources. But that's the same thing for attachements, and it's not such a big deal there.

comment:122 by anonymous, 17 years ago

for those eclipse users, i just figured out that you can edit the original trac description with mylar.

comment:123 by anonymous, 17 years ago

Cc: dbmarquardt@… added

comment:124 by anonymous, 17 years ago

Cc: david.hopwood@… added

comment:125 by vyt@…, 17 years ago

Cc: vyt@… removed

Sorry, I don't work with Trac now.

comment:126 by anonymous, 17 years ago

Cc: benjamin.plaquevent@… removed

Still using Trac but no more interrested by this ticket…

comment:127 by stephan@…, 17 years ago

I'm not sure if anybody else already suggested this:

The owner of a comment, i.e. the user that entered it, should be able to modify it within the next few (e.g. 10) minutes. This is sufficient for correcting typos and afterthoughts. No special permissions should be needed.

comment:128 by anonymous, 17 years ago

Cc: pierreroth64@… added

comment:129 by Christian Boos, 17 years ago

Milestone: 0.11.10.12
Priority: normalhigh
Severity: majorcritical

#6872 closed as duplicate.

Bumping up the prio for the next round.

comment:130 by anonymous, 17 years ago

It already is a lengthy discussion, which should be resolved with a config option. I'm for editing for comment owners and admins. People would comment on the problems that stem from enabling that option. There's too much speculation here.

And if somebody thinks it should not be editable (as some people here) disable that option.

comment:131 by OllyBetts, 16 years ago

Cc: OllyBetts added

Several of the previous comments (e.g. comment:59) suggest that disabling anonymous ticket changes will completely avoid spam comments on tickets. This may have been true back then, but it certainly isn't now - we require users to sign up for an account first, and are seeing spammers signing up for accounts and then spamming tickets. Currently we're deleting such comments "by hand" using sqlite (fortunately we're not yet seeing them mess with the ticket properties), and then nuking the accounts in the admin interface. One spammer has now signed up with the same account name 3 times, so we've had to create a dummy account with that name instead of just deleting it.

comment:132 by blake@…, 16 years ago

Cc: blake@… added

comment:133 by Heribert.Schuetz.extern@…, 16 years ago

Cc: Heribert.Schuetz.extern@… removed

comment:134 by olivier@…, 16 years ago

Cc: olivier@… added

Just another kind request for this feature. We are being unable to remove spam in our tracker at present.

comment:135 by fho@…, 16 years ago

We would need to edit/removing own comments too. Just now i created an comment for the wrong task, and need to remove it, it only confuses other users

comment:136 by bruce@…, 16 years ago

+1

Enabling an admin and/or owner of a ticket to delete comments in the Change History seems like a no-brainer.

comment:137 by trac@…, 16 years ago

Cc: trac@… removed

comment:138 by anonymous, 16 years ago

Please stop discussing this issue. Just do it. It is the number one big annoying issue for new trac users.

comment:139 by anonymous, 16 years ago

+1

comment:141 by Pierre Roth <pierreroth64@…>, 16 years ago

Cc: pierreroth64@… removed

comment:142 by zsphinx@…, 16 years ago

Cc: zsphinx@… added

+1 (text to avoid appearing as spam)

comment:143 by gary.wilson@…, 16 years ago

Cc: gary.wilson@… added

comment:144 by Christian Boos, 16 years ago

Milestone: 0.130.12

#7944 requested the ability to view the wiki markup. This will be possible once ticket comment edit is done.

By the way, that would be a nice little addition for 0.12, in the category "most wanted feature" ;-)

comment:145 by anonymous, 16 years ago

Resolution: fixed
Status: newclosed

comment:146 by robinbowes, 16 years ago

OMG, you're kidding me? Really? Is this fixed??? :)

comment:147 by Christian Boos, 16 years ago

Resolution: fixed
Status: closedreopened

Please, no random butchering, especially not on this ticket. Thanks.

by David Sorber <baranovich@…>, 16 years ago

Attachment: comment_edit_delete.patch added

comment:148 by David Sorber <baranovich@…>, 16 years ago

Summary: Edit ticket comments[patch]Edit ticket comments

My patch implements two new permissions, TICKET_EDIT_COMMENT and TICKET_DELETE_COMMENT. TICKET_DELETE_COMMENT inherits TICKET_EDIT_COMMENT. Users that have these permissions, are only allowed to edit or delete their own comments. The only exception being users that have TRAC_ADMIN, who can delete and/or edit all comments. Edited comments are appended with a timestamp and the username of the user making the edit. This information is used to generate a pretty "Edited by" message at the bottom of the edited comment.

This is my first patch, please be gentle :)

comment:149 by jribenfors@…, 16 years ago

Cc: jribenfors@… added

comment:150 by Christian Boos, 16 years ago

Replying to David Sorber <baranovich@…>:

This is my first patch, please be gentle :)

Great first patch! ;-)

Sorry for the delay, I wanted to test the patch first before replying.

My patch implements two new permissions, TICKET_EDIT_COMMENT and TICKET_DELETE_COMMENT. TICKET_DELETE_COMMENT inherits TICKET_EDIT_COMMENT. Users that have these permissions, are only allowed to edit or delete their own comments. The only exception being users that have TRAC_ADMIN, who can delete and/or edit all comments.

This was tricky, and it seems you have done it right.

Edited comments are appended with a timestamp and the username of the user making the edit. This information is used to generate a pretty "Edited by" message at the bottom of the edited comment.

So the original content is lost. But that's an acceptable trade-off, people who can't afford to lose content will have to wait for the ticket data-model to be expanded (let's hope it's not another 5 years ;-) ).

One thing I'm not sure about is the distinction EDIT/DELETE. Here, a DELETE comment will simply remove the 'comment' itself, not the whole change (that would be a lot more tricky, as this would require fixing the old_values of the next change if any). So the change entry stays and the comment is gone, as well as the Edit button, which makes it not possible to re-add a comment for that change later. So I wonder if it wouldn't be better to just have EDIT and special case the comment annotation when the edited comment is empty (e.g. Cleared by … on date). That would leave open the possibility to have a Delete button that would actually delete the whole change (as the TH:TicketDeletePlugin used to do in 0.10).

The edition in place is also nice, though there's no preview button (that can always be added later). Also, I'd preferer if the Edit button would be like the Reply one (i.e. an inlinebutton).

But all in all, this is a great contribution, I'll test it a bit more in the coming days, but I wanted to let you know I liked it!

by David Sorber <baranovich@…>, 16 years ago

Attachment: comment_edit.patch added

in reply to:  150 comment:151 by David Sorber <baranovich@…>, 16 years ago

Replying to cboos:

Replying to David Sorber <baranovich@…>:

Edited comments are appended with a timestamp and the username of the user making the edit. This information is used to generate a pretty "Edited by" message at the bottom of the edited comment.

So the original content is lost. But that's an acceptable trade-off, people who can't afford to lose content will have to wait for the ticket data-model to be expanded (let's hope it's not another 5 years ;-) ).

One thing I'm not sure about is the distinction EDIT/DELETE. Here, a DELETE comment will simply remove the 'comment' itself, not the whole change (that would be a lot more tricky, as this would require fixing the old_values of the next change if any). So the change entry stays and the comment is gone, as well as the Edit button, which makes it not possible to re-add a comment for that change later. So I wonder if it wouldn't be better to just have EDIT and special case the comment annotation when the edited comment is empty (e.g. Cleared by … on date). That would leave open the possibility to have a Delete button that would actually delete the whole change (as the TH:TicketDeletePlugin used to do in 0.10).

Hmmm… you are right, there really wasn't a good distinction between edit and delete. My original thought was to allow general users, with just the TICKET_EDIT_COMMENT, to correct typos in their comments but not be able to completely remove them and then allow admin users, with TICKET_DELETE_COMMENT or TRAC_ADMIN, to both correct comments and remove them entirely. I have revised my patch so that it only pertains to comment editing. As you suggested, if a user clears their comment while editing it, then the annotation is changed from 'Edited x <units> ago by <user>' to 'Comment removed x <units> ago by <user>'.

The edition in place is also nice, though there's no preview button (that can always be added later). Also, I'd preferer if the Edit button would be like the Reply one (i.e. an inlinebutton).

I added both of these enhancements in my revised patch :) Since comment editing is in place I decided it made sense to make comment edit preview in place as well.

But all in all, this is a great contribution, I'll test it a bit more in the coming days, but I wanted to let you know I liked it!

I'm glad you liked it. If there's anything I can do to improve the functionality or implementation, please let me know!

comment:152 by Erik Andersson <kirean@…>, 16 years ago

Cc: kirean@… added

comment:153 by Jacob Kaplan-Moss <jacob@…>, 16 years ago

Cc: jacob@… removed

comment:154 by jrgeller@…, 16 years ago

Cc: jrgeller@… added

Any chance of porting this to 0.11? I'd like to use the patch now. I applied it to 0.11, but I don't see the edit buttons logged in as admin nor do I see the permissions in the Permissions drop-down.

comment:155 by jrgeller@…, 16 years ago

I got the edit buttons and the permissions showing (had to do with a repo install of trac and pyshared symbolic links), but the save changes button errors out on the date parsing. Looks like it doesn't like the date syntax? This is the error:

Context Navigation
Error: Invalid Date

"2009-05-22T06:12:51+00:00" is an invalid date, or the date format is not known. Try "MM/DD/YY" instead.

This is the hidden field:

		  <input name="edit_comment_when" value="2009-05-22T06:12:51+00:00" type="hidden">

comment:156 by jrgeller@…, 16 years ago

Figured it out. In util/datefmt.py there is a regex for isoformat dates that didn't work with my string above due to there being no Z in the date string. Instead of Z I changed it to Z* and it works perfectly. I'll attempt a proper patch for 0.11 stable.

by jrgeller@…, 16 years ago

Attachment: comment-edit-11.patch added

Comment_edit patch that applies cleanly to 0.11

comment:157 by anonymous, 16 years ago

How to apply this patch? I just got handed a project that exists in track with 50% spam. I don't know about trac, or python. Do I just copy the scripts, or shall I need to edit each line and such as the scripts indicate with the little plus and minuses?

Sorry for my ignorance.

comment:158 by anonymous, 16 years ago

Make a copy of the trac python files on your server. On linux, copy the patch file to the same directory level as the trac python code folder (usually called trac) and type:

patch -p0 < comment-edit-11.patch

That's the easy way. Check the output of this command for anything that says failure. You can manually fix the failures, or just restore your backup of the code. Alternatively you can copy the plus lines over the minus lines, but the patch command is easier when it works.

comment:159 by nick@…, 15 years ago

Cc: nick@… added

So will this be making it into the 0.12 release afterall? I'm hoping so, and that SourceForge will get their version patched AFAP once it's released :\

in reply to:  158 comment:160 by Glenn Horton-Smith <gahs@…>, 15 years ago

Cc: gahs@… added

Replying to anonymous:

patch -p0 < comment-edit-11.patch

FWIW, this worked for me on 0.11.4 source, but not the just-released 0.11.5. Every hunk failed.

Bizarrely, the problem seems to be that the 0.11.5 source code (obtained using easy_install --upgrade Trac) somehow has MS-DOS style carriage returns at the end of each line ("\r\n") rather than Unix-style bare linefeeds ("\n"). My version of patch didn't like that, even with the —ignore-whitespace option.

I was able to patch 0.11.5 with no problem after removing all the carriage returns from the source code. I'm sure there must be a tool for this, but I just did it using find and awk as follows:

cd /usr/lib/python2.3/site-packages/Trac-0.11.5-py2.3.egg/trac
for i in `find . -name \*.py -o -name \*.html -o -name \*.css`; do
  mv $i $i.orig
  awk '// { gsub("\r",""); print $0;}' $i.orig > $i
  rm $i.orig
done

Hope that helps someone.

comment:161 by rupert.thurner, 15 years ago

we use http://trac-hacks.org/wiki/TicketChangePlugin. is this patch doing anything better?

in reply to:  161 comment:162 by Sebastian Krysmanski <sebastian@…>, 15 years ago

Replying to rupert.thurner:

we use http://trac-hacks.org/wiki/TicketChangePlugin. is this patch doing anything better?

Well, you don't need another plugin and this patch doesn't rely on the TicketDeletePlugin (which doesn't work anyway in Trac 0.11, if I remember correctly).

comment:163 by Sebastian Krysmanski <sebastian@…>, 15 years ago

Cc: sebastian@… added

I've updated the patch to Trac 0.11.5. Moreover the buttons are now displayed side by side (instead of edit being below reply).

by Sebastian Krysmanski <sebastian@…>, 15 years ago

Attachment: comment-edit-11-5.patch added

Patch against Trac 0.11.5 plus buttons displayed side by side

comment:164 by Remy Blank, 15 years ago

Owner: changed from Christian Boos to Remy Blank
Status: reopenednew

Sorry this patch had to wait so long. I'll integrate it this week-end.

comment:165 by Remy Blank, 15 years ago

I have committed a minimal version of comment editing in [8538], based on David's patch, updated by Sebastian. The "in line" editing was a great idea, and the presentation almost perfect. The minimal functionality differs from the patch in the following:

  • Used the comment number to identify the comment to be edited, rather than the timestamp of the edit. This avoids having to embed the timestamp in the edit form.
  • Provisionally removed the tag embedded in the comment to identify the editor and edit time. I don't think embedding that information in the comment was the right approach.
  • Only users having TICKET_EDIT_COMMENT are allowed to edit comments, and they can edit all comments.
  • Fixed the permission checks when POSTing. In the initial version, if a user didn't have TICKET_EDIT_COMMENT and issued a POST for a comment edit, the code would fall through to updating the ticket fields.

This is a first step with minimal functionality. I would like to extend this but I need some feedback about the following topics:

  • Who is allowed to edit which comments: Editing comments is quite similar to replacing attachments. The current state for attachments is that anonymous users are not allowed to replace attachments, authenticated users are allowed to replace their own attachments, and users having ATTACHMENT_DELETE permission can replace any attachments (and see #8605 for letting anonymous users replace their own attachments). Should we use the same scheme for editing comments? That is, authenticated users are allowed to edit their own comments, and users having TICKET_EDIT_COMMENT can edit all comments? Or should we define an additional TICKET_EDIT_OWN_COMMENT permission?
  • Visual hint that comment was edited: Do we need an edit indication? As I mentioned above, I don't think that embedding the hint in the comment itself is a good idea. I would rather add an additional row in the ticket_change table for a "virtual" field _last_edit that would mention the author and time of the last comment edit.
  • Edit history: We could actually keep the whole edit history by copying the old comment to _comment0, _comment1, … with the comment in newvalue and the author and edit time in oldvalue. This would allow displaying a "diff" link similar to what we have for the description.

Comments?

by Remy Blank, 15 years ago

Save comment edit history in _comment{n} pseudo-fields.

comment:166 by Remy Blank, 15 years ago

The patch above enhances the basic functionality in trunk by saving the comment edit history in additional rows of ticket_change with field names _comment0, _comment1, … and displaying the last edit info (time and author) below the ticket comment. The "(diff)" link is not yet functional, but is intended to lead to a diff view of the last edit, with the possibility of viewing the complete edit history of the comment (similar to what is shown for the description).

comment:167 by Christian Boos, 15 years ago

I also did a few enhancements on my own (see patch above), tell me what you think about them.

I've rebased your patch on top of mine, and this looks good. Saving previous changes is a good idea, but we must also reserve those _comment{n} names in order to prevent conflict with custom fields.

by Christian Boos, 15 years ago

rebased version, on top of ticket-edit-minor-fixes-r8561.diff

comment:168 by Christian Boos, 15 years ago

Some ideas for further improvements:

  1. for consistency, it should be possible to edit the description the same way as it is now for comment, so both will have the Edit and Reply inlined buttons. This could lead to the removal of the ticket description from the ticket change fields (#8295)
  2. the display for the Add comment preview could match the one of the ticket comment edit preview, i.e. be shown "in place" rather than at the top of the ticket; the rationale for the 0.11 way was that you should see at the same time the changes to the ticket fields in the ticket preview and eventually those of the ticket description. Point 1. makes this last reason disappear, and the ticket changes are anyway shown part of the comment preview

Also, I've noticed that even in Chrome, a code block with very long lines is correctly displayed in the preview, which means there's some hope for fixing #8608 ;-)

by Christian Boos, 15 years ago

several minor enhancements on top of r8538, take 2

in reply to:  167 ; comment:169 by Remy Blank, 15 years ago

Replying to cboos:

I also did a few enhancements on my own (see patch above), tell me what you think about them.

I'll check that, thanks.

I've rebased your patch on top of mine, and this looks good.

Thanks, but as my patch is already in my VCS, I'll rebase the other way around anyway. So there's no need to double the work :-)

Saving previous changes is a good idea, but we must also reserve those _comment{n} names in order to prevent conflict with custom fields.

This is already the case, custom fields must start with [a-zA-Z].

About your improvement ideas:

  1. Wouldn't that prevent modifying the description at the same time as ticket fields? If yes, then I think I prefer the current state.
  1. That sounds like a very good idea. I am always annoyed to have to click "skip" to go down from the preview to the editable fields. So except for point 1, I'd like to try that.

in reply to:  169 comment:170 by Christian Boos, 15 years ago

Replying to rblank: For the other improvement ideas, I've followed-up in comment:7:ticket:8295, as this was more about the description field.

comment:171 by Ryan Ollos <ryano@…>, 15 years ago

Cc: ryano@… added

comment:172 by Remy Blank, 15 years ago

Merged patches committed in [8562], including a few fixes like not storing the new comment if it is identical to the old one.

The next step is the comment history viewer. Looks like this is non-trivial :-)

comment:173 by Remy Blank, 15 years ago

I have hit a strange situation with [8562]. If I create a comment as one user, then edit it as a different user, I get an empty comment below the one that was edited. This seems to be due to this line in grouped_changelog_entries(), and the fact that I store the author of a comment edit in the author field of ticket_change with field='_comment{n}'. So in this case, the comment and _comment{n} rows have different authors, leading to a spurious comment.

My understanding is that the author is used in the uid to ensure that attachments added simultaneously by different authors are displayed correctly. However, it is not possible for different authors to create simultaneous comments.

I see two possible fixes:

  • Only use the author in the uid for attachments:
    • trac/ticket/web_ui.py

      diff --git a/trac/ticket/web_ui.py b/trac/ticket/web_ui.py
      a b  
      14621487        autonum = 0 # used for "root" numbers
      14631488        last_uid = current = None
      14641489        for date, author, field, old, new, permanent in changelog:
      1465             uid = date, author, permanent
       1490            uid = permanent and (date, author, permanent) or (date, permanent)
      14661491            if uid != last_uid:
      14671492                if current:
      14681493                    yield current
  • Drop the author from the uid altogether:
    • trac/ticket/web_ui.py

      diff --git a/trac/ticket/web_ui.py b/trac/ticket/web_ui.py
      a b  
      14621487        autonum = 0 # used for "root" numbers
      14631488        last_uid = current = None
      14641489        for date, author, field, old, new, permanent in changelog:
      1465             uid = date, author, permanent
       1490            uid = date, permanent
      14661491            if uid != last_uid:
      14671492                if current:
      14681493                    yield current

I would go with the latter, because the behavior with simultaneous attachments by different authors is actually undefined, due to Ticket.get_changelog() only sorting by time.

Thoughts?

comment:174 by Christian Boos, 15 years ago

This is tricky, as in ticket_change, we currently mix the changes and the metadata of the changes in one "event". Now here we need to add a change of the change to the mix…

I would effectively go here with the second solution, but see also ticket:6466#comment:10.

comment:175 by Remy Blank, 15 years ago

The ticket comment history and diff viewers have been committed in [8564]. I believe it also fixes the issue in comment:173 correctly.

With this the comment editing functionality is complete. I would appreciate getting some feedback about the functionality and how it is presented to the user.

The only remaining issue is the question of who is allowed to edit comments. As mentioned in comment:165, currently users having TICKET_EDIT_COMMENT permission are allowed to edit all comments. Should we add a TICKET_EDIT_OWN_COMMENT permission? Or should one always be allowed to edit one's own comments?

comment:176 by Christian Boos, 15 years ago

Great patch Remy, I really like it. I've added two minor corrections in r8565.


For the permission, I'd rather allow one to be always able to edit its own comments, in the spirit of #8605.

On the contrary, I'd rather not go down the way to mix up too many things in the permission action. Permission checking involves 3 different aspects: who can do what on what. who is the user/group, do what is the action, on what is the resource. We already mix up on what and do what with permission actions like TICKET_EDIT_DESCRIPTION for example, as the precision of the resource descriptor stops at the ticket level, not to mention the presence of the realm name itself in every action name, as a legacy from pre-0.11. For me the concept of "own" should rather be placed at the level of the who does it and I'd rather build that at the level of the user/group, perhaps using "virtual groups", see ticket:7438#comment:6 where I developed that idea.

Ideally, the actions should be only view/create/edit/delete, little more. See also the Permission model proposal of Jonathan Shapiro, a bit old but still interesting read.

in reply to:  176 comment:177 by Remy Blank, 15 years ago

Replying to cboos:

I've added two minor corrections in r8565.

I hadn't noticed the issue with the timeline. Good catch, thanks!

For the permission, I'd rather allow one to be always able to edit its own comments, in the spirit of #8605.

I also have a preference for that model. And for now, anonymous will not be allowed to edit any comments, until we find a way to encode the session ID with the author.

(…) perhaps using "virtual groups", see ticket:7438#comment:6 where I developed that idea.

This sounds like a good idea, and I wonder how you intend to implement such an @owner group. I suppose this will require permission-specific "checkers" that retrieve various information from the resource model, and hence a way to retrieve the model from the resource object.

comment:178 by Remy Blank, 15 years ago

The final functionality has been committed in [8567]:

  • All authenticated users are allowed to edit their own comments.
  • The layout of the inline buttons was broken on IE, this is now fixed.

This concludes the implementation of this feature. I'll leave this ticket open for a few days, for discussing possible enhancements, and in case the numerous people on the Cc list want to give their feedback ;-)

A big thank you to David Sorber for getting the ball rolling on this feature and providing a good basis with his patch.

comment:179 by David Sorber <baranovich@…>, 15 years ago

This is awesome!!! I'm sure a bunch of people are going to be pleased that this has been addressed. Glad I could help, even if it was only a little.

comment:180 by Remy Blank, 15 years ago

Resolution: fixed
Status: newclosed

No more reactions, considering as closed.

comment:181 by anonymous, 15 years ago

Thanks a lot for the feature implementation. I tested the 0.12dev version with the feature. +1 from me too.

The permission scheme is good. Edit functionality works nice. The properties part of the ticket comment is not editable, this is good too. Only one note: I prefer the initial version where "Delete" comment is supported. I don't like idea to use TicketDeletePlugin in this case. As it needs additional clicks in Admin panel. If TicketDeletePlugin can do deletion I believe the current feature implementation can do this too. Even more, it will be nice to have "Delete" button for the whole ticket deletion from the Trac project on the Ticket page, without switch to Admin panel and typing ticket id. I prefer to remove the ticket from the ticket page as I can see exactly what I will remove. Using admin panel I can mistake with id number for the ticket like this: 1096 or 1069 etc. As there is not preview mode(to see at least ticket summary) in the TicketDeletePlugin such deletion can "surprise" you in the future when you find the ticket again.

Is there the patch for Trac 0.11.5 with the latest changes or I should use comment-edit-11-5.patch?

comment:182 by Remy Blank, 15 years ago

Thanks for your comments. While I understand what you would like to do, this is not the use case for which this functionality was developed. The main use case was for editing typos, fixing formatting and removing or fixing erroneous information in ticket comments. All this while retaining the history of the edits.

Removing a comment completely requires editing the ticket properties, something that was left out on purpose. You can, however, edit the comment and remove all the text. Editing the property history will remain outside of Trac core for now. You could file an enhancement request with the TicketDeletePlugin for improving its user interface.

There will be no backport to 0.11-stable, and the final implementation is significantly different from the patches on this ticket, so I would advise updating to trunk if you need this functionality.

in reply to:  182 comment:183 by Drops <drop_s@…>, 15 years ago

Replying to rblank:

Thanks for your comments. While I understand what you would like to do, this is not the use case for which this functionality was developed. The main use case was for editing typos, fixing formatting and removing or fixing erroneous information in ticket comments. All this while retaining the history of the edits.

Thanks for the quick response. I understand your position but initial implementation before cboos review supposed "Delete" comment feature support too. This is not the same request as "Edit" but very close. I agree with you that "Delete" can be done out of Trac core via a plugin(TicketDeletePlugin in our case). From my point of view "delete comment", "delete ticket" are basis operations which should be part of Trac installation package.

Removing a comment completely requires editing the ticket properties, something that was left out on purpose.

In my case I wanted simpler feature than TicketDeletePlugin supposes. I wanted to delete only comment text and do not touch "property section" of the comment anyway(if it needs I can do this via TicketDeletePlugin). It will be nice to have small optimization for the current behavior, if "property section" is empty then whole comment can be removed. So "Delete" button can be applied only for "pure text" comment without "property section". The empty comment: without "property section" and without "text" has not a big sense and only occupies free space on the Ticket page.Usually the history change of the comment text is not needed for the case too.
Some UseCase example: during stabilization phase of our product release we do massive robust testing. Of course there are a lot of crashes in the product(client/server applications in my case). QA adds the call stack of the crashes to the ticket related with the test. As QA does not understand product implementation he/she very often adds the call stack in a wrong "crash" ticket or add the same call stack some times. A developer/manager review QA`s comments and remove duplicate stack or move the call stack to suitable ticket(or open new ticket). Using current "Edit comment" feature implementation we can manage the UC completely.

You can, however, edit the comment and remove all the text. Editing the property history will remain outside of Trac core for now.

Admin panel for this purpose is good place for the property history deletion. And TicketDeletePlugin can do all I need. So I can live with this :).

You could file an enhancement request with the TicketDeletePlugin for improving its user interface.

Yes, the full ticket deletion must have preview mode before confirm deletion(in Admin panel) or delete ticket action should be called from the Ticket page. The both methods are possible, but second is more preferable for me as do not need admin panel using.

There will be no backport to 0.11-stable, and the final implementation is significantly different from the patches on this ticket, so I would advise updating to trunk if you need this functionality.

Ok, I will try the existing patch for 0.11.5. Unfortunately I could not use 0.12dev version as it is not stable for production using(I am trying it in VMWare demo site, not in a real project). We migrated to Trac 0.11.5 from Jira 3.6.5 and have ~ 1200 tickets in the project at the moment. Trac is the new system for us and I investigate all features/plugins carefully at least try to do this. It is not a simple task as there is some chaus and feature duplication in the Trac plugins :(. I am waiting for 0.12-stable release but till this happens should use 0.11.5.

comment:184 by Drops <drop_s@…>, 15 years ago

I have successfully patched my Trac 0.11.5 by comment-edit-11-5.patch. It works normal. Thanks Sebastian Krysmanski for that specific patch preparation. I used TortoiseMerge application to apply patch.

in reply to:  184 ; comment:185 by Remy Blank, 15 years ago

Replying to Drops <drop_s@…>:

I have successfully patched my Trac 0.11.5 by comment-edit-11-5.patch. It works normal.

Keep in mind that, due to the final implementation of this feature being significantly different from the initial patch, you may get strange results when upgrading to 0.12. You have been warned :-)

in reply to:  185 comment:186 by Drops <drop_s@…>, 15 years ago

Replying to rblank:

Keep in mind that, due to the final implementation of this feature being significantly different from the initial patch, you may get strange results when upgrading to 0.12. You have been warned :-)

Thanks for the warning, I understand this, but the feature is realy HELPFUL for us and after Jira we have even more than had(in Jira we could only delete a comement and create it agian). I still need to fix/imlement(via patch or plugin) next features to be comfortable with Trac: ticket relation #31 in some common way and possibility hide/display change history of a ticket like in "Custom Query", see proposition http://www.mail-archive.com/trac-users@googlegroups.com/msg11980.html. (it seems tracked by #7640, really the full ticket history which displyed by default look like a spam and complecated ticket`s reading). These are 2 features which asked by my developers. I think they should be in Trac core)

comment:188 by chris.alex.thomas@…, 15 years ago

Cc: chris.alex.thomas@… removed

in reply to:  185 ; comment:189 by thomas.moschny@…, 15 years ago

Replying to rblank:

Replying to Drops <drop_s@…>:

I have successfully patched my Trac 0.11.5 by comment-edit-11-5.patch. It works normal.

Keep in mind that, due to the final implementation of this feature being significantly different from the initial patch, you may get strange results when upgrading to 0.12. You have been warned :-)

As 0.12 is not yet released, I suppose more people will go that way (using the patch now, updating to 0.12 later). Could you elaborate a bit on what "strange results" and/or problems are to be expected?

in reply to:  189 comment:190 by Remy Blank, 15 years ago

Replying to thomas.moschny@…:

Could you elaborate a bit on what "strange results" and/or problems are to be expected?

I haven't done a comprehensive analysis, but there's at least one thing I remember: the patch stores the last modification date and author as a (tagged) string at the end of the comment, like ^^1256831214&rblank^^. So once you update to 0.12, all the comments that have been edited will have such a string at the end. Not pretty, but not a big issue either.

comment:191 by casey@…, 15 years ago

Cc: casey@… removed

Get me out of here …

by andrew.macheret@…, 15 years ago

Attachment: comments-edit-11-6.patch added

Fixed the 11.5 patch to work for 11.6

comment:192 by akabat, 15 years ago

Will there be a 0.11.7 patch? I tried the 0.11.6 patch but it failed.

comment:193 by Christian Boos, 15 years ago

No, sorry, there will not. Worse even, you should not use the patch for 0.11.x if you expect to upgrade one day to 0.12 and above (comment:190).

You should really consider upgrading to 0.12 if you want this feature (0.12b1 is out since beginning of this month, and no one has been injured by the upgrade ;-) ).

comment:194 by johan.zandin@…, 15 years ago

Thanks for the change! TICKET_EDIT_COMMENT is now listed in http://trac.edgewall.org/wiki/TracPermissions too.

comment:195 by anonymous, 14 years ago

We have recently upgraded from .11.7 to .12.1 on our linux boxes. I have been unable to get comment editing working. Have given myself TRAC_ADMIN as well as TICKET_EDIT_COMMENT permissions. That does not seem to be enabling this feature for me.

I also do have a standalone version of Trac on my windows workstation. I seems to be working there for some reason.

Any suggestions or thoughts would be helpful in enabling this feature. Thanks in advance!

P.S:

Not sure if this is related. But I am also unable to get a live preview of my writen comments like I see in this ticket.

in reply to:  195 comment:196 by Christian Boos, 14 years ago

Replying to anonymous:

We have recently upgraded from .11.7 to .12.1 on our linux boxes. I have been unable to get comment editing working (…)

It's almost certainly an upgrade issue, you probably still use modified templates from your previous version. See #10041 for a similar situation.

Also, you should have created a new ticket for this issue (or asked for support), as there's a big number of people on CC: here… If the above tip is not enough (including the "support" one), then please create a new ticket.

comment:197 by anonymous, 14 years ago

Yes. That was it! We have modified our ticket template file. Thank you for you very quick response.

comment:198 by mromani@…, 13 years ago

For those needing a quick edit on a ticket comment but unable or unwilling to upgrade to the latest version of track, here's a tip: look into "ticket_change" table. Identify the row you want to change and edit the "newvalue" field.

sqlite3 /path/to/trac/repos/db/trac.db

select * from ticket_change where ticket=<number of ticket which the comment you want to edit belongs>

Build where clause to uniquely identify the table row you want to edit (hint: look at the tables' index).

update ticket_change set newvalue=<correct version of comment> where <condition to select the row>

Harder said than done.

HTH

comment:199 by Sebastian Krysmanski <sebastian@…>, 10 years ago

Cc: sebastian@… removed

comment:200 by Glenn Horton-Smith <gahs@…>, 5 years ago

Cc: gahs@… removed

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Remy Blank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Remy Blank to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.