Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
GitRon opened this issue Aug 21, 2023 · 11 comments
Closed

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

GitRon opened this issue Aug 21, 2023 · 11 comments
Assignees
Labels
bug Something isn't working

Comments

@GitRon
Copy link
Contributor

GitRon commented Aug 21, 2023

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

  • Python: 3.10
  • Django 4.2.4
  • Model Bakery 1.14.0
amureki added a commit that referenced this issue Aug 21, 2023
@amureki amureki self-assigned this Aug 21, 2023
@amureki amureki added the bug Something isn't working label Aug 21, 2023
@amureki
Copy link
Collaborator

amureki commented Aug 21, 2023

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
Copy link
Contributor Author

GitRon commented Aug 21, 2023

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

@GitRon
Copy link
Contributor Author

GitRon commented Aug 21, 2023

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
Copy link
Contributor Author

GitRon commented Aug 21, 2023

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
Copy link
Collaborator

amureki commented Aug 23, 2023

@GitRon thanks for the debug!

I suspect ff6b8ee to be the issue.

May I ask you to install the library with pinned version of c22808b (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
Copy link
Contributor Author

GitRon commented Aug 24, 2023

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
Copy link
Contributor Author

GitRon commented Aug 24, 2023

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

@amureki
Copy link
Collaborator

amureki commented Aug 24, 2023

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

@GitRon
Copy link
Contributor Author

GitRon commented Aug 24, 2023

Awesome! ❤️

amureki added a commit that referenced this issue Aug 24, 2023
- Add Python 3.12 support
- Revert erroneous optimisation of related logic (fix #439)
- Bring tox back
@amureki
Copy link
Collaborator

amureki commented Aug 24, 2023

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

@GitRon
Copy link
Contributor Author

GitRon commented Aug 25, 2023

Great! 🚀 I'll try it out immediately.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants