Merge pull request #396 from CatalystCode/bugfix/edit-label-with-shortcut

Bugfix/Enable editing of labels with shortcut keys

app/api/serializers.py

@@ -21,11 +21,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:
@@ -39,13 +36,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:

app/api/tests/test_api.py

@@ -308,7 +308,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'}
         create_default_roles()
         assign_user_to_role(project_member=project_member, project=project,
@@ -332,6 +334,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)