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
26722	Django silently discarding the user-provided on_delete with GenericRelation	Delizin	nobody	"In the [https://docs.djangoproject.com/en/1.9/ref/contrib/contenttypes/#generic-relations contenttypes framework documentation] it shows the content_type foreignkey field with the on_delete option set to models.CASCADE.

In  it states that [https://docs.djangoproject.com/en/1.9/ref/contrib/contenttypes/#reverse-generic-relations Reverse generic relations] section it states ""Unlike ForeignKey, GenericForeignKey does not accept an on_delete argument to customize this behavior"". 

Before realizing that it was not possible to change the on_delete method, I attempted to change it to models.SET_NULL using three different methods with only one resulting in an error or warning.

The first method I attempted was setting the content_type on_delete to models.SET_NULL, because it was the only value with an on_delete argument shown in the documentation (though I now assume that was perhaps shown because it will be required with Django 2.0). It is logical for this to not provide any warning or errors, but perhaps the documentation could be made clearer on this point. 

{{{
#!div style=""font-size: 80%""
Model:
  {{{#!python
  class foo(models.Model):
    content_type = models.ForeignKey(ContentType, null=True, blank=True, related_name='logentry_content_type', on_delete=models.SET_NULL)
    object_id = models.IntegerField(null=True, blank=True)
    content_object = GenericForeignKey('content_type', 'object_id')
  }}}
}}}

The second method was to try adding the on_delete argument to the GenericForeignKey. This was rejected immediately as an unexpected keyword. That behavior is expected based on the documentation.

Next I tried adding an on_delete argument to the GenericRelation itself. The argument was silently accepted and then discarded. 

{{{
#!div style=""font-size: 80%""
Model:
  {{{#!python
class bar(models.Model):
    relation = GenericRelation(foo, on_delete=models.SET_NULL)
  }}}
}}}

The issue appears to be that in the GenericRelation __init__ it is forcing the kwargs['on_delete'] to models.CASCADE and discarding any user input. 

{{{
#!div style=""font-size: 80%""
[https://github.com/django/django/blob/master/django/contrib/contenttypes/fields.py#L300 GenericRelation model definition] 
  {{{#!python
kwargs['on_delete'] = models.CASCADE
  }}}
}}}

My preference for a fix would be to allow GenericRelation to respect the user defined wishes for on_delete, but if that is not feasible then supplying an on_delete argument to GenericRelation should generate a warning or an error to prevent users from assuming that GenericRelation will respect the setting. 

This ticket is related to a question I originally asked in #django-dev on June 7th, 2015 at 15:15 UTC. I have attached the discussion below for completeness. 

{{{
#!div style=""font-size: 70%""
 {{{#!irc
[15:15] <delizin> In the django contenttypes framework docs for Generic Relations it shows the content_type foreignkey with the on_delete=models.CASCADE. However in use, if that foreign key is set to nullable and on_delete is set to models.SET_NULL, when the generic object is deleted, it still cascades. Adding on_delete=models.SET_NULL to the GenericRelation does not change this behavior either. Is this a bug, feature or just incorrect usage?
[15:16] <delizin> Relevant documentation: https://docs.djangoproject.com/en/1.9/ref/contrib/contenttypes/#generic-relations and the model I was testing with: https://dpaste.de/sYYJ
[15:22] <+bmispelon> delizin: I'm not very familiar with the contenttypes framework but the documentation you linked says: ""Unlike ForeignKey, GenericForeignKey does not accept an on_delete argument to customize this behavior""
[15:22] <+bmispelon> I have a sneaking suspicion that Django is happily accepting your `on_delete` and silently discarding it
[15:22] <+bmispelon> which would be a bug in my opinion
[15:24] <+bmispelon> https://github.com/django/django/blob/master/django/contrib/contenttypes/fields.py#L300
[15:27] <delizin> bmispelon: Interesting. I missed that part of the documentation, though I agree with you that it shouldn't just ignore the on_delete statement and should instead throw an error if you attempt to change it.
[15:28] <+bmispelon> this anti-pattern is sadly quite common in Django's codebase
[15:29] <+bmispelon> could you open an issue about it (Django silently discarding the user-provided on_delete)?
[15:29] <delizin> bmispelon: Certainly
[15:30] <+bmispelon> I think Django should also warn you when you define a generic relation based on a contenttype FK that has on_delete=SET_NULL (or anything other than CASCADE)
[15:30] <+bmispelon> because from what you're saying, it seems that Django will always cascade, no matter what
  }}}
}}}"	Bug	new	contrib.contenttypes	1.9	Normal				Accepted	0	0	0	0	0	0