added testing for named and pseudoanonymous assignment voting

added queries count tests for assignment and motion polls and votes

openslides/assignments/access_permissions.py

@@ -1,5 +1,8 @@
+import json
 from typing import Any, Dict, List
 
+from ..poll.access_permissions import BaseVoteAccessPermissions
+from ..poll.views import BasePoll
 from ..utils.access_permissions import BaseAccessPermissions
 from ..utils.auth import async_has_perm
 
@@ -50,13 +53,35 @@ class AssignmentPollAccessPermissions(BaseAccessPermissions):
     async def get_restricted_data(
         self, full_data: List[Dict[str, Any]], user_id: int
     ) -> List[Dict[str, Any]]:
-        return full_data
+        """
+        Poll-managers have full access, even during an active poll.
+        Non-published polls will be restricted:
+         - Remove votes* values from the poll
+         - Remove yes/no/abstain fields from options
+         - Remove voted_id field from the poll
+        """
+
+        if await async_has_perm(user_id, "assignments.can_manage_polls"):
+            data = full_data
+        else:
+            data = []
+            for poll in full_data:
+                if poll["state"] != BasePoll.STATE_PUBLISHED:
+                    poll = json.loads(
+                        json.dumps(poll)
+                    )  # copy, so we can remove some fields.
+                    del poll["votesvalid"]
+                    del poll["votesinvalid"]
+                    del poll["votescast"]
+                    del poll["voted_id"]
+                    for option in poll["options"]:
+                        del option["yes"]
+                        del option["no"]
+                        del option["abstain"]
+                data.append(poll)
+        return data
 
 
-class AssignmentVoteAccessPermissions(BaseAccessPermissions):
+class AssignmentVoteAccessPermissions(BaseVoteAccessPermissions):
     base_permission = "assignments.can_see"
-
+    manage_permission = "assignments.can_manage"
-    async def get_restricted_data(
-        self, full_data: List[Dict[str, Any]], user_id: int
-    ) -> List[Dict[str, Any]]:
-        return full_data

openslides/assignments/models.py

@@ -268,8 +268,23 @@ class Assignment(RESTModelMixin, AgendaItemWithListOfSpeakersMixin, models.Model
         return {"title": self.title}
 
 
+class AssignmentVoteManager(models.Manager):
+    """
+    Customized model manager to support our get_full_queryset method.
+    """
+
+    def get_full_queryset(self):
+        """
+        Returns the normal queryset with all assignment votes. In the background we
+        join and prefetch all related models.
+        """
+        return self.get_queryset().select_related("user", "option", "option__poll")
+
+
 class AssignmentVote(RESTModelMixin, BaseVote):
     access_permissions = AssignmentVoteAccessPermissions()
+    objects = AssignmentVoteManager()
+
     option = models.ForeignKey(
         "AssignmentOption", on_delete=models.CASCADE, related_name="votes"
     )
@@ -296,11 +311,32 @@ class AssignmentOption(RESTModelMixin, BaseOption):
         return self.poll
 
 
+class AssignmentPollManager(models.Manager):
+    """
+    Customized model manager to support our get_full_queryset method.
+    """
+
+    def get_full_queryset(self):
+        """
+        Returns the normal queryset with all assignment polls. In the background we
+        join and prefetch all related models.
+        """
+        return (
+            self.get_queryset()
+            .select_related("assignment")
+            .prefetch_related(
+                "options", "options__user", "options__votes", "groups", "voted"
+            )
+        )
+
+
 # Meta-TODO: Is this todo resolved?
 # TODO: remove the type-ignoring in the next line, after this is solved:
 #       https://github.com/python/mypy/issues/3855
 class AssignmentPoll(RESTModelMixin, BasePoll):
     access_permissions = AssignmentPollAccessPermissions()
+    objects = AssignmentPollManager()
+
     option_class = AssignmentOption
 
     assignment = models.ForeignKey(

openslides/assignments/views.py

@@ -330,8 +330,8 @@ class AssignmentPollViewSet(BasePollViewSet):
 
         required fields per pollmethod:
         - votes: Y
-        - YN: YN
+        - YN:    YN
-        - YNA: YNA
+        - YNA:   YNA
         """
         if not isinstance(data, dict):
             raise ValidationError({"detail": "Data must be a dict"})
@@ -428,8 +428,13 @@ class AssignmentPollViewSet(BasePollViewSet):
                     if not is_int(amount):
                         raise ValidationError({"detail": "Each amounts must be int"})
                     amount = int(amount)
-                    if amount < 1:
+                    if amount < 0:
-                        raise ValidationError({"detail": "At least 1 vote per option"})
+                        raise ValidationError(
+                            {"detail": "Negative votes are not allowed"}
+                        )
+                    # skip empty votes
+                    if amount == 0:
+                        continue
                     if not poll.allow_multiple_votes_per_candidate and amount != 1:
                         raise ValidationError(
                             {"detail": "Multiple votes are not allowed"}
@@ -485,27 +490,41 @@ class AssignmentPollViewSet(BasePollViewSet):
                 )
 
     def create_votes(self, data, poll, user=None):
+        """
+        Helper function for handle_(named|pseudoanonymous)_vote
+        Assumes data is already validated
+        """
         options = poll.get_options()
         if poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES:
             if isinstance(data, dict):
                 for option_id, amount in data.items():
+                    # skip empty votes
+                    if amount == 0:
+                        continue
                     option = options.get(pk=option_id)
                     vote = AssignmentVote.objects.create(
                         option=option, user=user, weight=Decimal(amount), value="Y"
                     )
                     inform_changed_data(vote, no_delete_on_restriction=True)
-            else:
+            else:  # global_no or global_abstain
                 option = options.first()
                 vote = AssignmentVote.objects.create(
-                    option=option, user=user, weight=Decimal(1), value=data
+                    option=option,
+                    user=user,
+                    weight=Decimal(poll.votes_amount),
+                    value=data,
                 )
                 inform_changed_data(vote, no_delete_on_restriction=True)
         elif poll.pollmethod in (
             AssignmentPoll.POLLMETHOD_YN,
             AssignmentPoll.POLLMETHOD_YNA,
         ):
-            pass
+            for option_id, result in data.items():
-            # TODO
+                option = options.get(pk=option_id)
+                vote = AssignmentVote.objects.create(
+                    option=option, user=user, value=result
+                )
+                inform_changed_data(vote, no_delete_on_restriction=True)
 
     def handle_named_vote(self, data, poll, user):
         """
@@ -514,9 +533,9 @@ class AssignmentPollViewSet(BasePollViewSet):
          - Exactly one of the three options must be given
          - 'N' is only valid if poll.global_no==True
          - 'A' is only valid if poll.global_abstain==True
-         - amonts must be integer numbers >= 1.
+         - amounts must be integer numbers >= 1.
          - ids should be integers of valid option ids for this poll
-         - amounts must be one ("1"), if poll.allow_multiple_votes_per_candidate if False
+         - amounts must be 0 or 1, if poll.allow_multiple_votes_per_candidate is False
          - The sum of all amounts must be poll.votes_amount votes
 
          Request data for YN/YNA pollmethod:

openslides/motions/access_permissions.py

@@ -1,6 +1,8 @@
 import json
 from typing import Any, Dict, List
 
+from ..poll.access_permissions import BaseVoteAccessPermissions
+from ..poll.views import BasePoll
 from ..utils.access_permissions import BaseAccessPermissions
 from ..utils.auth import async_has_perm, async_in_some_groups
 
@@ -183,7 +185,6 @@ class StateAccessPermissions(BaseAccessPermissions):
 
 class MotionPollAccessPermissions(BaseAccessPermissions):
     base_permission = "motions.can_see"
-    STATE_PUBLISHED = 4
 
     async def get_restricted_data(
         self, full_data: List[Dict[str, Any]], user_id: int
@@ -201,7 +202,7 @@ class MotionPollAccessPermissions(BaseAccessPermissions):
         else:
             data = []
             for poll in full_data:
-                if poll["state"] != self.STATE_PUBLISHED:
+                if poll["state"] != BasePoll.STATE_PUBLISHED:
                     poll = json.loads(
                         json.dumps(poll)
                     )  # copy, so we can remove some fields.
@@ -217,26 +218,6 @@ class MotionPollAccessPermissions(BaseAccessPermissions):
         return data
 
 
-class MotionVoteAccessPermissions(BaseAccessPermissions):
+class MotionVoteAccessPermissions(BaseVoteAccessPermissions):
     base_permission = "motions.can_see"
-    STATE_PUBLISHED = 4
+    manage_permission = "motions.can_manage_polls"
-
-    async def get_restricted_data(
-        self, full_data: List[Dict[str, Any]], user_id: int
-    ) -> List[Dict[str, Any]]:
-        """
-        Poll-managers have full access, even during an active poll.
-        Every user can see it's own votes.
-        If the pollstate is published, everyone can see the votes.
-        """
-
-        if await async_has_perm(user_id, "motions.can_manage_polls"):
-            data = full_data
-        else:
-            data = [
-                vote
-                for vote in full_data
-                if vote["pollstate"] == self.STATE_PUBLISHED
-                or vote["user_id"] == user_id
-            ]
-        return data

openslides/motions/models.py

@@ -867,12 +867,27 @@ class MotionBlock(RESTModelMixin, AgendaItemWithListOfSpeakersMixin, models.Mode
         return {"title": self.title}
 
 
+class MotionVoteManager(models.Manager):
+    """
+    Customized model manager to support our get_full_queryset method.
+    """
+
+    def get_full_queryset(self):
+        """
+        Returns the normal queryset with all motion votes. In the background we
+        join and prefetch all related models.
+        """
+        return self.get_queryset().select_related("user", "option", "option__poll")
+
+
 class MotionVote(RESTModelMixin, BaseVote):
     access_permissions = MotionVoteAccessPermissions()
     option = models.ForeignKey(
         "MotionOption", on_delete=models.CASCADE, related_name="votes"
     )
 
+    objects = MotionVoteManager()
+
     class Meta:
         default_permissions = ()
 
@@ -891,6 +906,23 @@ class MotionOption(RESTModelMixin, BaseOption):
         return self.poll
 
 
+class MotionPollManager(models.Manager):
+    """
+    Customized model manager to support our get_full_queryset method.
+    """
+
+    def get_full_queryset(self):
+        """
+        Returns the normal queryset with all motion polls. In the background we
+        join and prefetch all related models.
+        """
+        return (
+            self.get_queryset()
+            .select_related("motion")
+            .prefetch_related("options", "options__votes", "groups", "voted")
+        )
+
+
 # Meta-TODO: Is this todo resolved?
 # TODO: remove the type-ignoring in the next line, after this is solved:
 #       https://github.com/python/mypy/issues/3855
@@ -898,6 +930,8 @@ class MotionPoll(RESTModelMixin, BasePoll):
     access_permissions = MotionPollAccessPermissions()
     option_class = MotionOption
 
+    objects = MotionPollManager()
+
     motion = models.ForeignKey(Motion, on_delete=models.CASCADE, related_name="polls")
 
     POLLMETHOD_YN = "YN"

openslides/poll/access_permissions.py

@@ -0,0 +1,29 @@
+from typing import Any, Dict, List
+
+from ..poll.views import BasePoll
+from ..utils.access_permissions import BaseAccessPermissions
+from ..utils.auth import async_has_perm
+
+
+class BaseVoteAccessPermissions(BaseAccessPermissions):
+    manage_permission = ""  # set by subclass
+
+    async def get_restricted_data(
+        self, full_data: List[Dict[str, Any]], user_id: int
+    ) -> List[Dict[str, Any]]:
+        """
+        Poll-managers have full access, even during an active poll.
+        Every user can see it's own votes.
+        If the pollstate is published, everyone can see the votes.
+        """
+
+        if await async_has_perm(user_id, self.manage_permission):
+            data = full_data
+        else:
+            data = [
+                vote
+                for vote in full_data
+                if vote["pollstate"] == BasePoll.STATE_PUBLISHED
+                or vote["user_id"] == user_id
+            ]
+        return data

openslides/poll/views.py

@@ -136,9 +136,7 @@ class BasePollViewSet(ModelViewSet):
             self.assert_can_vote(poll, request)
 
             if request.user in poll.voted.all():
-                raise ValidationError(
+                self.permission_denied(request)
-                    {"detail": "You have already voted for this poll."}
-                )
             self.handle_pseudoanonymous_vote(request.data, poll)
             poll.voted.add(request.user)
 

tests/integration/motions/test_polls.py

@@ -1,18 +1,77 @@
 from decimal import Decimal
 
+import pytest
 from django.contrib.auth import get_user_model
 from django.urls import reverse
 from rest_framework import status
 from rest_framework.test import APIClient
 
 from openslides.core.config import config
-from openslides.motions.models import Motion, MotionPoll, MotionVote
+from openslides.motions.models import Motion, MotionOption, MotionPoll, MotionVote
 from openslides.poll.models import BasePoll
 from openslides.utils.auth import get_group_model
 from openslides.utils.autoupdate import inform_changed_data
 from tests.common_groups import GROUP_ADMIN_PK, GROUP_DEFAULT_PK, GROUP_DELEGATE_PK
 from tests.test_case import TestCase
 
+from ..helpers import count_queries
+
+
+@pytest.mark.django_db(transaction=False)
+def test_motion_poll_db_queries():
+    """
+    Tests that only the following db queries are done:
+    * 1 request to get the polls,
+    * 1 request to get all options for all polls,
+    * 1 request to get all votes for all options,
+    * 1 request to get all users for all votes,
+    * 1 request to get all poll groups,
+    = 5 queries
+    """
+    create_motion_polls()
+    assert count_queries(MotionPoll.get_elements) == 5
+
+
+@pytest.mark.django_db(transaction=False)
+def test_motion_vote_db_queries():
+    """
+    Tests that only 1 query is done when fetching MotionVotes
+    """
+    create_motion_polls()
+    assert count_queries(MotionVote.get_elements) == 1
+
+
+def create_motion_polls():
+    """
+    Creates 1 Motion with 5 polls with 5 options each which have 2 votes each
+    """
+    motion = Motion.objects.create(title="test_motion_wfLrsjEHXBmPplbvQ65N")
+    group1 = get_group_model().objects.get(pk=1)
+    group2 = get_group_model().objects.get(pk=2)
+
+    for index in range(5):
+        poll = MotionPoll.objects.create(
+            motion=motion, title=f"test_title_{index}", pollmethod="YN", type="named"
+        )
+        poll.groups.add(group1)
+        poll.groups.add(group2)
+
+        for j in range(5):
+            option = MotionOption.objects.create(poll=poll)
+
+            for k in range(2):
+                user = get_user_model().objects.create_user(
+                    username=f"test_username_{index}{j}{k}",
+                    password="test_password_kbzj5L8ZtVxBllZzoW6D",
+                )
+                MotionVote.objects.create(
+                    user=user,
+                    option=option,
+                    value=("Y" if k == 0 else "N"),
+                    weight=Decimal(1),
+                )
+                poll.voted.add(user)
+
 
 class CreateMotionPoll(TestCase):
     """
@@ -816,7 +875,7 @@ class VoteMotionPollPseudoanonymous(TestCase):
         response = self.client.post(
             reverse("motionpoll-vote", args=[self.poll.pk]), "A", format="json"
         )
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         option = MotionPoll.objects.get().options.get()
         self.assertEqual(option.yes, Decimal("0"))
         self.assertEqual(option.no, Decimal("1"))