Merge pull request #4992 from FinnStutzenstein/reworkPersonalNotes

Single create and update request for personal notes

client/src/app/core/ui-services/personal-note.service.ts

@@ -7,6 +7,12 @@ import { HttpService } from '../core-services/http.service';
 import { OperatorService } from '../core-services/operator.service';
 import { PersonalNote, PersonalNoteContent, PersonalNoteObject } from '../../shared/models/users/personal-note';
 
+type PersonalNoteRequestData = {
+    collection: string;
+    id: number;
+    content: object;
+}[];
+
 /**
  * Handles saving personal notes.
  */
@@ -17,7 +23,7 @@ export class PersonalNoteService {
     /**
      * The personal note object for the operator
      */
-    private personalNoteObject: PersonalNoteObject;
+    private personalNoteObject: PersonalNoteObject | null;
 
     /**
      * Watches for changes in the personal note model and the operator.
@@ -34,6 +40,7 @@ export class PersonalNoteService {
      */
     private updatePersonalNoteObject(): void {
         if (this.operator.isAnonymous) {
+            this.personalNoteObject = null;
             return;
         }
 
@@ -49,16 +56,13 @@ export class PersonalNoteService {
      * @param content The new content.
      */
     public async savePersonalNote(model: BaseModel | BaseViewModel, content: PersonalNoteContent): Promise<void> {
-        const pnObject: Partial<PersonalNoteObject> = this.personalNoteObject || {};
+        await this.savePersonalNoteObject([
-        if (!pnObject.notes) {
+            {
-            pnObject.notes = {};
+                collection: model.collectionString,
-        }
+                id: model.id,
-        if (!pnObject.notes[model.collectionString]) {
+                content: content
-            pnObject.notes[model.collectionString] = {};
+            }
-        }
+        ]);
-
-        pnObject.notes[model.collectionString][model.id] = content;
-        await this.savePersonalNoteObject(pnObject);
     }
 
     /**
@@ -75,7 +79,7 @@ export class PersonalNoteService {
         if (!pnObject.notes) {
             pnObject.notes = {};
         }
-        for (const model of models) {
+        const requestData: PersonalNoteRequestData = models.map(model => {
             if (!pnObject.notes[model.collectionString]) {
                 pnObject.notes[model.collectionString] = {};
             }
@@ -84,20 +88,21 @@ export class PersonalNoteService {
             } else {
                 pnObject.notes[model.collectionString][model.id] = { star: star, note: '' };
             }
-        }
+            return {
-        await this.savePersonalNoteObject(pnObject);
+                collection: model.collectionString,
+                id: model.id,
+                content: pnObject.notes[model.collectionString][model.id]
+            };
+        });
+        await this.savePersonalNoteObject(requestData);
     }
 
     /**
      * Sends an updated personal note to the server
      *
-     * @param pnObject a partial (if new) or complete personal note object
+     * @param requestData The data to send to the server
      */
-    private async savePersonalNoteObject(pnObject: Partial<PersonalNoteObject>): Promise<void> {
+    private async savePersonalNoteObject(requestData: PersonalNoteRequestData): Promise<void> {
-        if (!pnObject.id) {
+        await this.http.post(`/rest/users/personal-note/create_or_update/`, requestData);
-            await this.http.post('/rest/users/personal-note/', pnObject);
-        } else {
-            await this.http.put(`/rest/users/personal-note/${pnObject.id}/`, pnObject);
-        }
     }
 }

openslides/users/views.py

@@ -17,7 +17,6 @@ from django.contrib.sites.shortcuts import get_current_site
 from django.core import mail
 from django.core.exceptions import ValidationError as DjangoValidationError
 from django.db import transaction
-from django.db.utils import IntegrityError
 from django.http.request import QueryDict
 from django.utils.encoding import force_bytes, force_text
 from django.utils.http import urlsafe_base64_decode, urlsafe_base64_encode
@@ -608,13 +607,7 @@ class PersonalNoteViewSet(ModelViewSet):
         """
         if self.action in ("list", "retrieve"):
             result = self.get_access_permissions().check_permissions(self.request.user)
-        elif self.action in (
+        elif self.action in ("create_or_update", "destroy"):
-            "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().
@@ -623,29 +616,44 @@ class PersonalNoteViewSet(ModelViewSet):
             result = False
         return result
 
-    def perform_create(self, serializer):
+    @list_route(methods=["post"])
-        """
+    @transaction.atomic
-        Customized method to inject the request.user into serializer's save
+    def create_or_update(self, request, *args, **kwargs):
-        method so that the request.user can be saved into the model field.
-        """
-        try:
-            serializer.save(user=self.request.user)
-        except IntegrityError:
-            raise ValidationError(
-                {
-                    "detail": "The personal note for user {0} does already exist",
-                    "args": [self.request.user.id],
-                }
-            )
-
-    def update(self, request, *args, **kwargs):
         """
         Customized method to ensure that every user can change only his own
         personal notes.
+
+        [{
+            collection: <collection>,
+            id: <id>,
+            content: <Any>,
+        }, ...]
         """
-        if self.get_object().user != self.request.user:
+        # verify data:
-            self.permission_denied(request)
+        if not isinstance(request.data, list):
-        return super().update(request, *args, **kwargs)
+            raise ValidationError({"detail": "Data must be a list"})
+        for data in request.data:
+            if not isinstance(data, dict):
+                raise ValidationError({"detail": "Every entry must be a dict"})
+            if not isinstance(data.get("collection"), str):
+                raise ValidationError({"detail": "The collection must be a string"})
+            if not isinstance(data.get("id"), int):
+                raise ValidationError({"detail": "The id must be an integer"})
+
+        # get note
+        personal_note, _ = PersonalNote.objects.get_or_create(user=request.user)
+
+        # set defaults
+        if not personal_note.notes:
+            personal_note.notes = {}
+
+        for data in request.data:
+            if data["collection"] not in personal_note.notes:
+                personal_note.notes[data["collection"]] = {}
+            personal_note.notes[data["collection"]][data["id"]] = data["content"]
+
+        personal_note.save()
+        return Response()
 
     def destroy(self, request, *args, **kwargs):
         """

tests/integration/users/test_viewset.py

@@ -853,69 +853,75 @@ class PersonalNoteTest(TestCase):
     def test_create(self):
         admin_client = APIClient()
         admin_client.login(username="admin", password="admin")
+        content1 = {
+            "note": "note for the example.model with id 1 Oohae1JeuSedooyeeviH",
+            "star": True,
+        }
+        content2 = {
+            "note": "note for the example.model with id 2 gegjhjynjiohnhioaaiu",
+            "star": False,
+        }
         response = admin_client.post(
-            reverse("personalnote-list"),
+            reverse("personalnote-create-or-update"),
-            {
+            [
-                "notes": {
+                {"collection": "example-model", "id": 1, "content": content1},
-                    "example-model": {
+                {"collection": "example-model", "id": 2, "content": content2},
-                        "1": {
+            ],
-                            "note": "note for the example.model with id 1 Oohae1JeuSedooyeeviH",
-                            "star": True,
-                        }
-                    }
-                }
-            },
             format="json",
         )
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
+        self.assertEqual(response.status_code, status.HTTP_200_OK)
+        self.assertTrue(PersonalNote.objects.exists())
+        personal_note = PersonalNote.objects.get()
+        self.assertTrue("example-model" in personal_note.notes)
+        self.assertTrue("1" in personal_note.notes["example-model"])
+        self.assertTrue("2" in personal_note.notes["example-model"])
+        self.assertEqual(personal_note.notes["example-model"]["1"], content1)
+        self.assertEqual(personal_note.notes["example-model"]["2"], content2)
 
     def test_anonymous_create(self):
         guest_client = APIClient()
         response = guest_client.post(
-            reverse("personalnote-list"), {"notes": {}}, format="json"
+            reverse("personalnote-create-or-update"), [], format="json"
         )
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         self.assertFalse(PersonalNote.objects.exists())
 
-    def test_create_twice(self):
-        admin_client = APIClient()
-        admin_client.login(username="admin", password="admin")
-        response = admin_client.post(
-            reverse("personalnote-list"), {"notes": {}}, format="json"
-        )
-        self.assertEqual(response.status_code, status.HTTP_201_CREATED)
-        response = admin_client.post(
-            reverse("personalnote-list"), {"notes": {}}, format="json"
-        )
-        self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST)
-
     def test_update(self):
         admin_client = APIClient()
         admin_client.login(username="admin", password="admin")
         personal_note = PersonalNote.objects.create(
-            user=self.admin, notes="test_note_ld3mo1xjcnKNC(836qWe"
+            user=self.admin,
+            notes={"test_collection": {2: "test_note_ld3mo1xjcnKNC(836qWe"}},
         )
-        response = admin_client.put(
+        response = admin_client.post(
-            reverse("personalnote-detail", args=[personal_note.pk]),
+            reverse("personalnote-create-or-update"),
-            {"notes": "test_note_do2ncoi7ci2fm93LjwlO"},
+            [
+                {
+                    "collection": "test_collection",
+                    "id": 2,
+                    "content": "test_note_do2ncoi7ci2fm93LjwlO",
+                }
+            ],
             format="json",
         )
         self.assertEqual(response.status_code, status.HTTP_200_OK)
+        personal_note = PersonalNote.objects.get()
+        self.assertTrue("test_collection" in personal_note.notes)
+        self.assertTrue("2" in personal_note.notes["test_collection"])
         self.assertEqual(
-            PersonalNote.objects.get().notes, "test_note_do2ncoi7ci2fm93LjwlO"
+            personal_note.notes["test_collection"]["2"],
+            "test_note_do2ncoi7ci2fm93LjwlO",
         )
 
-    def test_update_other_user(self):
+    def test_delete_other_user(self):
         user = User.objects.create(username="user")
         admin_client = APIClient()
         admin_client.login(username="admin", password="admin")
         personal_note = PersonalNote.objects.create(
             user=user, notes="test_note_fof3joqmcufh32fn(/2f"
         )
-        response = admin_client.put(
+        response = admin_client.delete(
-            reverse("personalnote-detail", args=[personal_note.pk]),
+            reverse("personalnote-detail", args=[personal_note.pk])
-            {"notes": "test_note_1qowuddm3d8mF8h29fwI"},
-            format="json",
         )
         self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN)
         self.assertEqual(