WritableNestedModelSerializer and unique_together

[isasiluis28]

WritableNestedModelSerializer won't work when there is a foreign key relation with a unique constraint con the child model.

I already tried the mixin UniqueFieldsMixin, but it does not work either.

[ir4y]

Hi @isasiluis28 Thank you for the contribution.

Can you add a test which reproduces the bug?

[isasiluis28]

Sure @ir4y Suppose we add a new register via POST like this

This is the object we have created

Then if we try to update creating a new object for producto_detalles (without the ID field, this is because we want to delete the other objects), but with same ID of veterinaria that is already there before the update (which by the way is the one with a constraint)

Then it raises an IntegrityError

I think the problem is that is trying to create the new reverse relations without deleting the previous ones, so at that point we still have the previous objects, and when it tries to create the new relations it will raise an IntegrityError beacause of the existance of an object that already satisfies the constraint. I think that the algorithm has to remove first the relations where the IDs are not found anymore, then create the new reverse relations.

Looking at the code that handles the updates in NestedUpdateMixin I found that first tries to create or update the new relations, then deletes the relations that are no longer needed, doesn't it have to be the other way around? meaning first delete the relations, then create the new ones?

[ir4y]

I mean fork drf-writable-nested and add the test which will fail. TDD style.
Reproduce your issue in code.

[ruscoder]

Hello @isasiluis28! Thank you for the contribution. UniqueFieldsMixin cannot work with unique_together validator for the moment.

But I think it is possible to support this validator.

[ayushin]

Is there any progress on this?

[skamensky]

@ruscoder , any advice on how to implement this? I'd give it a go.

[samomar]

UniqueFieldsMixin does not work with unique_together for me

I have to send an empty list that deletes the items first then send the new items

I guess the way it works by default, it deletes the previous nested items and creates a new ones, we should await and confirm the items are deleted before sending in new items otherwise it'll raise an IntegrityError

[samomar]

The only way I can get this to work is by passing an ID or making one of my unique fields a primary key.

[Telemaco019]

Is there any progress? Is unique_together validator going to be supported in one of the next releases?

[FranzForstmayr]

@ruscoder Can you give a hint what's necessary to get this issue fixed?

[JohnMulligan]

I hit a wall on this as well.

[ruscoder]

Hi. Unfortunately, I haven't used drf-writable-nested for more than 6 years and I can't give a hint on how to implement it properly without diving into the implementation, but according to the discussion above there's a workaround suggested in #50 that has one limitation, but it might be not relevant for someone who affected by this issue.

So, I can recommend something like

class CustomWritableNestedModelSerializer(WritableNestedModelSerializer):
    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.delete_reverse_relations_if_need(instance, reverse_relations)
        self.update_or_create_reverse_relations(instance, reverse_relations)
        instance.refresh_from_db()
        return instance

that is actually copy-paste of original update with changed order of invocation of delete_reverse_relations_if_need and update_or_create_reverse_relations (according to the suggestions from #50).

In general, if someone would like to solve a conflict between #50 and #36 it might be merged into the main branch.