You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@allura.apache.org by br...@apache.org on 2019/05/17 16:01:08 UTC

[allura] 02/05: [#8265] avoid persisting tickets with None values, if error happened after new()

This is an automated email from the ASF dual-hosted git repository.

brondsem pushed a commit to branch db/8265
in repository https://gitbox.apache.org/repos/asf/allura.git

commit 4b53e49202916395d4585bce88faff251c0b468d
Author: Dave Brondsema <da...@brondsema.net>
AuthorDate: Thu May 16 17:21:22 2019 -0400

    [#8265] avoid persisting tickets with None values, if error happened after new()
---
 ForgeTracker/forgetracker/model/ticket.py | 27 ++++++++++++++++++++-------
 ForgeTracker/forgetracker/tracker_main.py |  5 +++--
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/ForgeTracker/forgetracker/model/ticket.py b/ForgeTracker/forgetracker/model/ticket.py
index 9d5611c..fc30b87 100644
--- a/ForgeTracker/forgetracker/model/ticket.py
+++ b/ForgeTracker/forgetracker/model/ticket.py
@@ -664,14 +664,16 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact):
         return d
 
     @classmethod
-    def new(cls):
-        '''Create a new ticket, safely (ensuring a unique ticket_num'''
+    def new(cls, form_fields=None):
+        '''Create a new ticket, safely (ensuring a unique ticket_num)'''
         while True:
             ticket_num = c.app.globals.next_ticket_num()
             ticket = cls(
                 app_config_id=c.app.config._id,
                 custom_fields=dict(),
                 ticket_num=ticket_num)
+            if form_fields:
+                ticket.update_fields_basics(form_fields)
             try:
                 session(ticket).flush(ticket)
                 h.log_action(log, 'opened').info('')
@@ -953,10 +955,11 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact):
             return 'nobody'
         return who.get_pref('display_name')
 
-    def update(self, ticket_form):
+    def update_fields_basics(self, ticket_form):
+        # "simple", non-persisting updates.  Must be safe to call within the Ticket.new() while its creating it
+
         # update is not allowed to change the ticket_num
         ticket_form.pop('ticket_num', None)
-        subscribe = ticket_form.pop('subscribe', False)
         self.labels = ticket_form.pop('labels', [])
         custom_users = set()
         other_custom_fields = set()
@@ -973,15 +976,15 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact):
             if 'custom_fields' not in ticket_form:
                 ticket_form['custom_fields'] = dict()
             ticket_form['custom_fields']['_milestone'] = milestone
-        attachment = None
-        if 'attachment' in ticket_form:
-            attachment = ticket_form.pop('attachment')
         for k, v in ticket_form.iteritems():
             if k == 'assigned_to':
                 if v:
                     user = c.project.user_in_project(v)
                     if user:
                         self.assigned_to_id = user._id
+            elif k in ('subscribe', 'attachment'):
+                # handled separately in update_fields_finish()
+                pass
             else:
                 setattr(self, k, v)
         if 'custom_fields' in ticket_form:
@@ -994,13 +997,23 @@ class Ticket(VersionedArtifact, ActivityObject, VotableArtifact):
                 elif k in other_custom_fields:
                     # strings are good enough for any other custom fields
                     self.custom_fields[k] = v
+
+    def update_fields_finish(self, ticket_form):
+        attachment = None
+        if 'attachment' in ticket_form:
+            attachment = ticket_form.pop('attachment')
         if attachment is not None:
             self.add_multiple_attachments(attachment)
             # flush the session to make attachments available in the
             # notification email
             ThreadLocalORMSession.flush_all()
+        subscribe = ticket_form.pop('subscribe', False)
         self.commit(subscribe=subscribe)
 
+    def update(self, ticket_form):
+        self.update_fields_basics(ticket_form)
+        self.update_fields_finish(ticket_form)
+
     def _move_attach(self, attachments, attach_metadata, app_config):
         for attach in attachments:
             attach.app_config_id = app_config._id
diff --git a/ForgeTracker/forgetracker/tracker_main.py b/ForgeTracker/forgetracker/tracker_main.py
index 1be5070..d2c567e 100644
--- a/ForgeTracker/forgetracker/tracker_main.py
+++ b/ForgeTracker/forgetracker/tracker_main.py
@@ -937,13 +937,14 @@ class RootController(BaseController, FeedController):
             if not ticket:
                 raise Exception('Ticket number not found.')
             require_access(ticket, 'update')
+            ticket.update_fields_basics(ticket_form)
         else:
             require_access(c.app, 'create')
             self.rate_limit(TM.Ticket, 'Ticket creation', redir='.')
-            ticket = TM.Ticket.new()
+            ticket = TM.Ticket.new(form_fields=ticket_form)
+        ticket.update_fields_finish(ticket_form)
         g.spam_checker.check(ticket_form['summary'] + u'\n' + ticket_form.get('description', ''), artifact=ticket,
                              user=c.user, content_type='ticket')
-        ticket.update(ticket_form)
         c.app.globals.invalidate_bin_counts()
         g.director.create_activity(c.user, 'created', ticket,
                                    related_nodes=[c.project], tags=['ticket'])