Motion rework

- remove motion version
- migrations for versions and change recommendations
- Redone motion comments. Wording changed from comment fields to comment
  sections
- fixed test order, tests are not atomic
- introduce get_group_model. Just use OpenSlides Groups and not the
django's ones.

.travis.yml

@@ -20,8 +20,8 @@ install:
   - pip freeze
   - cd client && npm install && cd ..
 script:
+  - cd client && npm run-script lint && cd ..
   - flake8 openslides tests
   - isort --check-only --diff --recursive openslides tests
   - python -m mypy openslides/
-  - pytest --cov --cov-fail-under=70
+  - pytest tests/old/ tests/integration/ tests/unit/ --cov --cov-fail-under=75
-  - cd client && npm run-script lint && cd ..

client/src/app/shared/models/motions/motion.ts

@@ -1,5 +1,4 @@
 import { BaseModel } from '../base.model';
-import { MotionVersion } from './motion-version';
 import { MotionSubmitter } from './motion-submitter';
 import { MotionLog } from './motion-log';
 import { Config } from '../core/config';
@@ -19,18 +18,23 @@ export class Motion extends BaseModel {
     protected _collectionString: string;
     public id: number;
     public identifier: string;
-    public versions: MotionVersion[];
+    public title: string;
-    public active_version: number;
+    public text: string;
+    public reason: string;
+    public amendment_paragraphs: string;
+    public modified_final_version: string;
     public parent_id: number;
     public category_id: number;
     public motion_block_id: number;
     public origin: string;
     public submitters: MotionSubmitter[];
     public supporters_id: number[];
-    public comments: Object;
+    public comments: Object[];
     public state_id: number;
+    public state_extension: string;
     public state_required_permission_to_see: string;
     public recommendation_id: number;
+    public recommendation_extension: string;
     public tags_id: number[];
     public attachments_id: number[];
     public polls: BaseModel[];
@@ -40,15 +44,14 @@ export class Motion extends BaseModel {
     // dynamic values
     public workflow: Workflow;
 
-    // for request
-    public title: string;
-    public text: string;
-
     public constructor(input?: any) {
         super();
         this._collectionString = 'motions/motion';
         this.identifier = '';
-        this.versions = [new MotionVersion()];
+        this.title = '';
+        this.text = '';
+        this.reason = '';
+        this.modified_final_version = '';
         this.origin = '';
         this.submitters = [];
         this.supporters_id = [];
@@ -101,62 +104,6 @@ export class Motion extends BaseModel {
         console.log('did addSubmitter. this.submitters: ', this.submitters);
     }
 
-    /**
-     * returns the most current title from versions
-     */
-    public get currentTitle(): string {
-        if (this.versions && this.versions[0]) {
-            return this.versions[0].title;
-        } else {
-            return '';
-        }
-    }
-
-    /**
-     * Patch the current version
-     *
-     * TODO: Altering the current version should be avoided.
-     */
-    public set currentTitle(newTitle: string) {
-        if (this.versions[0]) {
-            this.versions[0].title = newTitle;
-        }
-    }
-
-    /**
-     * returns the most current motion text from versions
-     */
-    public get currentText() {
-        if (this.versions) {
-            return this.versions[0].text;
-        } else {
-            return null;
-        }
-    }
-
-    public set currentText(newText: string) {
-        this.versions[0].text = newText;
-    }
-
-    /**
-     * returns the most current motion reason text from versions
-     */
-    public get currentReason() {
-        if (this.versions) {
-            return this.versions[0].reason;
-        } else {
-            return null;
-        }
-    }
-
-    /**
-     * Update the current reason.
-     * TODO: ignores motion versions. Should make a new one.
-     */
-    public set currentReason(newReason: string) {
-        this.versions[0].reason = newReason;
-    }
-
     /**
      * return the submitters as uses objects
      */
@@ -239,13 +186,6 @@ export class Motion extends BaseModel {
     public deserialize(input: any): void {
         Object.assign(this, input);
 
-        if (input.versions instanceof Array) {
-            this.versions = [];
-            input.versions.forEach(motionVersionData => {
-                this.versions.push(new MotionVersion(motionVersionData));
-            });
-        }
-
         if (input.submitters instanceof Array) {
             this.submitters = [];
             input.submitters.forEach(SubmitterData => {

client/src/app/site/motions/motion-detail/motion-detail.component.html

@@ -12,8 +12,8 @@
         <span *ngIf="motion && !editMotion"> {{motion.identifier}}</span>
         <span *ngIf="editMotion && !newMotion"> {{metaInfoForm.get('identifier').value}}</span>
         <span>:</span>
-        <span *ngIf="motion && !editMotion"> {{motion.currentTitle}}</span>
+        <span *ngIf="motion && !editMotion"> {{motion.title}}</span>
-        <span *ngIf="editMotion"> {{contentForm.get('currentTitle').value}}</span>
+        <span *ngIf="editMotion"> {{contentForm.get('title').value}}</span>
         <br>
         <div *ngIf="motion" class='motion-submitter'>
             <span translate>by</span> {{motion.submitterAsUser}}
@@ -223,12 +223,12 @@
     <form [formGroup]='contentForm' (ngSubmit)='saveMotion()'>
 
         <!-- Title -->
-        <div *ngIf="motion && motion.currentTitle || editMotion">
+        <div *ngIf="motion && motion.title || editMotion">
             <div *ngIf='!editMotion'>
-                <h2>{{motion.currentTitle}}</h2>
+                <h2>{{motion.title}}</h2>
             </div>
             <mat-form-field *ngIf="editMotion" class="wide-form">
-                <input matInput placeholder='Title' formControlName='currentTitle' [value]='motionCopy.currentTitle'>
+                <input matInput placeholder='Title' formControlName='title' [value]='motionCopy.title'>
             </mat-form-field>
 
         </div>
@@ -237,21 +237,21 @@
         <!-- TODO: this is a config variable. Read it out -->
         <h3 translate>The assembly may decide:</h3>
         <div *ngIf='motion && !editMotion'>
-            <div [innerHtml]='motion.currentText'></div>
+            <div [innerHtml]='motion.text'></div>
         </div>
         <mat-form-field *ngIf="motion && editMotion" class="wide-form">
-            <textarea matInput placeholder='Motion Text' formControlName='currentText' [value]='motionCopy.currentText'></textarea>
+            <textarea matInput placeholder='Motion Text' formControlName='text' [value]='motionCopy.text'></textarea>
         </mat-form-field>
 
 
         <!-- Reason -->
-        <div *ngIf="motion && motion.currentReason || editMotion">
+        <div *ngIf="motion && motion.reason || editMotion">
             <div *ngIf='!editMotion'>
                 <h4 translate>Reason</h4>
-                <div [innerHtml]='motion.currentReason'></div>
+                <div [innerHtml]='motion.reason'></div>
             </div>
             <mat-form-field *ngIf="editMotion" class="wide-form">
-                <textarea matInput placeholder="Reason" formControlName='currentReason' [value]='motionCopy.currentReason'></textarea>
+                <textarea matInput placeholder="Reason" formControlName='reason' [value]='motionCopy.reason'></textarea>
             </mat-form-field>
         </div>
 

client/src/app/site/motions/motion-detail/motion-detail.component.ts

@@ -114,9 +114,9 @@ export class MotionDetailComponent extends BaseComponent implements OnInit {
             origin: formMotion.origin
         });
         this.contentForm.patchValue({
-            currentTitle: formMotion.currentTitle,
+            title: formMotion.title,
-            currentText: formMotion.currentText,
+            text: formMotion.text,
-            currentReason: formMotion.currentReason
+            reason: formMotion.reason
         });
     }
 
@@ -134,9 +134,9 @@ export class MotionDetailComponent extends BaseComponent implements OnInit {
             origin: ['']
         });
         this.contentForm = this.formBuilder.group({
-            currentTitle: ['', Validators.required],
+            title: ['', Validators.required],
-            currentText: ['', Validators.required],
+            text: ['', Validators.required],
-            currentReason: ['']
+            reason: ['']
         });
     }
 
@@ -153,10 +153,6 @@ export class MotionDetailComponent extends BaseComponent implements OnInit {
         const newMotionValues = { ...this.metaInfoForm.value, ...this.contentForm.value };
         this.motionCopy.patchValues(newMotionValues);
 
-        // TODO: This is DRAFT. Reads out Motion version directly. Potentially insecure.
-        this.motionCopy.title = this.motionCopy.currentTitle;
-        this.motionCopy.text = this.motionCopy.currentText;
-
         // TODO: send to normal motion to verify
         this.dataSend.saveModel(this.motionCopy).subscribe(answer => {
             if (answer && answer.id && this.newMotion) {

client/src/app/site/start/start.component.ts

@@ -8,7 +8,6 @@ import { TranslateService } from '@ngx-translate/core'; // showcase
 import { User } from 'app/shared/models/users/user';
 import { Config } from '../../shared/models/core/config';
 import { Motion } from '../../shared/models/motions/motion';
-import { MotionVersion } from '../../shared/models/motions/motion-version';
 import { MotionSubmitter } from '../../shared/models/motions/motion-submitter';
 
 @Component({
@@ -152,15 +151,6 @@ export class StartComponent extends BaseComponent implements OnInit {
         `;
 
         for (let i = 1; i <= requiredMotions; ++i) {
-            // version
-            const newMotionVersion = new MotionVersion({
-                id: 200 + i,
-                version_number: 1,
-                create_time: 'now',
-                title: 'GenMo ' + i,
-                text: longMotionText,
-                reason: longMotionText
-            });
             // submitter
             const newMotionSubmitter = new MotionSubmitter({
                 id: 1,
@@ -172,7 +162,9 @@ export class StartComponent extends BaseComponent implements OnInit {
             const newMotion = new Motion({
                 id: 200 + i,
                 identifier: 'GenMo ' + i,
-                versions: [newMotionVersion],
+                title: 'title',
+                text: longMotionText,
+                reason: longMotionText,
                 origin: 'Generated',
                 submitters: [newMotionSubmitter],
                 state_id: 1

openslides/core/config.py

@@ -27,7 +27,6 @@ INPUT_TYPE_MAPPING = {
     'integer': int,
     'boolean': bool,
     'choice': str,
-    'comments': dict,
     'colorpicker': str,
     'datetimepicker': int,
     'majorityMethod': str,
@@ -133,31 +132,6 @@ class ConfigHandler:
             except DjangoValidationError as e:
                 raise ConfigError(e.messages[0])
 
-        if config_variable.input_type == 'comments':
-            if not isinstance(value, dict):
-                raise ConfigError(_('motions_comments has to be a dict.'))
-            valuecopy = dict()
-            for id, commentsfield in value.items():
-                try:
-                    id = int(id)
-                except ValueError:
-                    raise ConfigError(_('Each id has to be an int.'))
-
-                if id < 1:
-                    raise ConfigError(_('Each id has to be greater then 0.'))
-                # Deleted commentsfields are saved as None to block the used ids
-                if commentsfield is not None:
-                    if not isinstance(commentsfield, dict):
-                        raise ConfigError(_('Each commentsfield in motions_comments has to be a dict.'))
-                    if commentsfield.get('name') is None or commentsfield.get('public') is None:
-                        raise ConfigError(_('A name and a public property have to be given.'))
-                    if not isinstance(commentsfield['name'], str):
-                        raise ConfigError(_('name has to be string.'))
-                    if not isinstance(commentsfield['public'], bool):
-                        raise ConfigError(_('public property has to be bool.'))
-                valuecopy[id] = commentsfield
-            value = valuecopy
-
         if config_variable.input_type == 'static':
             if not isinstance(value, dict):
                 raise ConfigError(_('This has to be a dict.'))

openslides/global_settings.py

@@ -99,6 +99,8 @@ STATICFILES_DIRS = [
 
 AUTH_USER_MODEL = 'users.User'
 
+AUTH_GROUP_MODEL = 'users.Group'
+
 SESSION_COOKIE_NAME = 'OpenSlidesSessionID'
 
 SESSION_EXPIRE_AT_BROWSER_CLOSE = True

openslides/motions/access_permissions.py

@@ -1,9 +1,8 @@
 from copy import deepcopy
 from typing import Any, Dict, List, Optional
 
-from ..core.config import config
 from ..utils.access_permissions import BaseAccessPermissions
-from ..utils.auth import has_perm
+from ..utils.auth import has_perm, in_some_groups
 from ..utils.collection import CollectionElement
 
 
@@ -32,7 +31,7 @@ class MotionAccessPermissions(BaseAccessPermissions):
         """
         Returns the restricted serialized data for the instance prepared for
         the user. Removes motion if the user has not the permission to see
-        the motion in this state. Removes non public comment fields for
+        the motion in this state. Removes comments sections for
         some unauthorized users. Ensures that a user can only see his own
         personal notes.
         """
@@ -59,21 +58,12 @@ class MotionAccessPermissions(BaseAccessPermissions):
 
                 # Parse single motion.
                 if permission:
-                    if has_perm(user, 'motions.can_see_comments') or not full.get('comments'):
+                    full_copy = deepcopy(full)
-                        # Provide access to all fields.
+                    full_copy['comments'] = []
-                        motion = full
+                    for comment in full['comments']:
-                    else:
+                        if in_some_groups(user, comment['read_groups_id']):
-                        # Set private comment fields to None.
+                            full_copy['comments'].append(comment)
-                        full_copy = deepcopy(full)
+                    data.append(full_copy)
-                        for i, field in config['motions_comments'].items():
-                            if field is None or not field.get('public'):
-                                try:
-                                    full_copy['comments'][i] = None
-                                except IndexError:
-                                    # No data in range. Just do nothing.
-                                    pass
-                        motion = full_copy
-                    data.append(motion)
         else:
             data = []
 
@@ -82,25 +72,13 @@ class MotionAccessPermissions(BaseAccessPermissions):
     def get_projector_data(self, full_data: List[Dict[str, Any]]) -> List[Dict[str, Any]]:
         """
         Returns the restricted serialized data for the instance prepared
-        for the projector. Removes several comment fields.
+        for the projector. Removes all comments.
         """
-        # Parse data.
         data = []
         for full in full_data:
-            # Set private comment fields to None.
+            full_copy = deepcopy(full)
-            if full.get('comments') is not None:
+            full_copy['comments'] = []
-                full_copy = deepcopy(full)
+            data.append(full_copy)
-                for i, field in config['motions_comments'].items():
-                    if field is None or not field.get('public'):
-                        try:
-                            full_copy['comments'][i] = None
-                        except IndexError:
-                            # No data in range. Just do nothing.
-                            pass
-                data.append(full_copy)
-            else:
-                data.append(full)
-
         return data
 
 
@@ -123,6 +101,43 @@ class MotionChangeRecommendationAccessPermissions(BaseAccessPermissions):
         return MotionChangeRecommendationSerializer
 
 
+class MotionCommentSectionAccessPermissions(BaseAccessPermissions):
+    """
+    Access permissions container for MotionCommentSection and MotionCommentSectionViewSet.
+    """
+    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 MotionCommentSectionSerializer
+
+        return MotionCommentSectionSerializer
+
+    def get_restricted_data(
+            self,
+            full_data: List[Dict[str, Any]],
+            user: Optional[CollectionElement]) -> List[Dict[str, Any]]:
+        """
+        If the user has manage rights, he can see all sections. If not all sections
+        will be removed, when the user is not in at least one of the read_groups.
+        """
+        data: List[Dict[str, Any]] = []
+        if has_perm(user, 'motions.can_manage'):
+            data = full_data
+        else:
+            for full in full_data:
+                read_groups = full.get('read_groups_id', [])
+                if in_some_groups(user, read_groups):
+                    data.append(full)
+        return data
+
+
 class CategoryAccessPermissions(BaseAccessPermissions):
     """
     Access permissions container for Category and CategoryViewSet.

openslides/motions/apps.py

@@ -25,6 +25,7 @@ class MotionsAppConfig(AppConfig):
         from .views import (
             CategoryViewSet,
             MotionViewSet,
+            MotionCommentSectionViewSet,
             MotionBlockViewSet,
             MotionPollViewSet,
             MotionChangeRecommendationViewSet,
@@ -51,6 +52,7 @@ class MotionsAppConfig(AppConfig):
         router.register(self.get_model('Category').get_collection_string(), CategoryViewSet)
         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)
         router.register(self.get_model('Workflow').get_collection_string(), WorkflowViewSet)
         router.register(self.get_model('MotionChangeRecommendation').get_collection_string(),
                         MotionChangeRecommendationViewSet)
@@ -62,5 +64,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'):
+        for model_name in ('Category', 'Motion', 'MotionBlock', 'Workflow',
+                           'MotionChangeRecommendation', 'MotionCommentSection'):
             yield self.get_model(model_name)

openslides/motions/config_variables.py

@@ -106,15 +106,6 @@ def get_config_variables():
         group='Motions',
         subgroup='General')
 
-    yield ConfigVariable(
-        name='motions_allow_disable_versioning',
-        default_value=False,
-        input_type='boolean',
-        label='Allow to disable versioning',
-        weight=329,
-        group='Motions',
-        subgroup='General')
-
     yield ConfigVariable(
         name='motions_stop_submitting',
         default_value=False,
@@ -210,17 +201,6 @@ def get_config_variables():
         group='Motions',
         subgroup='Supporters')
 
-    # Comments
-
-    yield ConfigVariable(
-        name='motions_comments',
-        default_value={},
-        input_type='comments',
-        label='Comment fields for motions',
-        weight=353,
-        group='Motions',
-        subgroup='Comments')
-
     # Voting and ballot papers
 
     yield ConfigVariable(

openslides/motions/migrations/0011_motion_version.py

@@ -0,0 +1,131 @@
+# Generated by Django 2.1 on 2018-08-31 13:17
+
+import django.db.models.deletion
+import jsonfield.encoder
+import jsonfield.fields
+from django.db import migrations, models
+
+
+def copy_motion_version_content_to_motion(apps, schema_editor):
+    """
+    Move all motion version content of the active version to the motion.
+    """
+    Motion = apps.get_model('motions', 'Motion')
+
+    for motion in Motion.objects.all():
+        motion.title = motion.active_version.title
+        motion.text = motion.active_version.text
+        motion.reason = motion.active_version.reason
+        motion.modified_final_version = motion.active_version.modified_final_version
+        motion.amendment_paragraphs = motion.active_version.amendment_paragraphs
+        motion.save(skip_autoupdate=True)
+
+
+def migrate_active_change_recommendations(apps, schema_editor):
+    """
+    Delete all change recommendation of motion versions, that are not active. For active
+    change recommendations the motion id will be set.
+    """
+    MotionChangeRecommendation = apps.get_model('motions', 'MotionChangeRecommendation')
+    to_delete = []
+    for cr in MotionChangeRecommendation.objects.all():
+        # chack if version id matches the active version of the motion
+        if cr.motion_version.id == cr.motion_version.motion.active_version.id:
+            cr.motion = cr.motion_version.motion
+            cr.save(skip_autoupdate=True)
+        else:
+            to_delete.append(cr)
+
+    # delete non active change recommendations
+    for cr in to_delete:
+        cr.delete(skip_autoupdate=True)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('motions', '0010_auto_20180822_1042'),
+    ]
+
+    operations = [
+        # Create new fields. Title and Text have empty defaults, but the values
+        # should be overwritten by copy_motion_version_content_to_motion. In the next
+        # migration file these defaults are removed.
+        migrations.AddField(
+            model_name='motion',
+            name='title',
+            field=models.CharField(max_length=255, default=''),
+        ),
+        migrations.AddField(
+            model_name='motion',
+            name='text',
+            field=models.TextField(default=''),
+        ),
+        migrations.AddField(
+            model_name='motion',
+            name='reason',
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='motion',
+            name='modified_final_version',
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='motion',
+            name='amendment_paragraphs',
+            field=jsonfield.fields.JSONField(
+                dump_kwargs={
+                    'cls': jsonfield.encoder.JSONEncoder,
+                    'separators': (',', ':')
+                },
+                load_kwargs={},
+                null=True),
+        ),
+
+        # Copy old motion version data
+        migrations.RunPython(copy_motion_version_content_to_motion),
+
+        # Change recommendations
+        migrations.AddField(
+            model_name='motionchangerecommendation',
+            name='motion',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                null=True,  # This is reverted in the next migration
+                related_name='change_recommendations',
+                to='motions.Motion'),
+        ),
+        migrations.RunPython(migrate_active_change_recommendations),
+        migrations.RemoveField(
+            model_name='motionchangerecommendation',
+            name='motion_version',
+        ),
+
+        # remove motion version references from motion and state.
+        migrations.RemoveField(
+            model_name='motion',
+            name='active_version',
+        ),
+        migrations.AlterUniqueTogether(
+            name='motionversion',
+            unique_together=set(),
+        ),
+        migrations.RemoveField(
+            model_name='motionversion',
+            name='motion',
+        ),
+        migrations.RemoveField(
+            model_name='state',
+            name='leave_old_version_active',
+        ),
+        migrations.RemoveField(
+            model_name='state',
+            name='versioning',
+        ),
+
+        # Delete motion version.
+        migrations.DeleteModel(
+            name='MotionVersion',
+        ),
+    ]

openslides/motions/migrations/0012_motion_comments.py

@@ -0,0 +1,218 @@
+# Generated by Django 2.1 on 2018-08-31 13:17
+
+import django.db.models.deletion
+from django.conf import settings
+from django.contrib.auth.models import Permission
+from django.db import migrations, models
+
+import openslides
+
+
+def create_comment_sections_from_config_and_move_comments_to_own_model(apps, schema_editor):
+    ConfigStore = apps.get_model('core', 'ConfigStore')
+    Motion = apps.get_model('motions', 'Motion')
+    MotionComment = apps.get_model('motions', 'MotionComment')
+    MotionCommentSection = apps.get_model('motions', 'MotionCommentSection')
+    Group = apps.get_model(settings.AUTH_GROUP_MODEL)
+
+    # try to get old motions_comments config variable, where all comment fields are saved
+    try:
+        motions_comments = ConfigStore.objects.get(key='motions_comments')
+    except ConfigStore.DoesNotExist:
+        return
+    comments_sections = motions_comments.value
+
+    # Delete config value
+    motions_comments.delete()
+
+    # Get can_see_comments and can_manage_comments permissions and the associated groups
+    can_see_comments = Permission.objects.filter(codename='can_see_comments')
+    if len(can_see_comments) == 1:
+        # Save groups. list() is necessary to evaluate the database query right now.
+        can_see_groups = list(can_see_comments.get().group_set.all())
+    else:
+        can_see_groups = Group.objects.all()
+
+    can_manage_comments = Permission.objects.filter(codename='can_manage_comments')
+    if len(can_manage_comments) == 1:
+        # Save groups. list() is necessary to evaluate the database query right now.
+        can_manage_groups = list(can_manage_comments.get().group_set.all())
+    else:
+        can_manage_groups = Group.objects.all()
+
+    # Create comment sections. Map them to the old ids, so we can find the right section
+    # when creating actual comments
+    old_id_mapping = {}
+    # Keep track of the special comment sections "forState" and "forRecommendation". If a
+    # comment is found, the comment value will be assigned to new motion fields and not comments.
+    forStateId = None
+    forRecommendationId = None
+    for id, section in comments_sections.items():
+        if section is None:
+            continue
+        if section.get('forState', False):
+            forStateId = id
+        elif section.get('forRecommendation', False):
+            forRecommendationId = id
+        else:
+            comment_section = MotionCommentSection(name=section['name'])
+            comment_section.save(skip_autoupdate=True)
+            comment_section.read_groups.add(*[group.id for group in can_see_groups])
+            comment_section.write_groups.add(*[group.id for group in can_manage_groups])
+            old_id_mapping[id] = comment_section
+
+    # Create all comments objects
+    comments = []
+    for motion in Motion.objects.all():
+        if not isinstance(motion.comments, dict):
+            continue
+
+        for section_id, comment_value in motion.comments.items():
+            # Skip empty sections.
+            comment_value = comment_value.strip()
+            if comment_value == '':
+                continue
+            # Special comments will be moved to separate fields.
+            if section_id == forStateId:
+                motion.state_extension = comment_value
+                motion.save(skip_autoupdate=True)
+            elif section_id == forRecommendationId:
+                motion.recommendation_extension = comment_value
+                motion.save(skip_autoupdate=True)
+            else:
+                comment = MotionComment(
+                    comment=comment_value,
+                    motion=motion,
+                    section=old_id_mapping[section_id])
+                comments.append(comment)
+    MotionComment.objects.bulk_create(comments)
+
+
+class Migration(migrations.Migration):
+
+    dependencies = [
+        ('users', '0006_user_email'),
+        ('motions', '0011_motion_version'),
+    ]
+
+    operations = [
+        # Cleanup from last migration. Somehow cannot be done there.
+        migrations.AlterField(  # remove default=''
+            model_name='motion',
+            name='text',
+            field=models.TextField(),
+        ),
+        migrations.AlterField(  # remove default=''
+            model_name='motion',
+            name='title',
+            field=models.CharField(max_length=255),
+        ),
+        migrations.AlterField(  # remove null=True
+            model_name='motionchangerecommendation',
+            name='motion',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name='change_recommendations',
+                to='motions.Motion'),
+        ),
+
+        # Add extension fields for former "special comments". No hack anymore..
+        migrations.AddField(
+            model_name='motion',
+            name='recommendation_extension',
+            field=models.TextField(blank=True, null=True),
+        ),
+        migrations.AddField(
+            model_name='motion',
+            name='state_extension',
+            field=models.TextField(blank=True, null=True),
+        ),
+
+        migrations.AlterModelOptions(
+            name='motion',
+            options={
+                'default_permissions': (),
+                'ordering': ('identifier',),
+                'permissions': (
+                    ('can_see', 'Can see motions'),
+                    ('can_create', 'Can create motions'),
+                    ('can_support', 'Can support motions'),
+                    ('can_manage', 'Can manage motions')),
+                'verbose_name': 'Motion'},
+        ),
+        # Comments and CommentsSection models
+        migrations.CreateModel(
+            name='MotionComment',
+            fields=[
+                ('id', models.AutoField(
+                        auto_created=True,
+                        primary_key=True,
+                        serialize=False,
+                        verbose_name='ID')),
+                ('comment', models.TextField()),
+            ],
+            options={
+                'default_permissions': (),
+            },
+            bases=(openslides.utils.models.RESTModelMixin, models.Model),  # type: ignore
+        ),
+        migrations.CreateModel(
+            name='MotionCommentSection',
+            fields=[
+                ('id', models.AutoField(
+                    auto_created=True,
+                    primary_key=True,
+                    serialize=False,
+                    verbose_name='ID')),
+                ('name', models.CharField(max_length=255)),
+                ('read_groups', models.ManyToManyField(
+                    blank=True,
+                    related_name='read_comments',
+                    to=settings.AUTH_GROUP_MODEL)),
+                ('write_groups', models.ManyToManyField(
+                    blank=True,
+                    related_name='write_comments',
+                    to=settings.AUTH_GROUP_MODEL)),
+            ],
+            options={
+                'default_permissions': (),
+            },
+            bases=(openslides.utils.models.RESTModelMixin, models.Model),  # type: ignore
+        ),
+        migrations.AddField(
+            model_name='motioncomment',
+            name='section',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.PROTECT,
+                related_name='comments',
+                to='motions.MotionCommentSection'),
+        ),
+        migrations.AddField(
+            model_name='motioncomment',
+            name='motion',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                to='motions.Motion'),
+        ),
+        migrations.AlterUniqueTogether(
+            name='motioncomment',
+            unique_together={('motion', 'section')},
+        ),
+
+        # Move the comments and sections
+        migrations.RunPython(create_comment_sections_from_config_and_move_comments_to_own_model),
+
+        # Remove old comment field from motion, use the new model instead
+        migrations.RemoveField(
+            model_name='motion',
+            name='comments',
+        ),
+        migrations.AlterField(
+            model_name='motioncomment',
+            name='motion',
+            field=models.ForeignKey(
+                on_delete=django.db.models.deletion.CASCADE,
+                related_name='comments',
+                to='motions.Motion'),
+        ),
+    ]

openslides/motions/models.py

@@ -7,11 +7,7 @@ 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
-from django.utils.translation import (
+from django.utils.translation import ugettext as _, ugettext_noop
-    ugettext as _,
-    ugettext_lazy,
-    ugettext_noop,
-)
 from jsonfield import JSONField
 
 from openslides.agenda.models import Item
@@ -33,6 +29,7 @@ from .access_permissions import (
     MotionAccessPermissions,
     MotionBlockAccessPermissions,
     MotionChangeRecommendationAccessPermissions,
+    MotionCommentSectionAccessPermissions,
     WorkflowAccessPermissions,
 )
 from .exceptions import WorkflowError
@@ -48,10 +45,12 @@ class MotionManager(models.Manager):
         join and prefetch all related models.
         """
         return (self.get_queryset()
-                .select_related('active_version', 'state')
+                .select_related('state')
                 .prefetch_related(
                     'state__workflow',
-                    'versions',
+                    'comments',
+                    'comments__section',
+                    'comments__section__read_groups',
                     'agenda_items',
                     'log_messages',
                     'polls',
@@ -71,18 +70,26 @@ class Motion(RESTModelMixin, models.Model):
 
     objects = MotionManager()
 
-    active_version = models.ForeignKey(
+    title = models.CharField(max_length=255)
-        'MotionVersion',
+    """The title of a motion."""
-        on_delete=models.SET_NULL,
-        null=True,
-        related_name="active_version")
-    """
-    Points to a specific version.
 
-    Used be the permitted-version-system to deside which version is the active
+    text = models.TextField()
-    version. Could also be used to only choose a specific version as a default
+    """The text of a motion."""
-    version. Like the sighted versions on Wikipedia.
+
+    amendment_paragraphs = JSONField(null=True)
     """
+    If paragraph-based, diff-enabled amendment style is used, this field stores an array of strings or null values.
+    Each entry corresponds to a paragraph of the text of the original motion.
+    If the entry is null, then the paragraph remains unchanged.
+    If the entry is a string, this is the new text of the paragraph.
+    amendment_paragraphs and text are mutually exclusive.
+    """
+
+    modified_final_version = models.TextField(null=True, blank=True)
+    """A field to copy in the final version of the motion and edit it there."""
+
+    reason = models.TextField(null=True, blank=True)
+    """The reason for a motion."""
 
     state = models.ForeignKey(
         'State',
@@ -95,6 +102,11 @@ class Motion(RESTModelMixin, models.Model):
     This attribute is to get the current state of the motion.
     """
 
+    state_extension = models.TextField(blank=True, null=True)
+    """
+    A text field fo additional information about the state.
+    """
+
     recommendation = models.ForeignKey(
         'State',
         related_name='+',
@@ -104,6 +116,11 @@ class Motion(RESTModelMixin, models.Model):
     The recommendation of a person or committee for this motion.
     """
 
+    recommendation_extension = models.TextField(blank=True, null=True)
+    """
+    A text field fo additional information about the recommendation.
+    """
+
     identifier = models.CharField(max_length=255, null=True, blank=True,
                                   unique=True)
     """
@@ -168,11 +185,6 @@ class Motion(RESTModelMixin, models.Model):
     Users who support this motion.
     """
 
-    comments = JSONField(null=True)
-    """
-    Configurable fields for comments.
-    """
-
     # In theory there could be one then more agenda_item. But we support only
     # one. See the property agenda_item.
     agenda_items = GenericRelation(Item, related_name='motions')
@@ -183,8 +195,6 @@ class Motion(RESTModelMixin, models.Model):
             ('can_see', 'Can see motions'),
             ('can_create', 'Can create motions'),
             ('can_support', 'Can support motions'),
-            ('can_see_comments', 'Can see comments'),
-            ('can_manage_comments', 'Can manage comments'),
             ('can_manage', 'Can manage motions'),
         )
         ordering = ('identifier', )
@@ -197,34 +207,13 @@ class Motion(RESTModelMixin, models.Model):
         return self.title
 
     # TODO: Use transaction
-    def save(self, use_version=None, skip_autoupdate=False, *args, **kwargs):
+    def save(self, skip_autoupdate=False, *args, **kwargs):
         """
         Save the motion.
 
         1. Set the state of a new motion to the default state.
         2. Ensure that the identifier is not an empty string.
         3. Save the motion object.
-        4. Save the version data.
-        5. Set the active version for the motion if a new version object was saved.
-
-        The version data is *not* saved, if
-            1. the django-feature 'update_fields' is used or
-            2. the argument use_version is False (differ to None).
-
-        The argument use_version is choose the version object into which the
-        version data is saved.
-            * If use_version is False, no version data is saved.
-            * If use_version is None, the last version is used.
-            * Else the given version is used.
-
-        To create and use a new version object, you have to set it via the
-        use_version argument. You have to set the title, text/amendment_paragraphs,
-        modified final version and reason into this version object before giving it
-        to this save method. The properties motion.title, motion.text,
-        motion.amendment_paragraphs, motion.modified_final_version and motion.reason will be ignored.
-
-        text and amendment_paragraphs are mutually exclusive; if both are given,
-        amendment_paragraphs takes precedence.
         """
         if not self.state:
             self.reset_state()
@@ -256,55 +245,6 @@ class Motion(RESTModelMixin, models.Model):
                 # Save was successful. End loop.
                 break
 
-        if 'update_fields' in kwargs:
-            # Do not save the version data if only some motion fields are updated.
-            if not skip_autoupdate:
-                inform_changed_data(self)
-            return
-
-        if use_version is False:
-            # We do not need to save the version.
-            if not skip_autoupdate:
-                inform_changed_data(self)
-            return
-        elif use_version is None:
-            use_version = self.get_last_version()
-            # Save title, text, amendment paragraphs, modified final version and reason into the version object.
-            for attr in ['title', 'text', 'amendment_paragraphs', 'modified_final_version', 'reason']:
-                _attr = '_%s' % attr
-                data = getattr(self, _attr, None)
-                if data is not None:
-                    setattr(use_version, attr, data)
-                    delattr(self, _attr)
-
-        # If version is not in the database, test if it has new data and set
-        # the version_number.
-        if use_version.id is None:
-            if not self.version_data_changed(use_version):
-                # We do not need to save the version.
-                if not skip_autoupdate:
-                    inform_changed_data(self)
-                return
-            version_number = self.versions.aggregate(Max('version_number'))['version_number__max'] or 0
-            use_version.version_number = version_number + 1
-
-        # Necessary line if the version was set before the motion got an id.
-        use_version.motion = use_version.motion
-
-        # Always skip autoupdate. Maybe we run it later in this method.
-        use_version.save(skip_autoupdate=True)
-
-        # Set the active version of this motion. This has to be done after the
-        # version is saved in the database.
-        # TODO: Move parts of these last lines of code outside the save method
-        # when other versions than the last one should be edited later on.
-        if self.active_version is None or not self.state.leave_old_version_active:
-            # TODO: Don't call this if it was not a new version
-            self.active_version = use_version
-            # Always skip autoupdate. Maybe we run it later in this method.
-            self.save(update_fields=['active_version'], skip_autoupdate=True)
-
-        # Finally run autoupdate if it is not skipped by caller.
         if not skip_autoupdate:
             inform_changed_data(self)
 
@@ -319,24 +259,6 @@ class Motion(RESTModelMixin, models.Model):
             id=self.pk)
         return super().delete(skip_autoupdate=skip_autoupdate, *args, **kwargs)  # type: ignore
 
-    def version_data_changed(self, version):
-        """
-        Compare the version with the last version of the motion.
-
-        Returns True if the version data (title, text, amendment_paragraphs,
-        modified_final_version, reason) is different,
-        else returns False.
-        """
-        if not self.versions.exists():
-            # If there is no version in the database, the data has always changed.
-            return True
-
-        last_version = self.get_last_version()
-        for attr in ['title', 'text', 'amendment_paragraphs', 'modified_final_version', 'reason']:
-            if getattr(last_version, attr) != getattr(version, attr):
-                return True
-        return False
-
     def set_identifier(self):
         """
         Sets the motion identifier automaticly according to the config value if
@@ -421,188 +343,6 @@ class Motion(RESTModelMixin, models.Model):
             result = '0' * (settings.MOTION_IDENTIFIER_MIN_DIGITS - len(str(number))) + result
         return result
 
-    def get_title(self):
-        """
-        Get the title of the motion.
-
-        The title is taken from motion.version.
-        """
-        try:
-            return self._title
-        except AttributeError:
-            return self.get_active_version().title
-
-    def set_title(self, title):
-        """
-        Set the title of the motion.
-
-        The title will be saved in the version object, when motion.save() is
-        called.
-        """
-        self._title = title
-
-    title = property(get_title, set_title)
-    """
-    The title of the motion.
-
-    Is saved in a MotionVersion object.
-    """
-
-    def get_text(self):
-        """
-        Get the text of the motion.
-
-        Simular to get_title().
-        """
-        try:
-            return self._text
-        except AttributeError:
-            return self.get_active_version().text
-
-    def set_text(self, text):
-        """
-        Set the text of the motion.
-
-        Simular to set_title().
-        """
-        self._text = text
-
-    text = property(get_text, set_text)
-    """
-    The text of a motion.
-
-    Is saved in a MotionVersion object.
-    """
-
-    def get_amendment_paragraphs(self):
-        """
-        Get the paragraphs of the amendment.
-        Returns an array of entries that are either null (paragraph is not changed)
-        or a string (the new version of this paragraph).
-        """
-        try:
-            return self._amendment_paragraphs
-        except AttributeError:
-            return self.get_active_version().amendment_paragraphs
-
-    def set_amendment_paragraphs(self, text):
-        """
-        Set the paragraphs of the amendment.
-        Has to be an array of entries that are either null (paragraph is not changed)
-        or a string (the new version of this paragraph).
-        """
-        self._amendment_paragraphs = text
-
-    amendment_paragraphs = property(get_amendment_paragraphs, set_amendment_paragraphs)
-    """
-    The paragraphs of the amendment.
-
-    Is saved in a MotionVersion object.
-    """
-
-    def get_modified_final_version(self):
-        """
-        Get the modified_final_version of the motion.
-
-        Simular to get_title().
-        """
-        try:
-            return self._modified_final_version
-        except AttributeError:
-            return self.get_active_version().modified_final_version
-
-    def set_modified_final_version(self, modified_final_version):
-        """
-        Set the modified_final_version of the motion.
-
-        Simular to set_title().
-        """
-        self._modified_final_version = modified_final_version
-
-    modified_final_version = property(get_modified_final_version, set_modified_final_version)
-    """
-    The modified_final_version for the motion.
-
-    Is saved in a MotionVersion object.
-    """
-
-    def get_reason(self):
-        """
-        Get the reason of the motion.
-
-        Simular to get_title().
-        """
-        try:
-            return self._reason
-        except AttributeError:
-            return self.get_active_version().reason
-
-    def set_reason(self, reason):
-        """
-        Set the reason of the motion.
-
-        Simular to set_title().
-        """
-        self._reason = reason
-
-    reason = property(get_reason, set_reason)
-    """
-    The reason for the motion.
-
-    Is saved in a MotionVersion object.
-    """
-
-    def get_new_version(self, **kwargs):
-        """
-        Return a version object, not saved in the database.
-
-        The version data of the new version object is populated with the data
-        set via motion.title, motion.text, motion.amendment_paragraphs,
-        motion.modified_final_version and motion.reason if these data are not
-        given as keyword arguments. If the data is not set in the motion
-        attributes, it is populated with the data from the last version
-        object if such object exists.
-        """
-        if self.pk is None:
-            # Do not reference the MotionVersion object to an unsaved motion
-            new_version = MotionVersion(**kwargs)
-        else:
-            new_version = MotionVersion(motion=self, **kwargs)
-        if self.versions.exists():
-            last_version = self.get_last_version()
-        else:
-            last_version = None
-        for attr in ['title', 'text', 'amendment_paragraphs', 'modified_final_version', 'reason']:
-            if attr in kwargs:
-                continue
-            _attr = '_%s' % attr
-            data = getattr(self, _attr, None)
-            if data is None and last_version is not None:
-                data = getattr(last_version, attr)
-            if data is not None:
-                setattr(new_version, attr, data)
-        return new_version
-
-    def get_active_version(self):
-        """
-        Returns the active version of the motion.
-
-        If no active version is set by now, the last_version is used.
-        """
-        if self.active_version:
-            return self.active_version
-        else:
-            return self.get_last_version()
-
-    def get_last_version(self):
-        """
-        Return the newest version of the motion.
-        """
-        try:
-            return self.versions.order_by('-version_number')[0]
-        except IndexError:
-            return self.get_new_version()
-
     def is_submitter(self, user):
         """
         Returns True if user is a submitter of this motion, else False.
@@ -777,6 +517,76 @@ class Motion(RESTModelMixin, models.Model):
         return list(filter(lambda amend: amend.is_paragraph_based_amendment(), self.amendments.all()))
 
 
+class MotionCommentSection(RESTModelMixin, models.Model):
+    """
+    The model for comment sections for motions. Each comment is related to one section, so
+    each motions has the ability to have comments from the same section.
+    """
+    access_permissions = MotionCommentSectionAccessPermissions()
+
+    name = models.CharField(max_length=255)
+    """
+    The name of the section.
+    """
+
+    read_groups = models.ManyToManyField(
+        settings.AUTH_GROUP_MODEL,
+        blank=True,
+        related_name='read_comments')
+    """
+    These groups have read-access to the section.
+    """
+
+    write_groups = models.ManyToManyField(
+        settings.AUTH_GROUP_MODEL,
+        blank=True,
+        related_name='write_comments')
+    """
+    These groups have write-access to the section.
+    """
+
+    class Meta:
+        default_permissions = ()
+
+
+class MotionComment(RESTModelMixin, models.Model):
+    """
+    Represents a motion comment. A comment is always related to a motion and a comment
+    section. The section determinates the title of the category.
+    """
+
+    comment = models.TextField()
+    """
+    The comment.
+    """
+
+    motion = models.ForeignKey(
+        Motion,
+        on_delete=models.CASCADE,
+        related_name='comments')
+    """
+    The motion where this comment belongs to.
+    """
+
+    section = models.ForeignKey(
+        MotionCommentSection,
+        on_delete=models.PROTECT,
+        related_name='comments')
+    """
+    The section of the comment.
+    """
+
+    class Meta:
+        default_permissions = ()
+        unique_together = ('motion', 'section')
+
+    def get_root_rest_element(self):
+        """
+        Returns the motion to this instance which is the root REST element.
+        """
+        return self.motion
+
+
 class SubmitterManager(models.Manager):
     """
     Manager for Submitter model. Provides a customized add method.
@@ -840,68 +650,6 @@ class Submitter(RESTModelMixin, models.Model):
         return self.motion
 
 
-class MotionVersion(RESTModelMixin, models.Model):
-    """
-    A MotionVersion object saves some date of the motion.
-    """
-
-    motion = models.ForeignKey(
-        Motion,
-        on_delete=models.CASCADE,
-        related_name='versions')
-    """The motion to which the version belongs."""
-
-    version_number = models.PositiveIntegerField(default=1)
-    """An id for this version in realation to a motion.
-
-    Is unique for each motion.
-    """
-
-    title = models.CharField(max_length=255)
-    """The title of a motion."""
-
-    text = models.TextField()
-    """The text of a motion."""
-
-    amendment_paragraphs = JSONField(null=True)
-    """
-    If paragraph-based, diff-enabled amendment style is used, this field stores an array of strings or null values.
-    Each entry corresponds to a paragraph of the text of the original motion.
-    If the entry is null, then the paragraph remains unchanged.
-    If the entry is a string, this is the new text of the paragraph.
-    amendment_paragraphs and text are mutually exclusive.
-    """
-
-    modified_final_version = models.TextField(null=True, blank=True)
-    """A field to copy in the final version of the motion and edit it there."""
-
-    reason = models.TextField(null=True, blank=True)
-    """The reason for a motion."""
-
-    creation_time = models.DateTimeField(auto_now=True)
-    """Time when the version was saved."""
-
-    class Meta:
-        default_permissions = ()
-        unique_together = ("motion", "version_number")
-
-    def __str__(self):
-        """Return a string, representing this object."""
-        counter = self.version_number or ugettext_lazy('new')
-        return "Motion %s, Version %s" % (self.motion_id, counter)
-
-    @property
-    def active(self):
-        """Return True, if the version is the active version of a motion. Else: False."""
-        return self.active_version.exists()
-
-    def get_root_rest_element(self):
-        """
-        Returns the motion to this instance which is the root REST element.
-        """
-        return self.motion
-
-
 class MotionChangeRecommendationManager(models.Manager):
     """
     Customized model manager to support our get_full_queryset method.
@@ -916,18 +664,18 @@ class MotionChangeRecommendationManager(models.Manager):
 
 class MotionChangeRecommendation(RESTModelMixin, models.Model):
     """
-    A MotionChangeRecommendation object saves change recommendations for a specific MotionVersion
+    A MotionChangeRecommendation object saves change recommendations for a specific Motion
     """
 
     access_permissions = MotionChangeRecommendationAccessPermissions()
 
     objects = MotionChangeRecommendationManager()
 
-    motion_version = models.ForeignKey(
+    motion = models.ForeignKey(
-        MotionVersion,
+        Motion,
         on_delete=models.CASCADE,
         related_name='change_recommendations')
-    """The motion version to which the change recommendation belongs."""
+    """The motion to which the change recommendation belongs."""
 
     rejected = models.BooleanField(default=False)
     """If true, this change recommendation has been rejected"""
@@ -945,7 +693,7 @@ class MotionChangeRecommendation(RESTModelMixin, models.Model):
     """The number or the last affected line (inclusive)"""
 
     text = models.TextField(blank=True)
-    """The replacement for the section of the original text specified by version, line_from and line_to"""
+    """The replacement for the section of the original text specified by motion, line_from and line_to"""
 
     author = models.ForeignKey(
         settings.AUTH_USER_MODEL,
@@ -966,7 +714,7 @@ class MotionChangeRecommendation(RESTModelMixin, models.Model):
 
     def save(self, *args, **kwargs):
         recommendations = (MotionChangeRecommendation.objects
-                           .filter(motion_version=self.motion_version)
+                           .filter(motion=self.motion)
                            .exclude(pk=self.pk))
 
         if self.collides_with_other_recommendation(recommendations):
@@ -980,7 +728,7 @@ class MotionChangeRecommendation(RESTModelMixin, models.Model):
 
     def __str__(self):
         """Return a string, representing this object."""
-        return "Recommendation for Version %s, line %s - %s" % (self.motion_version_id, self.line_from, self.line_to)
+        return "Recommendation for Motion %s, line %s - %s" % (self.motion_id, self.line_from, self.line_to)
 
 
 class Category(RESTModelMixin, models.Model):
@@ -1272,16 +1020,6 @@ class State(RESTModelMixin, models.Model):
     allow_submitter_edit = models.BooleanField(default=False)
     """If true, the submitter can edit the motion in this state."""
 
-    versioning = models.BooleanField(default=False)
-    """
-    If true, editing the motion will create a new version by default.
-    This behavior can be changed by the form and view, e. g. via the
-    MotionDisableVersioningMixin.
-    """
-
-    leave_old_version_active = models.BooleanField(default=False)
-    """If true, new versions are not automaticly set active."""
-
     dont_set_identifier = models.BooleanField(default=False)
     """
     Decides if the motion gets an identifier.

openslides/motions/serializers.py

@@ -4,14 +4,15 @@ from django.db import transaction
 from django.utils.translation import ugettext as _
 
 from ..poll.serializers import default_votes_validator
+from ..utils.auth import get_group_model
 from ..utils.rest_api import (
     CharField,
     DecimalField,
     DictField,
     Field,
+    IdPrimaryKeyRelatedField,
     IntegerField,
     ModelSerializer,
-    PrimaryKeyRelatedField,
     SerializerMethodField,
     ValidationError,
 )
@@ -21,9 +22,10 @@ from .models import (
     Motion,
     MotionBlock,
     MotionChangeRecommendation,
+    MotionComment,
+    MotionCommentSection,
     MotionLog,
     MotionPoll,
-    MotionVersion,
     State,
     Submitter,
     Workflow,
@@ -88,8 +90,6 @@ class StateSerializer(ModelSerializer):
             'allow_support',
             'allow_create_poll',
             'allow_submitter_edit',
-            'versioning',
-            'leave_old_version_active',
             'dont_set_identifier',
             'show_state_extension_field',
             'show_recommendation_extension_field',
@@ -128,32 +128,6 @@ class WorkflowSerializer(ModelSerializer):
         return workflow
 
 
-class MotionCommentsJSONSerializerField(Field):
-    """
-    Serializer for motions's comments JSONField.
-    """
-    def to_representation(self, obj):
-        """
-        Returns the value of the field.
-        """
-        return obj
-
-    def to_internal_value(self, data):
-        """
-        Checks that data is a list of strings.
-        """
-        if type(data) is not dict:
-            raise ValidationError({'detail': 'Data must be a dict.'})
-        for id, comment in data.items():
-            try:
-                id = int(id)
-            except ValueError:
-                raise ValidationError({'detail': 'Id must be an int.'})
-            if type(comment) is not str:
-                raise ValidationError({'detail': 'Comment must be a string.'})
-        return data
-
-
 class AmendmentParagraphsJSONSerializerField(Field):
     """
     Serializer for motions's amendment_paragraphs JSONField.
@@ -291,25 +265,6 @@ class MotionPollSerializer(ModelSerializer):
         return instance
 
 
-class MotionVersionSerializer(ModelSerializer):
-    amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False)
-
-    """
-    Serializer for motion.models.MotionVersion objects.
-    """
-    class Meta:
-        model = MotionVersion
-        fields = (
-            'id',
-            'version_number',
-            'creation_time',
-            'title',
-            'text',
-            'amendment_paragraphs',
-            'modified_final_version',
-            'reason',)
-
-
 class MotionChangeRecommendationSerializer(ModelSerializer):
     """
     Serializer for motion.models.MotionChangeRecommendation objects.
@@ -318,7 +273,7 @@ class MotionChangeRecommendationSerializer(ModelSerializer):
         model = MotionChangeRecommendation
         fields = (
             'id',
-            'motion_version',
+            'motion',
             'rejected',
             'type',
             'other_description',
@@ -337,6 +292,47 @@ class MotionChangeRecommendationSerializer(ModelSerializer):
         return data
 
 
+class MotionCommentSectionSerializer(ModelSerializer):
+    """
+    Serializer for motion.models.MotionCommentSection objects.
+    """
+    read_groups = IdPrimaryKeyRelatedField(
+        many=True,
+        required=False,
+        queryset=get_group_model().objects.all())
+
+    write_groups = IdPrimaryKeyRelatedField(
+        many=True,
+        required=False,
+        queryset=get_group_model().objects.all())
+
+    class Meta:
+        model = MotionCommentSection
+        fields = (
+            'id',
+            'name',
+            'read_groups',
+            'write_groups',)
+
+
+class MotionCommentSerializer(ModelSerializer):
+    """
+    Serializer for motion.models.MotionComment objects.
+    """
+    read_groups_id = SerializerMethodField()
+
+    class Meta:
+        model = MotionComment
+        fields = (
+            'id',
+            'comment',
+            'section',
+            'read_groups_id',)
+
+    def get_read_groups_id(self, comment):
+        return [group.id for group in comment.section.read_groups.all()]
+
+
 class SubmitterSerializer(ModelSerializer):
     """
     Serializer for motion.models.Submitter objects.
@@ -355,17 +351,15 @@ class MotionSerializer(ModelSerializer):
     """
     Serializer for motion.models.Motion objects.
     """
-    active_version = PrimaryKeyRelatedField(read_only=True)
+    comments = MotionCommentSerializer(many=True, read_only=True)
-    comments = MotionCommentsJSONSerializerField(required=False)
     log_messages = MotionLogSerializer(many=True, read_only=True)
     polls = MotionPollSerializer(many=True, read_only=True)
-    modified_final_version = CharField(allow_blank=True, required=False, write_only=True)
+    modified_final_version = CharField(allow_blank=True, required=False)
-    reason = CharField(allow_blank=True, required=False, write_only=True)
+    reason = CharField(allow_blank=True, required=False)
     state_required_permission_to_see = SerializerMethodField()
-    text = CharField(write_only=True, allow_blank=True)
+    text = CharField(allow_blank=True)
-    title = CharField(max_length=255, write_only=True)
+    title = CharField(max_length=255)
-    amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False, write_only=True)
+    amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False)
-    versions = MotionVersionSerializer(many=True, read_only=True)
     workflow_id = IntegerField(
         min_value=1,
         required=False,
@@ -384,19 +378,19 @@ class MotionSerializer(ModelSerializer):
             'amendment_paragraphs',
             'modified_final_version',
             'reason',
-            'versions',
-            'active_version',
             'parent',
             'category',
+            'comments',
             'motion_block',
             'origin',
             'submitters',
             'supporters',
-            'comments',
             'state',
+            'state_extension',
             'state_required_permission_to_see',
             'workflow_id',
             'recommendation',
+            'recommendation_extension',
             'tags',
             'attachments',
             'polls',
@@ -416,11 +410,6 @@ class MotionSerializer(ModelSerializer):
         if 'reason' in data:
             data['reason'] = validate_html(data['reason'])
 
-        validated_comments = dict()
-        for id, comment in data.get('comments', {}).items():
-            validated_comments[id] = validate_html(comment)
-        data['comments'] = validated_comments
-
         if 'amendment_paragraphs' in data:
             data['amendment_paragraphs'] = list(map(lambda entry: validate_html(entry) if type(entry) is str else None,
                                                     data['amendment_paragraphs']))
@@ -451,7 +440,6 @@ class MotionSerializer(ModelSerializer):
         motion.category = validated_data.get('category')
         motion.motion_block = validated_data.get('motion_block')
         motion.origin = validated_data.get('origin', '')
-        motion.comments = validated_data.get('comments')
         motion.parent = validated_data.get('parent')
         motion.reset_state(validated_data.get('workflow_id'))
         motion.agenda_item_update_information['type'] = validated_data.get('agenda_type')
@@ -467,38 +455,17 @@ class MotionSerializer(ModelSerializer):
         """
         Customized method to update a motion.
         """
-        # Identifier, category, motion_block, origin and comments.
+        workflow_id = None
-        for key in ('identifier', 'category', 'motion_block', 'origin', 'comments'):
+        if 'workflow_id' in validated_data:
-            if key in validated_data.keys():
+            workflow_id = validated_data.pop('workflow_id')
-                setattr(motion, key, validated_data[key])
+
+        result = super().update(motion, validated_data)
 
-        # Workflow.
-        workflow_id = validated_data.get('workflow_id')
         if workflow_id is not None and workflow_id != motion.workflow_id:
             motion.reset_state(workflow_id)
+            motion.save()
 
-        # Decide if a new version is saved to the database.
+        return result
-        if (motion.state.versioning and
-                not validated_data.get('disable_versioning', False)):  # TODO
-            version = motion.get_new_version()
-        else:
-            version = motion.get_last_version()
-
-        # Title, text, reason, ...
-        for key in ('title', 'text', 'amendment_paragraphs', 'modified_final_version', 'reason'):
-            if key in validated_data.keys():
-                setattr(version, key, validated_data[key])
-
-        motion.save(use_version=version)
-
-        # Submitters, supporters, attachments and tags
-        for key in ('submitters', 'supporters', 'attachments', 'tags'):
-            if key in validated_data.keys():
-                attr = getattr(motion, key)
-                attr.clear()
-                attr.add(*validated_data[key])
-
-        return motion
 
     def get_state_required_permission_to_see(self, motion):
         """

openslides/motions/signals.py

@@ -54,54 +54,44 @@ def create_builtin_workflows(sender, **kwargs):
                                      action_word='Permit',
                                      recommendation_label='Permission',
                                      allow_create_poll=True,
-                                     allow_submitter_edit=True,
+                                     allow_submitter_edit=True)
-                                     versioning=True,
-                                     leave_old_version_active=True)
     state_2_3 = State.objects.create(name=ugettext_noop('accepted'),
                                      workflow=workflow_2,
                                      action_word='Accept',
                                      recommendation_label='Acceptance',
-                                     versioning=True,
                                      css_class='success')
     state_2_4 = State.objects.create(name=ugettext_noop('rejected'),
                                      workflow=workflow_2,
                                      action_word='Reject',
                                      recommendation_label='Rejection',
-                                     versioning=True,
                                      css_class='danger')
     state_2_5 = State.objects.create(name=ugettext_noop('withdrawed'),
                                      workflow=workflow_2,
                                      action_word='Withdraw',
-                                     versioning=True,
                                      css_class='default')
     state_2_6 = State.objects.create(name=ugettext_noop('adjourned'),
                                      workflow=workflow_2,
                                      action_word='Adjourn',
                                      recommendation_label='Adjournment',
-                                     versioning=True,
                                      css_class='default')
     state_2_7 = State.objects.create(name=ugettext_noop('not concerned'),
                                      workflow=workflow_2,
                                      action_word='Do not concern',
                                      recommendation_label='No concernment',
-                                     versioning=True,
                                      css_class='default')
     state_2_8 = State.objects.create(name=ugettext_noop('refered to committee'),
                                      workflow=workflow_2,
                                      action_word='Refer to committee',
                                      recommendation_label='Referral to committee',
-                                     versioning=True,
                                      css_class='default')
     state_2_9 = State.objects.create(name=ugettext_noop('needs review'),
                                      workflow=workflow_2,
                                      action_word='Needs review',
-                                     versioning=True,
                                      css_class='default')
     state_2_10 = State.objects.create(name=ugettext_noop('rejected (not authorized)'),
                                       workflow=workflow_2,
                                       action_word='Reject (not authorized)',
                                       recommendation_label='Rejection (not authorized)',
-                                      versioning=True,
                                       css_class='default')
     state_2_1.next_states.add(state_2_2, state_2_5, state_2_10)
     state_2_2.next_states.add(state_2_3, state_2_4, state_2_5, state_2_6, state_2_7, state_2_8, state_2_9)

openslides/motions/views.py

@@ -1,18 +1,17 @@
 import re
-from typing import Optional
+from typing import List, Optional
 
 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.db.models.deletion import ProtectedError
-from django.http import Http404
 from django.http.request import QueryDict
 from django.utils.translation import ugettext as _, ugettext_noop
 from rest_framework import status
 
 from ..core.config import config
-from ..utils.auth import has_perm
+from ..utils.auth import has_perm, in_some_groups
 from ..utils.autoupdate import inform_changed_data
 from ..utils.collection import CollectionElement
 from ..utils.exceptions import OpenSlidesError
@@ -32,6 +31,7 @@ from .access_permissions import (
     MotionAccessPermissions,
     MotionBlockAccessPermissions,
     MotionChangeRecommendationAccessPermissions,
+    MotionCommentSectionAccessPermissions,
     WorkflowAccessPermissions,
 )
 from .exceptions import WorkflowError
@@ -40,8 +40,9 @@ from .models import (
     Motion,
     MotionBlock,
     MotionChangeRecommendation,
+    MotionComment,
+    MotionCommentSection,
     MotionPoll,
-    MotionVersion,
     State,
     Submitter,
     Workflow,
@@ -56,7 +57,7 @@ class MotionViewSet(ModelViewSet):
     API endpoint for motions.
 
     There are the following views: metadata, list, retrieve, create,
-    partial_update, update, destroy, manage_version, support, set_state and
+    partial_update, update, destroy, support, set_state and
     create_poll.
     """
     access_permissions = MotionAccessPermissions()
@@ -77,7 +78,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 ('manage_version', 'set_state', 'set_recommendation',
+        elif self.action in ('set_state', 'manage_comments', 'set_recommendation',
                              'follow_recommendation', 'create_poll', 'manage_submitters',
                              'sort_submitters'):
             result = (has_perm(self.request.user, 'motions.can_see') and
@@ -130,7 +131,6 @@ class MotionViewSet(ModelViewSet):
                 'title',
                 'text',
                 'reason',
-                'comments',  # This is checked later.
             ]
             if parent_motion is not None:
                 # For creating amendments.
@@ -146,16 +146,6 @@ class MotionViewSet(ModelViewSet):
                 if key not in whitelist:
                     del request.data[key]
 
-        # Check permission to send comment data.
-        if (not has_perm(request.user, 'motions.can_see_comments') or
-                not has_perm(request.user, 'motions.can_manage_comments')):
-            try:
-                # Ignore comments data if user is not allowed to send comments.
-                del request.data['comments']
-            except KeyError:
-                # No comments here. Just do nothing.
-                pass
-
         # Validate data and create motion.
         serializer = self.get_serializer(data=request.data)
         serializer.is_valid(raise_exception=True)
@@ -223,9 +213,7 @@ class MotionViewSet(ModelViewSet):
 
         # Check permissions.
         if (not has_perm(request.user, 'motions.can_manage') and
-                not (motion.is_submitter(request.user) and motion.state.allow_submitter_edit) and
+                not (motion.is_submitter(request.user) and motion.state.allow_submitter_edit)):
-                not (has_perm(request.user, 'motions.can_see_comments') and
-                     has_perm(request.user, 'motions.can_manage_comments'))):
             self.permission_denied(request)
 
         # Check permission to send only some data.
@@ -233,9 +221,7 @@ class MotionViewSet(ModelViewSet):
             # Remove fields that the user is not allowed to change.
             # The list() is required because we want to use del inside the loop.
             keys = list(request.data.keys())
-            whitelist = [
+            whitelist: List[str] = []
-                'comments',  # This is checked later.
-            ]
             # Add title, text and reason to the whitelist only, if the user is the submitter.
             if motion.is_submitter(request.user) and motion.state.allow_submitter_edit:
                 whitelist.extend((
@@ -247,70 +233,22 @@ class MotionViewSet(ModelViewSet):
                 if key not in whitelist:
                     del request.data[key]
 
-        # Check comments
-        # "normal" comments only can be changed if the user has can_see_comments and
-        # can_manage_comments.
-        # "special" comments (for state and recommendation) can only be changed, if
-        # the user has the can_manage permission
-        if 'comments' in request.data:
-            request_comments = {}  # Here, all valid comments are saved.
-            for id, value in request.data['comments'].items():
-                field = config['motions_comments'].get(id)
-                if field:
-                    is_special_comment = field.get('forRecommendation') or field.get('forState')
-                    if (is_special_comment and has_perm(request.user, 'motions.can_manage')) or (
-                        not is_special_comment and has_perm(request.user, 'motions.can_see_comments') and
-                            has_perm(request.user, 'motions.can_manage_comments')):
-                        # The user has the required permission for at least one case
-                        # Save the comment!
-                        request_comments[id] = value
-
-            # two possibilities here: Either the comments dict is empty: then delete it, so
-            # the serializer will skip it. If we leave it empty, everything will be deleted :(
-            # Second, just special or normal comments are in the comments dict. Fill the original
-            # data, so it won't be delete.
-
-            if len(request_comments) == 0:
-                del request.data['comments']
-            else:
-                if motion.comments:
-                    for id, value in motion.comments.items():
-                        if id not in request_comments:
-                            # populate missing comments with original ones.
-                            request_comments[id] = value
-                request.data['comments'] = request_comments
-
-        # get changed comment fields
-        changed_comment_fields = []
-        comments = request.data.get('comments', {})
-        for id, value in comments.items():
-            if not motion.comments or motion.comments.get(id) != value:
-                field = config['motions_comments'].get(id)
-                if field:
-                    name = field['name']
-                    changed_comment_fields.append(name)
-
         # Validate data and update motion.
         serializer = self.get_serializer(
             motion,
             data=request.data,
             partial=kwargs.get('partial', False))
         serializer.is_valid(raise_exception=True)
-        updated_motion = serializer.save(disable_versioning=request.data.get('disable_versioning'))
+        updated_motion = serializer.save()
 
         # Write the log message, check removal of supporters and initiate response.
-        # TODO: Log if a version was updated.
+        # TODO: Log if a motion was updated.
         updated_motion.write_log([ugettext_noop('Motion updated')], request.user)
         if (config['motions_remove_supporters'] and updated_motion.state.allow_support and
                 not has_perm(request.user, 'motions.can_manage')):
             updated_motion.supporters.clear()
             updated_motion.write_log([ugettext_noop('All supporters removed')], request.user)
 
-        if len(changed_comment_fields) > 0:
-            updated_motion.write_log(
-                [ugettext_noop('Comment {} updated').format(', '.join(changed_comment_fields))],
-                request.user)
-
         # 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.supporters.all())
@@ -318,48 +256,66 @@ class MotionViewSet(ModelViewSet):
 
         return Response(serializer.data)
 
-    @detail_route(methods=['put', 'delete'])
+    @detail_route(methods=['POST', 'DELETE'])
-    def manage_version(self, request, pk=None):
+    def manage_comments(self, request, pk=None):
         """
-        Special view endpoint to permit and delete a version of a motion.
+        Create, update and delete motin comments.
-
+        Send a post request with {'section_id': <id>, 'comment': '<comment>'} to create
-        Send PUT {'version_number': <number>} to permit and DELETE
+        a new comment or update an existing comment.
-        {'version_number': <number>} to delete a version. Deleting the
+        Send a delete request with just {'section_id': <id>} to delete the comment.
-        active version is not allowed. Only managers can use this view.
+        For ever request, the user must have read and write permission for the given field.
         """
-        # Retrieve motion and version.
         motion = self.get_object()
-        version_number = request.data.get('version_number')
+
+        # Get the comment section
+        section_id = request.data.get('section_id')
+        if not section_id or not isinstance(section_id, int):
+            raise ValidationError({'detail': _('You have to provide a section_id of type int.')})
+
         try:
-            version = motion.versions.get(version_number=version_number)
+            section = MotionCommentSection.objects.get(pk=section_id)
-        except MotionVersion.DoesNotExist:
+        except MotionCommentSection.DoesNotExist:
-            raise Http404('Version %s not found.' % version_number)
+            raise ValidationError({'detail': _('A comment section with id {} does not exist').format(section_id)})
 
-        # Permit or delete version.
+        # the request user needs to see and write to the comment section
-        if request.method == 'PUT':
+        if (not in_some_groups(request.user, list(section.read_groups.values_list('pk', flat=True))) or
-            # Permit version.
+                not in_some_groups(request.user, list(section.write_groups.values_list('pk', flat=True)))):
-            motion.active_version = version
+            raise ValidationError({'detail': _('You are not allowed to see or write to the comment section.')})
-            motion.save(update_fields=['active_version'])
+
-            motion.write_log(
+        if request.method == 'POST':  # Create or update
-                message_list=[ugettext_noop('Version'),
+            # validate comment
-                              ' %d ' % version.version_number,
+            comment_value = request.data.get('comment', '')
-                              ugettext_noop('permitted')],
+            if not isinstance(comment_value, str):
-                person=self.request.user)
+                raise ValidationError({'detail': _('The comment should be a string.')})
-            message = _('Version %d permitted successfully.') % version.version_number
+
-        else:
+            comment, created = MotionComment.objects.get_or_create(
-            # Delete version.
+                    motion=motion,
-            # request.method == 'DELETE'
+                    section=section,
-            if version == motion.active_version:
+                    defaults={
-                raise ValidationError({'detail': _('You can not delete the active version of a motion.')})
+                        'comment': comment_value})
-            version.delete()
+            if not created:
-            motion.write_log(
+                comment.comment = comment_value
-                message_list=[ugettext_noop('Version'),
+                comment.save()
-                              ' %d ' % version.version_number,
+
-                              ugettext_noop('deleted')],
+            # write log
-                person=self.request.user)
+            motion.write_log(
-            message = _('Version %d deleted successfully.') % version.version_number
+                [ugettext_noop('Comment {} updated').format(section.name)],
+                request.user)
+            message = _('Comment {} updated').format(section.name)
+        else:  # DELETE
+            try:
+                comment = MotionComment.objects.get(motion=motion, section=section)
+            except MotionComment.DoesNotExist:
+                # Be silent about not existing comments, but do not create a log entry.
+                pass
+            else:
+                comment.delete()
+
+                motion.write_log(
+                    [ugettext_noop('Comment {} deleted').format(section.name)],
+                    request.user)
+            message = _('Comment {} deleted').format(section.name)
 
-        # Initiate response.
         return Response({'detail': message})
 
     @detail_route(methods=['POST', 'DELETE'])
@@ -588,17 +544,13 @@ class MotionViewSet(ModelViewSet):
         motion.set_state(motion.recommendation)
 
         # Set the special state comment.
-        extension = request.data.get('recommendationExtension')
+        extension = request.data.get('state_extension')
         if extension is not None:
-            # Find the special "state" comment field.
+            motion.state_extension = extension
-            for id, field in config['motions_comments'].items():
-                if isinstance(field, dict) and 'forState' in field and field['forState'] is True:
-                    motion.comments[id] = extension
-                    break
 
         # Save and write log.
         motion.save(
-            update_fields=['state', 'identifier', 'identifier_number', 'comments'],
+            update_fields=['state', 'identifier', 'identifier_number', 'state_extension'],
             skip_autoupdate=True)
         motion.write_log(
             message_list=[ugettext_noop('State set to'), ' ', motion.state.name],
@@ -700,6 +652,51 @@ class MotionChangeRecommendationViewSet(ModelViewSet):
             return Response({'detail': err.message}, status=400)
 
 
+class MotionCommentSectionViewSet(ModelViewSet):
+    """
+    API endpoint for motion comment fields.
+    """
+    access_permissions = MotionCommentSectionAccessPermissions()
+    queryset = MotionCommentSection.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', 'destroy', 'update', 'partial_update'):
+            result = (has_perm(self.request.user, 'motions.can_see') and
+                      has_perm(self.request.user, 'motions.can_manage'))
+        else:
+            result = False
+        return result
+
+    def destroy(self, *args, **kwargs):
+        """
+        Customized view endpoint to delete a motion comment section. Will return
+        an error for the user, if still comments for this section exist.
+        """
+        try:
+            result = super().destroy(*args, **kwargs)
+        except ProtectedError as e:
+            # The protected objects can just be motion comments.
+            motions = ['"' + str(comment.motion) + '"' for comment in e.protected_objects.all()]
+            count = len(motions)
+            motions_verbose = ', '.join(motions[:3])
+            if count > 3:
+                motions_verbose += ', ...'
+
+            if count == 1:
+                msg = _('This section has still comments in motion {}.').format(motions_verbose)
+            else:
+                msg = _('This section has still comments in motions {}.').format(motions_verbose)
+
+            msg += ' ' + _('Please remove all comments before deletion.')
+            raise ValidationError({'detail': msg})
+        return result
+
+
 class CategoryViewSet(ModelViewSet):
     """
     API endpoint for categories.
@@ -920,7 +917,7 @@ class WorkflowViewSet(ModelViewSet, ProtectedErrorMessageMixin):
 
     def destroy(self, *args, **kwargs):
         """
-        Customized view endpoint to delete a motion poll.
+        Customized view endpoint to delete a workflow.
         """
         try:
             result = super().destroy(*args, **kwargs)
@@ -949,7 +946,7 @@ class StateViewSet(CreateModelMixin, UpdateModelMixin, DestroyModelMixin, Generi
 
     def destroy(self, *args, **kwargs):
         """
-        Customized view endpoint to delete a motion poll.
+        Customized view endpoint to delete a state.
         """
         state = self.get_object()
         if state.workflow.first_state.pk == state.pk:  # is this the first state of the workflow?

openslides/users/signals.py

@@ -53,8 +53,6 @@ def create_builtin_groups_and_admin(**kwargs):
         'motions.can_create',
         'motions.can_manage',
         'motions.can_see',
-        'motions.can_see_comments',
-        'motions.can_manage_comments',
         'motions.can_support',
         'users.can_manage',
         'users.can_see_extra_data',
@@ -124,8 +122,6 @@ def create_builtin_groups_and_admin(**kwargs):
         permission_dict['motions.can_see'],
         permission_dict['motions.can_create'],
         permission_dict['motions.can_manage'],
-        permission_dict['motions.can_see_comments'],
-        permission_dict['motions.can_manage_comments'],
         permission_dict['users.can_see_name'],
         permission_dict['users.can_manage'],
         permission_dict['users.can_see_extra_data'],
@@ -158,8 +154,6 @@ def create_builtin_groups_and_admin(**kwargs):
         permission_dict['motions.can_see'],
         permission_dict['motions.can_create'],
         permission_dict['motions.can_manage'],
-        permission_dict['motions.can_see_comments'],
-        permission_dict['motions.can_manage_comments'],
         permission_dict['users.can_see_name'],
         permission_dict['users.can_manage'],
         permission_dict['users.can_see_extra_data'],

openslides/utils/auth.py

@@ -1,13 +1,33 @@
-from typing import Dict, Optional, Union, cast
+from typing import Dict, List, Optional, Union, cast
 
+from django.apps import apps
+from django.conf import settings
 from django.contrib.auth import get_user_model
 from django.contrib.auth.models import AnonymousUser
+from django.core.exceptions import ImproperlyConfigured
 from django.db.models import Model
 
 from .cache import element_cache
 from .collection import CollectionElement
 
 
+GROUP_DEFAULT_PK = 1  # This is the hard coded pk for the default group.
+
+
+def get_group_model() -> Model:
+    """
+    Return the Group model that is active in this project.
+    """
+    try:
+        return apps.get_model(settings.AUTH_GROUP_MODEL, require_ready=False)
+    except ValueError:
+        raise ImproperlyConfigured("AUTH_GROUP_MODEL must be of the form 'app_label.model_name'")
+    except LookupError:
+        raise ImproperlyConfigured(
+            "AUTH_GROUP_MODEL refers to model '%s' that has not been installed" % settings.AUTH_GROUP_MODEL
+        )
+
+
 def has_perm(user: Optional[CollectionElement], perm: str) -> bool:
     """
     Checks that user has a specific permission.
@@ -22,13 +42,13 @@ def has_perm(user: Optional[CollectionElement], perm: str) -> bool:
     if user is None and not anonymous_is_enabled():
         has_perm = False
     elif user is None:
-        # Use the permissions from the default group with id 1.
+        # Use the permissions from the default group.
-        default_group = CollectionElement.from_values(group_collection_string, 1)
+        default_group = CollectionElement.from_values(group_collection_string, GROUP_DEFAULT_PK)
         has_perm = perm in default_group.get_full_data()['permissions']
     else:
         # Get all groups of the user and then see, if one group has the required
-        # permission. If the user has no groups, then use group 1.
+        # permission. If the user has no groups, then use the default group.
-        group_ids = user.get_full_data()['groups_id'] or [1]
+        group_ids = user.get_full_data()['groups_id'] or [GROUP_DEFAULT_PK]
         for group_id in group_ids:
             group = CollectionElement.from_values(group_collection_string, group_id)
             if perm in group.get_full_data()['permissions']:
@@ -39,6 +59,38 @@ def has_perm(user: Optional[CollectionElement], perm: str) -> bool:
     return has_perm
 
 
+def in_some_groups(user: Optional[CollectionElement], groups: List[int]) -> bool:
+    """
+    Checks that user is in at least one given group. Groups can be given as a list
+    of ids or group instances.
+
+    User can be a CollectionElement of a user or None.
+    """
+
+    if len(groups) == 0:
+        return False  # early end here, if no groups are given.
+
+    # Convert user to right type
+    # TODO: Remove this and make use, that user has always the right type
+    user = user_to_collection_user(user)
+    if user is None and not anonymous_is_enabled():
+        in_some_groups = False
+    elif user is None:
+        # Use the permissions from the default group.
+        in_some_groups = GROUP_DEFAULT_PK in groups
+    else:
+        # Get all groups of the user and then see, if one group has the required
+        # permission. If the user has no groups, then use the default group.
+        group_ids = user.get_full_data()['groups_id'] or [GROUP_DEFAULT_PK]
+        for group_id in group_ids:
+            if group_id in groups:
+                in_some_groups = True
+                break
+        else:
+            in_some_groups = False
+    return in_some_groups
+
+
 def anonymous_is_enabled() -> bool:
     """
     Returns True if the anonymous user is enabled in the settings.

tests/integration/agenda/test_viewset.py

@@ -99,7 +99,6 @@ def test_agenda_item_db_queries():
     * 3 requests to get the assignments, motions and topics and
 
     * 1 request to get an agenda item (why?)
-    * 2 requests for the motionsversions.
     TODO: The last three request are a bug.
     """
     for index in range(10):
@@ -112,7 +111,7 @@ def test_agenda_item_db_queries():
     Motion.objects.create(title='motion2')
     Assignment.objects.create(title='assignment', open_posts=5)
 
-    assert count_queries(Item.get_elements) == 8
+    assert count_queries(Item.get_elements) == 6
 
 
 class ManageSpeaker(TestCase):

tests/integration/motions/test_viewset.py

@@ -2,23 +2,24 @@ import json
 
 import pytest
 from django.contrib.auth import get_user_model
-from django.contrib.auth.models import Permission
 from django.urls import reverse
 from rest_framework import status
 from rest_framework.test import APIClient
 
 from openslides.core.config import config
-from openslides.core.models import ConfigStore, Tag
+from openslides.core.models import Tag
 from openslides.motions.models import (
     Category,
     Motion,
     MotionBlock,
+    MotionComment,
+    MotionCommentSection,
     MotionLog,
     State,
     Submitter,
     Workflow,
 )
-from openslides.users.models import Group
+from openslides.utils.auth import get_group_model
 from openslides.utils.collection import CollectionElement
 from openslides.utils.test import TestCase
 
@@ -31,22 +32,39 @@ def test_motion_db_queries():
     Tests that only the following db queries are done:
     * 1 requests to get the list of all motions,
     * 1 request to get the associated workflow
-    * 1 request to get the motion versions,
+    * 1 request for all motion comments
+    * 1 request for all motion comment sections required for the comments
+    * 1 request for all users required for the read_groups of the sections
     * 1 request to get the agenda item,
     * 1 request to get the motion log,
     * 1 request to get the polls,
     * 1 request to get the attachments,
     * 1 request to get the tags,
     * 2 requests to get the submitters and supporters.
+
+    Two comment sections are created and for each motions two comments.
     """
+    section1 = MotionCommentSection.objects.create(name='test_section')
+    section2 = MotionCommentSection.objects.create(name='test_section')
+
     for index in range(10):
-        Motion.objects.create(title='motion{}'.format(index))
+        motion = Motion.objects.create(title='motion{}'.format(index))
+
+        MotionComment.objects.create(
+            comment='test_comment',
+            motion=motion,
+            section=section1)
+        MotionComment.objects.create(
+            comment='test_comment2',
+            motion=motion,
+            section=section2)
+
         get_user_model().objects.create_user(
             username='user_{}'.format(index),
             password='password')
     # TODO: Create some polls etc.
 
-    assert count_queries(Motion.get_elements) == 10
+    assert count_queries(Motion.get_elements) == 12
 
 
 @pytest.mark.django_db(transaction=False)
@@ -169,45 +187,6 @@ class CreateMotion(TestCase):
         motion = Motion.objects.get()
         self.assertEqual(motion.tags.get().name, 'test_tag_iRee3kiecoos4rorohth')
 
-    def test_with_multiple_comments(self):
-        comments = {
-            '1': 'comemnt1_sdpoiuffo3%7dwDwW)',
-            '2': 'comment2_iusd_D/TdskDWH(5DWas46WAd078'}
-        response = self.client.post(
-            reverse('motion-list'),
-            {'title': 'title_test_sfdAaufd56HR7sd5FDq7av',
-             'text': 'text_test_fiuhefF86()ew1Ef346AF6W',
-             'comments': comments},
-            format='json')
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        motion = Motion.objects.get()
-        self.assertEqual(motion.comments, comments)
-
-    def test_wrong_comment_format(self):
-        comments = [
-            'comemnt1_wpcjlwgj$§ks)skj2LdmwKDWSLw6',
-            'comment2_dq2Wd)Jwdlmm:,w82DjwQWSSiwjd']
-        response = self.client.post(
-            reverse('motion-list'),
-            {'title': 'title_test_sfdAaufd56HR7sd5FDq7av',
-             'text': 'text_test_fiuhefF86()ew1Ef346AF6W',
-             'comments': comments},
-            format='json')
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(response.data, {'comments': {'detail': 'Data must be a dict.'}})
-
-    def test_wrong_comment_id(self):
-        comment = {
-            'string': 'comemnt1_wpcjlwgj$§ks)skj2LdmwKDWSLw6'}
-        response = self.client.post(
-            reverse('motion-list'),
-            {'title': 'title_test_sfdAaufd56HR7sd5FDq7av',
-             'text': 'text_test_fiuhefF86()ew1Ef346AF6W',
-             'comments': comment},
-            format='json')
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(response.data, {'comments': {'detail': 'Id must be an int.'}})
-
     def test_with_workflow(self):
         """
         Test to create a motion with a specific workflow.
@@ -227,7 +206,7 @@ class CreateMotion(TestCase):
         """
         self.admin = get_user_model().objects.get(username='admin')
         self.admin.groups.add(2)
-        self.admin.groups.remove(3)
+        self.admin.groups.remove(4)
 
         response = self.client.post(
             reverse('motion-list'),
@@ -236,35 +215,6 @@ class CreateMotion(TestCase):
 
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
 
-    def test_non_admin_with_comment_data(self):
-        """
-        Test to create a motion by a non staff user that has permission to
-        manage motion comments and sends some additional fields.
-        """
-        self.admin = get_user_model().objects.get(username='admin')
-        self.admin.groups.add(2)
-        self.admin.groups.remove(4)
-        group_delegate = self.admin.groups.get()
-        group_delegate.permissions.add(Permission.objects.get(
-            content_type__app_label='motions',
-            codename='can_manage_comments',
-        ))
-        group_delegate.permissions.add(Permission.objects.get(
-            content_type__app_label='motions',
-            codename='can_see_comments',
-        ))
-
-        response = self.client.post(
-            reverse('motion-list'),
-            {'title': 'test_title_peiJozae0luew9EeL8bo',
-             'text': 'test_text_eHohS8ohr5ahshoah8Oh',
-             'comments': {'1': 'comment_for_field_one__xiek1Euhae9xah2wuuraaaa'}},
-            format='json',
-        )
-
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        self.assertEqual(Motion.objects.get().comments, {'1': 'comment_for_field_one__xiek1Euhae9xah2wuuraaaa'})
-
     def test_amendment_motion(self):
         """
         Test to create a motion with a parent motion as staff user.
@@ -383,7 +333,7 @@ class RetrieveMotion(TestCase):
     def test_user_without_can_see_user_permission_to_see_motion_and_submitter_data(self):
         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.
+        group = get_group_model().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('.')
         permission = group.permissions.get(content_type__app_label=app_label, codename=codename)
@@ -491,56 +441,6 @@ class UpdateMotion(TestCase):
         self.assertEqual(motion.title, 'new_title_ohph1aedie5Du8sai2ye')
         self.assertEqual(motion.supporters.count(), 0)
 
-    def test_with_new_version(self):
-        self.motion.set_state(State.objects.get(name='permitted'))
-        self.motion.save()
-        response = self.client.patch(
-            reverse('motion-detail', args=[self.motion.pk]),
-            {'text': 'test_text_aeb1iaghahChong5od3a'})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        motion = Motion.objects.get()
-        self.assertEqual(motion.versions.count(), 2)
-
-    def test_without_new_version(self):
-        self.motion.set_state(State.objects.get(name='permitted'))
-        self.motion.save()
-        response = self.client.patch(
-            reverse('motion-detail', args=[self.motion.pk]),
-            {'text': 'test_text_aeThaeroneiroo7Iophu',
-             'disable_versioning': True})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        motion = Motion.objects.get()
-        self.assertEqual(motion.versions.count(), 1)
-
-    def test_update_comment_creates_log_entry(self):
-        field_name = 'comment_field_name_texl2i7%sookqerpl29a'
-        config['motions_comments'] = {
-            '1': {
-                'name': field_name,
-                'public': False
-            }
-        }
-
-        # Update Config cache
-        CollectionElement.from_instance(
-            ConfigStore.objects.get(key='motions_comments')
-        )
-
-        response = self.client.patch(
-            reverse('motion-detail', args=[self.motion.pk]),
-            {'title': 'title_test_sfdAaufd56HR7sd5FDq7av',
-             'text': 'text_test_fiuhefF86()ew1Ef346AF6W',
-             'comments': {'1': 'comment1_sdpoiuffo3%7dwDwW)'}
-             },
-            format='json')
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-
-        motion_logs = MotionLog.objects.filter(motion=self.motion)
-        self.assertEqual(motion_logs.count(), 2)
-
-        motion_log = motion_logs.order_by('-time').first()
-        self.assertTrue(field_name in motion_log.message_list[0])
-
 
 class DeleteMotion(TestCase):
     """
@@ -564,7 +464,7 @@ class DeleteMotion(TestCase):
 
     def make_admin_delegate(self):
         group_admin = self.admin.groups.get(name='Admin')
-        group_delegates = Group.objects.get(name='Delegates')
+        group_delegates = get_group_model().objects.get(name='Delegates')
         self.admin.groups.remove(group_admin)
         self.admin.groups.add(group_delegates)
         CollectionElement.from_instance(self.admin)
@@ -594,51 +494,6 @@ class DeleteMotion(TestCase):
         self.assertEqual(motions, 0)
 
 
-class ManageVersion(TestCase):
-    """
-    Tests permitting and deleting versions.
-    """
-    def setUp(self):
-        self.client = APIClient()
-        self.client.login(username='admin', password='admin')
-        self.motion = Motion(
-            title='test_title_InieJ5HieZieg1Meid7K',
-            text='test_text_daePhougho7EenguWe4g')
-        self.motion.save()
-        self.version_2 = self.motion.get_new_version(title='new_title_fee7tef0seechazeefiW')
-        self.motion.save(use_version=self.version_2)
-
-    def test_permit(self):
-        self.assertEqual(Motion.objects.get(pk=self.motion.pk).active_version.version_number, 2)
-        response = self.client.put(
-            reverse('motion-manage-version', args=[self.motion.pk]),
-            {'version_number': '1'})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data, {'detail': 'Version 1 permitted successfully.'})
-        self.assertEqual(Motion.objects.get(pk=self.motion.pk).active_version.version_number, 1)
-
-    def test_permit_invalid_version(self):
-        response = self.client.put(
-            reverse('motion-manage-version', args=[self.motion.pk]),
-            {'version_number': '3'})
-        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
-
-    def test_delete(self):
-        response = self.client.delete(
-            reverse('motion-manage-version', args=[self.motion.pk]),
-            {'version_number': '1'})
-        self.assertEqual(response.status_code, status.HTTP_200_OK)
-        self.assertEqual(response.data, {'detail': 'Version 1 deleted successfully.'})
-        self.assertEqual(Motion.objects.get(pk=self.motion.pk).versions.count(), 1)
-
-    def test_delete_active_version(self):
-        response = self.client.delete(
-            reverse('motion-manage-version', args=[self.motion.pk]),
-            {'version_number': '2'})
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-        self.assertEqual(response.data, {'detail': 'You can not delete the active version of a motion.'})
-
-
 class ManageSubmitters(TestCase):
     """
     Tests adding and removing of submitters.
@@ -751,6 +606,506 @@ class ManageSubmitters(TestCase):
         self.assertEqual(self.motion.submitters.count(), 0)
 
 
+class ManageComments(TestCase):
+    """
+    Tests the manage_comment view.
+
+    Tests creation/updating and deletion of motion comments.
+    """
+    def setUp(self):
+        self.client = APIClient()
+        self.client.login(username='admin', password='admin')
+
+        self.admin = get_user_model().objects.get()
+        self.group_in = get_group_model().objects.get(pk=4)
+        self.group_out = get_group_model().objects.get(pk=2)  # The admin should not be in this group
+
+        self.motion = Motion(
+            title='test_title_SlqfMw(waso0saWMPqcZ',
+            text='test_text_f30skclqS9wWF=xdfaSL')
+        self.motion.save()
+
+        self.section_no_groups = MotionCommentSection(name='test_name_gj4F§(fj"(edm"§F3f3fs')
+        self.section_no_groups.save()
+
+        self.section_read = MotionCommentSection(name='test_name_2wv30(d2S&kvelkakl39')
+        self.section_read.save()
+        self.section_read.read_groups.add(self.group_in, self.group_out)  # Group out for testing multiple groups
+        self.section_read.write_groups.add(self.group_out)
+
+        self.section_read_write = MotionCommentSection(name='test_name_a3m9sd0(Mw2%slkrv30,')
+        self.section_read_write.save()
+        self.section_read_write.read_groups.add(self.group_in)
+        self.section_read_write.write_groups.add(self.group_in)
+
+    def test_retrieve_comment(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_read_write,
+            comment='test_comment_gwic37Csc&3lf3eo2')
+        comment.save()
+
+        response = self.client.get(reverse('motion-detail', args=[self.motion.pk]))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue('comments' in response.data)
+        comments = response.data['comments']
+        self.assertTrue(isinstance(comments, list))
+        self.assertEqual(len(comments), 1)
+        self.assertEqual(comments[0]['comment'], 'test_comment_gwic37Csc&3lf3eo2')
+
+    def test_retrieve_comment_no_read_permission(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_no_groups,
+            comment='test_comment_fgkj3C7veo3ijWE(j2DJ')
+        comment.save()
+
+        response = self.client.get(reverse('motion-detail', args=[self.motion.pk]))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue('comments' in response.data)
+        comments = response.data['comments']
+        self.assertTrue(isinstance(comments, list))
+        self.assertEqual(len(comments), 0)
+
+    def test_wrong_data_type(self):
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            None,
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data['detail'],
+            'You have to provide a section_id of type int.')
+
+    def test_wrong_comment_data_type(self):
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read_write.id,
+                'comment': [32, 'no_correct_data']
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data['detail'],
+            'The comment should be a string.')
+
+    def test_non_existing_section(self):
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': 42,
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(
+            response.data['detail'],
+            'A comment section with id 42 does not exist')
+
+    def test_create_comment(self):
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read_write.pk,
+                'comment': 'test_comment_fk3jrnfwsdg%fj=feijf'
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(MotionComment.objects.count(), 1)
+        comment = MotionComment.objects.get()
+        self.assertEqual(comment.comment, 'test_comment_fk3jrnfwsdg%fj=feijf')
+
+        # Check for a log entry
+        motion_logs = MotionLog.objects.filter(motion=self.motion)
+        self.assertEqual(motion_logs.count(), 1)
+        comment_log = motion_logs.get()
+        self.assertTrue(self.section_read_write.name in comment_log.message_list[0])
+
+    def test_update_comment(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_read_write,
+            comment='test_comment_fji387fqwdf&ff=)Fe3j')
+        comment.save()
+
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read_write.pk,
+                'comment': 'test_comment_fk3jrnfwsdg%fj=feijf'
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        comment = MotionComment.objects.get()
+        self.assertEqual(comment.comment, 'test_comment_fk3jrnfwsdg%fj=feijf')
+
+        # Check for a log entry
+        motion_logs = MotionLog.objects.filter(motion=self.motion)
+        self.assertEqual(motion_logs.count(), 1)
+        comment_log = motion_logs.get()
+        self.assertTrue(self.section_read_write.name in comment_log.message_list[0])
+
+    def test_delete_comment(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_read_write,
+            comment='test_comment_5CJ"8f23jd3j2,r93keZ')
+        comment.save()
+
+        response = self.client.delete(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read_write.pk
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(MotionComment.objects.count(), 0)
+
+        # Check for a log entry
+        motion_logs = MotionLog.objects.filter(motion=self.motion)
+        self.assertEqual(motion_logs.count(), 1)
+        comment_log = motion_logs.get()
+        self.assertTrue(self.section_read_write.name in comment_log.message_list[0])
+
+    def test_delete_not_existing_comment(self):
+        """
+        This should fail silently; no error, if the user wants to delete
+        a not existing comment.
+        """
+        response = self.client.delete(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read_write.pk
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(MotionComment.objects.count(), 0)
+
+        # Check that no log entry was created
+        motion_logs = MotionLog.objects.filter(motion=self.motion)
+        self.assertEqual(motion_logs.count(), 0)
+
+    def test_create_comment_no_write_permission(self):
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read.pk,
+                'comment': 'test_comment_f38jfwqfj830fj4j(FU3'
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(MotionComment.objects.count(), 0)
+        self.assertEqual(
+            response.data['detail'],
+            'You are not allowed to see or write to the comment section.')
+
+    def test_update_comment_no_write_permission(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_read,
+            comment='test_comment_jg38dwiej2D832(D§dk)')
+        comment.save()
+
+        response = self.client.post(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read.pk,
+                'comment': 'test_comment_fk3jrnfwsdg%fj=feijf'
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        comment = MotionComment.objects.get()
+        self.assertEqual(comment.comment, 'test_comment_jg38dwiej2D832(D§dk)')
+
+    def test_delete_comment_no_write_permission(self):
+        comment = MotionComment(
+            motion=self.motion,
+            section=self.section_read,
+            comment='test_comment_fej(NF§kfePOF383o8DN')
+        comment.save()
+
+        response = self.client.delete(
+            reverse('motion-manage-comments', args=[self.motion.pk]),
+            {
+                'section_id': self.section_read.pk
+            },
+            format='json')
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(MotionComment.objects.count(), 1)
+        comment = MotionComment.objects.get()
+        self.assertEqual(comment.comment, 'test_comment_fej(NF§kfePOF383o8DN')
+
+
+class TestMotionCommentSection(TestCase):
+    """
+    Tests creating, updating and deletion of comment sections.
+    """
+    def setUp(self):
+        self.client = APIClient()
+        self.client.login(username='admin', password='admin')
+
+        self.admin = get_user_model().objects.get()
+        self.group_in = get_group_model().objects.get(pk=4)
+        self.group_out = get_group_model().objects.get(pk=2)  # The admin should not be in this group
+
+    def test_retrieve(self):
+        """
+        Checks, if the sections can be seen by a manager.
+        """
+        section = MotionCommentSection(name='test_name_f3jOF3m8fp.<qiqmf32=')
+        section.save()
+
+        response = self.client.get(reverse('motioncommentsection-list'))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue(isinstance(response.data, list))
+        self.assertEqual(len(response.data), 1)
+        self.assertEqual(response.data[0]['name'], 'test_name_f3jOF3m8fp.<qiqmf32=')
+
+    def test_retrieve_non_manager_with_read_permission(self):
+        """
+        Checks, if the sections can be seen by a non manager, but he is in
+        one of the read_groups.
+        """
+        self.admin.groups.remove(self.group_in)  # group_in has motions.can_manage permission
+        self.admin.groups.add(self.group_out)  # group_out does not.
+
+        section = MotionCommentSection(name='test_name_f3mMD28LMcm29Coelwcm')
+        section.save()
+        section.read_groups.add(self.group_out, self.group_in)
+
+        response = self.client.get(reverse('motioncommentsection-list'))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(len(response.data), 1)
+        self.assertEqual(response.data[0]['name'], 'test_name_f3mMD28LMcm29Coelwcm')
+
+    def test_retrieve_non_manager_no_read_permission(self):
+        """
+        Checks, if sections are removed, if the user is a non manager and is in
+        any of the read_groups.
+        """
+        self.admin.groups.remove(self.group_in)
+
+        section = MotionCommentSection(name='test_name_f3jOF3m8fp.<qiqmf32=')
+        section.save()
+        section.read_groups.add(self.group_out)
+
+        response = self.client.get(reverse('motioncommentsection-list'))
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue(isinstance(response.data, list))
+        self.assertEqual(len(response.data), 0)
+
+    def test_create(self):
+        """
+        Create a section just with a name.
+        """
+        response = self.client.post(
+            reverse('motioncommentsection-list'),
+            {'name': 'test_name_ekjfen3n)F§zn83f§Fge'})
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        self.assertEqual(
+            MotionCommentSection.objects.get().name,
+            'test_name_ekjfen3n)F§zn83f§Fge')
+
+    def test_create_no_permission(self):
+        """
+        Try to create a section without can_manage permissions.
+        """
+        self.admin.groups.remove(self.group_in)
+
+        response = self.client.post(
+            reverse('motioncommentsection-list'),
+            {'name': 'test_name_wfl3jlkcmlq23ucn7eiq'})
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(MotionCommentSection.objects.count(), 0)
+
+    def test_create_no_name(self):
+        """
+        Create a section without a name. This should fail, because a name is required.
+        """
+        response = self.client.post(reverse('motioncommentsection-list'), {})
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(MotionCommentSection.objects.count(), 0)
+        self.assertEqual(response.data['name'][0], 'This field is required.')
+
+    def test_create_with_groups(self):
+        """
+        Create a section with name and both groups.
+        """
+        response = self.client.post(
+            reverse('motioncommentsection-list'),
+            {
+                'name': 'test_name_fg4kmFn73FhFk327f/3h',
+                'read_groups_id': [2, 3],
+                'write_groups_id': [3, 4],
+            })
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        comment = MotionCommentSection.objects.get()
+        self.assertEqual(
+            comment.name,
+            'test_name_fg4kmFn73FhFk327f/3h')
+        self.assertEqual(
+            list(comment.read_groups.values_list('pk', flat=True)),
+            [2, 3])
+        self.assertEqual(
+            list(comment.write_groups.values_list('pk', flat=True)),
+            [3, 4])
+
+    def test_create_with_one_group(self):
+        """
+        Create a section with a name and write_groups.
+        """
+        response = self.client.post(
+            reverse('motioncommentsection-list'),
+            {
+                'name': 'test_name_ekjfen3n)F§zn83f§Fge',
+                'write_groups_id': [1, 3],
+            })
+        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        comment = MotionCommentSection.objects.get()
+        self.assertEqual(
+            comment.name,
+            'test_name_ekjfen3n)F§zn83f§Fge')
+        self.assertEqual(comment.read_groups.count(), 0)
+        self.assertEqual(
+            list(comment.write_groups.values_list('pk', flat=True)),
+            [1, 3])
+
+    def test_create_with_non_existing_group(self):
+        """
+        Create a section with some non existing groups. This should fail.
+        """
+        response = self.client.post(
+            reverse('motioncommentsection-list'),
+            {
+                'name': 'test_name_4gnUVnF§29FnH3287fhG',
+                'write_groups_id': [42, 1, 8],
+            })
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(MotionCommentSection.objects.count(), 0)
+        self.assertEqual(
+            response.data['write_groups_id'][0],
+            'Invalid pk "42" - object does not exist.')
+
+    def test_update(self):
+        """
+        Update a section name.
+        """
+        section = MotionCommentSection(name='test_name_dlfgNDf37ND(g3fNf43g')
+        section.save()
+
+        response = self.client.put(
+            reverse('motioncommentsection-detail', args=[section.pk]),
+            {'name': 'test_name_ekjfen3n)F§zn83f§Fge'})
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        self.assertEqual(
+            MotionCommentSection.objects.get().name,
+            'test_name_ekjfen3n)F§zn83f§Fge')
+
+    def test_update_groups(self):
+        """
+        Update one of the groups.
+        """
+        section = MotionCommentSection(name='test_name_f3jFq3hShf/(fh2qlPOp')
+        section.save()
+        section.read_groups.add(2)
+        section.write_groups.add(3)
+
+        response = self.client.patch(
+            reverse('motioncommentsection-detail', args=[section.pk]),
+            {
+                'name': 'test_name_gkk3FhfhpmQMhC,Y378c',
+                'read_groups_id': [2, 4],
+            })
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        comment = MotionCommentSection.objects.get()
+        self.assertEqual(comment.name, 'test_name_gkk3FhfhpmQMhC,Y378c')
+        self.assertEqual(
+            list(comment.read_groups.values_list('pk', flat=True)),
+            [2, 4])
+        self.assertEqual(
+            list(comment.write_groups.values_list('pk', flat=True)),
+            [3])
+
+    def test_update_no_permission(self):
+        """
+        Try to update a section without can_manage permissions.
+        """
+        self.admin.groups.remove(self.group_in)
+
+        section = MotionCommentSection(name='test_name_wl2oxmmhe/2kd92lwPSi')
+        section.save()
+
+        response = self.client.patch(
+            reverse('motioncommentsection-list'),
+            {'name': 'test_name_2slmDMwmqqcmC92mcklw'})
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+        self.assertEqual(
+            MotionCommentSection.objects.get().name,
+            'test_name_wl2oxmmhe/2kd92lwPSi')
+
+    def test_delete(self):
+        """
+        Delete a section.
+        """
+        section = MotionCommentSection(name='test_name_ecMCq;ymwuZZ723kD)2k')
+        section.save()
+
+        response = self.client.delete(reverse('motioncommentsection-detail', args=[section.pk]))
+        self.assertEqual(response.status_code, status.HTTP_204_NO_CONTENT)
+        self.assertEqual(MotionCommentSection.objects.count(), 0)
+
+    def test_delete_non_existing_section(self):
+        """
+        Delete a non existing section.
+        """
+        response = self.client.delete(reverse('motioncommentsection-detail', args=[2]))
+        self.assertEqual(response.status_code, status.HTTP_404_NOT_FOUND)
+        self.assertEqual(MotionCommentSection.objects.count(), 0)
+
+    def test_delete_with_existing_comments(self):
+        """
+        Delete a section with existing comments. This should fail, because sections
+        are protected.
+        """
+        section = MotionCommentSection(name='test_name_ecMCq;ymwuZZ723kD)2k')
+        section.save()
+
+        motion = Motion(
+            title='test_title_SlqfMw(waso0saWMPqcZ',
+            text='test_text_f30skclqS9wWF=xdfaSL')
+        motion.save()
+
+        comment = MotionComment(
+            comment='test_comment_dlkMD23m)(D9020m0/Zd',
+            motion=motion,
+            section=section)
+        comment.save()
+
+        response = self.client.delete(reverse('motioncommentsection-detail', args=[section.pk]))
+        self.assertTrue('test_title_SlqfMw(waso0saWMPqcZ' in response.data['detail'])
+        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+
+    def test_delete_no_permission(self):
+        """
+        Try to delete a section without can_manage permissions
+        """
+        self.admin.groups.remove(self.group_in)
+
+        section = MotionCommentSection(name='test_name_wl2oxmmhe/2kd92lwPSi')
+        section.save()
+
+        response = self.client.delete(reverse('motioncommentsection-detail', args=[section.pk]))
+        self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
+        self.assertEqual(MotionCommentSection.objects.count(), 1)
+
+
 class CreateMotionChangeRecommendation(TestCase):
     """
     Tests motion change recommendation creation.
@@ -772,7 +1127,7 @@ class CreateMotionChangeRecommendation(TestCase):
             reverse('motionchangerecommendation-list'),
             {'line_from': '5',
              'line_to': '7',
-             'motion_version_id': '1',
+             'motion_id': '1',
              'text': '<p>New test</p>',
              'type': '0'})
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
@@ -785,7 +1140,7 @@ class CreateMotionChangeRecommendation(TestCase):
             reverse('motionchangerecommendation-list'),
             {'line_from': '5',
              'line_to': '7',
-             'motion_version_id': '1',
+             'motion_id': '1',
              'text': '<p>New test</p>',
              'type': '0'})
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
@@ -794,7 +1149,7 @@ class CreateMotionChangeRecommendation(TestCase):
             reverse('motionchangerecommendation-list'),
             {'line_from': '3',
              'line_to': '6',
-             'motion_version_id': '1',
+             'motion_id': '1',
              'text': '<p>New test</p>',
              'type': '0'})
         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
@@ -813,7 +1168,7 @@ class CreateMotionChangeRecommendation(TestCase):
             reverse('motionchangerecommendation-list'),
             {'line_from': '5',
              'line_to': '7',
-             'motion_version_id': '1',
+             'motion_id': '1',
              'text': '<p>New test</p>',
              'type': '0'})
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)
@@ -822,7 +1177,7 @@ class CreateMotionChangeRecommendation(TestCase):
             reverse('motionchangerecommendation-list'),
             {'line_from': '3',
              'line_to': '6',
-             'motion_version_id': '2',
+             'motion_id': '2',
              'text': '<p>New test</p>',
              'type': '0'})
         self.assertEqual(response.status_code, status.HTTP_201_CREATED)

tests/integration/users/test_viewset.py

@@ -480,8 +480,6 @@ class GroupUpdate(TestCase):
             'motions.can_create',
             'motions.can_manage',
             'motions.can_see',
-            'motions.can_manage_comments',
-            'motions.can_see_comments',
             'motions.can_support',
             'users.can_manage',
             'users.can_see_extra_data',

tests/old/motions/test_models.py

@@ -12,48 +12,6 @@ class ModelTest(TestCase):
         # Use the simple workflow
         self.workflow = Workflow.objects.get(pk=1)
 
-    def test_create_new_version(self):
-        motion = self.motion
-        self.assertEqual(motion.versions.count(), 1)
-
-        # new data, but no new version
-        motion.title = 'new title'
-        motion.save()
-        self.assertEqual(motion.versions.count(), 1)
-
-        # new data and new version
-        motion.text = 'new text'
-        motion.save(use_version=motion.get_new_version())
-        self.assertEqual(motion.versions.count(), 2)
-        self.assertEqual(motion.title, 'new title')
-        self.assertEqual(motion.text, 'new text')
-
-    def test_version_data(self):
-        motion = Motion()
-        self.assertEqual(motion.title, '')
-        with self.assertRaises(AttributeError):
-            self._title
-
-        motion.title = 'title'
-        self.assertEqual(motion._title, 'title')
-
-        motion.text = 'text'
-        self.assertEqual(motion._text, 'text')
-
-        motion.reason = 'reason'
-        self.assertEqual(motion._reason, 'reason')
-
-    def test_version(self):
-        motion = self.motion
-
-        motion.title = 'v2'
-        motion.save(use_version=motion.get_new_version())
-        motion.title = 'v3'
-        motion.save(use_version=motion.get_new_version())
-        with self.assertRaises(AttributeError):
-            self._title
-        self.assertEqual(motion.title, 'v3')
-
     def test_supporter(self):
         self.assertFalse(self.motion.is_supporter(self.test_user))
         self.motion.supporters.add(self.test_user)
@@ -97,37 +55,6 @@ class ModelTest(TestCase):
         Motion.objects.create(title='foo', text='bar', identifier='')
         Motion.objects.create(title='foo2', text='bar2', identifier='')
 
-    def test_do_not_create_new_version_when_permit_old_version(self):
-        motion = Motion()
-        motion.title = 'foo'
-        motion.text = 'bar'
-        motion.save()
-        first_version = motion.get_last_version()
-
-        motion = Motion.objects.get(pk=motion.pk)
-        motion.title = 'New Title'
-        motion.save(use_version=motion.get_new_version())
-        new_version = motion.get_last_version()
-        self.assertEqual(motion.versions.count(), 2)
-
-        motion.active_version = new_version
-        motion.save()
-        self.assertEqual(motion.versions.count(), 2)
-
-        motion.active_version = first_version
-        motion.save(use_version=False)
-        self.assertEqual(motion.versions.count(), 2)
-
-    def test_unicode_with_no_active_version(self):
-        motion = Motion.objects.create(
-            title='test_title_Koowoh1ISheemeey1air',
-            text='test_text_zieFohph0doChi1Uiyoh',
-            identifier='test_identifier_VohT1hu9uhiSh6ooVBFS')
-        motion.active_version = None
-        motion.save(update_fields=['active_version'])
-        # motion.__unicode__() raised an AttributeError
-        self.assertEqual(str(motion), 'test_title_Koowoh1ISheemeey1air')
-
     def test_is_amendment(self):
         config['motions_amendments_enabled'] = True
         amendment = Motion.objects.create(title='amendment', parent=self.motion)

tests/unit/motions/test_models.py

@@ -1,6 +1,6 @@
 from unittest import TestCase
 
-from openslides.motions.models import MotionChangeRecommendation, MotionVersion
+from openslides.motions.models import Motion, MotionChangeRecommendation
 
 
 class MotionChangeRecommendationTest(TestCase):
@@ -8,12 +8,12 @@ class MotionChangeRecommendationTest(TestCase):
         """
         Tests that a change recommendation directly before another one can be created
         """
-        version = MotionVersion()
+        motion = Motion()
         existing_recommendation = MotionChangeRecommendation()
         existing_recommendation.line_from = 5
         existing_recommendation.line_to = 7
         existing_recommendation.rejected = False
-        existing_recommendation.motion_version = version
+        existing_recommendation.motion = motion
         other_recommendations = [existing_recommendation]
 
         new_recommendation1 = MotionChangeRecommendation()

tests/unit/motions/test_views.py

@@ -22,33 +22,9 @@ class MotionViewSetUpdate(TestCase):
     @patch('openslides.motions.views.config')
     def test_simple_update(self, mock_config, mock_has_perm, mock_icd):
         self.request.user = 1
-        self.request.data.get.return_value = versioning_mock = MagicMock()
+        self.request.data.get.return_value = MagicMock()
         mock_has_perm.return_value = True
 
         self.view_instance.update(self.request)
 
-        self.mock_serializer.save.assert_called_with(disable_versioning=versioning_mock)
+        self.mock_serializer.save.assert_called()
-
-
-class MotionViewSetManageVersion(TestCase):
-    """
-    Tests views of MotionViewSet to manage versions.
-    """
-    def setUp(self):
-        self.request = MagicMock()
-        self.view_instance = MotionViewSet()
-        self.view_instance.request = self.request
-        self.view_instance.get_object = get_object_mock = MagicMock()
-        get_object_mock.return_value = self.mock_motion = MagicMock()
-
-    def test_activate_version(self):
-        self.request.method = 'PUT'
-        self.request.user.has_perm.return_value = True
-        self.view_instance.manage_version(self.request)
-        self.mock_motion.save.assert_called_with(update_fields=['active_version'])
-
-    def test_delete_version(self):
-        self.request.method = 'DELETE'
-        self.request.user.has_perm.return_value = True
-        self.view_instance.manage_version(self.request)
-        self.mock_motion.versions.get.return_value.delete.assert_called_with()