model-bakers/model_bakery

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

cb109 opened this issue · 5 comments

cb109 commented

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: #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 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...

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

cb109 commented

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.

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

cb109 commented

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 👍