Fixes: #719 - Enforce object/multiobject required at the model layer

[bctiemann]

Closes: #719

Summary

Follow-up to #700/#714, raised during review of that work (h/t @jnovinger): ObjectFieldType/MultiObjectFieldType.get_model_field() hardcoded blank=True unconditionally on their generated model field(s), so required was enforced only at the REST serializer layer for these two types -- every scalar type (fixed in #700) also enforces it at the model layer via full_clean().

• ObjectFieldType: both the direct ForeignKey branch and the LazyForeignKey branch (_make_lazy_cot_fk, shared by the self-referential and cross-COT cases) now pass blank=not field.required. FK fields are validated by Django's clean_fields(), so full_clean() now genuinely rejects an unset required object field.
• MultiObjectFieldType: the CustomManyToManyField also now passes blank=not field.required. Important finding from investigating this: this has no effect on full_clean() -- Django's clean_fields() only iterates _meta.fields, which explicitly excludes M2M fields (confirmed against Django's own Options.fields source). There's no way to validate M2M state before a row exists anyway, since through-rows need an existing pk on both sides. It does matter to the required-toggle pre-flight check in CustomObjectTypeField.clean(), though: that check queries existing data directly via values_list() rather than relying on per-field full_clean() validation, so it now correctly rejects toggling a multiobject field to required while an existing row has no related objects.

Scoped to the plain (non-polymorphic) case only, per the issue: polymorphic object fields (GenericForeignKey) and polymorphic multiobject fields (PolymorphicM2MDescriptor) have no real, directly-queryable backing field to attach blank to, and the existing required-toggle pre-flight check already treats both as unaffected -- no parallel mechanism is designed for them here.

Test plan

• RequiredFieldEnforcementTestCase.test_object_field_blank_matches_required / test_multiobject_field_blank_matches_required_but_full_clean_cannot_check_it (new, test_field_types.py) -- confirms blank follows required for both types, and that full_clean() enforces it for object but (correctly, structurally) cannot for multiobject.
• test_models.py: split the old "does not crash" relationship-field test to cover only the polymorphic cases (which remain unaffected); added test_required_toggle_rejected_when_existing_object_row_is_blank / _allowed_when_no_blank_object_rows and the multiobject equivalents, confirming the required-toggle pre-flight check now correctly handles both plain relationship types.
• Empirically verified (via a throwaway script, not committed) that req_sites (M2M) is absent from _meta.fields and present only in _meta.many_to_many, confirming why full_clean() structurally cannot check it.
• Broader regression pass (test_models, test_field_types, test_api, test_forms, test_views, test_graphql) -- 7 pre-existing failures observed (6 delete-path tests hitting the local netbox_branching app_label gap, 1 LOGIN_REQUIRED local-config artifact), all confirmed unrelated to this change.
• ruff check passes on all changed files.

[bctiemann]

@claude /review and limit response to actionable issues.

[github-actions]

Claude finished @bctiemann's task in 4m 13s —— View job

---

Review: #720 Enforce object/multiobject required at the model layer

Overall: the blank=not field.required fix in field_types.py is correct and the test additions are well-structured. Two actionable items below.

---

1. Stale comment in models.py (not a changed file — needs a follow-up edit)

models.py:2989-2991 still reads:

# Treat anything without a usable .blank, and GFKs specifically, as
# always blank=True, i.e. not checkable here - matching
# ObjectFieldType/MultiObjectFieldType's own concrete model fields,
# which hardcode blank=True regardless of required.

The last sentence is now wrong. After this PR, plain object/multiobject fields no longer hardcode blank=True — they pass blank=not field.required, meaning they do enter required_columns when required=True. The sentence should be updated to say only polymorphic object/multiobject fields remain excluded from the check.

Fix this →

---

2. M2M values_list semantics may silently skip blank rows

models.py:2997:

existing = model.objects.values_list(*required_columns.keys())

For a plain multiobject field with required=True, required_columns now includes the M2M field. Django's values_list('sites') on an M2M traversal generates an INNER JOIN on the through table — rows with no through-table entries are excluded from the result. The for row in existing.iterator() loop would never execute for a blank row, so ValidationError would not be raised.

If test_required_toggle_rejected_when_existing_multiobject_row_is_blank passes, it either means Django generates a LEFT OUTER JOIN here (please confirm), or the test is passing for an unrelated reason. If it's an INNER JOIN, the fix would need an annotation approach instead, e.g.:

from django.db.models import Count
blank_exists = (
    model.objects
    .annotate(_m2m_count=Count(field_name))
    .filter(_m2m_count=0)
    .exists()
)
if blank_exists:
    raise ValidationError(...)

This is worth verifying before merge, since a silently-passing check here gives false confidence that required M2M is enforced on toggle.

---

Everything else looks correct: the _make_lazy_cot_fk and direct FK branches are consistent, the polymorphic cases are correctly left untouched, and the full_clean() limitation for M2M is clearly documented in both comments and test names.

[bctiemann]

Thanks — pushed f1f15c9 addressing both.

1. Stale comment (models.py:2989-2991): fixed. Reworded to say only the polymorphic object/multiobject cases remain excluded from required_columns, since plain object/multiobject now follow required after this PR.

2. M2M values_list() JOIN type: verified this directly rather than relying on inference. Printed the compiled SQL for model.objects.values_list('sites') against a mix of blank and filled rows:

   SELECT "custom_objects_892_sites"."target_id" AS "sites"
   FROM "custom_objects_892"
   LEFT OUTER JOIN "custom_objects_892_sites"
     ON ("custom_objects_892"."id" = "custom_objects_892_sites"."source_id")
   

   It's a LEFT OUTER JOIN, not an INNER JOIN — a blank row comes back as (None,), not omitted. So test_required_toggle_rejected_when_existing_multiobject_row_is_blank was already passing for the right reason, not a false positive. Added test_required_toggle_rejected_when_one_of_several_multiobject_rows_is_blank as permanent regression coverage (a blank row sitting alongside a filled one, which would slip through undetected if this were ever an INNER JOIN) so this doesn't rely on inference going forward.