diff --git a/client/src/app/core/repositories/assignments/assignment-option-repository.service.ts b/client/src/app/core/repositories/assignments/assignment-option-repository.service.ts index f757e82b7..c32dcd9fc 100644 --- a/client/src/app/core/repositories/assignments/assignment-option-repository.service.ts +++ b/client/src/app/core/repositories/assignments/assignment-option-repository.service.ts @@ -22,6 +22,12 @@ const AssignmentOptionRelations: RelationDefinition[] = [ ownKey: 'votes', foreignViewModel: ViewAssignmentVote }, + { + type: 'M2M', + ownIdKey: 'voted_id', + ownKey: 'voted', + foreignViewModel: ViewUser + }, { type: 'M2O', ownIdKey: 'poll_id', diff --git a/client/src/app/core/repositories/assignments/assignment-poll-repository.service.ts b/client/src/app/core/repositories/assignments/assignment-poll-repository.service.ts index 4708fb2aa..58e2bd598 100644 --- a/client/src/app/core/repositories/assignments/assignment-poll-repository.service.ts +++ b/client/src/app/core/repositories/assignments/assignment-poll-repository.service.ts @@ -14,7 +14,6 @@ import { ViewAssignmentOption } from 'app/site/assignments/models/view-assignmen import { AssignmentPollTitleInformation, ViewAssignmentPoll } from 'app/site/assignments/models/view-assignment-poll'; import { BasePollRepositoryService } from 'app/site/polls/services/base-poll-repository.service'; import { ViewGroup } from 'app/site/users/models/view-group'; -import { ViewUser } from 'app/site/users/models/view-user'; import { CollectionStringMapperService } from '../../core-services/collection-string-mapper.service'; import { DataStoreService } from '../../core-services/data-store.service'; @@ -25,12 +24,6 @@ const AssignmentPollRelations: RelationDefinition[] = [ ownKey: 'groups', foreignViewModel: ViewGroup }, - { - type: 'M2M', - ownIdKey: 'voted_id', - ownKey: 'voted', - foreignViewModel: ViewUser - }, { type: 'O2M', ownIdKey: 'options_id', diff --git a/client/src/app/core/repositories/motions/motion-option-repository.service.ts b/client/src/app/core/repositories/motions/motion-option-repository.service.ts index f55014f43..a0078691a 100644 --- a/client/src/app/core/repositories/motions/motion-option-repository.service.ts +++ b/client/src/app/core/repositories/motions/motion-option-repository.service.ts @@ -10,6 +10,7 @@ import { MotionOption } from 'app/shared/models/motions/motion-option'; import { ViewMotionOption } from 'app/site/motions/models/view-motion-option'; import { ViewMotionPoll } from 'app/site/motions/models/view-motion-poll'; import { ViewMotionVote } from 'app/site/motions/models/view-motion-vote'; +import { ViewUser } from 'app/site/users/models/view-user'; import { BaseRepository } from '../base-repository'; import { CollectionStringMapperService } from '../../core-services/collection-string-mapper.service'; import { DataStoreService } from '../../core-services/data-store.service'; @@ -21,6 +22,12 @@ const MotionOptionRelations: RelationDefinition[] = [ ownKey: 'votes', foreignViewModel: ViewMotionVote }, + { + type: 'M2M', + ownIdKey: 'voted_id', + ownKey: 'voted', + foreignViewModel: ViewUser + }, { type: 'M2O', ownIdKey: 'poll_id', diff --git a/client/src/app/core/repositories/motions/motion-poll-repository.service.ts b/client/src/app/core/repositories/motions/motion-poll-repository.service.ts index bb35c03d2..3c6155fe0 100644 --- a/client/src/app/core/repositories/motions/motion-poll-repository.service.ts +++ b/client/src/app/core/repositories/motions/motion-poll-repository.service.ts @@ -14,7 +14,6 @@ import { ViewMotionOption } from 'app/site/motions/models/view-motion-option'; import { MotionPollTitleInformation, ViewMotionPoll } from 'app/site/motions/models/view-motion-poll'; import { BasePollRepositoryService } from 'app/site/polls/services/base-poll-repository.service'; import { ViewGroup } from 'app/site/users/models/view-group'; -import { ViewUser } from 'app/site/users/models/view-user'; import { CollectionStringMapperService } from '../../core-services/collection-string-mapper.service'; import { DataStoreService } from '../../core-services/data-store.service'; @@ -25,12 +24,6 @@ const MotionPollRelations: RelationDefinition[] = [ ownKey: 'groups', foreignViewModel: ViewGroup }, - { - type: 'M2M', - ownIdKey: 'voted_id', - ownKey: 'voted', - foreignViewModel: ViewUser - }, { type: 'O2M', ownIdKey: 'options_id', diff --git a/client/src/app/core/ui-services/voting-banner.service.ts b/client/src/app/core/ui-services/voting-banner.service.ts index b6f112244..6c72e7c7c 100644 --- a/client/src/app/core/ui-services/voting-banner.service.ts +++ b/client/src/app/core/ui-services/voting-banner.service.ts @@ -31,7 +31,7 @@ export class VotingBannerService { */ private checkForVotablePolls(polls: ViewBasePoll[]): void { // display no banner if in history mode or there are no polls to vote - const pollsToVote = polls.filter(poll => this.votingService.canVote(poll) && !poll.user_has_voted); + const pollsToVote = polls.filter(poll => this.votingService.canVote(poll) && !poll.user_has_voted_valid); if ((this.OSStatus.isInHistoryMode && this.currentBanner) || !pollsToVote.length) { this.sliceBanner(); return; diff --git a/client/src/app/core/ui-services/voting.service.ts b/client/src/app/core/ui-services/voting.service.ts index f50a67a71..dca820277 100644 --- a/client/src/app/core/ui-services/voting.service.ts +++ b/client/src/app/core/ui-services/voting.service.ts @@ -10,7 +10,7 @@ export enum VotingError { USER_HAS_NO_PERMISSION, USER_IS_ANONYMOUS, USER_NOT_PRESENT, - USER_HAS_VOTED + USER_HAS_VOTED_VALID } /** @@ -60,8 +60,8 @@ export class VotingService { if (!user.is_present) { return VotingError.USER_NOT_PRESENT; } - if (poll.type === PollType.Pseudoanonymous && poll.user_has_voted) { - return VotingError.USER_HAS_VOTED; + if (poll.type === PollType.Pseudoanonymous && poll.user_has_voted_valid) { + return VotingError.USER_HAS_VOTED_VALID; } } diff --git a/client/src/app/shared/models/poll/base-option.ts b/client/src/app/shared/models/poll/base-option.ts index d6b46bbc9..8f2ca9e85 100644 --- a/client/src/app/shared/models/poll/base-option.ts +++ b/client/src/app/shared/models/poll/base-option.ts @@ -6,6 +6,8 @@ export abstract class BaseOption extends BaseDecimalModel { public no: number; public abstain: number; public poll_id: number; + public user_has_voted: boolean; + public voted_id: number[]; protected getDecimalFields(): (keyof BaseOption)[] { return ['yes', 'no', 'abstain']; diff --git a/client/src/app/shared/models/poll/base-poll.ts b/client/src/app/shared/models/poll/base-poll.ts index be5b062f0..af893b136 100644 --- a/client/src/app/shared/models/poll/base-poll.ts +++ b/client/src/app/shared/models/poll/base-poll.ts @@ -46,10 +46,8 @@ export abstract class BasePoll = any> extends public votesinvalid: number; public votescast: number; public groups_id: number[]; - public voted_id: number[]; public majority_method: MajorityMethod; public onehundred_percent_base: PercentBase; - public user_has_voted: boolean; public get isCreated(): boolean { return this.state === PollState.Created; diff --git a/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.html b/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.html index 174606b5f..397c82e45 100644 --- a/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.html +++ b/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.html @@ -5,6 +5,12 @@ + + + Your vote is valid! + DANGER: Your vote is invalid! + You have not give any voting here! +

{{ 'Votes for this poll' | translate }}: {{ poll.votes_amount }} @@ -13,6 +19,9 @@
+
+ TODO: DO not show buttons, becuase, the user has already voted for this one +
{{ poll.stateVerbose }}
- check_circle - warning + check_circle + warning
diff --git a/client/src/app/site/polls/components/poll-progress/poll-progress.component.html b/client/src/app/site/polls/components/poll-progress/poll-progress.component.html index 558a1b36c..30467e76e 100644 --- a/client/src/app/site/polls/components/poll-progress/poll-progress.component.html +++ b/client/src/app/site/polls/components/poll-progress/poll-progress.component.html @@ -1,3 +1,7 @@ - {{ 'Cast votes' }}: {{ poll.voted_id.length }} / {{ max }} + + {{ 'Casted votes' | translate }}: {{ poll.votescast }} / {{ max }}, + {{ 'valid votes' | translate}}: {{ poll.votesvalid }} / {{ max }}, + {{ 'invalid votes' | translate}}: {{ poll.votesinvalid }} / {{ max }} + diff --git a/client/src/app/site/polls/components/poll-progress/poll-progress.component.ts b/client/src/app/site/polls/components/poll-progress/poll-progress.component.ts index 642fdee2e..7d57739de 100644 --- a/client/src/app/site/polls/components/poll-progress/poll-progress.component.ts +++ b/client/src/app/site/polls/components/poll-progress/poll-progress.component.ts @@ -30,7 +30,7 @@ export class PollProgressComponent extends BaseViewComponent implements OnInit { } public get valueInPercent(): number { - return (this.poll.voted_id.length / this.max) * 100; + return (this.poll.votesvalid / this.max) * 100; } /** diff --git a/client/src/app/site/polls/models/view-base-poll.ts b/client/src/app/site/polls/models/view-base-poll.ts index d6a3dba0d..2bb175911 100644 --- a/client/src/app/site/polls/models/view-base-poll.ts +++ b/client/src/app/site/polls/models/view-base-poll.ts @@ -138,6 +138,18 @@ export abstract class ViewBasePoll = any> extends Bas return states; } + public get user_has_voted_invalid(): boolean { + return this.options.some(option => option.user_has_voted) && !this.user_has_voted_valid; + } + + public get user_has_voted_valid(): boolean { + return this.options.every(option => option.user_has_voted); + } + + public get user_has_not_voted(): boolean { + return this.options.every(option => !option.user_has_voted); + } + public abstract getSlide(): ProjectorElementBuildDeskriptor; public abstract getContentObject(): BaseViewModel; diff --git a/openslides/assignments/migrations/0008_voting_1.py b/openslides/assignments/migrations/0008_voting_1.py index 1e80fb5be..9aef52814 100644 --- a/openslides/assignments/migrations/0008_voting_1.py +++ b/openslides/assignments/migrations/0008_voting_1.py @@ -87,11 +87,6 @@ class Migration(migrations.Migration): to=settings.AUTH_USER_MODEL, ), ), - migrations.AddField( - model_name="assignmentpoll", - name="voted", - field=models.ManyToManyField(blank=True, to=settings.AUTH_USER_MODEL), - ), migrations.AddField( model_name="assignmentpoll", name="allow_multiple_votes_per_candidate", @@ -134,11 +129,38 @@ class Migration(migrations.Migration): name="number_poll_candidates", field=models.BooleanField(default=False), ), + migrations.AddField( + model_name="assignmentoption", + name="voted", + field=models.ManyToManyField( + blank=True, + to=settings.AUTH_USER_MODEL, + related_name="assignmentoption_voted", + ), + ), migrations.AlterField( model_name="assignment", name="poll_description_default", field=models.CharField(blank=True, max_length=255), ), + migrations.AlterField( + model_name="assignmentoption", + name="poll", + field=models.ForeignKey( + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, + related_name="options", + to="assignments.AssignmentPoll", + ), + ), + migrations.AlterField( + model_name="assignmentvote", + name="option", + field=models.ForeignKey( + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, + related_name="votes", + to="assignments.AssignmentOption", + ), + ), migrations.RenameField( model_name="assignment", old_name="poll_description_default", @@ -166,6 +188,15 @@ class Migration(migrations.Migration): validators=[django.core.validators.MinValueValidator(Decimal("-2"))], ), ), + migrations.AlterField( + model_name="assignmentpoll", + name="assignment", + field=models.ForeignKey( + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, + related_name="polls", + to="assignments.Assignment", + ), + ), migrations.RenameField( model_name="assignmentpoll", old_name="votescast", new_name="db_votescast" ), diff --git a/openslides/assignments/models.py b/openslides/assignments/models.py index 440ec2672..db9b878f1 100644 --- a/openslides/assignments/models.py +++ b/openslides/assignments/models.py @@ -267,7 +267,7 @@ class AssignmentVote(RESTModelMixin, BaseVote): objects = AssignmentVoteManager() option = models.ForeignKey( - "AssignmentOption", on_delete=models.CASCADE, related_name="votes" + "AssignmentOption", on_delete=CASCADE_AND_AUTOUPDATE, related_name="votes" ) class Meta: @@ -279,11 +279,14 @@ class AssignmentOption(RESTModelMixin, BaseOption): vote_class = AssignmentVote poll = models.ForeignKey( - "AssignmentPoll", on_delete=models.CASCADE, related_name="options" + "AssignmentPoll", on_delete=CASCADE_AND_AUTOUPDATE, related_name="options" ) user = models.ForeignKey( settings.AUTH_USER_MODEL, on_delete=SET_NULL_AND_AUTOUPDATE, null=True ) + voted = models.ManyToManyField( + settings.AUTH_USER_MODEL, blank=True, related_name="assignmentoption_voted" + ) weight = models.IntegerField(default=0) class Meta: @@ -304,9 +307,7 @@ class AssignmentPollManager(BaseManager): super() .get_prefetched_queryset(*args, **kwargs) .select_related("assignment") - .prefetch_related( - "options", "options__user", "options__votes", "groups", "voted" - ) + .prefetch_related("options", "options__user", "options__votes", "groups") ) @@ -317,7 +318,7 @@ class AssignmentPoll(RESTModelMixin, BasePoll): option_class = AssignmentOption assignment = models.ForeignKey( - Assignment, on_delete=models.CASCADE, related_name="polls" + Assignment, on_delete=CASCADE_AND_AUTOUPDATE, related_name="polls" ) description = models.CharField(max_length=255, blank=True) diff --git a/openslides/assignments/views.py b/openslides/assignments/views.py index 90e41d24d..6ae7b7f06 100644 --- a/openslides/assignments/views.py +++ b/openslides/assignments/views.py @@ -403,11 +403,10 @@ class AssignmentPollViewSet(BasePollViewSet): - amounts must be integer numbers >= 0. - ids should be integers of valid option ids for this poll - 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 + - The sum of all amounts must be grater then 0 and <= poll.votes_amount YN/YNA: {: 'Y' | 'N' [|'A']} - - all option_ids must be given TODO: No it must not be that way. Single Votes have to be accepted - 'A' is only allowed in YNA pollmethod Votes for all options have to be given @@ -474,6 +473,11 @@ class AssignmentPollViewSet(BasePollViewSet): ) amount_sum += amount + if amount_sum <= 0: + raise ValidationError( + {"detail": "You must give at least one vote"} + ) + if amount_sum > poll.votes_amount: raise ValidationError( { @@ -501,68 +505,118 @@ class AssignmentPollViewSet(BasePollViewSet): poll.pollmethod == AssignmentPoll.POLLMETHOD_YNA and value not in ("Y", "N", "A",) ): - raise ValidationError("Every value must be Y, N or A") + raise ValidationError( + {"detail": "Every value must be Y, N or A"} + ) elif ( poll.pollmethod == AssignmentPoll.POLLMETHOD_YN and value not in ("Y", "N",) ): - raise ValidationError("Every value must be Y or N") + raise ValidationError({"detail": "Every value must be Y or N"}) options_data = data - # Check if all options were given - db_option_ids = set(option.id for option in poll.get_options()) - data_option_ids = set(int(option_id) for option_id in options_data.keys()) - if data_option_ids != db_option_ids: - raise ValidationError( - {"error": "You have to provide values for all options"} - ) + # Just for named/pseudoanonymous with YN/YNA skip the all-options-given check + if poll.type not in ( + AssignmentPoll.TYPE_NAMED, + AssignmentPoll.TYPE_PSEUDOANONYMOUS, + ) or poll.pollmethod not in ( + AssignmentPoll.POLLMETHOD_YN, + AssignmentPoll.POLLMETHOD_YNA, + ): + # Check if all options were given + db_option_ids = set(option.id for option in poll.get_options()) + data_option_ids = set(int(option_id) for option_id in options_data.keys()) + if data_option_ids != db_option_ids: + raise ValidationError( + {"error": "You have to provide values for all options"} + ) - def create_votes(self, data, poll, user=None): + def create_votes_type_votes(self, data, poll, user): """ 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: # global_no or global_abstain - option = options.first() + if isinstance(data, dict): + for option_id, amount in data.items(): + # Add user to the option's voted array + option = options.get(pk=option_id) + option.voted.add(user) + inform_changed_data(option) + + # skip creating votes with empty weights + if amount == 0: + continue vote = AssignmentVote.objects.create( - option=option, - user=user, - weight=Decimal(poll.votes_amount), - value=data, + option=option, user=user, weight=Decimal(amount), value="Y" ) inform_changed_data(vote, no_delete_on_restriction=True) + else: # global_no or global_abstain + option = options.order_by( + "pk" + ).first() # order by is important to always get + # the correct "first" option + option.voted.add(user) + inform_changed_data(option) + vote = AssignmentVote.objects.create( + option=option, user=user, weight=Decimal(poll.votes_amount), value=data, + ) + inform_changed_data(vote, no_delete_on_restriction=True) + + def create_votes_type_named_pseudoanonymous( + self, data, poll, check_user, vote_user + ): + """ check_user is used for the voted-array, vote_user is the one put into the vote """ + options = poll.get_options() + for option_id, result in data.items(): + option = options.get(pk=option_id) + option.voted.add(check_user) + inform_changed_data(option) + vote = AssignmentVote.objects.create( + option=option, user=vote_user, value=result + ) + inform_changed_data(vote, no_delete_on_restriction=True) + + def handle_named_vote(self, data, poll, user): + if poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES: + # Instead of reusing all existing votes for the user, delete all previous votes + for vote in poll.get_votes().filter(user=user): + vote.delete() + self.create_votes_type_votes(data, poll, user) elif poll.pollmethod in ( AssignmentPoll.POLLMETHOD_YN, AssignmentPoll.POLLMETHOD_YNA, ): - for option_id, result in data.items(): + # Delete all votes for the given options + option_ids = list(data.keys()) + for vote in AssignmentVote.objects.filter( + user=user, option_id__in=option_ids + ): + vote.delete() + self.create_votes_type_named_pseudoanonymous(data, poll, user, user) + + def handle_pseudoanonymous_vote(self, data, poll, user): + if poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES: + # check if the user has already voted + for option in poll.get_options(): + if user in option.voted.all(): + raise ValidationError({"detail": "You have already voted"}) + self.create_votes_type_votes(data, poll, user) + + elif poll.pollmethod in ( + AssignmentPoll.POLLMETHOD_YN, + AssignmentPoll.POLLMETHOD_YNA, + ): + # Ensure, that the user has not voted any of the given options yet. + options = poll.get_options() + for option_id in data.keys(): 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): - # Instead of reusing all existing votes for the user, delete all previous votes - for vote in poll.get_votes().filter(user=user): - vote.delete() - self.create_votes(data, poll, user) - - def handle_pseudoanonymous_vote(self, data, poll): - self.create_votes(data, poll) + if user in option.voted.all(): + raise ValidationError( + {"detail": f"You have already voted for option {option.pk}"} + ) + self.create_votes_type_named_pseudoanonymous(data, poll, user, None) def convert_option_data(self, poll, data): poll_options = poll.get_options() diff --git a/openslides/motions/migrations/0033_voting_1.py b/openslides/motions/migrations/0033_voting_1.py index 0d0ba8554..88c3cf38a 100644 --- a/openslides/motions/migrations/0033_voting_1.py +++ b/openslides/motions/migrations/0033_voting_1.py @@ -76,11 +76,6 @@ class Migration(migrations.Migration): ), preserve_default=False, ), - migrations.AddField( - model_name="motionpoll", - name="voted", - field=models.ManyToManyField(blank=True, to=settings.AUTH_USER_MODEL), - ), migrations.AddField( model_name="motionpoll", name="majority_method", @@ -112,11 +107,20 @@ class Migration(migrations.Migration): ), preserve_default=False, ), + migrations.AddField( + model_name="motionoption", + name="voted", + field=models.ManyToManyField( + blank=True, + to=settings.AUTH_USER_MODEL, + related_name="motionoption_voted", + ), + ), migrations.AlterField( model_name="motionvote", name="option", field=models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, related_name="votes", to="motions.MotionOption", ), @@ -142,11 +146,20 @@ class Migration(migrations.Migration): model_name="motionoption", name="poll", field=models.ForeignKey( - on_delete=django.db.models.deletion.CASCADE, + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, related_name="options", to="motions.MotionPoll", ), ), + migrations.AlterField( + model_name="motionpoll", + name="motion", + field=models.ForeignKey( + on_delete=openslides.utils.models.CASCADE_AND_AUTOUPDATE, + related_name="polls", + to="motions.Motion", + ), + ), migrations.RenameField( model_name="motionpoll", old_name="votescast", new_name="db_votescast" ), diff --git a/openslides/motions/models.py b/openslides/motions/models.py index dbc9b40f5..70c722093 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -873,7 +873,7 @@ class MotionVoteManager(BaseManager): class MotionVote(RESTModelMixin, BaseVote): access_permissions = MotionVoteAccessPermissions() option = models.ForeignKey( - "MotionOption", on_delete=models.CASCADE, related_name="votes" + "MotionOption", on_delete=CASCADE_AND_AUTOUPDATE, related_name="votes" ) objects = MotionVoteManager() @@ -887,7 +887,10 @@ class MotionOption(RESTModelMixin, BaseOption): vote_class = MotionVote poll = models.ForeignKey( - "MotionPoll", related_name="options", on_delete=models.CASCADE + "MotionPoll", related_name="options", on_delete=CASCADE_AND_AUTOUPDATE + ) + voted = models.ManyToManyField( + settings.AUTH_USER_MODEL, blank=True, related_name="motionoption_voted" ) class Meta: @@ -908,7 +911,7 @@ class MotionPollManager(BaseManager): super() .get_prefetched_queryset(*args, **kwargs) .select_related("motion") - .prefetch_related("options", "options__votes", "groups", "voted") + .prefetch_related("options", "options__votes", "groups") ) @@ -918,7 +921,9 @@ class MotionPoll(RESTModelMixin, BasePoll): objects = MotionPollManager() - motion = models.ForeignKey(Motion, on_delete=models.CASCADE, related_name="polls") + motion = models.ForeignKey( + Motion, on_delete=CASCADE_AND_AUTOUPDATE, related_name="polls" + ) POLLMETHOD_YN = "YN" POLLMETHOD_YNA = "YNA" diff --git a/openslides/poll/access_permissions.py b/openslides/poll/access_permissions.py index e95352585..95dc73be9 100644 --- a/openslides/poll/access_permissions.py +++ b/openslides/poll/access_permissions.py @@ -20,13 +20,8 @@ class BasePollAccessPermissions(BaseAccessPermissions): 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 - Remove fields given in self.assitional_fields from the poll """ - # add hast_voted for all users to check whether op has voted - for poll in full_data: - poll["user_has_voted"] = user_id in poll["voted_id"] - if await async_has_perm(user_id, self.manage_permission): data = full_data else: @@ -39,7 +34,6 @@ class BasePollAccessPermissions(BaseAccessPermissions): del poll["votesvalid"] del poll["votesinvalid"] del poll["votescast"] - del poll["voted_id"] for field in self.additional_fields: del poll[field] data.append(poll) @@ -77,6 +71,10 @@ class BaseOptionAccessPermissions(BaseAccessPermissions): self, full_data: List[Dict[str, Any]], user_id: int ) -> List[Dict[str, Any]]: + # add has_voted for all users to check whether op has voted + for option in full_data: + option["user_has_voted"] = user_id in option["voted_id"] + if await async_has_perm(user_id, self.manage_permission): data = full_data else: @@ -89,5 +87,6 @@ class BaseOptionAccessPermissions(BaseAccessPermissions): del option["yes"] del option["no"] del option["abstain"] + del option["voted_id"] data.append(option) return data diff --git a/openslides/poll/models.py b/openslides/poll/models.py index 54377c4c6..1e5f46990 100644 --- a/openslides/poll/models.py +++ b/openslides/poll/models.py @@ -1,5 +1,5 @@ from decimal import Decimal -from typing import Iterable, Optional, Tuple, Type +from typing import Iterable, Optional, Set, Tuple, Type from django.conf import settings from django.core.validators import MinValueValidator @@ -35,7 +35,8 @@ class BaseVote(models.Model): class BaseOption(models.Model): """ - All subclasses must have poll attribute with the related name "options" + All subclasses must have poll attribute with the related name "options". Also + they must have a "voted" relation to users. """ vote_class: Optional[Type["BaseVote"]] = None @@ -73,6 +74,30 @@ class BaseOption(models.Model): ) return cls.vote_class + def get_votes(self): + """ + Return a QuerySet with all vote objects related to this option. + """ + return self.get_vote_class().objects.filter(option=self) + + def pseudoanonymize(self): + for vote in self.get_votes(): + vote.user = None + vote.save() + + def reset(self): + self.voted.clear() + + # Delete votes + votes = self.get_votes() + votes_id = [vote.id for vote in votes] + votes.delete() + collection = self.get_vote_class().get_collection_string() + inform_deleted_data((collection, id) for id in votes_id) + + # update self because the changed voted relation + inform_changed_data(self) + class BasePoll(models.Model): option_class: Optional[Type["BaseOption"]] = None @@ -101,7 +126,6 @@ class BasePoll(models.Model): title = models.CharField(max_length=255, blank=True, null=False) groups = models.ManyToManyField(settings.AUTH_GROUP_MODEL, blank=True) - voted = models.ManyToManyField(settings.AUTH_USER_MODEL, blank=True) db_votesvalid = models.DecimalField( null=True, @@ -162,7 +186,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votesvalid else: - return Decimal(self.count_users_voted()) + return Decimal(self.amount_valid_votes()) def set_votesvalid(self, value): if self.type != self.TYPE_ANALOG: @@ -175,7 +199,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votesinvalid else: - return Decimal(0) + return Decimal(self.amount_invalid_votes()) def set_votesinvalid(self, value): if self.type != self.TYPE_ANALOG: @@ -188,7 +212,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votescast else: - return Decimal(self.count_users_voted()) + return Decimal(self.amount_voted_users()) def set_votescast(self, value): if self.type != self.TYPE_ANALOG: @@ -197,14 +221,30 @@ class BasePoll(models.Model): votescast = property(get_votescast, set_votescast) - def count_users_voted(self): - return self.voted.all().count() + def get_user_ids_with_valid_votes(self): + initial_option = self.get_options().first() + user_ids = set(map(lambda u: u.id, initial_option.voted.all())) + for option in self.get_options(): + user_ids = user_ids.intersection( + set(map(lambda u: u.id, option.voted.all())) + ) + return list(user_ids) - def get_options(self): - """ - Returns the option objects for the poll. - """ - return self.get_option_class().objects.filter(poll=self) + def get_all_voted_user_ids(self): + # TODO: This might be faster with only one DB query using distinct. + user_ids: Set[int] = set() + for option in self.get_options(): + user_ids.update(option.voted.all().values_list("pk", flat=True)) + return list(user_ids) + + def amount_valid_votes(self): + return len(self.get_user_ids_with_valid_votes()) + + def amount_invalid_votes(self): + return self.amount_voted_users() - self.amount_valid_votes() + + def amount_voted_users(self): + return len(self.get_all_voted_user_ids()) def create_options(self): """ Should be called after creation of this model. """ @@ -218,6 +258,12 @@ class BasePoll(models.Model): ) return cls.option_class + def get_options(self): + """ + Returns the option objects for the poll. + """ + return self.get_option_class().objects.filter(poll=self) + @classmethod def get_vote_class(cls): return cls.get_option_class().get_vote_class() @@ -229,22 +275,12 @@ class BasePoll(models.Model): return self.get_vote_class().objects.filter(option__poll__id=self.id) def pseudoanonymize(self): - for vote in self.get_votes(): - vote.user = None - vote.save() + for option in self.get_options(): + option.pseudoanonymize() def reset(self): - self.voted.clear() - - # Delete votes - votes = self.get_votes() - votes_id = [vote.id for vote in votes] - votes.delete() - collection = self.get_vote_class().get_collection_string() - inform_deleted_data((collection, id) for id in votes_id) - - # update options - inform_changed_data(self.get_options()) + for option in self.get_options(): + option.reset() # Reset state self.state = BasePoll.STATE_CREATED diff --git a/openslides/poll/serializers.py b/openslides/poll/serializers.py index 48dbaf833..e4d5c0501 100644 --- a/openslides/poll/serializers.py +++ b/openslides/poll/serializers.py @@ -22,7 +22,7 @@ class BaseVoteSerializer(ModelSerializer): return vote.option.poll.state -BASE_OPTION_FIELDS = ("id", "yes", "no", "abstain", "poll_id", "pollstate") +BASE_OPTION_FIELDS = ("id", "yes", "no", "abstain", "poll_id", "pollstate", "voted") class BaseOptionSerializer(ModelSerializer): @@ -31,6 +31,7 @@ class BaseOptionSerializer(ModelSerializer): abstain = DecimalField( max_digits=15, decimal_places=6, min_value=-2, read_only=True ) + voted = IdPrimaryKeyRelatedField(many=True, read_only=True) pollstate = SerializerMethodField() @@ -47,7 +48,6 @@ BASE_POLL_FIELDS = ( "votesinvalid", "votescast", "options", - "voted", "id", "onehundred_percent_base", "majority_method", @@ -59,7 +59,6 @@ class BasePollSerializer(ModelSerializer): groups = IdPrimaryKeyRelatedField( many=True, required=False, queryset=get_group_model().objects.all() ) - voted = IdPrimaryKeyRelatedField(many=True, read_only=True) options = IdPrimaryKeyRelatedField(many=True, read_only=True) votesvalid = DecimalField( diff --git a/openslides/poll/views.py b/openslides/poll/views.py index 3cd3534ec..8d5d9711d 100644 --- a/openslides/poll/views.py +++ b/openslides/poll/views.py @@ -210,13 +210,12 @@ class BasePollViewSet(ModelViewSet): elif poll.type == BasePoll.TYPE_NAMED: self.handle_named_vote(data, poll, request.user) - poll.voted.add(request.user) elif poll.type == BasePoll.TYPE_PSEUDOANONYMOUS: - self.handle_pseudoanonymous_vote(data, poll) - poll.voted.add(request.user) + self.handle_pseudoanonymous_vote(data, poll, request.user) + + inform_changed_data(poll) - inform_changed_data(poll) # needed for the changed voted relation return Response() def assert_can_vote(self, poll, request): @@ -224,7 +223,7 @@ class BasePollViewSet(ModelViewSet): Raises a permission denied, if the user is not allowed to vote. Analog: has to have manage permissions Named & Pseudoanonymous: has to be in a poll group and present - Only pseudoanonymous: has to not have voted yet + Note: For pseudoanonymous it is *not* tested, if the user has already voted! """ if poll.type == BasePoll.TYPE_ANALOG: if not self.has_manage_permissions(): @@ -239,10 +238,6 @@ class BasePollViewSet(ModelViewSet): ): self.permission_denied(request) - if poll.type == BasePoll.TYPE_PSEUDOANONYMOUS: - if request.user in poll.voted.all(): - self.permission_denied(request) - def parse_vote_value(self, obj, key): """ Raises a ValidationError on incorrect values, including None """ if key not in obj: @@ -278,13 +273,16 @@ class BasePollViewSet(ModelViewSet): def handle_named_vote(self, data, poll, user): """ - To be implemented by subclass. Handles the named vote. Assumes data is validated + To be implemented by subclass. Handles the named vote. Assumes data is validated. + Needs to manage the voted-array per option. """ raise NotImplementedError() - def handle_pseudoanonymous_vote(self, data, poll): + def handle_pseudoanonymous_vote(self, data, poll, user): """ - To be implemented by subclass. Handles the pseudoanonymous vote. Assumes data is validated + To be implemented by subclass. Handles the pseudoanonymous vote. Assumes data + is validated. Needs to check, if the vote is allowed by the voted-array per poll. + Needs to add the user to the voted-array. """ raise NotImplementedError() diff --git a/tests/integration/assignments/test_polls.py b/tests/integration/assignments/test_polls.py index 6d208b3a1..aa40d620f 100644 --- a/tests/integration/assignments/test_polls.py +++ b/tests/integration/assignments/test_polls.py @@ -48,6 +48,15 @@ def test_assignment_vote_db_queries(): assert count_queries(AssignmentVote.get_elements)() == 1 +@pytest.mark.django_db(transaction=False) +def test_assignment_option_db_queries(): + """ + Tests that only 1 query is done when fetching AssignmentOptions + """ + create_assignment_polls() + assert count_queries(AssignmentOption.get_elements)() == 1 + + def create_assignment_polls(): """ Creates 1 assignment with 3 candidates which has 5 polls in which each candidate got a random amount of votes between 0 and 10 from 3 users diff --git a/tests/integration/motions/test_polls.py b/tests/integration/motions/test_polls.py index c0da78417..f1ccee70e 100644 --- a/tests/integration/motions/test_polls.py +++ b/tests/integration/motions/test_polls.py @@ -41,6 +41,15 @@ def test_motion_vote_db_queries(): assert count_queries(MotionVote.get_elements)() == 1 +@pytest.mark.django_db(transaction=False) +def test_motion_option_db_queries(): + """ + Tests that only 1 query is done when fetching MotionOptions + """ + create_motion_polls() + assert count_queries(MotionOption.get_elements)() == 1 + + def create_motion_polls(): """ Creates 1 Motion with 5 polls with 5 options each which have 2 votes each