diff --git a/.travis.yml b/.travis.yml index 4de21867d..2e33f6453 100644 --- a/.travis.yml +++ b/.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 - - cd client && npm run-script lint && cd .. \ No newline at end of file + - pytest tests/old/ tests/integration/ tests/unit/ --cov --cov-fail-under=75 diff --git a/client/src/app/shared/models/motions/motion.ts b/client/src/app/shared/models/motions/motion.ts index dced05639..b3057b001 100644 --- a/client/src/app/shared/models/motions/motion.ts +++ b/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 active_version: number; + public title: string; + 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 => { diff --git a/client/src/app/site/motions/motion-detail/motion-detail.component.html b/client/src/app/site/motions/motion-detail/motion-detail.component.html index 6d954e780..5b0b425c5 100644 --- a/client/src/app/site/motions/motion-detail/motion-detail.component.html +++ b/client/src/app/site/motions/motion-detail/motion-detail.component.html @@ -12,8 +12,8 @@ {{motion.identifier}} {{metaInfoForm.get('identifier').value}} : - {{motion.currentTitle}} - {{contentForm.get('currentTitle').value}} + {{motion.title}} + {{contentForm.get('title').value}}
by {{motion.submitterAsUser}} @@ -223,12 +223,12 @@
-
+
-

{{motion.currentTitle}}

+

{{motion.title}}

- +
@@ -237,21 +237,21 @@

The assembly may decide:

-
+
- + -
+

Reason

-
+
- +
diff --git a/client/src/app/site/motions/motion-detail/motion-detail.component.ts b/client/src/app/site/motions/motion-detail/motion-detail.component.ts index 8a39d7891..81faa3488 100644 --- a/client/src/app/site/motions/motion-detail/motion-detail.component.ts +++ b/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, - currentText: formMotion.currentText, - currentReason: formMotion.currentReason + title: formMotion.title, + text: formMotion.text, + reason: formMotion.reason }); } @@ -134,9 +134,9 @@ export class MotionDetailComponent extends BaseComponent implements OnInit { origin: [''] }); this.contentForm = this.formBuilder.group({ - currentTitle: ['', Validators.required], - currentText: ['', Validators.required], - currentReason: [''] + title: ['', Validators.required], + text: ['', Validators.required], + 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) { diff --git a/client/src/app/site/start/start.component.ts b/client/src/app/site/start/start.component.ts index b873d10ea..a9e34e7e0 100644 --- a/client/src/app/site/start/start.component.ts +++ b/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 diff --git a/openslides/core/config.py b/openslides/core/config.py index bf28fae1d..1f5eacc06 100644 --- a/openslides/core/config.py +++ b/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.')) diff --git a/openslides/global_settings.py b/openslides/global_settings.py index c8c79a02b..4235ebba1 100644 --- a/openslides/global_settings.py +++ b/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 diff --git a/openslides/motions/access_permissions.py b/openslides/motions/access_permissions.py index 37c629358..d072bee0d 100644 --- a/openslides/motions/access_permissions.py +++ b/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'): - # Provide access to all fields. - motion = full - else: - # Set private comment fields to None. - full_copy = deepcopy(full) - 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) + full_copy = deepcopy(full) + full_copy['comments'] = [] + for comment in full['comments']: + if in_some_groups(user, comment['read_groups_id']): + full_copy['comments'].append(comment) + data.append(full_copy) 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. - if full.get('comments') is not None: - full_copy = deepcopy(full) - 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) - + full_copy = deepcopy(full) + full_copy['comments'] = [] + data.append(full_copy) 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. diff --git a/openslides/motions/apps.py b/openslides/motions/apps.py index 674c247b2..2acea6781 100644 --- a/openslides/motions/apps.py +++ b/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) diff --git a/openslides/motions/config_variables.py b/openslides/motions/config_variables.py index 3428ebae5..992edafb5 100644 --- a/openslides/motions/config_variables.py +++ b/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( diff --git a/openslides/motions/migrations/0011_motion_version.py b/openslides/motions/migrations/0011_motion_version.py new file mode 100644 index 000000000..01758ffb0 --- /dev/null +++ b/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', + ), + ] diff --git a/openslides/motions/migrations/0012_motion_comments.py b/openslides/motions/migrations/0012_motion_comments.py new file mode 100644 index 000000000..3e1c5a5fc --- /dev/null +++ b/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'), + ), + ] diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 63c26de97..0223d624d 100644 --- a/openslides/motions/models.py +++ b/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 ( - ugettext as _, - ugettext_lazy, - ugettext_noop, -) +from django.utils.translation import ugettext as _, 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( - 'MotionVersion', - on_delete=models.SET_NULL, - null=True, - related_name="active_version") - """ - Points to a specific version. + title = models.CharField(max_length=255) + """The title of a motion.""" - Used be the permitted-version-system to deside which version is the active - version. Could also be used to only choose a specific version as a default - version. Like the sighted versions on Wikipedia. + 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.""" 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( - MotionVersion, + motion = models.ForeignKey( + 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. diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 51692cc3b..530aaa807 100644 --- a/openslides/motions/serializers.py +++ b/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 = MotionCommentsJSONSerializerField(required=False) + comments = MotionCommentSerializer(many=True, read_only=True) 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) - reason = 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) state_required_permission_to_see = SerializerMethodField() - text = CharField(write_only=True, allow_blank=True) - title = CharField(max_length=255, write_only=True) - amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False, write_only=True) - versions = MotionVersionSerializer(many=True, read_only=True) + text = CharField(allow_blank=True) + title = CharField(max_length=255) + amendment_paragraphs = AmendmentParagraphsJSONSerializerField(required=False) 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. - for key in ('identifier', 'category', 'motion_block', 'origin', 'comments'): - if key in validated_data.keys(): - setattr(motion, key, validated_data[key]) + workflow_id = None + if 'workflow_id' in validated_data: + workflow_id = validated_data.pop('workflow_id') + + 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. - 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 + return result def get_state_required_permission_to_see(self, motion): """ diff --git a/openslides/motions/signals.py b/openslides/motions/signals.py index 41605faf6..e7abc8032 100644 --- a/openslides/motions/signals.py +++ b/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, - versioning=True, - leave_old_version_active=True) + allow_submitter_edit=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) diff --git a/openslides/motions/views.py b/openslides/motions/views.py index a8e026986..ed6a333f1 100644 --- a/openslides/motions/views.py +++ b/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 (has_perm(request.user, 'motions.can_see_comments') and - has_perm(request.user, 'motions.can_manage_comments'))): + not (motion.is_submitter(request.user) and motion.state.allow_submitter_edit)): 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 = [ - 'comments', # This is checked later. - ] + whitelist: List[str] = [] # 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']) - def manage_version(self, request, pk=None): + @detail_route(methods=['POST', 'DELETE']) + def manage_comments(self, request, pk=None): """ - Special view endpoint to permit and delete a version of a motion. - - Send PUT {'version_number': } to permit and DELETE - {'version_number': } to delete a version. Deleting the - active version is not allowed. Only managers can use this view. + Create, update and delete motin comments. + Send a post request with {'section_id': , 'comment': ''} to create + a new comment or update an existing comment. + Send a delete request with just {'section_id': } to delete the comment. + 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) - except MotionVersion.DoesNotExist: - raise Http404('Version %s not found.' % version_number) + section = MotionCommentSection.objects.get(pk=section_id) + except MotionCommentSection.DoesNotExist: + raise ValidationError({'detail': _('A comment section with id {} does not exist').format(section_id)}) - # Permit or delete version. - if request.method == 'PUT': - # Permit version. - motion.active_version = version - motion.save(update_fields=['active_version']) - motion.write_log( - message_list=[ugettext_noop('Version'), - ' %d ' % version.version_number, - ugettext_noop('permitted')], - person=self.request.user) - message = _('Version %d permitted successfully.') % version.version_number - else: - # Delete version. - # request.method == 'DELETE' - if version == motion.active_version: - raise ValidationError({'detail': _('You can not delete the active version of a motion.')}) - version.delete() - motion.write_log( - message_list=[ugettext_noop('Version'), - ' %d ' % version.version_number, - ugettext_noop('deleted')], - person=self.request.user) - message = _('Version %d deleted successfully.') % version.version_number + # the request user needs to see and write to the comment section + if (not in_some_groups(request.user, list(section.read_groups.values_list('pk', flat=True))) or + not in_some_groups(request.user, list(section.write_groups.values_list('pk', flat=True)))): + raise ValidationError({'detail': _('You are not allowed to see or write to the comment section.')}) + + if request.method == 'POST': # Create or update + # validate comment + comment_value = request.data.get('comment', '') + if not isinstance(comment_value, str): + raise ValidationError({'detail': _('The comment should be a string.')}) + + comment, created = MotionComment.objects.get_or_create( + motion=motion, + section=section, + defaults={ + 'comment': comment_value}) + if not created: + comment.comment = comment_value + comment.save() + + # write log + motion.write_log( + [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. - 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 + motion.state_extension = extension # 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? diff --git a/openslides/users/signals.py b/openslides/users/signals.py index fdbf84d1e..ea2447c92 100644 --- a/openslides/users/signals.py +++ b/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'], diff --git a/openslides/utils/auth.py b/openslides/utils/auth.py index 4d818db3b..3a5914c0e 100644 --- a/openslides/utils/auth.py +++ b/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. - default_group = CollectionElement.from_values(group_collection_string, 1) + # Use the permissions from the default group. + 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. - group_ids = user.get_full_data()['groups_id'] or [1] + # 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: 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. diff --git a/tests/integration/agenda/test_viewset.py b/tests/integration/agenda/test_viewset.py index 6c810a0fc..678aa49c9 100644 --- a/tests/integration/agenda/test_viewset.py +++ b/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): diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 26e8bf13a..edba5ecc8 100644 --- a/tests/integration/motions/test_viewset.py +++ b/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.New test

', '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': '

New test

', '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': '

New test

', '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': '

New test

', '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': '

New test

', 'type': '0'}) self.assertEqual(response.status_code, status.HTTP_201_CREATED) diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index f02c57846..ac124113f 100644 --- a/tests/integration/users/test_viewset.py +++ b/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', diff --git a/tests/old/motions/test_models.py b/tests/old/motions/test_models.py index 18ab6f696..9f742553c 100644 --- a/tests/old/motions/test_models.py +++ b/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) diff --git a/tests/unit/motions/test_models.py b/tests/unit/motions/test_models.py index f8c2bddba..42fb72d04 100644 --- a/tests/unit/motions/test_models.py +++ b/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() diff --git a/tests/unit/motions/test_views.py b/tests/unit/motions/test_views.py index 06275498a..e50734d7d 100644 --- a/tests/unit/motions/test_views.py +++ b/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) - - -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() + self.mock_serializer.save.assert_called()