From 3c9ed27113d9c290f8a0a519c0fed0499a14ae56 Mon Sep 17 00:00:00 2001
From: Tanner Prestegard <tanner.prestegard@ligo.org>
Date: Thu, 4 May 2017 13:07:22 -0500
Subject: [PATCH] cleaning up some of the label creation/removal code

---
 gracedb/api.py        | 13 ++++++++-----
 gracedb/view_logic.py | 20 +++++++++++++-------
 2 files changed, 21 insertions(+), 12 deletions(-)

diff --git a/gracedb/api.py b/gracedb/api.py
index bb9e71605..cf00477a3 100644
--- a/gracedb/api.py
+++ b/gracedb/api.py
@@ -711,7 +711,7 @@ class EventLabel(APIView):
         if label is not None:
             theLabel = event.labelling_set.filter(label__name=label).all()
             if len(theLabel) < 1:
-                return Response("Label Not %s Found" % label,
+                return Response("Label %s Not Found" % label,
                         status=status.HTTP_404_NOT_FOUND)
             theLabel = theLabel[0]
             return Response(labelToDict(theLabel, request=request))
@@ -731,15 +731,18 @@ class EventLabel(APIView):
     @event_and_auth_required
     def put(self, request, event, label):
         try:
-            rv = create_label(event, request, label)
+            rv, label_created = create_label(event, request, label)
         except ValueError, e:
             return Response(e.message,
                         status=status.HTTP_400_BAD_REQUEST)
 
-        return Response(rv, status=status.HTTP_201_CREATED)
+        # Return response and status code
+        if label_created:
+            return Response(rv, status=status.HTTP_201_CREATED)
+        else:
+            return Response(rv, status=status.HTTP_200_OK)
 
     def delete(self, request, graceid, label):
-        #return Response("Not Implemented", status=status.HTTP_501_NOT_IMPLEMENTED)
         try:
             event = Event.getByGraceid(graceid)
             rv = delete_label(event, request, label)
@@ -747,7 +750,7 @@ class EventLabel(APIView):
             return Response(e.message,
                         status=status.HTTP_400_BAD_REQUEST)
 
-        return Response(rv, status=status.HTTP_201_CREATED)
+        return Response(rv, status=status.HTTP_200_OK)
 
 #==================================================================
 # EventLog
diff --git a/gracedb/view_logic.py b/gracedb/view_logic.py
index 42fc6af2e..cc3d4904e 100644
--- a/gracedb/view_logic.py
+++ b/gracedb/view_logic.py
@@ -162,8 +162,11 @@ def create_label(event, request, labelName, doAlert=True, doXMPP=True):
         raise ValueError("No such Label '%s'" % labelName)
 
     # Don't add a label more than once.
+    # track whether label is actually created so as to
+    # send the correct HTTP response code
+    label_created = False
     if label in event.labels.all():
-            d['warning'] = "Event %s already labeled with '%s'" % (event.graceid(), labelName)
+        d['warning'] = "Event %s already labeled with '%s'" % (event.graceid(), labelName)
     else:
         labelling = Labelling(
                 event = event,
@@ -171,22 +174,25 @@ def create_label(event, request, labelName, doAlert=True, doXMPP=True):
                 creator = creator
             )
         labelling.save()
+        label_created = True
         message = "Label: %s" % label.name
         log = EventLog(event=event, issuer=creator, comment=message)
         try:       
             log.save()
         except Exception as e:
             # XXX This looks a bit odd to me.
-            logger.exception('Problem saving log message')
+            logger.exception('Problem saving log message (%s)' % str(e))
             d['error'] = str(e)
 
         try:
             issueAlertForLabel(event, label, doXMPP, event_url=event_url)
         except Exception as e:
-            logger.exception('Problem saving log message')
+            logger.exception('Problem issuing alert (%s)' % str(e))
             d['warning'] = "Problem issuing alert (%s)" % str(e)
-    # XXX Strange return value.  Just warnings.  Can really be ignored, I think.
-    return json.dumps(d)
+
+    # Return warning/error messages (for passing back to client)
+    # and label_created bool
+    return json.dumps(d), label_created
 
 def delete_label(event, request, labelName):
     # This function deletes a label. It starts out a lot like the create
@@ -206,7 +212,7 @@ def delete_label(event, request, labelName):
     # error if it isn't. There might be a more elegant way of doing this.
     if label not in event.labels.all():
             d['warning'] = "No label '%s' associated with event %s" % (labelName, event.graceid())
-            raise ValueError( "No label '%s' associated with event %s" % (labelName, event.graceid()))
+            raise ValueError("No label '%s' associated with event %s" % (labelName, event.graceid()))
     else:
         this_label = Labelling.objects.get(
                 event = event,
@@ -219,7 +225,7 @@ def delete_label(event, request, labelName):
             log.save()
         except Exception as e:
             # XXX This looks a bit odd to me. (<-- retained this message)
-            logger.exception('Problem saving log message')
+            logger.exception('Problem saving log message (%s)' % str(e))
             d['error'] = str(e)
 
     # Return the json for some reason. I don't do any alert stuff in here.
-- 
GitLab