Merge pull request #5800 from FinnStutzenstein/fixUserUpdateVoteDelegations

Fix vote_delegated_from_user_ids on user update

server/openslides/users/views.py

@@ -125,6 +125,7 @@ class UserViewSet(ModelViewSet):
         except IntegrityError as e:
             raise ValidationError({"detail": str(e)})
 
+    @transaction.atomic
     def update(self, request, *args, **kwargs):
         """
         Customized view endpoint to update an user.
@@ -203,27 +204,31 @@ class UserViewSet(ModelViewSet):
         response = super().update(request, *args, **kwargs)
 
         # after rest of the request succeeded, handle delegation changes
-        if new_delegation_ids:
+        if isinstance(new_delegation_ids, list):
             self.assert_no_self_delegation(user, new_delegation_ids)
             self.assert_vote_not_delegated(user)
 
             for id in new_delegation_ids:
+                try:
                     delegation_user = User.objects.get(id=id)
+                except User.DoesNotExist:
+                    raise ValidationError(
+                        {
+                            "detail": f"The user {id} does not exist and cannot be set as vote delegation"
+                        }
+                    )
                 self.assert_has_no_delegated_votes(delegation_user)
                 delegation_user.vote_delegated_to = user
                 delegation_user.save()
 
             delegations_to_remove = user.vote_delegated_from_users.exclude(
-            id__in=(new_delegation_ids or [])
+                id__in=new_delegation_ids
             )
             for old_delegation_user in delegations_to_remove:
                 old_delegation_user.vote_delegated_to = None
                 old_delegation_user.save()
 
-        # if only delegated_from was changed, we need an autoupdate for the operator
-        if new_delegation_ids or delegations_to_remove:
         inform_changed_data(user)
-
         return response
 
     def assert_vote_not_delegated(self, user):

server/tests/integration/users/test_viewset.py

@@ -304,6 +304,16 @@ class UserUpdate(TestCase):
         admin = User.objects.get(pk=self.admin.pk)
         self.assertIsNone(admin.vote_delegated_to_id)
 
+    def test_update_vote_delegated_from_invalid_id(self):
+        response = self.client.patch(
+            reverse("user-detail", args=[self.admin.pk]),
+            {"vote_delegated_from_users_id": [1234]},
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        admin = User.objects.get(pk=self.admin.pk)
+        self.assertIsNone(admin.vote_delegated_to_id)
+
     def setup_vote_delegation(self):
         """ login and setup user -> user2 delegation """
         self.user, _ = self.create_user()
@@ -327,13 +337,24 @@ class UserUpdate(TestCase):
         self.setup_vote_delegation()
         response = self.client.patch(
             reverse("user-detail", args=[self.user2.pk]),
-            {"vote_delegated_from_users_id": None},
+            {"vote_delegated_from_users_id": []},
         )
 
         self.assertEqual(response.status_code, status.HTTP_200_OK)
         user = User.objects.get(pk=self.user.pk)
         self.assertEqual(user.vote_delegated_to_id, None)
 
+    def test_update_no_reset_vote_delegated_from_on_none(self):
+        self.setup_vote_delegation()
+        response = self.client.patch(
+            reverse("user-detail", args=[self.user2.pk]),
+            {"vote_delegated_from_users_id": None},
+        )
+
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        user = User.objects.get(pk=self.user.pk)
+        self.assertEqual(user.vote_delegated_to_id, self.user2.id)
+
     def test_update_nested_vote_delegation_1(self):
         """ user -> user2 -> admin """
         self.setup_vote_delegation()