Cannot reference FK relation from inline ModelForm.save()

[Stanislas Guerra]

Hi devs,

Not sure that's a bug but that's a regression. In Django-1.7 with the following models:

from django.db import models


class Author(models.Model):
    name = models.TextField()


class Book(models.Model):
    author = models.ForeignKey("Author")
    title = models.TextField()

You can access book.author in the ModelForm.save() method:
admin.py:

from django import forms
from django.contrib import admin
from library import models


class BookForm(forms.ModelForm):
    class Meta:
        model = models.Book
        exclude = ()

    def save(self, *args, **kwargs):
        book = super(BookForm, self).save(*args, **kwargs)
        book.title = "%s by %s" % (book.title, book.author.name)
        return book


class BookInline(admin.TabularInline):
    model = models.Book
    extra = 1
    form = BookForm


class AuthorAdmin(admin.ModelAdmin):
    inlines = [BookInline]


admin.site.register(models.Author, AuthorAdmin)

And since commit ​45e049937d you get a RelatedObjectDoesNotExist exception: Book has no author.

Is it considered a normal behaviour?

Thanks.

[Stanislas Guerra]

Repo of a demo project: ​https://github.com/Starou/django-ticket24325

[Stanislas Guerra]

A small clarification: This exception happens when you add a new Author and create a Book with the inline in the same request.

[Tim Graham]

This might need to be documented on a backwards incompatible change, but I haven't fully investigated the issue.

[Tim Graham]

Hi Starou, could you take a look at the attached documentation note and let me know if addresses your issue?

[Stanislas Guerra]

Hi Tim,

I am affraid not. In the Book/Author example yes but not in my project because I use the pk of the parent object to create a path to store some images on the file system. And the pk isn't in the cleaned-data.

Here a pseudo-function to illustrate (imagine Book having a cover ImageField attribute with upload_to parameter bound to the next):

def image_upload_path(instance, filename):
    return "authors/%d/books/%d/%s" % (
        instance.author.pk,
        instance.pk,
        filename
    )

I agree with the concept of not binding an unsaved object in a relation but I think the exception is raised because of the parent instance being saved later in the process than before commit @5e049937d (because if you get pk, the object was saved, right?)
I am not experienced enough with the Django code to be 100% positive, just an intuition :-)

[Tim Graham]

That code isn't working on 1.7 for me:

def image_upload_path(instance, filename):
    return "authors/%d/books/%d/%s" % (
        instance.author.pk,
        instance.pk,
        filename
    )

class Author(models.Model):
    name = models.TextField()

class Book(models.Model):
    author = models.ForeignKey("Author")
    title = models.TextField()
    cover = models.ImageField(blank=True, upload_to=image_upload_path)

Exception:

TypeError at /admin/library/author/add/

%d format: a number is required, not NoneType

because instance.pk is None.

This is ​as documented: "In most cases, this object will not have been saved to the database yet, so if it uses the default AutoField, it might not yet have a value for its primary key field."

[Stanislas Guerra]

You are rigtht about the ImageField. The code in my project is a bit more complex than that with some nested forms and inlines etc and it runs under 1.7 and not 1.8.

I will investigate next week if I can find some time.

By the way, using book.author.pk instead of book.author.name in my previous demonstration gives the same result (works before commit @5e049937d)

Thank you for your time!

[Tim Graham]

You're welcome. I'll commit the doc patch and close this. Please open a new ticket if you find something else.

[Tim Graham <timograham@…>]

In 0af3822dc362b6253bda1c9699466dd0bbbf6066:

Fixed #24325 -- Documented change in ModelForm.save() foreign key access.

[Tim Graham <timograham@…>]

In 8657e7caaac41266d9ac0b73a21af64edc681613:

[1.8.x] Fixed #24325 -- Documented change in ModelForm.save() foreign key access.

Backport of 0af3822dc362b6253bda1c9699466dd0bbbf6066 from master

[Tim Graham <timograham@…>]

In d298b1ba5043eaa40f3f4bebe3c7634b359ba34b:

Reverted "Fixed #24325 -- Documented change in ModelForm.save() foreign key access."

This reverts commit 0af3822dc362b6253bda1c9699466dd0bbbf6066.
It's obsoleted by refs #24395.

[Tim Graham <timograham@…>]

In 81911f29b70710d8f72916c49a69aa5df5cd7df8:

[1.8.x] Reverted "Fixed #24325 -- Documented change in ModelForm.save() foreign key access."

This reverts commit 0af3822dc362b6253bda1c9699466dd0bbbf6066.
It's obsoleted by refs #24395.

Backport of d298b1ba5043eaa40f3f4bebe3c7634b359ba34b from master