From be21dbfd821448ca183f1f5812e0f709dbb592ac Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 27 Aug 2026 15:29:21 -0400 Subject: [PATCH 1/2] Closes #655: Surface context fields as attributes in global search results Credit to Ben White (@biwhite), who identified this gap and opened #656 with the core insight this builds on: setting SearchIndex.display_attrs from a COT's context=True fields, so they surface as supplementary "attributes" on global search results the same way every other NetBox model's SearchIndex does. Two issues in the original approach, found while reviewing #656 for correctness: - It added a get_display_attrs classmethod to the dynamically generated SearchIndex class, but nothing in NetBox core or this plugin ever calls a method by that name. The actual renderer is CachedValue.display_attrs (netbox core's extras/models/search.py), which only reads the plain display_attrs tuple and does its own getattr()/get__display() dispatch per entry -- the added method was dead code. - Nothing prevents context=True from being set on a multiobject (real M2M) field. CachedValue.display_attrs's getattr()-based renderer has no per-type dispatch: for a real ManyToManyField that returns the RelatedManager instance itself, not its contents, so a multiobject context field would leak a broken object repr into search results instead of the related objects. Excluded multiobject context fields from display_attrs to avoid this. Polymorphic and coordinates fields need no equivalent exclusion -- they have no real backing column under the field's own name, so the existing `present` check (guarding against stub models generated with skip_object_fields=True) already filters them out. Also consolidated the two separate self.fields queries (one filtered by search_weight, one by context) into a single pass. Ben's own PR (#656) has this exact same root idea but is out of sync with main and failing CI for that reason, not because of a problem with his approach; opening this instead since I don't have push access to his fork to rebase it directly. Adds two regression tests: a context field appears in display_attrs, and a multiobject context field does not. Full plugin suite verified in a clean venv: 1173 tests, 0 failures/errors, 2 skipped. --- netbox_custom_objects/models.py | 19 +++++++-- netbox_custom_objects/tests/test_models.py | 45 ++++++++++++++++++++++ 2 files changed, 61 insertions(+), 3 deletions(-) diff --git a/netbox_custom_objects/models.py b/netbox_custom_objects/models.py index cbe4cd09..3328c746 100644 --- a/netbox_custom_objects/models.py +++ b/netbox_custom_objects/models.py @@ -1639,15 +1639,28 @@ def register_custom_object_search_index(self, model): | {f.name for f in model._meta.local_many_to_many} ) fields = [] - for field in self.fields.filter(search_weight__gt=0): + display_attrs = [] + for field in self.fields.all(): if field.name not in present: continue - fields.append((field.name, field.search_weight)) + if field.search_weight > 0: + fields.append((field.name, field.search_weight)) + # Context fields surface as supplementary "attributes" on global search + # results (issue #655), via the same generic CachedValue.display_attrs + # mechanism every other NetBox model's SearchIndex uses. That mechanism + # renders a field with plain getattr() (falling back to + # get__display() for Django choices=), so it has no way to render + # a MultiObject field's RelatedManager as anything meaningful -- exclude + # those. Polymorphic and coordinates fields need no such exclusion: they + # have no real backing column under the field's own name, so `present` + # already filters them out above. + if field.context and field.type != CustomFieldTypeChoices.TYPE_MULTIOBJECT: + display_attrs.append(field.name) attrs = { "model": model, "fields": tuple(fields), - "display_attrs": tuple(), + "display_attrs": tuple(display_attrs), } search_index = type( f"{self.name}SearchIndex", diff --git a/netbox_custom_objects/tests/test_models.py b/netbox_custom_objects/tests/test_models.py index 9eb6ca52..0d8b86fc 100644 --- a/netbox_custom_objects/tests/test_models.py +++ b/netbox_custom_objects/tests/test_models.py @@ -246,6 +246,51 @@ def test_register_search_index_skips_object_field_absent_from_stub_model(self): # Must not raise FieldDoesNotExist, RecursionError, or any other exception. cot.register_custom_object_search_index(stub_model) + def test_register_search_index_includes_context_fields_in_display_attrs(self): + """Fields marked context=True surface as display_attrs (issue #655), so + NetBox's global search shows them as supplementary "attributes" alongside + each result, the same as any other model's SearchIndex.display_attrs.""" + cot = self.create_custom_object_type(name="ContextSearchTest", slug="context-search-test") + self.create_custom_object_type_field( + cot, name="name", label="Name", type="text", primary=True, search_weight=1000, + ) + self.create_custom_object_type_field( + cot, name="status", label="Status", type="text", context=True, + ) + cot.clear_model_cache(cot.id) + model = cot.get_model() + cot.register_custom_object_search_index(model) + + label = f"{APP_LABEL}.{cot.get_table_model_name(cot.id).lower()}" + search_index = registry["search"][label] + self.assertIn("status", search_index.display_attrs) + + def test_register_search_index_excludes_multiobject_context_fields(self): + """A context field of type multiobject must not reach display_attrs. + + NetBox core's CachedValue.display_attrs (the only consumer of this + attribute) renders each entry via plain getattr() on the instance -- + for a real ManyToManyField that returns the RelatedManager itself, not + its contents, so the search UI would show a broken object repr instead + of the related objects. Excluding multiobject context fields here avoids + surfacing that. + """ + cot = self.create_custom_object_type(name="ContextM2MTest", slug="context-m2m-test") + self.create_custom_object_type_field( + cot, name="name", label="Name", type="text", primary=True, search_weight=1000, + ) + self.create_custom_object_type_field( + cot, name="related_sites", label="Related Sites", type="multiobject", + related_object_type=self.get_site_object_type(), context=True, + ) + cot.clear_model_cache(cot.id) + model = cot.get_model() + cot.register_custom_object_search_index(model) + + label = f"{APP_LABEL}.{cot.get_table_model_name(cot.id).lower()}" + search_index = registry["search"][label] + self.assertNotIn("related_sites", search_index.display_attrs) + def test_skipped_object_field_with_stale_content_type_logs_warning(self): """When get_model_field raises NotImplementedError for an object field whose related_object_type_id is non-null (stale/deleted ContentType), a WARNING must From 96789a31bb05889ec212eb051bb2ac37f1f4ed53 Mon Sep 17 00:00:00 2001 From: Brian Tiemann Date: Thu, 27 Aug 2026 15:51:36 -0400 Subject: [PATCH 2/2] Address automated PR review comment on #684 - Strengthen the positive display_attrs test with assertNotIn("name", ...) so a regression that dumps every field into display_attrs (not just context=True ones) would be caught. - Add test_register_search_index_includes_object_context_fields, the missing other side of the TYPE_MULTIOBJECT exclusion: a TYPE_OBJECT (single FK) context field must still be included, since getattr() on it returns the related instance directly, not a manager. --- netbox_custom_objects/tests/test_models.py | 23 ++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/netbox_custom_objects/tests/test_models.py b/netbox_custom_objects/tests/test_models.py index 0d8b86fc..012f2966 100644 --- a/netbox_custom_objects/tests/test_models.py +++ b/netbox_custom_objects/tests/test_models.py @@ -264,6 +264,29 @@ def test_register_search_index_includes_context_fields_in_display_attrs(self): label = f"{APP_LABEL}.{cot.get_table_model_name(cot.id).lower()}" search_index = registry["search"][label] self.assertIn("status", search_index.display_attrs) + self.assertNotIn("name", search_index.display_attrs) + + def test_register_search_index_includes_object_context_fields(self): + """A context field of type object (a single real FK, unlike multiobject's + M2M) IS included in display_attrs -- getattr() on it returns the related + instance directly, not a manager, so NetBox's generic renderer handles it + correctly. Confirms the TYPE_MULTIOBJECT exclusion is scoped to just that + one type, not object fields generally.""" + cot = self.create_custom_object_type(name="ContextObjectTest", slug="context-object-test") + self.create_custom_object_type_field( + cot, name="name", label="Name", type="text", primary=True, search_weight=1000, + ) + self.create_custom_object_type_field( + cot, name="related_site", label="Related Site", type="object", + related_object_type=self.get_site_object_type(), context=True, + ) + cot.clear_model_cache(cot.id) + model = cot.get_model() + cot.register_custom_object_search_index(model) + + label = f"{APP_LABEL}.{cot.get_table_model_name(cot.id).lower()}" + search_index = registry["search"][label] + self.assertIn("related_site", search_index.display_attrs) def test_register_search_index_excludes_multiobject_context_fields(self): """A context field of type multiobject must not reach display_attrs.