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 6d24cfe66..0d4171ed3 100644 --- a/client/src/app/core/ui-services/voting-banner.service.ts +++ b/client/src/app/core/ui-services/voting-banner.service.ts @@ -34,7 +34,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_valid); + const pollsToVote = polls.filter(poll => this.votingService.canVote(poll) && !poll.user_has_voted); 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 dca820277..9930af398 100644 --- a/client/src/app/core/ui-services/voting.service.ts +++ b/client/src/app/core/ui-services/voting.service.ts @@ -9,8 +9,7 @@ export enum VotingError { POLL_WRONG_TYPE, USER_HAS_NO_PERMISSION, USER_IS_ANONYMOUS, - USER_NOT_PRESENT, - USER_HAS_VOTED_VALID + USER_NOT_PRESENT } /** @@ -60,9 +59,6 @@ export class VotingService { if (!user.is_present) { return VotingError.USER_NOT_PRESENT; } - if (poll.type === PollType.Pseudoanonymous && poll.user_has_voted_valid) { - return VotingError.USER_HAS_VOTED_VALID; - } } public getVotePermissionErrorVerbose(poll: ViewBasePoll): string | void { diff --git a/client/src/app/shared/components/charts/charts.component.ts b/client/src/app/shared/components/charts/charts.component.ts index 0c24b665e..4d8f9591e 100644 --- a/client/src/app/shared/components/charts/charts.component.ts +++ b/client/src/app/shared/components/charts/charts.component.ts @@ -211,9 +211,11 @@ export class ChartsComponent extends BaseViewComponent { yAxes: [ { gridLines: { - drawOnChartArea: false + drawBorder: false, + drawOnChartArea: false, + drawTicks: false }, - ticks: { beginAtZero: true, mirror: true, labelOffset: -20 }, + ticks: { mirror: true, labelOffset: -20 }, stacked: true } ] diff --git a/client/src/app/shared/components/check-input/check-input.component.ts b/client/src/app/shared/components/check-input/check-input.component.ts index 22292dd3a..7e7d6ca44 100644 --- a/client/src/app/shared/components/check-input/check-input.component.ts +++ b/client/src/app/shared/components/check-input/check-input.component.ts @@ -88,10 +88,12 @@ export class CheckInputComponent extends BaseViewComponent implements OnInit, Co * @param obj the value from the parent form. Type "any" is required by the interface */ public writeValue(obj: string | number): void { - if (obj && obj === this.checkboxValue) { - this.checkboxStateChanged(true); - } else { - this.contentForm.patchValue(obj); + if (obj || typeof obj === 'number') { + if (obj === this.checkboxValue) { + this.checkboxStateChanged(true); + } else { + this.contentForm.patchValue(obj); + } } } diff --git a/client/src/app/shared/models/assignments/assignment-poll.ts b/client/src/app/shared/models/assignments/assignment-poll.ts index aeed243d4..7fdda5b29 100644 --- a/client/src/app/shared/models/assignments/assignment-poll.ts +++ b/client/src/app/shared/models/assignments/assignment-poll.ts @@ -29,6 +29,13 @@ export class AssignmentPoll extends BasePoll< public static COLLECTIONSTRING = 'assignments/assignment-poll'; public static defaultGroupsConfig = 'assignment_poll_default_groups'; public static defaultPollMethodConfig = 'assignment_poll_method'; + public static DECIMAL_FIELDS = [ + 'votesvalid', + 'votesinvalid', + 'votescast', + 'amount_global_abstain', + 'amount_global_no' + ]; public id: number; public assignment_id: number; @@ -36,6 +43,8 @@ export class AssignmentPoll extends BasePoll< public allow_multiple_votes_per_candidate: boolean; public global_no: boolean; public global_abstain: boolean; + public amount_global_no: number; + public amount_global_abstain: number; public description: string; public get isMethodY(): boolean { @@ -63,4 +72,8 @@ export class AssignmentPoll extends BasePoll< public constructor(input?: any) { super(AssignmentPoll.COLLECTIONSTRING, input); } + + protected getDecimalFields(): string[] { + return AssignmentPoll.DECIMAL_FIELDS; + } } diff --git a/client/src/app/shared/models/base/base-decimal-model.ts b/client/src/app/shared/models/base/base-decimal-model.ts index 1f40f5cb9..e468247e5 100644 --- a/client/src/app/shared/models/base/base-decimal-model.ts +++ b/client/src/app/shared/models/base/base-decimal-model.ts @@ -1,7 +1,8 @@ import { BaseModel } from './base-model'; export abstract class BaseDecimalModel extends BaseModel { - protected abstract getDecimalFields(): (keyof this)[]; + // TODO: no more elegant solution available in current Typescript + protected abstract getDecimalFields(): string[]; public deserialize(input: any): void { if (input && typeof input === 'object') { diff --git a/client/src/app/shared/models/poll/base-option.ts b/client/src/app/shared/models/poll/base-option.ts index 8f2ca9e85..74d15e79c 100644 --- a/client/src/app/shared/models/poll/base-option.ts +++ b/client/src/app/shared/models/poll/base-option.ts @@ -6,10 +6,9 @@ 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)[] { + protected getDecimalFields(): string[] { 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 95166aada..11b7c5015 100644 --- a/client/src/app/shared/models/poll/base-poll.ts +++ b/client/src/app/shared/models/poll/base-poll.ts @@ -52,6 +52,7 @@ export abstract class BasePoll< public votescast: number; public groups_id: number[]; public majority_method: MajorityMethod; + public user_has_voted: boolean; public pollmethod: PM; public onehundred_percent_base: PB; @@ -91,7 +92,7 @@ export abstract class BasePoll< return this.state + 1; } - protected getDecimalFields(): (keyof BasePoll)[] { + protected getDecimalFields(): string[] { return ['votesvalid', 'votesinvalid', 'votescast']; } } diff --git a/client/src/app/shared/models/poll/base-vote.ts b/client/src/app/shared/models/poll/base-vote.ts index 4a5f0a7ba..b761984af 100644 --- a/client/src/app/shared/models/poll/base-vote.ts +++ b/client/src/app/shared/models/poll/base-vote.ts @@ -26,7 +26,7 @@ export abstract class BaseVote extends BaseDecimalModel { return VoteValueVerbose[this.value]; } - protected getDecimalFields(): (keyof BaseVote)[] { + protected getDecimalFields(): string[] { return ['weight']; } } diff --git a/client/src/app/shared/pipes/poll-key-verbose.pipe.ts b/client/src/app/shared/pipes/poll-key-verbose.pipe.ts index 7e0a35768..e2898f025 100644 --- a/client/src/app/shared/pipes/poll-key-verbose.pipe.ts +++ b/client/src/app/shared/pipes/poll-key-verbose.pipe.ts @@ -8,7 +8,9 @@ const PollValues = { votesabstain: 'Votes abstain', yes: 'Yes', no: 'No', - abstain: 'Abstain' + abstain: 'Abstain', + amount_global_abstain: 'Global abstain', + amount_global_no: 'Global no' }; /** diff --git a/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.html b/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.html index 29863703c..da6b6a668 100644 --- a/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.html +++ b/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.html @@ -57,21 +57,31 @@ -
+
- - - + + + + +
+ +
+ + +
@@ -196,12 +206,6 @@
-
- -
diff --git a/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.scss b/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.scss index 70e098f5c..1f87518de 100644 --- a/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.scss +++ b/client/src/app/site/assignments/components/assignment-detail/assignment-detail.component.scss @@ -26,6 +26,14 @@ } } +.new-ballot-button { + display: flex; + > * { + margin-left: auto; + margin-right: auto; + } +} + .election-document-list mat-list-item { height: 20px; } @@ -56,10 +64,6 @@ margin: 0; } } - - .ballot-button { - grid-column: 2; - } } .candidate-list-separator { diff --git a/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.html b/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.html index b8931ff06..3a8449c84 100644 --- a/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.html +++ b/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.html @@ -1,9 +1,14 @@ - + - +
- +
@@ -24,6 +29,8 @@
+ +
+ + +
+ + + +
@@ -53,7 +81,8 @@ diff --git a/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.ts b/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.ts index 5004194d0..6122a3340 100644 --- a/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.ts +++ b/client/src/app/site/assignments/components/assignment-poll-dialog/assignment-poll-dialog.component.ts @@ -5,6 +5,7 @@ import { MAT_DIALOG_DATA, MatDialogRef } from '@angular/material/dialog'; import { Title } from '@angular/platform-browser'; import { TranslateService } from '@ngx-translate/core'; +import { debounceTime, distinctUntilChanged } from 'rxjs/operators'; import { AssignmentPollMethod } from 'app/shared/models/assignments/assignment-poll'; import { PollType } from 'app/shared/models/poll/base-poll'; @@ -59,6 +60,9 @@ export class AssignmentPollDialogComponent extends BasePollDialogComponent { + this.pollForm.contentForm.valueChanges.pipe(debounceTime(150), distinctUntilChanged()).subscribe(() => { this.createDialog(); }) ); @@ -112,6 +116,8 @@ export class AssignmentPollDialogComponent extends BasePollDialogComponent ({ [sumValue]: ['', [Validators.min(-2)]] diff --git a/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.ts b/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.ts index c65a4344a..69c5f4c04 100644 --- a/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.ts +++ b/client/src/app/site/assignments/components/assignment-poll-vote/assignment-poll-vote.component.ts @@ -54,7 +54,7 @@ export class AssignmentPollVoteComponent extends BasePollVoteComponent> = new Map(); public readonly pollClassType = PollClassType.Assignment; + protected globalVoteKeys: VotingResult[] = [ + { + vote: 'amount_global_no', + showPercent: false, + hide: this.poll.amount_global_no === -2 || this.poll.amount_global_no === 0 + }, + { + vote: 'amount_global_abstain', + showPercent: false, + hide: this.poll.amount_global_abstain === -2 || this.poll.amount_global_abstain === 0 + } + ]; + public get pollmethodVerbose(): string { return AssignmentPollMethodVerbose[this.pollmethod]; } @@ -98,8 +111,31 @@ export class ViewAssignmentPoll extends ViewBasePoll { + return !key.hide; + }) + .map(key => ({ + votingOption: key.vote, + class: 'sums', + value: [ + { + amount: this[key.vote], + hide: key.hide, + showPercent: key.showPercent + } as VotingResult + ] + })) + ); + return tableData; } + + protected getDecimalFields(): string[] { + return AssignmentPoll.DECIMAL_FIELDS; + } } export interface ViewAssignmentPoll extends AssignmentPoll { diff --git a/client/src/app/site/motions/modules/motion-poll/motion-poll-dialog/motion-poll-dialog.component.ts b/client/src/app/site/motions/modules/motion-poll/motion-poll-dialog/motion-poll-dialog.component.ts index 7911b94b9..418c006c6 100644 --- a/client/src/app/site/motions/modules/motion-poll/motion-poll-dialog/motion-poll-dialog.component.ts +++ b/client/src/app/site/motions/modules/motion-poll/motion-poll-dialog/motion-poll-dialog.component.ts @@ -57,12 +57,12 @@ export class MotionPollDialogComponent extends BasePollDialogComponent +
@@ -16,3 +16,11 @@ + + +
+ + {{ 'You already voted on this poll' | translate }} + +
+ diff --git a/client/src/app/site/motions/modules/motion-poll/motion-poll-vote/motion-poll-vote.component.scss b/client/src/app/site/motions/modules/motion-poll/motion-poll-vote/motion-poll-vote.component.scss index eb949fffa..b4bb22d38 100644 --- a/client/src/app/site/motions/modules/motion-poll/motion-poll-vote/motion-poll-vote.component.scss +++ b/client/src/app/site/motions/modules/motion-poll/motion-poll-vote/motion-poll-vote.component.scss @@ -17,6 +17,15 @@ } } +.user-has-voted { + display: flex; + > * { + margin-top: 1em; + margin-left: auto; + margin-right: auto; + } +} + .voted-yes { background-color: $votes-yes-color; color: $vote-active-color; 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 e8cd3808d..e1411150d 100644 --- a/client/src/app/site/polls/models/view-base-poll.ts +++ b/client/src/app/site/polls/models/view-base-poll.ts @@ -22,7 +22,15 @@ export interface PollTableData { } export interface VotingResult { - vote?: 'yes' | 'no' | 'abstain' | 'votesvalid' | 'votesinvalid' | 'votescast'; + vote?: + | 'yes' + | 'no' + | 'abstain' + | 'votesvalid' + | 'votesinvalid' + | 'votescast' + | 'amount_global_no' + | 'amount_global_abstain'; amount?: number; icon?: string; hide?: boolean; @@ -176,18 +184,6 @@ export abstract class ViewBasePoll< public canBeVotedFor: () => boolean; - 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 3aff61112..4e4191b9b 100644 --- a/openslides/assignments/migrations/0008_voting_1.py +++ b/openslides/assignments/migrations/0008_voting_1.py @@ -31,6 +31,28 @@ class Migration(migrations.Migration): name="global_no", field=models.BooleanField(default=True), ), + migrations.AddField( + model_name="assignmentpoll", + name="db_amount_global_abstain", + field=models.DecimalField( + blank=True, + decimal_places=6, + max_digits=15, + null=True, + validators=[django.core.validators.MinValueValidator(Decimal("-2"))], + ), + ), + migrations.AddField( + model_name="assignmentpoll", + name="db_amount_global_no", + field=models.DecimalField( + blank=True, + decimal_places=6, + max_digits=15, + null=True, + validators=[django.core.validators.MinValueValidator(Decimal("-2"))], + ), + ), migrations.AddField( model_name="assignmentpoll", name="groups", @@ -76,6 +98,11 @@ class Migration(migrations.Migration): default=1, validators=[django.core.validators.MinValueValidator(1)] ), ), + migrations.AddField( + model_name="assignmentpoll", + name="voted", + field=models.ManyToManyField(blank=True, to=settings.AUTH_USER_MODEL), + ), migrations.AddField( model_name="assignmentvote", name="user", @@ -129,15 +156,6 @@ 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", diff --git a/openslides/assignments/models.py b/openslides/assignments/models.py index 0e185dee9..4740d1e43 100644 --- a/openslides/assignments/models.py +++ b/openslides/assignments/models.py @@ -254,14 +254,14 @@ class AssignmentOptionManager(BaseManager): def get_prefetched_queryset(self, *args, **kwargs): """ - Returns the normal queryset with all voted users. In the background we + Returns the normal queryset. In the background we join and prefetch all related models. """ return ( super() .get_prefetched_queryset(*args, **kwargs) .select_related("user", "poll") - .prefetch_related("voted", "votes") + .prefetch_related("votes") ) @@ -277,9 +277,6 @@ class AssignmentOption(RESTModelMixin, BaseOption): 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: @@ -301,7 +298,7 @@ class AssignmentPollManager(BaseManager): .get_prefetched_queryset(*args, **kwargs) .select_related("assignment") .prefetch_related( - "options", "options__user", "options__votes", "options__voted", "groups" + "options", "options__user", "options__votes", "voted", "groups" ) ) @@ -348,7 +345,23 @@ class AssignmentPoll(RESTModelMixin, BasePoll): ) global_abstain = models.BooleanField(default=True) + db_amount_global_abstain = models.DecimalField( + null=True, + blank=True, + default=Decimal("0"), + validators=[MinValueValidator(Decimal("-2"))], + max_digits=15, + decimal_places=6, + ) global_no = models.BooleanField(default=True) + db_amount_global_no = models.DecimalField( + null=True, + blank=True, + default=Decimal("0"), + validators=[MinValueValidator(Decimal("-2"))], + max_digits=15, + decimal_places=6, + ) votes_amount = models.IntegerField(default=1, validators=[MinValueValidator(1)]) """ For "votes" mode: The amount of votes a voter can give. """ @@ -358,26 +371,49 @@ class AssignmentPoll(RESTModelMixin, BasePoll): class Meta: default_permissions = () - @property - def amount_global_no(self): - if self.pollmethod != AssignmentPoll.POLLMETHOD_VOTES or not self.global_no: + def get_amount_global_abstain(self): + if not self.global_abstain: return None - no_sum = Decimal(0) - for option in self.options.all(): - no_sum += option.no - return no_sum - - @property - def amount_global_abstain(self): - if ( - self.pollmethod != AssignmentPoll.POLLMETHOD_VOTES - or not self.global_abstain + elif ( + self.type == self.TYPE_ANALOG + or self.pollmethod == AssignmentPoll.POLLMETHOD_VOTES ): + return self.db_amount_global_abstain + else: return None - abstain_sum = Decimal(0) - for option in self.options.all(): - abstain_sum += option.abstain - return abstain_sum + + def set_amount_global_abstain(self, value): + if ( + self.type != self.TYPE_ANALOG + and self.pollmethod != AssignmentPoll.POLLMETHOD_VOTES + ): + raise ValueError("Do not set amount_global_abstain YN/YNA polls") + self.db_amount_global_abstain = value + + amount_global_abstain = property( + get_amount_global_abstain, set_amount_global_abstain + ) + + def get_amount_global_no(self): + if not self.global_no: + return None + elif ( + self.type == self.TYPE_ANALOG + or self.pollmethod == AssignmentPoll.POLLMETHOD_VOTES + ): + return self.db_amount_global_no + else: + return None + + def set_amount_global_no(self, value): + if ( + self.type != self.TYPE_ANALOG + and self.pollmethod != AssignmentPoll.POLLMETHOD_VOTES + ): + raise ValueError("Do not set amount_global_no YN/YNA polls") + self.db_amount_global_no = value + + amount_global_no = property(get_amount_global_no, set_amount_global_no) def create_options(self, skip_autoupdate=False): related_users = AssignmentRelatedUser.objects.filter( @@ -404,3 +440,8 @@ class AssignmentPoll(RESTModelMixin, BasePoll): pass if not skip_autoupdate: inform_changed_data(self.assignment.list_of_speakers) + + def reset(self): + self.db_amount_global_abstain = Decimal(0) + self.db_amount_global_no = Decimal(0) + super().reset() diff --git a/openslides/assignments/views.py b/openslides/assignments/views.py index 2ad31c5a6..5476c3f7a 100644 --- a/openslides/assignments/views.py +++ b/openslides/assignments/views.py @@ -2,6 +2,7 @@ from decimal import Decimal from django.contrib.auth import get_user_model from django.db import transaction +from django.db.models import F from openslides.poll.views import BaseOptionViewSet, BasePollViewSet, BaseVoteViewSet from openslides.utils.auth import has_perm @@ -283,6 +284,10 @@ class AssignmentPollViewSet(BasePollViewSet): ) super().perform_create(serializer) + poll = AssignmentPoll.objects.get(pk=serializer.data["id"]) + poll.db_amount_global_abstain = Decimal(0) + poll.db_amount_global_no = Decimal(0) + poll.save() def handle_analog_vote(self, data, poll, user): for field in ["votesvalid", "votesinvalid", "votescast"]: @@ -291,9 +296,13 @@ class AssignmentPollViewSet(BasePollViewSet): global_no_enabled = ( poll.global_no and poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES ) + if global_no_enabled: + poll.amount_global_no = data.get("amount_global_no", Decimal(0)) global_abstain_enabled = ( poll.global_abstain and poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES ) + if global_abstain_enabled: + poll.amount_global_abstain = data.get("amount_global_abstain", Decimal(0)) options = poll.get_options() options_data = data.get("options") @@ -323,21 +332,8 @@ class AssignmentPollViewSet(BasePollViewSet): ) vote_obj.weight = vote["A"] vote_obj.save() + inform_changed_data(option) - # Create votes for global no and global abstain - first_option = options.first() - if "global_no" in data and global_no_enabled: - vote_obj, _ = AssignmentVote.objects.get_or_create( - option=first_option, value="N" - ) - vote_obj.weight = data["votescast"] - vote_obj.save() - if "global_abstain" in data and global_abstain_enabled: - vote_obj, _ = AssignmentVote.objects.get_or_create( - option=first_option, value="A" - ) - vote_obj.weight = data["votescast"] - vote_obj.save() poll.save() def validate_vote_data(self, data, poll, user): @@ -347,7 +343,7 @@ class AssignmentPollViewSet(BasePollViewSet): { "options": {: {"Y": , ["N": ], ["A": ] }}, ["votesvalid": ], ["votesinvalid": ], ["votescast": ], - ["global_no": ], ["global_abstain": ] + ["amount_global_no": ], ["amount_global_abstain": ] } All amounts are decimals as strings required fields per pollmethod: @@ -363,13 +359,11 @@ 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 grater then 0 and <= poll.votes_amount + - The sum of all amounts must be grater than 0 and <= poll.votes_amount YN/YNA: {: 'Y' | 'N' [|'A']} - 'A' is only allowed in YNA pollmethod - - Votes for all options have to be given """ if poll.type == AssignmentPoll.TYPE_ANALOG: if not isinstance(data, dict): @@ -403,10 +397,14 @@ class AssignmentPollViewSet(BasePollViewSet): poll.global_abstain and poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES ) - if ("global_no" in data and global_no_enabled) or ( - "global_abstain" in data and global_abstain_enabled - ): - data["votescast"] = self.parse_vote_value(data, "votescast") + if "amount_global_abstain" in data and global_abstain_enabled: + data["amount_global_abstain"] = self.parse_vote_value( + data, "amount_global_abstain" + ) + if "amount_global_no" in data and global_no_enabled: + data["amount_global_no"] = self.parse_vote_value( + data, "amount_global_no" + ) else: if poll.pollmethod == AssignmentPoll.POLLMETHOD_VOTES: @@ -415,6 +413,10 @@ class AssignmentPollViewSet(BasePollViewSet): for option_id, amount in data.items(): if not is_int(option_id): raise ValidationError({"detail": "Each id must be an int."}) + if not AssignmentOption.objects.filter(id=option_id).exists(): + raise ValidationError( + {"detail": f"Option {option_id} does not exist."} + ) if not is_int(amount): raise ValidationError( {"detail": "Each amounts must be int"} @@ -456,6 +458,10 @@ class AssignmentPollViewSet(BasePollViewSet): for option_id, value in data.items(): if not is_int(option_id): raise ValidationError({"detail": "Keys must be int"}) + if not AssignmentOption.objects.filter(id=option_id).exists(): + raise ValidationError( + {"detail": f"Option {option_id} does not exist."} + ) if ( poll.pollmethod == AssignmentPoll.POLLMETHOD_YNA and value not in ("Y", "N", "A",) @@ -471,33 +477,6 @@ class AssignmentPollViewSet(BasePollViewSet): options_data = data - 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()) - - # 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 - if data_option_ids != db_option_ids: - raise ValidationError( - {"error": "You have to provide values for all options"} - ) - else: - if not data_option_ids.issubset(db_option_ids): - raise ValidationError( - { - "error": "You gave the following invalid option ids: " - + ", ".join( - str(id) for id in data_option_ids.difference(db_option_ids) - ) - } - ) - def create_votes_type_votes(self, data, poll, user): """ Helper function for handle_(named|pseudoanonymous)_vote @@ -508,9 +487,6 @@ class AssignmentPollViewSet(BasePollViewSet): 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 @@ -519,16 +495,15 @@ class AssignmentPollViewSet(BasePollViewSet): ) 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) + if data == "A": + poll.amount_global_abstain = F("db_amount_global_abstain") + 1 + elif data == "N": + poll.amount_global_no = F("db_amount_global_no") + 1 + else: + raise RuntimeError("This should not happen") + poll.save() + + poll.voted.add(user) def create_votes_type_named_pseudoanonymous( self, data, poll, check_user, vote_user @@ -537,51 +512,37 @@ class AssignmentPollViewSet(BasePollViewSet): 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) + inform_changed_data(option, no_delete_on_restriction=True) + + poll.voted.add(check_user) def handle_named_vote(self, data, poll, user): + if user in poll.voted.all(): + raise ValidationError({"detail": "You have already voted"}) + 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, ): - # 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 user in poll.voted.all(): + raise ValidationError({"detail": "You have already voted"}) + 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) - 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): diff --git a/openslides/motions/migrations/0033_voting_1.py b/openslides/motions/migrations/0033_voting_1.py index da78dd429..b680625ee 100644 --- a/openslides/motions/migrations/0033_voting_1.py +++ b/openslides/motions/migrations/0033_voting_1.py @@ -57,6 +57,11 @@ 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="motionvote", name="user", @@ -107,15 +112,6 @@ 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", diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 138050bae..80cb29480 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -889,14 +889,14 @@ class MotionOptionManager(BaseManager): def get_prefetched_queryset(self, *args, **kwargs): """ - Returns the normal queryset with all voted users. In the background we + Returns the normal queryset. In the background we join and prefetch all related models. """ return ( super() .get_prefetched_queryset(*args, **kwargs) .select_related("poll") - .prefetch_related("voted", "votes") + .prefetch_related("votes") ) @@ -909,9 +909,6 @@ class MotionOption(RESTModelMixin, BaseOption): poll = models.ForeignKey( "MotionPoll", related_name="options", on_delete=CASCADE_AND_AUTOUPDATE ) - voted = models.ManyToManyField( - settings.AUTH_USER_MODEL, blank=True, related_name="motionoption_voted" - ) class Meta: default_permissions = () @@ -931,7 +928,7 @@ class MotionPollManager(BaseManager): super() .get_prefetched_queryset(*args, **kwargs) .select_related("motion") - .prefetch_related("options", "options__votes", "options__voted", "groups") + .prefetch_related("options", "options__votes", "voted", "groups") ) diff --git a/openslides/motions/views.py b/openslides/motions/views.py index 326be6fd9..cb3fb79bb 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -1224,26 +1224,27 @@ class MotionPollViewSet(BasePollViewSet): raise ValidationError("Data must be Y or N") if poll.type == MotionPoll.TYPE_PSEUDOANONYMOUS: - if user in poll.options.get().voted.all(): + if user in poll.voted.all(): raise ValidationError("You already voted on this poll") def handle_named_vote(self, data, poll, user): option = poll.options.get() vote, _ = MotionVote.objects.get_or_create(user=user, option=option) - self.handle_named_and_pseudoanonymous_vote(vote, data, user, option) + self.handle_named_and_pseudoanonymous_vote(data, user, poll, option, vote) def handle_pseudoanonymous_vote(self, data, poll, user): option = poll.options.get() vote = MotionVote.objects.create(user=None, option=option) - self.handle_named_and_pseudoanonymous_vote(vote, data, user, option) + self.handle_named_and_pseudoanonymous_vote(data, user, poll, option, vote) - def handle_named_and_pseudoanonymous_vote(self, vote, data, user, option): + def handle_named_and_pseudoanonymous_vote(self, data, user, poll, option, vote): vote.value = data vote.weight = Decimal("1") vote.save(no_delete_on_restriction=True) + inform_changed_data(option) - option.voted.add(user) - option.save() + poll.voted.add(user) + poll.save() class MotionOptionViewSet(BaseOptionViewSet): diff --git a/openslides/poll/access_permissions.py b/openslides/poll/access_permissions.py index 95dc73be9..576ee0ace 100644 --- a/openslides/poll/access_permissions.py +++ b/openslides/poll/access_permissions.py @@ -6,40 +6,6 @@ from ..utils.access_permissions import BaseAccessPermissions from ..utils.auth import async_has_perm -class BasePollAccessPermissions(BaseAccessPermissions): - manage_permission = "" # set by subclass - - additional_fields: List[str] = [] - """ Add fields to be removed from each unpublished poll """ - - 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. - Non-published polls will be restricted: - - Remove votes* values from the poll - - Remove yes/no/abstain fields from options - - Remove fields given in self.assitional_fields from the poll - """ - if await async_has_perm(user_id, self.manage_permission): - 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"] - for field in self.additional_fields: - del poll[field] - data.append(poll) - return data - - class BaseVoteAccessPermissions(BaseAccessPermissions): manage_permission = "" # set by subclass @@ -71,10 +37,6 @@ 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: @@ -87,6 +49,45 @@ class BaseOptionAccessPermissions(BaseAccessPermissions): del option["yes"] del option["no"] del option["abstain"] - del option["voted_id"] data.append(option) return data + + +class BasePollAccessPermissions(BaseAccessPermissions): + manage_permission = "" # set by subclass + + additional_fields: List[str] = [] + """ Add fields to be removed from each unpublished poll """ + + 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. + Non-published polls will be restricted: + - Remove votes* values from the poll + - Remove yes/no/abstain fields from options + - Remove fields given in self.assitional_fields from the poll + """ + + # add has_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: + 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 field in self.additional_fields: + del poll[field] + data.append(poll) + return data diff --git a/openslides/poll/models.py b/openslides/poll/models.py index 23ab76f58..305535b41 100644 --- a/openslides/poll/models.py +++ b/openslides/poll/models.py @@ -1,5 +1,5 @@ from decimal import Decimal -from typing import Iterable, Optional, Set, Tuple, Type +from typing import Iterable, Optional, Tuple, Type from django.conf import settings from django.core.validators import MinValueValidator @@ -35,8 +35,7 @@ class BaseVote(models.Model): class BaseOption(models.Model): """ - All subclasses must have poll attribute with the related name "options". Also - they must have a "voted" relation to users. + All subclasses must have poll attribute with the related name "options" """ vote_class: Optional[Type["BaseVote"]] = None @@ -86,8 +85,6 @@ class BaseOption(models.Model): vote.save() def reset(self): - self.voted.clear() - # Delete votes votes = self.get_votes() votes_id = [vote.id for vote in votes] @@ -126,6 +123,7 @@ 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, @@ -186,7 +184,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votesvalid else: - return Decimal(self.amount_valid_votes()) + return Decimal(self.amount_users_voted()) def set_votesvalid(self, value): if self.type != self.TYPE_ANALOG: @@ -199,7 +197,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votesinvalid else: - return Decimal(self.amount_invalid_votes()) + return Decimal(0) def set_votesinvalid(self, value): if self.type != self.TYPE_ANALOG: @@ -212,7 +210,7 @@ class BasePoll(models.Model): if self.type == self.TYPE_ANALOG: return self.db_votescast else: - return Decimal(self.amount_voted_users()) + return Decimal(self.amount_users_voted()) def set_votescast(self, value): if self.type != self.TYPE_ANALOG: @@ -221,32 +219,8 @@ class BasePoll(models.Model): votescast = property(get_votescast, set_votescast) - def get_user_ids_with_valid_votes(self): - if self.get_options().count(): - initial_option = self.get_options()[0] - 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) - else: - return [] - - def get_all_voted_user_ids(self): - user_ids: Set[int] = set() - for option in self.get_options(): - user_ids.update(map(lambda u: u.id, option.voted.all())) - 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 amount_users_voted(self): + return len(self.voted.all()) def create_options(self): """ Should be called after creation of this model. """ @@ -284,6 +258,8 @@ class BasePoll(models.Model): for option in self.get_options(): option.reset() + self.voted.clear() + # Reset state self.state = BasePoll.STATE_CREATED if self.type == self.TYPE_ANALOG: diff --git a/openslides/poll/serializers.py b/openslides/poll/serializers.py index e4d5c0501..a2b5d8311 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", "voted") +BASE_OPTION_FIELDS = ("id", "yes", "no", "abstain", "poll_id", "pollstate") class BaseOptionSerializer(ModelSerializer): @@ -31,7 +31,6 @@ 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() @@ -51,6 +50,7 @@ BASE_POLL_FIELDS = ( "id", "onehundred_percent_base", "majority_method", + "voted", ) @@ -60,6 +60,7 @@ class BasePollSerializer(ModelSerializer): many=True, required=False, queryset=get_group_model().objects.all() ) options = IdPrimaryKeyRelatedField(many=True, read_only=True) + voted = IdPrimaryKeyRelatedField(many=True, read_only=True) votesvalid = DecimalField( max_digits=15, decimal_places=6, min_value=-2, read_only=True diff --git a/openslides/poll/views.py b/openslides/poll/views.py index aa7e87a2b..bc7fcc643 100644 --- a/openslides/poll/views.py +++ b/openslides/poll/views.py @@ -175,12 +175,6 @@ class BasePollViewSet(ModelViewSet): @detail_route(methods=["POST"]) def reset(self, request, pk): poll = self.get_object() - - if poll.state not in (BasePoll.STATE_FINISHED, BasePoll.STATE_PUBLISHED): - raise ValidationError( - {"detail": "You can only reset this poll after it is finished"} - ) - poll.reset() return Response() diff --git a/tests/integration/assignments/test_polls.py b/tests/integration/assignments/test_polls.py index baabac71f..de27e12ef 100644 --- a/tests/integration/assignments/test_polls.py +++ b/tests/integration/assignments/test_polls.py @@ -53,12 +53,11 @@ def test_assignment_option_db_queries(): """ Tests that only the following db queries are done: * 1 request to get the options, - * 1 request to get all users that voted on the options, * 1 request to get all votes for all options, - = 3 queries + = 2 queries """ create_assignment_polls() - assert count_queries(AssignmentOption.get_elements)() == 3 + assert count_queries(AssignmentOption.get_elements)() == 2 def create_assignment_polls(): @@ -93,13 +92,13 @@ def create_assignment_polls(): username=f"test_username_{i}{j}", password="test_password_kbzj5L8ZtVxBllZzoW6D", ) + poll.voted.add(user) for option in poll.options.all(): weight = random.randint(0, 10) if weight > 0: AssignmentVote.objects.create( user=user, option=option, value="Y", weight=Decimal(weight) ) - option.voted.add(user) class CreateAssignmentPoll(TestCase): @@ -110,7 +109,7 @@ class CreateAssignmentPoll(TestCase): self.assignment.add_candidate(self.admin) def test_simple(self): - with self.assertNumQueries(50): + with self.assertNumQueries(40): response = self.client.post( reverse("assignmentpoll-list"), { @@ -1008,10 +1007,10 @@ class VoteAssignmentPollNamedYNA(VoteAssignmentPollBaseTestClass): {"1": "N"}, format="json", ) - self.assertHttpStatusVerbose(response, status.HTTP_200_OK) + self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) self.assertEqual(AssignmentVote.objects.count(), 1) vote = AssignmentVote.objects.get() - self.assertEqual(vote.value, "N") + self.assertEqual(vote.value, "Y") def test_too_many_options(self): self.start_poll() @@ -1197,14 +1196,14 @@ class VoteAssignmentPollNamedVotes(VoteAssignmentPollBaseTestClass): {"1": 0, "2": 1}, format="json", ) - self.assertHttpStatusVerbose(response, status.HTTP_200_OK) + self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) poll = AssignmentPoll.objects.get() option1 = poll.options.get(pk=1) option2 = poll.options.get(pk=2) - self.assertEqual(option1.yes, Decimal("0")) + self.assertEqual(option1.yes, Decimal("1")) self.assertEqual(option1.no, Decimal("0")) self.assertEqual(option1.abstain, Decimal("0")) - self.assertEqual(option2.yes, Decimal("1")) + self.assertEqual(option2.yes, Decimal("0")) self.assertEqual(option2.no, Decimal("0")) self.assertEqual(option2.abstain, Decimal("0")) @@ -1219,9 +1218,9 @@ class VoteAssignmentPollNamedVotes(VoteAssignmentPollBaseTestClass): poll = AssignmentPoll.objects.get() option = poll.options.get(pk=1) self.assertEqual(option.yes, Decimal("0")) - self.assertEqual(option.no, Decimal("2")) + self.assertEqual(option.no, Decimal("0")) self.assertEqual(option.abstain, Decimal("0")) - self.assertEqual(poll.amount_global_no, Decimal("2")) + self.assertEqual(poll.amount_global_no, Decimal("1")) self.assertEqual(poll.amount_global_abstain, Decimal("0")) def test_global_no_forbidden(self): @@ -1247,9 +1246,9 @@ class VoteAssignmentPollNamedVotes(VoteAssignmentPollBaseTestClass): option = poll.options.get(pk=1) self.assertEqual(option.yes, Decimal("0")) self.assertEqual(option.no, Decimal("0")) - self.assertEqual(option.abstain, Decimal("2")) + self.assertEqual(option.abstain, Decimal("0")) self.assertEqual(poll.amount_global_no, Decimal("0")) - self.assertEqual(poll.amount_global_abstain, Decimal("2")) + self.assertEqual(poll.amount_global_abstain, Decimal("1")) def test_global_abstain_forbidden(self): self.poll.global_abstain = False @@ -1302,17 +1301,6 @@ class VoteAssignmentPollNamedVotes(VoteAssignmentPollBaseTestClass): self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) self.assertFalse(AssignmentPoll.objects.get().get_votes().exists()) - def test_missing_option(self): - self.add_candidate() - self.start_poll() - response = self.client.post( - reverse("assignmentpoll-vote", args=[self.poll.pk]), - {"1": 1}, - format="json", - ) - self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) - self.assertFalse(AssignmentPoll.objects.get().get_votes().exists()) - def test_too_many_options(self): self.setup_for_multiple_votes() self.start_poll() @@ -1370,7 +1358,7 @@ class VoteAssignmentPollNamedVotes(VoteAssignmentPollBaseTestClass): def test_missing_data(self): self.start_poll() response = self.client.post(reverse("assignmentpoll-vote", args=[self.poll.pk])) - self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) + self.assertHttpStatusVerbose(response, status.HTTP_200_OK) self.assertFalse(AssignmentVote.objects.exists()) def test_wrong_data_format(self): @@ -1557,9 +1545,7 @@ class VoteAssignmentPollPseudoanonymousYNA(VoteAssignmentPollBaseTestClass): def test_missing_data(self): self.start_poll() response = self.client.post(reverse("assignmentpoll-vote", args=[self.poll.pk])) - self.assertHttpStatusVerbose( - response, status.HTTP_200_OK - ) # new "feature" because of partial requests: empty requests work! + self.assertHttpStatusVerbose(response, status.HTTP_200_OK) self.assertFalse(AssignmentVote.objects.exists()) def test_wrong_data_format(self): @@ -1718,17 +1704,6 @@ class VoteAssignmentPollPseudoanonymousVotes(VoteAssignmentPollBaseTestClass): self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) self.assertFalse(AssignmentPoll.objects.get().get_votes().exists()) - def test_missing_option(self): - self.add_candidate() - self.start_poll() - response = self.client.post( - reverse("assignmentpoll-vote", args=[self.poll.pk]), - {"1": 1}, - format="json", - ) - self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) - self.assertFalse(AssignmentPoll.objects.get().get_votes().exists()) - def test_too_many_options(self): self.setup_for_multiple_votes() self.start_poll() @@ -1786,7 +1761,7 @@ class VoteAssignmentPollPseudoanonymousVotes(VoteAssignmentPollBaseTestClass): def test_missing_data(self): self.start_poll() response = self.client.post(reverse("assignmentpoll-vote", args=[self.poll.pk])) - self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) + self.assertHttpStatusVerbose(response, status.HTTP_200_OK) self.assertFalse(AssignmentVote.objects.exists()) def test_wrong_data_format(self): @@ -1909,6 +1884,8 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) "votescast": "1.000000", "votesinvalid": "0.000000", "votesvalid": "1.000000", + "user_has_voted": False, + "voted_id": [self.user.id], }, "assignments/assignment-option:1": { "abstain": "1.000000", @@ -1919,8 +1896,6 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) "yes": "0.000000", "user_id": 1, "weight": 1, - "user_has_voted": False, - "voted_id": [self.user.id], }, "assignments/assignment-vote:1": { "id": 1, @@ -1971,6 +1946,7 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) "options_id": [1], "id": 1, "votes_amount": 1, + "user_has_voted": user == self.user, }, ) @@ -1984,7 +1960,7 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) vote.value = "A" vote.weight = Decimal("1") vote.save(no_delete_on_restriction=True, skip_autoupdate=True) - option.voted.add(self.user.id) + self.poll.voted.add(self.user.id) self.poll.state = AssignmentPoll.STATE_FINISHED self.poll.save(skip_autoupdate=True) response = self.client.post( @@ -2026,6 +2002,8 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) "votescast": "1.000000", "votesinvalid": "0.000000", "votesvalid": "1.000000", + "user_has_voted": user == self.user, + "voted_id": [self.user.id], }, "assignments/assignment-vote:1": { "pollstate": AssignmentPoll.STATE_PUBLISHED, @@ -2044,8 +2022,6 @@ class VoteAssignmentPollNamedAutoupdates(VoteAssignmentPollAutoupdatesBaseClass) "yes": "0.000000", "user_id": 1, "weight": 1, - "user_has_voted": user == self.user, - "voted_id": [self.user.id], }, }, ) @@ -2083,6 +2059,8 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( "title": self.poll.title, "description": self.description, "type": AssignmentPoll.TYPE_PSEUDOANONYMOUS, + "user_has_voted": False, + "voted_id": [self.user.id], "onehundred_percent_base": AssignmentPoll.PERCENT_BASE_CAST, "majority_method": AssignmentPoll.MAJORITY_TWO_THIRDS, "votes_amount": 1, @@ -2099,8 +2077,6 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( "yes": "0.000000", "user_id": 1, "weight": 1, - "user_has_voted": False, - "voted_id": [self.user.id], }, "assignments/assignment-vote:1": { "id": 1, @@ -2137,6 +2113,7 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( "options_id": [1], "id": 1, "votes_amount": 1, + "user_has_voted": user == self.user, }, ) @@ -2149,7 +2126,7 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( vote.value = "A" vote.weight = Decimal("1") vote.save(no_delete_on_restriction=True, skip_autoupdate=True) - option.voted.add(self.user.id) + self.poll.voted.add(self.user.id) self.poll.state = AssignmentPoll.STATE_FINISHED self.poll.save(skip_autoupdate=True) response = self.client.post( @@ -2191,6 +2168,8 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( "votescast": "1.000000", "votesinvalid": "0.000000", "votesvalid": "1.000000", + "user_has_voted": user == self.user, + "voted_id": [self.user.id], }, "assignments/assignment-vote:1": { "pollstate": AssignmentPoll.STATE_PUBLISHED, @@ -2209,8 +2188,6 @@ class VoteAssignmentPollPseudoanonymousAutoupdates( "yes": "0.000000", "user_id": 1, "weight": 1, - "user_has_voted": user == self.user, - "voted_id": [self.user.id], }, }, ) diff --git a/tests/integration/motions/test_polls.py b/tests/integration/motions/test_polls.py index 93844fe0b..788e2eb6a 100644 --- a/tests/integration/motions/test_polls.py +++ b/tests/integration/motions/test_polls.py @@ -47,11 +47,10 @@ def test_motion_option_db_queries(): Tests that only the following db queries are done: * 1 request to get the options, * 1 request to get all votes for all options, - * 1 request to get all users that voted on the options - = 5 queries + = 2 queries """ create_motion_polls() - assert count_queries(MotionOption.get_elements)() == 3 + assert count_queries(MotionOption.get_elements)() == 2 def create_motion_polls(): @@ -83,7 +82,7 @@ def create_motion_polls(): value=("Y" if k == 0 else "N"), weight=Decimal(1), ) - option.voted.add(user) + poll.voted.add(user) class CreateMotionPoll(TestCase): @@ -166,6 +165,8 @@ class CreateMotionPoll(TestCase): "votescast": "0.000000", "options_id": [1], "id": 1, + "voted_id": [], + "user_has_voted": False, }, ) self.assertEqual(autoupdate[1], []) @@ -910,6 +911,8 @@ class VoteMotionPollNamedAutoupdates(TestCase): "votescast": "1.000000", "options_id": [1], "id": 1, + "user_has_voted": False, + "voted_id": [self.user.id], }, "motions/motion-vote:1": { "pollstate": 2, @@ -926,8 +929,6 @@ class VoteMotionPollNamedAutoupdates(TestCase): "poll_id": 1, "pollstate": 2, "yes": "0.000000", - "user_has_voted": False, - "voted_id": [self.user.id], }, }, ) @@ -948,7 +949,7 @@ class VoteMotionPollNamedAutoupdates(TestCase): ) self.assertEqual( autoupdate[0]["motions/motion-option:1"], - {"id": 1, "poll_id": 1, "pollstate": 2, "user_has_voted": True}, + {"id": 1, "poll_id": 1, "pollstate": 2}, ) self.assertEqual(autoupdate[1], []) @@ -969,6 +970,7 @@ class VoteMotionPollNamedAutoupdates(TestCase): "groups_id": [GROUP_DELEGATE_PK], "options_id": [1], "id": 1, + "user_has_voted": user == self.user, }, ) self.assertEqual( @@ -977,8 +979,7 @@ class VoteMotionPollNamedAutoupdates(TestCase): "id": 1, "poll_id": 1, "pollstate": 2, - "user_has_voted": user == self.user, - }, + }, # noqa black and flake are no friends :( ) # Other users should not get a vote autoupdate @@ -1049,6 +1050,8 @@ class VoteMotionPollPseudoanonymousAutoupdates(TestCase): "votescast": "1.000000", "options_id": [1], "id": 1, + "user_has_voted": False, + "voted_id": [self.user.id], }, "motions/motion-vote:1": { "pollstate": 2, @@ -1065,8 +1068,6 @@ class VoteMotionPollPseudoanonymousAutoupdates(TestCase): "poll_id": 1, "pollstate": 2, "yes": "0.000000", - "user_has_voted": False, - "voted_id": [self.user.id], }, }, ) @@ -1090,6 +1091,7 @@ class VoteMotionPollPseudoanonymousAutoupdates(TestCase): "groups_id": [GROUP_DELEGATE_PK], "options_id": [1], "id": 1, + "user_has_voted": user == self.user, }, ) @@ -1153,12 +1155,12 @@ class VoteMotionPollPseudoanonymous(TestCase): self.assertEqual(poll.votesinvalid, Decimal("0")) self.assertEqual(poll.votescast, Decimal("1")) self.assertEqual(poll.get_votes().count(), 1) - self.assertEqual(poll.amount_valid_votes(), 1) + self.assertEqual(poll.amount_users_voted(), 1) option = poll.options.get() self.assertEqual(option.yes, Decimal("0")) self.assertEqual(option.no, Decimal("1")) self.assertEqual(option.abstain, Decimal("0")) - self.assertTrue(self.admin in option.voted.all()) + self.assertTrue(self.admin in poll.voted.all()) vote = option.votes.get() self.assertEqual(vote.user, None) @@ -1311,6 +1313,8 @@ class PublishMotionPoll(TestCase): "votescast": "0.000000", "options_id": [1], "id": 1, + "user_has_voted": False, + "voted_id": [], }, "motions/motion-vote:1": { "pollstate": 4, @@ -1327,8 +1331,6 @@ class PublishMotionPoll(TestCase): "poll_id": 1, "pollstate": 4, "yes": "0.000000", - "user_has_voted": False, - "voted_id": [], }, }, ) @@ -1362,12 +1364,12 @@ class PseudoanonymizeMotionPoll(TestCase): self.vote1 = MotionVote.objects.create( user=self.user1, option=self.option, value="Y", weight=Decimal(1) ) - self.option.voted.add(self.user1) + self.poll.voted.add(self.user1) self.user2, _ = self.create_user() self.vote2 = MotionVote.objects.create( user=self.user2, option=self.option, value="N", weight=Decimal(1) ) - self.option.voted.add(self.user2) + self.poll.voted.add(self.user2) def test_pseudoanonymize_poll(self): response = self.client.post( @@ -1376,7 +1378,7 @@ class PseudoanonymizeMotionPoll(TestCase): self.assertHttpStatusVerbose(response, status.HTTP_200_OK) poll = MotionPoll.objects.get() self.assertEqual(poll.get_votes().count(), 2) - self.assertEqual(poll.amount_valid_votes(), 2) + self.assertEqual(poll.amount_users_voted(), 2) self.assertEqual(poll.votesvalid, Decimal("2")) self.assertEqual(poll.votesinvalid, Decimal("0")) self.assertEqual(poll.votescast, Decimal("2")) @@ -1384,8 +1386,8 @@ class PseudoanonymizeMotionPoll(TestCase): self.assertEqual(option.yes, Decimal("1")) self.assertEqual(option.no, Decimal("1")) self.assertEqual(option.abstain, Decimal("0")) - self.assertTrue(self.user1 in option.voted.all()) - self.assertTrue(self.user2 in option.voted.all()) + self.assertTrue(self.user1 in poll.voted.all()) + self.assertTrue(self.user2 in poll.voted.all()) for vote in poll.get_votes().all(): self.assertTrue(vote.user is None) @@ -1432,19 +1434,19 @@ class ResetMotionPoll(TestCase): self.vote1 = MotionVote.objects.create( user=self.user1, option=self.option, value="Y", weight=Decimal(1) ) - self.option.voted.add(self.user1) + self.poll.voted.add(self.user1) self.user2, _ = self.create_user() self.vote2 = MotionVote.objects.create( user=self.user2, option=self.option, value="N", weight=Decimal(1) ) - self.option.voted.add(self.user2) + self.poll.voted.add(self.user2) def test_reset_poll(self): response = self.client.post(reverse("motionpoll-reset", args=[self.poll.pk])) self.assertHttpStatusVerbose(response, status.HTTP_200_OK) poll = MotionPoll.objects.get() self.assertEqual(poll.get_votes().count(), 0) - self.assertEqual(poll.amount_valid_votes(), 0) + self.assertEqual(poll.amount_users_voted(), 0) self.assertEqual(poll.votesvalid, None) self.assertEqual(poll.votesinvalid, None) self.assertEqual(poll.votescast, None) @@ -1463,12 +1465,3 @@ class ResetMotionPoll(TestCase): for user in (self.admin, self.user1, self.user2): self.assertDeletedAutoupdate(self.vote1, user=user) self.assertDeletedAutoupdate(self.vote2, user=user) - - def test_reset_wrong_state(self): - self.poll.state = MotionPoll.STATE_STARTED - self.poll.save() - response = self.client.post(reverse("motionpoll-reset", args=[self.poll.pk])) - self.assertHttpStatusVerbose(response, status.HTTP_400_BAD_REQUEST) - poll = MotionPoll.objects.get() - self.assertTrue(poll.get_votes().exists()) - self.assertEqual(poll.amount_valid_votes(), 2)