diff --git a/src/documents/migrations/1040_customfield_customfieldboolean_customfielddate_and_more.py b/src/documents/migrations/1040_customfield_customfieldboolean_customfielddate_and_more.py deleted file mode 100644 index 414b8b489..000000000 --- a/src/documents/migrations/1040_customfield_customfieldboolean_customfielddate_and_more.py +++ /dev/null @@ -1,241 +0,0 @@ -# Generated by Django 4.2.5 on 2023-10-31 17:28 - -import django.db.models.deletion -import django.utils.timezone -from django.conf import settings -from django.db import migrations -from django.db import models - - -class Migration(migrations.Migration): - dependencies = [ - migrations.swappable_dependency(settings.AUTH_USER_MODEL), - ("documents", "1039_consumptiontemplate"), - ] - - operations = [ - migrations.CreateModel( - name="CustomField", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ( - "created", - models.DateTimeField( - db_index=True, - default=django.utils.timezone.now, - verbose_name="created", - editable=False, - ), - ), - ("name", models.CharField(max_length=128)), - ( - "data_type", - models.CharField( - choices=[ - ("string", "String"), - ("url", "URL"), - ("date", "Date"), - ("boolean", "Boolean"), - ("integer", "Integer"), - ], - max_length=50, - verbose_name="data type", - editable=False, - ), - ), - ], - options={ - "verbose_name": "custom field", - "verbose_name_plural": "custom fields", - "ordering": ("created",), - }, - ), - migrations.CreateModel( - name="CustomFieldInstance", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ( - "created", - models.DateTimeField( - editable=False, - db_index=True, - default=django.utils.timezone.now, - verbose_name="created", - ), - ), - ( - "document", - models.ForeignKey( - editable=False, - on_delete=django.db.models.deletion.CASCADE, - related_name="custom_fields", - to="documents.document", - ), - ), - ( - "field", - models.ForeignKey( - editable=False, - on_delete=django.db.models.deletion.CASCADE, - related_name="fields", - to="documents.customfield", - ), - ), - ], - options={ - "verbose_name": "custom field instance", - "verbose_name_plural": "custom field instances", - "ordering": ("created",), - }, - ), - migrations.CreateModel( - name="CustomFieldBoolean", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("value", models.BooleanField()), - ( - "parent", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - parent_link=True, - related_name="boolean", - to="documents.customfieldinstance", - ), - ), - ], - ), - migrations.CreateModel( - name="CustomFieldDate", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("value", models.DateField()), - ( - "parent", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - parent_link=True, - related_name="date", - to="documents.customfieldinstance", - ), - ), - ], - ), - migrations.CreateModel( - name="CustomFieldInteger", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("value", models.IntegerField()), - ( - "parent", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - parent_link=True, - related_name="integer", - to="documents.customfieldinstance", - ), - ), - ], - ), - migrations.CreateModel( - name="CustomFieldShortText", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("value", models.CharField(max_length=128)), - ( - "parent", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - parent_link=True, - related_name="short_text", - to="documents.customfieldinstance", - ), - ), - ], - ), - migrations.CreateModel( - name="CustomFieldUrl", - fields=[ - ( - "id", - models.AutoField( - auto_created=True, - primary_key=True, - serialize=False, - verbose_name="ID", - ), - ), - ("value", models.URLField()), - ( - "parent", - models.OneToOneField( - on_delete=django.db.models.deletion.CASCADE, - parent_link=True, - related_name="url", - to="documents.customfieldinstance", - ), - ), - ], - ), - migrations.AddConstraint( - model_name="customfield", - constraint=models.UniqueConstraint( - fields=("name",), - name="documents_customfield_unique_name", - ), - ), - migrations.AddConstraint( - model_name="customfieldinstance", - constraint=models.UniqueConstraint( - fields=("document", "field"), - name="documents_customfieldinstance_unique_document_field", - ), - ), - ] diff --git a/src/documents/migrations/1040_customfield_customfieldinstance_and_more.py b/src/documents/migrations/1040_customfield_customfieldinstance_and_more.py new file mode 100644 index 000000000..2aab0ea8e --- /dev/null +++ b/src/documents/migrations/1040_customfield_customfieldinstance_and_more.py @@ -0,0 +1,124 @@ +# Generated by Django 4.2.6 on 2023-11-02 17:38 + +import django.db.models.deletion +import django.utils.timezone +from django.db import migrations +from django.db import models + + +class Migration(migrations.Migration): + dependencies = [ + ("documents", "1039_consumptiontemplate"), + ] + + operations = [ + migrations.CreateModel( + name="CustomField", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "created", + models.DateTimeField( + db_index=True, + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ("name", models.CharField(max_length=128)), + ( + "data_type", + models.CharField( + choices=[ + ("string", "String"), + ("url", "URL"), + ("date", "Date"), + ("boolean", "Boolean"), + ("integer", "Integer"), + ], + editable=False, + max_length=50, + verbose_name="data type", + ), + ), + ], + options={ + "verbose_name": "custom field", + "verbose_name_plural": "custom fields", + "ordering": ("created",), + }, + ), + migrations.CreateModel( + name="CustomFieldInstance", + fields=[ + ( + "id", + models.AutoField( + auto_created=True, + primary_key=True, + serialize=False, + verbose_name="ID", + ), + ), + ( + "created", + models.DateTimeField( + db_index=True, + default=django.utils.timezone.now, + editable=False, + verbose_name="created", + ), + ), + ("value_text", models.CharField(max_length=128, null=True)), + ("value_bool", models.BooleanField(null=True)), + ("value_url", models.URLField(null=True)), + ("value_date", models.DateField(null=True)), + ("value_int", models.IntegerField(null=True)), + ( + "document", + models.ForeignKey( + editable=False, + on_delete=django.db.models.deletion.CASCADE, + related_name="custom_fields", + to="documents.document", + ), + ), + ( + "field", + models.ForeignKey( + editable=False, + on_delete=django.db.models.deletion.CASCADE, + related_name="fields", + to="documents.customfield", + ), + ), + ], + options={ + "verbose_name": "custom field instance", + "verbose_name_plural": "custom field instances", + "ordering": ("created",), + }, + ), + migrations.AddConstraint( + model_name="customfield", + constraint=models.UniqueConstraint( + fields=("name",), + name="documents_customfield_unique_name", + ), + ), + migrations.AddConstraint( + model_name="customfieldinstance", + constraint=models.UniqueConstraint( + fields=("document", "field"), + name="documents_customfieldinstance_unique_document_field", + ), + ), + ] diff --git a/src/documents/models.py b/src/documents/models.py index 28ee8d73a..d1ced4359 100644 --- a/src/documents/models.py +++ b/src/documents/models.py @@ -4,7 +4,6 @@ import os import re from collections import OrderedDict from pathlib import Path -from typing import Any from typing import Final from typing import Optional @@ -960,6 +959,17 @@ class CustomFieldInstance(models.Model): editable=False, ) + # Actual data storage + value_text = models.CharField(max_length=128, null=True) + + value_bool = models.BooleanField(null=True) + + value_url = models.URLField(null=True) + + value_date = models.DateField(null=True) + + value_int = models.IntegerField(null=True) + class Meta: ordering = ("created",) verbose_name = _("custom field instance") @@ -978,134 +988,16 @@ class CustomFieldInstance(models.Model): def value(self): """ Based on the data type, access the actual value the instance stores + A little shorthand/quick way to get what is actually here """ if self.field.data_type == CustomField.FieldDataType.STRING: - return self.short_text.value + return self.value_text elif self.field.data_type == CustomField.FieldDataType.URL: - return self.url.value + return self.value_url elif self.field.data_type == CustomField.FieldDataType.DATE: - return self.date.value + return self.value_date elif self.field.data_type == CustomField.FieldDataType.BOOL: - return self.boolean.value + return self.value_bool elif self.field.data_type == CustomField.FieldDataType.INT: - return self.integer.value + return self.value_int raise NotImplementedError(self.field.data_type) - - @property - def field_type(self): - """ - Based on the data type, quick access to class for storing that value - """ - if self.field.data_type == CustomField.FieldDataType.STRING: - return CustomFieldShortText - elif self.field.data_type == CustomField.FieldDataType.URL: - return CustomFieldUrl - elif self.field.data_type == CustomField.FieldDataType.DATE: - return CustomFieldDate - elif self.field.data_type == CustomField.FieldDataType.BOOL: - return CustomFieldBoolean - elif self.field.data_type == CustomField.FieldDataType.INT: - return CustomFieldInteger - raise NotImplementedError(self.field.data_type) - - @staticmethod - def from_json( - document: Document, - field: OrderedDict, - value: Any, - ) -> "CustomFieldInstance": - instance, _ = CustomFieldInstance.objects.get_or_create( - document=document, - field=CustomField.objects.get(id=field["id"]), - ) - instance.field_type.objects.update_or_create( - parent=instance, - defaults={"value": value}, - ) - - return instance - - -class CustomFieldShortText(models.Model): - """ - Data storage for a short text custom field - """ - - value = models.CharField(max_length=128) - parent = models.OneToOneField( - CustomFieldInstance, - on_delete=models.CASCADE, - related_name="short_text", - parent_link=True, - ) - - def __str__(self) -> str: - return f"{self.value}" - - -class CustomFieldBoolean(models.Model): - """ - Data storage for a boolean custom field - """ - - value = models.BooleanField() - parent = models.OneToOneField( - CustomFieldInstance, - on_delete=models.CASCADE, - related_name="boolean", - parent_link=True, - ) - - def __str__(self) -> str: - return f"{self.value}" - - -class CustomFieldUrl(models.Model): - """ - Data storage for a URL custom field - """ - - value = models.URLField() - parent = models.OneToOneField( - CustomFieldInstance, - on_delete=models.CASCADE, - related_name="url", - parent_link=True, - ) - - def __str__(self) -> str: - return f"{self.value}" - - -class CustomFieldDate(models.Model): - """ - Data storage for a date custom field - """ - - value = models.DateField() - parent = models.OneToOneField( - CustomFieldInstance, - on_delete=models.CASCADE, - related_name="date", - parent_link=True, - ) - - def __str__(self) -> str: - return f"{self.value}" - - -class CustomFieldInteger(models.Model): - """ - Data storage for a date custom field - """ - - value = models.IntegerField() - parent = models.OneToOneField( - CustomFieldInstance, - on_delete=models.CASCADE, - related_name="integer", - parent_link=True, - ) - - def __str__(self) -> str: - return f"{self.value}" diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index c2ac364bb..581e3f72e 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -11,6 +11,7 @@ from django.contrib.auth.models import User from django.utils.crypto import get_random_string from django.utils.text import slugify from django.utils.translation import gettext as _ +from drf_writable_nested.serializers import NestedUpdateMixin from guardian.core import ObjectPermissionChecker from guardian.shortcuts import get_users_with_perms from rest_framework import fields @@ -396,9 +397,6 @@ class StoragePathField(serializers.PrimaryKeyRelatedField): return StoragePath.objects.all() -from drf_writable_nested.serializers import NestedUpdateMixin - - class CustomFieldSerializer(serializers.ModelSerializer): data_type = serializers.ChoiceField( choices=CustomField.FieldDataType, @@ -426,41 +424,82 @@ class CustomFieldOnUpdateSerializer(serializers.ModelSerializer): class CustomFieldInstanceSerializer(serializers.ModelSerializer): field = serializers.PrimaryKeyRelatedField(queryset=CustomField.objects.all()) - value = serializers.JSONField() + value = serializers.SerializerMethodField(read_only=True) + value_text = serializers.CharField(required=False, write_only=True) + value_bool = serializers.BooleanField(required=False, write_only=True) + value_url = serializers.URLField(required=False, write_only=True) + value_date = serializers.DateField(required=False, write_only=True) + value_int = serializers.IntegerField(required=False, write_only=True) + + def validate(self, data): + """ + Check that start is before finish. + """ + # Let the normal validation run first + data = super().validate(data) + # This field must exist, as it is validated + parent_field = data["field"] + type_to_key_map = { + CustomField.FieldDataType.STRING: "value_text", + CustomField.FieldDataType.URL: "value_url", + CustomField.FieldDataType.DATE: "value_date", + CustomField.FieldDataType.BOOL: "value_bool", + CustomField.FieldDataType.INT: "value_int", + } + # For the given data type, a certain key must exist + expected_key = type_to_key_map[parent_field.data_type] + if expected_key not in data: + raise serializers.ValidationError( + ( + f"Field of type {parent_field.data_type} must" + f' contain a "{expected_key}" key' + ), + ) + return data def create(self, validated_data): - print("hello from create") - from pprint import pprint - - pprint(dict(validated_data)) + type_to_key_map = { + CustomField.FieldDataType.STRING: "value_text", + CustomField.FieldDataType.URL: "value_url", + CustomField.FieldDataType.DATE: "value_date", + CustomField.FieldDataType.BOOL: "value_bool", + CustomField.FieldDataType.INT: "value_int", + } + # An instance is attached to a document document: Document = validated_data["document"] + # And to a CustomField custom_field: CustomField = validated_data["field"] - instance, _ = CustomFieldInstance.objects.get_or_create( - document=document, field=custom_field - ) - instance_data_class = instance.field_type - _, _ = instance_data_class.objects.update_or_create( - parent=instance, defaults={"value": validated_data["value"]} + # This key must exist, as it is validated + expected_key = type_to_key_map[custom_field.data_type] + + # Actually update or create the instance, providing the value + instance, _ = CustomFieldInstance.objects.update_or_create( + document=document, + field=custom_field, + defaults={expected_key: validated_data[expected_key]}, ) return instance - def update(self, instance: CustomFieldInstance, validated_data): - print("hello from update") - from pprint import pprint - - pprint(validated_data) - pprint(instance.id) - def get_value(self, obj: CustomFieldInstance): return obj.value class Meta: model = CustomFieldInstance - fields = ["value", "field"] + fields = [ + "value", + "value_text", + "value_bool", + "value_url", + "value_date", + "value_int", + "field", + ] class DocumentSerializer( - OwnedObjectSerializer, NestedUpdateMixin, DynamicFieldsModelSerializer + OwnedObjectSerializer, + NestedUpdateMixin, + DynamicFieldsModelSerializer, ): correspondent = CorrespondentField(allow_null=True) tags = TagsField(many=True) diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index fd7b7724c..168655380 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -1,11 +1,14 @@ +from datetime import date +from pprint import pprint + from django.contrib.auth.models import User from rest_framework import status from rest_framework.test import APITestCase -from documents.models import CustomField, CustomFieldInstance, CustomFieldShortText +from documents.models import CustomField +from documents.models import CustomFieldInstance from documents.models import Document from documents.tests.utils import DirectoriesMixin -from pprint import pprint class TestCustomField(DirectoriesMixin, APITestCase): @@ -76,38 +79,37 @@ class TestCustomField(DirectoriesMixin, APITestCase): data_type=CustomField.FieldDataType.URL, ) + date_value = date.today() + resp = self.client.patch( f"/api/documents/{doc.id}/", data={ "custom_fields": [ { "field": custom_field_string.id, - "value": "test value", + "value_text": "test value", }, { "field": custom_field_date.id, - "value": "2023-10-31", + "value_date": date_value.isoformat(), }, { "field": custom_field_int.id, - "value": "3", + "value_int": 3, }, { "field": custom_field_boolean.id, - "value": "True", + "value_bool": True, }, { "field": custom_field_url.id, - "value": "https://example.com", + "value_url": "https://example.com", }, ], }, format="json", ) - from pprint import pprint - print("Response data") - pprint(resp.json()) self.assertEqual(resp.status_code, status.HTTP_200_OK) resp_data = resp.json()["custom_fields"] @@ -116,7 +118,7 @@ class TestCustomField(DirectoriesMixin, APITestCase): resp_data, [ {"field": custom_field_string.id, "value": "test value"}, - {"field": custom_field_date.id, "value": "2023-10-31"}, + {"field": custom_field_date.id, "value": date_value.isoformat()}, {"field": custom_field_int.id, "value": 3}, {"field": custom_field_boolean.id, "value": True}, {"field": custom_field_url.id, "value": "https://example.com"}, @@ -141,7 +143,6 @@ class TestCustomField(DirectoriesMixin, APITestCase): ) self.assertEqual(CustomFieldInstance.objects.count(), 0) - self.assertEqual(CustomFieldShortText.objects.count(), 0) resp = self.client.patch( f"/api/documents/{doc.id}/", @@ -149,7 +150,7 @@ class TestCustomField(DirectoriesMixin, APITestCase): "custom_fields": [ { "field": custom_field_string.id, - "value": "test value", + "value_text": "test value", }, ], }, @@ -162,7 +163,6 @@ class TestCustomField(DirectoriesMixin, APITestCase): self.assertEqual(doc.custom_fields.first().value, "test value") self.assertEqual(CustomFieldInstance.objects.count(), 1) - self.assertEqual(CustomFieldShortText.objects.count(), 1) resp = self.client.patch( f"/api/documents/{doc.id}/", @@ -170,7 +170,7 @@ class TestCustomField(DirectoriesMixin, APITestCase): "custom_fields": [ { "field": custom_field_string.id, - "value": "a new test value", + "value_text": "a new test value", }, ], }, @@ -182,4 +182,91 @@ class TestCustomField(DirectoriesMixin, APITestCase): self.assertEqual(doc.custom_fields.first().value, "a new test value") self.assertEqual(CustomFieldInstance.objects.count(), 1) - self.assertEqual(CustomFieldShortText.objects.count(), 1) + + def test_delete_custom_field_instance(self): + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + custom_field_string = CustomField.objects.create( + name="Test Custom Field String", + data_type=CustomField.FieldDataType.STRING, + ) + custom_field_date = CustomField.objects.create( + name="Test Custom Field Date", + data_type=CustomField.FieldDataType.DATE, + ) + + date_value = date.today() + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_string.id, + "value_text": "a new test value", + }, + { + "field": custom_field_date.id, + "value_date": date_value.isoformat(), + }, + ], + }, + format="json", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(CustomFieldInstance.objects.count(), 2) + self.assertEqual(len(doc.custom_fields.all()), 2) + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_date.id, + "value_date": date_value.isoformat(), + }, + ], + }, + format="json", + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(CustomFieldInstance.objects.count(), 1) + self.assertEqual(Document.objects.count(), 1) + self.assertEqual(len(doc.custom_fields.all()), 1) + self.assertEqual(doc.custom_fields.first().value, date_value) + + def test_custom_field_validation(self): + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + custom_field_string = CustomField.objects.create( + name="Test Custom Field String", + data_type=CustomField.FieldDataType.STRING, + ) + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + data={ + "custom_fields": [ + { + "field": custom_field_string.id, + # Whoops, spelling + "value_test": "a new test value", + }, + ], + }, + format="json", + ) + from pprint import pprint + + pprint(resp.json()) + self.assertEqual(resp.status_code, status.HTTP_400_BAD_REQUEST) + self.assertEqual(CustomFieldInstance.objects.count(), 0) + self.assertEqual(len(doc.custom_fields.all()), 0) diff --git a/src/documents/tests/test_management_exporter.py b/src/documents/tests/test_management_exporter.py index 9a9713fc6..10a272991 100644 --- a/src/documents/tests/test_management_exporter.py +++ b/src/documents/tests/test_management_exporter.py @@ -153,7 +153,7 @@ class TestExportImport(DirectoriesMixin, FileSystemAssertsMixin, TestCase): manifest = self._do_export(use_filename_format=use_filename_format) - self.assertEqual(len(manifest), 194) + self.assertEqual(len(manifest), 169) # dont include consumer or AnonymousUser users self.assertEqual( @@ -247,7 +247,7 @@ class TestExportImport(DirectoriesMixin, FileSystemAssertsMixin, TestCase): self.assertEqual(Document.objects.get(id=self.d4.id).title, "wow_dec") self.assertEqual(GroupObjectPermission.objects.count(), 1) self.assertEqual(UserObjectPermission.objects.count(), 1) - self.assertEqual(Permission.objects.count(), 144) + self.assertEqual(Permission.objects.count(), 124) messages = check_sanity() # everything is alright after the test self.assertEqual(len(messages), 0) @@ -676,15 +676,15 @@ class TestExportImport(DirectoriesMixin, FileSystemAssertsMixin, TestCase): os.path.join(self.dirs.media_dir, "documents"), ) - self.assertEqual(ContentType.objects.count(), 36) - self.assertEqual(Permission.objects.count(), 144) + self.assertEqual(ContentType.objects.count(), 31) + self.assertEqual(Permission.objects.count(), 124) manifest = self._do_export() with paperless_environment(): self.assertEqual( len(list(filter(lambda e: e["model"] == "auth.permission", manifest))), - 144, + 124, ) # add 1 more to db to show objects are not re-created by import Permission.objects.create( @@ -692,7 +692,7 @@ class TestExportImport(DirectoriesMixin, FileSystemAssertsMixin, TestCase): codename="test_perm", content_type_id=1, ) - self.assertEqual(Permission.objects.count(), 145) + self.assertEqual(Permission.objects.count(), 125) # will cause an import error self.user.delete() @@ -701,5 +701,5 @@ class TestExportImport(DirectoriesMixin, FileSystemAssertsMixin, TestCase): with self.assertRaises(IntegrityError): call_command("document_importer", "--no-progress-bar", self.target) - self.assertEqual(ContentType.objects.count(), 36) - self.assertEqual(Permission.objects.count(), 145) + self.assertEqual(ContentType.objects.count(), 31) + self.assertEqual(Permission.objects.count(), 125)