"Please correct the error below." when saving add model form with inline formset and no auto primary key.

[Chris.Wesseling@…]

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 ​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.

[Chris.Wesseling@…]

Patch containing faulty bits of changeset 12206. patch using -R or --reverse

[Chris.Wesseling@…]

Testcase testing django.contrib.admin

[Natalia Bidart]

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.

[Natalia Bidart]

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.

[Natalia Bidart]

Running tests with:

(django-sprint)nessita@miro:~/pycon/fix_12749$ ./manage.py test primary_key_inline

exercises this issue.

[jkocherhans]

Pull the code from nessita's patches into an extra test case for admin_inlines.

[jkocherhans]

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.

[anonymous]

Replying to 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.

[Russell Keith-Magee]

@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.

[Russell Keith-Magee]

Draft fix; passes nessita's test, but breaks others in model_formsets

[Russell Keith-Magee]

Updated, fully working patch.

[Russell Keith-Magee]

(In [13034]) Fixed #12749 -- Corrected a problem with validation of inline primary keys. Thanks to Chris.Wesseling@… for the report, and nessita for the test case.

[anonymous]

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? :/

[Luke Plant]

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.

[Jacob]

Milestone 1.2 deleted