model-bakers / model_bakery

Object factory for Django
https://model-bakery.readthedocs.io/en/latest/
Other
845 stars 85 forks source link

Overwriting one attribute deletes foreign key data in v1.14 #439

Closed GitRon closed 1 year ago

GitRon commented 1 year ago

Describe the issue

When overwriting values of a related model, all other data set by the recipe are being reset/not set at all.

To Reproduce

I have a recipe for a model:

my_model = Recipe(
    MyModel,
    related_model=foreign_key('apps.myapp.tests.related_model'),
)

And a relationship:

related_model = Recipe(
    RelatedModel,
    name=seq('RelatedModel #'),
    my_flag=False,
)

This works:

my_obj = baker.make_recipe('apps.myapp.tests.my_model')
assert my_obj.related_model.name == 'RelatedModel #1'

This doesn't:

my_obj = baker.make_recipe('apps.myapp.tests.my_model', related_model__my_flag=True)
# the other values are being reset/deleted/are not set
assert my_obj.related_model.name == None

I downgraded to v1.13.0, then it works. Using 1.14.0, it doesn't.

Versions

amureki commented 1 year ago

Hey @GitRon ! This is an interesting issue.

I am trying to reproduce it here: #440 Could you, please, point me where my setup differs from yours? Review right there in PR would be fine, as well as the comments here.

GitRon commented 1 year ago

Hi @amureki - awesome that was fast! I checked out your branch and am trying to build the case so I can reproduce it.

GitRon commented 1 year ago

Ok, I tried to reproduce it as closely as possible but failed. The error occurs in a pre_save signal of the related model. I'm wondering if this is some kind of racing condition. Was anything changed in v1.14 that might cause this kind of behaviour?

GitRon commented 1 year ago

I debugged the issue. In baker.py l.510, I see that instance.save(**_save_kwargs) is happening. Couple of lines before, instance = self.model(**attrs) creates a temporary instance and this attrs contains only (!) the values I passed in the instantiation (my_flag=False). The save method is triggering the signal and therefore I get the error.

Seems quite obvious but I have no idea why I didn't manage to reproduce this with your test suite 😅

amureki commented 1 year ago

@GitRon thanks for the debug!

I suspect ff6b8eec1b38ec61d3d7adddb3d2de6b7487a758 to be the issue.

May I ask you to install the library with pinned version of c22808b8f2dde283f60ecddcccb3a2f979de9565 (commit before the one I am suspecting) and run your failing test?

python -m pip install 'model_bakery @ git+https://github.com/model-bakers/model_bakery@c22808b8f2dde283f60ecddcccb3a2f979de9565'

If so, I will revert the change and will release the new version ASAP.

Best, Rust

GitRon commented 1 year ago

Hi @amureki - you make it so easy to help you! Thx a lot for the pip command, would have had to google for it ❤️

I tried it two times and in both cases, the latest version crashed three tests and the pinned one worked.

Best from Cologne
Ronny

GitRon commented 1 year ago

Awesome! Are you going to release a bugfix release then as well?

amureki commented 1 year ago

@GitRon yup, later today will be live. :) Still want to put one more fix there.

GitRon commented 1 year ago

Awesome! ❤️

amureki commented 1 year ago

@GitRon live in 1.15.0 👍 Thanks for reporting and all the collaboration! 💟

GitRon commented 1 year ago

Great! 🚀 I'll try it out immediately.