Closes: #31 - Polymorphic object fields

[bctiemann]

Closes: #31

This PR addresses a major feature addition: polymorphic object/multi-object fields (using a GenericForeignKey pattern). This means that the related object type for these fields can be any of a selected list, rather than just a single related object type.

Editing a custom object with such a field shows an individual field in the edit form for each object type supported by the object/multiobject field, allowing the selection lists to work with existing object-type-based selection lists:

Unit tests have been added covering all UI and API routes, including validating that only the selected related object types can be assigned to the field.

NOTE:

        # KNOWN LIMITATION: polymorphic fields (is_polymorphic=True) store their
        # allowed target types on the related_object_types M2M, not on the
        # related_object_type FK.  This DFS therefore does not traverse edges
        # introduced by polymorphic fields.  A cycle that passes entirely through
        # polymorphic legs (e.g. A →(poly) B →(poly) A) will go undetected.
        #
        # Fixing this requires also iterating field.related_object_types.filter(
        # app_label=APP_LABEL) and recursing into each.  The check_polymorphic_recursion
        # signal already guards the direct A→B assignment, but cannot see multi-hop
        # cycles that depend on polymorphic fields already on intermediate types.

[bctiemann]

@claude /review

[bctiemann]

@claude /review

[bctiemann]

@claude /review

[bctiemann]

@claude /review

[bctiemann]

@claude /review

[bctiemann]

@claude /review

[arthanson]

Changes per mark - need to list the type of field next to each item on the detail view.

[bctiemann]

@claude /review all changes since the last review

[claude]

Claude finished @bctiemann's task in 2m 14s —— View job

---

Code Review: Changes Since arthanson's Last Approval (2026-05-04)

• Gather git log to identify changes since last review
• Review changed files
• Compile final review

---

Overview

10 commits landed after arthanson's approval, spanning four functional areas:

Commit	Area
381ea32	Serializer bug fix — duplicate create() stub removal
b546660	on_delete_behavior correctness for polymorphic GFK fields
f47780c	Test infrastructure — CI isolation for --keepdb runs
5898cc7	Schema pipeline — full polymorphic export/diff/apply support
ef568f7	Ruff lint fixes
746cbb0	Filterset — per-type filters for polymorphic fields
1595ed2	Filterset — inherit from Filter not ModelChoiceFilter
233eb77	Filterset — use IntegerField to prevent ModelChoiceField validation bypass

---

Bugs / Issues Found

1. _apply_field_alter does not skip related_object_types immutability guard — executor.py:348-350

The executor calls field.save() to apply pending_m2m["related_object_types"], which triggers CustomObjectTypeField.clean(). The model-level clean calls check_polymorphic_recursion. However, unlike _apply_field_add (which is a fresh save), _apply_field_alter sets related_object_types after field.save(). If the model's clean() or signal guard rejects the M2M change, the field row is left in a half-altered state (scalar attrs saved, M2M not updated). This is a pre-existing edge case from the main PR, and the executor's _apply_field_alter approach actually handles it as well as could be expected without wrapping in atomic(). Low severity, worth a comment.

2. _apply_field_add in executor.py is not atomic — executor.py:297-304

field = CustomObjectTypeField(**kwargs)
field.save()
# Wire up M2M related_object_types for polymorphic fields (must be after save).
if fc.schema_def.get("is_polymorphic"):
    rot_list = fc.schema_def.get("related_object_types", [])
    if rot_list:
        field.related_object_types.set(_resolve_related_object_types(rot_list))

If _resolve_related_object_types() or field.related_object_types.set() raises (unknown type, signal rejection), the field row exists in DB with is_polymorphic=True but zero related_object_types. This duplicates the pre-existing concern flagged by arthanson. Given the immutability guards on related_object_types, this is a data-integrity risk. Consider wrapping in transaction.atomic() and deleting the field on exception. Fix this →

3. PolymorphicMultiObjectFilter.__init__ does not call kwargs.setdefault("required", False) — filtersets.py:102-105

PolymorphicObjectFilter correctly calls kwargs.setdefault("required", False) at line 76. PolymorphicMultiObjectFilter does not, so if a caller doesn't pass required=False explicitly the field inherits the django_filters.Filter default (required=False is actually the default, so this is low-risk, but it's inconsistent and could bite on a NetBox FilterSet subclass override). Minor.

---

Code Quality — Positives

Filterset design (746cbb0, 1595ed2, 233eb77) is well-reasoned:

The three-commit progression from ModelChoiceFilter → Filter → IntegerField as the base shows good iteration. The final design is correct: baking content_type_id at construction time and doing two-column filtering ({name}_content_type_id AND {name}_object_id) is the right approach for GFK filtering. The explanatory docstring in PolymorphicObjectFilter about why IntegerField is used over ModelChoiceField is excellent.

_IntegerListField (filtersets.py:24-48):
Clean implementation. The validate() override explicitly allowing empty lists prevents false positive required-field errors on inactive filters.

Schema pipeline (5898cc7) is thorough:

• if/then/else in JSON Schema correctly encodes the branching requirement (polymorphic → related_object_types required; non-polymorphic → related_object_type required).
• _build_dep_order correctly walks both the singular related_object_type string and the list related_object_types — dependency ordering for polymorphic cross-COT references is properly handled.
• pending_m2m pattern in _apply_field_alter cleanly separates save-time vs post-save operations.
• Test coverage is comprehensive: export round-trips, comparator change detection, executor ADD/ALTER, dep ordering.

b546660 correctness:
Adding is_polymorphic=False to _ensure_all_fk_constraints is the right fix — previously iterating over polymorphic fields and silently no-op'ing on them was wasteful and misleading. The on_delete_behavior normalization to SET_NULL for polymorphic fields with the explanatory comment is exactly the right approach.

Test isolation fix (f47780c):
Replacing transaction.savepoint() / savepoint_rollback() with transaction.atomic() in the stale-table test is the correct fix. The comment in test_models.py explaining why raw savepoint rollback failed (validate_no_broken_transaction() while needs_rollback=True) is exemplary.

Serializer cleanup (381ea32):
Removing the trivial create() stub that shadowed the real atomic create() below it is a correct and important fix.

---

Minor

exporter.py:133 — related_object_types.all() in _export_field:

export_cot() already calls prefetch_related("related_object_types") on the field queryset (line 197), so calling .all() inside _export_field hits the prefetch cache rather than the DB. This is correct. No issue.

PolymorphicMultiObjectFilter.filter() deferred imports (filtersets.py:110-111):

The from django.apps import apps as django_apps and from netbox_custom_objects.constants import APP_LABEL are inside filter() to avoid circular imports. This is the established pattern in the codebase and is fine, but consider moving them to module level if the circular import concern is resolved.

---

Summary

The post-approval commits represent high-quality, well-tested completion work. The filterset support is correctly implemented and addresses the remaining open feature gap. The schema pipeline integration is thorough. The main actionable item is wrapping the executor _apply_field_add M2M assignment in transaction.atomic() to avoid orphaned polymorphic fields on failure.

[bctiemann]

Issues 2 and 3 have been addressed. The rest are low-impact and left as-is.

[arthanson]

If you create a poly field, link it to an object then go to that object it doesn't show that it is linked.

Note I have Butler Communications linked in this custom object:

But Butler Communications is showing now custom objects linked: