Rework on personal notes. Fixed #3262.

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].
+- Added support for export motions in a ZIP archive [#3189].
-- Performance improvement for zip creation [#3251]
+- Performance improvement for ZIP creation [#3251].
-- Bugfix: changing motion line length did not invalidate cache [#3202]
+- 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)

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

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 = []

openslides/motions/models.py

@@ -50,8 +50,7 @@ class MotionManager(models.Manager):
                     'attachments',
                     'tags',
                     'submitters',
-                    'supporters',
+                    'supporters'))
-                    'personal_notes'))
 
 
 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.

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

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': <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):
         """

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

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())

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),
+        ),
+    ]

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
+from .access_permissions import (
-
+    GroupAccessPermissions,
-
+    PersonalNoteAccessPermissions,
-class PersonalNote(models.Model):
+    UserAccessPermissions,
-    """
+)
-    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,
-        }
 
 
 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 = ()

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

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 .access_permissions import (
-from .models import Group, User
+    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):

openslides/utils/rest_api.py

@@ -17,6 +17,7 @@ from rest_framework.serializers import (  # noqa
     Field,
     FileField,
     IntegerField,
+    JSONField,
     ListField,
     ListSerializer,
     ManyRelatedField,

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,
+        * 2 requests to get the submitters and supporters.
-        * 1 requests to get the personal notes.
         """
         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,
+        * 2 requests to get the submitters and supporters.
-        * 1 request to get the personal notes.
         """
-        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,
+        * 2 requests to get the submitters and supporters.
-        * 1 request to get personal notes.
-
         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):
     """

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)