diff --git a/CHANGELOG.md b/CHANGELOG.md index 853a2bd..9697f9a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,17 @@ All notable changes to this project will be documented in this file. The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/), and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html). +## [Unreleased] + +### Changed + +- Internal simplification (no behaviour change): shared `_host_q()` for the + badge and view querysets, single error-render path, `field_info` tuples + unpacked directly (always 5-tuples), unreachable import fallbacks and the + redundant field de-dup set removed, bulk-action buttons moved to one + `typed/bulk_buttons.html` include, duplicate `{% csrf_token %}` dropped, + `MANIFEST.in` removed (package-data already ships templates). + ## [3.0.0] - 2026-09-16 ### Removed diff --git a/MANIFEST.in b/MANIFEST.in deleted file mode 100644 index b8e020e..0000000 --- a/MANIFEST.in +++ /dev/null @@ -1 +0,0 @@ -recursive-include netbox_custom_objects_tab/templates *.html diff --git a/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/bulk_buttons.html b/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/bulk_buttons.html new file mode 100644 index 0000000..1ed5973 --- /dev/null +++ b/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/bulk_buttons.html @@ -0,0 +1,20 @@ +{% if can_change or can_delete %} +
+ {% if can_change %} + + {% endif %} + {% if can_delete %} + + {% endif %} +
+{% endif %} diff --git a/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/tab.html b/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/tab.html index 71e59a0..ad191b2 100644 --- a/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/tab.html +++ b/netbox_custom_objects_tab/templates/netbox_custom_objects_tab/typed/tab.html @@ -66,32 +66,12 @@ {% endblocktrans %} - {% if can_change or can_delete %} -
- {% if can_change %} - - {% endif %} - {% if can_delete %} - - {% endif %} -
- {% endif %} + {% include "netbox_custom_objects_tab/typed/bulk_buttons.html" %} {% endif %}
- {% csrf_token %} {# Objects table #}
@@ -129,26 +109,7 @@
{% endif %} {% endif %} - {% if can_change or can_delete %} -
- {% if can_change %} - - {% endif %} - {% if can_delete %} - - {% endif %} -
- {% endif %} + {% include "netbox_custom_objects_tab/typed/bulk_buttons.html" %}
diff --git a/netbox_custom_objects_tab/views/__init__.py b/netbox_custom_objects_tab/views/__init__.py index 6783e7a..d57776b 100644 --- a/netbox_custom_objects_tab/views/__init__.py +++ b/netbox_custom_objects_tab/views/__init__.py @@ -21,19 +21,10 @@ def _resolve_dynamic_custom_object_models(): A restart is required whenever a new Custom Object Type is added. """ - try: - from netbox_custom_objects.models import CustomObject - except ImportError: - logger.warning("netbox_custom_objects plugin not installed — skipping") - return [] - - try: - app_config = apps.get_app_config(_CUSTOM_OBJECTS_APP) - except LookupError: - logger.warning("netbox_custom_objects app not found — skipping") - return [] + from netbox_custom_objects.models import CustomObject # Filter to dynamic CO models only (subclasses of CustomObject, not CustomObject itself). + app_config = apps.get_app_config(_CUSTOM_OBJECTS_APP) return [m for m in app_config.get_models() if issubclass(m, CustomObject) and m is not CustomObject] @@ -110,7 +101,6 @@ def dispatch(request, custom_object_type, pk, **kwargs): raise Http404(f"No '{action_name}' tab registered for {custom_object_type}") return view_cls.as_view()(request, custom_object_type=custom_object_type, pk=pk, **kwargs) - dispatch.__name__ = f"{action_name}_co_dispatch" return dispatch @@ -128,12 +118,9 @@ def _inject_co_urls(): ``plugins:netbox_custom_objects:customobject_{action}`` so each typed tab gets ``customobject_custom_objects_{slug}``. """ - try: - import netbox_custom_objects.urls as co_urls - from django.urls import path as url_path - from netbox.registry import registry - except ImportError: - return + import netbox_custom_objects.urls as co_urls + from django.urls import path as url_path + from netbox.registry import registry # Action names of the typed-tab views we registered on CO dynamic models. action_names = set() diff --git a/netbox_custom_objects_tab/views/typed.py b/netbox_custom_objects_tab/views/typed.py index 2f5597f..47cab2b 100644 --- a/netbox_custom_objects_tab/views/typed.py +++ b/netbox_custom_objects_tab/views/typed.py @@ -44,10 +44,7 @@ def _build_q_for_field(host_ct_id, instance_pk, field_info): last two are only meaningful for polymorphic fields. Returns Q() (an empty no-op filter) if the field can't be resolved, so callers can OR it safely. """ - field_name = field_info[0] - field_type = field_info[1] - is_poly = field_info[3] if len(field_info) >= 5 else False - through_model_name = field_info[4] if len(field_info) >= 5 else None + field_name, field_type, _label, is_poly, through_model_name = field_info if field_type == CustomFieldTypeChoices.TYPE_OBJECT: if is_poly: @@ -168,15 +165,11 @@ def _build_add_links(custom_object_type_slug, host_instance, field_infos, return links = [] seen = set() - for field_info in field_infos: - field_name = field_info[0] + for field_name, field_type, field_label, is_poly, _through in field_infos: if field_name in seen: continue seen.add(field_name) - - field_type = field_info[1] - field_label = (field_info[2] if len(field_info) >= 3 and field_info[2] else field_name) or field_name - is_poly = len(field_info) >= 5 and field_info[3] + field_label = field_label or field_name if not is_poly: prefill = {field_name: host_pk} @@ -198,6 +191,19 @@ def _build_add_links(custom_object_type_slug, host_instance, field_infos, return return links +def _host_q(host_ct_id, instance_pk, field_infos): + """OR of every field's Q for this host; None when no field could be resolved + (an empty Q() would match ALL rows, so callers must not filter on it).""" + q_filter = Q() + has_filter = False + for info in field_infos: + q = _build_q_for_field(host_ct_id, instance_pk, info) + if q.children: + q_filter |= q + has_filter = True + return q_filter if has_filter else None + + def _count_for_type(custom_object_type, field_infos, host_ct_id): """ Return a badge callable for one Custom Object Type. @@ -226,15 +232,8 @@ def _badge(instance): ) return None - q_filter = Q() - has_filter = False - for info in field_infos: - q = _build_q_for_field(host_ct_id, instance.pk, info) - if q.children: - q_filter |= q - has_filter = True - - if not has_filter: + q_filter = _host_q(host_ct_id, instance.pk, field_infos) + if q_filter is None: return None total = dynamic_model.objects.filter(q_filter).distinct().count() @@ -274,45 +273,31 @@ def get(self, request, pk, **kwargs): # Re-fetch CustomObjectType at request time (may have changed since ready()) from netbox_custom_objects.models import CustomObjectType as COTModel - error_context = { - "object": instance, - "tab": self.tab, - "base_template": _get_base_template(instance), - "table": None, - "preferences": {"pagination.placement": "bottom"}, - } - try: - cot = COTModel.objects.get(pk=cot_pk) - except COTModel.DoesNotExist: - return render(request, "netbox_custom_objects_tab/typed/tab.html", error_context) - + cot = COTModel.objects.filter(pk=cot_pk).first() try: - dynamic_model = cot.get_model() + dynamic_model = cot.get_model() if cot else None except Exception: logger.exception("Could not get model for CustomObjectType %s", cot_pk) - return render(request, "netbox_custom_objects_tab/typed/tab.html", error_context) - - # Build base queryset: union of all field filters for this type. - # Polymorphic fields contribute Q(pk__in=) or a - # (content_type_id, object_id) pair, both handled by _build_q_for_field. - # - # An empty Q() is the identity element of `|`, so filter(Q()) returns - # ALL rows. Track has_filter (mirrors _count_for_type) and short-circuit - # to .none() if every _build_q_for_field call returned an empty Q — - # otherwise an unresolvable through model or unknown field type would - # silently widen the tab to every row of the target type. - q_filter = Q() - has_filter = False - for info in field_infos: - q = _build_q_for_field(host_ct_id, instance.pk, info) - if q.children: - q_filter |= q - has_filter = True - - if has_filter: - base_qs = dynamic_model.objects.filter(q_filter).distinct() - else: + dynamic_model = None + if dynamic_model is None: + return render( + request, + "netbox_custom_objects_tab/typed/tab.html", + { + "object": instance, + "tab": self.tab, + "base_template": _get_base_template(instance), + "table": None, + "preferences": {"pagination.placement": "bottom"}, + }, + ) + + # Base queryset: OR of all field filters for this type (see _host_q). + q_filter = _host_q(host_ct_id, instance.pk, field_infos) + if q_filter is None: base_qs = dynamic_model.objects.none() + else: + base_qs = dynamic_model.objects.filter(q_filter).distinct() # Apply filterset filterset_class = get_filterset_class(dynamic_model) @@ -361,10 +346,7 @@ def get(self, request, pk, **kwargs): # Documented in README "Known Issues" and CHANGELOG [2.3.0]. add_links = _build_add_links(cot.slug, instance, field_infos, return_url) if can_add else [] - try: - add_label = cot.get_verbose_name() or str(cot) - except AttributeError: - add_label = str(cot) + add_label = cot.get_verbose_name() or str(cot) context = { "object": instance, @@ -388,7 +370,6 @@ def get(self, request, pk, **kwargs): return render(request, "netbox_custom_objects_tab/typed/tab.html", context) _TypedTabView.__name__ = f"{model_class.__name__}_{custom_object_type.slug}_TypedTabView" - _TypedTabView.__qualname__ = f"{model_class.__name__}_{custom_object_type.slug}_TypedTabView" return _TypedTabView @@ -405,11 +386,7 @@ def register_typed_tabs(model_classes, weight): ] # Non-polymorphic fields: single related_object_type FK. - # is_polymorphic=False keeps this queryset disjoint from poly_fields - # below — a field row with both attrs set (legacy misconfig: - # is_polymorphic is immutable upstream but related_object_type isn't - # nulled when toggled) would otherwise hit both querysets. _record's - # seen_field_keys stays as defence in depth. + # is_polymorphic=False keeps this queryset disjoint from poly_fields below. non_poly_fields = list( CustomObjectTypeField.objects.filter( is_polymorphic=False, @@ -433,15 +410,10 @@ def register_typed_tabs(model_classes, weight): # -> list of (name, type, label, is_polymorphic, through_model_name) ct_cot_fields = defaultdict(list) ct_cot_map = {} # (ct_id, cot_pk) -> CustomObjectType - seen_field_keys = set() # (field.pk, ct_id) — de-dup if a field appears in both querysets def _record(field, ct_id, is_poly): if ct_id is None: return - key_dup = (field.pk, ct_id) - if key_dup in seen_field_keys: - return - seen_field_keys.add(key_dup) key = (ct_id, field.custom_object_type_id) label = getattr(field, "label", "") or field.name through_name = field.through_model_name if is_poly else None diff --git a/pyproject.toml b/pyproject.toml index 163f7c4..2fe5130 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ where = ["."] include = ["netbox_custom_objects_tab*"] [tool.setuptools.package-data] -netbox_custom_objects_tab = ["**/*", "templates/**"] +netbox_custom_objects_tab = ["**/*"] [tool.pytest.ini_options] pythonpath = ["."] diff --git a/tests/test_views_typed_smoke.py b/tests/test_views_typed_smoke.py index 3878b61..5183491 100644 --- a/tests/test_views_typed_smoke.py +++ b/tests/test_views_typed_smoke.py @@ -53,8 +53,8 @@ def test_returns_none_when_zero_total(self): badge = _count_for_type( cot, [ - ("ref_object", CustomFieldTypeChoices.TYPE_OBJECT), - ("ref_multi", CustomFieldTypeChoices.TYPE_MULTIOBJECT), + ("ref_object", CustomFieldTypeChoices.TYPE_OBJECT, "", False, None), + ("ref_multi", CustomFieldTypeChoices.TYPE_MULTIOBJECT, "", False, None), ], host_ct_id=10, ) @@ -76,9 +76,9 @@ def test_uses_single_filter_with_OR_of_field_predicates(self): badge = _count_for_type( cot, [ - ("primary_device", CustomFieldTypeChoices.TYPE_OBJECT), - ("backup_device", CustomFieldTypeChoices.TYPE_OBJECT), - ("affected_devices", CustomFieldTypeChoices.TYPE_MULTIOBJECT), + ("primary_device", CustomFieldTypeChoices.TYPE_OBJECT, "", False, None), + ("backup_device", CustomFieldTypeChoices.TYPE_OBJECT, "", False, None), + ("affected_devices", CustomFieldTypeChoices.TYPE_MULTIOBJECT, "", False, None), ], host_ct_id=10, ) @@ -112,7 +112,9 @@ def test_returns_none_when_get_model_raises(self, caplog): cot = MagicMock() cot.get_model.side_effect = RuntimeError("broken model") cot.pk = 123 - badge = _count_for_type(cot, [("ref_object", CustomFieldTypeChoices.TYPE_OBJECT)], host_ct_id=10) + badge = _count_for_type( + cot, [("ref_object", CustomFieldTypeChoices.TYPE_OBJECT, "", False, None)], host_ct_id=10 + ) instance = MagicMock(pk=42) assert badge(instance) is None @@ -498,7 +500,7 @@ def test_returns_empty_when_reverse_fails(self): links = _build_add_links( "server", self._make_host(42), - [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device")], + [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device", False, None)], "/dcim/devices/42/", ) assert links == [] @@ -513,7 +515,7 @@ def test_single_field_produces_one_link_with_prefill_and_return_url(self): links = _build_add_links( "server", self._make_host(42), - [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device")], + [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device", False, None)], "/dcim/devices/42/custom-objects-server/", ) @@ -537,8 +539,8 @@ def test_multiple_fields_produce_multiple_links(self): "link", self._make_host(7), [ - ("primary_device", CustomFieldTypeChoices.TYPE_OBJECT, "Primary"), - ("backup_device", CustomFieldTypeChoices.TYPE_OBJECT, "Backup"), + ("primary_device", CustomFieldTypeChoices.TYPE_OBJECT, "Primary", False, None), + ("backup_device", CustomFieldTypeChoices.TYPE_OBJECT, "Backup", False, None), ], "/dcim/devices/7/", ) @@ -562,8 +564,8 @@ def test_duplicate_field_names_deduplicated(self): "x", self._make_host(1), [ - ("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device"), - ("device", CustomFieldTypeChoices.TYPE_MULTIOBJECT, "Device"), + ("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device", False, None), + ("device", CustomFieldTypeChoices.TYPE_MULTIOBJECT, "Device", False, None), ], "/dcim/devices/1/", ) @@ -580,27 +582,11 @@ def test_label_falls_back_to_field_name_when_label_blank(self): links = _build_add_links( "x", self._make_host(1), - [("device_ref", CustomFieldTypeChoices.TYPE_OBJECT, "")], + [("device_ref", CustomFieldTypeChoices.TYPE_OBJECT, "", False, None)], "/dcim/devices/1/", ) assert links[0]["label"] == "device_ref" - def test_two_tuple_field_infos_supported_label_defaults_to_name(self): - """Backward-compatible: 2-tuples (no label) work via star unpacking.""" - from netbox_custom_objects_tab.views.typed import _build_add_links - - with patch( - "netbox_custom_objects_tab.views.typed.reverse", - return_value="/plugins/custom-objects/x/add/", - ): - links = _build_add_links( - "x", - self._make_host(1), - [("device", CustomFieldTypeChoices.TYPE_OBJECT)], - "/dcim/devices/1/", - ) - assert links[0]["label"] == "device" - def test_return_url_with_query_string_is_url_encoded(self): """A return_url containing & and ? must be URL-encoded so it doesn't break the outer query string.""" from netbox_custom_objects_tab.views.typed import _build_add_links @@ -612,7 +598,7 @@ def test_return_url_with_query_string_is_url_encoded(self): links = _build_add_links( "x", self._make_host(1), - [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device")], + [("device", CustomFieldTypeChoices.TYPE_OBJECT, "Device", False, None)], "/dcim/devices/1/custom-objects-x/?tag=foo&q=bar", ) url = links[0]["url"]