From f7d392f1fccbb06a9147fd779ddd6bc003c67bd7 Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Mon, 24 Sep 2018 10:28:31 +0200 Subject: [PATCH] New motion features - Added weight and sort_parent fields to the motion model - Added motion sort view (adapted from agenda) - Added statute-paragraph model and tests for it - Added statute_paragraph foreign key to the motion model - Created migrations for sorting and statute-paragraph --- CHANGELOG.rst | 3 + .../src/app/shared/models/motions/motion.ts | 2 + .../models/motions/statute-paragraph.ts | 22 ++++ openslides/motions/access_permissions.py | 19 +++ openslides/motions/apps.py | 6 +- .../0013_motion_sorting_and_statute.py | 65 +++++++++++ openslides/motions/models.py | 54 +++++++++ openslides/motions/serializers.py | 14 ++- openslides/motions/views.py | 61 +++++++++- tests/integration/motions/test_viewset.py | 110 ++++++++++++++++++ 10 files changed, 352 insertions(+), 4 deletions(-) create mode 100644 client/src/app/shared/models/motions/statute-paragraph.ts create mode 100644 openslides/motions/migrations/0013_motion_sorting_and_statute.py diff --git a/CHANGELOG.rst b/CHANGELOG.rst index 68e4a7795..e4050a27a 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -17,6 +17,9 @@ Core: - Enabled docs for using OpenSlides with Gunicorn and Uvicorn in big mode [#3799, #3817]. +Motions: + - Option to customly sort motions [#3894]. + - Added support for adding a statute [#3894]. Version 2.3 (unreleased) ======================== diff --git a/client/src/app/shared/models/motions/motion.ts b/client/src/app/shared/models/motions/motion.ts index 83c62f8be..d16643837 100644 --- a/client/src/app/shared/models/motions/motion.ts +++ b/client/src/app/shared/models/motions/motion.ts @@ -37,6 +37,8 @@ export class Motion extends AgendaBaseModel { public polls: Object[]; public agenda_item_id: number; public log_messages: MotionLog[]; + public weight: number; + public sort_parent_id: number; public constructor(input?: any) { super('motions/motion', 'Motion', input); diff --git a/client/src/app/shared/models/motions/statute-paragraph.ts b/client/src/app/shared/models/motions/statute-paragraph.ts new file mode 100644 index 000000000..35f841e9c --- /dev/null +++ b/client/src/app/shared/models/motions/statute-paragraph.ts @@ -0,0 +1,22 @@ +import { BaseModel } from '../base/base-model'; + +/** + * Representation of a statute paragraph. + * @ignore + */ +export class StatuteParagraph extends BaseModel { + public id: number; + public title: string; + public text: string; + public weight: number; + + public constructor(input?: any) { + super('motions/statute-paragraph', input); + } + + public getTitle(): string { + return this.title; + } +} + +BaseModel.registerCollectionElement('motions/statute-paragraph', StatuteParagraph); diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index d072bee0d..1abf1651d 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -138,6 +138,25 @@ class MotionCommentSectionAccessPermissions(BaseAccessPermissions): return data +class StatuteParagraphAccessPermissions(BaseAccessPermissions): + """ + Access permissions container for StatuteParagraph and StatuteParagraphViewSet. + """ + def check_permissions(self, user): + """ + Returns True if the user has read access model instances. + """ + return has_perm(user, 'motions.can_see') + + def get_serializer_class(self, user=None): + """ + Returns serializer class. + """ + from .serializers import StatuteParagraphSerializer + + return StatuteParagraphSerializer + + class CategoryAccessPermissions(BaseAccessPermissions): """ Access permissions container for Category and CategoryViewSet. diff --git a/openslides/motions/apps.py b/openslides/motions/apps.py index f646ace63..c5ee14299 100644 --- a/openslides/motions/apps.py +++ b/openslides/motions/apps.py @@ -22,6 +22,7 @@ class MotionsAppConfig(AppConfig): ) from .views import ( CategoryViewSet, + StatuteParagraphViewSet, MotionViewSet, MotionCommentSectionViewSet, MotionBlockViewSet, @@ -47,6 +48,7 @@ class MotionsAppConfig(AppConfig): # Register viewsets. router.register(self.get_model('Category').get_collection_string(), CategoryViewSet) + router.register(self.get_model('StatuteParagraph').get_collection_string(), StatuteParagraphViewSet) router.register(self.get_model('Motion').get_collection_string(), MotionViewSet) router.register(self.get_model('MotionBlock').get_collection_string(), MotionBlockViewSet) router.register(self.get_model('MotionCommentSection').get_collection_string(), MotionCommentSectionViewSet) @@ -65,6 +67,6 @@ class MotionsAppConfig(AppConfig): Yields all Cachables required on startup i. e. opening the websocket connection. """ - for model_name in ('Category', 'Motion', 'MotionBlock', 'Workflow', - 'MotionChangeRecommendation', 'MotionCommentSection'): + for model_name in ('Category', 'StatuteParagraph', 'Motion', 'MotionBlock', + 'Workflow', 'MotionChangeRecommendation', 'MotionCommentSection'): yield self.get_model(model_name) diff --git a/openslides/motions/migrations/0013_motion_sorting_and_statute.py b/openslides/motions/migrations/0013_motion_sorting_and_statute.py new file mode 100644 index 000000000..92f4ac7af --- /dev/null +++ b/openslides/motions/migrations/0013_motion_sorting_and_statute.py @@ -0,0 +1,65 @@ +# Generated by Django 2.1.1 on 2018-09-24 08:26 + +import django.db.models.deletion +from django.db import migrations, models + +import openslides.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('motions', '0012_motion_comments'), + ] + + operations = [ + migrations.AlterModelOptions( + name='motionblock', + options={ + 'default_permissions': (), + 'verbose_name': 'Motion block'}, + ), + migrations.AddField( + model_name='motion', + name='sort_parent', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='children', + to='motions.Motion'), + ), + migrations.AddField( + model_name='motion', + name='weight', + field=models.IntegerField(default=10000), + ), + migrations.CreateModel( + name='StatuteParagraph', + fields=[ + ('id', models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name='ID')), + ('title', models.CharField(max_length=255)), + ('text', models.TextField()), + ('weight', models.IntegerField(default=10000)), + ], + options={ + 'ordering': ['weight', 'title'], + 'default_permissions': (), + }, + bases=(openslides.utils.models.RESTModelMixin, models.Model), + ), + migrations.AddField( + model_name='motion', + name='statute_paragraph', + field=models.ForeignKey( + blank=True, + null=True, + on_delete=django.db.models.deletion.SET_NULL, + related_name='motions', + to='motions.StatuteParagraph'), + ), + ] diff --git a/openslides/motions/models.py b/openslides/motions/models.py index c38392080..3b8928eb0 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -30,11 +30,37 @@ from .access_permissions import ( MotionBlockAccessPermissions, MotionChangeRecommendationAccessPermissions, MotionCommentSectionAccessPermissions, + StatuteParagraphAccessPermissions, WorkflowAccessPermissions, ) from .exceptions import WorkflowError +class StatuteParagraph(RESTModelMixin, models.Model): + """ + Model for parts of the statute + """ + access_permissions = StatuteParagraphAccessPermissions() + + title = models.CharField(max_length=255) + """Title of the statute paragraph.""" + + text = models.TextField() + """Content of the statute paragraph.""" + + weight = models.IntegerField(default=10000) + """ + A weight field to sort statute paragraphs. + """ + + class Meta: + default_permissions = () + ordering = ['weight', 'title'] + + def __str__(self): + return self.title + + class MotionManager(models.Manager): """ Customized model manager to support our get_full_queryset method. @@ -134,6 +160,21 @@ class Motion(RESTModelMixin, models.Model): Needed to find the next free motion identifier. """ + weight = models.IntegerField(default=10000) + """ + A weight field to sort motions. + """ + + sort_parent = models.ForeignKey( + 'self', + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name='children') + """ + A parent field for multi-depth sorting of motions. + """ + category = models.ForeignKey( 'Category', on_delete=models.SET_NULL, @@ -175,6 +216,19 @@ class Motion(RESTModelMixin, models.Model): Null if the motion is not an amendment. """ + statute_paragraph = models.ForeignKey( + StatuteParagraph, + on_delete=models.SET_NULL, + null=True, + blank=True, + related_name='motions') + """ + Field to reference to a statute paragraph if this motion is a + statute-amendment. + + Null if the motion is not a statute-amendment. + """ + tags = models.ManyToManyField(Tag, blank=True) """ Tags to categorise motions. diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index aa847c16b..f9484430f 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -28,6 +28,7 @@ from .models import ( MotionLog, MotionPoll, State, + StatuteParagraph, Submitter, Workflow, ) @@ -41,6 +42,15 @@ def validate_workflow_field(value): raise ValidationError({'detail': _('Workflow %(pk)d does not exist.') % {'pk': value}}) +class StatuteParagraphSerializer(ModelSerializer): + """ + Serializer for motion.models.StatuteParagraph objects. + """ + class Meta: + model = StatuteParagraph + fields = ('id', 'title', 'text', 'weight') + + class CategorySerializer(ModelSerializer): """ Serializer for motion.models.Category objects. @@ -404,7 +414,9 @@ class MotionSerializer(ModelSerializer): 'agenda_item_id', 'agenda_type', 'agenda_parent_id', - 'log_messages',) + 'log_messages', + 'sort_parent', + 'weight',) read_only_fields = ('state', 'recommendation',) # Some other fields are also read_only. See definitions above. def validate(self, data): diff --git a/openslides/motions/views.py b/openslides/motions/views.py index ed6a333f1..1e1ca0f6a 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -24,6 +24,7 @@ from ..utils.rest_api import ( UpdateModelMixin, ValidationError, detail_route, + list_route, ) from ..utils.views import BinaryTemplateView from .access_permissions import ( @@ -32,6 +33,7 @@ from .access_permissions import ( MotionBlockAccessPermissions, MotionChangeRecommendationAccessPermissions, MotionCommentSectionAccessPermissions, + StatuteParagraphAccessPermissions, WorkflowAccessPermissions, ) from .exceptions import WorkflowError @@ -44,6 +46,7 @@ from .models import ( MotionCommentSection, MotionPoll, State, + StatuteParagraph, Submitter, Workflow, ) @@ -78,7 +81,7 @@ class MotionViewSet(ModelViewSet): has_perm(self.request.user, 'motions.can_create') and (not config['motions_stop_submitting'] or has_perm(self.request.user, 'motions.can_manage'))) - elif self.action in ('set_state', 'manage_comments', 'set_recommendation', + elif self.action in ('set_state', 'sort', 'manage_comments', 'set_recommendation', 'follow_recommendation', 'create_poll', 'manage_submitters', 'sort_submitters'): result = (has_perm(self.request.user, 'motions.can_see') and @@ -256,6 +259,38 @@ class MotionViewSet(ModelViewSet): return Response(serializer.data) + @list_route(methods=['post']) + def sort(self, request): + """ + Sort motions. Also checks sort_parent field to prevent hierarchical loops. + + Note: This view is not tested! Maybe needs to be refactored. Add documentation + abou the data to be send. + """ + raise ValidationError({'detail': _('This view needs testing and refactoring!')}) + nodes = request.data.get('nodes', []) + sort_parent_id = request.data.get('sort_parent_id') + motions = [] + with transaction.atomic(): + for index, node in enumerate(nodes): + motion = Motion.objects.get(pk=node['id']) + motion.sort_parent_id = sort_parent_id + motion.weight = index + motion.save(skip_autoupdate=True) + motions.append(motion) + + # Now check consistency. TODO: Try to use less DB queries. + motion = Motion.objects.get(pk=node['id']) + ancestor = motion.sort_parent + while ancestor is not None: + if ancestor == motion: + raise ValidationError({'detail': _( + 'There must not be a hierarchical loop.')}) + ancestor = ancestor.sort_parent + + inform_changed_data(motions) + return Response({'detail': _('The motions has been sorted.')}) + @detail_route(methods=['POST', 'DELETE']) def manage_comments(self, request, pk=None): """ @@ -697,6 +732,30 @@ class MotionCommentSectionViewSet(ModelViewSet): return result +class StatuteParagraphViewSet(ModelViewSet): + """ + API endpoint for statute paragraphs. + + There are the following views: list, retrieve, create, + partial_update, update and destroy. + """ + access_permissions = StatuteParagraphAccessPermissions() + queryset = StatuteParagraph.objects.all() + + def check_view_permissions(self): + """ + Returns True if the user has required permissions. + """ + if self.action in ('list', 'retrieve'): + result = self.get_access_permissions().check_permissions(self.request.user) + elif self.action in ('create', 'partial_update', 'update', 'destroy'): + result = (has_perm(self.request.user, 'motions.can_see') and + has_perm(self.request.user, 'motions.can_manage')) + else: + result = False + return result + + class CategoryViewSet(ModelViewSet): """ API endpoint for categories. diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index dbe2e0e37..e77e94a7c 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -16,6 +16,7 @@ from openslides.motions.models import ( MotionCommentSection, MotionLog, State, + StatuteParagraph, Submitter, Workflow, ) @@ -79,6 +80,20 @@ def test_category_db_queries(): assert count_queries(Category.get_elements) == 1 +@pytest.mark.django_db(transaction=False) +def test_statute_paragraph_db_queries(): + """ + Tests that only the following db queries are done: + * 1 requests to get the list of all statute paragraphs. + """ + for index in range(10): + StatuteParagraph.objects.create( + title='statute_paragraph{}'.format(index), + text='text{}'.format(index)) + + assert count_queries(StatuteParagraph.get_elements) == 1 + + @pytest.mark.django_db(transaction=False) def test_workflow_db_queries(): """ @@ -91,6 +106,101 @@ def test_workflow_db_queries(): assert count_queries(Workflow.get_elements) == 3 +class TestStatuteParagraphs(TestCase): + """ + Tests all CRUD operations of statute paragraphs. + """ + def setUp(self): + self.client = APIClient() + self.client.login(username='admin', password='admin') + + def create_statute_paragraph(self): + self.title = 'test_title_fiWs82D0D)2kje3KDm2s' + self.text = 'test_text_3jfjoDqm,S;cmor3DJwk' + self.cp = StatuteParagraph.objects.create( + title=self.title, + text=self.text) + + def test_create_simple(self): + response = self.client.post( + reverse('statuteparagraph-list'), + {'title': 'test_title_f3FM328cq)tzdU238df2', + 'text': 'test_text_2fb)BEjwdI38=kfemiRkcOW'}) + self.assertEqual(response.status_code, status.HTTP_201_CREATED) + cp = StatuteParagraph.objects.get() + self.assertEqual(cp.title, 'test_title_f3FM328cq)tzdU238df2') + self.assertEqual(cp.text, 'test_text_2fb)BEjwdI38=kfemiRkcOW') + + def test_create_without_data(self): + response = self.client.post(reverse('statuteparagraph-list'), {}) + self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(response.data, {'title': ['This field is required.'], 'text': ['This field is required.']}) + + def test_create_non_admin(self): + self.admin = get_user_model().objects.get(username='admin') + self.admin.groups.add(2) + self.admin.groups.remove(4) + inform_changed_data(self.admin) + + response = self.client.post( + reverse('statuteparagraph-list'), + {'title': 'test_title_f3(Dj2jdP39fjW2kdcwe', + 'text': 'test_text_vlC)=fwWmcwcpWMvnuw('}) + + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_retrieve_simple(self): + self.create_statute_paragraph() + response = self.client.get(reverse('statuteparagraph-detail', args=[self.cp.pk])) + self.assertEqual(response.status_code, status.HTTP_200_OK) + self.assertEqual(sorted(response.data.keys()), sorted(( + 'id', + 'title', + 'text', + 'weight',))) + + def test_update_simple(self): + self.create_statute_paragraph() + response = self.client.patch( + reverse('statuteparagraph-detail', args=[self.cp.pk]), + {'text': 'test_text_ke(czr/cwk1Sl2seeFwE'}) + self.assertEqual(response.status_code, status.HTTP_200_OK) + cp = StatuteParagraph.objects.get() + self.assertEqual(cp.title, self.title) + self.assertEqual(cp.text, 'test_text_ke(czr/cwk1Sl2seeFwE') + + def test_update_non_admin(self): + self.admin = get_user_model().objects.get(username='admin') + self.admin.groups.add(2) + self.admin.groups.remove(4) + inform_changed_data(self.admin) + + self.create_statute_paragraph() + response = self.client.patch( + reverse('statuteparagraph-detail', args=[self.cp.pk]), + {'text': 'test_text_ke(czr/cwk1Sl2seeFwE'}) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + cp = StatuteParagraph.objects.get() + self.assertEqual(cp.text, self.text) + + def test_delete_simple(self): + self.create_statute_paragraph() + response = self.client.delete(reverse('statuteparagraph-detail', args=[self.cp.pk])) + self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT) + self.assertEqual(StatuteParagraph.objects.count(), 0) + + def test_delete_non_admin(self): + self.admin = get_user_model().objects.get(username='admin') + self.admin.groups.add(2) + self.admin.groups.remove(4) + inform_changed_data(self.admin) + + self.create_statute_paragraph() + response = self.client.delete(reverse('statuteparagraph-detail', args=[self.cp.pk])) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + self.assertEqual(StatuteParagraph.objects.count(), 1) + + class CreateMotion(TestCase): """ Tests motion creation.