fix(api): get_form(change=True) for existing objects — detail/update/bulk (#186)

The detail, update (PATCH), and bulk-PATCH views all build the form via
`model_admin.get_form(request, obj=obj)` WITHOUT `change=True`. Django's
own change view always passes `change=not add` (i.e. True for an
existing object — see `ModelAdmin._changeform_view`). A consumer
`get_form` override commonly branches on `change` to return a
change-specific form whose `Meta` omits a *form-only* field (one that
isn't a model field — e.g. an `admin_override` toggle rendered by a
custom form). Called without `change=True`, that override falls through
to `modelform_factory`, which raises
`FieldError: Unknown field(s) (admin_override) ...` → 500.

Observed in the laminr pilot: `GET /admin2/api/v1/package_reviews/
underreviewstatus/<pk>/` 500'd because `UnderReviewStatusAdmin.get_form`
returns its safe `UnderReviewStatusForm` only when `change=True`, else
defers to the default factory which chokes on the form-only
`admin_override` field.

Fix: pass `change=True` at all three existing-object call sites
(detail.py, update.py, bulk.py). create.py is unchanged — it correctly
calls `get_form(obj=None)` with the default `change=False` for the add
path.

Regression test (test_detail.py::test_detail_calls_get_form_with_change_true):
a get_form that raises unless `change=True` for an existing object now
yields a 200; asserts `change=True` was the value seen. 41/41
detail+update+bulk tests pass.

Co-authored-by: Martin Castro Laminrs <mcastro@laminr.ai>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: 3 people

django_admin_react/api/views/bulk.py

@@ -191,7 +191,8 @@ def _apply_one(
 
         return {"pk": pk, "ok": False, "error": _json.loads(body)["error"]}
 
-    form = model_admin.get_form(request, obj=obj)(
+    # change=True — bulk PATCH targets existing rows (see detail.py).
+    form = model_admin.get_form(request, obj=obj, change=True)(
         data=merged_initial_for_update(obj, writable, fields, model),
         files=None,
         instance=obj,

django_admin_react/api/views/detail.py

@@ -193,7 +193,16 @@ def _fields_payload(
 ) -> dict[str, dict[str, Any]]:
     """Build the per-field descriptor mapping (contract §4 ``fields``)."""
     readonly = set(model_admin.get_readonly_fields(request, obj) or ())
-    form = model_admin.get_form(request, obj=obj)(instance=obj)
+    # ``change=True`` — the detail view is always for an EXISTING object,
+    # so we mirror exactly how Django's change view calls ``get_form``
+    # (``ModelAdmin._changeform_view`` passes ``change=not add``). A
+    # consumer ``get_form`` override commonly branches on ``change`` to
+    # return a change-specific form (one whose Meta omits form-only
+    # fields like ``admin_override``). Calling without ``change=True``
+    # makes that override fall through to the default factory, which
+    # then raises ``FieldError`` on the form-only field and 500s the
+    # detail endpoint.
+    form = model_admin.get_form(request, obj=obj, change=True)(instance=obj)
 
     out: dict[str, dict[str, Any]] = {}
     for name in visible_names:

django_admin_react/api/views/update.py

@@ -124,7 +124,11 @@ def patch(
         if rejection is not None:
             return rejection
 
-        form = model_admin.get_form(request, obj=obj)(
+        # change=True — PATCH targets an existing object, so mirror
+        # Django's change view (see detail.py for the rationale; a
+        # consumer get_form override that branches on `change` must hit
+        # its change-form path, not the default factory).
+        form = model_admin.get_form(request, obj=obj, change=True)(
             data=merged_initial_for_update(obj, writable, payload, model),
             files=None,
             instance=obj,

tests/test_detail.py

@@ -18,6 +18,50 @@ def _url(pk: object) -> str:
     return f"/admin-react/api/v1/auth/group/{pk}/"
 
 
+@pytest.mark.django_db
+def test_detail_calls_get_form_with_change_true(superuser_client: Client) -> None:
+    """Regression: the detail view must call ``get_form(..., change=True)``
+    for an existing object — exactly how Django's change view invokes it.
+
+    A consumer ``get_form`` commonly branches on ``change`` and returns a
+    change-specific form whose ``Meta`` omits a *form-only* field (one
+    that isn't a model field, e.g. an ``admin_override`` toggle). If the
+    detail view calls ``get_form`` WITHOUT ``change=True``, that override
+    falls through to ``modelform_factory`` on the form-only field and
+    raises ``FieldError`` → 500. Observed in the laminr pilot on
+    ``package_reviews.UnderReviewStatus``.
+    """
+    from django import forms
+
+    g = Group.objects.create(name="example")
+    seen: dict[str, object] = {}
+    ok_form = forms.modelform_factory(Group, fields=["name"])
+
+    def branching_get_form(self, request, obj=None, change=False, **kwargs):  # noqa: ANN001
+        seen["change"] = change
+        if obj is not None and change:
+            return ok_form
+        # Mirror the consumer's failure mode: the non-change path would
+        # blow up building a form for a form-only field. Raise so the
+        # test fails loudly if the detail view didn't pass change=True.
+        raise AssertionError("get_form must be called with change=True for an existing object")
+
+    # Pin `get_fields` so Django's own get_fields → get_form(change=False)
+    # path is bypassed (laminr's admin sets `fields` via its @sections
+    # decorator, so only our explicit _fields_payload get_form call
+    # fires). This isolates the call path the fix targets.
+    with admin_override(
+        Group,
+        get_fields=lambda self, request, obj=None: ["name"],
+        get_fieldsets=lambda self, request, obj=None: [(None, {"fields": ["name"]})],
+        get_form=branching_get_form,
+    ):
+        response = superuser_client.get(_url(g.pk))
+
+    assert response.status_code == 200, response.content
+    assert seen.get("change") is True
+
+
 # --------------------------------------------------------------------------- #
 # Mandatory 8-row matrix                                                      #
 # --------------------------------------------------------------------------- #