Skip to content

Commit

Permalink
Feature/464 permission admin view slow (#508)
Browse files Browse the repository at this point in the history
* [#464] Limited data_choices in admin view

* [#464] Fix js

* [#464] Fix tests

* [#464] Add new control message

* Revert "[#464] Add new control message"

This reverts commit 59cca54.

* [#464] Fix tests

* [#464] Create endpoint objecttype versions_view + js call

* [#464] Fix tests

* [#464] Formatting code

* [#464] Improve JS

* [#464] Use url as internal

* [#464] Create auth tests

* [#464] Use pagination_helper

* [#464] Improve js

* [#464] Create new tests

* [#464] Change initial state

* [#464] Changes in tests
  • Loading branch information
danielmursa-dev authored Jan 30, 2025
1 parent dc7c0ee commit 014cfe2
Show file tree
Hide file tree
Showing 8 changed files with 159 additions and 62 deletions.
34 changes: 34 additions & 0 deletions src/objects/core/admin.py
Original file line number Diff line number Diff line change
@@ -1,9 +1,19 @@
import logging

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 zgw_consumers.service import pagination_helper

from .models import Object, ObjectRecord, ObjectType

logger = logging.getLogger(__name__)


@admin.register(ObjectType)
class ObjectTypeAdmin(admin.ModelAdmin):
Expand All @@ -13,6 +23,30 @@ class ObjectTypeAdmin(admin.ModelAdmin):
)
readonly_fields = ("_name",)

def get_urls(self):
urls = super().get_urls()
my_urls = [
path(
"<int:objecttype_id>/_versions/",
self.admin_site.admin_view(self.versions_view),
name="objecttype_versions",
)
]
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)
versions = list(pagination_helper(client, response.json()))
except (requests.RequestException, requests.JSONDecodeError):
logger.exception(
"Something went wrong while fetching objecttype versions"
)
return JsonResponse(versions, safe=False)


class ObjectRecordInline(admin.TabularInline):
model = ObjectRecord
Expand Down
4 changes: 4 additions & 0 deletions src/objects/core/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
1 change: 0 additions & 1 deletion src/objects/js/components/admin/permissions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const mount = () => {
ReactDOM.render(
<PermissionForm
objectFields={jsonScriptToVar('object-fields')}
dataFieldChoices={jsonScriptToVar('data-field-choices')}
tokenChoices={jsonScriptToVar('token-auth-choices')}
objecttypeChoices={jsonScriptToVar('object-type-choices')}
modeChoices={jsonScriptToVar('mode-choices')}
Expand Down
48 changes: 41 additions & 7 deletions src/objects/js/components/admin/permissions/permission-form.js
Original file line number Diff line number Diff line change
@@ -1,17 +1,51 @@
import React, { useState } from "react";
import React, { useEffect, useState } from "react";

import { CheckboxInput, TextInput, SelectInput } from "../../forms/inputs";
import { versionAuthFields } from "./auth-fields";


const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttypeChoices, modeChoices, formData}) => {
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"]);

const [fields, setFields] = useState( JSON.parse(values["fields"]) || {});

if (!values["fields"]) {
values["fields"] = "{}"
}

const [fields, setFields] = useState( JSON.parse(values["fields"]) || {} )
const [dataFieldChoices, setDataFieldChoices] = useState({});

const fetchObjecttypeVersions = (objecttype_id) => {
fetch(`/admin/core/objecttype/${objecttype_id}/_versions/`, {
method: 'GET',
})
.then(response => response.json())
.then(response_data => {
if (response_data?.length > 0) {
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 (
<fieldset className="module aligned">
<div className="form-row">
Expand Down Expand Up @@ -63,7 +97,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" || Object.keys(dataFieldChoices || {}).length === 0}
value={useFields}
onChange={(value) => {setUseFields(value)}}
/>
Expand All @@ -76,7 +110,7 @@ const PermissionForm = ({objectFields, dataFieldChoices, tokenChoices, objecttyp
value={useFields ? JSON.stringify(fields) : ""}
/>

{ useFields ?
{ useFields && dataFieldChoices && objectType in dataFieldChoices ?

<div className="form-row">
<label htmlFor="id_selected_fields">Fields:</label>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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" }}
Expand Down
72 changes: 72 additions & 0 deletions src/objects/tests/admin/test_core_views.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,72 @@
from django.urls import reverse

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

from ..utils import mock_objecttype, mock_objecttype_version, mock_service_oas_get


@disable_admin_mfa()
@requests_mock.Mocker()
class ObjectTypeAdminVersionsTests(WebTest):

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(
object_type.versions_url,
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=[object_type.pk + 1])
response = self.app.get(url, user=user)
self.assertEqual(response.status_code, 200)
self.assertEqual(response.json, [])

def test_endpoint_unreachable(self, m):
user = UserFactory.create(is_staff=True, is_superuser=True)
object_type = ObjectTypeFactory.create()
m.get(object_type.versions_url, exc=HTTPError)

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])
response = self.client.get(url)
redirect_url = f"{reverse('admin:login')}?next={url}"
self.assertRedirects(response, redirect_url, status_code=302)

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)
self.assertContains(
response,
f"You are authenticated as {user.username}, but are not authorized to access this page",
)
19 changes: 7 additions & 12 deletions src/objects/tests/admin/test_token_permissions.py
Original file line number Diff line number Diff line change
Expand Up @@ -37,18 +37,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"],
{
object_type.id: {
1: {},
2: {
"diameter": "record__data__diameter",
"plantDate": "record__data__plantDate",
},
}
},
)

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):
"""
Expand Down
42 changes: 1 addition & 41 deletions src/objects/token/admin.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -36,37 +33,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):
data_fields = {}
for object_type in ObjectType.objects.all():
client = build_client(object_type.service)
url = f"{object_type.url}/versions"

try:
response = client.get(url)
except requests.RequestException:
continue

try:
response_data = response.json()
except requests.JSONDecodeError:
continue

# 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
}

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)
Expand Down Expand Up @@ -98,19 +64,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 = (
self.get_data_field_choices() if objecttypes_available else {}
)

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):
Expand Down

0 comments on commit 014cfe2

Please sign in to comment.