Remy Blank avatar Remy Blank committed eab1be2

0.13dev: Improved the handling of concurrent ticket edits. New comments added while editing are added and highlighted. The user can review her changes and submit if desired.

Part of #7145.

Comments (0)

Files changed (10)

trac/htdocs/css/ticket.css

 }
 #trac-comment-editor .wikitoolbar { margin-left: -1px }
 #trac-add-comment :link, #trac-add-comment :visited { color: #b00 }
+.trac-new { border-left: 0.31em solid #c0f0c0; padding-left: 0.31em; }
 #changelog h3, #ticketchange h3 {
  border-bottom: 1px solid #d7d7d7;
  color: #999;

trac/htdocs/js/threaded_comments.js

   var comments = null;
   var toggle = $('#trac-threaded-toggle');
   toggle.click(function() {
-    if ($(this).checked()) {
-      if (!comments)
-        comments = $("div.change");
+    $(this).toggleClass('checked');
+    if ($(this).hasClass('checked')) {
+      comments = $("div.change");
       comments.each(function() {
         var children = $("a.follow-up", this).map(function() {
           var cnum = $(this).attr("href").replace('#comment:', '');

trac/ticket/model.py

         return self.id
 
     def save_changes(self, author=None, comment=None, when=None, db=None,
-                     cnum=''):
+                     cnum='', replyto=None):
         """
         Store ticket changes in the database. The ticket must already exist in
         the database.  Returns False if there were no changes to save, True
 
         :since 0.13: the `db` parameter is no longer needed and will be removed
         in version 0.14
+        :since 0.13: the `cnum` parameter is deprecated, and threading should
+        be controlled with the `replyto` argument
         """
         assert self.exists, "Cannot update a new ticket"
 
                     pass
 
         with self.env.db_transaction as db:
+            db("UPDATE ticket SET changetime=%s WHERE id=%s",
+               (when_ts, self.id))
+            
             # find cnum if it isn't provided
-            comment_num = cnum
-            if not comment_num:
+            if not cnum:
                 num = 0
                 for ts, old in db("""
                         SELECT DISTINCT tc1.time, COALESCE(tc2.oldvalue,'')
                         break
                     except ValueError:
                         num += 1
-                comment_num = str(num + 1)
+                cnum = str(num + 1)
+                if replyto:
+                    cnum = '%s.%s' % (replyto, cnum)
 
             # store fields
             for name in self._old.keys():
             db("""INSERT INTO ticket_change
                     (ticket,time,author,field,oldvalue,newvalue)
                   VALUES (%s,%s,%s,'comment',%s,%s)
-                  """, (self.id, when_ts, author, comment_num, comment))
-
-            db("UPDATE ticket SET changetime=%s WHERE id=%s",
-               (when_ts, self.id))
+                  """, (self.id, when_ts, author, cnum, comment))
 
         old_values = self._old
         self._old = {}
 
         for listener in TicketSystem(self.env).change_listeners:
             listener.ticket_changed(self, comment, author, old_values)
-        return True
+        return int(cnum.rsplit('.', 1)[-1])
 
     def get_changelog(self, when=None, db=None):
         """Return the changelog as a list of tuples of the form
         for listener in TicketSystem(self.env).change_listeners:
             listener.ticket_deleted(self)
 
-    def get_change(self, cnum, db=None):
-        """Return a ticket change by its number.
+    def get_change(self, cnum=None, cdate=None, db=None):
+        """Return a ticket change by its number or date.
 
         :since 0.13: the `db` parameter is no longer needed and will be removed
         in version 0.14
         """
-        row = self._find_change(cnum)
-        if row:
-            ts, author, comment = row
-            fields = {}
-            change = {'date': from_utimestamp(ts),
-                      'author': author, 'fields': fields}
-            for field, author, old, new in self.env.db_query("""
-                    SELECT field, author, oldvalue, newvalue 
-                    FROM ticket_change WHERE ticket=%s AND time=%s
-                    """, (self.id, ts)):
-                fields[field] = {'author': author, 'old': old, 'new': new}
+        if cdate is None:
+            row = self._find_change(cnum)
+            if not row:
+                return
+            cdate = from_utimestamp(row[0])
+        ts = to_utimestamp(cdate)
+        fields = {}
+        change = {'date': cdate, 'fields': fields}
+        for field, author, old, new in self.env.db_query("""
+                SELECT field, author, oldvalue, newvalue 
+                FROM ticket_change WHERE ticket=%s AND time=%s
+                """, (self.id, ts)):
+            fields[field] = {'author': author, 'old': old, 'new': new}
+            if field == 'comment':
+                change['author'] = author
+            elif not field.startswith('_'):
+                change.setdefault('author', author)
+        if fields:
             return change
 
-    def delete_change(self, cnum):
-        """Delete a ticket change."""
-        row = self._find_change(cnum)
-        if not row:
-            return
-        ts = row[0]
+    def delete_change(self, cnum=None, cdate=None):
+        """Delete a ticket change identified by its number or date."""
+        if cdate is None:
+            row = self._find_change(cnum)
+            if not row:
+                return
+            cdate = from_utimestamp(row[0])
+        ts = to_utimestamp(cdate)
         with self.env.db_transaction as db:
             # Find modified fields and their previous value
             fields = [(field, old, new)
 
         self.values['changetime'] = when
 
-    def get_comment_history(self, cnum, db=None):
-        """Retrieve the edit history of comment `cnum`.
+    def get_comment_history(self, cnum=None, cdate=None, db=None):
+        """Retrieve the edit history of a comment identified by its number or
+        date.
 
         :since 0.13: the `db` parameter is no longer needed and will be removed
         in version 0.14
         """
-        row = self._find_change(cnum)
-        if row:
+        if cdate is None:
+            row = self._find_change(cnum)
+            if not row:
+                return
             ts0, author0, last_comment = row
-            with self.env.db_query as db:
-                # Get all fields of the form "_comment%d"
-                rows = db("""SELECT field, author, oldvalue, newvalue 
-                             FROM ticket_change 
-                             WHERE ticket=%%s AND time=%%s AND field %s
-                             """ % db.like(),
-                             (self.id, ts0, db.like_escape('_comment') + '%'))
-                rows = sorted((int(field[8:]), author, old, new)
-                              for field, author, old, new in rows)
-                history = []
-                for rev, author, comment, ts in rows:
-                    history.append((rev, from_utimestamp(long(ts0)), author0,
-                                    comment))
-                    ts0, author0 = ts, author
-                history.sort()
-                rev = history[-1][0] + 1 if history else 0
+        else:
+            ts0, author0, last_comment = to_utimestamp(cdate), None, None
+        with self.env.db_query as db:
+            # Get last comment and author if not available
+            if last_comment is None:
+                last_comment = ''
+                for author0, last_comment in db("""
+                        SELECT author, newvalue FROM ticket_change 
+                        WHERE ticket=%s AND time=%s AND field='comment'
+                        """, (self.id, ts0)):
+                    break
+            if author0 is None:
+                for author0, last_comment in db("""
+                        SELECT author, new FROM ticket_change 
+                        WHERE ticket=%%s AND time=%%s AND NOT field %s LIMIT 1
+                        """ % db.like(),
+                        (self.id, ts0, db.like_escape('_') + '%')):
+                    break
+                else:
+                    return
+                
+            # Get all fields of the form "_comment%d"
+            rows = db("""SELECT field, author, oldvalue, newvalue 
+                         FROM ticket_change 
+                         WHERE ticket=%%s AND time=%%s AND field %s
+                         """ % db.like(),
+                         (self.id, ts0, db.like_escape('_comment') + '%'))
+            rows = sorted((int(field[8:]), author, old, new)
+                          for field, author, old, new in rows)
+            history = []
+            for rev, author, comment, ts in rows:
                 history.append((rev, from_utimestamp(long(ts0)), author0,
-                                last_comment))
-                return history
+                                comment))
+                ts0, author0 = ts, author
+            history.sort()
+            rev = history[-1][0] + 1 if history else 0
+            history.append((rev, from_utimestamp(long(ts0)), author0,
+                            last_comment))
+            return history
 
     def _find_change(self, cnum):
         """Find a comment by its number."""

trac/ticket/templates/ticket.html

                      .blur(function() { comment_focused = false; });
         $("#propertyform").autoSubmit({preview: '1'}, function(data, reply) {
           var items = $(reply);
+          // Update ticket box
           $("#ticket").replaceWith(items.filter('#ticket'));
-          var changes = $("#ticketchange-content").html(items.filter('ul.changes, div.comment'));
-          var show_preview = changes.children().length != 0;
+          // Unthread and update changelog
+          var threaded_toggle = $('#trac-threaded-toggle');
+          if (threaded_toggle.checked())
+            threaded_toggle.click();
+          $("#changelog").replaceWith(items.filter("#changelog"));
+          // Show warning
+          var new_changes = $("#changelog .trac-new");
+          $("#trac-edit-warning").toggle(new_changes.length != 0);
+          if (new_changes.length != 0)
+            $("#changelog").parent().show().removeClass("collapsed");
+          // Update view time
+          $("#propertyform input[name='view_time']").replaceWith(items.filter("input[name='view_time']"));
+          // Update preview
+          var preview = $("#ticketchange").html(items.filter('#preview').children());
+          var show_preview = preview.children().length != 0;
           $("#ticketchange").toggle(show_preview);
+          // Collapse property form if comment editor has focus
           if (show_preview && comment_focused)
             $("#modify").parent().addClass("collapsed");
         });
         });
         /*]]>*/
         <py:if test="preview_mode">
-        $("#changelog").parent().toggleClass("collapsed");
         $("#attachments").toggleClass("collapsed");
         $("#trac-add-comment").scrollToTop();
         </py:if>
                       py:with="alist = attachments; foldable = True"/>
         </py:if>
 
-        <div py:if="ticket.exists and changes">
+        <div style="${'display: none' if not changes else None}">
           <form id="trac-threaded-form" method="get" action="" style="display: none">
             <div>
               <input id="trac-threaded-toggle" type="checkbox" />
           <h2 class="foldable">Change History</h2>
 
           <div id="changelog">
-            <py:for each="change in changes"
-                    py:with="can_edit_comment = has_edit_comment or (authname and authname != 'anonymous'
-                                                                     and authname == change.author);
-                             show_editor = can_edit_comment and str(change.cnum) == cnum_edit;
-                             show_history = str(change.cnum) == cnum_hist;
-                             max_version = max(change.comment_history);
-                             comment_version = int(cversion or 0) if show_history else max_version;
-                             show_buttons = not show_editor and comment_version == max_version">
-              <div class="change${not show_buttons and ' trac-nobuttons' or ''}"
-                   id="${'trac-change-%d' % change.cnum if 'cnum' in change else None}">
-                <h3 class="change">
-                  <span class="threading" py:if="'cnum' in change"
-                        py:with="change_replies = replies.get(str(change.cnum), [])">
-                    <span id="comment:$change.cnum" class="cnum">${commentref('comment:', change.cnum)}</span>
-                    <py:if test="change_replies or 'replyto' in change">
-                      <py:if test="'replyto' in change">
-                        in reply to: ${commentref('&uarr;&nbsp;', change.replyto)}
-                        <py:if test="change_replies">; </py:if>
-                      </py:if>
-                      <py:if test="change_replies">
-                        <i18n:choose numeral="len(change_replies)">
-                          <span i18n:singular="">follow-up:</span>
-                          <span i18n:plural="">follow-ups:</span>
-                        </i18n:choose>
-                        <py:for each="reply in change_replies">
-                          ${commentref('&darr;&nbsp;', reply, 'follow-up')}
-                        </py:for></py:if>
-                    </py:if>
-                  </span>
-                  <i18n:msg params="date, author">Changed ${pretty_dateinfo(change.date)} by ${authorinfo(change.author)}</i18n:msg>
-                </h3>
-                <py:if test="show_buttons">
-                  <form py:if="'cnum' in change and can_edit_comment" method="get" action="#comment:${change.cnum}">
-                    <div class="inlinebuttons">
-                      <input type="hidden" name="cnum_edit" value="${change.cnum}"/>
-                      <input type="submit" value="${_('Edit')}" title="${_('Edit comment %(cnum)s', cnum=change.cnum)}"/>
-                    </div>
-                  </form>
-                  <form py:if="'cnum' in change and can_append" id="reply-to-comment-${change.cnum}"
-                        method="get" action="#comment">
-                    <div class="inlinebuttons">
-                      <input type="hidden" name="replyto" value="${change.cnum}"/>
-                      <input type="submit" value="${_('Reply')}" title="${_('Reply to comment %(cnum)s', cnum=change.cnum)}"/>
-                    </div>
-                  </form>
-                </py:if>
+            <py:for each="change in changes">
+              <div class="change${' trac-new' if change.date > start_time and 'attachment' not in change.fields else None}"
+                   id="${'trac-change-%d-%d' % (change.cnum, to_utimestamp(change.date)) if 'cnum' in change else None}">
                 <xi:include href="ticket_change.html"/>
-                <div py:if="not show_editor and len(change.comment_history) > 1" py:choose=""
-                     class="trac-lastedit ${'trac-shade' if comment_version != max_version else None}">
-                  <i18n:msg params="version, date, author" py:when="comment_version != max_version">
-                      Version ${comment_version}, edited ${pretty_dateinfo(change.comment_history[comment_version].date)}
-                      by ${authorinfo(change.comment_history[comment_version].author)}
-                  </i18n:msg>
-                  <i18n:msg params="date, author" py:otherwise="">
-                      Last edited ${pretty_dateinfo(change.comment_history[comment_version].date)}
-                      by ${authorinfo(change.comment_history[comment_version].author)}
-                  </i18n:msg>
-                  <py:if test="comment_version > 0">
-                    (<a href="${href.ticket(ticket.id, cnum_hist=change.cnum, cversion=comment_version - 1)
-                               }#comment:${change.cnum}">previous</a>)
-                  </py:if>
-                  <py:if test="comment_version &lt; max_version">
-                    (<a href="${href.ticket(ticket.id, cnum_hist=change.cnum, cversion=comment_version + 1)
-                               }#comment:${change.cnum}">next</a>)
-                  </py:if>
-                  <py:if test="comment_version > 0">
-                    (<a href="${href.ticket(ticket.id, action='comment-diff', cnum=change.cnum,
-                                            version=comment_version)}">diff</a>)
-                  </py:if>
-                </div>
               </div>
             </py:for>
           </div>
             action="${href.ticket(ticket.id) + '#trac-add-comment' if ticket.exists
                       else href.newticket() + '#ticket'}">
         <!--! Add comment -->
-        <div py:if="ticket.exists and can_append" class="field">
+        <div py:if="ticket.exists and can_append" id="trac-add-comment" class="field">
           <div class="trac-nav">
             <a href="#content" title="View ticket fields and description">View</a> &uarr;
           </div>
-          <h2 id="trac-add-comment">
+          <h2>
             <a id="edit" onfocus="$('#comment').get(0).focus()">Add a comment</a>
           </h2>
+          <div id="trac-edit-warning" class="warning system-message"
+               style="${'display: none' if start_time == ticket['changetime'] else None}">
+            This ticket has been modified since you started editing. You should review the
+            <em class="trac-new">other modifications</em> which have been appended above.
+            You can nevertheless proceed and submit your changes if you wish so.
+          </div>
           <!--! Comment field -->
           <fieldset class="iefix">
             <label for="comment" i18n:msg="">You may use
             <textarea id="comment" name="comment" class="wikitext trac-resizable" rows="10" cols="78">
 ${comment}</textarea>
           </fieldset>
-          <div py:if="preview_mode and chrome.warnings" i18n:msg="" class="warning system-message">
-            Warnings are shown at the <a href="#warning">top of the page</a>. The ticket validation
-            may have failed.
-          </div>
         </div>
 
         <div>
           </div>
         </div>
 
+        <!--! Preview of ticket changes -->
+        <div py:if="ticket.exists and can_append" id="ticketchange" class="ticketdraft"
+             style="${'display: none' if not (change_preview.fields or change_preview.comment)
+                                         or cnum_edit is not None else None}">
+          <xi:include href="ticket_change.html" py:with="change = change_preview"/>
+        </div>
+
         <!--! Author or Reporter -->
         <div py:if="authname == 'anonymous'" class="field">
           <fieldset py:choose="">
           </fieldset>
         </div>
 
-        <!--! Preview of ticket changes -->
-        <div py:if="ticket.exists and can_append" id="ticketchange" class="ticketdraft"
-             style="${'display: none' if not (change_preview.fields or change_preview.comment)
-                                         or cnum_edit is not None else None}">
-          <h3 class="change" id="${'comment:%d' % change_preview.cnum if 'cnum' in change_preview else None}">
-            <span class="threading" py:if="'replyto' in change_preview">
-              in reply to: ${commentref('&uarr;&nbsp;', change_preview.replyto)}
-            </span>
-            <i18n:msg params="author">Changed by ${authorinfo(change_preview.author)}</i18n:msg>
-          </h3>
-          <div id="ticketchange-content"><xi:include href="ticket_change.html" py:with="change = change_preview"/></div>
-        </div>
-
         <!--! Attachment on creation checkbox -->
         <p py:if="not ticket.exists and 'ATTACHMENT_CREATE' in perm(ticket.resource.child('attachment'))">
           <label>
         </div>
         <div class="buttons">
           <py:if test="ticket.exists">
-            <input type="hidden" name="ts" value="${timestamp}" />
+            <input type="hidden" name="start_time" value="${to_utimestamp(start_time)}" />
+            <input type="hidden" name="view_time" value="${to_utimestamp(ticket['changetime'])}" />
             <input type="hidden" name="replyto" value="${replyto}" />
-            <input type="hidden" name="cnum" value="${cnum}" />
           </py:if>
           <input type="submit" name="preview" value="${_('Preview')}" accesskey="r" />&nbsp;
           <input type="submit" name="submit" value="${_('Submit changes') if ticket.exists else _('Create ticket')}" />

trac/ticket/templates/ticket_change.html

 
 Arguments:
  - change: the change data
- - show_editor=False: if True, show a comment editor
- - edited_comment: the current value of the comment edito
- - cnum_edit: the comment number being edited
+ - hide_buttons=False: hide all buttons (Edit, Reply)
+ - cnum_edit=None: the comment number being edited
+ - edited_comment: the current value of the comment editor
+ - cnum_hist=None: the comment number for which to show a historical content
+ - can_append=False: True if the user is allowed to append to tickets
+ - has_edit_comment=False: True if the user is allowed to edit all comments
 -->
 <html xmlns="http://www.w3.org/1999/xhtml"
       xmlns:py="http://genshi.edgewall.org/"
       xmlns:xi="http://www.w3.org/2001/XInclude"
       xmlns:i18n="http://genshi.edgewall.org/i18n"
-      py:with="show_editor = value_of('show_editor', False)" py:strip="">
+      py:with="cnum = change.get('cnum'); hide_buttons = value_of('hide_buttons', False);
+               cnum_edit = value_of('cnum_edit'); cnum_hist = value_of('cnum_hist');
+               can_append = value_of('can_append', False); has_edit_comment = value_of('has_edit_comment', False);
+               can_edit_comment = has_edit_comment or (authname and authname != 'anonymous'
+                                                       and authname == change.author);
+               show_editor = can_edit_comment and str(cnum) == cnum_edit;
+               show_history = str(cnum) == cnum_hist;
+               max_version = max(change.comment_history) if change.comment_history else 0;
+               comment_version = int(cversion or 0) if show_history else max_version;
+               show_buttons = not hide_buttons and not show_editor and comment_version == max_version"
+      py:strip="">
+  <py:def function="commentref(prefix, cnum, cls=None)">
+    <a href="#comment:$cnum" class="$cls">$prefix$cnum</a>
+  </py:def>
+  <h3 class="change">
+    <span class="threading"
+          py:with="change_replies = replies.get(str(cnum), []) if 'cnum' in change else []">
+      <span py:if="'cnum' in change" id="comment:$cnum" class="cnum">${commentref('comment:', cnum)}</span>
+      <py:if test="'replyto' in change">
+        in reply to: ${commentref('&uarr;&nbsp;', change.replyto)}
+        <py:if test="change_replies">; </py:if>
+      </py:if>
+      <py:if test="change_replies">
+        <i18n:choose numeral="len(change_replies)">
+          <span i18n:singular="">follow-up:</span>
+          <span i18n:plural="">follow-ups:</span>
+        </i18n:choose>
+        <py:for each="reply in change_replies">
+          ${commentref('&darr;&nbsp;', reply, 'follow-up')}
+        </py:for>
+      </py:if>
+    </span>
+    <py:choose>
+      <py:when test="'date' in change">
+        <i18n:msg params="date, author">Changed ${pretty_dateinfo(change.date)} by ${authorinfo(change.author)}</i18n:msg>
+      </py:when>
+      <py:otherwise>
+        <i18n:msg params="author">Changed by ${authorinfo(change.author)}</i18n:msg>
+      </py:otherwise>
+    </py:choose>
+  </h3>
+  <div py:if="show_buttons" class="trac-ticket-buttons">
+    <form py:if="'cnum' in change and can_edit_comment" method="get" action="#comment:${cnum}">
+      <div class="inlinebuttons">
+        <input type="hidden" name="cnum_edit" value="${cnum}"/>
+        <input type="submit" value="${_('Edit')}" title="${_('Edit comment %(cnum)s', cnum=cnum)}"/>
+      </div>
+    </form>
+    <form py:if="'cnum' in change and can_append" id="reply-to-comment-${cnum}"
+          method="get" action="#comment">
+      <div class="inlinebuttons">
+        <input type="hidden" name="replyto" value="${cnum}"/>
+        <input type="submit" value="${_('Reply')}" title="${_('Reply to comment %(cnum)s', cnum=cnum)}"/>
+      </div>
+    </form>
+  </div>
   <ul py:if="change.fields" class="changes">
     <li py:for="field_name, field in sorted(change.fields.iteritems(), key=lambda item: item[1].label.lower())">
       <strong>${field.label}</strong>
     </li>
   </ul>
   <form py:if="show_editor" id="trac-comment-editor" method="post"
-        action="${href.ticket(ticket.id) + '#comment:%d' % change.cnum}">
+        action="${href.ticket(ticket.id) + '#comment:%d' % cnum}">
     <div>
       <textarea name="edited_comment" class="wikitext trac-resizable" rows="10" cols="78">
 ${edited_comment if edited_comment is not None else change.comment}</textarea>
-      <input type="hidden" name="cnum_edit" value="${change.cnum}"/>
+      <input type="hidden" name="cnum_edit" value="${cnum}"/>
     </div>
     <div class="buttons">
       <input type="submit" name="preview_comment" value="${_('Preview')}"
-             title="${_('Preview changes to comment %(cnum)s', cnum=change.cnum)}"/>
+             title="${_('Preview changes to comment %(cnum)s', cnum=cnum)}"/>
       <input type="submit" name="edit_comment" value="${_('Submit changes')}"
-             title="${_('Submit changes to comment %(cnum)s', cnum=change.cnum)}"/>
+             title="${_('Submit changes to comment %(cnum)s', cnum=cnum)}"/>
       <input type="submit" name="cancel_comment" value="${_('Cancel')}"
              title="Cancel comment edit"/>
     </div>
   </form>
   <py:choose>
-    <div py:when="str(change.cnum) == cnum_edit"
+    <div py:when="str(cnum) == cnum_edit"
          py:with="text = edited_comment if edited_comment is not None else change.comment"
          class="comment searchable ticketdraft" style="${'display: none' if not text else None}" xml:space="preserve">
       ${wiki_to_html(context, text, escape_newlines=preserve_newlines)}
       ${wiki_to_html(context, change.comment, escape_newlines=preserve_newlines)}
     </div>
   </py:choose>
+  <div py:if="not show_editor and len(change.comment_history) > 1" py:choose=""
+       class="trac-lastedit ${'trac-shade' if comment_version != max_version else None}">
+    <i18n:msg params="version, date, author" py:when="comment_version != max_version">
+        Version ${comment_version}, edited ${pretty_dateinfo(change.comment_history[comment_version].date)}
+        by ${authorinfo(change.comment_history[comment_version].author)}
+    </i18n:msg>
+    <i18n:msg params="date, author" py:otherwise="">
+        Last edited ${pretty_dateinfo(change.comment_history[comment_version].date)}
+        by ${authorinfo(change.comment_history[comment_version].author)}
+    </i18n:msg>
+    <py:if test="comment_version > 0">
+      (<a href="${href.ticket(ticket.id, cnum_hist=cnum, cversion=comment_version - 1)
+                 }#comment:${cnum}">previous</a>)
+    </py:if>
+    <py:if test="comment_version &lt; max_version">
+      (<a href="${href.ticket(ticket.id, cnum_hist=cnum, cversion=comment_version + 1)
+                 }#comment:${cnum}">next</a>)
+    </py:if>
+    <py:if test="comment_version > 0">
+      (<a href="${href.ticket(ticket.id, action='comment-diff', cnum=cnum,
+                              version=comment_version)}">diff</a>)
+    </py:if>
+  </div>
 </html>

trac/ticket/templates/ticket_preview.html

 <!--!
-Render a ticket box preview and a ticket change preview, to be sent as a reply
-to a ticket change auto-preview.
+Render data relevant to automatic ticket preview.
 -->
 <html xmlns="http://www.w3.org/1999/xhtml"
       xmlns:py="http://genshi.edgewall.org/"
       xmlns:xi="http://www.w3.org/2001/XInclude"
       xmlns:i18n="http://genshi.edgewall.org/i18n"
+      py:with="can_append = 'TICKET_APPEND' in perm(ticket.resource);
+               has_edit_comment = 'TICKET_EDIT_COMMENT' in perm(ticket.resource);"
       py:strip="">
-  <xi:include href="ticket_box.html" py:with="can_append = 'TICKET_APPEND' in perm(ticket.resource)"/>
-  <xi:include href="ticket_change.html" py:with="change = change_preview"/>
+  <xi:include href="ticket_box.html"/>
+  <div id="changelog">
+    <div py:for="change in changes"
+         class="change${' trac-new' if change.date > start_time and 'attachment' not in change.fields else None}"
+         id="${'trac-change-%d-%d' % (change.cnum, to_utimestamp(change.date)) if 'cnum' in change else None}">
+      <xi:include href="ticket_change.html" py:with="edited_comment = None; cnum_edit = 0"/>
+    </div>
+  </div>
+  <input type="hidden" name="view_time" value="${to_utimestamp(ticket['changetime'])}"/>
+  <div id="preview"><xi:include py:if="change_preview.fields or change_preview.comment"
+                                href="ticket_change.html" py:with="change = change_preview"/></div>
 </html>

trac/ticket/tests/model.py

         return from_utimestamp(ts)
     
     def assertChange(self, ticket, cnum, date, author, **fields):
-        change = ticket.get_change(cnum)
+        change = ticket.get_change(cnum=cnum)
         self.assertEqual(dict(date=date, author=author, fields=fields), change)
     
 
             ticket.modify_comment(self._find_change(ticket, 1),
                                   'joe (%d)' % i,
                                   'Comment 1 (%d)' % i, t[-1])
-        history = ticket.get_comment_history(1)
+        history = ticket.get_comment_history(cnum=1)
+        self.assertEqual((0, t[0], 'jack', 'Comment 1'), history[0])
+        for i in range(1, len(history)):
+            self.assertEqual((i, t[i], 'joe (%d)' % i,
+                             'Comment 1 (%d)' % i), history[i])
+        history = ticket.get_comment_history(cdate=self.t1)
         self.assertEqual((0, t[0], 'jack', 'Comment 1'), history[0])
         for i in range(1, len(history)):
             self.assertEqual((i, t[i], 'joe (%d)' % i,
         ticket = Ticket(self.env, self.id)
         self.assertEqual('a', ticket['keywords'])
         self.assertEqual('change4', ticket['foo'])
-        ticket.delete_change(4)
+        ticket.delete_change(cnum=4)
         self.assertEqual('a, b', ticket['keywords'])
         self.assertEqual('change3', ticket['foo'])
-        self.assertEqual(None, ticket.get_change(4))
-        self.assertNotEqual(None, ticket.get_change(3))
+        self.assertEqual(None, ticket.get_change(cnum=4))
+        self.assertNotEqual(None, ticket.get_change(cnum=3))
+        self.assertEqual(self.t3, ticket.time_changed)
+    
+    def test_delete_last_comment_by_date(self):
+        ticket = Ticket(self.env, self.id)
+        self.assertEqual('a', ticket['keywords'])
+        self.assertEqual('change4', ticket['foo'])
+        ticket.delete_change(cdate=self.t4)
+        self.assertEqual('a, b', ticket['keywords'])
+        self.assertEqual('change3', ticket['foo'])
+        self.assertEqual(None, ticket.get_change(cdate=self.t4))
+        self.assertNotEqual(None, ticket.get_change(cdate=self.t3))
         self.assertEqual(self.t3, ticket.time_changed)
     
     def test_delete_mid_comment(self):
             comment=dict(author='joe', old='4', new='Comment 4'),
             keywords=dict(author='joe', old='a, b', new='a'),
             foo=dict(author='joe', old='change3', new='change4'))
-        ticket.delete_change(3)
-        self.assertEqual(None, ticket.get_change(3))
+        ticket.delete_change(cnum=3)
+        self.assertEqual(None, ticket.get_change(cnum=3))
+        self.assertEqual('a', ticket['keywords'])
+        self.assertChange(ticket, 4, self.t4, 'joe',
+            comment=dict(author='joe', old='4', new='Comment 4'),
+            keywords=dict(author='joe', old='a, b, c', new='a'),
+            foo=dict(author='joe', old='change2', new='change4'))
+        self.assertEqual(self.t4, ticket.time_changed)
+        
+    def test_delete_mid_comment_by_date(self):
+        ticket = Ticket(self.env, self.id)
+        self.assertChange(ticket, 4, self.t4, 'joe',
+            comment=dict(author='joe', old='4', new='Comment 4'),
+            keywords=dict(author='joe', old='a, b', new='a'),
+            foo=dict(author='joe', old='change3', new='change4'))
+        ticket.delete_change(cdate=self.t3)
+        self.assertEqual(None, ticket.get_change(cdate=self.t3))
         self.assertEqual('a', ticket['keywords'])
         self.assertChange(ticket, 4, self.t4, 'joe',
             comment=dict(author='joe', old='4', new='Comment 4'),

trac/ticket/web_ui.py

             data.update({'action': None,
                          'reassign_owner': req.authname,
                          'resolve_resolution': None,
-                         'timestamp': str(ticket['changetime'])})
+                         'start_time': ticket['changetime']})
         elif req.method == 'POST': # 'Preview' or 'Submit'
             if 'cancel_comment' in req.args:
                 req.redirect(req.href.ticket(ticket.id))
                 req.args['preview'] = True
 
             # Preview an existing ticket (after a Preview or a failed Save)
+            start_time = from_utimestamp(long(req.args.get('start_time', 0)))
             data.update({
-                'action': action,
-                'timestamp': req.args.get('ts'),
+                'action': action, 'start_time': start_time,
                 'reassign_owner': (req.args.get('reassign_choice') 
                                    or req.authname),
                 'resolve_resolution': req.args.get('resolve_choice'),
                          'reassign_owner': req.authname,
                          'resolve_resolution': None,
                          # Store a timestamp for detecting "mid air collisions"
-                         'timestamp': str(ticket['changetime'])})
+                         'start_time': ticket['changetime']})
 
         data.update({'comment': req.args.get('comment'),
                      'cnum_edit': req.args.get('cnum_edit'),
         return 'ticket.html', data, None
 
     def _prepare_data(self, req, ticket, absurls=False):
-        return {'ticket': ticket,
+        return {'ticket': ticket, 'to_utimestamp': to_utimestamp,
                 'context': web_context(req, ticket.resource,
                                                 absurls=absurls),
                 'preserve_newlines': self.must_preserve_newlines}
 
         # Mid air collision?
         if ticket.exists and (ticket._old or comment or force_collision_check):
-            if req.args.get('ts') != str(ticket['changetime']):
+            changetime = ticket['changetime']
+            if req.args.get('view_time') != str(to_utimestamp(changetime)):
                 add_warning(req, _("Sorry, can not save your changes. "
                               "This ticket has been modified by someone else "
                               "since you started"))
 
         # Validate comment numbering
         try:
-            # comment index must be a number
-            int(req.args.get('cnum') or 0)
             # replyto must be 'description' or a number
             replyto = req.args.get('replyto')
             if replyto != 'description':
         req.redirect(req.href.ticket(ticket.id))
 
     def _do_save(self, req, ticket, action):
-        cnum = req.args.get('cnum')
-        replyto = req.args.get('replyto')
-        internal_cnum = cnum
-        if cnum and replyto: # record parent.child relationship
-            internal_cnum = '%s.%s' % (replyto, cnum)
-
         # Save the action controllers we need to call side-effects for before
         # we save the changes to the ticket.
         controllers = list(self._get_action_controllers(req, ticket, action))
 
         fragment = ''
         now = datetime.now(utc)
-        if ticket.save_changes(get_reporter_id(req, 'author'),
-                                     req.args.get('comment'), when=now,
-                                     cnum=internal_cnum):
-            fragment = '#comment:' + cnum if cnum else ''
+        cnum = ticket.save_changes(get_reporter_id(req, 'author'),
+                                   req.args.get('comment'), when=now,
+                                   replyto=req.args.get('replyto'))
+        if cnum:
+            fragment = '#comment:%d' % cnum
             try:
                 tn = TicketNotifyEmail(self.env)
                 tn.notify(ticket, newticket=False, modtime=now)
 
         # Insert change preview
         change_preview = {
-            'date': datetime.now(utc),
-            'author': author_id,
-            'fields': field_changes,
-            'preview': True,
+            'author': author_id, 'fields': field_changes, 'preview': True,
             'comment': req.args.get('comment', data.get('comment')),
+            'comment_history': {},
         }
         replyto = req.args.get('replyto')
         if replyto:
                                                             ticket[user])
         data.update({
             'context': context,
-            'fields': fields, 'changes': changes,
-            'replies': replies, 'cnum': cnum + 1,
+            'fields': fields, 'changes': changes, 'replies': replies,
             'attachments': AttachmentModule(self.env).attachment_data(context),
-            'action_controls': action_controls,
-            'action': selected_action,
+            'action_controls': action_controls, 'action': selected_action,
             'change_preview': change_preview,
         })
 

tracopt/ticket/deleter.py

 from trac.ticket.model import Ticket
 from trac.ticket.web_ui import TicketModule
 from trac.util import get_reporter_id
+from trac.util.datefmt import from_utimestamp
 from trac.util.translation import _
 from trac.web.api import IRequestFilter, IRequestHandler, ITemplateStreamFilter
 from trac.web.chrome import ITemplateProvider, add_notice, add_stylesheet
     # ITemplateStreamFilter methods
     
     def filter_stream(self, req, method, filename, stream, data):
-        if filename != 'ticket.html':
+        if filename not in ('ticket.html', 'ticket_preview.html'):
             return stream
         ticket = data.get('ticket')
         if not (ticket and ticket.exists
         
         def delete_comment():
             for event in buffer:
-                cnum = event[1][1].get('id')[12:]
+                cnum, cdate = event[1][1].get('id')[12:].split('-', 1)
                 return tag.form(
                     tag.div(
                         tag.input(type='hidden', name='action',
                                   value='delete-comment'),
                         tag.input(type='hidden', name='cnum', value=cnum),
+                        tag.input(type='hidden', name='cdate', value=cdate),
                         tag.input(type='submit', value=_('Delete'),
                                   title=_('Delete comment %(num)s',
                                           num=cnum)),
         return stream | Transformer('//div[@class="description"]'
                                     '/h3[@id="comment:description"]') \
             .after(delete_ticket).end() \
-            .select('//div[@class="change"]/@id') \
+            .select('//div[starts-with(@class, "change")]/@id') \
             .copy(buffer).end() \
-            .select('//div[@class="change" and @id]/h3[@class="change"]') \
-            .after(delete_comment)
+            .select('//div[starts-with(@class, "change") and @id]'
+                    '/div[@class="trac-ticket-buttons"]') \
+            .prepend(delete_comment)
 
     # IRequestFilter methods
     
         req.perm('ticket', id).require('TICKET_ADMIN')
         ticket = Ticket(self.env, id)
         action = req.args['action']
+        cnum = req.args.get('cnum')
         if req.method == 'POST':
             if 'cancel' in req.args:
                 href = req.href.ticket(id)
                 if action == 'delete-comment':
-                    href += '#comment:%s' % req.args.get('cnum')
+                    href += '#comment:%s' % cnum
                 req.redirect(href)
             
             if action == 'delete':
                 req.redirect(req.href())
             
             elif action == 'delete-comment':
-                cnum = int(req.args.get('cnum'))
-                ticket.delete_change(cnum)
+                cdate = from_utimestamp(long(req.args.get('cdate')))
+                ticket.delete_change(cdate=cdate)
                 add_notice(req, _('The ticket comment %(num)s on ticket '
                                   '#%(id)s has been deleted.',
                                   num=cnum, id=ticket.id))
         data = tm._prepare_data(req, ticket)
         tm._insert_ticket_data(req, ticket, data,
                                get_reporter_id(req, 'author'), {})
-        data.update(action=action, del_cnum=None)
+        data.update(action=action, cdate=None)
         
         if action == 'delete-comment':
-            cnum = int(req.args.get('cnum'))
-            data['del_cnum'] = cnum
+            data['cdate'] = req.args.get('cdate')
+            cdate = from_utimestamp(long(data['cdate']))
             for change in data['changes']:
-                if change.get('cnum') == cnum:
+                if change.get('date') == cdate:
                     data['change'] = change
+                    data['cnum'] = change.get('cnum')
                     break
             else:
                 raise TracError(_('Comment %(num)s not found', num=cnum))

tracopt/ticket/templates/ticket_delete.html

   <head>
     <title py:choose="action">
       <py:when test="'delete'"><i18n:msg params="id">Delete Ticket #$ticket.id</i18n:msg></py:when>
-      <py:otherwise><i18n:msg params="num, id">Delete comment $del_cnum on Ticket #$ticket.id</i18n:msg></py:otherwise>
+      <py:otherwise><i18n:msg params="num, id">Delete comment $cnum on Ticket #$ticket.id</i18n:msg></py:otherwise>
     </title>
   </head>
 
       </py:when>
 
       <py:otherwise>
-        <h1 i18n:msg="num, id">Delete comment $del_cnum on Ticket #$ticket.id</h1>
+        <h1 i18n:msg="num, id">Delete comment $cnum on Ticket #$ticket.id</h1>
         
         <div id="changelog">
           <div class="change">
-            <h3 class="change">
-              <span class="threading" py:if="'cnum' in change"
-                    py:with="change_replies = replies.get(str(change.cnum), [])">
-                <span class="cnum">comment:$change.cnum</span>
-              </span>
-              <i18n:msg params="date, author">Changed ${pretty_dateinfo(change.date)} by ${authorinfo(change.author)}</i18n:msg>
-            </h3>
-            <xi:include href="ticket_change.html"/>
+            <xi:include href="ticket_change.html" py:with="hide_buttons = True"/>
           </div>
         </div>
         
         <form id="edit" action="" method="post">
           <div>
             <input type="hidden" name="action" value="delete-comment"/>
-            <input type="hidden" name="cnum" value="$del_cnum"/>
+            <input type="hidden" name="cnum" value="$cnum"/>
+            <input type="hidden" name="cdate" value="$cdate"/>
             <p><strong>Are you sure you want to delete this ticket comment?</strong><br/>
                This is an irreversible operation.</p>
           </div>
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.