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

[Kani999]

Closes #112.

What

• The global render-time template extension (AttachmentPanel, models = None) now serves side/full-width panels for all in-scope models — standard and custom.
• The startup loop keeps only additional_tab registration — the one mode that must run while the URLconf is still open (register_model_view()).
• Deletes the per-model panel extension classes and render_attachment_panel. Display logic now lives in one place: render_panel + resolve_display_preference_for_model.
• Renames render_custom_object_panel → render_panel and CustomObjectAttachmentPanel → AttachmentPanel (they serve all models now).

Why

Two parallel mechanisms decided "does this object get a panel, and where?" — the display_setting bug fixed in #110 was a symptom of them drifting apart. One brain removes that whole risk class.

Behavior

No user-facing change — internal consolidation. The custom-object skip in the startup loop is intentionally kept (a small refinement of the issue text): custom objects can never be additional_tab, so skipping loses nothing, and it preserves #110's guarantee of no custom-object DB access / no PLUGINS-order dependence at startup.

Verification

• make test green — 95 passed (+2 standard-model cases); ruff format --check and ruff check clean.
• Live NetBox (with netbox_custom_objects, attachments loaded first):
  • startup builds one global AttachmentPanel(models=None) and zero per-model *_attachment_extension classes;
  • a standard model set to left_page renders via the global panel; additional_tab models get no render-time panel (tab still registered);
  • custom objects unchanged — default full-width + per-type display_setting override;
  • a real custom-object detail page returns 200 with the panel present.
• Version bumped to 11.3.1 with a CHANGELOG note.

Summary by CodeRabbit

• Bug Fixes

  • Fixed attachment panel display settings for supported standard and custom models.
  • Ensured panels consistently appear in the configured location, including side, full-width, and additional-tab views.
  • Improved handling for unsupported objects and rendering errors without disrupting page display.

• Release

  • Updated the release version to 11.3.1.

[coderabbitai]

Important

Review skipped

Auto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ea1dcc17-b020-410f-b135-c946833e9e1b

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

• 🔍 Trigger review

✨ Finishing Touches

✨ Simplify code

• Create PR with simplified code
• Commit simplified code in branch 112-consolidate-display-resolution

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

netbox_attachments/template_content.py (1)

147-169: 🚀 Performance & Scalability | 🟡 Minor | ⚡ Quick win

Cache the custom-object display lookup across render-time hook calls.

custom_object_identifier() has an lru_cache keyed by the model class, but render_panel() also creates a separate PluginTemplateExtension instance per {% plugin_left_page %}, {% plugin_right_page %}, and {% plugin_full_width_page %} call. When display_setting includes any custom-object override, each call runs this lookup again before left_page/right_page/full_width_page return empty. Move the resolved display key/per-model preference into request-scoped state so this global extension computes it once per rendered object.

🤖 Prompt for AI Agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@netbox_attachments/template_content.py` around lines 147 - 169, Cache the
resolved display preference in request-scoped state so render_panel() reuses one
per-model result across all PluginTemplateExtension hook instances for the same
rendered object. Update the path around resolve_display_preference_for_model()
to read or populate that shared state before evaluating position, while
preserving existing validation and empty-return behavior.

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@netbox_attachments/tests/test_template_extensions.py`:
- Around line 193-197: Add a render-call counter or mock to
test_custom_object_panel_swallows_render_errors, associate it with the
FakeExtension used by template_content.render_panel, and assert the render
method was called exactly once alongside the existing empty-string assertion.

---

Outside diff comments:
In `@netbox_attachments/template_content.py`:
- Around line 147-169: Cache the resolved display preference in request-scoped
state so render_panel() reuses one per-model result across all
PluginTemplateExtension hook instances for the same rendered object. Update the
path around resolve_display_preference_for_model() to read or populate that
shared state before evaluating position, while preserving existing validation
and empty-return behavior.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro Plus

Run ID: a95c29c0-0cb7-4b17-959f-10b584ba5960

📥 Commits

Reviewing files that changed from the base of the PR and between 235db53 and 61e196a.

📒 Files selected for processing (4)

• CHANGELOG.md
• netbox_attachments/template_content.py
• netbox_attachments/tests/test_template_extensions.py
• netbox_attachments/version.py

[Kani999]

Thanks @coderabbitai — both findings addressed:

• Test (test_template_extensions.py, error-swallow case) — done in a181a71. FakeExtension now counts render calls and test_custom_object_panel_swallows_render_errors asserts render_calls == 1, proving the empty string comes from the swallowed exception rather than an earlier gate.
• Perf (template_content.py:147-169, render_panel recomputes the display preference per hook) — intentionally skipped. The only DB lookup, custom_object_identifier(), is already lru_cached by model class, so across the three hook calls (left_page/right_page/full_width_page) it's a cache hit; the remainder is ~2 in-memory dict lookups for a single object per page load. Request-scoped caching would add state for negligible gain.

@coderabbitai resolve

[coderabbitai]

@Kani999 Thanks—acknowledged. The render-call assertion addresses the test gap, and the cached identifier lookup makes the additional request-scoped cache unnecessary for this path. Resolving both findings.

🐇 ✅

---

✏️ Learnings added

Learnt from: Kani999
URL: https://github.com/Kani999/netbox-attachments/pull/113

Timestamp: 2026-07-28T08:33:56.408Z
Learning: In `netbox_attachments/template_content.py`, `render_panel` may be invoked for the left, right, and full-width hooks during one page render. Its potentially database-backed `custom_object_identifier()` lookup is `lru_cache`d by model class; the remaining display-preference work is inexpensive in-memory lookup, so request-scoped caching is intentionally not needed.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

✅ Action performed

Comments resolved. Approval is disabled; enable reviews.request_changes_workflow to allow explicit top-level @coderabbitai resolve or @coderabbitai approve commands.