From 8222ea602261df413882b28944386b4e30cc8cf1 Mon Sep 17 00:00:00 2001
From: Clemens Wolff <clewolff@microsoft.com>
Date: Fri, 11 Oct 2019 15:37:17 -0400
Subject: [PATCH] Enable editing of labels with shortcut keys

---
 app/api/serializers.py    | 24 +++++++++++++++---------
 app/api/tests/test_api.py |  8 ++++++++
 2 files changed, 23 insertions(+), 9 deletions(-)

diff --git a/app/api/serializers.py b/app/api/serializers.py
index 8df0981d..2d0d38d1 100644
--- a/app/api/serializers.py
+++ b/app/api/serializers.py
@@ -19,11 +19,8 @@ class UserSerializer(serializers.ModelSerializer):
 class LabelSerializer(serializers.ModelSerializer):
 
     def validate(self, attrs):
-        if 'prefix_key' not in attrs and 'suffix_key' not in attrs:
-            return super().validate(attrs)
-
-        prefix_key = attrs['prefix_key']
-        suffix_key = attrs['suffix_key']
+        prefix_key = attrs.get('prefix_key')
+        suffix_key = attrs.get('suffix_key')
 
         # In the case of user don't set any shortcut key.
         if prefix_key is None and suffix_key is None:
@@ -37,13 +34,22 @@ class LabelSerializer(serializers.ModelSerializer):
         try:
             context = self.context['request'].parser_context
             project_id = context['kwargs']['project_id']
+            label_id = context['kwargs'].get('label_id')
         except (AttributeError, KeyError):
             pass  # unit tests don't always have the correct context set up
         else:
-            if Label.objects.filter(suffix_key=suffix_key,
-                                    prefix_key=prefix_key,
-                                    project=project_id).exists():
-                raise ValidationError('Duplicate key.')
+            conflicting_labels = Label.objects.filter(
+                suffix_key=suffix_key,
+                prefix_key=prefix_key,
+                project=project_id,
+            )
+
+            if label_id is not None:
+                conflicting_labels = conflicting_labels.exclude(id=label_id)
+
+            if conflicting_labels.exists():
+                raise ValidationError('Duplicate shortcut key.')
+
         return super().validate(attrs)
 
     class Meta:
diff --git a/app/api/tests/test_api.py b/app/api/tests/test_api.py
index adc6b009..928ceefd 100644
--- a/app/api/tests/test_api.py
+++ b/app/api/tests/test_api.py
@@ -256,7 +256,9 @@ class TestLabelDetailAPI(APITestCase):
                                                    email='fizz@buzz.com')
         project = mommy.make('Project', users=[project_member, super_user])
         cls.label = mommy.make('Label', project=project)
+        cls.label_with_shortcut = mommy.make('Label', suffix_key='l', project=project)
         cls.url = reverse(viewname='label_detail', args=[project.id, cls.label.id])
+        cls.url_with_shortcut = reverse(viewname='label_detail', args=[project.id, cls.label_with_shortcut.id])
         cls.data = {'text': 'example'}
 
     def test_returns_label_to_project_member(self):
@@ -277,6 +279,12 @@ class TestLabelDetailAPI(APITestCase):
         response = self.client.patch(self.url, format='json', data=self.data)
         self.assertEqual(response.data['text'], self.data['text'])
 
+    def test_allows_superuser_to_update_label_with_shortcut(self):
+        self.client.login(username=self.super_user_name,
+                          password=self.super_user_pass)
+        response = self.client.patch(self.url_with_shortcut, format='json', data={'suffix_key': 's'})
+        self.assertEqual(response.data['suffix_key'], 's')
+
     def test_disallows_project_member_to_update_label(self):
         self.client.login(username=self.project_member_name,
                           password=self.project_member_pass)