Edgewall Software
Modify

Opened 12 years ago

Closed 12 years ago

Last modified 12 years ago

#9217 closed enhancement (fixed)

Make comment optional for ticket.save_changes

Reported by: Felix Schwarz <felix.schwarz@…> Owned by: Felix Schwarz <felix.schwarz@…>
Priority: low Milestone: 0.12
Component: ticket system Version: 0.12dev
Severity: minor Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description

Ticket.save_changes currently requires an explicit specification of username and comment. However both arguments are actually optional as you can pass None and everything will work as well so I propose to declare them as optional which makes it a nicer API.

Attachments (0)

Change History (7)

comment:1 by Felix Schwarz <felix.schwarz@…>, 12 years ago

Due to #9218 I can't attach the patch so I'll paste it in the comments for now:

This one makes the comment parameter optional:

  • trac/ticket/model.py

    # HG changeset patch
    # Parent e537b06a46f5f056403884222aabd177d5728263
    
    diff -r e537b06a46f5 trac/ticket/model.py
    a b  
    234234
    235235        return self.id
    236236
    237     def save_changes(self, author, comment, when=None, db=None, cnum=''):
     237    def save_changes(self, author, comment=None, when=None, db=None, cnum=''):
    238238        """
    239239        Store ticket changes in the database. The ticket must already exist in
    240240        the database.  Returns False if there were no changes to save, True
  • trac/ticket/tests/model.py

    diff -r e537b06a46f5 trac/ticket/tests/model.py
    a b  
    123123        self.assertEqual(ticket_id, ticket.id)
    124124        self.assertEqual(ticket.resource.id, ticket_id)
    125125
     126    def test_can_save_ticket_without_explicit_comment(self):
     127        ticket = Ticket(self.env)
     128        ticket.insert()
     129        ticket['summary'] = 'another summary'
     130       
     131        ticket.save_changes('foo')
     132       
     133        changes = ticket.get_changelog()
     134        comment_change = filter(lambda change: change[2] == 'comment', changes)[0]
     135        self.assertEqual('', comment_change[3])
     136        self.assertEqual('', comment_change[4])
     137
     138
    126139    def test_ticket_default_values(self):
    127140        """
    128141        Verify that a ticket uses default values specified in the configuration
Last edited 12 years ago by Christian Boos (previous) (diff)

comment:2 by Felix Schwarz <felix.schwarz@…>, 12 years ago

This one makes the author parameter optional:

# HG changeset patch
# Parent 8eceeb03a8628e8dc127eaf2920cc83b616f7df6

diff -r 8eceeb03a862 trac/ticket/model.py
--- a/trac/ticket/model.py	Sun Apr 11 15:50:40 2010 +0200
+++ b/trac/ticket/model.py	Sun Apr 11 15:58:12 2010 +0200
@@ -234,7 +234,7 @@
 
         return self.id
 
-    def save_changes(self, author, comment=None, when=None, db=None, cnum=''):
+    def save_changes(self, author=None, comment=None, when=None, db=None, cnum=''):
         """
         Store ticket changes in the database. The ticket must already exist in
         the database.  Returns False if there were no changes to save, True
diff -r 8eceeb03a862 trac/ticket/tests/model.py
--- a/trac/ticket/tests/model.py	Sun Apr 11 15:50:40 2010 +0200
+++ b/trac/ticket/tests/model.py	Sun Apr 11 15:58:12 2010 +0200
@@ -135,6 +135,15 @@
         self.assertEqual('', comment_change[3])
         self.assertEqual('', comment_change[4])
 
+    def test_can_save_ticket_without_explicit_username(self):
+        ticket = Ticket(self.env)
+        ticket.insert()
+        ticket['summary'] = 'another summary'
+        
+        ticket.save_changes()
+        
+        for change in ticket.get_changelog():
+            self.assertEqual(None, change[1])
 
     def test_ticket_default_values(self):
         """
Version 0, edited 12 years ago by Felix Schwarz <felix.schwarz@…> (next)

comment:3 by Christian Boos, 12 years ago

Both changes look fine. Have you verified how a change without an author appears in the history? Listed as anonymous?

Please also consider adding directly the changeset comment in the patch, makes the lazy patch integrator happier ;-)

comment:4 by Felix Schwarz <felix.schwarz@…>, 12 years ago

The web_ui will render them as 'anonymous' though I did not explicitely test this as the current mock request is not suitable for testing the web_ui with unit tests (will do something about that in the next time).

However I noticed that my checkout was quite old so I'll re-diff with the latest trunk.

comment:5 by Felix Schwarz <felix.schwarz@…>, 12 years ago

Just noticed that the patches still apply without fuzz so I guess they are ready for commit.

comment:6 by Christian Boos, 12 years ago

Resolution: fixed
Status: newclosed

Both patches applied, in r9477 and r9478, thanks!

I had to rework a bit the first test, as it was not passing (change[3] was '1', auto-numbering).

comment:7 by Christian Boos, 12 years ago

Owner: set to Felix Schwarz <felix.schwarz@…>

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Felix Schwarz <felix.schwarz@…>.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Felix Schwarz <felix.schwarz@…> to the specified user.

Add Comment


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