#25148: Add a warning for `CASCADE`ing foreign keys on custom User models.
-------------------------------+--------------------------------------
     Reporter:  fcurella       |                    Owner:  nobody
         Type:  Uncategorized  |                   Status:  new
    Component:  contrib.auth   |                  Version:  1.8
     Severity:  Normal         |               Resolution:
     Keywords:                 |             Triage Stage:  Unreviewed
    Has patch:  0              |      Needs documentation:  0
  Needs tests:  0              |  Patch needs improvement:  0
Easy pickings:  0              |                    UI/UX:  0
-------------------------------+--------------------------------------
Changes (by fcurella):

 * needs_better_patch:   => 0
 * needs_tests:   => 0
 * needs_docs:   => 0


Old description:

> Assume a custom user models is created as such:
>
> {{{
> class Taco(models.Model):
>     flavor = models.CharField(max_length=64, unique=True)
>

> class User(AbstractBaseUser):
>     USERNAME_FIELD = 'username'
>
>     username = models.CharField(max_length=64, unique=True)
>     favorite_taco = models.ForeignKey(Taco, null=True)
> }}}
>
> When deleting the related objects, the user will also be deleted because
> is cascading:
>
> {{{
>         taco = Taco.objects.create(flavor="vegetarian")
>         user = User.objects.create_user(
>             username='taco fan',
>             favorite_taco=taco
>         )
>         User.objects.count()  # 1
>         taco.delete()
>         User.objects.count()  # 0
> }}}
>
> This is intended behavior, but it's still pretty dangerous and a common
> trap for beginners.
>
> I'd like to leverage the system check framework to add a Warning whenever
> a FK with `on_delete=CASCADE` is added to a User model. Something along
> the lines of:
>
> {{{
> class AbstractUser():
>     @classmethod
>     def check(cls, **kwargs):
>         errors = super(AbstractUser, cls).check(**kwargs)
>         for field in cls._meta.fields:
>             if isinstance(field, models.ForeignKey):
>                 if field.rel.on_delete == models.CASCADE:
>                     errors.append(checks.Warning(
>                         "Field {0} is set to CASCADE on delete. You might
> lose users".format(field.name),
>                         hint="Explicitly set {0}.on_delete to something
> else than `models.CASCADE".format(field.name),
>                         obj=field,
>                         id="{0}.W001".format(cls._meta.app_label)
>                     ))
>         return errors
> }}}

New description:

 Assume a custom user models is created as such:

 {{{
 class Taco(models.Model):
     flavor = models.CharField(max_length=64, unique=True)


 class User(AbstractBaseUser):
     USERNAME_FIELD = 'username'

     username = models.CharField(max_length=64, unique=True)
     favorite_taco = models.ForeignKey(Taco, null=True)
 }}}

 When deleting the related object, the user will also be deleted because of
 cascading:

 {{{
         taco = Taco.objects.create(flavor="vegetarian")
         user = User.objects.create_user(
             username='taco fan',
             favorite_taco=taco
         )
         User.objects.count()  # 1
         taco.delete()
         User.objects.count()  # 0
 }}}

 This is intended behavior, but it's still pretty dangerous and a common
 trap for beginners.

 I'd like to leverage the system check framework to add a Warning whenever
 a FK with `on_delete=CASCADE` is added to a User model. Something along
 the lines of:

 {{{
 class AbstractUser():
     @classmethod
     def check(cls, **kwargs):
         errors = super(AbstractUser, cls).check(**kwargs)
         for field in cls._meta.fields:
             if isinstance(field, models.ForeignKey):
                 if field.rel.on_delete == models.CASCADE:
                     errors.append(checks.Warning(
                         "Field {0} is set to CASCADE on delete. You might
 lose users".format(field.name),
                         hint="Explicitly set {0}.on_delete to something
 else than `models.CASCADE".format(field.name),
                         obj=field,
                         id="{0}.W001".format(cls._meta.app_label)
                     ))
         return errors
 }}}

--

--
Ticket URL: <https://code.djangoproject.com/ticket/25148#comment:1>
Django <https://code.djangoproject.com/>
The Web framework for perfectionists with deadlines.

-- 
You received this message because you are subscribed to the Google Groups 
"Django updates" group.
To unsubscribe from this group and stop receiving emails from it, send an email 
to [email protected].
To post to this group, send email to [email protected].
To view this discussion on the web visit 
https://groups.google.com/d/msgid/django-updates/066.155cc823fb008d33dfdcbc91ef6806c2%40djangoproject.com.
For more options, visit https://groups.google.com/d/optout.

Reply via email to