From b0a42e19e11fc343304202840a79b48be68f9b9c Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Tue, 12 Jun 2018 14:17:02 +0200 Subject: [PATCH] Sort submitters --- .travis.yml | 2 +- CHANGELOG.rst | 3 +- .../list-of-speakers-partial-management.html | 36 ++-- openslides/core/static/css/core/_site.scss | 5 +- openslides/motions/access_permissions.py | 3 +- .../migrations/0006_submitter_model.py | 72 +++++++ openslides/motions/models.py | 72 ++++++- openslides/motions/projector.py | 3 +- openslides/motions/serializers.py | 20 +- openslides/motions/signals.py | 3 +- openslides/motions/static/js/motions/base.js | 42 ++-- openslides/motions/static/js/motions/csv.js | 11 +- openslides/motions/static/js/motions/docx.js | 13 +- openslides/motions/static/js/motions/pdf.js | 11 +- openslides/motions/static/js/motions/site.js | 203 ++++++++++++------ .../templates/motions/motion-detail.html | 14 +- .../static/templates/motions/motion-list.html | 9 +- .../templates/motions/motion-submitters.html | 44 ++++ .../templates/motions/slide_motion.html | 4 +- openslides/motions/views.py | 144 ++++++++++++- tests/integration/motions/test_viewset.py | 129 ++++++++++- tests/unit/motions/test_views.py | 2 +- 22 files changed, 702 insertions(+), 143 deletions(-) create mode 100644 openslides/motions/migrations/0006_submitter_model.py create mode 100644 openslides/motions/static/templates/motions/motion-submitters.html diff --git a/.travis.yml b/.travis.yml index 9a9f8ec19..24f2819b0 100644 --- a/.travis.yml +++ b/.travis.yml @@ -27,7 +27,7 @@ script: - node_modules/.bin/karma start --browsers PhantomJS tests/karma/karma.conf.js - DJANGO_SETTINGS_MODULE='tests.settings' coverage run ./manage.py test tests.unit - - coverage report --fail-under=43 + - coverage report --fail-under=42 - DJANGO_SETTINGS_MODULE='tests.settings' coverage run ./manage.py test tests.integration - coverage report --fail-under=73 diff --git a/CHANGELOG.rst b/CHANGELOG.rst index dff0b5cde..9d25be287 100644 --- a/CHANGELOG.rst +++ b/CHANGELOG.rst @@ -8,7 +8,8 @@ Version 2.3 (unreleased) ======================== Motions: -- New feature to scroll the projector to a specific line [#3748]. + - New feature to scroll the projector to a specific line [#3748]. + - New possibility to sort submitters [#3647]. Version 2.2 (2018-06-06) diff --git a/openslides/agenda/static/templates/agenda/list-of-speakers-partial-management.html b/openslides/agenda/static/templates/agenda/list-of-speakers-partial-management.html index 8effcedcd..5299fdea2 100644 --- a/openslides/agenda/static/templates/agenda/list-of-speakers-partial-management.html +++ b/openslides/agenda/static/templates/agenda/list-of-speakers-partial-management.html @@ -1,26 +1,26 @@
-
-
- - - - - - + + + + - + + +
-

@@ -79,7 +79,7 @@

    -
  1. +
  2. {{ $index + 1 }}. {{ speaker.user.get_full_name() }} diff --git a/openslides/core/static/css/core/_site.scss b/openslides/core/static/css/core/_site.scss index 124eb6d17..63c12c2e8 100644 --- a/openslides/core/static/css/core/_site.scss +++ b/openslides/core/static/css/core/_site.scss @@ -274,11 +274,8 @@ strong, b, th { .meta { - h3 { - font-family: $font-condensed-light; - } - .heading, h3 { + font-family: $font-condensed-light; font-size: 22px; line-height: 24px; font-weight: 300; diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 013f8ab9f..57ccba291 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -43,7 +43,8 @@ class MotionAccessPermissions(BaseAccessPermissions): for full in full_data: # Check if user is submitter of this motion. if isinstance(user, CollectionElement): - is_submitter = user.get_full_data()['id'] in full.get('submitters_id', []) + is_submitter = user.get_full_data()['id'] in [ + submitter['user_id'] for submitter in full.get('submitters', [])] else: # Anonymous users can not be submitters. is_submitter = False diff --git a/openslides/motions/migrations/0006_submitter_model.py b/openslides/motions/migrations/0006_submitter_model.py new file mode 100644 index 000000000..dc890b5d2 --- /dev/null +++ b/openslides/motions/migrations/0006_submitter_model.py @@ -0,0 +1,72 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.8 on 2018-02-09 07:18 +from __future__ import unicode_literals + +import django.db.models.deletion +from django.conf import settings +from django.contrib.auth.models import AnonymousUser +from django.db import migrations, models + +import openslides.utils.models + + +def move_submitters_to_own_model(apps, schema_editor): + Motion = apps.get_model('motions', 'Motion') + Submitter = apps.get_model('motions', 'Submitter') + + for motion in Motion.objects.all(): + weight = 0 + for user in motion.submittersOld.all(): + # We cannot use the add method here, so do it manually... + if Submitter.objects.filter(user=user, motion=motion).exists(): + continue # The user is already a submitter. Skip this duplicate. + if isinstance(user, AnonymousUser): + continue # Skip the anonymous + + submitter = Submitter(user=user, motion=motion, weight=weight) + submitter.save(force_insert=True) + weight += 1 + + +class Migration(migrations.Migration): + + dependencies = [ + migrations.swappable_dependency(settings.AUTH_USER_MODEL), + ('motions', '0005_auto_20180202_1318'), + ] + + operations = [ + migrations.RenameField( + model_name='motion', + old_name='submitters', + new_name='submittersOld', + ), + migrations.CreateModel( + name='Submitter', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('weight', models.IntegerField(null=True)), + ], + options={ + 'default_permissions': (), + }, + bases=(openslides.utils.models.RESTModelMixin, models.Model), + ), + migrations.AddField( + model_name='submitter', + name='motion', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, related_name='submitters', to='motions.Motion'), + ), + migrations.AddField( + model_name='submitter', + name='user', + field=models.ForeignKey(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL), + ), + migrations.RunPython( + move_submitters_to_own_model + ), + migrations.RemoveField( + model_name='motion', + name='submittersOld', + ), + ] diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 0f5fc73c0..afca64661 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -1,6 +1,7 @@ from typing import Any, Dict # noqa from django.conf import settings +from django.contrib.auth.models import AnonymousUser from django.contrib.contenttypes.fields import GenericRelation from django.core.exceptions import ImproperlyConfigured, ValidationError from django.db import IntegrityError, models, transaction @@ -21,6 +22,7 @@ from openslides.poll.models import ( CollectDefaultVotesMixin, ) from openslides.utils.autoupdate import inform_changed_data +from openslides.utils.exceptions import OpenSlidesError from openslides.utils.models import RESTModelMixin from .access_permissions import ( @@ -157,11 +159,6 @@ class Motion(RESTModelMixin, models.Model): Tags to categorise motions. """ - submitters = models.ManyToManyField(settings.AUTH_USER_MODEL, related_name='motion_submitters', blank=True) - """ - Users who submit this motion. - """ - supporters = models.ManyToManyField(settings.AUTH_USER_MODEL, related_name='motion_supporters', blank=True) """ Users who support this motion. @@ -548,7 +545,7 @@ class Motion(RESTModelMixin, models.Model): """ Returns True if user is a submitter of this motion, else False. """ - return user in self.submitters.all() + return self.submitters.filter(user=user).exists() def is_supporter(self, user): """ @@ -706,6 +703,69 @@ class Motion(RESTModelMixin, models.Model): yield from amendment.get_amendments_deep() +class SubmitterManager(models.Manager): + """ + Manager for Submitter model. Provides a customized add method. + """ + def add(self, user, motion, skip_autoupdate=False): + """ + Customized manager method to prevent anonymous users to be a + submitter and that someone is not twice a submitter. Cares also + for the initial sorting of the submitters. + """ + if self.filter(user=user, motion=motion).exists(): + raise OpenSlidesError( + _('{user} is already a submitter.').format(user=user)) + if isinstance(user, AnonymousUser): + raise OpenSlidesError( + _('An anonymous user can not be a submitter.')) + weight = (self.filter(motion=motion).aggregate( + models.Max('weight'))['weight__max'] or 0) + submitter = self.model(user=user, motion=motion, weight=weight + 1) + submitter.save(force_insert=True, skip_autoupdate=skip_autoupdate) + return submitter + + +class Submitter(RESTModelMixin, models.Model): + """ + M2M Model for submitters. + """ + + objects = SubmitterManager() + """ + Use custom Manager. + """ + + user = models.ForeignKey( + settings.AUTH_USER_MODEL, + on_delete=models.CASCADE) + """ + ForeignKey to the user who is the submitter. + """ + + motion = models.ForeignKey( + Motion, + on_delete=models.CASCADE, + related_name='submitters') + """ + ForeignKey to the motion. + """ + + weight = models.IntegerField(null=True) + + class Meta: + default_permissions = () + + def __str__(self): + return str(self.user) + + def get_root_rest_element(self): + """ + Returns the motion to this instance which is the root REST element. + """ + return self.motion + + class MotionVersion(RESTModelMixin, models.Model): """ A MotionVersion object saves some date of the motion. diff --git a/openslides/motions/projector.py b/openslides/motions/projector.py index bbc02575f..68d9e9073 100644 --- a/openslides/motions/projector.py +++ b/openslides/motions/projector.py @@ -29,7 +29,8 @@ class MotionSlide(ProjectorElement): yield motion.agenda_item yield motion.state.workflow yield from self.required_motions_for_state_and_recommendation(motion) - yield from motion.submitters.all() + for submitter in motion.submitters.all(): + yield submitter.user yield from motion.supporters.all() yield from MotionChangeRecommendation.objects.filter(motion_version=motion.get_active_version().id) diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 0dcdd4031..da929765b 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -24,6 +24,7 @@ from .models import ( MotionPoll, MotionVersion, State, + Submitter, Workflow, ) @@ -290,6 +291,20 @@ class MotionChangeRecommendationSerializer(ModelSerializer): return data +class SubmitterSerializer(ModelSerializer): + """ + Serializer for motion.models.Submitter objects. + """ + class Meta: + model = Submitter + fields = ( + 'id', + 'user', + 'motion', + 'weight', + ) + + class MotionSerializer(ModelSerializer): """ Serializer for motion.models.Motion objects. @@ -310,6 +325,7 @@ class MotionSerializer(ModelSerializer): write_only=True) agenda_type = IntegerField(write_only=True, required=False, min_value=1, max_value=2) agenda_parent_id = IntegerField(write_only=True, required=False, min_value=1) + submitters = SubmitterSerializer(many=True, read_only=True) class Meta: model = Motion @@ -374,10 +390,6 @@ class MotionSerializer(ModelSerializer): motion.agenda_item_update_information['type'] = validated_data.get('agenda_type') motion.agenda_item_update_information['parent_id'] = validated_data.get('agenda_parent_id') motion.save() - if validated_data.get('submitters'): - motion.submitters.add(*validated_data['submitters']) - elif validated_data['request_user'].is_authenticated(): - motion.submitters.add(validated_data['request_user']) motion.supporters.add(*validated_data.get('supporters', [])) motion.attachments.add(*validated_data.get('attachments', [])) motion.tags.add(*validated_data.get('tags', [])) diff --git a/openslides/motions/signals.py b/openslides/motions/signals.py index 79303177a..4bc838522 100644 --- a/openslides/motions/signals.py +++ b/openslides/motions/signals.py @@ -130,6 +130,7 @@ def required_users(sender, request_user, **kwargs): if has_perm(request_user, 'motions.can_see'): for motion_collection_element in Collection(Motion.get_collection_string()).element_generator(): full_data = motion_collection_element.get_full_data() - submitters_supporters.update(full_data['submitters_id']) + submitters_supporters.update( + [submitter['user_id'] for submitter in full_data['submitters']]) submitters_supporters.update(full_data['supporters_id']) return submitters_supporters diff --git a/openslides/motions/static/js/motions/base.js b/openslides/motions/static/js/motions/base.js index 3f868e113..93de52069 100644 --- a/openslides/motions/static/js/motions/base.js +++ b/openslides/motions/static/js/motions/base.js @@ -209,6 +209,22 @@ angular.module('OpenSlidesApp.motions', [ } ]) +.factory('Submitter', [ + 'DS', + function (DS) { + return DS.defineResource({ + name: 'motions/submitter', + relations: { + belongsTo: { + 'users/user': { + localField: 'user', + localKey: 'user_id', + } + } + } + }); + } +]) .factory('Motion', [ 'DS', @@ -571,6 +587,7 @@ angular.module('OpenSlidesApp.motions', [ * There are the following possible actions. * - see * - update + * - update_submitters * - delete * - create_poll * - support @@ -604,6 +621,8 @@ angular.module('OpenSlidesApp.motions', [ this.state.allow_submitter_edit ) ); + case 'update_submitters': + return operator.hasPerms('motions.can_manage'); case 'delete': return ( operator.hasPerms('motions.can_manage') || @@ -755,20 +774,18 @@ angular.module('OpenSlidesApp.motions', [ localField: 'attachments', localKeys: 'attachments_id', }, - 'users/user': [ - { - localField: 'submitters', - localKeys: 'submitters_id', - }, - { - localField: 'supporters', - localKeys: 'supporters_id', - } - ], + 'users/user': { + localField: 'supporters', + localKeys: 'supporters_id', + }, 'motions/motion-poll': { localField: 'polls', foreignKey: 'motion_id', - } + }, + 'motions/submitter': { + localField: 'submitters', + foreignKey: 'motion_id', + }, }, hasOne: { 'motions/workflowstate': [ @@ -987,7 +1004,8 @@ angular.module('OpenSlidesApp.motions', [ 'Category', 'Workflow', 'MotionChangeRecommendation', - function(Motion, Category, Workflow, MotionChangeRecommendation) {} + 'Submitter', + function(Motion, Category, Workflow, MotionChangeRecommendation, Submitter) {} ]) diff --git a/openslides/motions/static/js/motions/csv.js b/openslides/motions/static/js/motions/csv.js index c839498ae..756552864 100644 --- a/openslides/motions/static/js/motions/csv.js +++ b/openslides/motions/static/js/motions/csv.js @@ -5,11 +5,12 @@ angular.module('OpenSlidesApp.motions.csv', []) .factory('MotionCsvExport', [ + '$filter', 'gettextCatalog', 'Config', 'CsvDownload', 'lineNumberingService', - function (gettextCatalog, Config, CsvDownload, lineNumberingService) { + function ($filter, gettextCatalog, Config, CsvDownload, lineNumberingService) { var makeHeaderline = function (params) { var headerline = ['Identifier', 'Title']; if (params.include.text) { @@ -77,8 +78,12 @@ angular.module('OpenSlidesApp.motions.csv', []) // Submitters if (params.include.submitters) { var submitters = []; - angular.forEach(motion.submitters, function(user) { - var user_short_name = [user.title, user.first_name, user.last_name].join(' ').trim(); + _.forEach($filter('orderBy')(motion.submitters, 'weight'), function (user) { + var user_short_name = [ + user.user.title, + user.user.first_name, + user.user.last_name + ].join(' ').trim(); submitters.push(user_short_name); }); row.push('"' + submitters.join('; ') + '"'); diff --git a/openslides/motions/static/js/motions/docx.js b/openslides/motions/static/js/motions/docx.js index 7d0baa8e4..eeb66a263 100644 --- a/openslides/motions/static/js/motions/docx.js +++ b/openslides/motions/static/js/motions/docx.js @@ -7,6 +7,7 @@ angular.module('OpenSlidesApp.motions.docx', ['OpenSlidesApp.core.docx']) .factory('MotionDocxExport', [ '$http', '$q', + '$filter', 'operator', 'Config', 'Category', @@ -15,8 +16,8 @@ angular.module('OpenSlidesApp.motions.docx', ['OpenSlidesApp.core.docx']) 'lineNumberingService', 'Html2DocxConverter', 'MotionComment', - function ($http, $q, operator, Config, Category, gettextCatalog, FileSaver, lineNumberingService, - Html2DocxConverter, MotionComment) { + function ($http, $q, $filter, operator, Config, Category, gettextCatalog, + FileSaver, lineNumberingService, Html2DocxConverter, MotionComment) { var PAGEBREAK = ''; @@ -116,9 +117,11 @@ angular.module('OpenSlidesApp.motions.docx', ['OpenSlidesApp.core.docx']) id: motion.id, identifier: motion.identifier || '', title: title, - submitters: params.include.submitters ? _.map(motion.submitters, function (submitter) { - return submitter.get_full_name(); - }).join(', ') : '', + submitters: params.include.submitters ? _.map( + $filter('orderBy')(motion.submitters, 'weight'), function (submitter) { + return submitter.user.get_full_name(); + } + ).join(', ') : '', status: motion.getStateName(), // Miscellaneous stuff preamble: gettextCatalog.getString(Config.get('motions_preamble').value), diff --git a/openslides/motions/static/js/motions/pdf.js b/openslides/motions/static/js/motions/pdf.js index 1b51fb797..4364272e7 100644 --- a/openslides/motions/static/js/motions/pdf.js +++ b/openslides/motions/static/js/motions/pdf.js @@ -6,6 +6,7 @@ angular.module('OpenSlidesApp.motions.pdf', ['OpenSlidesApp.core.pdf']) .factory('MotionContentProvider', [ '$q', + '$filter', 'operator', 'gettextCatalog', 'PDFLayout', @@ -17,7 +18,7 @@ angular.module('OpenSlidesApp.motions.pdf', ['OpenSlidesApp.core.pdf']) 'Motion', 'MotionComment', 'OpenSlidesSettings', - function($q, operator, gettextCatalog, PDFLayout, PdfMakeConverter, ImageConverter, + function($q, $filter, operator, gettextCatalog, PDFLayout, PdfMakeConverter, ImageConverter, HTMLValidizer, Category, Config, Motion, MotionComment, OpenSlidesSettings) { /** * Provides the content as JS objects for Motions in pdfMake context @@ -86,9 +87,11 @@ angular.module('OpenSlidesApp.motions.pdf', ['OpenSlidesApp.core.pdf']) var metaTableBody = []; // submitters - var submitters = _.map(motion.submitters, function (submitter) { - return submitter.get_full_name(); - }).join(', '); + var submitters = _.map( + $filter('orderBy')(motion.submitters, 'weight'), function (submitter) { + return submitter.user.get_full_name(); + } + ).join(', '); if (params.include.submitters) { metaTableBody.push([ { diff --git a/openslides/motions/static/js/motions/site.js b/openslides/motions/static/js/motions/site.js index e04173578..14dc9786c 100644 --- a/openslides/motions/static/js/motions/site.js +++ b/openslides/motions/static/js/motions/site.js @@ -89,6 +89,19 @@ angular.module('OpenSlidesApp.motions.site', [ } ] }) + .state('motions.motion.submitters', { + url: '/submitters/{id:int}', + controller: 'MotionSubmitterCtrl', + resolve: { + motionId: ['$stateParams', function($stateParams) { + return $stateParams.id; + }], + }, + data: { + title: gettext('Submitters'), + basePerm: 'motions.can_manage', + }, + }) .state('motions.motion.import', { url: '/import', controller: 'MotionImportCtrl', @@ -388,67 +401,73 @@ angular.module('OpenSlidesApp.motions.site', [ getFormFields: function (isCreateForm) { var workflows = Workflow.getAll(); var images = Mediafile.getAllImages(); - var formFields = [ - { + var formFields = []; + formFields.push({ key: 'identifier', type: 'input', templateOptions: { label: gettextCatalog.getString('Identifier') }, hide: true - }, - { - key: 'submitters_id', - type: 'select-multiple', - templateOptions: { - label: gettextCatalog.getString('Submitters'), - options: User.getAll(), - ngOptions: 'option.id as option.full_name for option in to.options', - placeholder: gettextCatalog.getString('Select or search a submitter ...') + }); + + if (isCreateForm) { + formFields.push({ + key: 'submitters_id', + type: 'select-multiple', + templateOptions: { + label: gettextCatalog.getString('Submitters'), + options: User.getAll(), + ngOptions: 'option.id as option.full_name for option in to.options', + placeholder: gettextCatalog.getString('Select or search a submitter ...'), + }, + hide: !operator.hasPerms('motions.can_manage') + }); + } + + formFields = formFields.concat([ + { + key: 'title', + type: 'input', + templateOptions: { + label: gettextCatalog.getString('Title'), + required: true + } }, - hide: !operator.hasPerms('motions.can_manage') - }, - { - key: 'title', - type: 'input', - templateOptions: { - label: gettextCatalog.getString('Title'), - required: true + { + template: '

    ' + Config.translate(Config.get('motions_preamble').value) + '

    ' + }, + { + key: 'text', + type: 'editor', + templateOptions: { + label: gettextCatalog.getString('Text'), + required: true + }, + data: { + ckeditorOptions: Editor.getOptions() + } + }, + { + key: 'reason', + type: 'editor', + templateOptions: { + label: gettextCatalog.getString('Reason'), + }, + data: { + ckeditorOptions: Editor.getOptions() + } + }, + { + key: 'disable_versioning', + type: 'checkbox', + templateOptions: { + label: gettextCatalog.getString('Trivial change'), + description: gettextCatalog.getString("Don't create a new version.") + }, + hide: true } - }, - { - template: '

    ' + Config.translate(Config.get('motions_preamble').value) + '

    ' - }, - { - key: 'text', - type: 'editor', - templateOptions: { - label: gettextCatalog.getString('Text'), - required: true - }, - data: { - ckeditorOptions: Editor.getOptions() - } - }, - { - key: 'reason', - type: 'editor', - templateOptions: { - label: gettextCatalog.getString('Reason'), - }, - data: { - ckeditorOptions: Editor.getOptions() - } - }, - { - key: 'disable_versioning', - type: 'checkbox', - templateOptions: { - label: gettextCatalog.getString('Trivial change'), - description: gettextCatalog.getString("Don't create a new version.") - }, - hide: true - }]; + ]); // show as agenda item + parent item if (isCreateForm) { @@ -1192,7 +1211,7 @@ angular.module('OpenSlidesApp.motions.site', [ ]; $scope.filter.propertyDict = { 'submitters': function (submitter) { - return submitter.get_short_name(); + return submitter.user.get_short_name(); }, 'supporters': function (supporter) { return supporter.get_short_name(); @@ -1235,7 +1254,7 @@ angular.module('OpenSlidesApp.motions.site', [ display_name: gettext('Identifier')}, {name: 'getTitle()', display_name: gettext('Title')}, - {name: 'submitters[0].get_short_name()', + {name: 'submitters[0].user.get_short_name()', display_name: gettext('Submitters')}, {name: 'category.' + Config.get('motions_export_category_sorting').value, display_name: gettext('Category')}, @@ -2117,7 +2136,7 @@ angular.module('OpenSlidesApp.motions.site', [ 'operator', 'ErrorMessage', 'EditingWarning', - function($scope, $state, Motion, Category, Config, Mediafile, MotionForm, + function ($scope, $state, Motion, Category, Config, Mediafile, MotionForm, Tag, User, Workflow, Agenda, motionId, operator, ErrorMessage, EditingWarning) { Category.bindAll({}, $scope, 'categories'); @@ -2176,7 +2195,7 @@ angular.module('OpenSlidesApp.motions.site', [ Motion.inject(motion); // save changed motion object on server Motion.save(motion).then( - function(success) { + function (success) { if (gotoDetailView) { $state.go('motions.motion.detail', {id: success.id}); } @@ -2201,7 +2220,7 @@ angular.module('OpenSlidesApp.motions.site', [ 'motionpollId', 'voteNumber', 'ErrorMessage', - function($scope, gettextCatalog, MotionPoll, MotionPollForm, motionpollId, + function ($scope, gettextCatalog, MotionPoll, MotionPollForm, motionpollId, voteNumber, ErrorMessage) { // set initial values for form model by create deep copy of motionpoll object // so detail view is not updated while editing poll @@ -2220,16 +2239,76 @@ angular.module('OpenSlidesApp.motions.site', [ votesinvalid: poll.votesinvalid, votescast: poll.votescast }) - .then(function(success) { + .then(function (success) { $scope.alert.show = false; $scope.closeThisDialog(); - }, function(error) { + }, function (error) { $scope.alert = ErrorMessage.forAlert(error); }); }; } ]) +.controller('MotionSubmitterCtrl', [ + '$scope', + '$filter', + '$http', + 'User', + 'Motion', + 'motionId', + 'ErrorMessage', + function ($scope, $filter, $http, User, Motion, motionId, ErrorMessage) { + User.bindAll({}, $scope, 'users'); + $scope.submitterSelectBox = {}; + $scope.alert = {}; + + $scope.$watch(function () { + return Motion.lastModified(motionId); + }, function () { + $scope.motion = Motion.get(motionId); + $scope.submitters = $filter('orderBy')($scope.motion.submitters, 'weight'); + }); + + $scope.addSubmitter = function (userId) { + $scope.submitterSelectBox = {}; + $http.post('/rest/motions/motion/' + $scope.motion.id + '/manage_submitters/', { + 'user': userId + }).then( + function (success) { + $scope.alert.show = false; + }, function (error) { + $scope.alert = ErrorMessage.forAlert(error); + } + ); + }; + + $scope.removeSubmitter = function (userId) { + $http.delete('/rest/motions/motion/' + $scope.motion.id + '/manage_submitters/', { + headers: {'Content-Type': 'application/json'}, + data: JSON.stringify({user: userId}) + }).then( + function (success) { + $scope.alert.show = false; + }, function (error) { + $scope.alert = ErrorMessage.forAlert(error); + } + ); + }; + + // save reordered list of submitters + $scope.treeOptions = { + dropped: function (event) { + var submitterIds = _.map($scope.submitters, function (submitter) { + return submitter.id; + }); + $http.post('/rest/motions/motion/' + $scope.motion.id + '/sort_submitters/', { + submitters: submitterIds, + }); + } + }; + } +]) + .controller('MotionImportCtrl', [ '$scope', '$q', @@ -2239,7 +2318,7 @@ angular.module('OpenSlidesApp.motions.site', [ 'MotionBlock', 'User', 'MotionCsvExport', - function($scope, $q, gettext, Category, Motion, MotionBlock, User, MotionCsvExport) { + function ($scope, $q, gettext, Category, Motion, MotionBlock, User, MotionCsvExport) { // set initial data for csv import $scope.motions = []; @@ -2295,7 +2374,7 @@ angular.module('OpenSlidesApp.motions.site', [ } // submitter if (motion.submitter && motion.submitter !== '') { - angular.forEach(User.getAll(), function (user) { + _.forEach(User.getAll(), function (user) { var user_short_name = [user.title, user.first_name, user.last_name].join(' ').trim(); if (user_short_name == motion.submitter.trim()) { motion.submitters_id = [user.id]; diff --git a/openslides/motions/static/templates/motions/motion-detail.html b/openslides/motions/static/templates/motions/motion-detail.html index f365ffa30..fab9b04e2 100644 --- a/openslides/motions/static/templates/motions/motion-detail.html +++ b/openslides/motions/static/templates/motions/motion-detail.html @@ -125,9 +125,15 @@
    -

    Submitters

    -
    - {{ submitter.get_full_name() }} +

    Submitters

    +

    + Submitters + + + +

    +
    + {{ submitter.user.get_full_name() }}
    @@ -211,7 +217,7 @@
    - {{ config('motions_recommendations_by') }} + {{ config('motions_recommendations_by') | translate }}
      diff --git a/openslides/motions/static/templates/motions/motion-list.html b/openslides/motions/static/templates/motions/motion-list.html index f1987afc3..f9d24b668 100644 --- a/openslides/motions/static/templates/motions/motion-list.html +++ b/openslides/motions/static/templates/motions/motion-list.html @@ -628,8 +628,8 @@
      by - - {{ submitter.get_full_name() }},, + + {{ submitter.user.get_full_name() }},, ... [+{{ motion.submitters.length - 1 }}] @@ -640,6 +640,11 @@ Edit + · + + Edit submitters + + · +
      +
      + + + Back + +
      +

      {{ motion.getAgendaTitle() }}

      +

      Sort submitters

      +
      +
      + +
      + +
      +
        +
      1. + + {{ $index + 1 }}. + {{ submitter.user.get_full_name() }} + +
      +
      + + +
      +
      + {{ alert.msg }} +
      + +
      +
      diff --git a/openslides/motions/static/templates/motions/slide_motion.html b/openslides/motions/static/templates/motions/slide_motion.html index 8fb4c71d2..8c39371c4 100644 --- a/openslides/motions/static/templates/motions/slide_motion.html +++ b/openslides/motions/static/templates/motions/slide_motion.html @@ -12,8 +12,8 @@

      Submitters

      -
      - {{ submitter.get_full_name() }}
      +
      + {{ submitter.user.get_full_name() }},
      diff --git a/openslides/motions/views.py b/openslides/motions/views.py index f9d8b08e9..68d095623 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -2,9 +2,11 @@ import re from typing import Optional # noqa from django.conf import settings +from django.contrib.auth import get_user_model from django.core.exceptions import ValidationError as DjangoValidationError from django.db import IntegrityError, transaction from django.http import Http404 +from django.http.request import QueryDict from django.utils.translation import ugettext as _ from django.utils.translation import ugettext_noop from rest_framework import status @@ -13,6 +15,7 @@ from ..core.config import config from ..utils.auth import has_perm from ..utils.autoupdate import inform_changed_data from ..utils.collection import CollectionElement +from ..utils.exceptions import OpenSlidesError from ..utils.rest_api import ( DestroyModelMixin, GenericViewSet, @@ -39,6 +42,7 @@ from .models import ( MotionPoll, MotionVersion, State, + Submitter, Workflow, ) from .serializers import MotionPollSerializer @@ -73,7 +77,8 @@ class MotionViewSet(ModelViewSet): (not config['motions_stop_submitting'] or has_perm(self.request.user, 'motions.can_manage'))) elif self.action in ('manage_version', 'set_state', 'set_recommendation', - 'follow_recommendation', 'create_poll'): + 'follow_recommendation', 'create_poll', 'manage_submitters', + 'sort_submitters'): result = (has_perm(self.request.user, 'motions.can_see') and has_perm(self.request.user, 'motions.can_manage')) elif self.action == 'support': @@ -150,6 +155,38 @@ class MotionViewSet(ModelViewSet): serializer.is_valid(raise_exception=True) motion = serializer.save(request_user=request.user) + # Check for submitters and make ids unique + if isinstance(request.data, QueryDict): + submitters_id = request.data.getlist('submitters_id') + else: + submitters_id = request.data.get('submitters_id') + if submitters_id is None: + submitters_id = [] + if not isinstance(submitters_id, list): + raise ValidationError({'detail': _('If submitters_id is given, it has to be a list.')}) + + submitters_id_unique = set() + for id in submitters_id: + try: + submitters_id_unique.add(int(id)) + except ValueError: + continue + + submitters = [] + for submitter_id in submitters_id_unique: + try: + submitters.append(get_user_model().objects.get(pk=submitter_id)) + except get_user_model().DoesNotExist: + continue # Do not add users that do not exist + + # Add the request user, if he is authenticated and no submitters were given: + if len(submitters) == 0 and request.user.is_authenticated(): + submitters.append(request.user) + + # create all submitters + for submitter in submitters: + Submitter.objects.add(submitter, motion) + # Write the log message and initiate response. motion.write_log([ugettext_noop('Motion created')], request.user) @@ -264,10 +301,9 @@ class MotionViewSet(ModelViewSet): [ugettext_noop('Comment {} updated').format(', '.join(changed_comment_fields))], request.user) - # Send new submitters and supporters via autoupdate because users + # Send new supporters via autoupdate because users # without permission to see users may not have them but can get it now. - new_users = list(updated_motion.submitters.all()) - new_users.extend(updated_motion.supporters.all()) + new_users = list(updated_motion.supporters.all()) inform_changed_data(new_users) return Response(serializer.data) @@ -316,6 +352,106 @@ class MotionViewSet(ModelViewSet): # Initiate response. return Response({'detail': message}) + @detail_route(methods=['POST', 'DELETE']) + def manage_submitters(self, request, pk=None): + """ + POST: Add a user as a submitter to this motion. + DELETE: Remove the user as a submitter from this motion. + For both cases provide ['user': } for the user to add or remove. + """ + motion = self.get_object() + + if request.method == 'POST': + user_id = request.data.get('user') + + # Check permissions and other conditions. Get user instance. + if user_id is None: + raise ValidationError({'detail': _('You have to provide a user.')}) + else: + try: + user = get_user_model().objects.get(pk=int(user_id)) + except (ValueError, get_user_model().DoesNotExist): + raise ValidationError({'detail': _('User does not exist.')}) + + # Try to add the user. This ensurse that a user is not twice a submitter + try: + Submitter.objects.add(user, motion) + except OpenSlidesError as e: + raise ValidationError({'detail': str(e)}) + message = _('User %s was successfully added as a submitter.') % user + + # Send new submitter via autoupdate because users without permission + # to see users may not have it but can get it now. + inform_changed_data(user) + + else: # DELETE + user_id = request.data.get('user') + + # Check permissions and other conditions. Get user instance. + if user_id is None: + raise ValidationError({'detail': _('You have to provide a user.')}) + else: + try: + user = get_user_model().objects.get(pk=int(user_id)) + except (ValueError, get_user_model().DoesNotExist): + raise ValidationError({'detail': _('User does not exist.')}) + + queryset = Submitter.objects.filter(motion=motion, user=user) + 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 submitter instance or none. + submitter = queryset.get() + except Submitter.DoesNotExist: + raise ValidationError({'detail': _('The user is not a submitter.')}) + else: + name = str(submitter.user) + submitter.delete() + message = _('User {} successfully removed as a submitter.').format(name) + + # Initiate response. + return Response({'detail': message}) + + @detail_route(methods=['POST']) + def sort_submitters(self, request, pk=None): + """ + Special view endpoint to sort the submitters. + Send {'submitters': [, , ...]} as payload. + """ + # Retrieve motion. + motion = self.get_object() + + # Check data + submitter_ids = request.data.get('submitters') + if not isinstance(submitter_ids, list): + raise ValidationError( + {'detail': _('Invalid data.')}) + + # Get all submitters + submitters = {} + for submitter in motion.submitters.all(): + submitters[submitter.pk] = submitter + + # Check and sort submitters + valid_submitters = [] + for submitter_id in submitter_ids: + if not isinstance(submitter_id, int) or submitters.get(submitter_id) is None: + raise ValidationError( + {'detail': _('Invalid data.')}) + valid_submitters.append(submitters[submitter_id]) + weight = 1 + with transaction.atomic(): + for submitter in valid_submitters: + submitter.weight = weight + submitter.save(skip_autoupdate=True) + weight += 1 + + # send autoupdate + inform_changed_data(motion) + + # Initiate response. + return Response({'detail': _('Submitters successfully sorted.')}) + @detail_route(methods=['post', 'delete']) def support(self, request, pk=None): """ diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index bbebe4697..9b1558627 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -15,6 +15,7 @@ from openslides.motions.models import ( MotionBlock, MotionLog, State, + Submitter, Workflow, ) from openslides.users.models import Group @@ -175,7 +176,7 @@ class CreateMotion(TestCase): self.assertEqual(motion.title, 'test_title_OoCoo3MeiT9li5Iengu9') self.assertEqual(motion.identifier, '1') self.assertTrue(motion.submitters.exists()) - self.assertEqual(motion.submitters.get().username, 'admin') + self.assertEqual(motion.submitters.get().user.username, 'admin') def test_with_reason(self): response = self.client.post( @@ -467,14 +468,15 @@ class RetrieveMotion(TestCase): user = get_user_model().objects.create_user( username='username_ohS2opheikaSa5theijo', password='password_kau4eequaisheeBateef') - self.motion.submitters.add(user) + Submitter.objects.add(user, self.motion) submitter_client = APIClient() submitter_client.force_login(user) response = submitter_client.get(reverse('motion-detail', args=[self.motion.pk])) self.assertEqual(response.status_code, status.HTTP_200_OK) def test_user_without_can_see_user_permission_to_see_motion_and_submitter_data(self): - self.motion.submitters.add(get_user_model().objects.get(username='admin')) + admin = get_user_model().objects.get(username='admin') + Submitter.objects.add(admin, self.motion) group = Group.objects.get(pk=1) # Group with pk 1 is for anonymous and default users. permission_string = 'users.can_see_name' app_label, codename = permission_string.split('.') @@ -486,7 +488,8 @@ class RetrieveMotion(TestCase): response_1 = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) self.assertEqual(response_1.status_code, status.HTTP_200_OK) - response_2 = guest_client.get(reverse('user-detail', args=[response_1.data['submitters_id'][0]])) + submitter_id = response_1.data['submitters'][0]['user_id'] + response_2 = guest_client.get(reverse('user-detail', args=[submitter_id])) self.assertEqual(response_2.status_code, status.HTTP_200_OK) extra_user = get_user_model().objects.create_user( @@ -552,7 +555,7 @@ class UpdateMotion(TestCase): username='test_username_uqu6PhoovieB9eicah0o', password='test_password_Xaesh8ohg6CoheTe3awo') motion = Motion.objects.get() - motion.submitters.add(non_admin) + Submitter.objects.add(non_admin, self.motion) motion.supporters.clear() response = self.client.patch( reverse('motion-detail', args=[self.motion.pk]), @@ -566,7 +569,7 @@ class UpdateMotion(TestCase): admin = get_user_model().objects.get(username='admin') group_admin = admin.groups.get(name='Admin') admin.groups.remove(group_admin) - self.motion.submitters.add(admin) + Submitter.objects.add(admin, self.motion) supporter = get_user_model().objects.create_user( username='test_username_ahshi4oZin0OoSh9chee', password='test_password_Sia8ahgeenixu5cei2Ib') @@ -678,7 +681,7 @@ class DeleteMotion(TestCase): def test_delete_own_motion_as_delegate(self): self.make_admin_delegate() self.put_motion_in_complex_workflow() - self.motion.submitters.add(self.admin) + Submitter.objects.add(self.admin, self.motion) response = self.client.delete( reverse('motion-detail', args=[self.motion.pk])) @@ -732,6 +735,118 @@ class ManageVersion(TestCase): self.assertEqual(response.data, {'detail': 'You can not delete the active version of a motion.'}) +class ManageSubmitters(TestCase): + """ + Tests adding and removing of submitters. + """ + def setUp(self): + self.client = APIClient() + self.client.login(username='admin', password='admin') + + self.admin = get_user_model().objects.get() + self.motion = Motion( + title='test_title_SlqfMw(waso0saWMPqcZ', + text='test_text_f30skclqS9wWF=xdfaSL') + self.motion.save() + + def test_add_existing_user(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + self.assertEqual(response.status_code, 200) + self.assertEqual(self.motion.submitters.count(), 1) + + def test_add_non_existing_user(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': 1337}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_add_user_twice(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 1) + + def test_add_user_no_data(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk])) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_add_user_invalid_data(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': ['invalid_str']}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_add_without_permission(self): + admin = get_user_model().objects.get(username='admin') + group_admin = admin.groups.get(name='Admin') + group_delegates = type(group_admin).objects.get(name='Delegates') + admin.groups.add(group_delegates) + admin.groups.remove(group_admin) + CollectionElement.from_instance(admin) + + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + self.assertEqual(response.status_code, 403) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_remove_existing_user(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + self.assertEqual(response.status_code, 200) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_remove_non_existing_user(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': 1337}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 1) + + def test_remove_existing_user_twice(self): + response = self.client.post( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': self.admin.pk}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_remove_user_no_data(self): + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk])) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + def test_remove_user_invalid_data(self): + response = self.client.delete( + reverse('motion-manage-submitters', args=[self.motion.pk]), + {'user': ['invalid_str']}) + self.assertEqual(response.status_code, 400) + self.assertEqual(self.motion.submitters.count(), 0) + + class CreateMotionChangeRecommendation(TestCase): """ Tests motion change recommendation creation. diff --git a/tests/unit/motions/test_views.py b/tests/unit/motions/test_views.py index 21b841c44..fd2a2ed53 100644 --- a/tests/unit/motions/test_views.py +++ b/tests/unit/motions/test_views.py @@ -11,6 +11,7 @@ class MotionViewSetCreate(TestCase): def setUp(self): self.request = MagicMock() self.request.data.get.return_value = None + self.request.user.is_authenticated.return_value = False self.view_instance = MotionViewSet() self.view_instance.request = self.request self.view_instance.format_kwarg = MagicMock() @@ -21,7 +22,6 @@ class MotionViewSetCreate(TestCase): @patch('openslides.motions.views.has_perm') @patch('openslides.motions.views.config') def test_simple_create(self, mock_config, mock_has_perm, mock_icd): - self.request.user = 1 mock_has_perm.return_value = True self.view_instance.create(self.request)