-
Notifications
You must be signed in to change notification settings - Fork 252
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Get tests passing for UI 2.0 #3632
Changes from 3 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1 @@ | ||
Added `get_absolute_url` method on `BaseModel` which will attempt to resolve the detail view route for all subclassed models. |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -36,7 +36,7 @@ def settings(request): | |
|
||
return { | ||
"settings": django_settings, | ||
"base_template": "base_react.html" if request.COOKIES.get("newui", False) else "base_django.html", | ||
"root_template": "base_react.html" if request.COOKIES.get("newui", False) else "base_django.html", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #3615 Introduced a bug on some views that were using the |
||
} | ||
|
||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1 @@ | ||
{% extends base_template|default:'base_django.html' %} | ||
{% extends root_template|default:'base_django.html' %} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -76,7 +76,9 @@ def get_route_for_model(model, action, api=False): | |
|
||
suffix = "" if not api else "-api" | ||
prefix = f"{model._meta.app_label}{suffix}:{model._meta.model_name}" | ||
sep = "_" if not api else "-" | ||
sep = "" | ||
if action != "": | ||
sep = "_" if not api else "-" | ||
Comment on lines
+79
to
+81
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Many of our routes right now do not end in |
||
viewname = f"{prefix}{sep}{action}" | ||
|
||
if model._meta.app_label in settings.PLUGINS: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -126,23 +126,22 @@ def get(self, request, *args, **kwargs): | |
if isinstance(instance, ChangeLoggedModel): | ||
changelog_url = instance.get_changelog_url() | ||
|
||
# TODO: we shouldn't be importing a private-named function from another module. Should it be renamed? | ||
from nautobot.extras.templatetags.plugins import _get_registered_content | ||
|
||
temp_fake_context = { | ||
"object": instance, | ||
"request": request, | ||
"settings": {}, | ||
"csrf_token": "", | ||
"perms": {}, | ||
} | ||
|
||
plugin_tabs = _get_registered_content(instance, "detail_tabs", temp_fake_context, return_html=False) | ||
|
||
# TODO: this feels inelegant - should the tabs lookup be a dedicated endpoint rather than piggybacking | ||
# on the object-retrieve endpoint? | ||
# TODO: similar functionality probably needed in NautobotUIViewSet as well, not currently present | ||
if request.GET.get("viewconfig", None) == "true": | ||
# TODO: we shouldn't be importing a private-named function from another module. Should it be renamed? | ||
from nautobot.extras.templatetags.plugins import _get_registered_content | ||
|
||
temp_fake_context = { | ||
"object": instance, | ||
"request": request, | ||
"settings": {}, | ||
"csrf_token": "", | ||
"perms": {}, | ||
} | ||
|
||
plugin_tabs = _get_registered_content(instance, "detail_tabs", temp_fake_context, return_html=False) | ||
Comment on lines
+133
to
+144
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. When get to a polished implementation for plugin views we will likely overhaul this. |
||
resp = {"tabs": plugin_tabs} | ||
return JsonResponse(resp) | ||
else: | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -181,7 +181,13 @@ def get_return_url(self, request, obj=None): | |
# Note that the use of both `obj.present_in_database` and `obj.pk` is correct here because this conditional | ||
# handles all three of the create, update, and delete operations. When Django deletes an instance | ||
# from the DB, it sets the instance's PK field to None, regardless of the use of a UUID. | ||
if obj is not None and obj.present_in_database and obj.pk and hasattr(obj, "get_absolute_url"): | ||
if ( | ||
obj is not None | ||
and obj.present_in_database | ||
and obj.pk | ||
and hasattr(obj, "get_absolute_url") | ||
and obj.get_absolute_url() is not None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Handle error now |
||
): | ||
return obj.get_absolute_url() | ||
|
||
# Fall back to the default URL (if specified) for the view. | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Likely to be replaced with changes in #3500