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

Broken reverse FK behaviour in Django 4.1 #347

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

@amureki amureki self-assigned this Sep 26, 2022
@amureki
Copy link
Collaborator Author

amureki commented Nov 2, 2022

I'd close this for now, as I did not receive any feedback from users regarding this change in Django 4.1.
It might still appear, and then we'll think on the way we tackle this.

@amureki amureki closed this Nov 2, 2022
@amureki amureki deleted the django-41-reverse-fk branch November 2, 2022 15:20
@jefftriplett
Copy link

Sorry to delay. I am seeing this too in a few projects now that I have time to dig back in.

This is the pattern I'm seeing it with:

job = JobSerializer(
    baker.prepare(
        "jobs.Job",
        company=baker.make("companies.Company"),
        employee=baker.make("companies.Employee"),
    )
).data

Generates:

> ValueError: 'Job' instance needs to have a primary key value before this relationship can be used.

@amureki amureki restored the django-41-reverse-fk branch March 16, 2023 08:42
@amureki
Copy link
Collaborator Author

amureki commented Mar 16, 2023

@jefftriplett sorry Jeff, I missed your reply. Thanks for the report!

May I assume your model structure looks like the following? Just to make sure I am not missing any specifics:

class Job(models.Model):
    company = models.ForeignKey("companies.Company")
    employee = models.ForeignKey("companies.Employee")

Did you find a way around it? I am honestly can't think of a good way yet of solving it that can comply with changes in Django.

@syphar
Copy link

syphar commented Mar 20, 2023

@amureki I was working on upgrading to django 4.1, and we are potentially hitting the same issue (I think), but a little different:

class Parameters(models.Model): 
     something = models.IntegerField() 

class Opportunity(models.Model):
     parameters = models.OneToOneField(Parameters, blank=True, null=True)

and then this fails:

baker.make_recipe("opportunity.Opportunity", parameters=p)

Which is strange, this this is make_recipe, not prepare_recipe, right?

@syphar
Copy link

syphar commented Mar 20, 2023

If needed I can try to build a minimal example how to reproduce this

@amureki
Copy link
Collaborator Author

amureki commented Mar 20, 2023

If needed I can try to build a minimal example how to reproduce this

That would be wonderful! It is not clear for me, how p object is created and how does the recipe looks like.
I tried something to your example, but was unable to reproduce the issue:

class Person(models.Model):
    age = models.IntegerField()

class LonelyPerson(models.Model):
    only_friend = models.OneToOneField(Person, on_delete=models.CASCADE, blank=True, null=True)

def test_default_behaviour_for_one_to_one():
    only_friend = baker.make(models.Person)
    lonely_person = baker.make(models.LonelyPerson, only_friend=only_friend)

    assert lonely_person.pk
    assert lonely_person.only_friend.pk

def test_default_behaviour_for_one_to_one__prepare():
    only_friend = baker.prepare(models.Person)
    lonely_person = baker.prepare(models.LonelyPerson, only_friend=only_friend)

    assert not lonely_person.pk
    assert not lonely_person.only_friend.pk

@syphar
Copy link

syphar commented Mar 21, 2023

@amureki forget my comment above, thank you for investing the time.

I was stupid and the actual problem wasn't related to model-bakery

@amureki
Copy link
Collaborator Author

amureki commented Mar 21, 2023

@amureki forget my comment above, thank you for investing the time.

I was stupid and the actual problem wasn't related to model-bakery

No, there is nothing wrong with that, it was good to check O2O relations anyway. Please tell me if you'd stumble upon that issue later, and good luck with your Django 4.1 upgrade! 🤞

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants