Edgewall Software
Modify

Opened 10 years ago

Closed 5 years ago

Last modified 2 years ago

#454 closed enhancement (fixed)

[patch]Edit ticket comments

Reported by: tim.alsop@… Owned by: rblank
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@…, gahs@…, sebastian@…, ryano@…
Release Notes:
API 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@…> 6 years ago.
comment_edit.patch (10.7 KB) - added by David Sorber <baranovich@…> 5 years ago.
comment-edit-11.patch (11.3 KB) - added by jrgeller@… 5 years ago.
Comment_edit patch that applies cleanly to 0.11
comment-edit-11-5.patch (11.5 KB) - added by Sebastian Krysmanski <sebastian@…> 5 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 rblank 5 years ago.
Save comment edit history in _comment{n} pseudo-fields.
454-save-edit-history-r8561.2.patch (6.5 KB) - added by cboos 5 years ago.
rebased version, on top of ticket-edit-minor-fixes-r8561.diff
ticket-edit-minor-fixes-r8561.diff (7.6 KB) - added by cboos 5 years ago.
several minor enhancements on top of r8538, take 2
comments-edit-11-6.patch (11.4 KB) - added by andrew.macheret@… 5 years ago.
Fixed the 11.5 patch to work for 11.6

Download all attachments as: .zip

Change History (206)

comment:1 Changed 10 years ago by anonymous

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 Changed 10 years ago by anonymous

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

comment:3 Changed 10 years ago by daniel

  • Priority changed from normal to low
  • Summary changed from comment edit needed to Edit 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 Changed 10 years ago by anonymous

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 Changed 10 years ago by josh aka anonymous above

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 Changed 10 years ago by bkc@…

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 Changed 10 years ago by cboos@…

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 Changed 10 years ago by louis@…

  • Cc louis@… added

I'm interested in this too.

comment:9 Changed 9 years ago by anonymous

  • Cc chris@… added

comment:10 Changed 9 years ago by cboos

  • Owner changed from jonas to cboos
  • Priority changed from low to normal
  • Severity changed from normal to enhancement
  • Status changed from new to assigned

#1180 has been marked as duplicate of this ticket

comment:11 Changed 9 years ago by Florent Guillaume <fg@…>

  • Cc fg@… added

comment:12 Changed 9 years ago by anonymous

  • Cc jforcier@… added

comment:13 Changed 9 years ago by anonymous

  • Cc pkou@… added

comment:14 Changed 9 years ago by d.mills@…

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 Changed 9 years ago by anonymous

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 Changed 9 years ago by cboos

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 Changed 9 years ago by anonymous

  • 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 Changed 9 years ago by fg@…

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 Changed 9 years ago by Gunnar Wagenknecht <gunnar@…>

  • Cc gunnar@… added

comment:20 Changed 9 years ago by cboos

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 Changed 9 years ago by Joe Wreschnig <piman at sacredchao.net>

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 Changed 9 years ago by cboos

  • Priority changed from normal to high

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 Changed 9 years ago by Jeff Forcier (jforcier at strozllc dot com)

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 Changed 9 years ago by chris@…

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 Changed 9 years ago by anonymous

  • Cc louis@… chris@… fg@… jforcier@… pkou@… tiger@… gunnar@… removed
  • Owner changed from cboos to anonymous
  • Priority changed from high to lowest
  • Status changed from assigned to new

comment:26 Changed 9 years ago by mgood

  • Cc louis@… chris@… fg@… jforcier@… pkou@… tiger@… gunnar@… added
  • Priority changed from lowest to normal

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

comment:27 Changed 9 years ago by cboos

  • Milestone set to 1.0
  • Owner changed from anonymous to cboos
  • Priority changed from normal to high

comment:28 Changed 9 years ago by anonymous

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 Changed 9 years ago by chris@…

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

comment:30 Changed 9 years ago by Joe Wreschnig

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 Changed 9 years ago by lycos42@…

  • Cc lycos42@… added
  • Milestone changed from 1.0 to 0.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 Changed 9 years ago by anonymous

  • Cc shishz@… added

comment:33 Changed 9 years ago by jacob@…

  • 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 Changed 9 years ago by lycos42@…

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

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

comment:35 Changed 9 years ago by chris@…

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 Changed 9 years ago by cmlenz

  • Milestone changed from 0.9.1 to 0.9.2

comment:37 Changed 9 years ago by anonymous

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 Changed 9 years ago by fg@…

  • Cc fg@… removed

comment:39 Changed 9 years ago by cboos

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

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

comment:40 Changed 9 years ago by Gunnar Wagenknecht <gunnar@…>

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 Changed 9 years ago by chris@…

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

comment:42 Changed 9 years ago by vyt@…

  • Cc vyt@… added

comment:43 Changed 9 years ago by Gunnar Wagenknecht <gunnar@…>

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 Changed 9 years ago by cboos

  • Status changed from new to assigned

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 Changed 9 years ago by cmlenz

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 Changed 9 years ago by cmlenz

  • Milestone changed from 0.9.3 to 1.0

(Moving non-trivial enhancements to next milestone)

comment:47 Changed 9 years ago by anonymous

  • Cc steve.webster@… added

comment:48 Changed 9 years ago by cmlenz

  • Milestone changed from 1.0 to 0.10

comment:49 Changed 9 years ago by anonymous

  • Cc robin-trac@… added

comment:50 Changed 9 years ago by anonymous

  • Cc changed from 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 to 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 Changed 9 years ago by dkocher@…

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

comment:52 Changed 9 years ago by Markus Tacker <m@…>

  • Cc m@… added
  • Keywords SPAM added

comment:53 Changed 9 years ago by anonymous

  • 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 Changed 9 years ago by anonymous

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 Changed 9 years ago by eblot

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 Changed 9 years ago by chris@…

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 Changed 9 years ago by jacob@…

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 Changed 9 years ago by chris@…

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 Changed 9 years ago by Gunnar Wagenknecht <gunnar@…>

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 Changed 9 years ago by chris@…

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

comment:61 Changed 9 years ago by jacob@…

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 Changed 9 years ago by cboos

  • Status changed from assigned to new

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 Changed 9 years ago by andrei@…

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 Changed 9 years ago by anonymous

  • Cc mankoff@… added; louis@… removed

comment:65 Changed 9 years ago by anonymous

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

comment:66 Changed 9 years ago by anonymous

  • Cc jforcier@… removed

comment:67 Changed 8 years ago by anonymous

  • Cc casey@… added

comment:68 Changed 8 years ago by cboos

  • Milestone changed from 0.10 to 0.11
  • Priority changed from high to normal

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 Changed 8 years ago by Axel.Thimm@…

  • 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 Changed 8 years ago by chris@…

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 Changed 8 years ago by Chris Ashworth

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 Changed 8 years ago by anonymous

  • Cc dkirov@… added

comment:73 Changed 8 years ago by anonymous

  • Cc trac@… added

i also support verson tracking of ticket edits

comment:74 Changed 8 years ago by anonymous

  • Cc gunnar@… removed

comment:75 Changed 8 years ago by cboos

#3609 marked as duplicate.

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

  • 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 Changed 8 years ago by anonymous

  • Cc stuff@… added

comment:78 follow-up: Changed 8 years ago by anonymous

  • 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

comment:79 in reply to: ↑ 78 Changed 8 years ago by cboos

  • Severity changed from normal to major

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 Changed 8 years ago by anonymous

  • Cc dkirov@… removed

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

comment:81 Changed 8 years ago by Martin Delisle <martin.delisle@…>

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

comment:82 Changed 8 years ago by Martin Delisle <martin.delisle@…>

  • Keywords SPAM added

comment:83 Changed 8 years ago by maz <mzizka@…>

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 Changed 8 years ago by cboos

  • Milestone changed from 0.11 to 0.12

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

comment:85 Changed 8 years ago by anonymous

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 follow-up: Changed 8 years ago by trisweb

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 Changed 8 years ago by robinbowes

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 Changed 8 years ago by andrei@…

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 Changed 8 years ago by anonymous

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.

comment:89 in reply to: ↑ 86 Changed 8 years ago by eblot

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 Changed 8 years ago by anonymous

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 Changed 8 years ago by eblot

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 Changed 8 years ago by bkc

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

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

comment:93 Changed 8 years ago by anonymous

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 Changed 8 years ago by anonymous

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 follow-up: Changed 8 years ago by 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.

comment:96 in reply to: ↑ 95 Changed 8 years ago by anonymous

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 follow-up: Changed 8 years ago by anonymous

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.

comment:98 in reply to: ↑ 97 ; follow-up: Changed 8 years ago by 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.

comment:99 in reply to: ↑ 98 Changed 8 years ago by anonymous

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 follow-up: Changed 8 years ago by 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.

comment:101 in reply to: ↑ 100 ; follow-up: Changed 8 years ago by chris@…

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.

comment:102 in reply to: ↑ 101 ; follow-up: Changed 8 years ago by anonymous

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 follow-up: Changed 7 years ago by anonymous

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…

comment:104 in reply to: ↑ 103 Changed 7 years ago by cboos

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.

comment:105 in reply to: ↑ 102 Changed 7 years ago by anonymous

Replying to anonymous:

Replying to chris@growl:

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

comment:106 Changed 7 years ago by anonymous

  • Cc stuff@… removed

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

comment:107 Changed 7 years ago by basic

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

comment:108 Changed 7 years ago by red

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 Changed 7 years ago by mgood

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 Changed 7 years ago by robin-trac@…

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 Changed 7 years ago by cboos

  • Milestone changed from 0.12 to 0.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 follow-up: Changed 7 years ago by robin-trac@…

  • 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.

comment:113 in reply to: ↑ 112 ; follow-up: Changed 7 years ago by cboos

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 Changed 7 years ago by anonymous

  • Cc Heribert.Schuetz.extern@… added

comment:115 in reply to: ↑ 113 Changed 7 years ago by eblot

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 Changed 7 years ago by Ian Mayo

  • Cc ianmayo@… added

Add another interested party.

comment:117 follow-up: Changed 7 years ago by Benjamin Plaquevent <benjamin.plaquevent@…>

  • 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.

comment:118 in reply to: ↑ 117 Changed 7 years ago by eblot

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 follow-ups: Changed 7 years ago by anonymous

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

comment:120 in reply to: ↑ 119 Changed 7 years ago by eblot

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?

comment:121 in reply to: ↑ 119 Changed 7 years ago by cboos

  • 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 Changed 7 years ago by anonymous

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

comment:123 Changed 7 years ago by anonymous

  • Cc dbmarquardt@… added

comment:124 Changed 7 years ago by anonymous

  • Cc david.hopwood@… added

comment:125 Changed 7 years ago by vyt@…

  • Cc vyt@… removed

Sorry, I don't work with Trac now.

comment:126 Changed 7 years ago by anonymous

  • Cc benjamin.plaquevent@… removed

Still using Trac but no more interrested by this ticket…

comment:127 Changed 7 years ago by stephan@…

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 Changed 7 years ago by anonymous

  • Cc pierreroth64@… added

comment:129 Changed 7 years ago by cboos

  • Milestone changed from 0.11.1 to 0.12
  • Priority changed from normal to high
  • Severity changed from major to critical

#6872 closed as duplicate.

Bumping up the prio for the next round.

comment:130 Changed 6 years ago by anonymous

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 Changed 6 years ago by OllyBetts

  • 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 Changed 6 years ago by blake@…

  • Cc blake@… added

comment:133 Changed 6 years ago by Heribert.Schuetz.extern@…

  • Cc Heribert.Schuetz.extern@… removed

comment:134 Changed 6 years ago by olivier@…

  • Cc olivier@… added

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

comment:135 Changed 6 years ago by fho@…

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 Changed 6 years ago by bruce@…

+1

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

comment:137 Changed 6 years ago by trac@…

  • Cc trac@… removed

comment:138 Changed 6 years ago by anonymous

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

comment:139 Changed 6 years ago by anonymous

+1

comment:140 Changed 6 years ago by Simon <saimen54@…>

comment:141 Changed 6 years ago by Pierre Roth <pierreroth64@…>

  • Cc pierreroth64@… removed

comment:142 Changed 6 years ago by zsphinx@…

  • Cc zsphinx@… added

+1 (text to avoid appearing as spam)

comment:143 Changed 6 years ago by gary.wilson@…

  • Cc gary.wilson@… added

comment:144 Changed 6 years ago by cboos

  • Milestone changed from 0.13 to 0.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 Changed 6 years ago by anonymous

  • Resolution set to fixed
  • Status changed from new to closed

comment:146 Changed 6 years ago by robinbowes

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

comment:147 Changed 6 years ago by cboos

  • Resolution fixed deleted
  • Status changed from closed to reopened

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

Changed 6 years ago by David Sorber <baranovich@…>

comment:148 Changed 6 years ago by David Sorber <baranovich@…>

  • Summary changed from Edit ticket comments to [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 Changed 5 years ago by jribenfors@…

  • Cc jribenfors@… added

comment:150 follow-up: Changed 5 years ago by cboos

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!

Changed 5 years ago by David Sorber <baranovich@…>

comment:151 in reply to: ↑ 150 Changed 5 years ago by David Sorber <baranovich@…>

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 Changed 5 years ago by Erik Andersson <kirean@…>

  • Cc kirean@… added

comment:153 Changed 5 years ago by Jacob Kaplan-Moss <jacob@…>

  • Cc jacob@… removed

comment:154 Changed 5 years ago by jrgeller@…

  • 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 Changed 5 years ago by jrgeller@…

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 Changed 5 years ago by jrgeller@…

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.

Changed 5 years ago by jrgeller@…

Comment_edit patch that applies cleanly to 0.11

comment:157 Changed 5 years ago by anonymous

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 follow-up: Changed 5 years ago by anonymous

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 Changed 5 years ago by nick@…

  • 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 :\

comment:160 in reply to: ↑ 158 Changed 5 years ago by Glenn Horton-Smith <gahs@…>

  • 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 follow-up: Changed 5 years ago by rupert.thurner

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

comment:162 in reply to: ↑ 161 Changed 5 years ago by Sebastian Krysmanski <sebastian@…>

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 Changed 5 years ago by Sebastian Krysmanski <sebastian@…>

  • 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).

Changed 5 years ago by Sebastian Krysmanski <sebastian@…>

Patch against Trac 0.11.5 plus buttons displayed side by side

comment:164 Changed 5 years ago by rblank

  • Owner changed from cboos to rblank
  • Status changed from reopened to new

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

comment:165 Changed 5 years ago by rblank

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?

Changed 5 years ago by rblank

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

comment:166 Changed 5 years ago by rblank

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 follow-up: Changed 5 years ago by cboos

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.

Changed 5 years ago by cboos

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

comment:168 Changed 5 years ago by cboos

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 ;-)

Changed 5 years ago by cboos

several minor enhancements on top of r8538, take 2

comment:169 in reply to: ↑ 167 ; follow-up: Changed 5 years ago by rblank

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.

comment:170 in reply to: ↑ 169 Changed 5 years ago by cboos

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 Changed 5 years ago by Ryan Ollos <ryano@…>

  • Cc ryano@… added

comment:172 Changed 5 years ago by rblank

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 Changed 5 years ago by rblank

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 Changed 5 years ago by cboos

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 Changed 5 years ago by rblank

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 follow-up: Changed 5 years ago by cboos

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.

comment:177 in reply to: ↑ 176 Changed 5 years ago by rblank

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 Changed 5 years ago by rblank

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 Changed 5 years ago by David Sorber <baranovich@…>

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 Changed 5 years ago by rblank

  • Resolution set to fixed
  • Status changed from new to closed

No more reactions, considering as closed.

comment:181 Changed 5 years ago by anonymous

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 follow-up: Changed 5 years ago by 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.

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.

comment:183 in reply to: ↑ 182 Changed 5 years ago by Drops <drop_s@…>

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 follow-up: Changed 5 years ago by Drops <drop_s@…>

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.

comment:185 in reply to: ↑ 184 ; follow-ups: Changed 5 years ago by 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 :-)

comment:186 in reply to: ↑ 185 Changed 5 years ago by Drops <drop_s@…>

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 Changed 5 years ago by chris.alex.thomas@…

  • Cc chris.alex.thomas@… removed

comment:189 in reply to: ↑ 185 ; follow-up: Changed 5 years ago by thomas.moschny@…

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?

comment:190 in reply to: ↑ 189 Changed 5 years ago by rblank

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 Changed 5 years ago by casey@…

  • Cc casey@… removed

Get me out of here …

Changed 5 years ago by andrew.macheret@…

Fixed the 11.5 patch to work for 11.6

comment:192 Changed 4 years ago by akabat

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

comment:193 Changed 4 years ago by cboos

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 Changed 4 years ago by johan.zandin@…

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

comment:195 follow-up: Changed 4 years ago by anonymous

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.

comment:196 in reply to: ↑ 195 Changed 4 years ago by cboos

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 Changed 4 years ago by anonymous

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

comment:198 Changed 2 years ago by mromani@…

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

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as closed The owner will remain rblank.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from rblank to the specified user.
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.