Foreign keys with on_delete=SET_NULL should not be deleted on update IMHO

[izimobil]

Hi,

I have the following models (simplified for clarity):

class User(models.Model):
    first_name = models.CharField(
        'First name',
        max_length=150,
    )
    last_name = models.CharField(
        'Last name',
        max_length=150,
    )


class Badge(models.Model):
    number = models.BigIntegerField(
        'Number',
        primary_key=True,
        unique=True
    )
    user = models.ForeignKey(
        User,
        models.SET_NULL,
        verbose_name="Owner",
        related_name='badges',
        blank=True,
        null=True
    )

And the following serializers:

class UserBadgeSerializer(serializers.ModelSerializer):
    class Meta:
        model = Badge
        fields = ('number',)


class UserSerializer(WritableNestedModelSerializer):
    badges = UserBadgeSerializer(many=True, required=False)

    class Meta:
        model = User
        fields = (
            'pk', 'first_name', 'last_name', 'badges'
        )

As you can see a "Badge" can live without a user (hence the SET_NULL on the foreign key).
When I do the following request:

POST /api/users {
    "first_name": "Foo",
    "last_name": "Bar",
    "badges": [{"number": 1234}, {"number": 1235}]
}

All is fine, the user and badges are created and associated.
However, when I update the user to remove one of its badges:

PUT /api/users/1/ {
    "first_name": "Foo",
    "last_name": "Bar",
    "badges": [{"number": 1234}]
}

The badge 1235 is removed from the user (which is fine) but also gets deleted from the db.

I think (but I might be wrong) the correct behavior for foreign keys with on_delete != CASCADE, would be to only update the foreignkey and set it to None.

I have a working patch for this, if you're interested I can submit a pull request.

In short, in NestedUpdateMixin.delete_reverse_relations_if_need():

                    qs = model_class.objects.filter(pk__in=pks_to_delete)
                    if related_field.remote_field.on_delete == CASCADE:
                        qs.delete()
                    else:
                        qs.update(**{related_field.name: None})

Regards.

[ruscoder]

Hello @izimobil! Thank you for your contribution. I agree with you, that we shouldn't delete related instances when they have null=True.

I'll try to review your PR.

[ruscoder]

Available in 0.7.0