Opened 12 years ago
Closed 8 years ago
#19963 closed New feature (fixed)
Add support for date_hierarchy across relations
Reported by: | 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)
Change History (17)
comment:2 by , 12 years ago
It looks like this commit did it: https://github.com/django/django/commit/e74e207cce54802f897adcb42149440ee154821e
comment:3 by , 12 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 , 12 years ago
Version: | 1.5 → 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 by , 12 years ago
Severity: | Normal → Release blocker |
---|---|
Triage Stage: | Unreviewed → Accepted |
Yeah, I was unclear, the commit involves some backwards incompatibility 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.
comment:6 by , 12 years ago
Owner: | changed from | to
---|---|
Status: | new → assigned |
I guess I'm supposed to fix my own mess...
comment:7 by , 12 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
by , 12 years ago
Attachment: | validation-of-date_hierarchy-across-relation-failing-test.diff added |
---|
comment:8 by , 12 years ago
Owner: | removed |
---|---|
Severity: | Release blocker → Normal |
Status: | assigned → new |
Summary: | date_hierarchy suddenly doesn’t support fields on related models → Add support for date_hierarchy across relations |
Type: | Bug → 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:10 by , 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 , 9 years ago
Cc: | added |
---|
comment:12 by , 9 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 , 9 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 , 9 years ago
Owner: | set to |
---|---|
Status: | new → assigned |
comment:15 by , 9 years ago
Has patch: | set |
---|
Submitted a PR: https://github.com/django/django/pull/6563