Opened 4 years ago

Closed 4 years ago

#30981 closed Bug (fixed)

Admin changelist crashes when using F() and OrderBy() expressions in the ModelAdmin's admin_order_field.

Reported by: Petr Dlouhý Owned by: Hasan Ramezani
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: yes Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When I use admin_order_field with F() expression or any other type of expression that doesn't have as_sql such as:

class UserProfileAdmin(admin.ModelAdmin):
    def get_sum_amount(self, obj):
          return obj.sum_amount
      get_sum_amount.admin_order_field = F('sum_amount')

I got the following crash:

Traceback (most recent call last):
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/template/base.py", line 835, in _resolve_lookup
    return self.get_fields()
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/views/main.py", line 81, in __init__
    self.queryset = self.get_queryset(request)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/views/main.py", line 435, in get_queryset
    ordering = self.get_ordering(request, qs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/views/main.py", line 296, in get_ordering
    elif order_field.startswith('-') and pfx == '-':
AttributeError: 'F' object has no attribute 'startswith'

This is similar issue like https://code.djangoproject.com/ticket/28958 but wasn't fixed by the patch.

Change History (6)

comment:1 by Petr Dlouhý, 4 years ago

Note: this is motivated by usage with nulls_last such as F('sum_amount').desc(nulls_last=True), which does throw a different traceback:

Traceback (most recent call last):
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/core/handlers/exception.py", line 34, in inner
    response = get_response(request)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/core/handlers/base.py", line 115, in _get_response
    response = self.process_exception_by_middleware(e, request)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/core/handlers/base.py", line 113, in _get_response
    response = wrapped_callback(request, *callback_args, **callback_kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/options.py", line 606, in wrapper
    return self.admin_site.admin_view(view)(*args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/views/decorators/cache.py", line 44, in _wrapped_view_func
    response = view_func(request, *args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/sites.py", line 223, in inner
    return view(request, *args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/utils/decorators.py", line 45, in _wrapper
    return bound_method(*args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/utils/decorators.py", line 142, in _wrapped_view
    response = view_func(request, *args, **kwargs)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/options.py", line 1672, in changelist_view
    cl = self.get_changelist_instance(request)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/options.py", line 744, in get_changelist_instance
    sortable_by,
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/views/main.py", line 81, in __init__
    self.queryset = self.get_queryset(request)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/contrib/admin/views/main.py", line 436, in get_queryset
    qs = qs.order_by(*ordering)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/db/models/query.py", line 1074, in order_by
    obj.query.add_ordering(*field_names)
  File "/home/petr/.local/share/virtualenvs/aklub_project-HrPEZ5ak/lib/python3.6/site-packages/django/db/models/sql/query.py", line 1804, in add_ordering
    if not hasattr(item, 'resolve_expression') and not ORDER_PATTERN.match(item):
TypeError: expected string or bytes-like object

comment:2 by Mariusz Felisiak, 4 years ago

Summary: Admin changelist crashes when using query expression in the ModelAdmin's admin_order_fieldAdmin changelist crashes when using F() and OrderBy() expressions in the ModelAdmin's admin_order_field.
Triage Stage: UnreviewedAccepted
Type: UncategorizedBug
Version: 2.2master

Thanks for this report, we have here two issues. First, Admin's changelist crashes when using a F() expression in admin_order_field, we can fix this easily but checking resolve_expression instead of as_sql, e.g.

diff --git a/django/contrib/admin/views/main.py b/django/contrib/admin/views/main.py
index 2f0e915d5e..f8b3015fb8 100644
--- a/django/contrib/admin/views/main.py
+++ b/django/contrib/admin/views/main.py
@@ -308,7 +308,7 @@ class ChangeList:
                     order_field = self.get_ordering_field(field_name)
                     if not order_field:
                         continue  # No 'admin_order_field', skip it
-                    if hasattr(order_field, 'as_sql'):
+                    if hasattr(order_field, 'resolve_expression'):
                         # order_field is an expression.
                         ordering.append(order_field.desc() if pfx == '-' else order_field.asc())
                     # reverse order if order_field has already "-" as prefix

Second issue with F('sum_amount').desc(nulls_last=True) is related with the fact that this is an instance of an OrderBy() expression, so .asc() and .desc() don't return new instances of an OrderBy().

comment:3 by Hasan Ramezani, 4 years ago

Has patch: set
Owner: changed from nobody to Hasan Ramezani
Status: newassigned

comment:4 by Mariusz Felisiak, 4 years ago

Needs tests: set
Patch needs improvement: set

comment:5 by Hasan Ramezani, 4 years ago

Needs tests: unset
Patch needs improvement: unset

comment:6 by Mariusz Felisiak <felisiak.mariusz@…>, 4 years ago

Resolution: fixed
Status: assignedclosed

In 0284a26:

Fixed #30981 -- Fixed admin changelist crash when using F() or OrderBy() expressions in admin_order_field.

Note: See TracTickets for help on using tickets.
Back to Top