Code

Opened 17 months ago

Last modified 10 months ago

#19963 new New feature

Add support for date_hierarchy across relations

Reported by: jbg@… Owned by:
Component: contrib.admin Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

I'm not sure exactly when this changed, but I think it was in the last few weeks of development leading up to the 1.5 release.

ModelAdmin.date_hierarchy used to support fields on related models. For example, I have a model Event with a day field, which is a ForeignKey to Day, which in turn has a date field (of type DateField).

This used to work:

class EventAdmin(ModelAdmin):
    date_hierarchy = 'day__date'

Suddenly, as of a recent commit, it doesn't. This isn't addressed in the documentation in any way I can find.

Attachments (1)

validation-of-date_hierarchy-across-relation-failing-test.diff (959 bytes) - added by aaugustin 16 months ago.

Download all attachments as: .zip

Change History (11)

comment:1 Changed 17 months ago by jbg@…

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
2013-03-02 11:34:28,310 while handling request
Traceback (most recent call last):
  File "/home/.../django/django/db/models/options.py", line 341, in get_field_by_name
    return self._name_map[name]
KeyError: 'day__date'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/.../django/django/core/handlers/base.py", line 130, in get_response
    response = response.render()
  File "/home/.../django/django/template/response.py", line 105, in render
    self.content = self.rendered_content
  File "/home/.../django/django/template/response.py", line 82, in rendered_content
    content = template.render(context)
  File "/home/.../django/django/template/base.py", line 140, in render
    return self._render(context)
  File "/home/.../django/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)
  File "/home/.../django/django/template/base.py", line 837, in render
    bit = self.render_node(node, context)
  File "/home/.../django/django/template/base.py", line 851, in render_node
    return node.render(context)
  File "/home/.../django/django/template/loader_tags.py", line 123, in render
    return compiled_parent._render(context)
  File "/home/.../django/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)
  File "/home/.../django/django/template/base.py", line 837, in render
    bit = self.render_node(node, context)
  File "/home/.../django/django/template/base.py", line 851, in render_node
    return node.render(context)
  File "/home/.../django/django/template/loader_tags.py", line 123, in render
    return compiled_parent._render(context)
  File "/home/.../django/django/template/base.py", line 134, in _render
    return self.nodelist.render(context)
  File "/home/.../django/django/template/base.py", line 837, in render
    bit = self.render_node(node, context)
  File "/home/.../django/django/template/base.py", line 851, in render_node
    return node.render(context)
  File "/home/.../django/django/template/loader_tags.py", line 62, in render
    result = block.nodelist.render(context)
  File "/home/.../django/django/template/base.py", line 837, in render
    bit = self.render_node(node, context)
  File "/home/.../django/django/template/base.py", line 851, in render_node
    return node.render(context)
  File "/home/.../django/django/template/loader_tags.py", line 62, in render
    result = block.nodelist.render(context)
  File "/home/.../django/django/template/base.py", line 837, in render
    bit = self.render_node(node, context)
  File "/home/.../django/django/template/base.py", line 851, in render_node
    return node.render(context)
  File "/home/.../django/django/template/base.py", line 1192, in render
    _dict = func(*resolved_args, **resolved_kwargs)
  File "/home/.../django/django/contrib/admin/templatetags/admin_list.py", line 295, in date_hierarchy
    field = cl.opts.get_field_by_name(field_name)[0]
  File "/home/.../django/django/db/models/options.py", line 347, in get_field_by_name
    % (self.object_name, name))
django.db.models.fields.FieldDoesNotExist: Event has no field named 'day__date'

comment:3 Changed 17 months ago by aaugustin

This commit isn't in 1.5, only in master. Indeed, it's backwards-incompatible, as documented in the 1.6 release notes.

Are you sure the problem is in 1.5?

comment:4 Changed 17 months ago by jbg@…

  • Version changed from 1.5 to master

My apologies, I left the "Version" field at its default and was ambiguous in my description. The commit was in the last few weeks of development leading up to the 1.5 release, but you're correct, the problem exists only in master.

I don't see anywhere in the 1.6 release notes where it says that date_hierarchy no longer supports fields on related models.

comment:5 Changed 17 months ago by aaugustin

  • Severity changed from Normal to Release blocker
  • Triage Stage changed from Unreviewed to Accepted

Yeah, I was unclear, the commit involves some backwards incompatibilities but the problem you're reporting isn't part of them. I just wanted to point out that the known incompatibilities were documented.

The bug is here. Django now needs to determine if the target field is a date field or a datetime field, and this line doesn't take into account relations.

Last edited 17 months ago by aaugustin (previous) (diff)

comment:6 Changed 17 months ago by aaugustin

  • Owner changed from nobody to aaugustin
  • Status changed from new to assigned

I guess I'm supposed to fix my own mess...

comment:7 Changed 16 months ago by aaugustin

I tried using date_hierarchy across a relation and it doesn't pass the admin validation:

ImproperlyConfigured: 'ChildAdmin.date_hierarchy' refers to field 'parent__date' that is missing from model 'testapp.Child'.

Clearly it isn't a supported configuration. Validation is only performed when DEBUG = True, but that doesn't make it a supported configuration when DEBUG = False.

In fact the validation code is similar to the code I've added to look up the field:
https://github.com/django/django/blob/master/django/contrib/admin/validation.py#L136-L142

I'm attaching a failing test case.


For the record, if Django started supporting date_hierarchy across relations, the following patch would fix the problem described above:

--- a/django/contrib/admin/templatetags/admin_list.py
+++ b/django/contrib/admin/templatetags/admin_list.py
@@ -7,6 +7,7 @@ from django.contrib.admin.util import (lookup_field, display_for_field,
 from django.contrib.admin.views.main import (ALL_VAR, EMPTY_CHANGELIST_VALUE,
     ORDER_VAR, PAGE_VAR, SEARCH_VAR)
 from django.contrib.admin.templatetags.admin_static import static
+from django.contrib.admin.utils import get_fields_from_path
 from django.core.exceptions import ObjectDoesNotExist
 from django.db import models
 from django.utils import formats
@@ -292,7 +293,7 @@ def date_hierarchy(cl):
     """
     if cl.date_hierarchy:
         field_name = cl.date_hierarchy
-        field = cl.opts.get_field_by_name(field_name)[0]
+        field = get_fields_from_path(cl.model, field_name)[-1]
         dates_or_datetimes = 'datetimes' if isinstance(field, models.DateTimeField) else 'dates'
         year_field = '%s__year' % field_name
         month_field = '%s__month' % field_name

comment:8 Changed 16 months ago by aaugustin

  • Owner aaugustin deleted
  • Severity changed from Release blocker to Normal
  • Status changed from assigned to new
  • Summary changed from date_hierarchy suddenly doesn’t support fields on related models to Add support for date_hierarchy across relations
  • Type changed from Bug to New feature

Re-qualifying the ticket as a feature request — it seems useful to me.

Unless unexpected problems crop up, this means:

  • upgrading the validation code,
  • adding the validation tests I just uploaded,
  • applying the diff I pasted above,
  • adding tests,
  • adding docs.

comment:9 Changed 16 months ago by julien

Aymeric, your plan sounds like the right way to go.

comment:10 Changed 10 months ago by jbg@…

This is purely a "practicality" suggestion, but given that this used to work, and it doesn't look like it will work in 1.6 final, it may be worth mentioning in the 1.6 release notes? Despite the fact that it was an unsupported configuration and was not mentioned in the docs (as far as I can tell), it worked fine, and if I'd been using it for months (after assuming it would work, trying it out, and finding that it did work) then I imagine other people might have been too.

So even just mentioning in the release notes, perhaps alongside the other date-related backwards incompatibilities, that it no longer works in 1.6 might save a bunch of people a headache!

Add Comment

Modify Ticket

Change Properties
<Author field>
Action
as new
The owner will be changed from (none) to anonymous. Next status will be 'assigned'
as The resolution will be set. Next status will be 'closed'
Author


E-mail address and user name can be saved in the Preferences.

 
Note: See TracTickets for help on using tickets.