[Django] #24325: Cannot reference FK relation from inline ModelForm.save()
Django
-------------------------------+------------------------------
Reporter: Starou | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.8alpha1
Severity: Normal | Keywords: inline ModelForm
Triage Stage: Unreviewed | Has patch: 0
Easy pickings: 0 | UI/UX: 0
-------------------------------+------------------------------
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
[https://github.com/django/django/commit/45e049937d2564d11035827ca9a9221b86215e45
45e049937d] you get a `RelatedObjectDoesNotExist` exception: Book has no
author.
Is it considered a normal behaviour?
Thanks.
--
Ticket URL: <https://code.djangoproject.com/ticket/24325>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.
Django
Reporter: Starou | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.8alpha1
Keywords: inline ModelForm | Triage Stage: Unreviewed
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by Starou):
* needs_better_patch: => 0
* needs_tests: => 0
* needs_docs: => 0
Comment:
Repo of a demo project: https://github.com/Starou/django-ticket24325
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:1>
Django
Reporter: Starou | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.8alpha1
Keywords: inline ModelForm | Triage Stage: Unreviewed
Easy pickings: 0 | UI/UX: 0
Comment (by Starou):
A small clarification: This exception happens when you add a new Author
and create a Book with the inline in the same request.
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:2>
Django
Reporter: Starou | Owner: nobody
Type: Uncategorized | Status: new
Component: Forms | Version: 1.8alpha1
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* stage: Unreviewed => Accepted
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:3>
Django
Reporter: Starou | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8alpha1
Severity: Normal | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by knbk):
* type: Uncategorized => Bug
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:4>
Django
Reporter: Starou | Owner: nobody
Type: Bug | Status: new
Component: Forms | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* severity: Normal => Release blocker
Comment:
This might need to be documented on a backwards incompatible change, but I
haven't fully investigated the issue.
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:5>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Component: Forms | Version: 1.8alpha1
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 0 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* owner: nobody => timgraham
* status: new => assigned
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:6>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Needs tests: 0 | Patch needs improvement: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* has_patch: 0 => 1
Comment:
Hi Starou, could you take a look at the attached documentation note and
let me know if addresses your issue?
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:7>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Starou):
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 :-)
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:8>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by timgraham):
That code isn't working on 1.7 for me:
{{{
def image_upload_path(instance, filename):
return "authors/%d/books/%d/%s" % params = (
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
[https://docs.djangoproject.com/en/1.7/ref/models/fields/#django.db.models.FileField.upload_to
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."
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:9>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Starou):
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!
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:10>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: assigned
Severity: Release blocker | Resolution:
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Changes (by timgraham):
* component: Forms => Documentation
Comment:
You're welcome. I'll commit the doc patch and close this. Please open a
new ticket if you find something else.
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:11>
Django
Reporter: Starou | Owner: timgraham
Component: Documentation | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: inline ModelForm | 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:"0af3822dc362b6253bda1c9699466dd0bbbf6066"]:
{{{
#!CommitTicketReference repository=""
revision="0af3822dc362b6253bda1c9699466dd0bbbf6066"
Fixed #24325 -- Documented change in ModelForm.save() foreign key access.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:12>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: closed
Component: Documentation | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"8657e7caaac41266d9ac0b73a21af64edc681613"]:
{{{
#!CommitTicketReference repository=""
revision="8657e7caaac41266d9ac0b73a21af64edc681613"
[1.8.x] Fixed #24325 -- Documented change in ModelForm.save() foreign key
access.
Backport of 0af3822dc362b6253bda1c9699466dd0bbbf6066 from master
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:13>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: closed
Component: Documentation | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"d298b1ba5043eaa40f3f4bebe3c7634b359ba34b"]:
{{{
#!CommitTicketReference repository=""
revision="d298b1ba5043eaa40f3f4bebe3c7634b359ba34b"
Reverted "Fixed #24325 -- Documented change in ModelForm.save() foreign
key access."
This reverts commit 0af3822dc362b6253bda1c9699466dd0bbbf6066.
It's obsoleted by refs #24395.
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:14>
Django
Reporter: Starou | Owner: timgraham
Type: Bug | Status: closed
Component: Documentation | Version: 1.8alpha1
Severity: Release blocker | Resolution: fixed
Keywords: inline ModelForm | Triage Stage: Accepted
Has patch: 1 | Needs documentation: 0
Easy pickings: 0 | UI/UX: 0
Comment (by Tim Graham <timograham@…>):
In [changeset:"81911f29b70710d8f72916c49a69aa5df5cd7df8"]:
{{{
#!CommitTicketReference repository=""
revision="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
}}}
--
Ticket URL: <https://code.djangoproject.com/ticket/24325#comment:15>