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

[BUG] baker.make() dunder syntax fails for through-table model in 1.3.3 #273

Closed
cb109 opened this issue Dec 10, 2021 · 5 comments
Closed
Labels
bug Something isn't working high priority

Comments

@cb109
Copy link
Contributor

cb109 commented Dec 10, 2021

The v1.3.3 release broke a few of our tests where we use model_bakery's "dunder syntax" to specify fields on ForeignKeys on a through-table model.

I am able to reproduce it in a minimal example, but have no clue yet how to fix it, hoping you can point me in the right direction.

Expected behavior

baker.make() should create the through-table model instance with instances in both ForeignKey fields as specified.

Actual behavior

Model instance creation fails with:

model_bakery/baker.py:98: in make
    return baker.make(
model_bakery/baker.py:324: in make
    return self._make(**params)
model_bakery/baker.py:371: in _make
    self.model_attrs[field.name] = self.generate_value(
model_bakery/baker.py:594: in generate_value
    return generator(**generator_attrs)
model_bakery/random_gen.py:248: in gen_related
    return make(model, **attrs)
model_bakery/baker.py:98: in make
    return baker.make(
model_bakery/baker.py:324: in make
    return self._make(**params)
model_bakery/baker.py:371: in _make
    self.model_attrs[field.name] = self.generate_value(
model_bakery/baker.py:566: in generate_value
    if field.has_default() and field.name not in self.rel_fields:
E   AttributeError: 'ManyToManyRel' object has no attribute 'has_default'

Reproduction Steps

Given these models:

class Player(models.Model):
    name = models.CharField(max_length=128)
    playlist = models.ManyToManyField("Content", through="PlayerContent")


class Content(models.Model):
    name = models.CharField(max_length=128)


class PlayerContent(models.Model):
    player = models.ForeignKey(Player, on_delete=models.CASCADE)
    content = models.ForeignKey(Content, on_delete=models.CASCADE)

Failure can be reproduced via:

dummy = baker.make(
    models.PlayerContent,
    content__name="Indiana Jones",
    player__name="TV"
)

Versions

Python: 3.8.10
Django: 2.2.24
Model Bakery: 1.3.3

Notes

  • I will provide a PR to reproduce this in a test: Bug/through table dunder syntax fails in 1.3.3 #274
  • The problem does not appear in v1.3.2
  • Passing an instance for content or player directly (e.g. content=content_instance) in the example above will make the problem disappear, it only comes up when both fields are passed with the dunder syntax.
  • Removing the playlist ManyToManyField with the through Option will also make the problem disappear.
@cb109 cb109 changed the title baker.make() dunder syntax fails for through-table model in 1.3.3 [BUG] baker.make() dunder syntax fails for through-table model in 1.3.3 Dec 10, 2021
@amureki
Copy link
Collaborator

amureki commented Dec 29, 2021

@cb109 thank you for providing such a detailed report and a PR with a test. Much appreciated!

I suspect https://github.com/model-bakers/model_bakery/pull/207/files#diff-e5857deb915e241f429a0c118e89e06a3388d3ce1466e3aa4b960b7055172b6dL322 to be the reason for this bug, but not yet sure how to tackle it.

Before I try to dig into it, @timjklein36 @berinhard, do you have ideas?
It would be nice to solve it before the next release...

@timjklein36
Copy link
Collaborator

@amureki I don't have any ideas without looking into it further.

@cb109
Copy link
Contributor Author

cb109 commented Jan 10, 2022

I did some experimental changes, all tests (model_bakery + our own testsuite) pass when I change

    def get_related(
        self,
    ) -> List[Union[ManyToOneRel, OneToOneRel]]:
        return [r for r in self.model._meta.related_objects if not r.many_to_many]

into

    def get_related(
        self,
    ) -> List[Union[ManyToOneRel, OneToOneRel, ManyToManyRel]]:
        return [r for r in self.model._meta.related_objects]

That's probably not desired, but maybe it helps figuring out a solution, just wanted to mention what I observed.

@berinhard berinhard added bug Something isn't working high priority labels Mar 31, 2022
@berinhard
Copy link
Member

Thanks for the tips here @cb109! This is was fixed in #299 and a new release will be ready soon.

@cb109
Copy link
Contributor Author

cb109 commented Apr 11, 2022

Since I just updated our codebase to use 1.5.0 just wanted to leave a quick feedback that the issue indeed seems to be solved and all our own tests are passing again 👍

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

No branches or pull requests

4 participants