Refactor: Consolidate attachment-panel display resolution to the render-time extension (#112)

CHANGELOG.md

@@ -5,6 +5,12 @@ All notable changes to this project are documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/),
 and this project follows [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## [11.3.1] - 2026-07-28
+
+### Changed
+
+- Consolidated attachment-panel display resolution (issue #112). Side and full-width panels for every in-scope model — standard and custom — are now served by a single render-time template extension; the startup loop registers only `additional_tab` tab views. This removes the duplicated display logic that previously had to be kept in sync across the two paths (the source of the `display_setting` bug fixed in 11.3.0). No user-facing behavior change.
+
 ## [11.3.0] - 2026-07-28
 
 ### Fixed

netbox_attachments/template_content.py

@@ -40,8 +40,8 @@ def resolve_effective_display_preference(
 
 def resolve_display_preference_for_model(model, plugin_settings: dict | None = None) -> str:
     """
-    The one model -> effective display position answer, shared by the startup loop and
-    the render-time panel so the two cannot drift (issue #112).
+    The one model -> effective display position answer, shared by the startup loop
+    (additional_tab) and the render-time panel (all positions).
 
     Custom objects are keyed in display_setting by their type name — the same
     identifier scope_filter uses — but resolving that name costs a query, so it is
@@ -74,17 +74,6 @@ def _render_or_empty(extension, template_name: str, label: str, extra_context: d
         return ""
 
 
-def render_attachment_panel(self) -> str:
-    model_name = self.models[0] if (hasattr(self, "models") and self.models) else getattr(self, "model", None)
-    if model_name is None:
-        logger.error("No model or models attribute found on extension")
-        return ""
-    if "." not in str(model_name):
-        logger.error(f"Invalid model name format: {model_name!r}")
-        return ""
-    return _render_or_empty(self, ATTACHMENT_PANEL_TEMPLATE, str(model_name))
-
-
 def create_add_attachment_button(model_name: str, url_pattern_name: str):
     from netbox.plugins import PluginTemplateExtension
 
@@ -155,28 +144,22 @@ def get_children(self, request, parent):
     return view_name
 
 
-def render_custom_object_panel(extension, position: str) -> str:
+def render_panel(extension, position: str) -> str:
     """
-    Render the attachment panel for `position` if the object in context is an in-scope
-    custom object configured to display there; otherwise return an empty string.
+    Render the attachment panel for `position` when the object in context is an in-scope
+    model whose effective display preference is `position`; otherwise return ''.
 
-    Ordered cheapest-check-first: a global extension is offered every object in NetBox,
-    so non-custom-object pages must exit on a string compare before any settings or
-    database work. The CustomObjectType name lookups further down are cached per
-    generated class (see custom_object_identifier).
+    Serves standard and custom models alike. Checks run cheapest-first: the display
+    resolution bails on a string compare before validate_object_type, the only step
+    that can hit the database.
     """
     obj = extension.context.get("object")
     model = type(obj)
-    # NetBox hands global extensions whatever is in context — None, a non-model, or
-    # even a model class (whose type() is the metaclass). Everything downstream reads
-    # the CLASS's _meta, so that is what must exist; checking obj._meta instead would
-    # let a model class through and crash on ModelBase._meta.
+    # Context may hold None, a non-model, or a model class (whose type() is the
+    # metaclass); everything below reads the class's _meta, so guard that.
     if not hasattr(model, "_meta"):
         return ""
 
-    if not is_custom_object_model(model):
-        return ""
-
     if resolve_display_preference_for_model(model) != position:
         return ""
 
@@ -186,33 +169,29 @@ def render_custom_object_panel(extension, position: str) -> str:
     return _render_or_empty(extension, ATTACHMENT_PANEL_TEMPLATE, model._meta.label_lower)
 
 
-def create_custom_object_attachment_panel():
+def create_attachment_panel():
     """
-    Build the globally registered extension that renders attachment panels on custom
-    object detail pages.
-
-    Custom object models cannot be enumerated at import time: netbox_custom_objects
-    withholds its dynamic models until its own ready() has finished, and plugin ready()
-    order follows PLUGINS, so they are absent whenever this plugin loads first (issue
-    #110). Registering with models = None defers the decision to render time, when the
-    models reliably exist — which also lets object types created after startup work
-    without a NetBox restart.
+    Global extension (models = None) that renders attachment panels at request time.
+
+    Deferring to render time keeps it independent of PLUGINS load order and picks up
+    custom object types created after startup — neither of which a startup-time model
+    enumeration can do.
     """
     from netbox.plugins import PluginTemplateExtension
 
-    class CustomObjectAttachmentPanel(PluginTemplateExtension):
+    class AttachmentPanel(PluginTemplateExtension):
         models = None
 
         def left_page(self):
-            return render_custom_object_panel(self, "left_page")
+            return render_panel(self, "left_page")
 
         def right_page(self):
-            return render_custom_object_panel(self, "right_page")
+            return render_panel(self, "right_page")
 
         def full_width_page(self):
-            return render_custom_object_panel(self, "full_width_page")
+            return render_panel(self, "full_width_page")
 
-    return CustomObjectAttachmentPanel
+    return AttachmentPanel
 
 
 def get_template_extensions() -> List[Type]:
@@ -224,10 +203,9 @@ def get_template_extensions() -> List[Type]:
     except Exception:
         return []
 
-    # Registered up front so custom object support survives any failure below: it
-    # enumerates no models and touches no database. Self-gating, so it is also safe
-    # to register without netbox_custom_objects installed.
-    extensions = [create_custom_object_attachment_panel()]
+    # Registered up front so the render-time panel survives any failure below: it
+    # enumerates no models, touches no database, and self-gates per request.
+    extensions = [create_attachment_panel()]
 
     try:
         plugin_settings = _get_plugin_settings()
@@ -237,8 +215,8 @@ def get_template_extensions() -> List[Type]:
             logger.warning("Invalid create_add_button value, defaulting to True")
             should_add_button = True
 
-        # Custom objects are deliberately not collected here; CustomObjectAttachmentPanel
-        # handles them at render time.
+        # This loop only registers additional_tab tabs, which must exist at startup;
+        # side/full-width panels are served at request time by the render-time panel.
         all_models = list(apps.get_models())
         logger.debug(f"Found {len(all_models)} standard Django models")
 
@@ -257,44 +235,26 @@ def get_template_extensions() -> List[Type]:
             )
 
         for model in unique_models:
-            # When netbox_custom_objects loads first its dynamic models are already in
-            # apps.get_models(); a per-model extension here would render a second panel
-            # alongside the global one, and the additional_tab branch below must never
-            # see them (their detail pages cannot host tabs or top buttons).
-            # Consolidating the two mechanisms is tracked in issue #112.
+            # Custom objects are served entirely at render time and can never use
+            # additional_tab, so skip them here — this also keeps startup free of
+            # custom-object DB access and PLUGINS-order dependence.
             if is_custom_object_model(model):
                 continue
 
             if not validate_object_type(model):
                 continue
 
+            if resolve_display_preference_for_model(model, plugin_settings=plugin_settings) != "additional_tab":
+                continue
+
             app_label = model._meta.app_label
             model_name = model._meta.model_name
             app_model_name = model._meta.label_lower
+            view_name = register_attachment_tab_view(model)
 
-            display_preference = resolve_display_preference_for_model(model, plugin_settings=plugin_settings)
-
-            if display_preference == "additional_tab":
-                view_name = register_attachment_tab_view(model)
-
-                if should_add_button:
-                    url_pattern_name = f"{app_label}:{model_name}_{view_name}"
-                    extensions.append(create_add_attachment_button(app_model_name, url_pattern_name))
-                continue
-
-            from netbox.plugins import PluginTemplateExtension
-
-            extension_name = f"{app_label}_{model_name}_attachment_extension"
-            extension_class = type(
-                extension_name,
-                (PluginTemplateExtension,),
-                {
-                    "models": [app_model_name],
-                    display_preference: render_attachment_panel,
-                },
-            )
-
-            extensions.append(extension_class)
+            if should_add_button:
+                url_pattern_name = f"{app_label}:{model_name}_{view_name}"
+                extensions.append(create_add_attachment_button(app_model_name, url_pattern_name))
 
     except OperationalError:
         logger.error("Database is not ready, skipping template extensions setup")

netbox_attachments/tests/test_template_extensions.py

@@ -11,8 +11,10 @@ def __init__(self, obj, raises=False):
         self.context = {"object": obj}
         self.raises = raises
         self.rendered = []
+        self.render_calls = 0
 
     def render(self, template_name, extra_context=None):
+        self.render_calls += 1
         if self.raises:
             raise RuntimeError("template blew up")
         self.rendered.append(template_name)
@@ -104,59 +106,79 @@ def test_custom_object_panel_renders_at_configured_position(monkeypatch):
     stub_custom_object_support(monkeypatch, settings={"display_default": "full_width_page"})
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == (
+    assert template_content.render_panel(extension, "full_width_page") == (
         "<panel:netbox_attachments/netbox_attachment_panel.html>"
     )
-    assert template_content.render_custom_object_panel(extension, "left_page") == ""
-    assert template_content.render_custom_object_panel(extension, "right_page") == ""
+    assert template_content.render_panel(extension, "left_page") == ""
+    assert template_content.render_panel(extension, "right_page") == ""
 
 
 def test_custom_object_panel_honours_left_page_setting(monkeypatch):
     stub_custom_object_support(monkeypatch, settings={"display_default": "left_page"})
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "left_page") != ""
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "left_page") != ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
 
 
 def test_custom_object_panel_falls_back_from_additional_tab_to_full_width(monkeypatch):
     """Custom object pages cannot host a tab, so additional_tab must land on full_width_page."""
     stub_custom_object_support(monkeypatch, settings={"display_default": "additional_tab"})
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") != ""
-    assert template_content.render_custom_object_panel(extension, "left_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") != ""
+    assert template_content.render_panel(extension, "left_page") == ""
 
 
-def test_custom_object_panel_skips_non_custom_objects(monkeypatch):
-    stub_custom_object_support(monkeypatch, is_custom_object=False)
+def test_panel_serves_standard_in_scope_model(monkeypatch):
+    """The consolidated panel serves standard models too, not only custom objects."""
+    stub_custom_object_support(monkeypatch, is_custom_object=False, settings={"display_default": "full_width_page"})
     extension = FakeExtension(make_object("dcim", "device"))
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") != ""
+
+
+def test_panel_skips_standard_model_out_of_scope(monkeypatch):
+    stub_custom_object_support(
+        monkeypatch, is_custom_object=False, in_scope=False, settings={"display_default": "full_width_page"}
+    )
+    extension = FakeExtension(make_object("dcim", "device"))
+
+    assert template_content.render_panel(extension, "full_width_page") == ""
+    assert extension.rendered == []
+
+
+def test_panel_skips_additional_tab_models(monkeypatch):
+    """additional_tab models get a startup-registered tab, never a render-time panel."""
+    stub_custom_object_support(monkeypatch, is_custom_object=False, settings={"display_default": "additional_tab"})
+    extension = FakeExtension(make_object("dcim", "device"))
+
+    for position in ("left_page", "right_page", "full_width_page"):
+        assert template_content.render_panel(extension, position) == ""
     assert extension.rendered == []
 
 
 def test_custom_object_panel_skips_out_of_scope_custom_objects(monkeypatch):
     stub_custom_object_support(monkeypatch, in_scope=False)
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
 
 
 def test_custom_object_panel_skips_when_no_object_in_context(monkeypatch):
     """PluginTemplateExtension.render reads context['object'], which can be absent or None."""
     stub_custom_object_support(monkeypatch)
     extension = FakeExtension(None)
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
 
 
 def test_custom_object_panel_skips_objects_without_meta(monkeypatch):
     """NetBox offers global extensions whatever is in context; it need not be a model."""
     stub_custom_object_support(monkeypatch)
     extension = FakeExtension("not a model")
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
 
 
 def test_custom_object_panel_declines_model_classes(monkeypatch):
@@ -166,15 +188,16 @@ def test_custom_object_panel_declines_model_classes(monkeypatch):
     model_class = type(make_object())
     extension = FakeExtension(model_class)
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
     assert extension.rendered == []
 
 
 def test_custom_object_panel_swallows_render_errors(monkeypatch):
     stub_custom_object_support(monkeypatch)
     extension = FakeExtension(make_object(), raises=True)
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
+    assert extension.render_calls == 1  # the '' came from the swallowed error, not an earlier gate
 
 
 def test_custom_object_panel_display_setting_uses_the_type_name_key(monkeypatch):
@@ -196,8 +219,8 @@ def test_custom_object_panel_display_setting_uses_the_type_name_key(monkeypatch)
     )
     extension = FakeExtension(make_object(model_name="table134model"))
 
-    assert template_content.render_custom_object_panel(extension, "left_page") != ""
-    assert template_content.render_custom_object_panel(extension, "full_width_page") == ""
+    assert template_content.render_panel(extension, "left_page") != ""
+    assert template_content.render_panel(extension, "full_width_page") == ""
 
 
 def test_custom_object_panel_skips_identifier_lookup_without_display_setting(monkeypatch):
@@ -207,7 +230,7 @@ def test_custom_object_panel_skips_identifier_lookup_without_display_setting(mon
     monkeypatch.setattr(template_content, "custom_object_identifier", lambda model: calls.append(model) or None)
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") != ""
+    assert template_content.render_panel(extension, "full_width_page") != ""
     assert calls == []
 
 
@@ -222,20 +245,20 @@ def test_custom_object_panel_falls_back_when_identifier_unresolvable(monkeypatch
     monkeypatch.setattr(template_content, "custom_object_identifier", lambda model: None)
     extension = FakeExtension(make_object())
 
-    assert template_content.render_custom_object_panel(extension, "full_width_page") != ""
+    assert template_content.render_panel(extension, "full_width_page") != ""
 
 
 def test_custom_object_panel_defers_scope_check_until_position_matches(monkeypatch):
     """
     validate_object_type can query the database, so it must run only for the one hook
-    that will render — not once per hook, and never on non-custom-object pages.
+    that will render — not once per hook.
     """
     calls = []
     stub_custom_object_support(monkeypatch, settings={"display_default": "full_width_page"})
     monkeypatch.setattr(template_content, "validate_object_type", lambda model: calls.append(model) or True)
     extension = FakeExtension(make_object())
 
     for position in ("left_page", "right_page", "full_width_page"):
-        template_content.render_custom_object_panel(extension, position)
+        template_content.render_panel(extension, position)
 
     assert len(calls) == 1

netbox_attachments/version.py

@@ -1 +1 @@
-__version__ = "11.3.0"
+__version__ = "11.3.1"