don't save ticket state in multiple locations

core/tickets/api_v2.py

@@ -24,6 +24,17 @@ class IssueSerializer(serializers.ModelSerializer):
         fields = ('id', 'timeline', 'name', 'state', 'assigned_to', 'last_activity')
         read_only_fields = ('id', 'timeline', 'last_activity')
 
+    def to_internal_value(self, data):
+        ret = super().to_internal_value(data)
+        if 'state' in data:
+            ret['state'] = data['state']
+        return ret
+
+    def validate(self, attrs):
+        if 'state' in attrs:
+            if attrs['state'] not in [x[0] for x in STATE_CHOICES]:
+                raise serializers.ValidationError('invalid state')
+        return attrs
     @staticmethod
     def get_timeline(obj):
         timeline = []
@@ -53,16 +64,6 @@ class IssueSerializer(serializers.ModelSerializer):
             })
         return sorted(timeline, key=lambda x: x['timestamp'])
 
-    def update(self, instance, validated_data):
-        if 'state' in validated_data:
-            instance.state = validated_data['state']
-            instance.save()
-            StateChange.objects.create(
-                issue_thread=instance,
-                state=validated_data['state'],
-            )
-        return instance
-
 
 class IssueViewSet(viewsets.ModelViewSet):
     serializer_class = IssueSerializer
@@ -154,6 +155,7 @@ def get_available_states(request):
     def get_state_choices():
         for state in STATE_CHOICES:
             yield {'value': list(state)[0], 'text': list(state)[1]}
+
     return Response(get_state_choices())
 
 

core/tickets/models.py

@@ -2,6 +2,8 @@ from django.db import models
 from django_softdelete.models import SoftDeleteModel
 
 from inventory.models import Event
+from django.db.models.signals import post_save
+from django.dispatch import receiver
 
 STATE_CHOICES = (
     ('pending_new', 'New'),
@@ -24,11 +26,23 @@ STATE_CHOICES = (
 class IssueThread(SoftDeleteModel):
     id = models.AutoField(primary_key=True)
     name = models.CharField(max_length=255)
-    state = models.CharField('state', choices=STATE_CHOICES, max_length=32, default='pending_new')
     assigned_to = models.CharField(max_length=255, null=True)
     last_activity = models.DateTimeField(auto_now=True)
     manually_created = models.BooleanField(default=False)
 
+    @property
+    def state(self):
+        try:
+            return self.state_changes.order_by('-timestamp').first().state
+        except AttributeError:
+            return 'none'
+
+    @state.setter
+    def state(self, value):
+        if self.state == value:
+            return
+        self.state_changes.create(state=value)
+
     class Meta:
         permissions = [
             ('send_mail', 'Can send mail'),
@@ -36,6 +50,12 @@ class IssueThread(SoftDeleteModel):
         ]
 
 
+@receiver(post_save, sender=IssueThread)
+def create_issue_thread(sender, instance, created, **kwargs):
+    if created:
+        StateChange.objects.create(issue_thread=instance, state='pending_new')
+
+
 class Comment(models.Model):
     id = models.AutoField(primary_key=True)
     issue_thread = models.ForeignKey(IssueThread, on_delete=models.CASCADE, related_name='comments')
@@ -46,5 +66,5 @@ class Comment(models.Model):
 class StateChange(models.Model):
     id = models.AutoField(primary_key=True)
     issue_thread = models.ForeignKey(IssueThread, on_delete=models.CASCADE, related_name='state_changes')
-    state = models.CharField(max_length=255)
+    state = models.CharField(max_length=255, choices=STATE_CHOICES, default='pending_new')
     timestamp = models.DateTimeField(auto_now_add=True)

core/tickets/tests/v2/test_tickets.py

@@ -37,11 +37,6 @@ class IssueApiTest(TestCase):
             issue_thread=issue,
             timestamp=now,
         )
-        state = StateChange.objects.create(
-            issue_thread=issue,
-            state="pending_new",
-            timestamp=now + timedelta(seconds=1),
-        )
         mail2 = Email.objects.create(
             subject='test',
             body='test',
@@ -66,23 +61,20 @@ class IssueApiTest(TestCase):
         self.assertEqual(response.json()[0]['assigned_to'], None)
         self.assertEqual(response.json()[0]['last_activity'], issue.last_activity.strftime('%Y-%m-%dT%H:%M:%S.%fZ'))
         self.assertEqual(len(response.json()[0]['timeline']), 4)
-        self.assertEqual(response.json()[0]['timeline'][0]['type'], 'mail')
+        self.assertEqual(response.json()[0]['timeline'][0]['type'], 'state')
-        self.assertEqual(response.json()[0]['timeline'][1]['type'], 'state')
+        self.assertEqual(response.json()[0]['timeline'][1]['type'], 'mail')
         self.assertEqual(response.json()[0]['timeline'][2]['type'], 'mail')
         self.assertEqual(response.json()[0]['timeline'][3]['type'], 'comment')
-        self.assertEqual(response.json()[0]['timeline'][0]['id'], mail1.id)
+        self.assertEqual(response.json()[0]['timeline'][1]['id'], mail1.id)
-        self.assertEqual(response.json()[0]['timeline'][1]['id'], state.id)
         self.assertEqual(response.json()[0]['timeline'][2]['id'], mail2.id)
         self.assertEqual(response.json()[0]['timeline'][3]['id'], comment.id)
-        self.assertEqual(response.json()[0]['timeline'][0]['sender'], 'test')
+        self.assertEqual(response.json()[0]['timeline'][0]['state'], 'pending_new')
-        self.assertEqual(response.json()[0]['timeline'][0]['recipient'], 'test')
+        self.assertEqual(response.json()[0]['timeline'][1]['sender'], 'test')
-        self.assertEqual(response.json()[0]['timeline'][0]['subject'], 'test')
+        self.assertEqual(response.json()[0]['timeline'][1]['recipient'], 'test')
-        self.assertEqual(response.json()[0]['timeline'][0]['body'], 'test')
+        self.assertEqual(response.json()[0]['timeline'][1]['subject'], 'test')
-        self.assertEqual(response.json()[0]['timeline'][0]['timestamp'],
+        self.assertEqual(response.json()[0]['timeline'][1]['body'], 'test')
-                         mail1.timestamp.strftime('%Y-%m-%dT%H:%M:%S.%fZ'))
-        self.assertEqual(response.json()[0]['timeline'][1]['state'], 'pending_new')
         self.assertEqual(response.json()[0]['timeline'][1]['timestamp'],
-                         state.timestamp.strftime('%Y-%m-%dT%H:%M:%S.%fZ'))
+                         mail1.timestamp.strftime('%Y-%m-%dT%H:%M:%S.%fZ'))
         self.assertEqual(response.json()[0]['timeline'][2]['sender'], 'test')
         self.assertEqual(response.json()[0]['timeline'][2]['recipient'], 'test')
         self.assertEqual(response.json()[0]['timeline'][2]['subject'], 'test')
@@ -94,19 +86,22 @@ class IssueApiTest(TestCase):
                          comment.timestamp.strftime('%Y-%m-%dT%H:%M:%S.%fZ'))
 
     def test_manual_creation(self):
-        response = self.client.post('/api/2/tickets/manual/', {'name': 'test issue', 'sender': 'test',
+        response = self.client.post('/api/2/tickets/manual/',
-                                                               'recipient': 'test', 'body': 'test'})
+                                    {'name': 'test issue', 'sender': 'test', 'recipient': 'test', 'body': 'test'},
+                                    content_type='application/json')
         self.assertEqual(response.status_code, 201)
         self.assertEqual(response.json()['state'], 'pending_new')
         self.assertEqual(response.json()['name'], 'test issue')
         self.assertEqual(response.json()['assigned_to'], None)
         timeline = response.json()['timeline']
-        self.assertEqual(len(timeline), 1)
+        self.assertEqual(len(timeline), 2)
-        self.assertEqual(timeline[0]['type'], 'mail')
+        self.assertEqual(timeline[0]['type'], 'state')
-        self.assertEqual(timeline[0]['sender'], 'test')
+        self.assertEqual(timeline[0]['state'], 'pending_new')
-        self.assertEqual(timeline[0]['recipient'], 'test')
+        self.assertEqual(timeline[1]['type'], 'mail')
-        self.assertEqual(timeline[0]['subject'], 'test issue')
+        self.assertEqual(timeline[1]['sender'], 'test')
-        self.assertEqual(timeline[0]['body'], 'test')
+        self.assertEqual(timeline[1]['recipient'], 'test')
+        self.assertEqual(timeline[1]['subject'], 'test issue')
+        self.assertEqual(timeline[1]['body'], 'test')
 
     def test_post_comment(self):
         issue = IssueThread.objects.create(
@@ -122,21 +117,23 @@ class IssueApiTest(TestCase):
         issue = IssueThread.objects.create(
             name="test issue",
         )
-        response = self.client.patch(f'/api/2/tickets/{issue.id}/', {'state': 'pending_open'}, content_type='application/json')
+        response = self.client.patch(f'/api/2/tickets/{issue.id}/', {'state': 'pending_open'},
+                                     content_type='application/json')
         self.assertEqual(response.status_code, 200)
         self.assertEqual(response.json()['state'], 'pending_open')
         self.assertEqual(response.json()['name'], 'test issue')
         self.assertEqual(response.json()['assigned_to'], None)
         timeline = response.json()['timeline']
-        self.assertEqual(len(timeline), 1)
+        self.assertEqual(len(timeline), 2)
         self.assertEqual(timeline[0]['type'], 'state')
-        self.assertEqual(timeline[0]['state'], 'pending_open')
+        self.assertEqual(timeline[0]['state'], 'pending_new')
-
+        self.assertEqual(timeline[1]['type'], 'state')
+        self.assertEqual(timeline[1]['state'], 'pending_open')
 
     def test_state_change_invalid_state(self):
         issue = IssueThread.objects.create(
             name="test issue",
         )
-        response = self.client.patch(f'/api/2/tickets/{issue.id}/', {'state': 'invalid'}, content_type='application/json')
+        response = self.client.patch(f'/api/2/tickets/{issue.id}/', {'state': 'invalid'},
+                                     content_type='application/json')
         self.assertEqual(response.status_code, 400)
-