Make comment optional for ticket.save_changes

[Felix Schwarz <felix.schwarz@…>]

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.

[Felix Schwarz <felix.schwarz@…>]

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

@@ -234 +234 @@
 
         return self.id
 
-    def save_changes(self, author, comment, when=None, db=None, cnum=''):
+    def save_changes(self, author, 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

trac/ticket/tests/model.py

@@ -123 +123 @@
         self.assertEqual(ticket_id, ticket.id)
         self.assertEqual(ticket.resource.id, ticket_id)
 
+    def test_can_save_ticket_without_explicit_comment(self):
+        ticket = Ticket(self.env)
+        ticket.insert()
+        ticket['summary'] = 'another summary'
+        
+        ticket.save_changes('foo')
+        
+        changes = ticket.get_changelog()
+        comment_change = filter(lambda change: change[2] == 'comment', changes)[0]
+        self.assertEqual('', comment_change[3])
+        self.assertEqual('', comment_change[4])
+
+
     def test_ticket_default_values(self):
         """
         Verify that a ticket uses default values specified in the configuration

[Felix Schwarz <felix.schwarz@…>]

This one makes the author parameter optional:

trac/ticket/model.py

@@ -234 +234 @@
 
         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

trac/ticket/tests/model.py

@@ -135 +135 @@
         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):
         """

[Christian Boos]

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

[Felix Schwarz <felix.schwarz@…>]

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.

[Felix Schwarz <felix.schwarz@…>]

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

[Christian Boos]

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