diff --git a/CHANGELOG b/CHANGELOG index c05b48c08..dc1f6d151 100644 --- a/CHANGELOG +++ b/CHANGELOG @@ -14,36 +14,36 @@ Agenda: Motions: - New export dialog [#3185]. -- New feature: Personal notes for motions [#3190]. +- New feature: Personal notes for motions [#3190, #3267]. - Fixed issue when creating/deleting motion comment fields in the settings [#3187]. - Fixed empty motion comment field in motion update form [#3194]. - Removed server side image to base64 transformation and added local transformation [#3181] -- Added support for export motions in a zip archive [#3189]. -- Performance improvement for zip creation [#3251] -- Bugfix: changing motion line length did not invalidate cache [#3202] +- Added support for export motions in a ZIP archive [#3189]. +- Performance improvement for ZIP creation [#3251]. +- Bugfix: Changing motion line length did not invalidate cache [#3202]. - Bugfix: Added more distance in motion PDF for DEL-tags in new lines [#3211]. - Added warning message if an edit dialog was already opened by another client [#3212]. - Reworked DOCX export parser and added comments to DOCX [#3258]. -- New pdf export for personal note and comments [#3239]. +- New PDF export for personal note and comments [#3239]. Users: - User without permission to see users can now see agenda item speakers, motion submitters and supporters, assignment candidates, mediafile uploader and chat message users if they have the respective - permissions [#3191]. + permissions [#3191, #3233]. - Added support for password validation using Django or custom validators e. g. for minimum password length [#3200]. -- Fixed compare of duplicated users while csv user import [#3201]. +- Fixed compare of duplicated users while CSV user import [#3201]. - Added fast mass import for users [#3290]. Core: - No reload on logoff. OpenSlides is now a full single page application [#3172]. - Adding support for choosing image files as logos for projector and - pdf [#3184, #3207, #3208]. + PDF [#3184, #3207, #3208]. - Fixing error when clearing empty chat [#3199]. - Added notify system [#3212]. - Enhanced performance esp. for server restart and first connection of all @@ -51,14 +51,14 @@ Core: - Fixes autoupdate bug for a user without user.can_see_name permission [#3233]. Mediafiles: -- Fixed reloading of PDF on page change [#3274] +- Fixed reloading of PDF on page change [#3274]. General: - Switched from npm to Yarn [#3188]. - Several bugfixes and minor improvements. -- Improved performance for pdf generation significantly (by upgrading +- Improved performance for PDF generation significantly (by upgrading to pdfmake 0.1.30) [#3278, #3285]. -- Bugfixes for PDF creation [#3227, #3251, #3279, #3286] +- Bugfixes for PDF creation [#3227, #3251, #3279, #3286]. Version 2.1.1 (2017-04-05) diff --git a/openslides/global_settings.py b/openslides/global_settings.py index f5a7124cf..e47f8fe8a 100644 --- a/openslides/global_settings.py +++ b/openslides/global_settings.py @@ -92,8 +92,6 @@ STATICFILES_DIRS = [ AUTH_USER_MODEL = 'users.User' -AUTH_PERSONAL_NOTE_MODEL = 'users.PersonalNote' - 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 473133b0f..dd3d7065e 100644 --- a/openslides/motions/access_permissions.py +++ b/openslides/motions/access_permissions.py @@ -71,16 +71,6 @@ class MotionAccessPermissions(BaseAccessPermissions): # No data in range. Just do nothing. pass motion = full_copy - - # Now filter personal notes. - motion = motion.copy() - motion['personal_notes'] = [] - if user is not None: - for personal_note in full.get('personal_notes', []): - if personal_note.get('user_id') == user.id: - motion['personal_notes'].append(personal_note) - break - data.append(motion) else: data = [] diff --git a/openslides/motions/models.py b/openslides/motions/models.py index 06f1b29a3..4c72872f3 100644 --- a/openslides/motions/models.py +++ b/openslides/motions/models.py @@ -50,8 +50,7 @@ class MotionManager(models.Manager): 'attachments', 'tags', 'submitters', - 'supporters', - 'personal_notes')) + 'supporters')) class Motion(RESTModelMixin, models.Model): @@ -171,8 +170,6 @@ class Motion(RESTModelMixin, models.Model): Configurable fields for comments. Contains a list of strings. """ - personal_notes = GenericRelation(settings.AUTH_PERSONAL_NOTE_MODEL, related_name='motions') - # 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') @@ -642,14 +639,6 @@ class Motion(RESTModelMixin, models.Model): """ return self.agenda_item.pk - def set_personal_note(self, user, note=None, star=None, skip_autoupdate=False): - """ - Saves or overrides a personal note to this motion for a given user. - """ - user.set_personal_note(self, note, star) - if not skip_autoupdate: - inform_changed_data(self) - def write_log(self, message_list, person=None, skip_autoupdate=False): """ Write a log message. diff --git a/openslides/motions/serializers.py b/openslides/motions/serializers.py index 47e25bd01..319fde060 100644 --- a/openslides/motions/serializers.py +++ b/openslides/motions/serializers.py @@ -269,7 +269,6 @@ class MotionSerializer(ModelSerializer): """ active_version = PrimaryKeyRelatedField(read_only=True) comments = MotionCommentsJSONSerializerField(required=False) - personal_notes = SerializerMethodField() log_messages = MotionLogSerializer(many=True, read_only=True) polls = MotionPollSerializer(many=True, read_only=True) reason = CharField(allow_blank=True, required=False, write_only=True) @@ -300,7 +299,6 @@ class MotionSerializer(ModelSerializer): 'submitters', 'supporters', 'comments', - 'personal_notes', 'state', 'state_required_permission_to_see', 'workflow_id', @@ -387,12 +385,6 @@ class MotionSerializer(ModelSerializer): return motion - def get_personal_notes(self, motion): - """ - Returns the personal notes of all users. - """ - return [personal_note.get_data() for personal_note in motion.personal_notes.all()] - def get_state_required_permission_to_see(self, motion): """ Returns the permission (as string) that is required for non diff --git a/openslides/motions/views.py b/openslides/motions/views.py index b527a55f4..cec48e6a4 100644 --- a/openslides/motions/views.py +++ b/openslides/motions/views.py @@ -63,7 +63,7 @@ class MotionViewSet(ModelViewSet): """ if self.action in ('list', 'retrieve'): result = self.get_access_permissions().check_permissions(self.request.user) - elif self.action in ('metadata', 'partial_update', 'update', 'set_personal_note'): + elif self.action in ('metadata', 'partial_update', 'update'): result = has_perm(self.request.user, 'motions.can_see') # For partial_update and update requests the rest of the check is # done in the update method. See below. @@ -372,20 +372,6 @@ class MotionViewSet(ModelViewSet): person=request.user) return Response({'detail': message}) - @detail_route(methods=['put']) - def set_personal_note(self, request, pk=None): - """ - Special view endpoint to save a personal note to a motion. - - Send PUT with {'note': , 'star': True|False}. - """ - motion = self.get_object() - if not request.user.is_authenticated(): - raise ValidationError({'detail': _('Anonymous users are not able to set personal notes.')}) - motion.set_personal_note(request.user, request.data.get('note'), bool(request.data.get('star'))) - message = _('Your personal note was successfully saved.') - return Response({'detail': message}) - @detail_route(methods=['post']) def create_poll(self, request, pk=None): """ diff --git a/openslides/users/access_permissions.py b/openslides/users/access_permissions.py index 246aceb28..956d596e5 100644 --- a/openslides/users/access_permissions.py +++ b/openslides/users/access_permissions.py @@ -136,3 +136,51 @@ class GroupAccessPermissions(BaseAccessPermissions): from .serializers import GroupSerializer return GroupSerializer + + +class PersonalNoteAccessPermissions(BaseAccessPermissions): + """ + Access permissions container for personal notes. Every authenticated user + can handle personal notes. + """ + def check_permissions(self, user): + """ + Returns True if the user has read access model instances. + """ + # Every authenticated user can retrieve personal notes. + return not isinstance(user, AnonymousUser) + + def get_serializer_class(self, user=None): + """ + Returns serializer class. + """ + from .serializers import PersonalNoteSerializer + + return PersonalNoteSerializer + + def get_restricted_data(self, container, user): + """ + Returns the restricted serialized data for the instance prepared + for the user. Everybody gets only his own personal notes. + """ + # Expand full_data to a list if it is not one. + full_data = container.get_full_data() if isinstance(container, Collection) else [container.get_full_data()] + + # Parse data. + for full in full_data: + if full['user_id'] == user.id: + data = [full] + break + else: + data = [] + + # Reduce result to a single item or None if it was not a collection at + # the beginning of the method. + if isinstance(container, Collection): + restricted_data = data + elif data: + restricted_data = data[0] + else: + restricted_data = None + + return restricted_data diff --git a/openslides/users/apps.py b/openslides/users/apps.py index 7bbd60678..e4d7deb69 100644 --- a/openslides/users/apps.py +++ b/openslides/users/apps.py @@ -20,7 +20,7 @@ class UsersAppConfig(AppConfig): from ..utils.rest_api import router from .config_variables import get_config_variables from .signals import create_builtin_groups_and_admin, get_permission_change_data - from .views import GroupViewSet, UserViewSet + from .views import GroupViewSet, PersonalNoteViewSet, UserViewSet # Define config variables config.update_config_variables(get_config_variables()) @@ -36,11 +36,12 @@ class UsersAppConfig(AppConfig): # Register viewsets. router.register(self.get_model('User').get_collection_string(), UserViewSet) router.register(self.get_model('Group').get_collection_string(), GroupViewSet) + router.register(self.get_model('PersonalNote').get_collection_string(), PersonalNoteViewSet) def get_startup_elements(self): """ Yields all collections required on startup i. e. opening the websocket connection. """ - for model in ('User', 'Group'): + for model in ('User', 'Group', 'PersonalNote'): yield Collection(self.get_model(model).get_collection_string()) diff --git a/openslides/users/migrations/0005_personalnote_rework.py b/openslides/users/migrations/0005_personalnote_rework.py new file mode 100644 index 000000000..e1bbd66c1 --- /dev/null +++ b/openslides/users/migrations/0005_personalnote_rework.py @@ -0,0 +1,43 @@ +# -*- coding: utf-8 -*- +# Generated by Django 1.10.7 on 2017-05-23 11:25 +from __future__ import unicode_literals + +import django.db.models.deletion +import jsonfield.fields +from django.conf import settings +from django.db import migrations, models + +import openslides.utils.models + + +class Migration(migrations.Migration): + + dependencies = [ + ('users', '0004_personalnote'), + ] + + operations = [ + migrations.RemoveField( + model_name='personalnote', + name='content_type', + ), + migrations.RemoveField( + model_name='personalnote', + name='user', + ), + migrations.DeleteModel( + name='PersonalNote', + ), + migrations.CreateModel( + name='PersonalNote', + fields=[ + ('id', models.AutoField(auto_created=True, primary_key=True, serialize=False, verbose_name='ID')), + ('notes', jsonfield.fields.JSONField()), + ('user', models.OneToOneField(on_delete=django.db.models.deletion.CASCADE, to=settings.AUTH_USER_MODEL)), + ], + options={ + 'default_permissions': (), + }, + bases=(openslides.utils.models.RESTModelMixin, models.Model), + ), + ] diff --git a/openslides/users/models.py b/openslides/users/models.py index bb03ad205..c110567eb 100644 --- a/openslides/users/models.py +++ b/openslides/users/models.py @@ -9,47 +9,17 @@ from django.contrib.auth.models import ( Permission, PermissionsMixin, ) -from django.contrib.contenttypes.fields import GenericForeignKey -from django.contrib.contenttypes.models import ContentType from django.db import models from django.db.models import Prefetch, Q +from jsonfield import JSONField from ..utils.collection import CollectionElement from ..utils.models import RESTModelMixin -from .access_permissions import GroupAccessPermissions, UserAccessPermissions - - -class PersonalNote(models.Model): - """ - Model for personal notes and likes (stars) of a user concerning different - openslides models like motions. - - To use this in your app simply run e. g. - - user.set_personal_note(motion, note, star) - - in a setter view and add a SerializerMethodField to your serializer that - calls get_data for all users. - """ - user = models.ForeignKey('User', on_delete=models.CASCADE, related_name='personal_notes') - content_type = models.ForeignKey(ContentType, on_delete=models.CASCADE) - object_id = models.PositiveIntegerField() - content_object = GenericForeignKey() - note = models.TextField(blank=True) - star = models.BooleanField(default=False, blank=True) - - class Meta: - default_permissions = () - - def get_data(self): - """ - Returns note and star to be serialized in content object serializers. - """ - return { - 'user_id': self.user_id, - 'note': self.note, - 'star': self.star, - } +from .access_permissions import ( + GroupAccessPermissions, + PersonalNoteAccessPermissions, + UserAccessPermissions, +) class UserManager(BaseUserManager): @@ -246,29 +216,6 @@ class User(RESTModelMixin, PermissionsMixin, AbstractBaseUser): """ raise RuntimeError('Do not use user.has_perm() but use openslides.utils.auth.has_perm') - def set_personal_note(self, content_object, note=None, star=None): - """ - Saves or overrides a personal note for this user for a given object - like motion. - """ - changes = {} - if note is not None: - changes['note'] = note - if star is not None: - changes['star'] = star - if changes: - # TODO: This is prone to race-conditions in rare cases. Fix it. - personal_note, created = PersonalNote.objects.update_or_create( - user=self, - content_type=ContentType.objects.get_for_model(content_object), - object_id=content_object.id, - user_id=self.id, - defaults=changes, - ) - else: - personal_note = None - return personal_note - class GroupManager(GroupManager): """ @@ -295,3 +242,33 @@ class Group(RESTModelMixin, DjangoGroup): class Meta: default_permissions = () + + +class PersonalNoteManager(models.Manager): + """ + Customized model manager to support our get_full_queryset method. + """ + def get_full_queryset(self): + """ + Returns the normal queryset with all personal notes. In the background all + users are prefetched from the database. + """ + return self.get_queryset().select_related('user') + + +class PersonalNote(RESTModelMixin, models.Model): + """ + Model for personal notes (e. g. likes/stars) of a user concerning different + openslides objects like motions. + """ + access_permissions = PersonalNoteAccessPermissions() + + objects = PersonalNoteManager() + + user = models.OneToOneField( + User, + on_delete=models.CASCADE) + notes = JSONField() + + class Meta: + default_permissions = () diff --git a/openslides/users/serializers.py b/openslides/users/serializers.py index 5b82e068b..fcde2e24a 100644 --- a/openslides/users/serializers.py +++ b/openslides/users/serializers.py @@ -6,11 +6,12 @@ from django.utils.translation import ugettext_lazy from ..utils.autoupdate import inform_changed_data from ..utils.rest_api import ( IdPrimaryKeyRelatedField, + JSONField, ModelSerializer, RelatedField, ValidationError, ) -from .models import Group, User +from .models import Group, PersonalNote, User USERCANSEESERIALIZER_FIELDS = ( 'id', @@ -147,3 +148,15 @@ class GroupSerializer(ModelSerializer): """ instance = super().update(*args, **kwargs) return Group.objects.get(pk=instance.pk) + + +class PersonalNoteSerializer(ModelSerializer): + """ + Serializer for users.models.PersonalNote objects. + """ + notes = JSONField() + + class Meta: + model = PersonalNote + fields = ('id', 'user', 'notes', ) + read_only_fields = ('user', ) diff --git a/openslides/users/views.py b/openslides/users/views.py index ef63744d5..5169fa457 100644 --- a/openslides/users/views.py +++ b/openslides/users/views.py @@ -26,8 +26,12 @@ from ..utils.rest_api import ( status, ) from ..utils.views import APIView -from .access_permissions import GroupAccessPermissions, UserAccessPermissions -from .models import Group, User +from .access_permissions import ( + GroupAccessPermissions, + PersonalNoteAccessPermissions, + UserAccessPermissions, +) +from .models import Group, PersonalNote, User from .serializers import GroupSerializer, PermissionRelatedField @@ -268,6 +272,57 @@ class GroupViewSet(ModelViewSet): return Response(status=status.HTTP_204_NO_CONTENT) +class PersonalNoteViewSet(ModelViewSet): + """ + API endpoint for personal notes. + + There are the following views: metadata, list, retrieve, create, + partial_update, update, and destroy. + """ + access_permissions = PersonalNoteAccessPermissions() + queryset = PersonalNote.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 ('metadata', 'create', 'partial_update', 'update', 'destroy'): + # Every authenticated user can see metadata and create personal + # notes for himself and can manipulate only his own personal notes. + # See self.perform_create(), self.update() and self.destroy(). + result = self.request.user.is_authenticated() + else: + result = False + return result + + def perform_create(self, serializer): + """ + Customized method to inject the request.user into serializer's save + method so that the request.user can be saved into the model field. + """ + serializer.save(user=self.request.user) + + def update(self, request, *args, **kwargs): + """ + Customized method to ensure that every user can change only his own + personal notes. + """ + if self.get_object().user != self.request.user: + self.permission_denied(request) + return super().update(request, *args, **kwargs) + + def destroy(self, request, *args, **kwargs): + """ + Customized method to ensure that every user can delete only his own + personal notes. + """ + if self.get_object().user != self.request.user: + self.permission_denied(request) + return super().destroy(request, *args, **kwargs) + + # Special API views class UserLoginView(APIView): diff --git a/openslides/utils/rest_api.py b/openslides/utils/rest_api.py index e9487c65f..f7c94e455 100644 --- a/openslides/utils/rest_api.py +++ b/openslides/utils/rest_api.py @@ -17,6 +17,7 @@ from rest_framework.serializers import ( # noqa Field, FileField, IntegerField, + JSONField, ListField, ListSerializer, ManyRelatedField, diff --git a/tests/integration/motions/test_viewset.py b/tests/integration/motions/test_viewset.py index 6c75123ac..fb2f171bc 100644 --- a/tests/integration/motions/test_viewset.py +++ b/tests/integration/motions/test_viewset.py @@ -42,11 +42,10 @@ class TestMotionDBQueries(TestCase): * 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, - * 1 requests to get the personal notes. + * 2 requests to get the submitters and supporters. """ self.client.force_login(get_user_model().objects.get(pk=1)) - with self.assertNumQueries(15): + with self.assertNumQueries(14): self.client.get(reverse('motion-list')) @use_cache() @@ -61,10 +60,9 @@ class TestMotionDBQueries(TestCase): * 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, - * 1 request to get the personal notes. + * 2 requests to get the submitters and supporters. """ - with self.assertNumQueries(14): + with self.assertNumQueries(13): self.client.get(reverse('motion-list')) @@ -400,12 +398,10 @@ class RetrieveMotion(TestCase): * 3 request to get the polls (1 of them is possibly a bug), * 1 request to get the attachments, * 1 request to get the tags, - * 2 requests to get the submitters and supporters, - * 1 request to get personal notes. - + * 2 requests to get the submitters and supporters. TODO: Fix all bugs. """ - with self.assertNumQueries(19): + with self.assertNumQueries(18): self.client.get(reverse('motion-detail', args=[self.motion.pk])) def test_guest_state_with_required_permission_to_see(self): @@ -458,14 +454,6 @@ class RetrieveMotion(TestCase): response_3 = guest_client.get(reverse('user-detail', args=[extra_user.pk])) self.assertEqual(response_3.status_code, status.HTTP_403_FORBIDDEN) - def test_anonymous_without_personal_notes(self): - self.motion.set_personal_note(get_user_model().objects.get(pk=1), note='admin_personal_note_OoGh8choro0oosh0roob') - config['general_system_enable_anonymous'] = True - guest_client = APIClient() - response = guest_client.get(reverse('motion-detail', args=[self.motion.pk])) - self.assertEqual(response.status_code, status.HTTP_200_OK) - self.assertNotContains(response, 'admin_personal_note_OoGh8choro0oosh0roob') - class UpdateMotion(TestCase): """ diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index c74e821a2..676fa3bd1 100644 --- a/tests/integration/users/test_viewset.py +++ b/tests/integration/users/test_viewset.py @@ -3,7 +3,7 @@ from rest_framework import status from rest_framework.test import APIClient from openslides.core.config import config -from openslides.users.models import Group, User +from openslides.users.models import Group, PersonalNote, User from openslides.users.serializers import UserFullSerializer from openslides.utils.test import TestCase, use_cache @@ -540,3 +540,35 @@ class GroupDelete(TestCase): response = admin_client.delete(reverse('group-detail', args=[group_pk])) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + +class PersonalNoteTest(TestCase): + """ + Tests for PersonalNote model. + """ + def test_anonymous_without_personal_notes(self): + admin = User.objects.get(pk=1) + personal_note = PersonalNote.objects.create(user=admin, notes='["admin_personal_note_OoGh8choro0oosh0roob"]') + config['general_system_enable_anonymous'] = True + guest_client = APIClient() + response = guest_client.get(reverse('personalnote-detail', args=[personal_note.pk])) + self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) + + def test_admin_send_JSON(self): + admin_client = APIClient() + admin_client.login(username='admin', password='admin') + response = admin_client.post( + reverse('personalnote-list'), + { + "notes": { + "example-model": { + "1": { + "note": "note for the example.model with id 1 Oohae1JeuSedooyeeviH", + "star": True + } + } + } + }, + format='json' + ) + self.assertEqual(response.status_code, status.HTTP_201_CREATED)