From e2d4fafe6df3c012febe901aa57a75d035212321 Mon Sep 17 00:00:00 2001 From: Joshua Sangmeister Date: Thu, 22 Apr 2021 11:27:16 +0200 Subject: [PATCH] Fix vote delegation update error --- docker/docker-compose.dev.yml | 2 + server/openslides/users/views.py | 20 ++-- .../tests/integration/users/test_viewset.py | 96 +++++++++++++++++++ 3 files changed, 111 insertions(+), 7 deletions(-) diff --git a/docker/docker-compose.dev.yml b/docker/docker-compose.dev.yml index 4bfddc97b..7ab1be5e0 100644 --- a/docker/docker-compose.dev.yml +++ b/docker/docker-compose.dev.yml @@ -19,6 +19,8 @@ services: depends_on: - redis - postgres + stdin_open: true + tty: true postgres: image: postgres:11 diff --git a/server/openslides/users/views.py b/server/openslides/users/views.py index 6dc2537a2..3b15c99c4 100644 --- a/server/openslides/users/views.py +++ b/server/openslides/users/views.py @@ -167,6 +167,11 @@ class UserViewSet(ModelViewSet): ): request.data["username"] = user.username + # handle delegated_from field seperately since its a SerializerMethodField + new_delegation_ids = request.data.get("vote_delegated_from_users_id") + if "vote_delegated_from_users_id" in request.data: + del request.data["vote_delegated_from_users_id"] + # check that no chains are created with vote delegation delegate_id = request.data.get("vote_delegated_to_id") if delegate_id: @@ -181,16 +186,15 @@ class UserViewSet(ModelViewSet): self.assert_no_self_delegation(user, [delegate_id]) self.assert_vote_not_delegated(delegate) - self.assert_has_no_delegated_votes(user) + # if vote_delegated_from is reset in this request, we can skip this check + if new_delegation_ids != []: + self.assert_has_no_delegated_votes(user) inform_changed_data(delegate) if user.vote_delegated_to: inform_changed_data(user.vote_delegated_to) - - # handle delegated_from field seperately since its a SerializerMethodField - new_delegation_ids = request.data.get("vote_delegated_from_users_id") - if "vote_delegated_from_users_id" in request.data: - del request.data["vote_delegated_from_users_id"] + elif "vote_delegated_to_id" in request.data and user.vote_delegated_to: + inform_changed_data(user.vote_delegated_to) try: response = super().update(request, *args, **kwargs) @@ -199,8 +203,10 @@ class UserViewSet(ModelViewSet): # after rest of the request succeeded, handle delegation changes if isinstance(new_delegation_ids, list): + user.refresh_from_db() # fetch updated model self.assert_no_self_delegation(user, new_delegation_ids) - self.assert_vote_not_delegated(user) + if new_delegation_ids: + self.assert_vote_not_delegated(user) for id in new_delegation_ids: try: diff --git a/server/tests/integration/users/test_viewset.py b/server/tests/integration/users/test_viewset.py index 0d164b250..ab99de703 100644 --- a/server/tests/integration/users/test_viewset.py +++ b/server/tests/integration/users/test_viewset.py @@ -217,6 +217,40 @@ class UserUpdate(TestCase): [user.pk], ) + def test_update_vote_delegation_same_user(self): + user = User.objects.create_user( + username="non-admin EVnE4n103fPZXcVV", + password="non-admin 1WywRnqKbcdtQwS2", + vote_delegated_to=self.admin, + ) + + response = self.client.patch( + reverse("user-detail", args=[user.pk]), + {"vote_delegated_to_id": self.admin.pk, "vote_delegated_from_users_id": []}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + user = User.objects.get(pk=user.pk) + self.assertEqual(user.vote_delegated_to_id, self.admin.pk) + + def test_update_remove_vote_delegation(self): + user = User.objects.create_user( + username="non-admin EVnE4n103fPZXcVV", + password="non-admin 1WywRnqKbcdtQwS2", + vote_delegated_to=self.admin, + ) + + response = self.client.patch( + reverse("user-detail", args=[user.pk]), + {"vote_delegated_to_id": None, "vote_delegated_from_users_id": []}, + ) + + self.assertEqual(response.status_code, status.HTTP_200_OK) + user = User.objects.get(pk=user.pk) + self.assertEqual(user.vote_delegated_to_id, None) + admin = User.objects.get(username="admin") + self.assertEqual(admin.vote_delegated_from_users.count(), 0) + def test_update_vote_delegation_non_admin(self): user = User.objects.create_user( username="non-admin WpBQRSsCg6qNWNtN6bLP", @@ -380,6 +414,68 @@ class UserUpdate(TestCase): user2 = User.objects.get(pk=self.user2.pk) self.assertIsNone(user2.vote_delegated_to_id) + def test_update_vote_delegation_both_1(self): + """ Change user -> user2 to admin -> user in one request. """ + self.user2 = User.objects.create_user( + username="user2", + password="non-admin 1WywRnqKbcdtQwS2", + ) + self.user = User.objects.create_user( + username="user", + password="non-admin 1WywRnqKbcdtQwS2", + vote_delegated_to=self.user2, + ) + response = self.client.patch( + reverse("user-detail", args=[self.user.pk]), + { + "vote_delegated_to_id": None, + "vote_delegated_from_users_id": [self.admin.pk], + }, + ) + + self.assertHttpStatusVerbose(response, status.HTTP_200_OK) + admin = User.objects.get(pk=self.admin.pk) + self.assertEqual(admin.vote_delegated_to_id, self.user.id) + user = User.objects.get(pk=self.user.pk) + self.assertIsNone(user.vote_delegated_to_id) + self.assertEqual( + list(user.vote_delegated_from_users.values_list("id", flat=True)), + [self.admin.pk], + ) + user2 = User.objects.get(pk=self.user2.pk) + self.assertEqual(user2.vote_delegated_from_users.count(), 0) + + def test_update_vote_delegation_both_2(self): + """ Change user -> user2 to user2 -> admin in one request. """ + self.user2 = User.objects.create_user( + username="user2", + password="non-admin 1WywRnqKbcdtQwS2", + ) + self.user = User.objects.create_user( + username="user", + password="non-admin 1WywRnqKbcdtQwS2", + vote_delegated_to=self.user2, + ) + response = self.client.patch( + reverse("user-detail", args=[self.user2.pk]), + { + "vote_delegated_to_id": self.admin.pk, + "vote_delegated_from_users_id": [], + }, + ) + + self.assertHttpStatusVerbose(response, status.HTTP_200_OK) + admin = User.objects.get(pk=self.admin.pk) + self.assertEqual( + list(admin.vote_delegated_from_users.values_list("id", flat=True)), + [self.user2.pk], + ) + user = User.objects.get(pk=self.user.pk) + self.assertIsNone(user.vote_delegated_to_id) + user2 = User.objects.get(pk=self.user2.pk) + self.assertEqual(user2.vote_delegated_to, self.admin) + self.assertEqual(user2.vote_delegated_from_users.count(), 0) + class UserDelete(TestCase): """