Allow renaming non-archived worker configurations

arkindex/process/serializers/workers.py

@@ -348,8 +348,18 @@ class WorkerConfigurationSerializer(WorkerConfigurationListSerializer):
     configuration = serializers.DictField(allow_empty=False, read_only=True)
 
     class Meta(WorkerConfigurationListSerializer.Meta):
-        # Only allow updating `archived`
-        read_only_fields = ('id', 'name', 'configuration')
+        # The configuration cannot be updated
+        read_only_fields = ('id', 'configuration')
+
+    def validate(self, data):
+        name = data.get('name')
+        data_archived = data.get('archived')
+        instance_archived = self.instance.archived
+        # Archived configurations cannot be renamed, but un-archiving a configuration and renaming it at the same time is
+        # possible. It's also possible to archive and rename a configuration simultaneously.
+        if instance_archived and data_archived is not False and name:
+            raise ValidationError({'name': 'Archived configurations cannot be renamed.'})
+        return data
 
 
 class WorkerConfigurationExistsErrorSerializer(serializers.Serializer):

arkindex/process/tests/test_worker_configurations.py

@@ -448,12 +448,12 @@ class TestWorkerConfigurations(FixtureAPITestCase):
         with self.assertNumQueries(7):
             response = self.client.put(
                 reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
-                data={'archived': True}
+                data={'archived': True, 'name': 'new name'}
             )
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         self.worker_config.refresh_from_db()
-        self.assertEqual(self.worker_config.name, 'config time')
+        self.assertEqual(self.worker_config.name, 'new name')
         self.assertDictEqual(self.worker_config.configuration, {'key': 'value'})
         self.assertTrue(self.worker_config.archived)
 
@@ -471,12 +471,12 @@ class TestWorkerConfigurations(FixtureAPITestCase):
         with self.assertNumQueries(6):
             response = self.client.put(
                 reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
-                data={'archived': True}
+                data={'archived': True, 'name': 'new name'}
             )
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         self.worker_config.refresh_from_db()
-        self.assertEqual(self.worker_config.name, 'config time')
+        self.assertEqual(self.worker_config.name, 'new name')
         self.assertDictEqual(self.worker_config.configuration, {'key': 'value'})
         self.assertTrue(self.worker_config.archived)
 
@@ -493,12 +493,12 @@ class TestWorkerConfigurations(FixtureAPITestCase):
         with self.assertNumQueries(4):
             response = self.client.put(
                 reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
-                data={'archived': True}
+                data={'archived': True, 'name': 'a new name'}
             )
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         self.worker_config.refresh_from_db()
-        self.assertEqual(self.worker_config.name, 'config time')
+        self.assertEqual(self.worker_config.name, 'a new name')
         self.assertDictEqual(self.worker_config.configuration, {'key': 'value'})
         self.assertTrue(self.worker_config.archived)
 
@@ -527,11 +527,63 @@ class TestWorkerConfigurations(FixtureAPITestCase):
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         self.worker_config.refresh_from_db()
-        self.assertEqual(self.worker_config.name, 'config time')
+        self.assertEqual(self.worker_config.name, 'new name')
+        self.assertTrue(self.worker_config.archived)
+        # The configuration was not updated
         self.assertDictEqual(self.worker_config.configuration, {'key': 'value'})
-        # Only the archived state was updated
+
+    def test_update_archived_configuration_name(self):
+        """
+        Archived configurations cannot be renamed with an update
+        """
+        self.client.force_login(self.user)
+        self.worker_config.archived = True
+        self.worker_config.save()
+        self.assertTrue(self.worker_config.archived)
+
+        with self.assertNumQueries(8):
+            response = self.client.put(
+                reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
+                data={
+                    'name': 'new name',
+                    'archived': True,
+                },
+                format='json',
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(response.json(), {
+            'name': ['Archived configurations cannot be renamed.'],
+        })
+
+        self.worker_config.refresh_from_db()
+        self.assertEqual(self.worker_config.name, 'config time')
         self.assertTrue(self.worker_config.archived)
 
+    def test_update_archived_configuration_name_unarchive(self):
+        """
+        Archived configurations can be renamed if they are also unarchived
+        """
+        self.client.force_login(self.user)
+        self.worker_config.archived = True
+        self.worker_config.save()
+        self.assertTrue(self.worker_config.archived)
+
+        with self.assertNumQueries(9):
+            response = self.client.put(
+                reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
+                data={
+                    'name': 'new name',
+                    'archived': False,
+                },
+                format='json',
+            )
+            self.assertEqual(response.status_code, status.HTTP_200_OK)
+
+        self.worker_config.refresh_from_db()
+        self.assertEqual(self.worker_config.name, 'new name')
+        self.assertFalse(self.worker_config.archived)
+
     def test_partial_update_requires_login(self):
         with self.assertNumQueries(0):
             response = self.client.patch(
@@ -592,7 +644,7 @@ class TestWorkerConfigurations(FixtureAPITestCase):
 
     def test_partial_update_contributor_repository(self):
         """
-        Can update a configuration with contributor rights on the repository
+        Can partial update a configuration with contributor rights on the repository
         """
         self.worker_1.memberships.all().delete()
         self.worker_1.repository.memberships.all().delete()
@@ -615,7 +667,7 @@ class TestWorkerConfigurations(FixtureAPITestCase):
 
     def test_partial_update_contributor_worker(self):
         """
-        Can update a configuration with contributor rights on the worker
+        Can partial update a configuration with contributor rights on the worker
         """
         self.worker_1.memberships.all().delete()
         self.worker_1.repository.memberships.all().delete()
@@ -638,7 +690,7 @@ class TestWorkerConfigurations(FixtureAPITestCase):
 
     def test_partial_update_admin(self):
         """
-        Admins can update any configuration
+        Admins can partial update any configuration
         """
         self.worker_1.memberships.all().delete()
         self.worker_1.repository.memberships.all().delete()
@@ -660,7 +712,7 @@ class TestWorkerConfigurations(FixtureAPITestCase):
 
     def test_partial_update_ignored_fields(self):
         """
-        Fields that should not be editable are ignored when sent in update requests
+        Fields that should not be editable are ignored when sent in partial update requests
         """
         # Get as much rights as possible so that this test does not fail if something goes wrong with the endpoint's
         # permissions; we do not care about rights in this test.
@@ -683,7 +735,34 @@ class TestWorkerConfigurations(FixtureAPITestCase):
             self.assertEqual(response.status_code, status.HTTP_200_OK)
 
         self.worker_config.refresh_from_db()
-        self.assertEqual(self.worker_config.name, 'config time')
+        self.assertEqual(self.worker_config.name, 'new name')
+        self.assertTrue(self.worker_config.archived)
+        # The configuration was not updated
         self.assertDictEqual(self.worker_config.configuration, {'key': 'value'})
-        # Only the archived state was updated
+
+    def test_partial_update_archived_configuration_name(self):
+        """
+        Archived configurations cannot be renamed with a partial update
+        """
+        self.client.force_login(self.user)
+        self.worker_config.archived = True
+        self.worker_config.save()
+        self.assertTrue(self.worker_config.archived)
+
+        with self.assertNumQueries(8):
+            response = self.client.patch(
+                reverse('api:configuration-retrieve', kwargs={'pk': str(self.worker_config.id)}),
+                data={
+                    'name': 'new name',
+                },
+                format='json',
+            )
+            self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+
+        self.assertEqual(response.json(), {
+            'name': ['Archived configurations cannot be renamed.'],
+        })
+
+        self.worker_config.refresh_from_db()
+        self.assertEqual(self.worker_config.name, 'config time')
         self.assertTrue(self.worker_config.archived)