Delete reverse relations before update or create in NestedUpdateMixin

[gagantrivedi]

Hi, is there a reason why we are calling self.update_or_create_reverse_relations(instance, reverse_relations) before calling self.delete_reverse_relations_if_need(instance, reverse_relations) here:

class NestedUpdateMixin(BaseNestedModelSerializer):
    """
    Adds update nested feature
    """
    default_error_messages = {
        'cannot_delete_protected': _(
            "Cannot delete {instances} because "
            "protected relation exists")
    }

    def update(self, instance, validated_data):
        relations, reverse_relations = self._extract_relations(validated_data)

        # Create or update direct relations (foreign key, one-to-one)
        self.update_or_create_direct_relations(
            validated_data,
            relations,
        )

        # Update instance
        instance = super(NestedUpdateMixin, self).update(
            instance,
            validated_data,
        )
        self.update_or_create_reverse_relations(instance, reverse_relations)
        self.delete_reverse_relations_if_need(instance, reverse_relations)
        instance.refresh_from_db()
        return instance

The reason why I am concerned about this is - It violates certain conditions(that I check at the time of object creation using AFTER_SAVE hook) in my code by creating the object before deleting the one that needs to be deleted. If there is no reason behind the order, I think we should switch the order to delete first before create/update since it makes logical sense as well?

[ir4y]

I don't think that there was a specific reason for this particular order. Could you please create a pull request?

@ruscoder what do you think?

[gagantrivedi]

Hi @ir4y looking into a bit more, I realized that changing the order breaks this:

    if related_field.one_to_one:
                # If an object already exists, fill in the pk so
                # we don't try to duplicate it
                pk_name = field.Meta.model._meta.pk.attname
                if pk_name not in related_data and "pk" in related_data:
         

https://github.com/beda-software/drf-writable-nested/blob/master/drf_writable_nested/mixins.py#L150 https://github.com/beda-software/drf-writable-nested/blob/master/tests/test_writable_nested_model_serializer.py#L230

Since the object will not exist if we delete it first. But I also don't think it's an odd behavior to delete the object if pk was not given instead of inferring the pk(this is what I would expect). Let me know what do you guys think - I will be happy to shoot a PR

[ir4y]

@gagantrivedi Since there is a test that covers this scenario we can't change this behavior. We need to keep backward capability. I think in your particular case you can override update and use the modified version of NestedUpdateMixin.

Could you please contribute to https://github.com/beda-software/drf-writable-nested#known-problems-with-solutions by describing your case?

[ruscoder]

Related #49 (duplicate of #50)