Related object tabs#482
Conversation
|
|
Kani999
force-pushed
the
feature/related-object-tabs
branch
from
fda8748 to
9d84499
Compare
|
Instead of a static list in PLUGINS_CONFIG, a per-COT "Show as dedicated tab" checkbox in the admin UI would be cleaner. High-priority COTs get their own tab; everything else falls back to a consolidated "Custom Objects" tab. |
|
@damsitt thanks for the suggestion — done. Replaced 12 commits, |
|
@Kani999 We're working on finalizing the v0.5.0 release of netbox-custom-objects this week. It already has a huge number of major features on-train, but it would be nice to get this one in there too. However, as it was not a stakeholder promise it isn't the end of the world if it has to be deferred to a v0.6.0. What is your feeling on the readiness? Is this week realistic? |
- __init__.py: call clear_url_caches() after inject_co_urls() in ready() so URL resolver picks up injected CO patterns in tests and management commands (flagged by CodeRabbit on PR netboxlabs#482) - combined_tab.html: render non-empty non-None values instead of always showing em-dash for non-URL/object fields in the else branch - combined_tab.html: replace plain edit button with a proper Bootstrap dropdown toggle so the action dropdown renders correctly - README.md: minor wording tweak ("Custom Object Type (COT)")
Smoke Test Results
Import Test Data |
|
Hi @mcolemann — from my side it feels ready. I've just run through the full smoke test matrix above and all scenarios pass except permission gating (which I haven't had a chance to verify yet, but the underlying logic is standard NetBox — it should just work). One known limitation worth calling out before merge: if you rename a COT's slug while the server is running, the old dedicated tab continues to appear alongside the new one until the process is restarted. The tab registry picks up the new slug immediately (live toggle works correctly), but the old URL entry stays in the router until the next startup. It's documented in the smoke test table (row 10) and in a If you or the team can spin up the test data (script + JSON are attached above) and walk through a few scenarios, that would be the fastest path to confirming it's v0.5.0-ready. Happy to address any issues that come up during your review. |
Permission gating — observed behaviour (smoke test row 11)Tested with a user who has view: Device only (no permissions on any custom object type). Base panel (the "Custom Objects" linked-objects panel in the left-side panels area): "Custom Objects" combined tab (the tab injected at the top of the device detail page): Dedicated tabs: Summary:
Neither issue is introduced by this PR, but worth noting before v0.5.0 ships. |
Kani999
force-pushed
the
feature/related-object-tabs
branch
from
4dadc29 to
7efcede
Compare
Kani999
changed the title
|
@Kani999 Thanks for pushing this forward. I think, in the interest of avoiding too much churn and destabilization, I'd like to defer this to a v0.6.0 release. (But note that doesn't mean it will be a long time before that release; it's just the next one to be cut from the The main thing I'm worried about is polymorphic object/multiobject fields, which are just about to land in |
- Add tab_views.py with combined and typed tab view factories - Combined tab shows all linked custom objects with actions, tags, column config, quick search - Typed tabs show per-COT filtered view with bulk actions and per-field filters - Auto-discover referenced models from app registry (never call get_model during registration) - Support CO-to-CO tabs (custom objects referencing other custom objects) - Badge callables use OR + distinct to avoid double-counting - Add combined_tab.html and typed_tab.html templates
- Add typed_tab_slugs to default_settings in PluginConfig - Call register_all_tabs() in ready() before super().ready() - Call inject_co_urls() and deduplicate_registry() after super().ready() - Add model_view_tabs to customobject.html for CO-to-CO tab support - Fix JournalEntryTable and ObjectChangeTable: remove unsupported user kwarg - Fix journal/changelog views to pass self.tab instead of string literals
- Add Related Object Tabs section explaining combined and typed tabs - Document typed_tab_slugs PLUGINS_CONFIG setting with example - Note that restart is required after config changes
- Replace _build_filterset_form with shared dynamic_forms.build_filterset_form_class (per prior PR netboxlabs#445 feedback / commit 26a39a5 invariant) - Extract _build_link_q helper; removes copy-pasted Q loop from _count_for_type._badge and TypedTabView.get - Drop CustomObjectType.objects.get(pk=cot_pk) refetch from typed-tab badge; use captured cot directly - Narrow bare except Exception to (OperationalError, ProgrammingError) in _count_linked and _get_linked_objects - prefetch_related('tags') in _get_linked_objects — kills per-row tag N+1 - Switch .restrict() try/except AttributeError to hasattr() guard; matches NetBox core pattern in netbox/views/generic/feature_views.py - Fix stale module docstring: typed-tab opt-in is typed_tab_slugs in PLUGINS_CONFIG, not show_tab=True
Apply .restrict(user, 'view') to linked-CO querysets so users without view permission on a referenced Custom Object model don't see its rows rendered in the related-object tab bodies. - _get_linked_objects now takes a user and restricts each per-model qs before filtering; the combined-tab view passes request.user. - TypedTabView.get restricts dynamic_model.objects before the link-Q filter so the typed-tab body is also gated. The combined-tab badge count can still include rows the user can't see; the body render itself is now restricted.
Pass a permission string into ViewTab so NetBox's core tab template tag skips the typed tab entirely for users without <app>.view_<model> permission on the underlying Custom Object model. Previously the badge count could include restricted rows while the body correctly hid them, producing a badge-vs-empty-body mismatch. The permission string is derived from the CO model resolved via model_ct_map at registration time (cot.object_type_id -> model), so no cot.get_model() call is introduced during tab registration. The combined tab is left unguarded intentionally — it aggregates across all CO types and is filtered per-row by the restrict() fix already in place.
ViewTab.visible now reads show_dedicated_tab per request from the DB, so toggling the flag takes effect on the next request without re-registration. Tolerates missing COTs and DB errors by hiding the tab.
_register_typed_tabs now registers a typed-tab view for every COT with an OBJECT/MULTIOBJECT field, leaving display gating to the ViewTab.visible predicate. This makes "show_dedicated_tab False at startup, toggled True later" work without restart, since the view is already registered. Removes the approach-c-backlog TODO (resolved by this change plus the upcoming refresh / signal mechanism).
Defends against bookmarked URLs hit after a flag flip. Visibility predicate hides the tab from the strip; this 404s the URL itself. Tests use RequestFactory + direct view invocation rather than the test Client because dynamically-registered NetBox typed-tab URLs aren't wired into the test environment's URL conf (URL conf is frozen at app-startup).
Workers keep a local version counter; on each request, middleware (next commit) compares against the Redis key co:tab_registry_version and re-runs idempotent register_all_tabs + inject_co_urls + clear_url_caches on mismatch. Lock-protected with double-checked version read for threaded worker setups. Cache failures are swallowed.
Calls refresh_tab_registry_if_stale on every request so workers self-heal their typed-tab registry when the Redis version key has been bumped by another worker's COT save/delete.
post_save and post_delete handlers, wrapped in transaction.on_commit, increment co:tab_registry_version. The middleware in each worker detects the bump and re-runs the (idempotent) registration logic. Skipped during migrations; cache failures are swallowed.
Validates the full chain: create new COT, signal bumps version, refresh registers the typed-tab view, related-model detail page renders the new tab — all without restart. Test calls refresh_tab_registry_if_stale() explicitly because plugin middleware activation in the test client is flaky for the post-startup-registered URL/registry path.
When a brand-new COT was created with show_dedicated_tab=True and then given an OBJECT/MULTIOBJECT field, the dedicated tab did not appear until a service restart. Two separate root causes: 1. The post_save signal only watched CustomObjectType. Workers that refreshed after the COT was created (before any field existed) found no referenced model and advanced their local version to match Redis. When the field was later added, no second signal fired, so those workers never re-ran register_all_tabs(). Fix: also connect bump_tab_registry_version to CustomObjectTypeField post_save/post_delete. 2. Even after register_all_tabs() ran at runtime, the new tab's URL was absent from Django's compiled URL conf. get_model_urls() is called once at app URL conf import time and the resulting list is static; register_model_view() at runtime adds to registry['views'] but never causes get_model_urls() to re-run. Fix: _inject_typed_tab_urls_into_app_urlconfs() finds the inner Python list that include() wraps for each model's <pk>/ resolver and appends the new URL pattern directly, so clear_url_caches() causes the rebuilt resolver to find it.
The third sentence warned that a service restart was required for changes to take effect. That limitation no longer exists.
- signals.py: pass timeout=None to cache.add() so the co:tab_registry_version key never expires. Without this Django's default 300s TTL resets the counter, causing workers whose local version is above the reset value to skip re-registration. - tab_views.py: reword the ordering comment in refresh_tab_registry_if_stale to accurately describe why _local_tab_registry_version is advanced last. - test_tab_views.py: add URL-injection assertion to test_refresh_registers_newly_created_cot — verifies that _inject_typed_tab_urls_into_app_urlconfs() mutated dcim.urls so Django's URL resolver finds the new typed tab after clear_url_caches.
- Move Http404 to module-level imports (was a local import inside TypedTabView.get) - Import CustomObjectType alongside CustomObjectTypeField at the top - Replace `CustomObjectType as COTModel` local alias with the already-imported name - Remove four WHAT-comments that duplicated what the code already said
typed_tab: add custom-objects-subtabs/mb-3 to sub-tab nav, drop extra mt-3 from results pane, add select-all box for multi-page bulk ops, wrap bulk buttons in btn-list/bulk-action-buttons, rename labels to Bulk Edit/Delete with trans, add block.super to modals block. combined_tab: replace inline style with ps-1, distinguish empty-state message when search is active.
… field Without this widget, the "No change / Yes / No" three-state semantics required by NetBox's bulk-edit machinery were broken — Django's default NullBooleanSelect renders "Unknown / Yes / No" which the bulk-edit view does not interpret correctly.
- Expand signals.py docstring: document Redis-flush/restart recovery path
- tab_views.py module docstring: add Known Limitations section describing
slug-rename duplicate-tab behaviour (both tabs appear until restart)
- Move Http404 import to the django.* group (ruff I-001)
- Fix __module__ string in _build_typed_table_class: 'database.tables'
was a stale leftover from the original tab plugin; corrected to
'netbox_custom_objects.tab_views' for accurate tracebacks
- Replace model_name.startswith('table') in inject_co_urls with a
set-membership check against actual registered CO models, removing
implicit coupling to the dynamic model naming scheme
- Strengthen test_edit_form_persists_show_dedicated_tab: assert 302
redirect instead of accepting 200, so form validation failures are
caught rather than silently passed
- __init__.py: call clear_url_caches() after inject_co_urls() in ready() so URL resolver picks up injected CO patterns in tests and management commands (flagged by CodeRabbit on PR netboxlabs#482) - combined_tab.html: render non-empty non-None values instead of always showing em-dash for non-URL/object fields in the else branch - combined_tab.html: replace plain edit button with a proper Bootstrap dropdown toggle so the action dropdown renders correctly - README.md: minor wording tweak ("Custom Object Type (COT)")
Kani999
force-pushed
the
feature/related-object-tabs
branch
from
7efcede to
2ee95e5
Compare
|
Understood, thanks for the context — agreed it's better to land related-object tabs on top of finalized polymorphic field support than to chase a moving target. I've already retargeted this PR from |
When a source COT has object fields pointing to two or more different CO target types, inject_co_urls() deduplicates by tab name and only keeps one URL pattern (wildcard <str:custom_object_type>). The closure-captured model_class then routes all targets through the first-registered target's queryset, returning wrong data or a spurious 404 on the second target. Fix: re-resolve the target model from the URL slug when the captured model_class belongs to the CO app — mirroring the identical pattern already used in CombinedTabView.get().
The per-row actions cell had an unclosed <a> tag and a duplicated dropdown-toggle anchor, leaving users without an edit link and rendering two carets. Replace with a proper edit anchor pointing at customobject_edit and a single dropdown toggle, matching the pattern used in customobjecttype.html.
Kani999
force-pushed
the
feature/related-object-tabs
branch
from
b0f88da to
d3faf82
Compare
|
Heads up — this is not ready to merge yet. I need to:
I'll push the revised commits and updated test results once that's done. |
|
@Kani999 I've converted this PR to a draft per your note above. Just mark it as "ready for review" when the time comes. Thanks! |
5 participants
Summary
Integrates the standalone netbox-custom-objects-tab plugin into core netbox-custom-objects (ref: CESNET/netbox-custom-objects-tab#8).
Closes #26
What's implemented
co:tab_registry_version) is bumped bypost_save/post_deletesignals onCustomObjectTypeandCustomObjectTypeField. A thin middleware (TabRegistryRefreshMiddleware) checks the counter on every request and re-runs idempotent tab registration + URL injection on mismatch. Cost: one Redis GET per request; re-registration runs only on version change. Works correctly across multiple Gunicorn/Granian workers without restart.__init__.py), templates, and README documentation.Resolved during review
typed_tab_slugslist with a per-COTshow_dedicated_tabBooleanField (commits977feee…fd4155b), per @damsitt's suggestion. Surfaces the toggle on the edit form, bulk-edit, CSV import, list table, API serializer, and detail page.show_dedicated_tabor creating a new COT now takes effect on the next page load across all workers, with no restart required. Implemented via:ViewTab.visible()predicate — readsshow_dedicated_tablive from the DB per request (handles toggle on existing COTs)signals.py—post_save/post_deleteonCustomObjectType+CustomObjectTypeFieldbumpco:tab_registry_versionin Redis viatransaction.on_commitmiddleware.py(TabRegistryRefreshMiddleware) — callsrefresh_tab_registry_if_stale()on every request; re-runsregister_all_tabs()+ URL conf mutation +clear_url_caches()on version mismatch (handles new-COT creation)_inject_typed_tab_urls_into_app_urlconfs()— mutates the live Python list that each app'sinclude()wraps, so Django's rebuilt resolver sees new typed-tab URL patterns afterclear_url_caches()Known limitations
cot_pk, not the slug). A process restart clears stale entries. Proper fix would track the old slug viapre_saveand remove it from the registry; left as a future improvement.Images