From 707da938443f29a020e005bf36acbde2ccb87171 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 18:13:51 -0800 Subject: [PATCH 01/13] Fix: use api version > 7 for new custom field select format --- docs/api.md | 5 ++ src-ui/src/environments/environment.prod.ts | 2 +- src-ui/src/environments/environment.ts | 2 +- src/documents/serialisers.py | 77 ++++++++++++++----- src/documents/tests/test_api_custom_fields.py | 64 ++++++++++++++- src/documents/views.py | 4 + src/paperless/settings.py | 4 +- 7 files changed, 131 insertions(+), 27 deletions(-) diff --git a/docs/api.md b/docs/api.md index 49e63c04b..59f9d7efc 100644 --- a/docs/api.md +++ b/docs/api.md @@ -573,3 +573,8 @@ Initial API version. #### Version 6 - Moved acknowledge tasks endpoint to be under `/api/tasks/acknowledge/`. + +#### Version 7 + +- The format of select type custom fields has changed to return the options + as an array of objects with `id` and `label` fields. diff --git a/src-ui/src/environments/environment.prod.ts b/src-ui/src/environments/environment.prod.ts index 702b584cb..d2108ee86 100644 --- a/src-ui/src/environments/environment.prod.ts +++ b/src-ui/src/environments/environment.prod.ts @@ -3,7 +3,7 @@ const base_url = new URL(document.baseURI) export const environment = { production: true, apiBaseUrl: document.baseURI + 'api/', - apiVersion: '6', + apiVersion: '7', appTitle: 'Paperless-ngx', version: '2.14.5', webSocketHost: window.location.host, diff --git a/src-ui/src/environments/environment.ts b/src-ui/src/environments/environment.ts index 6256f3ae3..2cad64ce0 100644 --- a/src-ui/src/environments/environment.ts +++ b/src-ui/src/environments/environment.ts @@ -5,7 +5,7 @@ export const environment = { production: false, apiBaseUrl: 'http://localhost:8000/api/', - apiVersion: '6', + apiVersion: '7', appTitle: 'Paperless-ngx', version: 'DEVELOPMENT', webSocketHost: 'localhost:8000', diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index eb1eba8f1..a9746ea2b 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -495,7 +495,27 @@ class StoragePathField(serializers.PrimaryKeyRelatedField): return StoragePath.objects.all() +class ReadWriteSerializerMethodField(serializers.SerializerMethodField): + """ + Based on https://stackoverflow.com/a/62579804 + """ + + def __init__(self, method_name=None, *args, **kwargs): + self.method_name = method_name + kwargs["source"] = "*" + super(serializers.SerializerMethodField, self).__init__(*args, **kwargs) + + def to_internal_value(self, data): + return {self.field_name: data} + + class CustomFieldSerializer(serializers.ModelSerializer): + def __init__(self, *args, **kwargs): + self.api_version = int( + kwargs.pop("api_version", settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1]), + ) + super().__init__(*args, **kwargs) + data_type = serializers.ChoiceField( choices=CustomField.FieldDataType, read_only=False, @@ -503,6 +523,8 @@ class CustomFieldSerializer(serializers.ModelSerializer): document_count = serializers.IntegerField(read_only=True) + extra_data = ReadWriteSerializerMethodField(required=False) + class Meta: model = CustomField fields = [ @@ -544,18 +566,39 @@ class CustomFieldSerializer(serializers.ModelSerializer): or "select_options" not in attrs["extra_data"] or not isinstance(attrs["extra_data"]["select_options"], list) or len(attrs["extra_data"]["select_options"]) == 0 - or not all( - len(option.get("label", "")) > 0 - for option in attrs["extra_data"]["select_options"] + or ( + # version 6 and below require a list of strings + self.api_version < 7 + and not all( + len(option) > 0 + for option in attrs["extra_data"]["select_options"] + ) + ) + or ( + # version 7 and above require a list of objects with labels + self.api_version >= 7 + and not all( + len(option.get("label", "")) > 0 + for option in attrs["extra_data"]["select_options"] + ) ) ): raise serializers.ValidationError( {"error": "extra_data.select_options must be a valid list"}, ) # labels are valid, generate ids if not present - for option in attrs["extra_data"]["select_options"]: - if option.get("id") is None: - option["id"] = get_random_string(length=16) + if self.api_version < 7: + attrs["extra_data"]["select_options"] = [ + { + "label": option, + "id": get_random_string(length=16), + } + for option in attrs["extra_data"]["select_options"] + ] + else: + for option in attrs["extra_data"]["select_options"]: + if option.get("id") is None: + option["id"] = get_random_string(length=16) elif ( "data_type" in attrs and attrs["data_type"] == CustomField.FieldDataType.MONETARY @@ -575,19 +618,15 @@ class CustomFieldSerializer(serializers.ModelSerializer): ) return super().validate(attrs) - -class ReadWriteSerializerMethodField(serializers.SerializerMethodField): - """ - Based on https://stackoverflow.com/a/62579804 - """ - - def __init__(self, method_name=None, *args, **kwargs): - self.method_name = method_name - kwargs["source"] = "*" - super(serializers.SerializerMethodField, self).__init__(*args, **kwargs) - - def to_internal_value(self, data): - return {self.field_name: data} + def get_extra_data(self, obj): + extra_data = obj.extra_data + if self.api_version < 7 and obj.data_type == CustomField.FieldDataType.SELECT: + # Convert the select options with ids to a list of strings + extra_data["select_options"] = [ + option["label"] for option in extra_data["select_options"] + ] + field = serializers.JSONField() + return field.to_representation(extra_data) class CustomFieldInstanceSerializer(serializers.ModelSerializer): diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 8c809429f..cd15de006 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -43,10 +43,13 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): ]: resp = self.client.post( self.ENDPOINT, - data={ - "data_type": field_type, - "name": name, - }, + data=json.dumps( + { + "data_type": field_type, + "name": name, + }, + ), + content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) @@ -272,6 +275,59 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): doc.refresh_from_db() self.assertEqual(doc.custom_fields.first().value, None) + def test_custom_field_select_old_version(self): + """ + GIVEN: + - Select custom field exists with old version of select options + WHEN: + - API post request is made for custom fields with api version header < 7 + - API get request is made for custom fields with api version header < 7 + THEN: + - The select options are returned in the old format + """ + resp = self.client.post( + self.ENDPOINT, + headers={"Accept": "application/json; version=6"}, + data=json.dumps( + { + "data_type": "select", + "name": "Select Field", + "extra_data": { + "select_options": [ + "Option 1", + "Option 2", + ], + }, + }, + ), + content_type="application/json", + ) + self.assertEqual(resp.status_code, status.HTTP_201_CREATED) + + field = CustomField.objects.get(name="Select Field") + self.assertEqual( + field.extra_data["select_options"], + [ + {"label": "Option 1", "id": ANY}, + {"label": "Option 2", "id": ANY}, + ], + ) + + resp = self.client.get( + f"{self.ENDPOINT}{field.id}/", + headers={"Accept": "application/json; version=6"}, + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + data = resp.json() + self.assertEqual( + data["extra_data"]["select_options"], + [ + "Option 1", + "Option 2", + ], + ) + def test_create_custom_field_monetary_validation(self): """ GIVEN: diff --git a/src/documents/views.py b/src/documents/views.py index f98932a6f..a4e0e0f63 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2065,6 +2065,10 @@ class CustomFieldViewSet(ModelViewSet): ) ) + def get_serializer(self, *args, **kwargs): + kwargs.setdefault("api_version", self.request.version) + return super().get_serializer(*args, **kwargs) + class SystemStatusView(PassUserMixin): permission_classes = (IsAuthenticated,) diff --git a/src/paperless/settings.py b/src/paperless/settings.py index ef842dde6..dcfdc020d 100644 --- a/src/paperless/settings.py +++ b/src/paperless/settings.py @@ -341,10 +341,10 @@ REST_FRAMEWORK = { "rest_framework.authentication.SessionAuthentication", ], "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning", - "DEFAULT_VERSION": "1", + "DEFAULT_VERSION": "7", # Make sure these are ordered and that the most recent version appears # last - "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6"], + "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6", "7"], } if DEBUG: From ce85110860580e5be85b90de875ff4f093f64139 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 18:51:55 -0800 Subject: [PATCH 02/13] Use context --- src/documents/serialisers.py | 5 ++++- src/documents/views.py | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index a9746ea2b..87c6657da 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -511,8 +511,11 @@ class ReadWriteSerializerMethodField(serializers.SerializerMethodField): class CustomFieldSerializer(serializers.ModelSerializer): def __init__(self, *args, **kwargs): + context = kwargs.get("context") self.api_version = int( - kwargs.pop("api_version", settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1]), + context.get("request").version + if context.get("request") + else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], ) super().__init__(*args, **kwargs) diff --git a/src/documents/views.py b/src/documents/views.py index a4e0e0f63..2868c1e4e 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2066,7 +2066,7 @@ class CustomFieldViewSet(ModelViewSet): ) def get_serializer(self, *args, **kwargs): - kwargs.setdefault("api_version", self.request.version) + kwargs.setdefault("context", self.get_serializer_context()) return super().get_serializer(*args, **kwargs) From c809e5adfeae485d0cfdd992a74619c79f2828f2 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:09:59 -0800 Subject: [PATCH 03/13] Handle document serialzier --- src/documents/serialisers.py | 37 +++++++++++++ src/documents/tests/test_api_custom_fields.py | 54 +++++++++++++++++++ 2 files changed, 91 insertions(+) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 87c6657da..3a7f505db 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -646,10 +646,26 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): custom_field.data_type, ) + api_version = int( + self.context.get("request").version + if self.context.get("request") + else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + ) + if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK: # prior to update so we can look for any docs that are going to be removed self.reflect_doclinks(document, custom_field, validated_data["value"]) + if ( + custom_field.data_type == CustomField.FieldDataType.SELECT + and api_version < 7 + ): + # Convert the index of the option in the field.extra_data["select_options"] list + # to the actual value + validated_data["value"] = custom_field.extra_data["select_options"][ + validated_data["value"] + ]["id"] + # Actually update or create the instance, providing the value # to fill in the correct attribute based on the type instance, _ = CustomFieldInstance.objects.update_or_create( @@ -660,6 +676,21 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): return instance def get_value(self, obj: CustomFieldInstance): + api_version = int( + self.context.get("request").version + if self.context.get("request") + else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + ) + if api_version < 7 and obj.field.data_type == CustomField.FieldDataType.SELECT: + # return the index of the option in the field.extra_data["select_options"] list + return next( + ( + idx + for idx, option in enumerate(obj.field.extra_data["select_options"]) + if option["id"] == obj.value + ), + None, + ) return obj.value def validate(self, data): @@ -958,6 +989,12 @@ class DocumentSerializer( ): kwargs["full_perms"] = True + self.api_version = int( + context.get("request").version + if context.get("request") + else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + ) + super().__init__(*args, **kwargs) class Meta: diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index cd15de006..e4a261ff7 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -328,6 +328,60 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): ], ) + def test_custom_field_select_value_old_version(self): + """ + GIVEN: + - Existing document with custom field select + WHEN: + - API post request is made to add the field for document with api version header < 7 + - API get request is made for document with api version header < 7 + THEN: + - The select value is returned in the old format, the index of the option + """ + custom_field_select = CustomField.objects.create( + name="Select Field", + data_type=CustomField.FieldDataType.SELECT, + extra_data={ + "select_options": [ + {"label": "Option 1", "id": "abc-123"}, + {"label": "Option 2", "id": "def-456"}, + ], + }, + ) + + doc = Document.objects.create( + title="WOW", + content="the content", + checksum="123", + mime_type="application/pdf", + ) + CustomFieldInstance.objects.create( + document=doc, + field=custom_field_select, + value_select="abc-123", + ) + + resp = self.client.patch( + f"/api/documents/{doc.id}/", + headers={"Accept": "application/json; version=6"}, + data={ + "custom_fields": [ + {"field": custom_field_select.id, "value": 0}, + ], + }, + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + self.assertEqual(doc.custom_fields.first().value, "abc-123") + + resp = self.client.get( + f"/api/documents/{doc.id}/", + headers={"Accept": "application/json; version=6"}, + ) + self.assertEqual(resp.status_code, status.HTTP_200_OK) + + data = resp.json() + self.assertEqual(data["custom_fields"][0]["value"], 0) + def test_create_custom_field_monetary_validation(self): """ GIVEN: From 3cb43b02fff6428c9fab310053f5abbf6b587e55 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:14:11 -0800 Subject: [PATCH 04/13] Dont make default version change --- docs/api.md | 5 ++++- src/documents/tests/test_api_custom_fields.py | 4 +++- src/paperless/settings.py | 2 +- 3 files changed, 8 insertions(+), 3 deletions(-) diff --git a/docs/api.md b/docs/api.md index 59f9d7efc..d02a5117c 100644 --- a/docs/api.md +++ b/docs/api.md @@ -577,4 +577,7 @@ Initial API version. #### Version 7 - The format of select type custom fields has changed to return the options - as an array of objects with `id` and `label` fields. + as an array of objects with `id` and `label` fields. When creating or + updating a custom field value of a document for a select type custom field, + the value should be the `id` of the option whereas previously was the index + of the option. diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index e4a261ff7..0479c103a 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -151,7 +151,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): def test_custom_field_select_unique_ids(self): """ GIVEN: - - Nothing - Existing custom field WHEN: - API request to create custom field with select options without id @@ -172,6 +171,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, }, ), + headers={"Accept": "application/json; version=7"}, content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) @@ -197,6 +197,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, }, ), + headers={"Accept": "application/json; version=7"}, content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) @@ -560,6 +561,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, ], }, + headers={"Accept": "application/json; version=7"}, format="json", ) diff --git a/src/paperless/settings.py b/src/paperless/settings.py index dcfdc020d..53a6902d5 100644 --- a/src/paperless/settings.py +++ b/src/paperless/settings.py @@ -341,7 +341,7 @@ REST_FRAMEWORK = { "rest_framework.authentication.SessionAuthentication", ], "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning", - "DEFAULT_VERSION": "7", + "DEFAULT_VERSION": "1", # Make sure these are ordered and that the most recent version appears # last "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6", "7"], From 30c5ec7fc4df7cd3da4b9a3c78ab7ee985c3e9ea Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:18:54 -0800 Subject: [PATCH 05/13] Update api.md --- docs/api.md | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/api.md b/docs/api.md index d02a5117c..536418844 100644 --- a/docs/api.md +++ b/docs/api.md @@ -577,7 +577,7 @@ Initial API version. #### Version 7 - The format of select type custom fields has changed to return the options - as an array of objects with `id` and `label` fields. When creating or - updating a custom field value of a document for a select type custom field, - the value should be the `id` of the option whereas previously was the index - of the option. + as an array of objects with `id` and `label` fields as opposed to a simple + list of strings. When creating or updating a custom field value of a + document for a select type custom field, the value should be the `id` of + the option whereas previously was the index of the option. From 69b922295055f47bde63e5581b7be79efdbed6ce Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:24:59 -0800 Subject: [PATCH 06/13] Fix these tests --- src/documents/tests/test_api_filter_by_custom_fields.py | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/documents/tests/test_api_filter_by_custom_fields.py b/src/documents/tests/test_api_filter_by_custom_fields.py index c7e9092ed..deb97bf29 100644 --- a/src/documents/tests/test_api_filter_by_custom_fields.py +++ b/src/documents/tests/test_api_filter_by_custom_fields.py @@ -1,7 +1,7 @@ import json +import types from collections.abc import Callable from datetime import date -from unittest.mock import Mock from urllib.parse import quote from django.contrib.auth.models import User @@ -149,7 +149,12 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase): document, data=data, partial=True, - context={"request": Mock()}, + context={ + "request": types.SimpleNamespace( + method="GET", + version="7", + ), + }, ) serializer.is_valid(raise_exception=True) serializer.save() From 1566011f0b1cd6bcb59a3e6bb73bc777432e7dc1 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 19:55:39 -0800 Subject: [PATCH 07/13] Update serialisers.py --- src/documents/serialisers.py | 15 ++++----------- 1 file changed, 4 insertions(+), 11 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 3a7f505db..3894181f4 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -515,7 +515,7 @@ class CustomFieldSerializer(serializers.ModelSerializer): self.api_version = int( context.get("request").version if context.get("request") - else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], ) super().__init__(*args, **kwargs) @@ -628,8 +628,7 @@ class CustomFieldSerializer(serializers.ModelSerializer): extra_data["select_options"] = [ option["label"] for option in extra_data["select_options"] ] - field = serializers.JSONField() - return field.to_representation(extra_data) + return serializers.JSONField().to_representation(extra_data) class CustomFieldInstanceSerializer(serializers.ModelSerializer): @@ -649,7 +648,7 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): api_version = int( self.context.get("request").version if self.context.get("request") - else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], ) if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK: @@ -679,7 +678,7 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): api_version = int( self.context.get("request").version if self.context.get("request") - else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], ) if api_version < 7 and obj.field.data_type == CustomField.FieldDataType.SELECT: # return the index of the option in the field.extra_data["select_options"] list @@ -989,12 +988,6 @@ class DocumentSerializer( ): kwargs["full_perms"] = True - self.api_version = int( - context.get("request").version - if context.get("request") - else settings.REST_FRAMEWORK["ALLOWED_VERSIONS"][-1], - ) - super().__init__(*args, **kwargs) class Meta: From 1ee7728fc5f8370e21814fa82234810c0b024d57 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 21:47:16 -0800 Subject: [PATCH 08/13] Fix post old format --- src/documents/serialisers.py | 30 +++++++++---------- src/documents/tests/test_api_custom_fields.py | 24 +++++++-------- 2 files changed, 26 insertions(+), 28 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index 3894181f4..d163e769d 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -645,26 +645,10 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): custom_field.data_type, ) - api_version = int( - self.context.get("request").version - if self.context.get("request") - else settings.REST_FRAMEWORK["DEFAULT_VERSION"], - ) - if custom_field.data_type == CustomField.FieldDataType.DOCUMENTLINK: # prior to update so we can look for any docs that are going to be removed self.reflect_doclinks(document, custom_field, validated_data["value"]) - if ( - custom_field.data_type == CustomField.FieldDataType.SELECT - and api_version < 7 - ): - # Convert the index of the option in the field.extra_data["select_options"] list - # to the actual value - validated_data["value"] = custom_field.extra_data["select_options"][ - validated_data["value"] - ]["id"] - # Actually update or create the instance, providing the value # to fill in the correct attribute based on the type instance, _ = CustomFieldInstance.objects.update_or_create( @@ -702,6 +686,12 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): """ data = super().validate(data) field: CustomField = data["field"] + api_version = int( + self.context.get("request").version + if self.context.get("request") + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], + ) + if "value" in data and data["value"] is not None: if ( field.data_type == CustomField.FieldDataType.URL @@ -729,6 +719,14 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): MaxLengthValidator(limit_value=128)(data["value"]) elif field.data_type == CustomField.FieldDataType.SELECT: select_options = field.extra_data["select_options"] + + if api_version < 7: + # Convert the index of the option in the field.extra_data["select_options"] + # list to the options unique id + data["value"] = field.extra_data["select_options"][data["value"]][ + "id" + ] + try: next( option diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 0479c103a..8cb42aa8d 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -356,23 +356,22 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): checksum="123", mime_type="application/pdf", ) - CustomFieldInstance.objects.create( - document=doc, - field=custom_field_select, - value_select="abc-123", - ) resp = self.client.patch( f"/api/documents/{doc.id}/", headers={"Accept": "application/json; version=6"}, - data={ - "custom_fields": [ - {"field": custom_field_select.id, "value": 0}, - ], - }, + data=json.dumps( + { + "custom_fields": [ + {"field": custom_field_select.id, "value": 1}, + ], + }, + ), + content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) - self.assertEqual(doc.custom_fields.first().value, "abc-123") + doc.refresh_from_db() + self.assertEqual(doc.custom_fields.first().value, "def-456") resp = self.client.get( f"/api/documents/{doc.id}/", @@ -381,7 +380,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): self.assertEqual(resp.status_code, status.HTTP_200_OK) data = resp.json() - self.assertEqual(data["custom_fields"][0]["value"], 0) + self.assertEqual(data["custom_fields"][0]["value"], 1) def test_create_custom_field_monetary_validation(self): """ @@ -980,6 +979,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): resp = self.client.patch( f"/api/documents/{doc.id}/", + headers={"Accept": "application/json; version=7"}, data={ "custom_fields": [ {"field": custom_field_select.id, "value": "not an option"}, From 2ae70b64826cb8690acde34ce2b7860990a6e843 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 22:27:30 -0800 Subject: [PATCH 09/13] DRY --- src/documents/serialisers.py | 174 ++++++++++-------- src/documents/tests/test_api_custom_fields.py | 3 +- src/documents/views.py | 4 - 3 files changed, 99 insertions(+), 82 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index d163e769d..dda834094 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -495,20 +495,6 @@ class StoragePathField(serializers.PrimaryKeyRelatedField): return StoragePath.objects.all() -class ReadWriteSerializerMethodField(serializers.SerializerMethodField): - """ - Based on https://stackoverflow.com/a/62579804 - """ - - def __init__(self, method_name=None, *args, **kwargs): - self.method_name = method_name - kwargs["source"] = "*" - super(serializers.SerializerMethodField, self).__init__(*args, **kwargs) - - def to_internal_value(self, data): - return {self.field_name: data} - - class CustomFieldSerializer(serializers.ModelSerializer): def __init__(self, *args, **kwargs): context = kwargs.get("context") @@ -526,8 +512,6 @@ class CustomFieldSerializer(serializers.ModelSerializer): document_count = serializers.IntegerField(read_only=True) - extra_data = ReadWriteSerializerMethodField(required=False) - class Meta: model = CustomField fields = [ @@ -569,39 +553,18 @@ class CustomFieldSerializer(serializers.ModelSerializer): or "select_options" not in attrs["extra_data"] or not isinstance(attrs["extra_data"]["select_options"], list) or len(attrs["extra_data"]["select_options"]) == 0 - or ( - # version 6 and below require a list of strings - self.api_version < 7 - and not all( - len(option) > 0 - for option in attrs["extra_data"]["select_options"] - ) - ) - or ( - # version 7 and above require a list of objects with labels - self.api_version >= 7 - and not all( - len(option.get("label", "")) > 0 - for option in attrs["extra_data"]["select_options"] - ) + or not all( + len(option.get("label", "")) > 0 + for option in attrs["extra_data"]["select_options"] ) ): raise serializers.ValidationError( {"error": "extra_data.select_options must be a valid list"}, ) # labels are valid, generate ids if not present - if self.api_version < 7: - attrs["extra_data"]["select_options"] = [ - { - "label": option, - "id": get_random_string(length=16), - } - for option in attrs["extra_data"]["select_options"] - ] - else: - for option in attrs["extra_data"]["select_options"]: - if option.get("id") is None: - option["id"] = get_random_string(length=16) + for option in attrs["extra_data"]["select_options"]: + if option.get("id") is None: + option["id"] = get_random_string(length=16) elif ( "data_type" in attrs and attrs["data_type"] == CustomField.FieldDataType.MONETARY @@ -621,14 +584,51 @@ class CustomFieldSerializer(serializers.ModelSerializer): ) return super().validate(attrs) - def get_extra_data(self, obj): - extra_data = obj.extra_data - if self.api_version < 7 and obj.data_type == CustomField.FieldDataType.SELECT: - # Convert the select options with ids to a list of strings - extra_data["select_options"] = [ - option["label"] for option in extra_data["select_options"] + def to_internal_value(self, data): + ret = super().to_internal_value(data) + + if ( + self.api_version < 7 + and ret.get("data_type", "") == CustomField.FieldDataType.SELECT + and isinstance(ret.get("extra_data", {}).get("select_options"), list) + ): + ret["extra_data"]["select_options"] = [ + { + "label": option, + "id": get_random_string(length=16), + } + for option in ret["extra_data"]["select_options"] ] - return serializers.JSONField().to_representation(extra_data) + + return ret + + def to_representation(self, instance): + ret = super().to_representation(instance) + + if ( + self.api_version < 7 + and instance.data_type == CustomField.FieldDataType.SELECT + ): + # Convert the select options with ids to a list of strings + ret["extra_data"]["select_options"] = [ + option["label"] for option in ret["extra_data"]["select_options"] + ] + + return ret + + +class ReadWriteSerializerMethodField(serializers.SerializerMethodField): + """ + Based on https://stackoverflow.com/a/62579804 + """ + + def __init__(self, method_name=None, *args, **kwargs): + self.method_name = method_name + kwargs["source"] = "*" + super(serializers.SerializerMethodField, self).__init__(*args, **kwargs) + + def to_internal_value(self, data): + return {self.field_name: data} class CustomFieldInstanceSerializer(serializers.ModelSerializer): @@ -659,21 +659,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): return instance def get_value(self, obj: CustomFieldInstance): - api_version = int( - self.context.get("request").version - if self.context.get("request") - else settings.REST_FRAMEWORK["DEFAULT_VERSION"], - ) - if api_version < 7 and obj.field.data_type == CustomField.FieldDataType.SELECT: - # return the index of the option in the field.extra_data["select_options"] list - return next( - ( - idx - for idx, option in enumerate(obj.field.extra_data["select_options"]) - if option["id"] == obj.value - ), - None, - ) return obj.value def validate(self, data): @@ -686,11 +671,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): """ data = super().validate(data) field: CustomField = data["field"] - api_version = int( - self.context.get("request").version - if self.context.get("request") - else settings.REST_FRAMEWORK["DEFAULT_VERSION"], - ) if "value" in data and data["value"] is not None: if ( @@ -720,13 +700,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): elif field.data_type == CustomField.FieldDataType.SELECT: select_options = field.extra_data["select_options"] - if api_version < 7: - # Convert the index of the option in the field.extra_data["select_options"] - # list to the options unique id - data["value"] = field.extra_data["select_options"][data["value"]][ - "id" - ] - try: next( option @@ -752,6 +725,53 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): return data + def to_internal_value(self, data): + ret = super().to_internal_value(data) + + api_version = int( + self.context.get("request").version + if self.context.get("request") + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], + ) + if ( + api_version < 7 + and ret.get("field").data_type == CustomField.FieldDataType.SELECT + and ret.get("value") is not None + ): + # Convert the index of the option in the field.extra_data["select_options"] + # list to the options unique id + ret["value"] = ret.get("field").extra_data["select_options"][ret["value"]][ + "id" + ] + + return ret + + def to_representation(self, instance): + ret = super().to_representation(instance) + + api_version = int( + self.context.get("request").version + if self.context.get("request") + else settings.REST_FRAMEWORK["DEFAULT_VERSION"], + ) + if ( + api_version < 7 + and instance.field.data_type == CustomField.FieldDataType.SELECT + ): + # return the index of the option in the field.extra_data["select_options"] list + ret["value"] = next( + ( + idx + for idx, option in enumerate( + instance.field.extra_data["select_options"], + ) + if option["id"] == instance.value + ), + None, + ) + + return ret + def reflect_doclinks( self, document: Document, diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 8cb42aa8d..8a9e32174 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -249,7 +249,8 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): resp = self.client.patch( f"{self.ENDPOINT}{custom_field_select.id}/", - json.dumps( + headers={"Accept": "application/json; version=7"}, + data=json.dumps( { "extra_data": { "select_options": [ diff --git a/src/documents/views.py b/src/documents/views.py index 2868c1e4e..f98932a6f 100644 --- a/src/documents/views.py +++ b/src/documents/views.py @@ -2065,10 +2065,6 @@ class CustomFieldViewSet(ModelViewSet): ) ) - def get_serializer(self, *args, **kwargs): - kwargs.setdefault("context", self.get_serializer_context()) - return super().get_serializer(*args, **kwargs) - class SystemStatusView(PassUserMixin): permission_classes = (IsAuthenticated,) From e531ba82cec94f00ddc9275519c671f16c4eef18 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 22:42:42 -0800 Subject: [PATCH 10/13] DRY-er --- src/documents/serialisers.py | 21 ++++++++------------- 1 file changed, 8 insertions(+), 13 deletions(-) diff --git a/src/documents/serialisers.py b/src/documents/serialisers.py index dda834094..0732fd242 100644 --- a/src/documents/serialisers.py +++ b/src/documents/serialisers.py @@ -671,7 +671,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): """ data = super().validate(data) field: CustomField = data["field"] - if "value" in data and data["value"] is not None: if ( field.data_type == CustomField.FieldDataType.URL @@ -699,7 +698,6 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): MaxLengthValidator(limit_value=128)(data["value"]) elif field.data_type == CustomField.FieldDataType.SELECT: select_options = field.extra_data["select_options"] - try: next( option @@ -725,16 +723,18 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): return data - def to_internal_value(self, data): - ret = super().to_internal_value(data) - - api_version = int( + def get_api_version(self): + return int( self.context.get("request").version if self.context.get("request") else settings.REST_FRAMEWORK["DEFAULT_VERSION"], ) + + def to_internal_value(self, data): + ret = super().to_internal_value(data) + if ( - api_version < 7 + self.get_api_version() < 7 and ret.get("field").data_type == CustomField.FieldDataType.SELECT and ret.get("value") is not None ): @@ -749,13 +749,8 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): def to_representation(self, instance): ret = super().to_representation(instance) - api_version = int( - self.context.get("request").version - if self.context.get("request") - else settings.REST_FRAMEWORK["DEFAULT_VERSION"], - ) if ( - api_version < 7 + self.get_api_version() < 7 and instance.field.data_type == CustomField.FieldDataType.SELECT ): # return the index of the option in the field.extra_data["select_options"] list From bb96b5f98e4ed87970465f3d19e59511b43871ed Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sat, 25 Jan 2025 22:48:14 -0800 Subject: [PATCH 11/13] Update test_api_custom_fields.py --- src/documents/tests/test_api_custom_fields.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index 8a9e32174..f94d73edc 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -280,11 +280,12 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): def test_custom_field_select_old_version(self): """ GIVEN: - - Select custom field exists with old version of select options + - Nothing WHEN: - API post request is made for custom fields with api version header < 7 - API get request is made for custom fields with api version header < 7 THEN: + - The select options are created with unique ids - The select options are returned in the old format """ resp = self.client.post( From 2e956b0f13b65d71607915207f385787f4533e04 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Sun, 26 Jan 2025 07:28:07 -0800 Subject: [PATCH 12/13] Update api default version to newest --- src/documents/tests/test_api_custom_fields.py | 5 ---- src/documents/tests/test_api_documents.py | 28 +++++++++++++------ src/paperless/settings.py | 2 +- 3 files changed, 20 insertions(+), 15 deletions(-) diff --git a/src/documents/tests/test_api_custom_fields.py b/src/documents/tests/test_api_custom_fields.py index f94d73edc..8e24226dc 100644 --- a/src/documents/tests/test_api_custom_fields.py +++ b/src/documents/tests/test_api_custom_fields.py @@ -171,7 +171,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, }, ), - headers={"Accept": "application/json; version=7"}, content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_201_CREATED) @@ -197,7 +196,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, }, ), - headers={"Accept": "application/json; version=7"}, content_type="application/json", ) self.assertEqual(resp.status_code, status.HTTP_200_OK) @@ -249,7 +247,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): resp = self.client.patch( f"{self.ENDPOINT}{custom_field_select.id}/", - headers={"Accept": "application/json; version=7"}, data=json.dumps( { "extra_data": { @@ -562,7 +559,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): }, ], }, - headers={"Accept": "application/json; version=7"}, format="json", ) @@ -981,7 +977,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): resp = self.client.patch( f"/api/documents/{doc.id}/", - headers={"Accept": "application/json; version=7"}, data={ "custom_fields": [ {"field": custom_field_select.id, "value": "not an option"}, diff --git a/src/documents/tests/test_api_documents.py b/src/documents/tests/test_api_documents.py index ea5227c8a..70db15217 100644 --- a/src/documents/tests/test_api_documents.py +++ b/src/documents/tests/test_api_documents.py @@ -2029,31 +2029,37 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase): self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#a6cee3") self.assertEqual( - self.client.get(f"/api/tags/{response.data['id']}/", format="json").data[ - "colour" - ], + self.client.get( + f"/api/tags/{response.data['id']}/", + headers={"Accept": "application/json; version=1"}, + format="json", + ).data["colour"], 1, ) def test_tag_color(self): response = self.client.post( "/api/tags/", - {"name": "tag", "colour": 3}, + data={"name": "tag", "colour": 3}, + headers={"Accept": "application/json; version=1"}, format="json", ) self.assertEqual(response.status_code, status.HTTP_201_CREATED) self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#b2df8a") self.assertEqual( - self.client.get(f"/api/tags/{response.data['id']}/", format="json").data[ - "colour" - ], + self.client.get( + f"/api/tags/{response.data['id']}/", + headers={"Accept": "application/json; version=1"}, + format="json", + ).data["colour"], 3, ) def test_tag_color_invalid(self): response = self.client.post( "/api/tags/", - {"name": "tag", "colour": 34}, + data={"name": "tag", "colour": 34}, + headers={"Accept": "application/json; version=1"}, format="json", ) self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) @@ -2061,7 +2067,11 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase): def test_tag_color_custom(self): tag = Tag.objects.create(name="test", color="#abcdef") self.assertEqual( - self.client.get(f"/api/tags/{tag.id}/", format="json").data["colour"], + self.client.get( + f"/api/tags/{tag.id}/", + headers={"Accept": "application/json; version=1"}, + format="json", + ).data["colour"], 1, ) diff --git a/src/paperless/settings.py b/src/paperless/settings.py index 53a6902d5..dcfdc020d 100644 --- a/src/paperless/settings.py +++ b/src/paperless/settings.py @@ -341,7 +341,7 @@ REST_FRAMEWORK = { "rest_framework.authentication.SessionAuthentication", ], "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning", - "DEFAULT_VERSION": "1", + "DEFAULT_VERSION": "7", # Make sure these are ordered and that the most recent version appears # last "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6", "7"], From 7cbb3a3e8ed7386d5b9620e695b8b8cd05375ae1 Mon Sep 17 00:00:00 2001 From: shamoon <4887959+shamoon@users.noreply.github.com> Date: Mon, 27 Jan 2025 00:13:17 -0800 Subject: [PATCH 13/13] Add api deprecation policy --- docs/api.md | 6 ++++++ src/paperless/settings.py | 2 +- 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/docs/api.md b/docs/api.md index 536418844..050443c19 100644 --- a/docs/api.md +++ b/docs/api.md @@ -541,6 +541,12 @@ server, the following procedure should be performed: 2. Determine whether the client is compatible with this server based on the presence/absence of these headers and their values if present. +### API Version Deprecation Policy + +Older API versions are guaranteed to be supported for at least one year +after the release of a new API version. After that, support for older +API versions may be (but is not guaranteed to be) dropped. + ### API Changelog #### Version 1 diff --git a/src/paperless/settings.py b/src/paperless/settings.py index dcfdc020d..a817abd70 100644 --- a/src/paperless/settings.py +++ b/src/paperless/settings.py @@ -343,7 +343,7 @@ REST_FRAMEWORK = { "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning", "DEFAULT_VERSION": "7", # Make sure these are ordered and that the most recent version appears - # last + # last. See api.md#api-versioning when adding new versions. "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6", "7"], }