From 65d5bbccd6ebbcf18ee79304bd24efdea478f9ff Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Tobias=20Ho=CC=88=C3=9Fl?= Date: Sun, 18 Jun 2017 20:20:44 +0200 Subject: [PATCH] Prevent colliding change recommendations - fixes #3298 --- CHANGELOG | 2 + openslides/motions/models.py | 21 ++++++- openslides/motions/views.py | 10 +++ tests/integration/motions/test_viewset.py | 77 +++++++++++++++++++++++ tests/unit/motions/test_models.py | 41 ++++++++++++ 5 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 tests/unit/motions/test_models.py diff --git a/CHANGELOG b/CHANGELOG index 6f80a2ec4..ecffd17ac 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -28,6 +28,8 @@ Motions: client [#3212]. - Reworked DOCX export parser and added comments to DOCX [#3258]. - New PDF export for personal note and comments [#3239]. +- Bugfix: Creating colliding change recommendation is now prevented + on server side [#3304] Users: - User without permission to see users can now see agenda item speakers, diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 4c72872f3..7fa52e0f6 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -1,6 +1,6 @@ from django.conf import settings from django.contrib.contenttypes.fields import GenericRelation -from django.core.exceptions import ImproperlyConfigured +from django.core.exceptions import ImproperlyConfigured, ValidationError from django.db import IntegrityError, models, transaction from django.db.models import Max from django.utils import formats, timezone @@ -771,6 +771,25 @@ class MotionChangeRecommendation(RESTModelMixin, models.Model): creation_time = models.DateTimeField(auto_now=True) """Time when the change recommendation was saved.""" + def collides_with_other_recommendation(self, recommendations): + for recommendation in recommendations: + if (not (self.line_from < recommendation.line_from and self.line_to <= recommendation.line_from) and + not (self.line_from >= recommendation.line_to and self.line_to > recommendation.line_to)): + return True + + return False + + def save(self, *args, **kwargs): + recommendations = (MotionChangeRecommendation.objects + .filter(motion_version=self.motion_version) + .exclude(pk=self.pk)) + + if self.collides_with_other_recommendation(recommendations): + raise ValidationError('The recommendation collides with an existing one (line %s - %s).' % + (self.line_from, self.line_to)) + + return super().save(*args, **kwargs) + class Meta: default_permissions = () diff --git a/openslides/motions/views.py b/openslides/motions/views.py index cec48e6a4..ec1882e51 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -3,6 +3,7 @@ import re from django.conf import settings from django.contrib.staticfiles import finders +from django.core.exceptions import ValidationError as DjangoValidationError from django.db import IntegrityError, transaction from django.http import Http404 from django.utils.translation import ugettext as _ @@ -449,6 +450,15 @@ 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) + class CategoryViewSet(ModelViewSet): """ diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index fb2f171bc..580e9f456 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -607,6 +607,83 @@ class ManageVersion(TestCase): self.assertEqual(response.data, {'detail': 'You can not delete the active version of a motion.'}) +class CreateMotionChangeRecommendation(TestCase): + """ + Tests motion change recommendation creation. + """ + def setUp(self): + self.client = APIClient() + self.client.login(username='admin', password='admin') + + self.client.post( + reverse('motion-list'), + {'title': 'test_title_OoCoo3MeiT9li5Iengu9', + 'text': 'test_text_thuoz0iecheiheereiCi'}) + + def test_simple(self): + """ + Creating a change plain, simple change recommendation + """ + response = self.client.post( + reverse('motionchangerecommendation-list'), + {'line_from': '5', + 'line_to': '7', + 'motion_version_id': '1', + 'text': '

New test

', + 'type': '0'}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + def test_collission(self): + """ + Two change recommendations with overlapping lines should lead to a Bad Request + """ + response = self.client.post( + reverse('motionchangerecommendation-list'), + {'line_from': '5', + 'line_to': '7', + 'motion_version_id': '1', + 'text': '

New test

', + 'type': '0'}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.post( + reverse('motionchangerecommendation-list'), + {'line_from': '3', + 'line_to': '6', + 'motion_version_id': '1', + 'text': '

New test

', + 'type': '0'}) + 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).'}) + + def test_no_collission_different_motions(self): + """ + Two change recommendations with overlapping lines, but affecting different motions, should not interfere + """ + self.client.post( + reverse('motion-list'), + {'title': 'test_title_OoCoo3MeiT9li5Iengu9', + 'text': 'test_text_thuoz0iecheiheereiCi'}) + + response = self.client.post( + reverse('motionchangerecommendation-list'), + {'line_from': '5', + 'line_to': '7', + 'motion_version_id': '1', + 'text': '

New test

', + 'type': '0'}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + response = self.client.post( + reverse('motionchangerecommendation-list'), + {'line_from': '3', + 'line_to': '6', + 'motion_version_id': '2', + 'text': '

New test

', + 'type': '0'}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + + class SupportMotion(TestCase): """ Tests supporting a motion. diff --git a/tests/unit/motions/test_models.py b/tests/unit/motions/test_models.py new file mode 100644 index 000000000..f8c2bddba --- /dev/null +++ b/tests/unit/motions/test_models.py @@ -0,0 +1,41 @@ +from unittest import TestCase + +from openslides.motions.models import MotionChangeRecommendation, MotionVersion + + +class MotionChangeRecommendationTest(TestCase): + def test_overlapping_line_numbers(self): + """ + Tests that a change recommendation directly before another one can be created + """ + version = MotionVersion() + existing_recommendation = MotionChangeRecommendation() + existing_recommendation.line_from = 5 + existing_recommendation.line_to = 7 + existing_recommendation.rejected = False + existing_recommendation.motion_version = version + other_recommendations = [existing_recommendation] + + new_recommendation1 = MotionChangeRecommendation() + new_recommendation1.line_from = 3 + new_recommendation1.line_to = 5 + collides = new_recommendation1.collides_with_other_recommendation(other_recommendations) + self.assertFalse(collides) + + new_recommendation2 = MotionChangeRecommendation() + new_recommendation2.line_from = 3 + new_recommendation2.line_to = 6 + collides = new_recommendation2.collides_with_other_recommendation(other_recommendations) + self.assertTrue(collides) + + new_recommendation3 = MotionChangeRecommendation() + new_recommendation3.line_from = 6 + new_recommendation3.line_to = 8 + collides = new_recommendation3.collides_with_other_recommendation(other_recommendations) + self.assertTrue(collides) + + new_recommendation4 = MotionChangeRecommendation() + new_recommendation4.line_from = 7 + new_recommendation4.line_to = 9 + collides = new_recommendation4.collides_with_other_recommendation(other_recommendations) + self.assertFalse(collides)