Opened 11 years ago

Closed 8 years ago

#19963 closed New feature (fixed)

Add support for date_hierarchy across relations

Reported by: jbg@… Owned by: Vytis Banaitis
Component: contrib.admin Version: dev
Severity: Normal Keywords:
Cc: hugo@… Triage Stage: Accepted
Has patch: yes 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 Aymeric Augustin 11 years ago.

Download all attachments as: .zip

Change History (17)

comment:1 by jbg@…, 11 years ago

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 by Aymeric Augustin, 11 years ago

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 by jbg@…, 11 years ago

Version: 1.5master

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 by Aymeric Augustin, 11 years ago

Severity: NormalRelease blocker
Triage Stage: UnreviewedAccepted

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 11 years ago by Aymeric Augustin (previous) (diff)

comment:6 by Aymeric Augustin, 11 years ago

Owner: changed from nobody to Aymeric Augustin
Status: newassigned

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

comment:7 by Aymeric Augustin, 11 years ago

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 by Aymeric Augustin, 11 years ago

Owner: Aymeric Augustin removed
Severity: Release blockerNormal
Status: assignednew
Summary: date_hierarchy suddenly doesn’t support fields on related modelsAdd support for date_hierarchy across relations
Type: BugNew 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 by Julien Phalip, 11 years ago

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

comment:10 by jbg@…, 11 years ago

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!

comment:11 by Hugo Osvaldo Barrera, 9 years ago

Cc: hugo@… added

comment:12 by Hannes Tismer, 8 years ago

This ticket is fairly old. I've just discovered that it is still valid for Django 1.8. Any plans on reimplementing the RelatedField/ForeignKey feature for date_hierarchy?

Thanks

comment:13 by Tim Graham, 8 years ago

No, there aren't any plans outside of the discussion in this ticket, so feel free to take ownership and provide a patch.

comment:14 by Vytis Banaitis, 8 years ago

Owner: set to Vytis Banaitis
Status: newassigned

comment:15 by Vytis Banaitis, 8 years ago

Has patch: set

comment:16 by Tim Graham <timograham@…>, 8 years ago

Resolution: fixed
Status: assignedclosed

In 2f9c4e2b:

Fixed #19963 -- Added support for date_hierarchy across relations.

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