Refactor detail views to the UI Component Framework (#196) - #395
Open
jvanderaa wants to merge 1 commit into
Open
Refactor detail views to the UI Component Framework (#196)#395jvanderaa wants to merge 1 commit into
jvanderaa wants to merge 1 commit into
Conversation
Re-homes the UIComponent refactor from the u/pavan-uicomponents-easy-kc integration branch onto develop. That branch pinned nautobot >=2.4.16,<3.0.0 and predated Nautobot 3.0, so it could no longer be tested against a supported Nautobot; Nautobot 2 support lives on ltm-2.4, not develop. - Move CircuitMaintenance, CircuitImpact, Note, RawNotification, and NotificationSource detail views to declarative ObjectDetailContent panels and drop the custom *_retrieve.html templates and the now-unused note.html. - Import the UI components from the public nautobot.apps.ui surface rather than nautobot.core.ui.*, per app development standards. - Disable the related-object badge link on the ParsedNotification panel; ParsedNotification is detail-only, so no list URL exists to link to. This is enforced by Nautobot 3's test_body_content_table_list_url. - Handle RedirectAuthorize in validate_source so OAuth sources redirect to the provider consent flow instead of raising, log ValueError failures, and report the result via a messages flash and a redirect to the detail view. - Add table columns for the new panels, and a regression test for the OAuth validate path. Co-authored-by: Pavan-183 <pavan.pulicherla@networktocode.com>
jvanderaa
requested review from
glennmatthews,
pke11y and
scetron
as code owners
August 5, 2026 15:21
This was referenced Aug 5, 2026
Comment on lines
75
to
77
| maintenance = tables.Column(linkify=True) | ||
| circuit = tables.Column(linkify=True) | ||
| impact = tables.Column(linkify=True) |
Contributor
There was a problem hiding this comment.
Remove maintenance and circuit columns? They're no longer in Meta.fields - intentional?
| circuit = tables.Column(linkify=True) | ||
| impact = tables.Column(linkify=True) | ||
|
|
||
| cid = tables.Column(accessor="circuit.cid", verbose_name="ID", linkify=True) |
Contributor
There was a problem hiding this comment.
IIRC the . pattern in accessors is deprecated by django-tables2, which prefers __ instead.
| "title", | ||
| "last_updated", | ||
| "comment", | ||
| ) # pylint:disable=nb-use-fields-all |
Comment on lines
+164
to
+165
| raw_notification_source = tables.Column(accessor="raw_notification.source", verbose_name="Source") | ||
| stamp = tables.Column(accessor="raw_notification.stamp", verbose_name="Received") |
Contributor
There was a problem hiding this comment.
Missing from Meta.fields? Remove?
| ObjectView, | ||
| ) | ||
| from nautobot.circuits.models import Circuit | ||
| from nautobot.core.choices import ButtonActionColorChoices |
Contributor
There was a problem hiding this comment.
importable from nautobot.apps.choices (preferred)
| ) | ||
| from nautobot.circuits.models import Circuit | ||
| from nautobot.core.choices import ButtonActionColorChoices | ||
| from nautobot.core.templatetags import helpers |
Contributor
There was a problem hiding this comment.
import individual template tags from nautobot.apps.templatetags instead.
Comment on lines
+337
to
+341
| elif not isinstance(value, (bytes, bytearray)): | ||
| try: | ||
| value = bytes(value or b"") | ||
| except (TypeError, ValueError): | ||
| value = b"" |
Contributor
There was a problem hiding this comment.
possible to add test coverage for this?
Comment on lines
+430
to
+431
| setattr(instance, "account", source.get_account_id()) | ||
| setattr(instance, "source_type", source.__class__.__name__) |
Contributor
There was a problem hiding this comment.
Not obvious to me why we need to use setattr() here and below, versus just directly assigning instance.account = source.get_account_id(), etc.
Comment on lines
+454
to
+459
| items = [] | ||
| for val in value: | ||
| items.append(helpers.hyperlinked_object(val)) | ||
| if not items: | ||
| return helpers.HTML_NONE | ||
| return format_html( |
Contributor
There was a problem hiding this comment.
Add test coverage? LInes 457-458 look unnecessary/unreachable to me.
nautobot-bot
pushed a commit
that referenced
this pull request
Aug 10, 2026
…r Tool ## [nautobot-app-v2.7.3 (2026-08-07)](https://github.com/nautobot/cookiecutter-nautobot-app/releases/tag/nautobot-app-v2.7.3) ### Changed - [#395](nautobot/cookiecutter-nautobot-app#395) - Added `target` and `recursive` options to `invoke pylint` tasks. - [#396](nautobot/cookiecutter-nautobot-app#396) - Added a `--diff` option to the `invoke ruff` tasks.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #196.
Re-homes the UI Component Framework refactor onto
develop. The original work is @Pavan-183's (#350) and he is credited as co-author; this PR adapts it to Nautobot 3, incorporates review feedback, and adds test coverage.Why this targets
developrather than the integration branch#350 was merged into the
u/pavan-uicomponents-easy-kcintegration branch. That branch pinsnautobot = ">=2.4.16,<3.0.0"and was cut before Nautobot 3.0 shipped, so it can no longer be tested against a supported Nautobot — its CIstablelegs fail at import withImportError: cannot import name 'PluginCustomValidator'(renamed toCustomValidatorin Nautobot 3), in files unrelated to the refactor.Nautobot 2 support lives on
ltm-2.4;developandmainare Nautobot 3 only. So there is no Nautobot 2 obligation for this change, and the stale imports simply disappear by building ondevelop, which already carries the migrated versions of those files.The refactor itself is still needed —
develophad not independently migrated. Its detail views still usedget_extra_context()and all four*_retrieve.htmltemplates were still present.What this does
ObjectDetailContentpanels.*_retrieve.htmltemplates andnote.html, which the framework now renders.parsednotification.htmlis kept —ParsedNotificationViewis still a plainObjectView.ParsedNotificationTablefor the new panels.Nautobot 3 adaptations (not in the original)
nautobot.apps.uisurface rather thannautobot.core.ui.object_detail/nautobot.core.ui.choices. All the components are exported there, and app development standards prefernautobot.apps.*.enable_related_link=Falseon the ParsedNotification panel.ParsedNotificationis detail-only —urls.pydefines no list route — so the panel's badge link had no URL to resolve to and logged a warning on every Circuit Maintenance page view. Nautobot 3 enforces this viatest_body_content_table_list_url, which failed until this was set. This is a genuine bug the 2.4-era branch could not surface.Review feedback incorporated from #350
validate_sourceno longer raises on OAuth sources.Source.test_authentication()deliberately re-raisesRedirectAuthorizefor OAuth sources (e.g. Gmail) needing consent. The refactor caught only(AttributeError, TypeError, ValueError), so it escaped as an unhandled 500 — and the "Validate Authentication" button links straight to that action. It now catchesRedirectAuthorizeand redirects to the provider consent flow, logsValueErrorfailures withexc_info=True, and reports the outcome via amessagesflash plus a redirect to the detail view. This replaces the temporary scaffolding that rendered the legacy template with three hand-built empty tables.return_urlGET parameter and redirected to it. That is intentionally not restored — redirecting to a user-supplied URL is an open-redirect risk. The redirect always goes to the object's own detail page.CustomMaintenanceFieldsPanelwhose computednotification_providervalue was never read.serializer_class/table_class/action_buttonsdeclarations onRawNotificationUIViewSet.NotificationObjectFieldsPanel.get_datatoValueErrorand restored logging on the failure path.ButtonActionColorChoicesfor the Validate button, no out-of-scope view churn).Deliberately excluded
forms.pychange swappedCircuitMaintenanceBulkEditForm's bases to(NautobotBulkEditForm, TagsBulkEditFormMixin). Mixins belong before the base form class, sodevelop's existing order is kept.developintentionally removedcustom_view_additional_permissionsfrom thevalidate_sourceaction; that removal is preserved rather than reverted.{% endblock %}, stripped trailing newlines) is omitted to keep the diff reviewable.Testing
test_views196 OK (31 skipped) after theenable_related_linkfix.test_basics.TestVersion('3.0.1a0' != '3.1.2a0'), which reproduces identically on a pristinedevelopcheckout — a stale app version in the local container image, unrelated to this change.ruff checkandruff format --checkclean.test_validate_view_redirect_authorize, a regression test for the OAuth path, and updates the two existing validate tests for the redirect-and-flash behavior.Supersedes
u/pavan-uicomponents-easy-kc; that branch can be retired once this lands.Screenshots
Detail-view rendering is unchanged in intent but now driven by panels rather than templates. Happy to attach before/after captures of the Circuit Maintenance, Notification Source, and Raw Notification detail pages if that would help review.