mirror of
				https://github.com/paperless-ngx/paperless-ngx.git
				synced 2025-10-30 03:56:23 -05:00 
			
		
		
		
	Fix: backwards-compatible versioned API response for custom field select fields (#8912)
This commit is contained in:
		
							
								
								
									
										14
									
								
								docs/api.md
									
									
									
									
									
								
							
							
						
						
									
										14
									
								
								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 | 2.  Determine whether the client is compatible with this server based on | ||||||
|     the presence/absence of these headers and their values if present. |     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 | ### API Changelog | ||||||
|  |  | ||||||
| #### Version 1 | #### Version 1 | ||||||
| @@ -573,3 +579,11 @@ Initial API version. | |||||||
| #### Version 6 | #### Version 6 | ||||||
|  |  | ||||||
| -   Moved acknowledge tasks endpoint to be under `/api/tasks/acknowledge/`. | -   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 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. | ||||||
|   | |||||||
| @@ -3,7 +3,7 @@ const base_url = new URL(document.baseURI) | |||||||
| export const environment = { | export const environment = { | ||||||
|   production: true, |   production: true, | ||||||
|   apiBaseUrl: document.baseURI + 'api/', |   apiBaseUrl: document.baseURI + 'api/', | ||||||
|   apiVersion: '6', |   apiVersion: '7', | ||||||
|   appTitle: 'Paperless-ngx', |   appTitle: 'Paperless-ngx', | ||||||
|   version: '2.14.5', |   version: '2.14.5', | ||||||
|   webSocketHost: window.location.host, |   webSocketHost: window.location.host, | ||||||
|   | |||||||
| @@ -5,7 +5,7 @@ | |||||||
| export const environment = { | export const environment = { | ||||||
|   production: false, |   production: false, | ||||||
|   apiBaseUrl: 'http://localhost:8000/api/', |   apiBaseUrl: 'http://localhost:8000/api/', | ||||||
|   apiVersion: '6', |   apiVersion: '7', | ||||||
|   appTitle: 'Paperless-ngx', |   appTitle: 'Paperless-ngx', | ||||||
|   version: 'DEVELOPMENT', |   version: 'DEVELOPMENT', | ||||||
|   webSocketHost: 'localhost:8000', |   webSocketHost: 'localhost:8000', | ||||||
|   | |||||||
| @@ -496,6 +496,15 @@ class StoragePathField(serializers.PrimaryKeyRelatedField): | |||||||
|  |  | ||||||
|  |  | ||||||
| class CustomFieldSerializer(serializers.ModelSerializer): | class CustomFieldSerializer(serializers.ModelSerializer): | ||||||
|  |     def __init__(self, *args, **kwargs): | ||||||
|  |         context = kwargs.get("context") | ||||||
|  |         self.api_version = int( | ||||||
|  |             context.get("request").version | ||||||
|  |             if context.get("request") | ||||||
|  |             else settings.REST_FRAMEWORK["DEFAULT_VERSION"], | ||||||
|  |         ) | ||||||
|  |         super().__init__(*args, **kwargs) | ||||||
|  |  | ||||||
|     data_type = serializers.ChoiceField( |     data_type = serializers.ChoiceField( | ||||||
|         choices=CustomField.FieldDataType, |         choices=CustomField.FieldDataType, | ||||||
|         read_only=False, |         read_only=False, | ||||||
| @@ -575,6 +584,38 @@ class CustomFieldSerializer(serializers.ModelSerializer): | |||||||
|             ) |             ) | ||||||
|         return super().validate(attrs) |         return super().validate(attrs) | ||||||
|  |  | ||||||
|  |     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 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): | class ReadWriteSerializerMethodField(serializers.SerializerMethodField): | ||||||
|     """ |     """ | ||||||
| @@ -682,6 +723,50 @@ class CustomFieldInstanceSerializer(serializers.ModelSerializer): | |||||||
|  |  | ||||||
|         return data |         return data | ||||||
|  |  | ||||||
|  |     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 ( | ||||||
|  |             self.get_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) | ||||||
|  |  | ||||||
|  |         if ( | ||||||
|  |             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 | ||||||
|  |             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( |     def reflect_doclinks( | ||||||
|         self, |         self, | ||||||
|         document: Document, |         document: Document, | ||||||
|   | |||||||
| @@ -43,10 +43,13 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): | |||||||
|         ]: |         ]: | ||||||
|             resp = self.client.post( |             resp = self.client.post( | ||||||
|                 self.ENDPOINT, |                 self.ENDPOINT, | ||||||
|                 data={ |                 data=json.dumps( | ||||||
|  |                     { | ||||||
|                         "data_type": field_type, |                         "data_type": field_type, | ||||||
|                         "name": name, |                         "name": name, | ||||||
|                     }, |                     }, | ||||||
|  |                 ), | ||||||
|  |                 content_type="application/json", | ||||||
|             ) |             ) | ||||||
|             self.assertEqual(resp.status_code, status.HTTP_201_CREATED) |             self.assertEqual(resp.status_code, status.HTTP_201_CREATED) | ||||||
|  |  | ||||||
| @@ -148,7 +151,6 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): | |||||||
|     def test_custom_field_select_unique_ids(self): |     def test_custom_field_select_unique_ids(self): | ||||||
|         """ |         """ | ||||||
|         GIVEN: |         GIVEN: | ||||||
|             - Nothing |  | ||||||
|             - Existing custom field |             - Existing custom field | ||||||
|         WHEN: |         WHEN: | ||||||
|             - API request to create custom field with select options without id |             - API request to create custom field with select options without id | ||||||
| @@ -245,7 +247,7 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): | |||||||
|  |  | ||||||
|         resp = self.client.patch( |         resp = self.client.patch( | ||||||
|             f"{self.ENDPOINT}{custom_field_select.id}/", |             f"{self.ENDPOINT}{custom_field_select.id}/", | ||||||
|             json.dumps( |             data=json.dumps( | ||||||
|                 { |                 { | ||||||
|                     "extra_data": { |                     "extra_data": { | ||||||
|                         "select_options": [ |                         "select_options": [ | ||||||
| @@ -272,6 +274,113 @@ class TestCustomFieldsAPI(DirectoriesMixin, APITestCase): | |||||||
|         doc.refresh_from_db() |         doc.refresh_from_db() | ||||||
|         self.assertEqual(doc.custom_fields.first().value, None) |         self.assertEqual(doc.custom_fields.first().value, None) | ||||||
|  |  | ||||||
|  |     def test_custom_field_select_old_version(self): | ||||||
|  |         """ | ||||||
|  |         GIVEN: | ||||||
|  |             - 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( | ||||||
|  |             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_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", | ||||||
|  |         ) | ||||||
|  |  | ||||||
|  |         resp = self.client.patch( | ||||||
|  |             f"/api/documents/{doc.id}/", | ||||||
|  |             headers={"Accept": "application/json; version=6"}, | ||||||
|  |             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) | ||||||
|  |         doc.refresh_from_db() | ||||||
|  |         self.assertEqual(doc.custom_fields.first().value, "def-456") | ||||||
|  |  | ||||||
|  |         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"], 1) | ||||||
|  |  | ||||||
|     def test_create_custom_field_monetary_validation(self): |     def test_create_custom_field_monetary_validation(self): | ||||||
|         """ |         """ | ||||||
|         GIVEN: |         GIVEN: | ||||||
|   | |||||||
| @@ -2029,31 +2029,37 @@ class TestDocumentApi(DirectoriesMixin, DocumentConsumeDelayMixin, APITestCase): | |||||||
|         self.assertEqual(response.status_code, status.HTTP_201_CREATED) |         self.assertEqual(response.status_code, status.HTTP_201_CREATED) | ||||||
|         self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#a6cee3") |         self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#a6cee3") | ||||||
|         self.assertEqual( |         self.assertEqual( | ||||||
|             self.client.get(f"/api/tags/{response.data['id']}/", format="json").data[ |             self.client.get( | ||||||
|                 "colour" |                 f"/api/tags/{response.data['id']}/", | ||||||
|             ], |                 headers={"Accept": "application/json; version=1"}, | ||||||
|  |                 format="json", | ||||||
|  |             ).data["colour"], | ||||||
|             1, |             1, | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|     def test_tag_color(self): |     def test_tag_color(self): | ||||||
|         response = self.client.post( |         response = self.client.post( | ||||||
|             "/api/tags/", |             "/api/tags/", | ||||||
|             {"name": "tag", "colour": 3}, |             data={"name": "tag", "colour": 3}, | ||||||
|  |             headers={"Accept": "application/json; version=1"}, | ||||||
|             format="json", |             format="json", | ||||||
|         ) |         ) | ||||||
|         self.assertEqual(response.status_code, status.HTTP_201_CREATED) |         self.assertEqual(response.status_code, status.HTTP_201_CREATED) | ||||||
|         self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#b2df8a") |         self.assertEqual(Tag.objects.get(id=response.data["id"]).color, "#b2df8a") | ||||||
|         self.assertEqual( |         self.assertEqual( | ||||||
|             self.client.get(f"/api/tags/{response.data['id']}/", format="json").data[ |             self.client.get( | ||||||
|                 "colour" |                 f"/api/tags/{response.data['id']}/", | ||||||
|             ], |                 headers={"Accept": "application/json; version=1"}, | ||||||
|  |                 format="json", | ||||||
|  |             ).data["colour"], | ||||||
|             3, |             3, | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|     def test_tag_color_invalid(self): |     def test_tag_color_invalid(self): | ||||||
|         response = self.client.post( |         response = self.client.post( | ||||||
|             "/api/tags/", |             "/api/tags/", | ||||||
|             {"name": "tag", "colour": 34}, |             data={"name": "tag", "colour": 34}, | ||||||
|  |             headers={"Accept": "application/json; version=1"}, | ||||||
|             format="json", |             format="json", | ||||||
|         ) |         ) | ||||||
|         self.assertEqual(response.status_code, status.HTTP_400_BAD_REQUEST) |         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): |     def test_tag_color_custom(self): | ||||||
|         tag = Tag.objects.create(name="test", color="#abcdef") |         tag = Tag.objects.create(name="test", color="#abcdef") | ||||||
|         self.assertEqual( |         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, |             1, | ||||||
|         ) |         ) | ||||||
|  |  | ||||||
|   | |||||||
| @@ -1,7 +1,7 @@ | |||||||
| import json | import json | ||||||
|  | import types | ||||||
| from collections.abc import Callable | from collections.abc import Callable | ||||||
| from datetime import date | from datetime import date | ||||||
| from unittest.mock import Mock |  | ||||||
| from urllib.parse import quote | from urllib.parse import quote | ||||||
|  |  | ||||||
| from django.contrib.auth.models import User | from django.contrib.auth.models import User | ||||||
| @@ -149,7 +149,12 @@ class TestCustomFieldsSearch(DirectoriesMixin, APITestCase): | |||||||
|             document, |             document, | ||||||
|             data=data, |             data=data, | ||||||
|             partial=True, |             partial=True, | ||||||
|             context={"request": Mock()}, |             context={ | ||||||
|  |                 "request": types.SimpleNamespace( | ||||||
|  |                     method="GET", | ||||||
|  |                     version="7", | ||||||
|  |                 ), | ||||||
|  |             }, | ||||||
|         ) |         ) | ||||||
|         serializer.is_valid(raise_exception=True) |         serializer.is_valid(raise_exception=True) | ||||||
|         serializer.save() |         serializer.save() | ||||||
|   | |||||||
| @@ -341,10 +341,10 @@ REST_FRAMEWORK = { | |||||||
|         "rest_framework.authentication.SessionAuthentication", |         "rest_framework.authentication.SessionAuthentication", | ||||||
|     ], |     ], | ||||||
|     "DEFAULT_VERSIONING_CLASS": "rest_framework.versioning.AcceptHeaderVersioning", |     "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 |     # 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"], |     "ALLOWED_VERSIONS": ["1", "2", "3", "4", "5", "6", "7"], | ||||||
| } | } | ||||||
|  |  | ||||||
| if DEBUG: | if DEBUG: | ||||||
|   | |||||||
		Reference in New Issue
	
	Block a user
	 shamoon
					shamoon