[Django] #24377: UUIDField as primary key breaks inline admins
Django
---------------------------------+-----------------------
Reporter: yoyoma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8alpha1
Severity: Release blocker | Keywords:
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
---------------------------------+-----------------------
{{{
class Parent(models.Model):
uuid = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
name = models.CharField(max_length=255)
class Child(models.Model):
uuid = models.UUIDField(primary_key=True, default=uuid.uuid4,
editable=False)
name = models.CharField(max_length=255)
parent = models.ForeignKey(Parent)
}}}
{{{
class ChildInline(admin.TabularInline):
model = Child
class ParentAdmin(admin.ModelAdmin):
inlines = [ChildInline]
}}}
== Given the above models and admins: ==
If you attempt to create a {{{Parent}}}, all the {{{Child}}} fields will
contain {{{This field is required.}}} errors.
I think the issue here is {{{default=uuid.uuid4}}}, and I think what's
happening is, since the {{{uuid4}}} value provided to the {{{uuid}}} field
on page load will differ from the {{{uuid4}}} value provided when the form
is posted, Django is considering the state as having been changed
(resulting in validation).
I've set the Component as Forms, since this is most likely a form issue,
rather than being admin-specific. I've also set the Severity to Release
Blocker, since the last {{{UUIDField}}} issue I encountered was marked as
such, and since this sort of breaks the (probably) most common usage of
{{{UUIDField}}}. Please let me know if I should not set the Severity in
the future.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: yoyoma | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8alpha1
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yoyoma):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Ok, so pertaining my crackpot theory of why this issue occurs, you'll have
to make allowances because I'm sleep deprived. Obviously, the issue is not
caused by Django somehow knowing the original GET version of the
{{{uuid4}}} value when you POST the form over... stateless HTTP.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:1>
Django
Reporter: yoyoma | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Unreviewed
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by yoyoma):
* version: 1.8alpha1 => master
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:2>
Django
Reporter: yoyoma | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by claudep):
Could be a duplicate of #15665…
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:3>
Django
Reporter: yoyoma | Owner: nobody
Type: Bug | Status: new
Severity: Release blocker | Resolution:
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by mjtamlyn):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:4>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* status: new => assigned
* owner: nobody => timgraham
Comment:
There appear to be a couple problems here:
Since the use of `UUIDField` as a primary key requires
`default=uuid.uuid4`, initializing an object sets its primary key in
[https://github.com/django/django/blob/bed504d70bede3431a213203c13a33905d6dbf77/django/db/models/base.py#L426-L427
Model.__init__()]:
{{{
>>> p = Parent()
>>> p.pk
UUID('cde58b1c-4a5a-471a-901c-6e43847f0a0e')
}}}
That value is then used as the initial value for
[https://github.com/django/django/blob/bed504d70bede3431a213203c13a33905d6dbf77/django/forms/models.py#L931
InlineForeignKeyField] on the inlines. It needs to be ignored since it
will be regenerated on subsequent requests since the field isn't editable
and there's no way to preserve state across requests.
Secondly, the primary key on the inline uses an auto-generated value from
the `default` of the `Child.uuid` field during the save request, but it's
empty on the request for the initial form. This discrepancy causes
`Form.has_changed()` to return `True` which triggers "required" validation
on other fields, even on otherwise empty inlines.
I have a patch that seems to fix these issues and am working on adding
tests.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:5>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | 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
Comment:
[https://github.com/django/django/pull/4176 PR]
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:6>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 0 => 1
Comment:
Unfortunately, there is a failing test for a case involving model
inheritance and editable primary keys.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:7>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 1
Comment (by yoyoma):
@timgraham - I tried to debug this some and ended up down a deep rabbit
hole.
I ended up '''possibly''' finding another bug. With stable/1.8.x (without
your changes), given the example models, if you create a {{{Child}}}
instance for an existing {{{Parent}}}, set {{{max_num = 1}}} on the inline
(just to avoid empty forms, since you only created 1 child), then visit
the change view for that {{{Parent}}} and save it, all should be well, but
all is not well. The global {{{Please correct the errors below.}}} appears
at the top, but there are no errors. Removing the inline admin fixes that
issue, and checking "delete" on the inline instance is not met with this
issue, so the issue is certainly pertaining validation of the inline form
or instance.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:8>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* needs_better_patch: 1 => 0
Comment:
Tests are passing on my pull request now.
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:9>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: master
Severity: Release blocker | Resolution:
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by yoyoma):
@timgraham - see also #24391 (pertains my original theory of the cause of
this, which now seems more relevant)
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:10>
Django
Reporter: yoyoma | Owner: timgraham
Component: Forms | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
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:"1306cd1e8acfb13602ee8dc40993b2505cd7523b"]:
{{{
#!CommitTicketReference repository=""
revision="1306cd1e8acfb13602ee8dc40993b2505cd7523b"
Fixed #24377 -- Fixed model inline formsets with primary key's that have
defaults.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:11>
Django
Reporter: yoyoma | Owner: timgraham
Type: Bug | Status: closed
Component: Forms | Version: master
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"41d5ed480cd55a71b739e779ca11f24caaa2b302"]:
{{{
#!CommitTicketReference repository=""
revision="41d5ed480cd55a71b739e779ca11f24caaa2b302"
[1.8.x] Fixed #24377 -- Fixed model inline formsets with primary key's
that have defaults.
Backport of 1306cd1e8acfb13602ee8dc40993b2505cd7523b from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:12>
Django
Reporter: Michael Angeletti | Owner: Tim Graham
Type: Bug | Status: closed
Component: Forms | Version: dev
Severity: Release blocker | Resolution: fixed
Keywords: | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Mariusz Felisiak <felisiak.mariusz@…>):
In [changeset:"b91d62cca07638741f5902713983f71478589b0e" b91d62c]:
{{{
#!CommitTicketReference repository=""
revision="b91d62cca07638741f5902713983f71478589b0e"
Refs #24377 -- Added assertions for model inlines with primary key that
has a default.
This ensures that a model field default is ignored.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24377#comment:13>