[Django] #26449: formfield_overrides doesn't work for models.DateTimeField with SplitDateTimeWidget
Django
SplitDateTimeWidget
-------------------------------+--------------------
Reporter: MarysiaLowas | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.9
Severity: Normal | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+--------------------
When you override the default widget used in Django Admin for
models.DateTimeField using formfield_overrides with any widget that
derives from SplitDateTimeWidget, the form will not pass the validation
with 'Enter a valid date/time.' error message.
It seems that when updating the default formfield_overrides dict, it
overrides the default values instead of merging them. At present, it seems
to affect only the DateTimeField.
So, this code will trigger the error: 'Enter a valid date/time.'
formfield_overrides = {models.DateTimeField:
{widgets.AdminSplitDateTime},}
And this code will be fine:
formfield_overrides = {models.DateTimeField: {'form_class':
forms.SplitDateTimeField,'widget': widgets.AdminSplitDateTime},}
The problem in django/contrib/admin/option.py around line 117.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: nobody
Type: Uncategorized | Status: new
Component: contrib.admin | Version: 1.9
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* needs_better_patch: => 0
* stage: Unreviewed => Accepted
* needs_tests: => 0
* needs_docs: => 0
Old description:
> When you override the default widget used in Django Admin for
> models.DateTimeField using formfield_overrides with any widget that
> derives from SplitDateTimeWidget, the form will not pass the validation
> with 'Enter a valid date/time.' error message.
>
> It seems that when updating the default formfield_overrides dict, it
> overrides the default values instead of merging them. At present, it
> seems to affect only the DateTimeField.
>
> So, this code will trigger the error: 'Enter a valid date/time.'
> formfield_overrides = {models.DateTimeField:
> {widgets.AdminSplitDateTime},}
>
> And this code will be fine:
> formfield_overrides = {models.DateTimeField: {'form_class':
> forms.SplitDateTimeField,'widget': widgets.AdminSplitDateTime},}
>
> The problem in django/contrib/admin/option.py around line 117.
New description:
When you override the default widget used in Django Admin for
models.DateTimeField using formfield_overrides with any widget that
derives from SplitDateTimeWidget, the form will not pass the validation
with 'Enter a valid date/time.' error message.
It seems that when updating the default formfield_overrides dict, it
overrides the default values instead of merging them. At present, it seems
to affect only the DateTimeField.
So, this code will trigger the error: 'Enter a valid date/time.'
formfield_overrides = {models.DateTimeField: {'widgets':
widgets.AdminSplitDateTime},}
And this code will be fine:
formfield_overrides = {models.DateTimeField: {'form_class':
forms.SplitDateTimeField,'widget': widgets.AdminSplitDateTime},}
The problem in django/contrib/admin/option.py around line 117.
--
Comment:
Tested according to the OP description (minor edit made to the
description). I'm able to reproduce this on 1.9 and master. 1.8 is not
affeced
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:1>
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: yakky
Type: Uncategorized | Status: assigned
Component: contrib.admin | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* owner: nobody => yakky
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:2>
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: yakky
Type: Uncategorized | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yakky):
After a bit of digging, this is actually a legitimate behavior due to this
deprecation
https://github.com/django/django/blob/stable/1.8.x/django/forms/fields.py#L507
which went into effect in 1.9 and it's documented in 1.9 release notes.
That said the resulting error is kind of misleading, but being
`AdminSplitDateTime` undocumented there is actually little way to overcome
this.
How about adding a documentation on how to use the `AdminSplitDateTime`
widget in its docstring?
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:3>
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: yakky
Type: Uncategorized | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yakky):
After discussing this with Markus Holtermann we can either provide a bit
more detailed information in the release notes with a simple sentence how
eventually adapt the code, or do nothing at all
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:4>
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: contrib.admin | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* type: Uncategorized => Cleanup/optimization
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:5>
Django
SplitDateTimeWidget
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yakky):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:6>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 1 => 0
* component: contrib.admin => Documentation
Comment:
On further reflection, I don't think there's any mention in the
documentation (I'm looking in `docs/ref/forms/widgets.txt`) that
`SplitDateTimeWidget` must be used with `SplitDateTimeField`. A note there
seems like a better course of action rather than repeating requirements of
the superclass in a subclass's docstring.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:7>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yakky):
timgraham should I also leave the note in AdminSplitDateTime, or should I
remove it?
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:8>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I don't think it's needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:9>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarysiaLowas):
I would like to clarify one thing as I'm not sure I understand the
solution.
Django Admin, by default uses the `AdminSplitDateTime` widget for model
`DateTimeField` with `forms.SplitDateTimeField`. However, when I try to
change the default widget with `formfield_overrides` to another widget
derived from `SplitDateTimeWidget`, it doesn't work -> validation error.
This is especially striking, when I try to override the default
`AdminSplitDateTime` widget with the very same `AdminSplitDateTime`
widget. (That was just for tests, in fact I wanted to use Django Suit
`suit.widgets.SuitSplitDateTimeWidget`)
From my investigation, it turned out that Django Admin treats model
`DateTimeField` a bit specially. It defines not only the default widget
but also the `form_class`.
{{{#!python
FORMFIELD_FOR_DBFIELD_DEFAULTS = {
models.DateTimeField: {
'form_class': forms.SplitDateTimeField,
'widget': widgets.AdminSplitDateTime
},
models.DateField: {'widget': widgets.AdminDateWidget},
models.TimeField: {'widget': widgets.AdminTimeWidget},
#...
}
}}}
It works fine, but when I want to override it with:
{{{#!python
formfield_overrides = {
models.DateTimeField: {'widget':
suit.widgets.SuitSplitDateTimeWidget},
}
}}}
The resulting dictionary looks something like that:
{{{#!python
overrides = {
models.DateTimeField: { 'widget':
suit.widgets.SuitSplitDateTimeWidget},
models.DateField: {'widget': widgets.AdminDateWidget},
models.TimeField: {'widget': widgets.AdminTimeWidget},
#...
}
}}}
The `form_class` is gone as so the `forms.DateTimeField` is used instead
of `forms.SplitDateTimeField` and that causes the validation error.
Is it a desired behaviour?
If yes, maybe it's good to update also the `formfield_overrides`
documentation, not just the docstring for `AdminSplitDateTime` widget?
Or maybe it's better to keep the `form_class` value and override just
`widget`?
BTW, I planned to look into this but I run out of time during Sprints. I'm
still quite happy to carry on if needed.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:10>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
I didn't look at the code much, but your description of the issue reminds
me of #12915. Could be related.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:11>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yakky):
The behavior described in this ticket is due to this deprecation
https://github.com/django/django/blob/stable/1.8.x/django/forms/fields.py#L507
, thus it's an intended behavior
The only possible fix here is to document better how to use
`formfield_overrides` for `AdminSplitDateTime`
`AdminSplitDateTime` per se it's not documented as is considered part of
the private API
We can document `SplitDateTimeWidget` but this won't solve this ticket
anyway
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:12>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by MarysiaLowas):
@timgraham That ticket you mentioned is about inheriting and this issue is
about merging dictionaries.
@yakky It's not so much about `AdminSplitDateTime` as about
`formfield_overrides` for `DateTimeField`. I wanted to use
`formfield_overrides` to override the default widget for `DateTimeField`,
and I was doing it according to the published documentation (not the
inline docs in the code), and I got an error.
Please see the PR I created which should solve this problem
[https://github.com/django/django/pull/6553 PR for #26449 ]
This is my first contribution to Django, so please forgive me if I
happened not to follow the procedures or custom at any point. That was
unconsciously done.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:13>
Django
--------------------------------------+------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: Cleanup/optimization | Status: assigned
Component: Documentation | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Accepted
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:14>
Django
-------------------------------------+-------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Type: | Status: assigned
Cleanup/optimization |
Component: contrib.admin | Version: 1.9
Severity: Normal | Resolution:
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Documentation => contrib.admin
* stage: Accepted => Ready for checkin
Comment:
Makes sense. For future reference, don't forget to check "Has patch" so
the ticket appears in the review queue.
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:15>
Django
-------------------------------------+-------------------------------------
Reporter: MarysiaLowas | Owner: yakky
Cleanup/optimization |
Component: contrib.admin | Version: 1.9
Severity: Normal | Resolution: fixed
Keywords: | Triage Stage: Ready for
| checkin
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Tim Graham <timograham@…>):
* status: assigned => closed
* resolution: => fixed
Comment:
In [changeset:"b9290b1d49538c1092b59c41e6046b11c25ecafb" b9290b1d]:
{{{
#!CommitTicketReference repository=""
revision="b9290b1d49538c1092b59c41e6046b11c25ecafb"
Fixed #26449 -- Merged admin's FORMFIELD_FOR_DBFIELD_DEFAULTS with
formfield_overrides.
Useful for overriding the DateTimeField widget.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/26449#comment:16>