From ccc48e6b3fe1ad351a4acc3daddadb045bab61a7 Mon Sep 17 00:00:00 2001 From: Sean Date: Wed, 14 Oct 2020 15:08:14 +0200 Subject: [PATCH] Add "point of order" feature to ListOfSpeakers Adds the option "point of order" to the list of speakers - You can make a point of order even though you normally have no permission to add yourself to the ListOfSpeakers - You can make a point of order even though you are already on theListOfSpeakers (but you may only be there once) - new points of order will be on top of the list of speakers - Point of orders will be highlighted by a red triangle This feature can be used to request to speak with a higher level of urgency --- .../list-of-speakers-repository.service.ts | 31 ++++- .../list-of-speakers-content.component.html | 86 +++++++++--- .../list-of-speakers-content.component.scss | 23 ++- .../list-of-speakers-content.component.ts | 69 ++++++--- .../sorting-list/sorting-list.component.scss | 1 - .../src/app/shared/models/agenda/speaker.ts | 1 + .../list-of-speakers.component.html | 4 +- .../list-of-speakers.component.ts | 2 +- client/src/tslint.json | 1 - server/openslides/agenda/config_variables.py | 10 ++ .../migrations/0010_speaker_point_of_order.py | 18 +++ server/openslides/agenda/models.py | 42 ++++-- server/openslides/agenda/serializers.py | 10 +- server/openslides/agenda/views.py | 75 ++++++---- .../tests/integration/agenda/test_viewset.py | 131 +++++++++++++++++- server/tests/unit/agenda/test_views.py | 6 +- 16 files changed, 416 insertions(+), 94 deletions(-) create mode 100644 server/openslides/agenda/migrations/0010_speaker_point_of_order.py diff --git a/client/src/app/core/repositories/agenda/list-of-speakers-repository.service.ts b/client/src/app/core/repositories/agenda/list-of-speakers-repository.service.ts index 417d2ec52..e0e8c4013 100644 --- a/client/src/app/core/repositories/agenda/list-of-speakers-repository.service.ts +++ b/client/src/app/core/repositories/agenda/list-of-speakers-repository.service.ts @@ -143,6 +143,14 @@ export class ListOfSpeakersRepositoryService extends BaseHasContentObjectReposit } }; + public async delete(viewModel: ViewListOfSpeakers): Promise { + throw new Error('Not supported'); + } + + public async create(model: ListOfSpeakers): Promise { + throw new Error('Not supported'); + } + /** * Add a new speaker to a list of speakers. * Sends the users id to the server @@ -150,9 +158,13 @@ export class ListOfSpeakersRepositoryService extends BaseHasContentObjectReposit * @param userId {@link User} id of the new speaker * @param listOfSpeakers the target agenda item */ - public async createSpeaker(listOfSpeakers: ViewListOfSpeakers, userId: number): Promise { + public async createSpeaker( + listOfSpeakers: ViewListOfSpeakers, + userId: number, + pointOfOrder?: boolean + ): Promise { const restUrl = this.getRestUrl(listOfSpeakers.id, 'manage_speaker'); - return await this.httpService.post(restUrl, { user: userId }); + return await this.httpService.post(restUrl, { user: userId, point_of_order: pointOfOrder }); } /** @@ -162,9 +174,20 @@ export class ListOfSpeakersRepositoryService extends BaseHasContentObjectReposit * @param speakerId (otional) the speakers id. If no id is given, the speaker with the * current operator is removed. */ - public async delete(listOfSpeakers: ViewListOfSpeakers, speakerId?: number): Promise { + public async deleteSpeaker( + listOfSpeakers: ViewListOfSpeakers, + speakerId?: number, + pointOfOrder?: boolean + ): Promise { const restUrl = this.getRestUrl(listOfSpeakers.id, 'manage_speaker'); - await this.httpService.delete(restUrl, speakerId ? { speaker: speakerId } : null); + const payload: { speaker?: number; point_of_order?: boolean } = {}; + if (speakerId) { + payload.speaker = speakerId; + } + if (pointOfOrder) { + payload.point_of_order = pointOfOrder; + } + await this.httpService.delete(restUrl, payload); } /** diff --git a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.html b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.html index f7cf7ed53..47ce29bc3 100644 --- a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.html +++ b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.html @@ -4,9 +4,7 @@ {{ title }} - - lock - + lock @@ -22,6 +20,7 @@
{{ number + 1 }}.
{{ speaker.getTitle() }}
+
warning
{{ durationString(speaker) }} ({{ 'Start time' | translate }}: {{ startTimeToString(speaker) }})
@@ -30,7 +29,7 @@ mat-icon-button matTooltip="{{ 'Remove' | translate }}" *osPerms="'agenda.can_manage_list_of_speakers'" - (click)="onDeleteButton(speaker)" + (click)="removeSpeaker(speaker)" > close @@ -53,6 +52,15 @@ {{ activeSpeaker.getListTitle() }} + + + @@ -110,21 +119,34 @@ mat-icon-button matTooltip="{{ 'Mark speaker' | translate }}" (click)="onMarkButton(speaker)" + *osPerms="'agenda.can_manage_list_of_speakers'; and: !speaker.point_of_order" > {{ speaker.marked ? 'star' : 'star_border' }} + + + - - - star - + star @@ -155,17 +177,37 @@
- -
-
- - -
+
+ + + + + + + + +
diff --git a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.scss b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.scss index bab4353a5..76b2aad69 100644 --- a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.scss +++ b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.scss @@ -26,7 +26,7 @@ @include desktop { .finished-speaker-grid { - grid-template-areas: 'number name time controls'; + grid-template-areas: 'number name point-of-order time controls'; grid-template-columns: 30px 1fr min-content min-content; } } @@ -41,6 +41,18 @@ margin: 0; } + .point-of-order { + grid-area: point-of-order; + margin: 0; + //allows pushing this grid area as small as possible and aligns the end to the same level + white-space: nowrap; + font-size: 80%; + + .mat-icon { + line-height: 19px; + } + } + .time { grid-area: time; margin: 0; @@ -102,7 +114,7 @@ } .search-new-speaker-form { - padding: 15px 25px 10px 25px; + padding: 15px 25px 0 25px; width: auto; .search-users-field { @@ -117,7 +129,12 @@ } .add-self-buttons { - padding: 15px 0 20px 25px; + margin: 15px 25px; + display: flex; + + button + button { + margin-left: 20px; + } } .speaker-warning { diff --git a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.ts b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.ts index 60adcbb77..90ef75b3e 100644 --- a/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.ts +++ b/client/src/app/shared/components/list-of-speakers-content/list-of-speakers-content.component.ts @@ -58,6 +58,8 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i public showFistContributionHint: boolean; + public showPointOfOrders: boolean; + public get title(): string { return this.viewListOfSpeakers?.getTitle(); } @@ -70,10 +72,6 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i return this.operator.hasPerms(this.permission.agendaCanManageListOfSpeakers); } - public get isOpInList(): boolean { - return this.waitingSpeakers.some(speaker => speaker.user_id === this.operator.user.id); - } - public get canAddSelf(): boolean { return !this.config.instant('agenda_present_speakers_only') || this.operator.user.is_present; } @@ -98,7 +96,7 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i private isListOfSpeakersEmptyEvent = new EventEmitter(); @Output() - private hasFinishesSpeakersEvent = new EventEmitter(); + private canReaddLastSpeakerEvent = new EventEmitter(); public constructor( title: Title, @@ -129,7 +127,7 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i this.addSpeakerForm.valueChanges.subscribe(formResult => { // resetting a form triggers a form.next(null) - check if user_id if (formResult && formResult.user_id) { - this.addNewSpeaker(formResult.user_id); + this.addUserAsNewSpeaker(formResult.user_id); } }), // observe changes to the viewport @@ -144,6 +142,10 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i // observe changes to the agenda_show_first_contribution config this.config.get('agenda_show_first_contribution').subscribe(show => { this.showFistContributionHint = show; + }), + // observe point of order settings + this.config.get('agenda_enable_point_of_order_speakers').subscribe(show => { + this.showPointOfOrders = show; }) ); } @@ -157,8 +159,16 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i return this.isListOfSpeakersEmptyEvent.emit(!this.activeSpeaker); } - private hasFinishesSpeakers(): void { - this.hasFinishesSpeakersEvent.emit(this.finishedSpeakers?.length > 0); + private updateCanReaddLastSpeaker(): void { + let canReaddLast; + if (this.finishedSpeakers?.length > 0) { + const lastSpeaker = this.finishedSpeakers[this.finishedSpeakers.length - 1]; + const isLastSpeakerWaiting = this.waitingSpeakers.some(speaker => speaker.user_id === lastSpeaker.user_id); + canReaddLast = !lastSpeaker.point_of_order && !isLastSpeakerWaiting; + } else { + canReaddLast = false; + } + this.canReaddLastSpeakerEvent.emit(canReaddLast); } /** @@ -166,10 +176,13 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i * * @param userId the user id to add to the list. No parameter adds the operators user as speaker. */ - public addNewSpeaker(userId?: number): void { - this.listOfSpeakersRepo - .createSpeaker(this.viewListOfSpeakers, userId) - .then(() => this.addSpeakerForm.reset(), this.raiseError); + public async addUserAsNewSpeaker(userId?: number): Promise { + try { + await this.listOfSpeakersRepo.createSpeaker(this.viewListOfSpeakers, userId); + this.addSpeakerForm.reset(); + } catch (e) { + this.raiseError(e); + } } /** @@ -178,13 +191,13 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i * @param speaker optional speaker to remove. If none is given, * the operator themself is removed */ - public async onDeleteButton(speaker?: ViewSpeaker): Promise { + public async removeSpeaker(speaker?: ViewSpeaker): Promise { const title = this.translate.instant( 'Are you sure you want to delete this speaker from this list of speakers?' ); if (await this.promptService.open(title)) { try { - await this.listOfSpeakersRepo.delete(this.viewListOfSpeakers, speaker ? speaker.id : null); + await this.listOfSpeakersRepo.deleteSpeaker(this.viewListOfSpeakers, speaker ? speaker.id : null); this.filterUsers(); } catch (e) { this.raiseError(e); @@ -192,6 +205,30 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i } } + public async addPointOfOrder(): Promise { + const title = this.translate.instant('Are you sure you want to submit a new point of order?'); + if (await this.promptService.open(title)) { + try { + await this.listOfSpeakersRepo.createSpeaker(this.viewListOfSpeakers, undefined, true); + } catch (e) { + this.raiseError(e); + } + } + } + + public removePointOfOrder(): void { + this.listOfSpeakersRepo.deleteSpeaker(this.viewListOfSpeakers, undefined, true).catch(this.raiseError); + } + + public isOpInWaitlist(pointOfOrder: boolean = false): boolean { + if (!this.waitingSpeakers) { + return false; + } + return this.waitingSpeakers.some( + speaker => speaker.user_id === this.operator.user.id && speaker.point_of_order === pointOfOrder + ); + } + /** * Click on the mic button to mark a speaker as speaking * @@ -268,7 +305,7 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i }); this.activeSpeaker = allSpeakers?.find(speaker => speaker.state === SpeakerState.CURRENT); - this.hasFinishesSpeakers(); + this.updateCanReaddLastSpeaker(); this.isListOfSpeakersEmpty(); } @@ -293,7 +330,7 @@ export class ListOfSpeakersContentComponent extends BaseViewComponentDirective i */ public async onCreateUser(username: string): Promise { const newUser = await this.userRepository.createFromString(username); - this.addNewSpeaker(newUser.id); + this.addUserAsNewSpeaker(newUser.id); } /** diff --git a/client/src/app/shared/components/sorting-list/sorting-list.component.scss b/client/src/app/shared/components/sorting-list/sorting-list.component.scss index 5ec088215..94a71d4aa 100644 --- a/client/src/app/shared/components/sorting-list/sorting-list.component.scss +++ b/client/src/app/shared/components/sorting-list/sorting-list.component.scss @@ -33,7 +33,6 @@ .section-three { display: table-cell; padding-right: 10px; - vertical-align: middle; width: auto; white-space: nowrap; } diff --git a/client/src/app/shared/models/agenda/speaker.ts b/client/src/app/shared/models/agenda/speaker.ts index 403100245..d4ee00d42 100644 --- a/client/src/app/shared/models/agenda/speaker.ts +++ b/client/src/app/shared/models/agenda/speaker.ts @@ -15,6 +15,7 @@ export class Speaker extends BaseModel { public weight: number; public marked: boolean; public item_id: number; + public point_of_order: boolean; /** * ISO datetime string to indicate the begin time of the speech. Empty if diff --git a/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.html b/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.html index 74fa9376b..9051ed798 100644 --- a/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.html +++ b/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.html @@ -18,7 +18,7 @@ mat-icon-button matTooltip="{{ 'Re-add last speaker' | translate }}" (click)="readdLastSpeaker()" - [disabled]="!hasFinishedSpeakers" + [disabled]="!canReaddLastSpeaker" > undo @@ -31,7 +31,7 @@ [speakers]="viewListOfSpeakers" [sortMode]="manualSortMode" (isListOfSpeakersEmptyEvent)="isListOfSpeakersEmpty = $event" - (hasFinishesSpeakersEvent)="hasFinishedSpeakers = $event" + (canReaddLastSpeakerEvent)="canReaddLastSpeaker = $event" > diff --git a/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.ts b/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.ts index b38c1f771..102f6f8b9 100644 --- a/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.ts +++ b/client/src/app/site/agenda/components/list-of-speakers/list-of-speakers.component.ts @@ -63,7 +63,7 @@ export class ListOfSpeakersComponent extends BaseViewComponentDirective implemen /** * filled by child component */ - public hasFinishedSpeakers: boolean; + public canReaddLastSpeaker: boolean; /** * Constructor for speaker list component. Generates the forms. diff --git a/client/src/tslint.json b/client/src/tslint.json index 708c9a964..9eeecf0ce 100644 --- a/client/src/tslint.json +++ b/client/src/tslint.json @@ -15,7 +15,6 @@ "order": ["static-field", "static-method", "instance-field", "constructor", "instance-method"] } ], - "no-unused-variable": true, "typedef": [ true, "call-signature", diff --git a/server/openslides/agenda/config_variables.py b/server/openslides/agenda/config_variables.py index a97ba3f28..48c72bf6d 100644 --- a/server/openslides/agenda/config_variables.py +++ b/server/openslides/agenda/config_variables.py @@ -136,6 +136,16 @@ def get_config_variables(): validators=(MinValueValidator(-1),), ) + yield ConfigVariable( + name="agenda_enable_point_of_order_speakers", + default_value=False, + input_type="boolean", + label="Enables point of order speakers", + weight=223, + group="Agenda", + subgroup="List of speakers", + ) + yield ConfigVariable( name="agenda_countdown_warning_time", default_value=0, diff --git a/server/openslides/agenda/migrations/0010_speaker_point_of_order.py b/server/openslides/agenda/migrations/0010_speaker_point_of_order.py new file mode 100644 index 000000000..7b496ffca --- /dev/null +++ b/server/openslides/agenda/migrations/0010_speaker_point_of_order.py @@ -0,0 +1,18 @@ +# Generated by Django 2.2.16 on 2020-10-13 11:53 + +from django.db import migrations, models + + +class Migration(migrations.Migration): + + dependencies = [ + ("agenda", "0009_item_tags"), + ] + + operations = [ + migrations.AddField( + model_name="speaker", + name="point_of_order", + field=models.BooleanField(default=False), + ), + ] diff --git a/server/openslides/agenda/models.py b/server/openslides/agenda/models.py index 6167e00be..385f0316a 100644 --- a/server/openslides/agenda/models.py +++ b/server/openslides/agenda/models.py @@ -424,7 +424,7 @@ class SpeakerManager(models.Manager): Manager for Speaker model. Provides a customized add method. """ - def add(self, user, list_of_speakers, skip_autoupdate=False): + def add(self, user, list_of_speakers, skip_autoupdate=False, point_of_order=False): """ Customized manager method to prevent anonymous users to be on the list of speakers and that someone is twice on one list (off coming @@ -433,20 +433,39 @@ class SpeakerManager(models.Manager): if isinstance(user, AnonymousUser): raise OpenSlidesError("An anonymous user can not be on lists of speakers.") + if point_of_order and not config["agenda_enable_point_of_order_speakers"]: + raise OpenSlidesError("Point of order speakers are not enabled.") + if self.filter( - user=user, list_of_speakers=list_of_speakers, begin_time=None + user=user, + list_of_speakers=list_of_speakers, + begin_time=None, + point_of_order=point_of_order, ).exists(): raise OpenSlidesError(f"{user} is already on the list of speakers.") if config["agenda_present_speakers_only"] and not user.is_present: raise OpenSlidesError("Only present users can be on the lists of speakers.") - weight = ( - self.filter(list_of_speakers=list_of_speakers).aggregate( - models.Max("weight") - )["weight__max"] - or 0 - ) + + if point_of_order: + weight = ( + self.filter(list_of_speakers=list_of_speakers).aggregate( + models.Min("weight") + )["weight__min"] + or 0 + ) - 1 + else: + weight = ( + self.filter(list_of_speakers=list_of_speakers).aggregate( + models.Max("weight") + )["weight__max"] + or 0 + ) + 1 + speaker = self.model( - list_of_speakers=list_of_speakers, user=user, weight=weight + 1 + list_of_speakers=list_of_speakers, + user=user, + weight=weight, + point_of_order=point_of_order, ) speaker.save( force_insert=True, @@ -495,6 +514,11 @@ class Speaker(RESTModelMixin, models.Model): Marks a speaker. """ + point_of_order = models.BooleanField(default=False) + """ + Identifies the speaker as someone with a point of order + """ + class Meta: default_permissions = () permissions = (("can_be_speaker", "Can put oneself on the list of speakers"),) diff --git a/server/openslides/agenda/serializers.py b/server/openslides/agenda/serializers.py index b7bea0865..b05ae295b 100644 --- a/server/openslides/agenda/serializers.py +++ b/server/openslides/agenda/serializers.py @@ -10,7 +10,15 @@ class SpeakerSerializer(ModelSerializer): class Meta: model = Speaker - fields = ("id", "user", "begin_time", "end_time", "weight", "marked") + fields = ( + "id", + "user", + "begin_time", + "end_time", + "weight", + "marked", + "point_of_order", + ) class RelatedItemRelatedField(RelatedField): diff --git a/server/openslides/agenda/views.py b/server/openslides/agenda/views.py index f35b430ca..3c5b83410 100644 --- a/server/openslides/agenda/views.py +++ b/server/openslides/agenda/views.py @@ -310,8 +310,10 @@ class ListOfSpeakersViewSet( def manage_speaker(self, request, pk=None): """ Special view endpoint to add users to the list of speakers or remove - them. Send POST {'user': } to add a new speaker. Omit - data to add yourself. Send DELETE {'speaker': } or + them. Send POST {'user': } to add a new speaker. + Send POST {'user': , 'point_of_order': True } to add a point + of order to the list of speakers. + Omit data to add yourself. Send DELETE {'speaker': } or DELETE {'speaker': [, , ...]} to remove one or more speakers from the list of speakers. Omit data to remove yourself. Send PATCH {'user': , 'marked': } to mark the speaker. @@ -329,16 +331,22 @@ class ListOfSpeakersViewSet( if request.method == "POST": # Add new speaker # Retrieve user_id user_id = request.data.get("user") + point_of_order = request.data.get("point_of_order") or False + if not isinstance(point_of_order, bool): + raise ValidationError({"detail": "point_of_order has to be a bool."}) # Check permissions and other conditions. Get user instance. if user_id is None: # Add oneself - if not has_perm(self.request.user, "agenda.can_be_speaker"): + if not point_of_order and not has_perm( + self.request.user, "agenda.can_be_speaker" + ): self.permission_denied(request) if list_of_speakers.closed: raise ValidationError({"detail": "The list of speakers is closed."}) user = self.request.user else: + point_of_order = False # not for someone else # Add someone else. if not has_perm( self.request.user, "agenda.can_manage_list_of_speakers" @@ -352,7 +360,9 @@ class ListOfSpeakersViewSet( # Try to add the user. This ensurse that a user is not twice in the # list of coming speakers. try: - speaker = Speaker.objects.add(user, list_of_speakers) + speaker = Speaker.objects.add( + user, list_of_speakers, point_of_order=point_of_order + ) except OpenSlidesError as e: raise ValidationError({"detail": str(e)}) @@ -360,7 +370,7 @@ class ListOfSpeakersViewSet( # to see users may not have it but can get it now. inform_changed_data(user, disable_history=True) - # Toggle 'marked' for the speaker + # Set 'marked' for the speaker elif request.method == "PATCH": # Check permissions if not has_perm(self.request.user, "agenda.can_manage_list_of_speakers"): @@ -380,17 +390,12 @@ class ListOfSpeakersViewSet( queryset = Speaker.objects.filter( list_of_speakers=list_of_speakers, user=user, begin_time=None ) - try: - # We assume that there aren't multiple entries for speakers that - # did not yet begin to speak, because this - # is forbidden by the Manager's add method. We assume that - # there is only one speaker instance or none. - speaker = queryset.get() - except Speaker.DoesNotExist: + + if not queryset.exists(): raise ValidationError( {"detail": "The user is not in the list of speakers."} ) - else: + for speaker in queryset.all(): speaker.marked = marked speaker.save() @@ -400,21 +405,29 @@ class ListOfSpeakersViewSet( # Check permissions and other conditions. Get speaker instance. if speaker_ids is None: + point_of_order = request.data.get("point_of_order") or False + if not isinstance(point_of_order, bool): + raise ValidationError( + {"detail": "point_of_order has to be a bool."} + ) # Remove oneself queryset = Speaker.objects.filter( - list_of_speakers=list_of_speakers, user=self.request.user + list_of_speakers=list_of_speakers, + user=self.request.user, + point_of_order=point_of_order, ).exclude(weight=None) - try: - # We assume that there aren't multiple entries because this - # is forbidden by the Manager's add method. We assume that - # there is only one speaker instance or none. - speaker = queryset.get() - except Speaker.DoesNotExist: + + if not queryset.exists(): raise ValidationError( - {"detail": "You are not on the list of speakers."} + {"detail": "The user is not in the list of speakers."} ) - else: - speaker.delete() + # We delete all() from the queryset and do not use get(): + # The Speaker.objects.add method should assert, that there + # is only one speaker. But due to race conditions, sometimes + # there are multiple ones. Using all() ensures, that there is + # no server crash, if this happens. + queryset.all().delete() + inform_changed_data(list_of_speakers) else: # Remove someone else. if not has_perm( @@ -423,7 +436,7 @@ class ListOfSpeakersViewSet( self.permission_denied(request) if isinstance(speaker_ids, int): speaker_ids = [speaker_ids] - deleted_speaker_count = 0 + deleted_some_speakers = False for speaker_id in speaker_ids: try: speaker = Speaker.objects.get(pk=int(speaker_id)) @@ -431,9 +444,9 @@ class ListOfSpeakersViewSet( pass else: speaker.delete(skip_autoupdate=True) - deleted_speaker_count += 1 + deleted_some_speakers = True # send autoupdate if speakers are deleted - if deleted_speaker_count > 0: + if deleted_some_speakers: inform_changed_data(list_of_speakers) return Response() @@ -544,6 +557,16 @@ class ListOfSpeakersViewSet( if not last_speaker: raise ValidationError({"detail": "There is no last speaker at the moment."}) + if last_speaker.point_of_order: + raise ValidationError( + {"detail": "You cannot readd a point of order speaker."} + ) + + if list_of_speakers.speakers.filter( + user=last_speaker.user, begin_time=None + ).exists(): + raise ValidationError({"detail": "The last speaker is already waiting."}) + next_speaker = list_of_speakers.get_next_speaker() new_weight = 1 # if there is a next speaker, insert last speaker before it diff --git a/server/tests/integration/agenda/test_viewset.py b/server/tests/integration/agenda/test_viewset.py index d0f9fc54c..a6c457131 100644 --- a/server/tests/integration/agenda/test_viewset.py +++ b/server/tests/integration/agenda/test_viewset.py @@ -294,6 +294,7 @@ class ManageSpeaker(TestCase): title="test_title_aZaedij4gohn5eeQu8fe" ).list_of_speakers self.user, _ = self.create_user() + self.admin = get_user_model().objects.get(username="admin") def test_add_oneself_once(self): response = self.client.post( @@ -303,9 +304,7 @@ class ManageSpeaker(TestCase): self.assertTrue(Speaker.objects.all().exists()) def test_add_oneself_twice(self): - Speaker.objects.add( - get_user_model().objects.get(username="admin"), self.list_of_speakers - ) + Speaker.objects.add(self.admin, self.list_of_speakers) response = self.client.post( reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]) ) @@ -320,9 +319,19 @@ class ManageSpeaker(TestCase): self.assertEqual(response.status_code, 400) def test_remove_oneself(self): - Speaker.objects.add( - get_user_model().objects.get(username="admin"), self.list_of_speakers + Speaker.objects.add(self.admin, self.list_of_speakers) + response = self.client.delete( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]) ) + self.assertEqual(response.status_code, 200) + self.assertFalse(Speaker.objects.all().exists()) + + def test_remove_oneself_if_twice_on_los(self): + # This one is a test, if there is malformed speaker data, that + # this method still works. + Speaker.objects.add(self.admin, self.list_of_speakers) + s2 = Speaker(user=self.admin, list_of_speakers=self.list_of_speakers, weight=2) + s2.save() response = self.client.delete( reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]) ) @@ -347,6 +356,94 @@ class ManageSpeaker(TestCase): ).exists() ) + def test_point_of_order_single(self): + config["agenda_enable_point_of_order_speakers"] = True + self.assertEqual(Speaker.objects.all().count(), 0) + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(Speaker.objects.all().count(), 1) + self.assertTrue(Speaker.objects.get().point_of_order) + self.assertEqual(Speaker.objects.get().weight, -1) + + def test_point_of_order_not_enabled(self): + self.assertEqual(Speaker.objects.all().count(), 0) + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 400) + self.assertEqual(Speaker.objects.all().count(), 0) + + def test_point_of_order_before_other(self): + config["agenda_enable_point_of_order_speakers"] = True + normal_speaker = Speaker.objects.add(self.user, self.list_of_speakers) + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(Speaker.objects.all().count(), 2) + poo_speaker = Speaker.objects.get(user=self.admin) + self.assertTrue(poo_speaker.point_of_order) + self.assertEqual(poo_speaker.weight, normal_speaker.weight - 1) + + def test_point_of_order_with_normal_speaker(self): + config["agenda_enable_point_of_order_speakers"] = True + Speaker.objects.add(self.admin, self.list_of_speakers) + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(Speaker.objects.all().count(), 2) + self.assertTrue(Speaker.objects.filter(user=self.admin).count(), 2) + + def test_point_of_order_twice(self): + config["agenda_enable_point_of_order_speakers"] = True + Speaker.objects.add(self.admin, self.list_of_speakers, point_of_order=True) + response = self.client.post( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 400) + self.assertEqual(Speaker.objects.all().count(), 1) + + def test_remove_point_of_order(self): + config["agenda_enable_point_of_order_speakers"] = True + Speaker.objects.add(self.admin, self.list_of_speakers, point_of_order=True) + response = self.client.delete( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 200) + self.assertFalse(Speaker.objects.all().exists()) + + def test_remove_point_of_order_with_normal_speaker(self): + config["agenda_enable_point_of_order_speakers"] = True + Speaker.objects.add(self.admin, self.list_of_speakers) + Speaker.objects.add(self.admin, self.list_of_speakers, point_of_order=True) + response = self.client.delete( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), + {"point_of_order": True}, + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(Speaker.objects.all().count(), 1) + self.assertFalse(Speaker.objects.get().point_of_order) + + def test_remove_self_with_point_of_order(self): + config["agenda_enable_point_of_order_speakers"] = True + Speaker.objects.add(self.admin, self.list_of_speakers) + Speaker.objects.add(self.admin, self.list_of_speakers, point_of_order=True) + response = self.client.delete( + reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]) + ) + self.assertEqual(response.status_code, 200) + self.assertEqual(Speaker.objects.all().count(), 1) + self.assertTrue(Speaker.objects.get().point_of_order) + def test_invalid_data_string_instead_of_integer(self): response = self.client.post( reverse("listofspeakers-manage-speaker", args=[self.list_of_speakers.pk]), @@ -447,6 +544,7 @@ class ManageSpeaker(TestCase): speaker.end_time = timezone.now() speaker.weight = None speaker.save() + return speaker def test_readd_last_speaker_no_speaker(self): response = self.client.post( @@ -514,6 +612,29 @@ class ManageSpeaker(TestCase): ) self.assertEqual(response.status_code, 403) + def test_readd_last_speaker_already_waiting(self): + self.util_add_user_as_last_speaker() + Speaker.objects.add(self.user, self.list_of_speakers) + + response = self.client.post( + reverse( + "listofspeakers-readd-last-speaker", args=[self.list_of_speakers.pk] + ) + ) + self.assertEqual(response.status_code, 400) + + def test_readd_last_speaker_point_of_order(self): + speaker = self.util_add_user_as_last_speaker() + speaker.point_of_order = True + speaker.save() + + response = self.client.post( + reverse( + "listofspeakers-readd-last-speaker", args=[self.list_of_speakers.pk] + ) + ) + self.assertEqual(response.status_code, 400) + class Speak(TestCase): """ diff --git a/server/tests/unit/agenda/test_views.py b/server/tests/unit/agenda/test_views.py index ae1e1bc22..df182bdcc 100644 --- a/server/tests/unit/agenda/test_views.py +++ b/server/tests/unit/agenda/test_views.py @@ -32,7 +32,7 @@ class ListOfSpeakersViewSetManageSpeaker(TestCase): self.view_instance.manage_speaker(self.request) mock_speaker.objects.add.assert_called_with( - self.request.user, self.mock_list_of_speakers + self.request.user, self.mock_list_of_speakers, point_of_order=False ) @patch("openslides.agenda.views.inform_changed_data") @@ -56,7 +56,7 @@ class ListOfSpeakersViewSetManageSpeaker(TestCase): MockUser.objects.get.assert_called_with(pk=2) mock_speaker.objects.add.assert_called_with( - mock_user, self.mock_list_of_speakers + mock_user, self.mock_list_of_speakers, point_of_order=False ) @patch("openslides.agenda.views.Speaker") @@ -66,7 +66,7 @@ class ListOfSpeakersViewSetManageSpeaker(TestCase): self.request.data = {} self.view_instance.manage_speaker(self.request) mock_queryset = mock_speaker.objects.filter.return_value.exclude.return_value - mock_queryset.get.return_value.delete.assert_called_with() + mock_queryset.all.return_value.delete.assert_called_with() @patch("openslides.agenda.views.inform_changed_data") @patch("openslides.agenda.views.has_perm")