[Django] #12749: "Please correct the error below." when saving add model form with inline formset and no auto primary key.

522 views
Skip to first unread message

Django

unread,
Feb 1, 2010, 11:58:01 AM2/1/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
------------------------------------+---------------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nobody
Status: new | Milestone:
Component: django.contrib.admin | Version: SVN
Keywords: | Stage: Unreviewed
Has_patch: 1 |
------------------------------------+---------------------------------------
When a Model has a field with primary_key=True and the Admin has an Inline
ManyToMany, trying to save will result in "Please correct the error
below." with no errors shown.

This has been introduced in [12206].
Just the changes in django/contrib/admin/options.py to be precise.
I expected reverting the patch would result in failing tests. But no tests
failed.
So either [12206] contained noise code that was doing nothing or it needed
better tests.

I've written a test for my usecase that was broken by [12206] and I
suggest that someone that knows the purpose
of [http://code.djangoproject.com/changeset/12206#file0 the options.py
part of 12206] expresses that purpose in a test.

I'll add both my test and a patch containing part of the faulty changeset,
but triagers, this ticket might need a better patch.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749>
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

Django

unread,
Feb 7, 2010, 10:58:46 AM2/7/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Unreviewed | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by jezdez):

* needs_better_patch: => 0
* needs_docs: => 0
* needs_tests: => 0
* milestone: => 1.2

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:1>

Django

unread,
Feb 10, 2010, 7:19:07 AM2/10/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by russellm):

* stage: Unreviewed => Accepted

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:2>

Django

unread,
Feb 23, 2010, 12:31:57 PM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nessita
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by nessita):

* owner: nobody => nessita
* status: new => assigned

Comment:

This ticket has two sub-problems in it:

* {{{django/contrib/admin/templates/admin/change_form.html}}} should not
iterate over the {{{adminform.form.non_field_errors}}} and should not
build the {{{<ul>}}} by its own, but use the {{{{{ non_field_errors }}}}}
directly instead. We may need to check the rest of the forms for this
problem. I'll be doing the follow up and submitting patches to ticket
#11681.

* For this particular example, I'm also getting the error "Model
fashionista with pk 1 does not exist.", I have to investigate this
further.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:3>

Django

unread,
Feb 23, 2010, 5:00:57 PM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nessita
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Comment (by nessita):

I've reproduced the problem using the attached models.py and tests.py
files.
After a lot of (a little bit frustrating) debugging, I've come up to the
following conclusions:

* The design presented by the reporter doesn't seem to make sense, since
it generates a mutual dependency between the model ''Fashionista'' and
''ShoppingWeakness'': to create a ''Fashionista'', an instance of
''ShoppingWeakness'' is needed; and to create the latter, an instance of
''Fashionista'' is needed.

* The attempt to create a ''Fashionista'' ends up with the error
{{{Model fashionista with pk 1 does not exist}}}
but this error is never shown in the admin's change_form.html, so the
user only sees the {{{Please correct the error below}}} message and
nothing else.

* When printing the {{{{{ errors }}}}} value in the change_form.html
template, there seems to be a buggy generated HTML, as per the screenshot
attached.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:4>

Django

unread,
Feb 23, 2010, 5:06:37 PM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nessita
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Comment (by nessita):

Running tests with:
{{{
(django-sprint)nessita@miro:~/pycon/fix_12749$ ./manage.py test
primary_key_inline
}}}
exercises this issue.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:5>

Django

unread,
Feb 23, 2010, 5:07:09 PM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by nessita):

* owner: nessita => nobody
* status: assigned => new

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:6>

Django

unread,
Feb 23, 2010, 6:49:36 PM2/23/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: nobody
Status: new | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by nessita):

* cc: nessita (added)

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:7>

Django

unread,
Mar 6, 2010, 4:45:05 PM3/6/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: jkocherhans
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by jkocherhans):

* owner: nobody => jkocherhans
* status: new => assigned

Comment:

I'm not quite sure how to fix this yet, but the fix should probably be in
`ForeignKey.validate()`. Generally, when we're adding inline objects, the
value of the FK to the parent object will be `None` since the parent
hasn't been saved yet. If the value is `None`, the `ForeignKey` validation
is skipped. However, in this case, the `ForeignKey` has a value of 1 since
the Person object has already been saved. Person isn't the *real* parent
object, but the real parent's primary key is a `ForeignKey` to Person.

[12206] just rolled back part of model validation, but it was the part of
model validation that allowed cases like this to pass. Reverting that
isn't an option.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:8>

Django

unread,
Mar 31, 2010, 8:22:08 AM3/31/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: jkocherhans
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Comment (by anonymous):

Replying to [comment:4 nessita]:
> * The design presented by the reporter doesn't seem to make sense,
since it generates a mutual dependency between the model ''Fashionista''
and ''ShoppingWeakness'': to create a ''Fashionista'', an instance of
''ShoppingWeakness'' is needed; and to create the latter, an instance of
''Fashionista'' is needed.

It's legal to create a Fashionista without !ShoppingWeakness (blank=True).
In fact, that's the only way to do it now: save the Fashionista, then
apply Weaknesses. The UI shortcut of having inline forms for the
Weaknesses is just broken.

This Unittest was "designed" just to point that out.

As for your little frustration.. I really relate to that. I bumped into
this
after some time without updating trunk. After a ''lot'' of weeding and
searching, I could boil it down to this example and part of changeset.

Fixing it was difficult cause the intention of [12206] wasn't really clear
to
me, and not fully documented/locked in a unittest.

It would still be very helpful if all of the model validation would have a
proper unittest so a rollback would break tests.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:9>

Django

unread,
Apr 10, 2010, 6:23:41 AM4/10/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: jkocherhans
Status: closed | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: fixed | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 0 |
---------------------------------------------+------------------------------
Changes (by anonymous):

* status: assigned => closed
* resolution: => fixed

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:10>

Django

unread,
Apr 25, 2010, 12:54:12 PM4/25/10
to djang...@holovaty.com, django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
---------------------------------------------+------------------------------
Reporter: Chris.W...@cwi.nl | Owner: jkocherhans
Status: assigned | Milestone: 1.2
Component: django.contrib.admin | Version: SVN
Resolution: | Keywords:
Stage: Accepted | Has_patch: 1
Needs_docs: 0 | Needs_tests: 0
Needs_better_patch: 1 |
---------------------------------------------+------------------------------
Changes (by russellm):

* needs_better_patch: 0 => 1

Comment:

@jkocherhans - I'm not completely certain ForeignKey.validate() is at
fault here.

You can make the reported failure go away by adding the following check:
{{{
if self.rel.to._meta.get_field(self.rel.field_name).rel:
return
}}}
to ForeignKey.validate(). The #12507-related comment indicates that the
'value not supplied' case is usually when saving new inlines; this snippet
catches the case of an inline where the inlined foreign key points to an
object whose primary key is a OneToOneField or ForeignKey.

However, I think this reveals a deeper problem with validation. The
current {{{ if value is None: return }}} is applied to *every* foreign
key, not just the ones in inlines. This isn't an immediate problem for the
Null case, since the only additional validation that is performed is that
the related object actually exists. However, it isn't true to say that
every foreign key to a RelatedField can be handled in the same way. The
need to skip validation for inlines is the exception not the rule.

It seems to me that the problem is a layer higher up. Regardless of the
value, it's the inlined foreign key that shouldn't ever be validated. I
think the problem lies in _get_validation_exclusions() not identifying an
inlined foreign key as something that should be excluded from validation
when creating a new object. I've attached a sample implementation that
passes the provided test case; however, this breaks 3 other cases in
model_formset. I need to dig deeper to work out why, but it's getting late
and I can't see the problem right now.
Django <http://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

--
You received this message because you are subscribed to the Google Groups "Django updates" group.
To post to this group, send email to django-...@googlegroups.com.
To unsubscribe from this group, send email to django-update...@googlegroups.com.
For more options, visit this group at http://groups.google.com/group/django-updates?hl=en.

Django

unread,
May 4, 2011, 9:19:51 PM5/4/11
to django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
-------------------------------------+-------------------------------------
Reporter: | Owner: jkocherhans
Chris.Wesseling@… | Status: reopened
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: 1.2 | Keywords:
Version: SVN | Has patch: 1
Resolution: | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Changes (by anonymous):

* status: closed => reopened
* type: => Uncategorized
* resolution: fixed =>
* severity: => Normal
* easy: => 0


Comment:

i'm using Django 1.2.5 and experiencing the same. My model consists of:

{{{
Name = models.CharField(unique=True, db_index=True, max_length=255)
Outdoor = models.BooleanField(default=False, blank=True)
Floor = models.IntegerField(null=True, blank=True)
Disabled = models.BooleanField(default=False, blank=True)
}}}

my LocationAdmin(admin.ModelAdmin) consists of:

{{{
list_display = ('Name','Floor','Outdoor','Disabled')
list_editable = ('Name','Floor','Outdoor','Disabled')
}}}

When I try to tick Disabled or edit anything else in the Admin interface,
clicking on save just shows: "Please correct the errors below." and no
errors are given

Wasn't this supposed to have been fixed for 1.2? :/

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:12>

Django

unread,
May 4, 2011, 9:48:44 PM5/4/11
to django-...@googlegroups.com
#12749: "Please correct the error below." when saving add model form with inline
formset and no auto primary key.
-------------------------------------+-------------------------------------
Reporter: | Owner: jkocherhans
Chris.Wesseling@… | Status: closed
Type: | Component: contrib.admin
Uncategorized | Severity: Normal
Milestone: 1.2 | Keywords:
Version: SVN | Has patch: 1
Resolution: fixed | Needs tests: 0
Triage Stage: Accepted | Easy pickings: 0
Needs documentation: 0 |
Patch needs improvement: 1 |
-------------------------------------+-------------------------------------
Changes (by lukeplant):

* status: reopened => closed
* resolution: => fixed


Comment:

Your bug does not sound like the original bug, which was about models with
no auto primary key, and with inlines in the admin - neither of which
appear to describe your situation. Please open a new bug with enough
information to reproduce your problem.

--
Ticket URL: <http://code.djangoproject.com/ticket/12749#comment:13>

Reply all
Reply to author
Forward
0 new messages