🐛 [#177] fix: OBField creation

 - add test that was missing from #178
 - fix OBField creation code and pull it into a function
   the order of get_args return value is not guaranteed:
   https://docs.python.org/3/library/typing.html#typing.get_args

Author: CharString

backend/src/openbeheer/api/views.py

@@ -2,18 +2,15 @@
 
 from abc import ABC
 from collections.abc import Callable
-from functools import partial, reduce
-from operator import or_
+from functools import partial
 from typing import (
     TYPE_CHECKING,
     Iterable,
     Mapping,
     NoReturn,
     Protocol,
     Sequence,
-    get_args,
     get_origin,
-    get_type_hints,
     runtime_checkable,
 )
 from uuid import UUID
@@ -26,7 +23,6 @@
 from furl import furl
 from msgspec import (
     UNSET,
-    Meta,
     Struct,
     ValidationError,
     convert,
@@ -53,9 +49,8 @@
     VersionSummary,
     ZGWError,
     ZGWResponse,
-    as_ob_fieldtype,
-    options,
 )
+from openbeheer.types._open_beheer import ob_fields_of_type
 from openbeheer.utils.decorators import handle_service_errors
 
 if TYPE_CHECKING:
@@ -437,33 +432,8 @@ def parse_ob_fields(
                             Options are inferred from the type annotation of
                             `self.data_type`, but that may be set more general.
         """
-
-        def to_ob_field(name: str, annotation: type) -> OBField:
-            # closure over option_overrides
-            not_applicable = object()
-            args: tuple[type, Meta] = get_args(annotation)
-            t, meta = args if args else (annotation, None)
-
-            ob_field = OBField(
-                name=name,
-                type=as_ob_fieldtype(t, meta),
-                options=option_overrides.get(name, options(annotation)),
-            )
-
-            for filter_name in [name, f"{name}__in"]:
-                if (
-                    value := getattr(params, filter_name, not_applicable)
-                ) is not not_applicable:
-                    ob_field.filter_lookup = filter_name
-                    ob_field.filter_value = value
-                else:
-                    ob_field.options = UNSET
-
-            return ob_field
-
-        attrs = get_type_hints(self.return_data_type, include_extras=True)
         return sorted(
-            (to_ob_field(field, annotation) for field, annotation in attrs.items()),
+            ob_fields_of_type(self.return_data_type, params, option_overrides),
             key=sort_key,
         )
 
@@ -641,32 +611,13 @@ def _expand(self, client, object: T):
         return expand_one(client, self.expansions, object)
 
     def get_fields(self) -> list[OBField]:
-        field_types = reduce(
-            or_,
-            (
-                get_type_hints(obj, include_extras=True)
-                for obj in reversed(self.data_type.mro())
-            ),
-        )
+        ob_fields = ob_fields_of_type(self.data_type)
 
-        fields = []
-        for field, annotation in field_types.items():
-            if field == "_expand":
-                continue
-
-            args: tuple[type, Meta] = get_args(annotation)
-            t, meta = args if args else (annotation, None)
-
-            fields.append(
-                OBField(
-                    name=field,
-                    type=as_ob_fieldtype(t, meta),
-                    options=options(t) or UNSET,
-                    filter_lookup=UNSET,
-                    editable=field not in self.expansions,
-                )
-            )
-        return fields
+        def adapt[F: OBField](field: F) -> F:
+            field.editable = field.name not in self.expansions
+            return field
+
+        return [adapt(f) for f in ob_fields if f.name != "Expand"]
 
     @handle_service_errors
     def get(self, request: Request, slug: str, uuid: UUID, *args, **kwargs) -> Response:

backend/src/openbeheer/types/_open_beheer.py

@@ -6,13 +6,23 @@
 import datetime
 import enum
 from functools import singledispatch
+from itertools import starmap
 from types import NoneType, UnionType
-from typing import Self, Sequence, Type
+from typing import (
+    Annotated,
+    Iterable,
+    Mapping,
+    Self,
+    Sequence,
+    Type,
+    get_args,
+    get_type_hints,
+)
 
 import msgspec
 from ape_pie import APIClient
 from furl import furl
-from msgspec import UNSET, Struct, UnsetType, structs
+from msgspec import UNSET, Meta, Struct, UnsetType, structs
 
 from .ztc import (
     BesluitType,
@@ -97,47 +107,44 @@ class OBFieldType(enum.StrEnum):
     # jsx = enum.auto()
 
 
-def as_ob_fieldtype(t: type | UnionType, meta: msgspec.Meta | None) -> OBFieldType:
+def as_ob_fieldtype(
+    t: type | UnionType | Annotated, meta: Meta | None = None
+) -> OBFieldType:
     "Return the `OBFieldType` for some annotation `t`"
-    if isinstance(t, UnionType):
-        return as_ob_fieldtype(
-            next(ut for ut in t.__args__ if ut not in (NoneType, UnsetType)), meta
-        )
-    if t is bool:
-        return OBFieldType.boolean
-    if t in (int, float):
-        return OBFieldType.number
-    if t is str:
-        # We return either "string" (input) or text" for the field type based on the
-        # "max_length" meta attribute.
-        #
-        # Fields don't have this property set all the time, and it's absence can
-        # possibly indicate no limit. Therefore, the default should be "text"
-        # (textarea).
-        #
-        # Only for fields with a smaller "max_length" set we will use "string" (input).
-        try:
-            if meta and isinstance(meta.max_length, int) and meta.max_length <= 50:
-                # Small fields should get an input
-                return OBFieldType.string
-        except (AttributeError, TypeError):
-            pass
-
-        # Large fields should get a textarea
-        return OBFieldType.text
-
-    if t is datetime.date:
-        return OBFieldType.date
-    return OBFieldType.string
-
-
-def options(t: type | UnionType) -> list[OBOption]:
+
+    args = get_args(t)
+    meta = meta or next((arg for arg in args if isinstance(arg, Meta)), None)
+
+    match (t, meta):
+        case type(), _ if t is bool:
+            return OBFieldType.boolean
+        case type(), _ if t in (int, float):
+            return OBFieldType.number
+        case _, Meta(max_length=int(n)) if str in args and n <= 50:
+            # small strings get an input widget
+            return OBFieldType.string
+        case type(), _ if t is str:
+            # large ones a textarea
+            return OBFieldType.text
+        case type(), _ if t is datetime.date:
+            return OBFieldType.date
+        case _ if args:
+            # unpack Unions, Annotated etc.
+            return as_ob_fieldtype(
+                next(ut for ut in args if ut not in (NoneType, UnsetType)), meta
+            )
+        case _:
+            # fallback to input widget
+            return OBFieldType.string
+
+
+def options(t: type | UnionType | Annotated) -> list[OBOption]:
     "Find an enum in the type and turn it into options."
     match t:
         case enum.EnumType():
             return OBOption.from_enum(t)
-        case UnionType():
-            return [option for ut in t.__args__ for option in options(ut)]
+        case _ if get_args(t):
+            return sum(map(options, get_args(t)), [])
         case _:
             return []
 
@@ -165,10 +172,37 @@ class OBField[T](Struct, rename="camel", omit_defaults=True):
 
     def __post_init__(self):
         # camelize value of name
-        self.name = "".join(
-            part.title() if n else part for n, part in enumerate(self.name.split("_"))
+        self.name = _camelize(self.name)
+
+
+def ob_fields_of_type(
+    data_type: type,
+    query_params: OBPagedQueryParams | None = None,
+    option_overrides: Mapping[str, list[OBOption]] = {},
+) -> Iterable[OBField]:
+    def to_ob_field(name: str, annotation: type) -> OBField:
+        # closure over option_overrides
+        not_applicable = object()
+
+        ob_field = OBField(
+            name=name,
+            type=as_ob_fieldtype(annotation),
+            options=option_overrides.get(name, options(annotation)) or UNSET,
         )
 
+        if query_params:
+            for filter_name in [name, f"{name}__in"]:
+                if (
+                    value := getattr(query_params, filter_name, not_applicable)
+                ) is not not_applicable:
+                    ob_field.filter_lookup = filter_name
+                    ob_field.filter_value = value
+
+        return ob_field
+
+    attrs = get_type_hints(data_type, include_extras=True)
+    return starmap(to_ob_field, attrs.items())
+
 
 class OBList[T](Struct):
     """Used to draw list views on the frontend."""
@@ -329,3 +363,7 @@ class ZaakTypeWithUUID(UUIDMixin, ZaakType):
 
 class ZaakObjectTypeWithUUID(UUIDMixin, ZaakObjectType):
     uuid: str | UnsetType = UNSET
+
+
+def _camelize(s: str) -> str:
+    return "".join(part.title() if n else part for n, part in enumerate(s.split("_")))

backend/src/openbeheer/types/test_ob_types.py

@@ -0,0 +1,99 @@
+from datetime import date
+from enum import Enum
+from typing import Annotated
+from unittest import TestCase
+
+from hypothesis import assume, given, strategies as st  # noqa: F401
+from msgspec import Meta, Struct
+from msgspec.json import decode, encode
+
+from . import ztc
+from ._open_beheer import _camelize, ob_fields_of_type, options
+
+ZTC_DATATYPES = [
+    struct
+    for name in dir(ztc)
+    if (struct := getattr(ztc, name))
+    and isinstance(struct, type)
+    and issubclass(struct, Struct)
+]
+
+ZTC_ENUMS = [
+    struct
+    for name in dir(ztc)
+    if (struct := getattr(ztc, name))
+    and isinstance(struct, type)
+    and issubclass(struct, Enum)
+]
+
+
+@st.composite
+def ztc_struct_instances(draw):
+    struct_type = draw(st.sampled_from(ZTC_DATATYPES))
+    return draw(st.from_type(struct_type))
+
+
+class OBFieldsTest(TestCase):
+    def test_simple_field_type_example(self):
+        class MyStruct(Struct):
+            boolean: bool
+            integer: int
+            ieee_754: float
+            string: Annotated[str, Meta(max_length=49)]
+            text: str
+            date: date
+
+        (b, i, f, s, t, d) = ob_fields_of_type(MyStruct)
+        assert b.name == "boolean"
+        assert b.type == "boolean"
+        assert i.name == "integer"
+        assert i.type == "number"
+        assert f.name == "ieee754"  # camelcased
+        assert f.type == "number"
+        assert s.name == "string"
+        assert s.type == "string"
+        assert t.name == "text"
+        assert t.type == "text"
+        assert d.name == "date"
+        assert d.type == "date"
+
+    @given(ztc_struct_instances())
+    def test_all_fields_are_present(self, instance: Struct):
+        def as_seen_by_frontend(struct) -> dict:
+            """Return struct as the dict that is seen by the frontend
+            This also asserts that all generated OBFields are serializable
+            """
+            return decode(encode(struct), type=dict)
+
+        ob_fields = ob_fields_of_type(type(instance))
+
+        ob_fields_as_dict = map(as_seen_by_frontend, ob_fields)
+        field_names = {field["name"] for field in ob_fields_as_dict}
+
+        instance_dict = as_seen_by_frontend(instance)
+        expected_struct_attributes = set(map(_camelize, instance_dict))
+
+        assert field_names == expected_struct_attributes
+
+
+class OBOptionsTest(TestCase):
+    @given(enum=st.sampled_from(ZTC_ENUMS))
+    def test_length(self, enum):
+        "Every enum member should have an OBOption"
+        ob_options = options(enum)
+        assert len(ob_options) == len(enum)
+
+    @given(enum=st.sampled_from(ZTC_ENUMS))
+    def test_values(self, enum: type[Enum]):
+        "Every OBOption.value should be a valid Enum value"
+        ob_options = options(enum)
+
+        for option in ob_options:
+            assert option.value in enum
+
+    def test_annotated_enum(self):
+        enum_annotation = Annotated[
+            ztc.VertrouwelijkheidaanduidingEnum, Meta(description="blabla")
+        ]
+        ob_options = options(enum_annotation)
+        assert len(ob_options) == len(ztc.VertrouwelijkheidaanduidingEnum)

backend/src/openbeheer/zaaktype/tests/test_zaaktype_detail.py

@@ -341,6 +341,9 @@ def test_retrieve_zaaktype(self):
             self.assertEqual(len(vertrouwelijkheidaanduiding_field["options"]), 8)
             self.assertEqual(fields_by_name["beginGeldigheid"]["type"], "date")
 
+            # has some editable fields
+            assert any(f.get("editable") for f in fields_by_name.values())
+
     def test_patch_zaaktype(self):
         zaaktype = self.helper.create_zaaktype()