View function is shown incorrectly

[bakefayat]

by upgrading django to version 4.0.2 view function on django-debug-toolbar doesn't display properly.
The expected string should be the class name: DashboardView not "backend_base.views.view".
"backend_base" is app name.

The class itself is very simple:
class DashboardView(LoginRequiredMixin, NavBarMixin, TemplateView):
template_name = 'dashboard.html'

backend_base.views.view

source of issue: mario-signorino on stackoverflow.

[matthiask]

It would be interesting to see how you're instantiating the DashboardView... it seems that maybe debug_toolbar.utils.get_name_from_obj doesn't do the right thing.

[mariosgn]

The view is just as described:

class DashboardView(LoginRequiredMixin, TemplateView):
    template_name = 'dashboard.html' 

and it is linked in urls.py as:

path('', DashboardView.as_view(), name="dashboard"),

Everything is in this virtualenv with Python 3.10.4:

asgiref==3.5.2
async-timeout==4.0.2
beautifulsoup4==4.11.1
certifi==2022.6.15
charset-normalizer==2.0.12
Deprecated==1.2.13
Django==4.0.5
django-bootstrap5==21.3
django-debug-toolbar==3.4.0
greenlet==1.1.2
idna==3.3
jsons==1.6.3
mqtt==0.0.1
packaging==21.3
paho-mqtt==1.6.1
pika==1.2.1
psycopg2==2.9.3
pyparsing==3.0.9
redis==4.3.3
requests==2.28.0
soupsieve==2.3.2.post1
SQLAlchemy==1.4.37
sqlparse==0.4.2
typish==1.9.3
urllib3==1.26.9
wrapt==1.14.1

if I look in debug_toolbar.utils.get_name_from_obj I see:

    if hasattr(obj, "__name__"):
        name = obj.__name__
    else:
        name = obj.__class__.__name__

where (in my case) obj is: '<function View.as_view.<locals>.view at 0x7f26f3d5dea0>'.

The first if hasattr(obj, "__name__"): pass and set name to 'view'.
Then the second if hasattr(obj, "__module__"): set module to 'backend_base.views' and hence, name to 'backend_base.views.view'`

The final status is:

[matthiask]

Interesting. I see that the module path is correct. obj.__name__ didn't work for class-based views earlier so maybe something has changed in this area. We might try changing the code to prefer obj.__class__.__name__ to obj.__name__ if the former exists (even though I personally have a strong preference for FBVs over CBVs ;-)

[matthiask]

Ah no, forget about that. We should do the same thing Django's admindocs app does, see

https://github.com/django/django/blob/9a22d1769b042a88741f0ff3087f10d94f325d86/django/contrib/admindocs/utils.py#L26-L32

[mariosgn]

yes, it seems to work: just replacing

def get_name_from_obj(obj):
    if hasattr(obj, "__name__"):
        name = obj.__name__
    else:
        name = obj.__class__.__name__

    if hasattr(obj, "__module__"):
        module = obj.__module__
        name = f"{module}.{name}"

    return name

with

def get_name_from_obj(view_func):
    if hasattr(view_func, "view_class"):
        klass = view_func.view_class
        return f"{klass.__module__}.{klass.__qualname__}"
    mod_name = view_func.__module__
    view_name = getattr(view_func, "__qualname__", view_func.__class__.__name__)
    return mod_name + "." + view_name

fix my problem.
Not sure though about any side effects.

[matthiask]

Do you want to submit this as a pull request? I think the code looks fine; if the testsuite passes then we're good.

[leandrodesouzadev]

I had ran a test with this change. And looks like all the tests related to this function fail:

FAIL: test_class (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 32, in test_class
    self.assertEqual(res, "tests.test_utils.A")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_class.<locals>.A' != 'tests.test_utils.A'
- tests.test_utils.GetNameFromObjTestCase.test_class.<locals>.A
+ tests.test_utils.A


======================================================================
FAIL: test_func (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 21, in test_func
    self.assertEqual(res, "tests.test_utils.x")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_func.<locals>.x' != 'tests.test_utils.x'
- tests.test_utils.GetNameFromObjTestCase.test_func.<locals>.x
+ tests.test_utils.x


======================================================================
FAIL: test_lambda (tests.test_utils.GetNameFromObjTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/leandro/dev/contrib/django-debug-toolbar/tests/test_utils.py", line 25, in test_lambda
    self.assertEqual(res, "tests.test_utils.<lambda>")
AssertionError: 'tests.test_utils.GetNameFromObjTestCase.test_lambda.<locals>.<lambda>' != 'tests.test_utils.<lambda>'
- tests.test_utils.GetNameFromObjTestCase.test_lambda.<locals>.<lambda>
+ tests.test_utils.<lambda>

[bluesurfer]

Any update on this? It was a very useful feature..

[matthiask]

Nobody took the time to submit a pull request until now, so there's no update yet, unfortunately :-/

[tim-schilling]

@leandrodesouzadev are you working on this?

[leandrodesouzadev]

Actually no, i wasn't quite sure if i could just make the changes on the tests so they pass.
Can i just make this changes?

[tim-schilling]

I think that makes sense. Let's see it and decide.

[leandrodesouzadev]

I have implemented the feature and modified some tests to make it work. Now the code it's essentially the same as the quoted by @matthiask:

Ah no, forget about that. We should do the same thing Django's admindocs app does, see

https://github.com/django/django/blob/9a22d1769b042a88741f0ff3087f10d94f325d86/django/contrib/admindocs/utils.py#L26-L32

I wonder if there's any reason for us to keep this piece of code that's essentially the same as the django implementation.
I think we have 3 options here:

1. Refactor our get_name_from_obj function to do the same as the django admindocs;
2. Refactor our get_name_from_obj function to be a proxy to the django admindocs get_view_name function;
3. Just use the get_view_name function directly from the django admindocs module.

For the first case, the implementation would be the following:

def get_name_from_obj(obj: Any) -> str:
    if hasattr(obj, "view_class"):
        klass = obj.view_class
        return f"{klass.__module__}.{klass.__qualname__}"
    mod_name = obj.__module__
    view_name = getattr(obj, "__qualname__", obj.__class__.__name__)
    return mod_name + "." + view_name

For the second case, the implementation would be the following:

from django.contrib.admindocs.utils import get_view_name
# ...
def get_name_from_obj(obj: Any) -> str:
    return get_view_name(obj)

For the third case, i saw that are we're going to need to change the following lines to use the get_view_name function from the django admindocs module:
Panel class, on the __init__ method
RequestPanel class, on the generate_stats method
I see the benefit on this 3rd implementation that we could get away of the get_name_from_obj function, as well of the 3 tests for that utility function. But we have a drawback if they ever change this functionality.

Waiting on your suggestions. Cheers.