From 787abf0118da3685ace439b29013d893fb2550f6 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 31 Dec 2024 14:57:32 +0100 Subject: [PATCH 01/17] [#464] Limited data_choices in admin view --- src/objects/token/admin.py | 33 ++++++++++++++++----------------- 1 file changed, 16 insertions(+), 17 deletions(-) diff --git a/src/objects/token/admin.py b/src/objects/token/admin.py index 7898b30d..40650eb9 100644 --- a/src/objects/token/admin.py +++ b/src/objects/token/admin.py @@ -36,28 +36,23 @@ def get_object_fields(self): object_fields = build_spec(get_field_names(object_serializer.fields), ui=True) return object_fields - def get_data_field_choices(self): + def get_data_field_choices(self, object_type_id): data_fields = {} - for object_type in ObjectType.objects.all(): - client = build_client(object_type.service) - url = f"{object_type.url}/versions" + object_type = ObjectType.objects.filter(id=object_type_id).first() + if not object_type: + return data_fields - try: - response = client.get(url) - except requests.RequestException: - continue - - try: - response_data = response.json() - except requests.JSONDecodeError: - continue + client = build_client(object_type.service) + try: + response = client.get(f"{object_type.url}/versions") + response_data = response.json() # TODO: remove check once API V1 is removed if "results" in response_data: response_data = response_data["results"] # use only first level of properties - data_fields[object_type.id] = { + data_fields[object_type_id] = { version["version"]: { prop: f"record__data__{prop}" for prop in list(version["jsonSchema"].get("properties", {}).keys()) @@ -65,6 +60,9 @@ def get_data_field_choices(self): for version in response_data } + except (requests.RequestException, requests.JSONDecodeError): + pass + return data_fields def get_form_data(self, request, object_id) -> dict: @@ -99,9 +97,10 @@ def get_extra_context(self, request, object_id): for object_type in ObjectType.objects.all() ] objecttypes_available = can_connect_to_objecttypes() - data_field_choices = ( - self.get_data_field_choices() if objecttypes_available else {} - ) + data_field_choices = {} + obj = self.get_object(request, unquote(object_id)) if object_id else None + if objecttypes_available and obj and obj.object_type and obj.use_fields: + data_field_choices = self.get_data_field_choices(obj.object_type.id) return { "object_fields": self.get_object_fields(), From a1348bf76f38caff7b05e21da0e56763c7ab5e34 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 31 Dec 2024 14:57:53 +0100 Subject: [PATCH 02/17] [#464] Fix js --- .../js/components/admin/permissions/auth-fields.js | 8 ++++++++ .../js/components/admin/permissions/permission-form.js | 8 +++++--- 2 files changed, 13 insertions(+), 3 deletions(-) diff --git a/src/objects/js/components/admin/permissions/auth-fields.js b/src/objects/js/components/admin/permissions/auth-fields.js index e421345d..95249ebf 100644 --- a/src/objects/js/components/admin/permissions/auth-fields.js +++ b/src/objects/js/components/admin/permissions/auth-fields.js @@ -61,6 +61,14 @@ const authFields = (object_fields, dataFields, fields, setFields) => { }; const versionAuthFields = (objectType, objectFields, dataFieldChoices, fields, setFields) => { + if (!(objectType in dataFieldChoices)) { + return ( +
+

To be able to select fields, first you must save the instance and then you can edit the fields!

+

Click on button 'Save and continue editing'

+
+ ); + } const dataFields = dataFieldChoices[objectType]; const objecttypeVersions = Object.entries(dataFieldChoices).reduce((acc, [k, v]) => { diff --git a/src/objects/js/components/admin/permissions/permission-form.js b/src/objects/js/components/admin/permissions/permission-form.js index b0ad9f95..30fbe204 100644 --- a/src/objects/js/components/admin/permissions/permission-form.js +++ b/src/objects/js/components/admin/permissions/permission-form.js @@ -9,8 +9,10 @@ const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttyp const [mode, setMode] = useState(values["mode"]); const [useFields, setUseFields] = useState(values["use_fields"]); const [objectType, setObjectType] = useState(values["object_type"]); - - const [fields, setFields] = useState( JSON.parse(values["fields"]) || {}); + if (!values["fields"]) { + values["fields"] = "{}" + } + const [fields, setFields] = useState( JSON.parse(values["fields"]) || {}) return (
@@ -63,7 +65,7 @@ const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttyp name="use_fields" id="id_use_fields" label="Use field-based authorization" - disabled={mode === "read_and_write" || Object.keys(dataFieldChoices).length === 0} + disabled={!mode || mode === "read_and_write"} value={useFields} onChange={(value) => {setUseFields(value)}} /> From 95290016c26714696195ec433423681b78f3b221 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 31 Dec 2024 15:54:44 +0100 Subject: [PATCH 03/17] [#464] Fix tests --- .../tests/admin/test_token_permissions.py | 94 ++++++++++++++++--- 1 file changed, 81 insertions(+), 13 deletions(-) diff --git a/src/objects/tests/admin/test_token_permissions.py b/src/objects/tests/admin/test_token_permissions.py index 644c2b84..47de75d8 100644 --- a/src/objects/tests/admin/test_token_permissions.py +++ b/src/objects/tests/admin/test_token_permissions.py @@ -6,7 +6,12 @@ from requests.exceptions import ConnectionError from objects.accounts.tests.factories import UserFactory -from objects.token.tests.factories import ObjectTypeFactory, TokenAuthFactory +from objects.token.constants import PermissionModes +from objects.token.tests.factories import ( + ObjectTypeFactory, + PermissionFactory, + TokenAuthFactory, +) from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get @@ -37,18 +42,7 @@ def test_add_permission_choices_without_properties(self, m): response = self.app.get(self.url) self.assertEqual(response.status_code, 200) - self.assertEqual( - response.context["data_field_choices"], - { - object_type.id: { - 1: {}, - 2: { - "diameter": "record__data__diameter", - "plantDate": "record__data__plantDate", - }, - } - }, - ) + self.assertEqual(response.context["data_field_choices"], {}) def test_get_permission_with_unavailable_objecttypes(self, m): """ @@ -63,3 +57,77 @@ def test_get_permission_with_unavailable_objecttypes(self, m): response = self.app.get(self.url) self.assertEqual(response.status_code, 200) + + +@disable_admin_mfa() +@requests_mock.Mocker() +class ChangePermissionTests(WebTest): + + def setUp(self): + user = UserFactory(is_superuser=True, is_staff=True) + self.app.set_user(user) + + self.object_type = ObjectTypeFactory.create(service__api_root=OBJECT_TYPES_API) + self.token_auth = TokenAuthFactory.create() + + def test_change_permission_data_field_choices_disabled(self, m): + url = reverse_lazy("admin:token_permission_change") + # use_fields disabled + permission = PermissionFactory.create( + object_type=self.object_type, + mode=PermissionModes.read_only, + token_auth=self.token_auth, + ) + + url = reverse_lazy("admin:token_permission_change", args=(permission.id,)) + + # mock objecttypes api + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get(f"{OBJECT_TYPES_API}objecttypes", json=[]) + m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) + version1 = mock_objecttype_version( + self.object_type.url, attrs={"jsonSchema": {}} + ) + version2 = mock_objecttype_version(self.object_type.url, attrs={"version": 2}) + m.get(f"{self.object_type.url}/versions", json=[version1, version2]) + + response = self.app.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.context["data_field_choices"], {}) + + def test_change_permission_data_field_choices_enabled(self, m): + url = reverse_lazy("admin:token_permission_change") + # use_fields enabled + permission = PermissionFactory.create( + object_type=self.object_type, + mode=PermissionModes.read_only, + token_auth=self.token_auth, + use_fields=True, + ) + + url = reverse_lazy("admin:token_permission_change", args=(permission.id,)) + + # mock objecttypes api + mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") + m.get(f"{OBJECT_TYPES_API}objecttypes", json=[]) + m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) + version1 = mock_objecttype_version( + self.object_type.url, attrs={"jsonSchema": {}} + ) + version2 = mock_objecttype_version(self.object_type.url, attrs={"version": 2}) + m.get(f"{self.object_type.url}/versions", json=[version1, version2]) + + response = self.app.get(url) + self.assertEqual(response.status_code, 200) + self.assertEqual( + response.context["data_field_choices"], + { + self.object_type.id: { + 1: {}, + 2: { + "diameter": "record__data__diameter", + "plantDate": "record__data__plantDate", + }, + } + }, + ) From 59cca54ac0ba125951dcd7df06f2771d0321f29a Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 2 Jan 2025 09:57:40 +0100 Subject: [PATCH 04/17] [#464] Add new control message --- .../js/components/admin/permissions/auth-fields.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/objects/js/components/admin/permissions/auth-fields.js b/src/objects/js/components/admin/permissions/auth-fields.js index 95249ebf..1fe42852 100644 --- a/src/objects/js/components/admin/permissions/auth-fields.js +++ b/src/objects/js/components/admin/permissions/auth-fields.js @@ -61,6 +61,14 @@ const authFields = (object_fields, dataFields, fields, setFields) => { }; const versionAuthFields = (objectType, objectFields, dataFieldChoices, fields, setFields) => { + if (Object.keys(dataFieldChoices).length === 0) { + return ( +
+

ObjectTypes API is not reachable. Field-based authorization is impossible

+
+ ); + } + if (!(objectType in dataFieldChoices)) { return (
From fb2cce64119cab7aa47a25f300b57cd2d7cf3518 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 2 Jan 2025 10:01:17 +0100 Subject: [PATCH 05/17] Revert "[#464] Add new control message" This reverts commit 59cca54ac0ba125951dcd7df06f2771d0321f29a. --- .../js/components/admin/permissions/auth-fields.js | 8 -------- 1 file changed, 8 deletions(-) diff --git a/src/objects/js/components/admin/permissions/auth-fields.js b/src/objects/js/components/admin/permissions/auth-fields.js index 1fe42852..95249ebf 100644 --- a/src/objects/js/components/admin/permissions/auth-fields.js +++ b/src/objects/js/components/admin/permissions/auth-fields.js @@ -61,14 +61,6 @@ const authFields = (object_fields, dataFields, fields, setFields) => { }; const versionAuthFields = (objectType, objectFields, dataFieldChoices, fields, setFields) => { - if (Object.keys(dataFieldChoices).length === 0) { - return ( -
-

ObjectTypes API is not reachable. Field-based authorization is impossible

-
- ); - } - if (!(objectType in dataFieldChoices)) { return (
From f8cced223bcb1c29046bfbfb00fdd46ae4acca62 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 2 Jan 2025 10:22:41 +0100 Subject: [PATCH 06/17] [#464] Fix tests --- .../tests/admin/test_token_permissions.py | 25 ------------------- src/objects/token/admin.py | 2 +- 2 files changed, 1 insertion(+), 26 deletions(-) diff --git a/src/objects/tests/admin/test_token_permissions.py b/src/objects/tests/admin/test_token_permissions.py index 47de75d8..bf36539f 100644 --- a/src/objects/tests/admin/test_token_permissions.py +++ b/src/objects/tests/admin/test_token_permissions.py @@ -70,31 +70,6 @@ def setUp(self): self.object_type = ObjectTypeFactory.create(service__api_root=OBJECT_TYPES_API) self.token_auth = TokenAuthFactory.create() - def test_change_permission_data_field_choices_disabled(self, m): - url = reverse_lazy("admin:token_permission_change") - # use_fields disabled - permission = PermissionFactory.create( - object_type=self.object_type, - mode=PermissionModes.read_only, - token_auth=self.token_auth, - ) - - url = reverse_lazy("admin:token_permission_change", args=(permission.id,)) - - # mock objecttypes api - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - m.get(f"{OBJECT_TYPES_API}objecttypes", json=[]) - m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) - version1 = mock_objecttype_version( - self.object_type.url, attrs={"jsonSchema": {}} - ) - version2 = mock_objecttype_version(self.object_type.url, attrs={"version": 2}) - m.get(f"{self.object_type.url}/versions", json=[version1, version2]) - - response = self.app.get(url) - self.assertEqual(response.status_code, 200) - self.assertEqual(response.context["data_field_choices"], {}) - def test_change_permission_data_field_choices_enabled(self, m): url = reverse_lazy("admin:token_permission_change") # use_fields enabled diff --git a/src/objects/token/admin.py b/src/objects/token/admin.py index 40650eb9..2b5b79f0 100644 --- a/src/objects/token/admin.py +++ b/src/objects/token/admin.py @@ -99,7 +99,7 @@ def get_extra_context(self, request, object_id): objecttypes_available = can_connect_to_objecttypes() data_field_choices = {} obj = self.get_object(request, unquote(object_id)) if object_id else None - if objecttypes_available and obj and obj.object_type and obj.use_fields: + if objecttypes_available and obj and obj.object_type: data_field_choices = self.get_data_field_choices(obj.object_type.id) return { From 1dedca20bf531764382900482418d670bdfc3046 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 7 Jan 2025 16:24:04 +0100 Subject: [PATCH 07/17] [#464] Create endpoint objecttype versions_view + js call --- src/objects/core/admin.py | 31 +++++++++++++++ src/objects/core/models.py | 4 ++ .../admin/permissions/auth-fields.js | 8 ---- .../js/components/admin/permissions/index.js | 1 - .../admin/permissions/permission-form.js | 39 +++++++++++++++++-- .../admin/token/permission/change_form.html | 1 - src/objects/token/admin.py | 38 +----------------- 7 files changed, 71 insertions(+), 51 deletions(-) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index 57cb47a6..3381d696 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -1,6 +1,11 @@ from django.contrib import admin from django.contrib.gis import forms from django.contrib.gis.db.models import GeometryField +from django.http import JsonResponse +from django.urls import path + +import requests +from zgw_consumers.client import build_client from .models import Object, ObjectRecord, ObjectType @@ -13,6 +18,32 @@ class ObjectTypeAdmin(admin.ModelAdmin): ) readonly_fields = ("_name",) + def get_urls(self): + urls = super().get_urls() + my_urls = [ + path( + "/versions/", + self.admin_site.admin_view(self.versions_view), + ) + ] + return my_urls + urls + + def versions_view(self, request, objecttype_id): + versions = {} + if objecttype := self.get_object(request, objecttype_id): + client = build_client(objecttype.service) + try: + response = client.get(objecttype.versions_url) + response_data = response.json() + + # TODO: remove check once API V1 is removed + if "results" in response_data: + versions = response_data["results"] + + except (requests.RequestException, requests.JSONDecodeError): + pass + return JsonResponse(versions, safe=False) + class ObjectRecordInline(admin.TabularInline): model = ObjectRecord diff --git a/src/objects/core/models.py b/src/objects/core/models.py index 03addf37..3ad77620 100644 --- a/src/objects/core/models.py +++ b/src/objects/core/models.py @@ -42,6 +42,10 @@ def url(self): # zds_client.get_operation_url() can be used here but it increases HTTP overhead return f"{self.service.api_root}objecttypes/{self.uuid}" + @property + def versions_url(self): + return f"{self.url}/versions" + def clean_fields(self, exclude: Iterable[str] | None = None) -> None: super().clean_fields(exclude=exclude) diff --git a/src/objects/js/components/admin/permissions/auth-fields.js b/src/objects/js/components/admin/permissions/auth-fields.js index 95249ebf..e421345d 100644 --- a/src/objects/js/components/admin/permissions/auth-fields.js +++ b/src/objects/js/components/admin/permissions/auth-fields.js @@ -61,14 +61,6 @@ const authFields = (object_fields, dataFields, fields, setFields) => { }; const versionAuthFields = (objectType, objectFields, dataFieldChoices, fields, setFields) => { - if (!(objectType in dataFieldChoices)) { - return ( -
-

To be able to select fields, first you must save the instance and then you can edit the fields!

-

Click on button 'Save and continue editing'

-
- ); - } const dataFields = dataFieldChoices[objectType]; const objecttypeVersions = Object.entries(dataFieldChoices).reduce((acc, [k, v]) => { diff --git a/src/objects/js/components/admin/permissions/index.js b/src/objects/js/components/admin/permissions/index.js index f2a7f22a..ea25a390 100644 --- a/src/objects/js/components/admin/permissions/index.js +++ b/src/objects/js/components/admin/permissions/index.js @@ -12,7 +12,6 @@ const mount = () => { ReactDOM.render( { +const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoices, formData}) => { const {values, errors} = formData; const [mode, setMode] = useState(values["mode"]); const [useFields, setUseFields] = useState(values["use_fields"]); const [objectType, setObjectType] = useState(values["object_type"]); if (!values["fields"]) { values["fields"] = "{}" - } + } + const [fields, setFields] = useState( JSON.parse(values["fields"]) || {}) + const [dataFieldChoices, setDataFieldChoices] = useState(dataFieldChoices); + const fetchObjecttypeVersions = (objecttype_id) => { + fetch(`/admin/core/objecttype/`+ objecttype_id +`/versions/`, { + method: 'GET', + }) + .then(response => response.json()) + .then(response_data => { + const objecttypes = { + [objecttype_id]: response_data.reduce((acc, version) => { + const properties = Object.keys(version.jsonSchema?.properties || {}); + acc[version.version] = properties.reduce((propsAcc, prop) => { + propsAcc[prop] = `record__data__${prop}`; + return propsAcc; + }, {}); + return acc; + }, {}) + }; + setDataFieldChoices(objecttypes); + }) + .catch(error => { + console.error('An error occurred while fetching the Objecttype versions endpoint:', error); + }); + }; + useEffect(() => { + if (objectType) { + fetchObjecttypeVersions(objectType); + } + }, [objectType]); + return (
@@ -37,6 +67,7 @@ const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttyp errors={errors["object_type"]} onChange={(value) => { setObjectType(value); + fetchObjecttypeVersions(value); setFields({}); }} /> @@ -78,7 +109,7 @@ const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttyp value={useFields ? JSON.stringify(fields) : ""} /> - { useFields ? + { useFields && dataFieldChoices && objectType in dataFieldChoices ?
diff --git a/src/objects/templates/admin/token/permission/change_form.html b/src/objects/templates/admin/token/permission/change_form.html index 3b62f619..9cdf2249 100644 --- a/src/objects/templates/admin/token/permission/change_form.html +++ b/src/objects/templates/admin/token/permission/change_form.html @@ -5,7 +5,6 @@ {# Inject backend-controlled constants/content into the frontend state #} {{ object_fields|json_script:"object-fields" }} - {{ data_field_choices|json_script:"data-field-choices" }} {{ token_auth_choices|json_script:"token-auth-choices" }} {{ object_type_choices|json_script:"object-type-choices" }} {{ mode_choices|json_script:"mode-choices" }} diff --git a/src/objects/token/admin.py b/src/objects/token/admin.py index 2b5b79f0..3160567d 100644 --- a/src/objects/token/admin.py +++ b/src/objects/token/admin.py @@ -36,35 +36,6 @@ def get_object_fields(self): object_fields = build_spec(get_field_names(object_serializer.fields), ui=True) return object_fields - def get_data_field_choices(self, object_type_id): - data_fields = {} - object_type = ObjectType.objects.filter(id=object_type_id).first() - if not object_type: - return data_fields - - client = build_client(object_type.service) - try: - response = client.get(f"{object_type.url}/versions") - response_data = response.json() - - # TODO: remove check once API V1 is removed - if "results" in response_data: - response_data = response_data["results"] - - # use only first level of properties - data_fields[object_type_id] = { - version["version"]: { - prop: f"record__data__{prop}" - for prop in list(version["jsonSchema"].get("properties", {}).keys()) - } - for version in response_data - } - - except (requests.RequestException, requests.JSONDecodeError): - pass - - return data_fields - def get_form_data(self, request, object_id) -> dict: obj = self.get_object(request, unquote(object_id)) if object_id else None ModelForm = self.get_form(request, obj, change=not obj) @@ -96,20 +67,13 @@ def get_extra_context(self, request, object_id): (object_type.pk, str(object_type)) for object_type in ObjectType.objects.all() ] - objecttypes_available = can_connect_to_objecttypes() - data_field_choices = {} - obj = self.get_object(request, unquote(object_id)) if object_id else None - if objecttypes_available and obj and obj.object_type: - data_field_choices = self.get_data_field_choices(obj.object_type.id) - return { "object_fields": self.get_object_fields(), - "data_field_choices": data_field_choices, "token_auth_choices": token_auth_choices, "object_type_choices": object_type_choices, "mode_choices": mode_choices, "form_data": self.get_form_data(request, object_id), - "objecttypes_available": objecttypes_available, + "objecttypes_available": can_connect_to_objecttypes(), } def change_view(self, request, object_id, form_url="", extra_context=None): From 7eb4f51e69a372e5c76fade35d3fb5e4334b7d0e Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 7 Jan 2025 16:49:53 +0100 Subject: [PATCH 08/17] [#464] Fix tests --- .../tests/admin/test_token_permissions.py | 57 +++---------------- 1 file changed, 7 insertions(+), 50 deletions(-) diff --git a/src/objects/tests/admin/test_token_permissions.py b/src/objects/tests/admin/test_token_permissions.py index bf36539f..2a743719 100644 --- a/src/objects/tests/admin/test_token_permissions.py +++ b/src/objects/tests/admin/test_token_permissions.py @@ -42,7 +42,13 @@ def test_add_permission_choices_without_properties(self, m): response = self.app.get(self.url) self.assertEqual(response.status_code, 200) - self.assertEqual(response.context["data_field_choices"], {}) + + self.assertEqual(version1["jsonSchema"], {}) + self.assertTrue("diameter", version2["jsonSchema"]["properties"].keys()) + self.assertTrue("plantDate", version2["jsonSchema"]["properties"].keys()) + + self.assertFalse("record__data__diameter" in str(response.content)) + self.assertFalse("record__data__plantDate" in str(response.content)) def test_get_permission_with_unavailable_objecttypes(self, m): """ @@ -57,52 +63,3 @@ def test_get_permission_with_unavailable_objecttypes(self, m): response = self.app.get(self.url) self.assertEqual(response.status_code, 200) - - -@disable_admin_mfa() -@requests_mock.Mocker() -class ChangePermissionTests(WebTest): - - def setUp(self): - user = UserFactory(is_superuser=True, is_staff=True) - self.app.set_user(user) - - self.object_type = ObjectTypeFactory.create(service__api_root=OBJECT_TYPES_API) - self.token_auth = TokenAuthFactory.create() - - def test_change_permission_data_field_choices_enabled(self, m): - url = reverse_lazy("admin:token_permission_change") - # use_fields enabled - permission = PermissionFactory.create( - object_type=self.object_type, - mode=PermissionModes.read_only, - token_auth=self.token_auth, - use_fields=True, - ) - - url = reverse_lazy("admin:token_permission_change", args=(permission.id,)) - - # mock objecttypes api - mock_service_oas_get(m, OBJECT_TYPES_API, "objecttypes") - m.get(f"{OBJECT_TYPES_API}objecttypes", json=[]) - m.get(self.object_type.url, json=mock_objecttype(self.object_type.url)) - version1 = mock_objecttype_version( - self.object_type.url, attrs={"jsonSchema": {}} - ) - version2 = mock_objecttype_version(self.object_type.url, attrs={"version": 2}) - m.get(f"{self.object_type.url}/versions", json=[version1, version2]) - - response = self.app.get(url) - self.assertEqual(response.status_code, 200) - self.assertEqual( - response.context["data_field_choices"], - { - self.object_type.id: { - 1: {}, - 2: { - "diameter": "record__data__diameter", - "plantDate": "record__data__plantDate", - }, - } - }, - ) From 7f5fdd664ed75415d4f22d81c1e905d4959cbffc Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Tue, 7 Jan 2025 16:54:50 +0100 Subject: [PATCH 09/17] [#464] Formatting code --- src/objects/tests/admin/test_token_permissions.py | 7 +------ src/objects/token/admin.py | 3 --- 2 files changed, 1 insertion(+), 9 deletions(-) diff --git a/src/objects/tests/admin/test_token_permissions.py b/src/objects/tests/admin/test_token_permissions.py index 2a743719..9e414941 100644 --- a/src/objects/tests/admin/test_token_permissions.py +++ b/src/objects/tests/admin/test_token_permissions.py @@ -6,12 +6,7 @@ from requests.exceptions import ConnectionError from objects.accounts.tests.factories import UserFactory -from objects.token.constants import PermissionModes -from objects.token.tests.factories import ( - ObjectTypeFactory, - PermissionFactory, - TokenAuthFactory, -) +from objects.token.tests.factories import ObjectTypeFactory, TokenAuthFactory from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get diff --git a/src/objects/token/admin.py b/src/objects/token/admin.py index 3160567d..cdda8264 100644 --- a/src/objects/token/admin.py +++ b/src/objects/token/admin.py @@ -2,9 +2,6 @@ from django.contrib.admin.utils import unquote from django.utils.translation import gettext_lazy as _ -import requests -from zgw_consumers.client import build_client - from objects.api.serializers import ObjectSerializer from objects.core.models import ObjectType from objects.core.utils import can_connect_to_objecttypes From d3e962fadbbb118894665155f2d6d7a61631bb30 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Wed, 8 Jan 2025 14:47:49 +0100 Subject: [PATCH 10/17] [#464] Improve JS --- src/objects/core/admin.py | 14 ++++++------ .../admin/permissions/permission-form.js | 22 ++++++++++--------- 2 files changed, 19 insertions(+), 17 deletions(-) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index 3381d696..c03c5240 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -29,17 +29,17 @@ def get_urls(self): return my_urls + urls def versions_view(self, request, objecttype_id): - versions = {} + versions = { + "count": 0, + "next": None, + "previous": None, + "results": [], + } if objecttype := self.get_object(request, objecttype_id): client = build_client(objecttype.service) try: response = client.get(objecttype.versions_url) - response_data = response.json() - - # TODO: remove check once API V1 is removed - if "results" in response_data: - versions = response_data["results"] - + versions = response.json() except (requests.RequestException, requests.JSONDecodeError): pass return JsonResponse(versions, safe=False) diff --git a/src/objects/js/components/admin/permissions/permission-form.js b/src/objects/js/components/admin/permissions/permission-form.js index 8d064fbd..a1d7d1c8 100644 --- a/src/objects/js/components/admin/permissions/permission-form.js +++ b/src/objects/js/components/admin/permissions/permission-form.js @@ -22,17 +22,19 @@ const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoi }) .then(response => response.json()) .then(response_data => { - const objecttypes = { - [objecttype_id]: response_data.reduce((acc, version) => { - const properties = Object.keys(version.jsonSchema?.properties || {}); - acc[version.version] = properties.reduce((propsAcc, prop) => { - propsAcc[prop] = `record__data__${prop}`; - return propsAcc; - }, {}); - return acc; - }, {}) - }; + if (response_data?.results?.length > 0) { + const objecttypes = { + [objecttype_id]: response_data.results.reduce((acc, version) => { + const properties = Object.keys(version?.jsonSchema?.properties || {}); + acc[version.version] = properties.reduce((propsAcc, prop) => { + propsAcc[prop] = `record__data__${prop}`; + return propsAcc; + }, {}); + return acc; + }, {}) + }; setDataFieldChoices(objecttypes); + } }) .catch(error => { console.error('An error occurred while fetching the Objecttype versions endpoint:', error); From bec86e7abd4bb2d608b69e876c1c93f70a14b21a Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Mon, 27 Jan 2025 09:33:06 +0100 Subject: [PATCH 11/17] [#464] Use url as internal --- src/objects/core/admin.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index c03c5240..300743b7 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -22,7 +22,7 @@ def get_urls(self): urls = super().get_urls() my_urls = [ path( - "/versions/", + "/_versions/", self.admin_site.admin_view(self.versions_view), ) ] From e1d27a77d299535487bc15989b4dbeef74bfcc96 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Mon, 27 Jan 2025 11:17:15 +0100 Subject: [PATCH 12/17] [#464] Create auth tests --- src/objects/core/admin.py | 1 + src/objects/tests/admin/test_core_views.py | 46 ++++++++++++++++++++++ 2 files changed, 47 insertions(+) create mode 100644 src/objects/tests/admin/test_core_views.py diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index 300743b7..a768d28f 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -24,6 +24,7 @@ def get_urls(self): path( "/_versions/", self.admin_site.admin_view(self.versions_view), + name="objecttype_versions", ) ] return my_urls + urls diff --git a/src/objects/tests/admin/test_core_views.py b/src/objects/tests/admin/test_core_views.py new file mode 100644 index 00000000..4a679752 --- /dev/null +++ b/src/objects/tests/admin/test_core_views.py @@ -0,0 +1,46 @@ +from django.urls import reverse_lazy + +from django_webtest import WebTest +import requests_mock +from django.test import TestCase, override_settings +from maykin_2fa.test import disable_admin_mfa +from requests.exceptions import ConnectionError + +from objects.accounts.tests.factories import UserFactory +from objects.token.tests.factories import ObjectTypeFactory, TokenAuthFactory +from django.urls import reverse +from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get + + +@disable_admin_mfa() +class ObjectTypeAdminVersionsTests(WebTest): + + def test_authentication_view(self): + url = reverse("admin:objecttype_versions", args=[1]) + response = self.client.get(url) + redirect_url = f"{reverse('admin:login')}?next={url}" + self.assertRedirects(response, redirect_url, status_code=302) + + def test_permission_view(self): + user = UserFactory.create(is_staff=False, is_superuser=False) + url = reverse("admin:objecttype_versions", args=[1]) + response = self.app.get(url, user=user, auto_follow=True) + self.assertContains( + response, + f"You are authenticated as {user.username}, but are not authorized to access this page", + ) + + user.is_staff = True + user.save() + + url = reverse("admin:objecttype_versions", args=[1]) + response = self.app.get(url, user=user) + self.assertEqual( + response.json, + { + "count": 0, + "next": None, + "previous": None, + "results": [], + }, + ) From 2577daeb1d57830c74f8f7b66ea321b0c92ce990 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Mon, 27 Jan 2025 13:48:57 +0100 Subject: [PATCH 13/17] [#464] Use pagination_helper --- src/objects/core/admin.py | 18 ++++++++++-------- src/objects/tests/admin/test_core_views.py | 18 ++---------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/objects/core/admin.py b/src/objects/core/admin.py index a768d28f..f60a23dc 100644 --- a/src/objects/core/admin.py +++ b/src/objects/core/admin.py @@ -1,3 +1,5 @@ +import logging + from django.contrib import admin from django.contrib.gis import forms from django.contrib.gis.db.models import GeometryField @@ -6,9 +8,12 @@ import requests from zgw_consumers.client import build_client +from zgw_consumers.service import pagination_helper from .models import Object, ObjectRecord, ObjectType +logger = logging.getLogger(__name__) + @admin.register(ObjectType) class ObjectTypeAdmin(admin.ModelAdmin): @@ -30,19 +35,16 @@ def get_urls(self): return my_urls + urls def versions_view(self, request, objecttype_id): - versions = { - "count": 0, - "next": None, - "previous": None, - "results": [], - } + versions = [] if objecttype := self.get_object(request, objecttype_id): client = build_client(objecttype.service) try: response = client.get(objecttype.versions_url) - versions = response.json() + versions = list(pagination_helper(client, response.json())) except (requests.RequestException, requests.JSONDecodeError): - pass + logger.exception( + "Something went wrong while fetching objecttype versions" + ) return JsonResponse(versions, safe=False) diff --git a/src/objects/tests/admin/test_core_views.py b/src/objects/tests/admin/test_core_views.py index 4a679752..5c3c074a 100644 --- a/src/objects/tests/admin/test_core_views.py +++ b/src/objects/tests/admin/test_core_views.py @@ -1,15 +1,9 @@ -from django.urls import reverse_lazy +from django.urls import reverse from django_webtest import WebTest -import requests_mock -from django.test import TestCase, override_settings from maykin_2fa.test import disable_admin_mfa -from requests.exceptions import ConnectionError from objects.accounts.tests.factories import UserFactory -from objects.token.tests.factories import ObjectTypeFactory, TokenAuthFactory -from django.urls import reverse -from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get @disable_admin_mfa() @@ -35,12 +29,4 @@ def test_permission_view(self): url = reverse("admin:objecttype_versions", args=[1]) response = self.app.get(url, user=user) - self.assertEqual( - response.json, - { - "count": 0, - "next": None, - "previous": None, - "results": [], - }, - ) + self.assertEqual(response.json, []) From bf92e2ad6a5997a4eaa13cf1f2d62c77ac533071 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Mon, 27 Jan 2025 14:16:01 +0100 Subject: [PATCH 14/17] [#464] Improve js --- .../components/admin/permissions/permission-form.js | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/objects/js/components/admin/permissions/permission-form.js b/src/objects/js/components/admin/permissions/permission-form.js index a1d7d1c8..1c2b6b04 100644 --- a/src/objects/js/components/admin/permissions/permission-form.js +++ b/src/objects/js/components/admin/permissions/permission-form.js @@ -13,18 +13,18 @@ const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoi values["fields"] = "{}" } - const [fields, setFields] = useState( JSON.parse(values["fields"]) || {}) + const [fields, setFields] = useState( JSON.parse(values["fields"]) || {} ) const [dataFieldChoices, setDataFieldChoices] = useState(dataFieldChoices); const fetchObjecttypeVersions = (objecttype_id) => { - fetch(`/admin/core/objecttype/`+ objecttype_id +`/versions/`, { + fetch(`/admin/core/objecttype/${objecttype_id}/_versions/`, { method: 'GET', }) .then(response => response.json()) .then(response_data => { - if (response_data?.results?.length > 0) { + if (response_data?.length > 0) { const objecttypes = { - [objecttype_id]: response_data.results.reduce((acc, version) => { + [objecttype_id]: response_data.reduce((acc, version) => { const properties = Object.keys(version?.jsonSchema?.properties || {}); acc[version.version] = properties.reduce((propsAcc, prop) => { propsAcc[prop] = `record__data__${prop}`; @@ -69,7 +69,6 @@ const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoi errors={errors["object_type"]} onChange={(value) => { setObjectType(value); - fetchObjecttypeVersions(value); setFields({}); }} /> @@ -98,7 +97,7 @@ const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoi name="use_fields" id="id_use_fields" label="Use field-based authorization" - disabled={!mode || mode === "read_and_write"} + disabled={!mode || mode === "read_and_write" || Object.keys(dataFieldChoices || {}).length === 0} value={useFields} onChange={(value) => {setUseFields(value)}} /> From 50be3f2a5eec98e474c9d60516c59a315b7749c1 Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 30 Jan 2025 10:25:40 +0100 Subject: [PATCH 15/17] [#464] Create new tests --- src/objects/tests/admin/test_core_views.py | 55 ++++++++++++++++++---- 1 file changed, 46 insertions(+), 9 deletions(-) diff --git a/src/objects/tests/admin/test_core_views.py b/src/objects/tests/admin/test_core_views.py index 5c3c074a..cdc85a1f 100644 --- a/src/objects/tests/admin/test_core_views.py +++ b/src/objects/tests/admin/test_core_views.py @@ -1,21 +1,65 @@ from django.urls import reverse +import requests_mock from django_webtest import WebTest from maykin_2fa.test import disable_admin_mfa from objects.accounts.tests.factories import UserFactory +from objects.token.tests.factories import ObjectTypeFactory + +from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get @disable_admin_mfa() +@requests_mock.Mocker() class ObjectTypeAdminVersionsTests(WebTest): - def test_authentication_view(self): + def test_valid_response_view(self, m): + objecttypes_api = "https://example.com/objecttypes/v1/" + object_type = ObjectTypeFactory.create(service__api_root=objecttypes_api) + mock_service_oas_get(m, objecttypes_api, "objecttypes") + m.get(f"{objecttypes_api}objecttypes", json=[]) + m.get(object_type.url, json=mock_objecttype(object_type.url)) + version = mock_objecttype_version(object_type.url, attrs={"jsonSchema": {}}) + m.get( + f"{object_type.url}/versions", + json={ + "count": 1, + "next": None, + "previous": None, + "results": [version], + }, + ) + + user = UserFactory.create(is_staff=True, is_superuser=True) + + # object_type exist + url = reverse("admin:objecttype_versions", args=[object_type.pk]) + response = self.app.get(url, user=user) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json), 1) + + # object_type does not exist + url = reverse("admin:objecttype_versions", args=[100]) + response = self.app.get(url, user=user) + self.assertEqual(response.status_code, 200) + self.assertEqual(len(response.json), 0) + + def test_endpoint_unreachable(self, m): + user = UserFactory.create(is_staff=True, is_superuser=True) + object_type = ObjectTypeFactory.create() + url = reverse("admin:objecttype_versions", args=[object_type.pk]) + + with self.assertRaises(requests_mock.exceptions.NoMockAddress): + self.app.get(url, user=user) + + def test_invalid_authentication_view(self, m): url = reverse("admin:objecttype_versions", args=[1]) response = self.client.get(url) redirect_url = f"{reverse('admin:login')}?next={url}" self.assertRedirects(response, redirect_url, status_code=302) - def test_permission_view(self): + def test_invalid_permission_view(self, m): user = UserFactory.create(is_staff=False, is_superuser=False) url = reverse("admin:objecttype_versions", args=[1]) response = self.app.get(url, user=user, auto_follow=True) @@ -23,10 +67,3 @@ def test_permission_view(self): response, f"You are authenticated as {user.username}, but are not authorized to access this page", ) - - user.is_staff = True - user.save() - - url = reverse("admin:objecttype_versions", args=[1]) - response = self.app.get(url, user=user) - self.assertEqual(response.json, []) From f24a82e0522bb350bdc75ee342d557c0ac1f33bb Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 30 Jan 2025 10:30:26 +0100 Subject: [PATCH 16/17] [#464] Change initial state --- src/objects/js/components/admin/permissions/permission-form.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/objects/js/components/admin/permissions/permission-form.js b/src/objects/js/components/admin/permissions/permission-form.js index 1c2b6b04..7c02bdc2 100644 --- a/src/objects/js/components/admin/permissions/permission-form.js +++ b/src/objects/js/components/admin/permissions/permission-form.js @@ -14,7 +14,7 @@ const PermissionForm = ({objectFields, tokenChoices, objecttypeChoices, modeChoi } const [fields, setFields] = useState( JSON.parse(values["fields"]) || {} ) - const [dataFieldChoices, setDataFieldChoices] = useState(dataFieldChoices); + const [dataFieldChoices, setDataFieldChoices] = useState({}); const fetchObjecttypeVersions = (objecttype_id) => { fetch(`/admin/core/objecttype/${objecttype_id}/_versions/`, { From 8abb3a0f8e38b971643d62165d06ff52bc12301e Mon Sep 17 00:00:00 2001 From: Daniel Mursa Date: Thu, 30 Jan 2025 14:15:14 +0100 Subject: [PATCH 17/17] [#464] Changes in tests --- src/objects/tests/admin/test_core_views.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/objects/tests/admin/test_core_views.py b/src/objects/tests/admin/test_core_views.py index cdc85a1f..dd9b51c7 100644 --- a/src/objects/tests/admin/test_core_views.py +++ b/src/objects/tests/admin/test_core_views.py @@ -3,6 +3,7 @@ import requests_mock from django_webtest import WebTest from maykin_2fa.test import disable_admin_mfa +from requests.exceptions import HTTPError from objects.accounts.tests.factories import UserFactory from objects.token.tests.factories import ObjectTypeFactory @@ -22,7 +23,7 @@ def test_valid_response_view(self, m): m.get(object_type.url, json=mock_objecttype(object_type.url)) version = mock_objecttype_version(object_type.url, attrs={"jsonSchema": {}}) m.get( - f"{object_type.url}/versions", + object_type.versions_url, json={ "count": 1, "next": None, @@ -40,18 +41,20 @@ def test_valid_response_view(self, m): self.assertEqual(len(response.json), 1) # object_type does not exist - url = reverse("admin:objecttype_versions", args=[100]) + url = reverse("admin:objecttype_versions", args=[object_type.pk + 1]) response = self.app.get(url, user=user) self.assertEqual(response.status_code, 200) - self.assertEqual(len(response.json), 0) + self.assertEqual(response.json, []) def test_endpoint_unreachable(self, m): user = UserFactory.create(is_staff=True, is_superuser=True) object_type = ObjectTypeFactory.create() - url = reverse("admin:objecttype_versions", args=[object_type.pk]) + m.get(object_type.versions_url, exc=HTTPError) - with self.assertRaises(requests_mock.exceptions.NoMockAddress): - self.app.get(url, user=user) + url = reverse("admin:objecttype_versions", args=[object_type.pk]) + response = self.app.get(url, user=user) + self.assertEqual(response.status_code, 200) + self.assertEqual(response.json, []) def test_invalid_authentication_view(self, m): url = reverse("admin:objecttype_versions", args=[1])