Skip to content
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

Merged
merged 4 commits into from
Apr 21, 2023
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions changes/2901.added
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.
7 changes: 0 additions & 7 deletions examples/example_plugin/example_plugin/models.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from django.db import models
from django.urls import reverse

from nautobot.apps.models import extras_features, OrganizationalModel

Expand All @@ -26,9 +25,6 @@ class Meta:
def __str__(self):
return f"{self.name} - {self.number}"

def get_absolute_url(self):
return reverse("plugins:example_plugin:examplemodel", kwargs={"pk": self.pk})

def to_csv(self):
return (
self.name,
Expand All @@ -51,6 +47,3 @@ class AnotherExampleModel(OrganizationalModel):

class Meta:
ordering = ["name"]

def get_absolute_url(self):
return reverse("plugins:example_plugin:anotherexamplemodel", kwargs={"pk": self.pk})
16 changes: 0 additions & 16 deletions nautobot/circuits/models.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.contrib.contenttypes.models import ContentType
from django.core.exceptions import ValidationError
from django.db import models
from django.urls import reverse

from nautobot.dcim.fields import ASNField
from nautobot.dcim.models import CableTermination, PathEndpoint
Expand Down Expand Up @@ -56,9 +55,6 @@ class Meta:
def __str__(self):
return self.name

def get_absolute_url(self):
return reverse("circuits:providernetwork", args=[self.slug])

def to_csv(self):
return (
self.provider.name,
Expand Down Expand Up @@ -127,9 +123,6 @@ class Meta:
def __str__(self):
return self.name

def get_absolute_url(self):
return reverse("circuits:provider", args=[self.slug])

def to_csv(self):
return (
self.name,
Expand Down Expand Up @@ -165,9 +158,6 @@ class Meta:
def __str__(self):
return self.name

def get_absolute_url(self):
return reverse("circuits:circuittype", args=[self.slug])

def to_csv(self):
return (
self.name,
Expand Down Expand Up @@ -255,9 +245,6 @@ class Meta:
def __str__(self):
return self.cid

def get_absolute_url(self):
return reverse("circuits:circuit", args=[self.pk])

def to_csv(self):
return (
self.cid,
Expand Down Expand Up @@ -325,9 +312,6 @@ class Meta:
def __str__(self):
return f"Termination {self.term_side}: {self.site or self.provider_network}"

def get_absolute_url(self):
return reverse("circuits:circuittermination", args=[self.pk])

def clean(self):
super().clean()

Expand Down
2 changes: 2 additions & 0 deletions nautobot/core/api/urls.py
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,8 @@
path("graphql/", GraphQLDRFAPIView.as_view(), name="graphql-api"),
# Plugins
path("plugins/", include((plugin_api_patterns, "plugins-api"))),
# Core (keeping for backwards compatibility for the moment)
bryanculver marked this conversation as resolved.
Show resolved Hide resolved
path("core/", include((core_api_patterns, "core-api"))),
# UI
path("ui/", include((ui_api_patterns, "ui-api"))),
]
70 changes: 35 additions & 35 deletions nautobot/core/api/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -210,49 +210,49 @@ def get_serializer(self, *args, **kwargs):
return super().get_serializer(*args, **kwargs)

# TODO: the below needs to be either fixed or removed as part of issue #3042.
# def get_serializer_class(self):
# logger = logging.getLogger("nautobot.core.api.views.ModelViewSet")

# # If using 'brief' mode, find and return the nested serializer for this model, if one exists
# if self.brief:
# logger.debug("Request is for 'brief' format; initializing nested serializer")
# try:
# serializer = get_serializer_for_model(self.queryset.model, prefix="Nested")
# logger.debug(f"Using serializer {serializer}")
# return serializer
# except SerializerNotFound:
# logger.debug(f"Nested serializer for {self.queryset.model} not found!")

# # Fall back to the hard-coded serializer class
# return self.serializer_class
def get_serializer_class(self):
logger = logging.getLogger("nautobot.core.api.views.ModelViewSet")

# If using 'brief' mode, find and return the nested serializer for this model, if one exists
if self.brief:
logger.debug("Request is for 'brief' format; initializing nested serializer")
try:
serializer = get_serializer_for_model(self.queryset.model, prefix="Nested")
logger.debug(f"Using serializer {serializer}")
return serializer
except SerializerNotFound:
logger.debug(f"Nested serializer for {self.queryset.model} not found!")

# Fall back to the hard-coded serializer class
return self.serializer_class

# TODO: this is part of issue #3042.
def get_serializer_context(self):
ctx = super().get_serializer_context()
ctx["request"] = None
try:
depth = int(self.request.query_params.get("depth", 0))
except ValueError:
depth = 0 # Ignore non-numeric parameters and keep default 0 depth
ctx["depth"] = depth
# def get_serializer_context(self):
# ctx = super().get_serializer_context()
# ctx["request"] = None
# try:
# depth = int(self.request.query_params.get("depth", 0))
# except ValueError:
# depth = 0 # Ignore non-numeric parameters and keep default 0 depth
# ctx["depth"] = depth
Comment on lines +230 to +237
Copy link
Member Author

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


return ctx
# return ctx

# TODO: the below needs to be either fixed or remvoed as part of issue #3042.
# def get_queryset(self):
# # If using brief mode, clear all prefetches from the queryset and append only brief_prefetch_fields (if any)
# if self.brief:
# # v2 TODO(jathan): Replace prefetch_related with select_related
# return super().get_queryset().prefetch_related(None).prefetch_related(*self.brief_prefetch_fields)
def get_queryset(self):
# If using brief mode, clear all prefetches from the queryset and append only brief_prefetch_fields (if any)
if self.brief:
# v2 TODO(jathan): Replace prefetch_related with select_related
return super().get_queryset().prefetch_related(None).prefetch_related(*self.brief_prefetch_fields)

# return super().get_queryset()
return super().get_queryset()

# def initialize_request(self, request, *args, **kwargs):
# # Check if brief=True has been passed
# if request.method == "GET" and request.GET.get("brief"):
# self.brief = True
def initialize_request(self, request, *args, **kwargs):
# Check if brief=True has been passed
if request.method == "GET" and request.GET.get("brief"):
self.brief = True

# return super().initialize_request(request, *args, **kwargs)
return super().initialize_request(request, *args, **kwargs)

def restrict_queryset(self, request, *args, **kwargs):
"""
Expand Down
2 changes: 1 addition & 1 deletion nautobot/core/context_processors.py
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#3615 Introduced a bug on some views that were using the base_template context. Missed because we didn't have passing tests, which this should fix!!!

}


Expand Down
25 changes: 25 additions & 0 deletions nautobot/core/models/__init__.py
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import uuid

from django.db import models
from django.urls import NoReverseMatch, reverse

from nautobot.core.models.querysets import RestrictedQuerySet
from nautobot.core.utils.lookup import get_route_for_model


class BaseModel(models.Model):
Expand Down Expand Up @@ -34,6 +36,29 @@ def present_in_database(self):
"""
return not self._state.adding

def get_absolute_url(self):
"""
Return the canonical URL for this object.
"""

# Iterate the pk-like fields and try to get a URL, or return None.
fields = ["pk", "slug"] # TODO: Eventually all PKs
actions = ["retrieve", "detail", ""] # TODO: Eventually all retrieve

for field in fields:
if not hasattr(self, field):
continue

for action in actions:
route = get_route_for_model(self, action)

try:
return reverse(route, kwargs={field: getattr(self, field)})
except NoReverseMatch:
continue

return None
bryanculver marked this conversation as resolved.
Show resolved Hide resolved

class Meta:
abstract = True

Expand Down
6 changes: 0 additions & 6 deletions nautobot/core/models/name_color_content_types.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
from django.contrib.contenttypes.models import ContentType
from django.db import models
from django.db.models import Q
from django.urls import reverse

from nautobot.core.choices import ColorChoices
from nautobot.core.models import BaseModel
Expand Down Expand Up @@ -84,11 +83,6 @@ def __str__(self):
def natural_key(self):
return (self.name,)

def get_absolute_url(self):
ct = f"{self._meta.app_label}:{self._meta.model_name}"
# TODO(timizuo): Replace self.slug with natural key or pk
return reverse(ct, args=[self.slug])

def get_content_types(self):
return ",".join(f"{ct.app_label}.{ct.model}" for ct in self.content_types.all())

Expand Down
2 changes: 1 addition & 1 deletion nautobot/core/templates/base.html
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' %}
4 changes: 3 additions & 1 deletion nautobot/core/testing/views.py
Original file line number Diff line number Diff line change
Expand Up @@ -772,7 +772,9 @@ def test_list_objects_with_constrained_permission(self):
if hasattr(self.model, "name"):
self.assertIn(instance1.name, content, msg=content)
self.assertNotIn(instance2.name, content, msg=content)
elif hasattr(self.model, "get_absolute_url"):
elif (
hasattr(self.model, "get_absolute_url") and instance1.get_absolute_url() is not None
): # TODO fix RelationshipAssociation
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

RelationshipAssociation Doesn't play well with this test case so may need some tests overloaded since this and like DynamicGroupMembers don't make sense to have their own list vies.

self.assertIn(instance1.get_absolute_url(), content, msg=content)
self.assertNotIn(instance2.get_absolute_url(), content, msg=content)

Expand Down
4 changes: 3 additions & 1 deletion nautobot/core/utils/lookup.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Many of our routes right now do not end in _detail or _retrieve. For now, this works and we can choose to clean up our routes whenever instead of scope-creaping this.

viewname = f"{prefix}{sep}{action}"

if model._meta.app_label in settings.PLUGINS:
Expand Down
25 changes: 12 additions & 13 deletions nautobot/core/views/generic.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The 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:
Expand Down
8 changes: 7 additions & 1 deletion nautobot/core/views/mixins.py
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member Author

Choose a reason for hiding this comment

The 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.
Expand Down
4 changes: 0 additions & 4 deletions nautobot/dcim/models/cables.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@
from django.core.exceptions import ObjectDoesNotExist, ValidationError
from django.db import models
from django.db.models import Sum
from django.urls import reverse
from django.utils.functional import classproperty

from nautobot.core.models.fields import ColorField
Expand Down Expand Up @@ -136,9 +135,6 @@ def __str__(self):
pk = self.pk or self._pk
return self.label or f"#{pk}"

def get_absolute_url(self):
return reverse("dcim:cable", args=[self.pk])

@classproperty # https://github.com/PyCQA/pylint-django/issues/240
def STATUS_CONNECTED(cls): # pylint: disable=no-self-argument
"""Return a cached "connected" `Status` object for later reference."""
Expand Down