Unable to re-save inlines with custom char primary key

[marcob]

Define a simple models.py like this one:

from django.db import models

class Master(models.Model):
    codice = models.CharField(
            primary_key=True,
            max_length=30,
            )

class Detail(models.Model):
    codice = models.CharField(
            primary_key=True,
            max_length=30,
            )
    fk = models.ForeignKey(Master)

And an admin.py like this one:

from django.contrib import admin
from models import Master, Detail

class DetailInline(admin.TabularInline):
    model = Detail

class MasterAdmin(admin.ModelAdmin):
    inlines = [DetailInline,]

admin.site.register(Master, MasterAdmin)

Create and save a Master record with a Detail inline record.
Reopen the Master record, try to save it again.
Bang!

AttributeError at /admin/pluto/master/test/
'unicode' object has no attribute 'pk'
Exception Location: 	C:\work\esempio\lib\django\forms\models.py in save_existing_objects, line 521

I fixed it changing line 521:

            try:
                pk_value = form.fields[pk_name].clean(raw_pk_value).pk
            except AttributeError:
                pk_value = form.fields[pk_name].clean(raw_pk_value)

[Russell Keith-Magee]

Verified that the problem exists, but the suggested fix doesn't work - it may fix the problem for charfield PK's, but it breaks all other cases (and the test suite verifies this).

[marcob]

Russell, sorry for not having attached a patch, but the fix "runs" only with an Attribute Error Exception. So it's impossible it can break other cases.
You have to substitute line 521 with this 4 lines:

  try:
    pk_value = form.fields[pk_name].clean(raw_pk_value).pk
  except AttributeError:
    pk_value = form.fields[pk_name].clean(raw_pk_value)

Btw I do agree this is a workaround. We need to fix clean (with charfield it doesn't return the instance).

[Russell Keith-Magee]

Apologies Marco - I misread your suggestion as just removing the .pk portion. However, you are correct - this is a workaround, not a solution.

[Zain Memon]

When the pk isn't explicitly specified in the model, it is rendered with a ModelChoiceAdmin field. But when it is explicitly specified, it's rendered with the field for the data type of the pk (CharField in this case).

ModelChoiceAdmin's clean() method returns the model object instance, while CharField's clean() method returns a char. Thus the bug.

Fix incoming.

[Zain Memon]

Tests demonstrating the bug

[Zain Memon]

Fix for this bug

[Russell Keith-Magee]

General approach to this problem looks good, but two problems with the patch:

1. It breaks the modeltests.model_formsets unit test suite.
2. The patch removes the use of "existing_objects". This exists as an optimization - the call to get_queryset() means that all inline objects can be retrieved with a single SQL statement, whereas the patch changes this so that each inline object requires a independent SQL select.

[Russell Keith-Magee]

I committed (commented out) a slightly more robust version of the provided test case as a part of [10725]. If anyone gets around to looking at this before I do, the new patch just needs to uncomment the appropriate tests.

[Zain Memon]

Apparently there are cases other than ModelChoiceAdmin where the clean function will return a model instance (as demonstrated by the failing model_formsets test case), so I'm just checking for the existence of a pk attribute and using it if it exists. Also, uncommented your test case.

[Zain Memon]

Fix incorporating russellm's suggestions in comment 8 + uncommenting the tests from [10725]

[Jacob]

(In [10777]) Fixed #10992: fixed a bug saving inlines with custom primary key fields. Thanks, Zain.

[Jacob]

Milestone 1.1 deleted