diff --git a/CHANGELOG.md b/CHANGELOG.md index f7aa287a..e357e3ec 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,10 @@ and this project adheres to - 🛂(backend) stop to list public doc to everyone #234 - 🚚(frontend) change visibility in share modal #235 +## Fixed + +- 🐛 Fix forcing ID when creating a document via API endpoint #234 + ## [1.3.0] - 2024-09-05 diff --git a/src/backend/core/api/serializers.py b/src/backend/core/api/serializers.py index 2abcc91e..3ef1ebad 100644 --- a/src/backend/core/api/serializers.py +++ b/src/backend/core/api/serializers.py @@ -163,6 +163,29 @@ class DocumentSerializer(BaseResourceSerializer): "updated_at", ] + def get_fields(self): + """Dynamically make `id` read-only on PUT requests but writable on POST requests.""" + fields = super().get_fields() + + request = self.context.get("request") + if request and request.method == "POST": + fields["id"].read_only = False + + return fields + + def validate_id(self, value): + """Ensure the provided ID does not already exist when creating a new document.""" + request = self.context.get("request") + + # Only check this on POST (creation) + if request and request.method == "POST": + if models.Document.objects.filter(id=value).exists(): + raise serializers.ValidationError( + "A document with this ID already exists. You cannot override it." + ) + + return value + class LinkDocumentSerializer(BaseResourceSerializer): """ diff --git a/src/backend/core/tests/documents/test_api_documents_create.py b/src/backend/core/tests/documents/test_api_documents_create.py index 84b97c94..ba125ff5 100644 --- a/src/backend/core/tests/documents/test_api_documents_create.py +++ b/src/backend/core/tests/documents/test_api_documents_create.py @@ -2,6 +2,8 @@ Tests for Documents API endpoint in impress's core app: create """ +from uuid import uuid4 + import pytest from rest_framework.test import APIClient @@ -46,3 +48,51 @@ def test_api_documents_create_authenticated(): document = Document.objects.get() assert document.title == "my document" assert document.accesses.filter(role="owner", user=user).exists() + + +def test_api_documents_create_force_id_success(): + """It should be possible to force the document ID when creating a document.""" + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + forced_id = uuid4() + + response = client.post( + "/api/v1.0/documents/", + { + "id": str(forced_id), + "title": "my document", + }, + format="json", + ) + + assert response.status_code == 201 + documents = Document.objects.all() + assert len(documents) == 1 + assert documents[0].id == forced_id + + +def test_api_documents_create_force_id_existing(): + """ + It should not be possible to use the ID of an existing document when forcing ID on creation. + """ + user = factories.UserFactory() + client = APIClient() + client.force_login(user) + + document = factories.DocumentFactory() + + response = client.post( + "/api/v1.0/documents/", + { + "id": str(document.id), + "title": "my document", + }, + format="json", + ) + + assert response.status_code == 400 + assert response.json() == { + "id": ["A document with this ID already exists. You cannot override it."] + }