id	summary	reporter	owner	description	type	status	component	version	severity	resolution	keywords	cc	stage	has_patch	needs_docs	needs_tests	needs_better_patch	easy	ui_ux
23449	Saving models with unsaved ForeignKey	Patrick Robetson	nobody	"Previous tickets relating to this are #10811 and #8892 (which have been closed in lastest Master/1.8)
Some work has already been done to deal with assigning unsaved foreign keys to models (a ValueError is now raised when assigning) but I still think there is more work to be done.

First off, I do not think it is strictly 'right' that a ValueError is raised if you assign a ForeignKey with a null ID to a model. Something like this might be useful:

{{{

class ModelA(models.Model):
    name = models.CharField(max_length=10)

class ModelB(models.Model):
    name = models.CharField(max_length=10)
    a = models.ForeignKey(ModelA) # note the lack of null=True. Which I believe more common than the edge case when null=True is defined
}}}

{{{
b = ModelB(name='me', a=ModelA())
if b.conditional_test():
   b.save()
else:
   # discard b and don't save
}}}

I don't think any checks should be done until a `ModelB` object is actually saved to the database - when creating a `ModelB` object there is no requirement that it be saved to the database, so why should a database-related check (that `a.id != None`) be made?

I am aware of the chance of data loss, but this is also an edge case - if `ModelB` is defined as having `null=True` for the `ModelA ForeignKey` (if `null=False` then an `IntegrityError` is raised upon save as expected)
So in this case, perhaps a `ValueError` should *only* be raised if `null=True` on the ForeignKey field.

In my view, this is a regression - a model is just a Python object, and there is no requirement to save it to the database. It should be treated this way (database agnostic) until it is actually saved.


Further to this - Django 1.7 has several issues with saving models that have a ForeignKey with a null id (most likely why this `ValueError` implementation was introduced.

{{{
# Django 1.7, with null=False set on the Foreign Key field
>>> b = ModelB(name='b', a=ModelA(name='a'))
>>> b.a.save()
>>> print b.a.pk
7 # a now has a pk
>>> b.save()
Integrity Error raised... a_id is null

# re-assiging post save fixes this:
>>> b = ModelB(name='b', a=ModelA(name='a'))
>>> b.a.save()
>>> b.a = b.a
>>> print b.a.pk
7 # a now has a pk
>>> b.save()
# works!
}}}

In light of this, I think that the `ValueError` check should either be reverted (or left only for when `null=True` on the ForeignKey - the only case when data loss is possible) AND in other cases, when saving a model with a foreign key, any 'backwards' relationships should have their `_id` value set (so as to avoid having to re-assign the foreign key). I'm not too sure of the internals of Dj models, but perhaps a ForeignKey field could add some sort of post_save() signal to itself, so that when `a` is saved

{{{
>>> b = ModelB(name='b', a=ModelA(name='a'))
>>> b.a.save()
# behind the scenes, in a.save() (or wherever)
... self.save()
... self.send_post_save_signal()
... # in ForeignKey
... def deal_with_foreign_key_post_save(obj):
...     modelb.a_id = obj.id
}}}


"	Bug	closed	Database layer (models, ORM)	dev	Normal	wontfix		ANUBHAV JOSHI	Accepted	0	0	0	0	0	0