in NestedUpdateMixin would be better to delete_reverse before update_or_create_reverse

[dz0]

I think, if we have unique validator (let's say for field x), and before request we had some related items {"id":1, "x": 10}, {"id":2, "x": 5}

and in request { "id":42, "stuf":"bar", "related": [ {"id":2, "x": 10} ] }

here we delete id=1, but the other "takes over" his value. (ps.: not tested)

so maybe switch the lines?

        self.update_or_create_reverse_relations(instance, reverse_relations)
        self.delete_reverse_relations_if_need(instance, reverse_relations)

[isasiluispy]

totally agree, this is a duplicated of my issue #49 , take a look I am using the way you are saying (switching the lines) and works perfectly.

[ruscoder]

@isasiluispy @dz0 Hello! Thanks for the contribution. It really makes sense. I'll prepare a fix soon.

[ruscoder]

I've found that your solution breaks some important tests.

[ruscoder]

I've fixed another bug in delete method.

Now, only one test is broken when I switch update and delete: WritableNestedModelSerializerTest.test_update_reverse_one_to_one_without_pk

This test covers functionality from this PR https://github.com/beda-software/drf-writable-nested/pull/36

[isasiluispy]

@ruscoder will this bug be fixed ? I think it's not wotking properly yet, for unique_together constraints like this example #49 I still have to swicth the lines to get it working :(

[ruscoder]

@isasiluispy Your suggested fix brakes functionality from #36. I don't know how we can fix it saving backward compatibility for the moment.

As for me, I don't understand the worth of #36, but a few people asked for this feature. If you want, you can make a PR with fix for #36 and for this issue.

[claytondaley]

To solve this problem, the logic in #36 needs to be moved into its own utility function and update changed to:

        self.materialize_one_to_one_pk(instance, reverse_relations)  # code from #36 relocated here
        self.delete_reverse_relations_if_need(instance, reverse_relations)
        self.update_or_create_reverse_relations(instance, reverse_relations)

Hopefully that helps someone who's motivated. I can't help ATM because I'm just here to evaluate the project as a replacement for our DIY logic.