From 7b77c183fbe5791f4847c6c5f4575c760e7ad8ff Mon Sep 17 00:00:00 2001 From: jedi Date: Sat, 30 Dec 2023 18:34:35 +0100 Subject: [PATCH] don't save ticket state in multiple locations --- core/tickets/api_v2.py | 22 +++++----- core/tickets/models.py | 24 ++++++++++- core/tickets/tests/v2/test_tickets.py | 59 +++++++++++++-------------- 3 files changed, 62 insertions(+), 43 deletions(-) diff --git a/core/tickets/api_v2.py b/core/tickets/api_v2.py index 7e494e0..c93ae68 100644 --- a/core/tickets/api_v2.py +++ b/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()) diff --git a/core/tickets/models.py b/core/tickets/models.py index a0187c5..be694d1 100644 --- a/core/tickets/models.py +++ b/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) diff --git a/core/tickets/tests/v2/test_tickets.py b/core/tickets/tests/v2/test_tickets.py index 557550f..e15d813 100644 --- a/core/tickets/tests/v2/test_tickets.py +++ b/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'][1]['type'], 'state') + self.assertEqual(response.json()[0]['timeline'][0]['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'], state.id) + self.assertEqual(response.json()[0]['timeline'][1]['id'], mail1.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]['recipient'], 'test') - self.assertEqual(response.json()[0]['timeline'][0]['subject'], 'test') - self.assertEqual(response.json()[0]['timeline'][0]['body'], 'test') - self.assertEqual(response.json()[0]['timeline'][0]['timestamp'], - 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'][0]['state'], 'pending_new') + self.assertEqual(response.json()[0]['timeline'][1]['sender'], 'test') + self.assertEqual(response.json()[0]['timeline'][1]['recipient'], 'test') + self.assertEqual(response.json()[0]['timeline'][1]['subject'], 'test') + self.assertEqual(response.json()[0]['timeline'][1]['body'], 'test') 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', - 'recipient': 'test', 'body': 'test'}) + response = self.client.post('/api/2/tickets/manual/', + {'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(timeline[0]['type'], 'mail') - self.assertEqual(timeline[0]['sender'], 'test') - self.assertEqual(timeline[0]['recipient'], 'test') - self.assertEqual(timeline[0]['subject'], 'test issue') - self.assertEqual(timeline[0]['body'], 'test') + self.assertEqual(len(timeline), 2) + self.assertEqual(timeline[0]['type'], 'state') + self.assertEqual(timeline[0]['state'], 'pending_new') + self.assertEqual(timeline[1]['type'], 'mail') + self.assertEqual(timeline[1]['sender'], '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) -