From 9323bdef206733b87673e498e79ceacd44605e67 Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Mon, 2 Sep 2019 11:09:03 +0200 Subject: [PATCH] Added config for more verbose errors on reset password - Added settings.py docs - Fixed left-overs from #4920 - Reworked all server messages to a new argument formet, so that the client can translate server messages --- README.rst | 11 +- SETTINGS.rst | 122 ++++++++++++++++ .../app/core/core-services/http.service.ts | 37 ++++- openslides/agenda/signals.py | 3 +- openslides/agenda/views.py | 21 ++- openslides/assignments/serializers.py | 11 +- openslides/assignments/views.py | 71 +++++++--- openslides/core/serializers.py | 2 +- openslides/core/views.py | 5 +- openslides/mediafiles/views.py | 4 +- openslides/motions/models.py | 21 ++- openslides/motions/numbering.py | 36 ++--- openslides/motions/serializers.py | 9 +- openslides/motions/views.py | 132 ++++++++++++------ openslides/poll/serializers.py | 2 +- openslides/users/models.py | 12 +- openslides/users/views.py | 38 +++-- openslides/utils/settings.py.tpl | 12 +- openslides/utils/views.py | 15 +- tests/integration/motions/test_viewset.py | 89 +++++++----- 20 files changed, 469 insertions(+), 184 deletions(-) create mode 100644 SETTINGS.rst diff --git a/README.rst b/README.rst index 69312c0e0..ac53ed948 100644 --- a/README.rst +++ b/README.rst @@ -115,8 +115,17 @@ Download the latest portable version of OpenSlides for Windows from install steps. Simply unzip the downloaded file and run ``openslides.exe``. +Configuration +============= + +Please consider reading the `OpenSlides configuration +`_ page to +find out about all configurations, especially when using OpenSlides for big +assemblies. + + Using the Dockerfile -=============================== +==================== You can either pull the image ``openslides/openslides`` or build it yourself (via `docker build -t openslides/openslides .`). You have all prequistes installed diff --git a/SETTINGS.rst b/SETTINGS.rst new file mode 100644 index 000000000..e59c4c857 --- /dev/null +++ b/SETTINGS.rst @@ -0,0 +1,122 @@ +========================== + OpenSlides configuration +========================== + +First, locate your `settings.py`. Since this is a regular python file, +experienced users can also write more advanced configurations with e.g. swithing +between two sets of configs. This also means, that the syntax need to be correct +for OpenSlides to start. + +All presented settings must be written ` = ` to follow the +correct syntax. + +The `settings.py` is just an extension for Django settings. Please visit the +`Django settings documentation +`_ to get an overview about +all existing settings. + +SECURITY +======== + +For `DEBUG` and `SECRET_KEY` see the sections in the django settings +documenataion. + +`RESET_PASSWORD_VERBOSE_ERRORS`: Default: `True`. Set to `False` to disable. +Controls the verbosity on errors during a reset password. If enabled, an error +will be shown, if there does not exist a user with a given email address. So one +can check, if a email is registered. If this is not wanted, disable verbose +messages. An success message will always be shown. + +`AUTH_PASSWORD_VALIDATORS`: Add custom password validators, e.g. a min-length +validator. See `django auth docs +`_ +for mor information. + +Directories +=========== + +`OPENSLIDES_USER_DATA_DIR`: The path, where all user data is saved, like static +files, mediafiles and the default database. This path can be different to the +location of the `settings.py`. + +`STATICFILES_DIRS` and `STATIC_ROOT`: Managing static files. Because the clint +is not delivered by the server anymore, these settings are obsolete. + +`MEDIA_ROOT`: The location of mediafiles. The default is a `media` folder inside +`OPENSLIDES_USER_DATA_DIR`, but can be altered to another path. + +Email +===== + +Please refer to the Django settings documentation. A changed email backend is +useful for debugging to print all email the the console:: + + EMAIL_BACKEND = 'django.core.mail.backends.console.EmailBackend' + + +Logging +======= + +To setup basic logging see `logging +`_. +We recommend to enable all OpenSlides related logging with level `INFO` per +default:: + + LOGGING = { + 'formatters': + 'lessnoise': { + 'format': '[{levelname}] {name} {message}', + 'style': '{', + 'datefmt': '[%Y-%m-%d %H:%M:%S %z]', + }, + }, + 'handlers': { + 'console': { + 'class': 'logging.StreamHandler', + 'formatter': 'lessnoise', + }, + }, + 'loggers': { + 'openslides': { + 'handlers': ['console'], + 'level': os.getenv('OPENSLIDES_LOG_LEVEL', 'INFO'), + }, + }, + } + +With the environment variable `OPENSLIDES_LOG_LEVEL` the level can be adjusted +without changing the `settings.py`. + +Big mode and caching +==================== + +When running multiple workers redis is required as a message broker between the +workers. Set `use_redis = True` to enable redis and visit `OpenSLides in big +mode +`_. + +When seting `use_redis = True`, three settings are important: + +- Caching: `REDIS_ADDRESS` is used to provide caching with redis across all + workers. +- Channels: The "message queue" for the workers. Adjust the `hosts`-part to the + redis address. +- Sessions: All sessions are managed in redis to ensure them across all workers. + Please adjust the `SESSION_REDIS` fields to point to the redis instance. + + +Advanced +======== + +`PING_INTERVAL` and `PING_TIMEOUT` are settings for the clients how frequently +to ping the server (interval) and how big is the timeout. If a ping took longer +than the timeout, the clients does a forced reconnect. + +`COMPRESSION`: Enable or disables the compression when sending data. This does +not affect the client. + +`PRIORITIZED_GROUP_IDS`: A list of group ids. If one client is logged in and the +operator is in one of these groups, the client disconnected and reconnects again. +All requests urls (including websockets) are now prefixed with `/prioritize`, so +these requests from "prioritized clients" can be routed to different servers. + diff --git a/client/src/app/core/core-services/http.service.ts b/client/src/app/core/core-services/http.service.ts index 8b31e59fe..f2770163d 100644 --- a/client/src/app/core/core-services/http.service.ts +++ b/client/src/app/core/core-services/http.service.ts @@ -17,6 +17,20 @@ export enum HTTPMethod { DELETE = 'delete' } +export interface DetailResponse { + detail: string | string[]; + args?: string[]; +} + +function isDetailResponse(obj: any): obj is DetailResponse { + return ( + obj && + typeof obj === 'object' && + (typeof obj.detail === 'string' || obj.detail instanceof Array) && + (!obj.args || obj.args instanceof Array) + ); +} + /** * Service for managing HTTP requests. Allows to send data for every method. Also (TODO) will do generic error handling. */ @@ -128,13 +142,13 @@ export class HttpService { } else if (!e.error) { error += this.translate.instant("The server didn't respond."); } else if (typeof e.error === 'object') { - if (e.error.detail) { - error += this.translate.instant(this.processErrorTexts(e.error.detail)); + if (isDetailResponse(e.error)) { + error += this.processDetailResponse(e.error); } else { error = Object.keys(e.error) .map(key => { const capitalizedKey = key.charAt(0).toUpperCase() + key.slice(1); - return this.translate.instant(capitalizedKey) + ': ' + this.processErrorTexts(e.error[key]); + return this.translate.instant(capitalizedKey) + ': ' + this.processDetailResponse(e.error[key]); }) .join(', '); } @@ -155,12 +169,21 @@ export class HttpService { * @param str a string or a string array to join together. * @returns Error text(s) as single string */ - private processErrorTexts(str: string | string[]): string { - if (str instanceof Array) { - return str.join(' '); + private processDetailResponse(response: DetailResponse): string { + let message: string; + if (response.detail instanceof Array) { + message = response.detail.join(' '); } else { - return str; + message = response.detail; } + message = this.translate.instant(message); + + if (response.args && response.args.length > 0) { + for (let i = 0; i < response.args.length; i++) { + message = message.replace(`{${i}}`, response.args[i].toString()); + } + } + return message; } /** diff --git a/openslides/agenda/signals.py b/openslides/agenda/signals.py index 20dee81fa..59b8d21d8 100644 --- a/openslides/agenda/signals.py +++ b/openslides/agenda/signals.py @@ -64,7 +64,8 @@ def listen_to_related_object_post_save(sender, instance, created, **kwargs): except Item.DoesNotExist: raise ValidationError( { - "detail": f"The parent item with id {parent_id} does not exist" + "detail": "The parent item with id {0} does not exist", + "args": [parent_id], } ) attrs["weight"] = parent.weight + 1 diff --git a/openslides/agenda/views.py b/openslides/agenda/views.py index 873f5c32b..efc6618be 100644 --- a/openslides/agenda/views.py +++ b/openslides/agenda/views.py @@ -86,7 +86,7 @@ class ItemViewSet(ModelViewSet, TreeSortMixin): try: model = get_model_from_collection_string(collection) except ValueError: - raise ValidationError("Invalid collection") + raise ValidationError({"detail": "Invalid collection"}) try: content_object = model.objects.get(pk=id) @@ -220,7 +220,10 @@ class ItemViewSet(ModelViewSet, TreeSortMixin): parent = Item.objects.get(pk=request.data["parent_id"]) except Item.DoesNotExist: raise ValidationError( - {"detail": f"Parent item {request.data['parent_id']} does not exist"} + { + "detail": "Parent item {0} does not exist", + "args": [request.data["parent_id"]], + } ) # Collect ancestors @@ -237,7 +240,8 @@ class ItemViewSet(ModelViewSet, TreeSortMixin): if item_id in ancestors: raise ValidationError( { - "detail": f"Assigning item {item_id} to one of its children is not possible." + "detail": "Assigning item {0} to one of its children is not possible.", + "args": [item_id], } ) @@ -245,7 +249,9 @@ class ItemViewSet(ModelViewSet, TreeSortMixin): try: items.append(Item.objects.get(pk=item_id)) except Item.DoesNotExist: - raise ValidationError({"detail": f"Item {item_id} does not exist"}) + raise ValidationError( + {"detail": "Item {0} does not exist", "args": [item_id]} + ) # OK, assign new parents. for item in items: @@ -257,7 +263,9 @@ class ItemViewSet(ModelViewSet, TreeSortMixin): inform_changed_data(items) # Send response. - return Response({"detail": f"{len(items)} items successfully assigned."}) + return Response( + {"detail": "{0} items successfully assigned.", "args": [len(items)]} + ) class ListOfSpeakersViewSet( @@ -464,7 +472,8 @@ class ListOfSpeakersViewSet( except Speaker.DoesNotExist: raise ValidationError( { - "detail": f"There is no one speaking at the moment according to {list_of_speakers}." + "detail": "There is no one speaking at the moment according to {0}.", + "args": [list_of_speakers], } ) current_speaker.end_speech() diff --git a/openslides/assignments/serializers.py b/openslides/assignments/serializers.py index 126d80776..7c061d143 100644 --- a/openslides/assignments/serializers.py +++ b/openslides/assignments/serializers.py @@ -147,20 +147,25 @@ class AssignmentAllPollSerializer(ModelSerializer): if len(votes) != len(options): raise ValidationError( { - "detail": f"You have to submit data for {len(options)} candidates." + "detail": "You have to submit data for {0} candidates.", + "args": [len(options)], } ) for index, option in enumerate(options): if len(votes[index]) != len(instance.get_vote_values()): raise ValidationError( { - "detail": f"You have to submit data for {len(instance.get_vote_values())} vote values." + "detail": "You have to submit data for {0} vote values", + "args": [len(instance.get_vote_values())], } ) for vote_value, __ in votes[index].items(): if vote_value not in instance.get_vote_values(): raise ValidationError( - {"detail": f"Vote value {vote_value} is invalid."} + { + "detail": "Vote value {0} is invalid.", + "args": [vote_value], + } ) instance.set_vote_objects_with_values( option, votes[index], skip_autoupdate=True diff --git a/openslides/assignments/views.py b/openslides/assignments/views.py index 5984d9b59..d0ac09214 100644 --- a/openslides/assignments/views.py +++ b/openslides/assignments/views.py @@ -131,8 +131,12 @@ class AssignmentViewSet(ModelViewSet): self.mark_elected can play with it. """ if not isinstance(request.data, dict): - detail = f"Invalid data. Expected dictionary, got {type(request.data)}." - raise ValidationError({"detail": detail}) + raise ValidationError( + { + "detail": "Invalid data. Expected dictionary, got {0}.", + "args": [type(request.data)], + } + ) user_str = request.data.get("user", "") try: user_pk = int(user_str) @@ -144,7 +148,7 @@ class AssignmentViewSet(ModelViewSet): user = get_user_model().objects.get(pk=user_pk) except get_user_model().DoesNotExist: raise ValidationError( - {"detail": f"Invalid data. User {user_pk} does not exist."} + {"detail": "Invalid data. User {0} does not exist.", "args": [user_pk]} ) return user @@ -157,46 +161,60 @@ class AssignmentViewSet(ModelViewSet): user = self.get_user_from_request_data(request) assignment = self.get_object() if request.method == "POST": - message = self.nominate_other(request, user, assignment) + return self.nominate_other(request, user, assignment) else: # request.method == 'DELETE' - message = self.delete_other(request, user, assignment) - return Response({"detail": message}) + return self.delete_other(request, user, assignment) def nominate_other(self, request, user, assignment): if assignment.is_elected(user): - raise ValidationError({"detail": f"User {user} is already elected."}) - if assignment.phase == assignment.PHASE_FINISHED: - detail = ( - "You can not nominate someone to this election because it is finished." + raise ValidationError( + {"detail": "User {0} is already elected.", "args": [str(user)]} + ) + if assignment.phase == assignment.PHASE_FINISHED: + raise ValidationError( + { + "detail": "You can not nominate someone to this election because it is finished." + } ) - raise ValidationError({"detail": detail}) if assignment.phase == assignment.PHASE_VOTING and not has_perm( request.user, "assignments.can_manage" ): # To nominate another user during voting you have to be a manager. self.permission_denied(request) if assignment.is_candidate(user): - raise ValidationError({"detail": f"User {user} is already nominated."}) + raise ValidationError( + {"detail": "User {0} is already nominated.", "args": [str(user)]} + ) assignment.set_candidate(user) # Send new candidate via autoupdate because users without permission # to see users may not have it but can get it now. inform_changed_data(user) - return f"User {user} was nominated successfully." + return Response( + {"detail": "User {0} was nominated successfully.", "args": [str(user)]} + ) def delete_other(self, request, user, assignment): # To delete candidature status you have to be a manager. if not has_perm(request.user, "assignments.can_manage"): self.permission_denied(request) if assignment.phase == assignment.PHASE_FINISHED: - detail = "You can not delete someone's candidature to this election because it is finished." - raise ValidationError({"detail": detail}) + raise ValidationError( + { + "detail": "You can not delete someone's candidature to this election because it is finished." + } + ) if not assignment.is_candidate(user) and not assignment.is_elected(user): raise ValidationError( - {"detail": f"User {user} has no status in this election."} + { + "detail": "User {0} has no status in this election.", + "args": [str(user)], + } ) assignment.delete_related_user(user) - return f"Candidate {user} was withdrawn successfully." + return Response( + {"detail": "Candidate {0} was withdrawn successfully.", "args": [str(user)]} + ) @detail_route(methods=["post", "delete"]) def mark_elected(self, request, pk=None): @@ -209,18 +227,25 @@ class AssignmentViewSet(ModelViewSet): if request.method == "POST": if not assignment.is_candidate(user): raise ValidationError( - {"detail": f"User {user} is not a candidate of this election."} + { + "detail": "User {0} is not a candidate of this election.", + "args": [str(user)], + } ) assignment.set_elected(user) - message = f"User {user} was successfully elected." + message = "User {0} was successfully elected." else: # request.method == 'DELETE' if not assignment.is_elected(user): - detail = f"User {user} is not an elected candidate of this election." - raise ValidationError({"detail": detail}) + raise ValidationError( + { + "detail": "User {0} is not an elected candidate of this election.", + "args": [str(user)], + } + ) assignment.set_candidate(user) - message = f"User {user} was successfully unelected." - return Response({"detail": message}) + message = "User {0} was successfully unelected." + return Response({"detail": message, "args": [str(user)]}) @detail_route(methods=["post"]) def create_poll(self, request, pk=None): diff --git a/openslides/core/serializers.py b/openslides/core/serializers.py index 302124d8d..619d8a390 100644 --- a/openslides/core/serializers.py +++ b/openslides/core/serializers.py @@ -62,7 +62,7 @@ def elements_validator(value: Any) -> None: ) if element["name"] not in projector_slides: raise ValidationError( - {"detail": f"Unknown projector element {element['name']},"} + {"detail": "Unknown projector element {0}.", "args": [element["name"]]} ) diff --git a/openslides/core/views.py b/openslides/core/views.py index 620366d3d..274ce87d5 100644 --- a/openslides/core/views.py +++ b/openslides/core/views.py @@ -291,8 +291,9 @@ class ProjectorViewSet(ModelViewSet): projector_instance.scroll = request.data projector_instance.save() - message = f"Setting scroll to {request.data} was successful." - return Response({"detail": message}) + return Response( + {"detail": "Setting scroll to {0} was successful.", "args": [request.data]} + ) class ProjectionDefaultViewSet(ListModelMixin, RetrieveModelMixin, GenericViewSet): diff --git a/openslides/mediafiles/views.py b/openslides/mediafiles/views.py index 414050e5f..d0c98b958 100644 --- a/openslides/mediafiles/views.py +++ b/openslides/mediafiles/views.py @@ -126,7 +126,7 @@ class MediafileViewSet(ModelViewSet): mediafiles.append(Mediafile.objects.get(pk=id)) except Mediafile.DoesNotExist: raise ValidationError( - {"detail": f"The mediafile with id {id} does not exist"} + {"detail": "The mediafile with id {0} does not exist", "args": [id]} ) # Search for valid parents (None is not included, but also safe!) @@ -181,7 +181,7 @@ class MediafileViewSet(ModelViewSet): mediafiles.append(Mediafile.objects.get(pk=id)) except Mediafile.DoesNotExist: raise ValidationError( - {"detail": f"The mediafile with id {id} does not exist"} + {"detail": "The mediafile with id {0} does not exist", "args": [id]} ) if not mediafiles: return Response() diff --git a/openslides/motions/models.py b/openslides/motions/models.py index c93c91831..e2e03fe65 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -1,6 +1,5 @@ from django.conf import settings from django.contrib.auth.models import AnonymousUser -from django.core.exceptions import ImproperlyConfigured, ValidationError from django.db import IntegrityError, models, transaction from django.db.models import Max from jsonfield import JSONField @@ -18,6 +17,7 @@ from openslides.poll.models import ( from openslides.utils.autoupdate import inform_changed_data from openslides.utils.exceptions import OpenSlidesError from openslides.utils.models import RESTModelMixin +from openslides.utils.rest_api import ValidationError from ..utils.models import CASCADE_AND_AUTOUPDATE, SET_NULL_AND_AUTOUPDATE from .access_permissions import ( @@ -401,17 +401,9 @@ class Motion(RESTModelMixin, AgendaItemWithListOfSpeakersMixin, models.Model): Returns the number used in the set_identifier method with leading zero charaters according to the config value. """ - result = str(number) - if config["motions_identifier_min_digits"]: - if not isinstance(config["motions_identifier_min_digits"], int): - raise ImproperlyConfigured( - "Config value 'motions_identifier_min_digits' must be an integer." - ) - result = ( - "0" * (config["motions_identifier_min_digits"] - len(str(number))) - + result - ) - return result + return "0" * (config["motions_identifier_min_digits"] - len(str(number))) + str( + number + ) def is_submitter(self, user): """ @@ -766,7 +758,10 @@ class MotionChangeRecommendation(RESTModelMixin, models.Model): if self.collides_with_other_recommendation(recommendations): raise ValidationError( - f"The recommendation collides with an existing one (line {self.line_from} - {self.line_to})." + { + "detail": "The recommendation collides with an existing one (line {0} - {1}).", + "args": [self.line_from, self.line_to], + } ) result = super().save(*args, **kwargs) diff --git a/openslides/motions/numbering.py b/openslides/motions/numbering.py index a0966bd5a..960609dfc 100644 --- a/openslides/motions/numbering.py +++ b/openslides/motions/numbering.py @@ -1,7 +1,6 @@ from collections import defaultdict from typing import Any, Dict, List, Tuple -from django.conf import settings from django.db import transaction from django.db.models import Model @@ -41,11 +40,8 @@ def numbering(main_category: Category) -> List[Model]: - Both counters may be filled with leading zeros according to `Motion.extend_identifier_number` - On errors, ValidationErrors with appropriate content will be raised. """ - # If MOTION_IDENTIFIER_WITHOUT_BLANKS is set, don't use blanks when building identifier. - without_blank = ( - hasattr(settings, "MOTION_IDENTIFIER_WITHOUT_BLANKS") - and settings.MOTION_IDENTIFIER_WITHOUT_BLANKS - ) + # If the config is false, don't use blanks when building identifier. + without_blank = not config["motions_identifier_with_blank"] # Get child categories (to build affected categories) and precalculate all prefixes. child_categories = get_child_categories(main_category) @@ -171,9 +167,10 @@ def get_amendment_level_mapping( if motion.parent_id is not None and motion.parent_id not in affected_motion_ids: raise ValidationError( { - "detail": f'Amendment "{motion}" cannot be numbered, because ' - f"it's lead motion ({motion.parent}) is not in category " - f"{main_category} or any subcategory." + "detail": 'Amendment "{0}" cannot be numbered, because ' + "it's lead motion ({1}) is not in category " + "{2} or any subcategory.", + "args": [str(motion), str(motion.parent), str(main_category)], } ) return max_amendment_level, amendment_level_mapping @@ -234,17 +231,22 @@ def check_new_identifiers_for_conflicts( # We do have a conflict. Build a nice error message. conflicting_motion = conflicting_motions.first() if conflicting_motion.category: - error_message = ( - "Numbering aborted because the motion identifier " - f'"{conflicting_motion.identifier}" already exists in ' - f"category {conflicting_motion.category}." + raise ValidationError( + { + "detail": 'Numbering aborted because the motion identifier "{0}" already exists in category {1}.', + "args": [ + conflicting_motion.identifier, + str(conflicting_motion.category), + ], + } ) else: - error_message = ( - "Numbering aborted because the motion identifier " - f'"{conflicting_motion.identifier}" already exists.' + raise ValidationError( + { + "detail": 'Numbering aborted because the motion identifier "{0}" already exists.', + "args": [conflicting_motion.identifier], + } ) - raise ValidationError({"detail": error_message}) def update_identifiers(affected_motions, new_identifier_mapping) -> List[Model]: diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index ab3076eac..de68e0eb4 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -41,7 +41,9 @@ def validate_workflow_field(value): Validator to ensure that the workflow with the given id exists. """ if not Workflow.objects.filter(pk=value).exists(): - raise ValidationError({"detail": f"Workflow {value} does not exist."}) + raise ValidationError( + {"detail": "Workflow {0} does not exist.", "args": [value]} + ) class StatuteParagraphSerializer(ModelSerializer): @@ -307,13 +309,14 @@ class MotionPollSerializer(ModelSerializer): if len(votes) != len(instance.get_vote_values()): raise ValidationError( { - "detail": f"You have to submit data for {len(instance.get_vote_values())} vote values." + "detail": "You have to submit data for {0} vote values.", + "args": [len(instance.get_vote_values())], } ) for vote_value in votes.keys(): if vote_value not in instance.get_vote_values(): raise ValidationError( - {"detail": f"Vote value {vote_value} is invalid."} + {"detail": "Vote value {0} is invalid.", "args": [vote_value]} ) instance.set_vote_objects_with_values( instance.get_options().get(), votes, skip_autoupdate=True diff --git a/openslides/motions/views.py b/openslides/motions/views.py index 2632b0b1b..80da4f09d 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -2,7 +2,6 @@ from typing import List, Set import jsonschema from django.contrib.auth import get_user_model -from django.core.exceptions import ValidationError as DjangoValidationError from django.db import transaction from django.db.models import Case, When from django.db.models.deletion import ProtectedError @@ -358,7 +357,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): section = MotionCommentSection.objects.get(pk=section_id) except MotionCommentSection.DoesNotExist: raise ValidationError( - {"detail": f"A comment section with id {section_id} does not exist."} + { + "detail": "A comment section with id {0} does not exist.", + "args": [section_id], + } ) # the request user needs to see and write to the comment section @@ -448,7 +450,9 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Remove all submitters. Submitter.objects.filter(motion=motion).delete() @@ -459,7 +463,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): submitter = get_user_model().objects.get(pk=submitter_id) except get_user_model().DoesNotExist: raise ValidationError( - {"detail": f"Submitter {submitter_id} does not exist"} + { + "detail": "Submitter {0} does not exist", + "args": [submitter_id], + } ) Submitter.objects.add(submitter, motion) new_submitters.append(submitter) @@ -479,7 +486,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( - {"detail": f"{len(motion_result)} motions successfully updated."} + { + "detail": "{0} motions successfully updated.", + "args": [len(motion_result)], + } ) @detail_route(methods=["post", "delete"]) @@ -566,7 +576,9 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Get category category = None @@ -575,7 +587,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): category = Category.objects.get(pk=item["category"]) except Category.DoesNotExist: raise ValidationError( - {"detail": f"Category {item['category']} does not exist"} + { + "detail": "Category {0} does not exist", + "args": [item["category"]], + } ) # Set category @@ -601,7 +616,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( - {"detail": f"Category of {len(motion_result)} motions successfully set."} + { + "detail": "Category of {0} motions successfully set.", + "args": [len(motion_result)], + } ) @list_route(methods=["post"]) @@ -645,7 +663,9 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Get motion block motion_block = None @@ -654,7 +674,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): motion_block = MotionBlock.objects.get(pk=item["motion_block"]) except MotionBlock.DoesNotExist: raise ValidationError( - {"detail": f"MotionBlock {item['motion_block']} does not exist"} + { + "detail": "MotionBlock {0} does not exist", + "args": [item["motion_block"]], + } ) # Set motion bock @@ -681,7 +704,8 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( { - "detail": f"Motion block of {len(motion_result)} motions successfully set." + "detail": "Motion block of {0} motions successfully set.", + "args": [len(motion_result)], } ) @@ -714,7 +738,7 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): ) if not motion.state.is_next_or_previous_state_id(state_id): raise ValidationError( - {"detail": f"You can not set the state to {state_id}."} + {"detail": "You can not set the state to {0}.", "args": [state_id]} ) motion.set_state(state_id) else: @@ -786,7 +810,9 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Set or reset state. state_id = item["state"] @@ -794,7 +820,7 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): if state_id not in [item.id for item in valid_states]: # States of different workflows are not allowed. raise ValidationError( - {"detail": f"You can not set the state to {state_id}."} + {"detail": "You can not set the state to {0}.", "args": [state_id]} ) motion.set_state(state_id) @@ -826,7 +852,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( - {"detail": f"State of {len(motion_result)} motions successfully set."} + { + "detail": "State of {0} motions successfully set.", + "args": [len(motion_result)], + } ) @detail_route(methods=["put"]) @@ -858,7 +887,8 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): ]: raise ValidationError( { - "detail": f"You can not set the recommendation to {recommendation_state_id}." + "detail": "You can not set the recommendation to {0}.", + "args": [recommendation_state_id], } ) motion.set_recommendation(recommendation_state_id) @@ -875,7 +905,6 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): if motion.recommendation else "None" ) - message = f"The recommendation of the motion was set to {label}." # Fire autoupdate again to save information to OpenSlides history. inform_changed_data( @@ -884,7 +913,12 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): user_id=request.user.pk, ) - return Response({"detail": message}) + return Response( + { + "detail": "The recommendation of the motion was set to {0}.", + "args": [label], + } + ) @list_route(methods=["post"]) @transaction.atomic @@ -927,7 +961,9 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Set or reset recommendation. recommendation_state_id = item["recommendation"] @@ -944,7 +980,8 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): ]: raise ValidationError( { - "detail": "You can not set the recommendation to {recommendation_state_id}." + "detail": "You can not set the recommendation to {0}.", + "args": [recommendation_state_id], } ) motion.set_recommendation(recommendation_state_id) @@ -971,7 +1008,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( - {"detail": f"{len(motion_result)} motions successfully updated."} + { + "detail": "{0} motions successfully updated.", + "args": [len(motion_result)], + } ) @detail_route(methods=["post"]) @@ -1070,12 +1110,16 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): try: motion = Motion.objects.get(pk=item["id"]) except Motion.DoesNotExist: - raise ValidationError({"detail": f"Motion {item['id']} does not exist"}) + raise ValidationError( + {"detail": "Motion {0} does not exist", "args": [item["id"]]} + ) # Set new tags for tag_id in item["tags"]: if not Tag.objects.filter(pk=tag_id).exists(): - raise ValidationError({"detail": f"Tag {tag_id} does not exist"}) + raise ValidationError( + {"detail": "Tag {0} does not exist", "args": [tag_id]} + ) motion.tags.set(item["tags"]) # Finish motion. @@ -1086,7 +1130,10 @@ class MotionViewSet(TreeSortMixin, ModelViewSet): # Send response. return Response( - {"detail": f"{len(motion_result)} motions successfully updated."} + { + "detail": "{0} motions successfully updated.", + "args": [len(motion_result)], + } ) @@ -1164,15 +1211,6 @@ class MotionChangeRecommendationViewSet(ModelViewSet): result = False return result - def create(self, request, *args, **kwargs): - """ - Creating a Change Recommendation, custom exception handling - """ - try: - return super().create(request, *args, **kwargs) - except DjangoValidationError as err: - return Response({"detail": err.message}, status=400) - def perform_create(self, serializer): """ Customized method to add history information. @@ -1253,12 +1291,12 @@ class MotionCommentSectionViewSet(ModelViewSet): motions_verbose += ", ..." if count == 1: - msg = f"This section has still comments in motion {motions_verbose}." + msg = "This section has still comments in motion {0}." else: - msg = f"This section has still comments in motions {motions_verbose}." + msg = "This section has still comments in motions {0}." msg += " " + "Please remove all comments before deletion." - raise ValidationError({"detail": msg}) + raise ValidationError({"detail": msg, "args": [motions_verbose]}) return result def update(self, *args, **kwargs): @@ -1440,7 +1478,8 @@ class CategoryViewSet(TreeSortMixin, ModelViewSet): ) return Response( { - "detail": f"All motions in category {main_category} numbered successfully." + "detail": "All motions in category {0} numbered successfully.", + "args": [str(main_category)], } ) @@ -1503,7 +1542,7 @@ class MotionBlockViewSet(ModelViewSet): class ProtectedErrorMessageMixin: - def getProtectedErrorMessage(self, name, error): + def raiseProtectedError(self, name, error): # The protected objects can just be motions.. motions = ['"' + str(m) + '"' for m in error.protected_objects.all()] count = len(motions) @@ -1512,10 +1551,15 @@ class ProtectedErrorMessageMixin: motions_verbose += ", ..." if count == 1: - msg = f"This {name} is assigned to motion {motions_verbose}." + msg = f"This {0} is assigned to motion {1}." else: - msg = f"This {name} is assigned to motions {motions_verbose}." - return f"{msg} Please remove all assignments before deletion." + msg = f"This {0} is assigned to motions {1}." + raise ValidationError( + { + "detail": f"{msg} Please remove all assignments before deletion.", + "args": [name, motions_verbose], + } + ) class WorkflowViewSet(ModelViewSet, ProtectedErrorMessageMixin): @@ -1555,8 +1599,7 @@ class WorkflowViewSet(ModelViewSet, ProtectedErrorMessageMixin): try: result = super().destroy(*args, **kwargs) except ProtectedError as err: - msg = self.getProtectedErrorMessage("workflow", err) - raise ValidationError({"detail": msg}) + self.raiseProtectedError("workflow", err) # Change motion default workflows in the config if int(config["motions_workflow"]) == workflow_pk: @@ -1608,8 +1651,7 @@ class StateViewSet(ModelViewSet, ProtectedErrorMessageMixin): try: result = super().destroy(*args, **kwargs) except ProtectedError as err: - msg = self.getProtectedErrorMessage("workflow", err) - raise ValidationError({"detail": msg}) + self.raiseProtectedError("workflow", err) inform_changed_data(workflow) return result diff --git a/openslides/poll/serializers.py b/openslides/poll/serializers.py index 48c56ea9c..72b7f132b 100644 --- a/openslides/poll/serializers.py +++ b/openslides/poll/serializers.py @@ -14,6 +14,6 @@ def default_votes_validator(data): and data[key] < -2 ): raise ValidationError( - {"detail": f"Value for {key} must not be less than -2"} + {"detail": "Value for {0} must not be less than -2", "args": [key]} ) return data diff --git a/openslides/users/models.py b/openslides/users/models.py index 4f8833ae7..abd101619 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -227,7 +227,7 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): try: message = message.format(**message_format) except KeyError as err: - raise ValidationError({"detail": f"Invalid property {err}."}) + raise ValidationError({"detail": "Invalid property {0}", "args": [err]}) subject_format = format_dict( {"event_name": config["general_event_name"], "username": self.username} @@ -235,7 +235,7 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): try: subject = subject.format(**subject_format) except KeyError as err: - raise ValidationError({"detail": f"Invalid property {err}."}) + raise ValidationError({"detail": "Invalid property {0}", "args": [err]}) # Create an email and send it. email = mail.EmailMessage( @@ -255,7 +255,10 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): helptext = " Is the email sender correct?" connection.close() raise ValidationError( - {"detail": f"Error {error}. Cannot send email.{helptext}"} + { + "detail": "Error {0}. Cannot send email.{1}", + "args": [error, helptext], + } ) except smtplib.SMTPRecipientsRefused: pass # Run into returning false later @@ -263,7 +266,8 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): # Nice error message on auth failure raise ValidationError( { - "detail": f"Error {e.smtp_code}: Authentication failure. Please contact your local administrator." + "detail": "Error {0}: Authentication failure. Please contact your local administrator.", + "args": [e.smtp_code], } ) else: diff --git a/openslides/users/views.py b/openslides/users/views.py index d1290d625..78e75340b 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -202,7 +202,8 @@ class UserViewSet(ModelViewSet): errors = " ".join(errors) raise ValidationError( { - "detail": f'The default password of user "{user.username}" is not valid: {errors}' + "detail": 'The default password of user "{0}" is not valid: {1}', + "args": [user.username, str(errors)], } ) @@ -331,7 +332,8 @@ class UserViewSet(ModelViewSet): inform_changed_data(created_users) return Response( { - "detail": f"{len(created_users)} users successfully imported.", + "detail": "{0} users successfully imported.", + "args": [len(created_users)], "importedTrackIds": imported_track_ids, } ) @@ -362,7 +364,8 @@ class UserViewSet(ModelViewSet): except ConnectionRefusedError: raise ValidationError( { - "detail": f"Cannot connect to SMTP server on {settings.EMAIL_HOST}:{settings.EMAIL_PORT}" + "detail": "Cannot connect to SMTP server on {0}:{1}", + "args": [settings.EMAIL_HOST, settings.EMAIL_PORT], } ) except smtplib.SMTPException as err: @@ -391,7 +394,7 @@ class UserViewSet(ModelViewSet): def assert_list_of_ints(self, ids, ids_name="user_ids"): """ Asserts, that ids is a list of ints. Raises a ValidationError, if not. """ if not isinstance(ids, list): - raise ValidationError({"detail": f"{ids_name} must be a list"}) + raise ValidationError({"detail": "{0} must be a list", "args": [ids_name]}) for id in ids: if not isinstance(id, int): raise ValidationError({"detail": "Every id must be a int"}) @@ -546,7 +549,10 @@ class GroupViewSet(ModelViewSet): inform_changed_data(group) return Response( - {"detail": f"Permissions of group {group.pk} successfully changed."} + { + "detail": "Permissions of group {0} successfully changed.", + "args": [group.pk], + } ) def inform_permission_change( @@ -627,7 +633,8 @@ class PersonalNoteViewSet(ModelViewSet): except IntegrityError: raise ValidationError( { - "detail": f"The personal note for user {self.request.user.id} does already exist" + "detail": "The personal note for user {0} does already exist", + "args": [self.request.user.id], } ) @@ -812,7 +819,14 @@ class PasswordResetView(APIView): Loop over all users and send emails. """ to_email = request.data.get("email") - for user in self.get_users(to_email): + users = self.get_users(to_email) + + if len(users) == 0 and getattr(settings, "RESET_PASSWORD_VERBOSE_ERRORS", True): + raise ValidationError( + {"detail": "No users with email {0} found.", "args": [to_email]} + ) + + for user in users: current_site = get_current_site(request) site_name = current_site.name if has_perm(user, "users.can_change_password") or has_perm( @@ -850,14 +864,16 @@ class PasswordResetView(APIView): except smtplib.SMTPRecipientsRefused: raise ValidationError( { - "detail": f"Error: The email to {to_email} was refused by the server. Please contact your local administrator." + "detail": "Error: The email to {0} was refused by the server. Please contact your local administrator.", + "args": [to_email], } ) except smtplib.SMTPAuthenticationError as e: # Nice error message on auth failure raise ValidationError( { - "detail": f"Error {e.smtp_code}: Authentication failure. Please contact your administrator." + "detail": "Error {0}: Authentication failure. Please contact your administrator.", + "args": [e.smtp_code], } ) except ConnectionRefusedError: @@ -866,7 +882,7 @@ class PasswordResetView(APIView): "detail": "Connection refused error. Please contact your administrator." } ) - return super().post(request, *args, **kwargs) + return Response() def get_users(self, email): """Given an email, return matching user(s) who should receive a reset. @@ -878,7 +894,7 @@ class PasswordResetView(APIView): active_users = User.objects.filter( **{"email__iexact": email, "is_active": True} ) - return (u for u in active_users if u.has_usable_password()) + return [u for u in active_users if u.has_usable_password()] def get_email_body(self, **context): """ diff --git a/openslides/utils/settings.py.tpl b/openslides/utils/settings.py.tpl index 7d82c5ff5..28b11fb74 100644 --- a/openslides/utils/settings.py.tpl +++ b/openslides/utils/settings.py.tpl @@ -2,10 +2,7 @@ Settings file for OpenSlides. For more information on this file, see -https://docs.djangoproject.com/en/1.10/topics/settings/ - -For the full list of settings and their values, see -https://docs.djangoproject.com/en/1.10/ref/settings/ +https://github.com/OpenSlides/OpenSlides/blob/master/SETTINGS.rst """ import os @@ -40,6 +37,13 @@ SECRET_KEY = %(secret_key)r DEBUG = %(debug)s +# Controls the verbosity on errors during a reset password. If enabled, an error +# will be shown, if there does not exist a user with a given email address. So one +# can check, if a email is registered. If this is not wanted, disable verbose +# messages. An success message will always be shown. + +RESET_PASSWORD_VERBOSE_ERRORS = True + # Email settings # For SSL/TLS specific settings see https://docs.djangoproject.com/en/1.11/topics/email/#smtp-backend diff --git a/openslides/utils/views.py b/openslides/utils/views.py index 1b0880c97..015a1b492 100644 --- a/openslides/utils/views.py +++ b/openslides/utils/views.py @@ -59,7 +59,7 @@ class TreeSortMixin: does not have every model, the remaining models are sorted correctly. """ if not isinstance(request.data, list): - raise ValidationError("The data must be a list.") + raise ValidationError({"detail": "The data must be a list."}) # get all item ids to verify, that the user send all ids. all_model_ids = set(model.objects.all().values_list("pk", flat=True)) @@ -91,9 +91,11 @@ class TreeSortMixin: node[weight_key] = weight weight += 2 if id in ids_found: - raise ValidationError(f"Duplicate id: {id}") + raise ValidationError({"detail": "Duplicate id: {0}", "args": [id]}) if id not in all_model_ids: - raise ValidationError(f"Id does not exist: {id}") + raise ValidationError( + {"detail": "Id does not exist: {0}", "args": [id]} + ) ids_found.add(id) # Add children, if exist. @@ -105,14 +107,17 @@ class TreeSortMixin: child.get("id"), int ): raise ValidationError( - "child must be a dict with an id as integer" + {"detail": "child must be a dict with an id as integer"} ) child[parent_id_key] = id nodes_to_check.append(child) if len(all_model_ids) != len(ids_found): raise ValidationError( - f"Did not recieved {len(all_model_ids)} ids, got {len(ids_found)}." + { + "detail": "Did not recieved {0} ids, got {1}.", + "args": [len(all_model_ids), len(ids_found)], + } ) # Do the actual update: diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 8cfbd5b1c..d63d9cbac 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -927,8 +927,9 @@ class ManageComments(TestCase): ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( - response.data["detail"], "A comment section with id 42 does not exist." + response.data["detail"], "A comment section with id {0} does not exist." ) + self.assertEqual(response.data["args"][0], "42") def test_create_comment(self): response = self.client.post( @@ -1309,7 +1310,7 @@ class TestMotionCommentSection(TestCase): response = self.client.delete( reverse("motioncommentsection-detail", args=[section.pk]) ) - self.assertTrue("test_title_SlqfMw(waso0saWMPqcZ" in response.data["detail"]) + self.assertEqual(response.data["args"][0], '"test_title_SlqfMw(waso0saWMPqcZ"') self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual(MotionCommentSection.objects.count(), 1) @@ -1510,12 +1511,8 @@ class CreateMotionChangeRecommendation(TestCase): }, ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertEqual( - response.data, - { - "detail": "The recommendation collides with an existing one (line 3 - 6)." - }, - ) + self.assertEqual(response.data["args"][0], "3") + self.assertEqual(response.data["args"][1], "6") def test_no_collission_different_motions(self): """ @@ -1635,7 +1632,10 @@ class SetState(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.data, - {"detail": "You can not set the state to %d." % invalid_state_id}, + { + "detail": "You can not set the state to {0}.", + "args": [str(invalid_state_id)], + }, ) def test_reset(self): @@ -1672,7 +1672,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( response.data, - {"detail": "The recommendation of the motion was set to Acceptance."}, + { + "detail": "The recommendation of the motion was set to {0}.", + "args": ["Acceptance"], + }, ) self.assertEqual( Motion.objects.get(pk=self.motion.pk).recommendation.name, "accepted" @@ -1699,7 +1702,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.data, - {"detail": "You can not set the recommendation to %d." % invalid_state_id}, + { + "detail": "You can not set the recommendation to {0}.", + "args": [str(invalid_state_id)], + }, ) def test_set_invalid_recommendation(self): @@ -1712,7 +1718,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.data, - {"detail": "You can not set the recommendation to %d." % invalid_state_id}, + { + "detail": "You can not set the recommendation to {0}.", + "args": [str(invalid_state_id)], + }, ) def test_set_invalid_recommendation_2(self): @@ -1727,7 +1736,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) self.assertEqual( response.data, - {"detail": "You can not set the recommendation to %d." % invalid_state_id}, + { + "detail": "You can not set the recommendation to {0}.", + "args": [str(invalid_state_id)], + }, ) def test_reset(self): @@ -1739,7 +1751,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( response.data, - {"detail": "The recommendation of the motion was set to None."}, + { + "detail": "The recommendation of the motion was set to {0}.", + "args": ["None"], + }, ) self.assertTrue(Motion.objects.get(pk=self.motion.pk).recommendation is None) @@ -1753,7 +1768,10 @@ class SetRecommendation(TestCase): self.assertEqual(response.status_code, status.HTTP_200_OK) self.assertEqual( response.data, - {"detail": "The recommendation of the motion was set to Acceptance."}, + { + "detail": "The recommendation of the motion was set to {0}.", + "args": ["Acceptance"], + }, ) self.assertEqual( Motion.objects.get(pk=self.motion.pk).recommendation.name, "accepted" @@ -1839,6 +1857,10 @@ class NumberMotionsInCategories(TestCase): """ Tests numbering motions in categories. + Default test environment: + - *without* blanks + - 1 min digit + Testdata. All names (and prefixes) are prefixed with "test_". The ordering is ensured with "category_weight". Category tree (with motions M and amendments A): @@ -1926,25 +1948,21 @@ class NumberMotionsInCategories(TestCase): def test_numbering(self): response = self.client.post(reverse("category-numbering", args=[self.A.pk])) self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertEqual(Motion.objects.get(pk=self.M1.pk).identifier, "test_A 1") - self.assertEqual(Motion.objects.get(pk=self.M3.pk).identifier, "test_A 2") - self.assertEqual(Motion.objects.get(pk=self.M2.pk).identifier, "test_C 3") + self.assertEqual(Motion.objects.get(pk=self.M1.pk).identifier, "test_A1") + self.assertEqual(Motion.objects.get(pk=self.M3.pk).identifier, "test_A2") + self.assertEqual(Motion.objects.get(pk=self.M2.pk).identifier, "test_C3") + self.assertEqual(Motion.objects.get(pk=self.M2_A1.pk).identifier, "test_C3-2") self.assertEqual( - Motion.objects.get(pk=self.M2_A1.pk).identifier, "test_C 3 - 2" - ) - self.assertEqual( - Motion.objects.get(pk=self.M2_A1_A1.pk).identifier, "test_C 3 - 2 - 1" - ) - self.assertEqual( - Motion.objects.get(pk=self.M2_A2.pk).identifier, "test_C 3 - 1" + Motion.objects.get(pk=self.M2_A1_A1.pk).identifier, "test_C3-2-1" ) + self.assertEqual(Motion.objects.get(pk=self.M2_A2.pk).identifier, "test_C3-1") - def test_with_blanks(self): + def test_with_blanks_and_leading_zeros(self): config["motions_amendments_prefix"] = "-X" - config["motions_identifier_with_blank"] = False + config["motions_identifier_with_blank"] = True config["motions_identifier_min_digits"] = 3 response = self.client.post(reverse("category-numbering", args=[self.A.pk])) - config["motions_identifier_with_blank"] = True + config["motions_identifier_with_blank"] = False config["motions_identifier_min_digits"] = 1 self.assertEqual(response.status_code, status.HTTP_200_OK) @@ -1963,16 +1981,17 @@ class NumberMotionsInCategories(TestCase): ) def test_existing_identifier_no_category(self): + # config["motions_identifier_with_blank"] = True conflicting_motion = Motion( title="test_title_al2=2k21fjv1lsck3ehlWExg", text="test_text_3omvpEhnfg082ejplk1m", ) conflicting_motion.save() - conflicting_motion.identifier = "test_C 3 - 2 - 1" + conflicting_motion.identifier = "test_C3-2-1" conflicting_motion.save() response = self.client.post(reverse("category-numbering", args=[self.A.pk])) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertTrue("test_C 3 - 2 - 1" in response.data["detail"]) + self.assertEqual("test_C3-2-1", response.data["args"][0]) def test_existing_identifier_with_category(self): conflicting_category = Category.objects.create( @@ -1984,20 +2003,20 @@ class NumberMotionsInCategories(TestCase): category=conflicting_category, ) conflicting_motion.save() - conflicting_motion.identifier = "test_C 3 - 2 - 1" + conflicting_motion.identifier = "test_C3-2-1" conflicting_motion.save() response = self.client.post(reverse("category-numbering", args=[self.A.pk])) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertTrue("test_C 3 - 2 - 1" in response.data["detail"]) - self.assertTrue(conflicting_category.name in response.data["detail"]) + self.assertEqual("test_C3-2-1", response.data["args"][0]) + self.assertEqual(conflicting_category.name, response.data["args"][1]) def test_incomplete_amendment_tree(self): self.M2_A1.category = None self.M2_A1.save() response = self.client.post(reverse("category-numbering", args=[self.A.pk])) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) - self.assertTrue(self.M2_A1_A1.title in response.data["detail"]) - self.assertTrue(self.M2_A1.title in response.data["detail"]) + self.assertEqual(self.M2_A1_A1.title, response.data["args"][0]) + self.assertEqual(self.M2_A1.title, response.data["args"][1]) class TestMotionBlock(TestCase):