From 76ff4602a2c1bad5e4ca005950c4653b40ea7a9f Mon Sep 17 00:00:00 2001 From: FinnStutzenstein Date: Tue, 10 Sep 2019 11:21:39 +0200 Subject: [PATCH] Single create and update request for personal notes --- .../core/ui-services/personal-note.service.ts | 47 +++++++----- openslides/users/views.py | 62 ++++++++------- tests/integration/users/test_viewset.py | 76 ++++++++++--------- 3 files changed, 102 insertions(+), 83 deletions(-) diff --git a/client/src/app/core/ui-services/personal-note.service.ts b/client/src/app/core/ui-services/personal-note.service.ts index c0809b806..cb827a589 100644 --- a/client/src/app/core/ui-services/personal-note.service.ts +++ b/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 { - const pnObject: Partial = this.personalNoteObject || {}; - if (!pnObject.notes) { - pnObject.notes = {}; - } - if (!pnObject.notes[model.collectionString]) { - pnObject.notes[model.collectionString] = {}; - } - - pnObject.notes[model.collectionString][model.id] = content; - await this.savePersonalNoteObject(pnObject); + await this.savePersonalNoteObject([ + { + collection: model.collectionString, + id: model.id, + content: content + } + ]); } /** @@ -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: '' }; } - } - await this.savePersonalNoteObject(pnObject); + return { + 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): Promise { - if (!pnObject.id) { - await this.http.post('/rest/users/personal-note/', pnObject); - } else { - await this.http.put(`/rest/users/personal-note/${pnObject.id}/`, pnObject); - } + private async savePersonalNoteObject(requestData: PersonalNoteRequestData): Promise { + await this.http.post(`/rest/users/personal-note/create_or_update/`, requestData); } } diff --git a/openslides/users/views.py b/openslides/users/views.py index 78e75340b..ec78ef8c4 100644 --- a/openslides/users/views.py +++ b/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 ( - "metadata", - "create", - "partial_update", - "update", - "destroy", - ): + elif self.action in ("create_or_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): - """ - Customized method to inject the request.user into serializer's save - 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): + @list_route(methods=["post"]) + @transaction.atomic + def create_or_update(self, request, *args, **kwargs): """ Customized method to ensure that every user can change only his own personal notes. + + [{ + collection: , + id: , + content: , + }, ...] """ - if self.get_object().user != self.request.user: - self.permission_denied(request) - return super().update(request, *args, **kwargs) + # verify data: + if not isinstance(request.data, list): + 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): """ diff --git a/tests/integration/users/test_viewset.py b/tests/integration/users/test_viewset.py index ed4a2f5fc..062529486 100644 --- a/tests/integration/users/test_viewset.py +++ b/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"), - { - "notes": { - "example-model": { - "1": { - "note": "note for the example.model with id 1 Oohae1JeuSedooyeeviH", - "star": True, - } - } - } - }, + reverse("personalnote-create-or-update"), + [ + {"collection": "example-model", "id": 1, "content": content1}, + {"collection": "example-model", "id": 2, "content": content2}, + ], 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( - reverse("personalnote-detail", args=[personal_note.pk]), - {"notes": "test_note_do2ncoi7ci2fm93LjwlO"}, + response = admin_client.post( + reverse("personalnote-create-or-update"), + [ + { + "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( - reverse("personalnote-detail", args=[personal_note.pk]), - {"notes": "test_note_1qowuddm3d8mF8h29fwI"}, - format="json", + response = admin_client.delete( + reverse("personalnote-detail", args=[personal_note.pk]) ) self.assertEqual(response.status_code, status.HTTP_403_FORBIDDEN) self.assertEqual(