From 6d4bd3c650249b5d85b8df71cc64dd8558f1d2f6 Mon Sep 17 00:00:00 2001 From: Bernardo Fontes Date: Thu, 31 Mar 2022 18:48:43 -0300 Subject: [PATCH 1/6] Get ManyToOneRel fields (reverse FK) back to work --- model_bakery/baker.py | 10 +++++++++- tests/generic/models.py | 15 +++++++++++++++ tests/test_baker.py | 14 ++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/model_bakery/baker.py b/model_bakery/baker.py index 78ea561b..74c1bff2 100644 --- a/model_bakery/baker.py +++ b/model_bakery/baker.py @@ -606,7 +606,15 @@ def _handle_one_to_many(self, instance: Model, attrs: Dict[str, Any]): manager = getattr(instance, key) for value in values: - if not value.pk: + # Django will handle any operation to persist nested non-persisted FK because + # save don't do so and, thus, raise constraint errors. That's why save() only gets + # called if the object doesn't have a pk and also doesn't hold fk pointers + fks = any([ + fk + for fk in value._meta.fields + if isinstance(fk, ForeignKey) or isinstance(fk, OneToOneField) + ]) + if not value.pk and not fks: value.save() try: diff --git a/tests/generic/models.py b/tests/generic/models.py index 832f1896..271c7df8 100755 --- a/tests/generic/models.py +++ b/tests/generic/models.py @@ -455,3 +455,18 @@ class NonStandardManager(models.Model): name = models.CharField(max_length=30) manager = models.Manager() + + +### The followin models were added after issue 291 +# Since they doesn't hold much meaning, they are only numbered ones +class Issue291Model1(models.Model): + pass + + +class Issue291Model2(models.Model): + m2m_model_1 = models.ManyToManyField(Issue291Model1) + + +class Issue291Model3(models.Model): + fk_model_2 = models.ForeignKey(Issue291Model2, related_name="bazs", on_delete=models.CASCADE) + name = models.CharField(max_length=32) diff --git a/tests/test_baker.py b/tests/test_baker.py index 7a53937b..f02b25e1 100644 --- a/tests/test_baker.py +++ b/tests/test_baker.py @@ -569,6 +569,20 @@ def test_field_lookup_for_related_field_does_not_work_with_prepare(self): assert not person.pk assert 0 == models.RelatedNamesModel.objects.count() + def test_ensure_reverse_fk_for_many_to_one_is_working(self): + """This is a regression test to make sure issue 291 is fixed""" + fk1, fk2 = baker.prepare(models.Issue291Model3, fk_model_2=None, name="custom name", _quantity=2) + obj = baker.make( + models.Issue291Model2, + m2m_model_1=[baker.make(models.Issue291Model1)], + bazs=[fk1, fk2], + ) + + assert obj.bazs.count() == 2 + related_1, related_2 = obj.bazs.all() + assert related_1.name == "custom name" + assert related_2.name == "custom name" + @pytest.mark.django_db class TestHandlingUnsupportedModels: From a5f48e358a959418075cb004f1d754c18e565f7e Mon Sep 17 00:00:00 2001 From: Bernardo Fontes Date: Thu, 31 Mar 2022 18:49:22 -0300 Subject: [PATCH 2/6] Update changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4c64159c..5900b3aa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). - Extend type hints in `model_bakery.recipe` module, make `Recipe` class generic [PR #292](https://github.com/model-bakers/model_bakery/pull/292) - Explicitly add _fill_optional parameters to baker.make and baker.prepare to aid IDE autocomplete function. [PR #264](https://github.com/model-bakers/model_bakery/pull/264) - Fixed errors with reverse M2M relationships [PR #299](https://github.com/model-bakers/model_bakery/pull/299) +- Fixed errors with reverse M2O relationships [PR #300](https://github.com/model-bakers/model_bakery/pull/300) ### Removed From ea2d23ef57640dd0df60f5e9619903ed7c69247a Mon Sep 17 00:00:00 2001 From: Bernardo Fontes Date: Thu, 31 Mar 2022 18:53:38 -0300 Subject: [PATCH 3/6] Run linter --- model_bakery/baker.py | 12 +++++++----- tests/generic/models.py | 6 ++++-- tests/test_baker.py | 4 +++- 3 files changed, 14 insertions(+), 8 deletions(-) diff --git a/model_bakery/baker.py b/model_bakery/baker.py index 74c1bff2..cf82bc13 100644 --- a/model_bakery/baker.py +++ b/model_bakery/baker.py @@ -609,11 +609,13 @@ def _handle_one_to_many(self, instance: Model, attrs: Dict[str, Any]): # Django will handle any operation to persist nested non-persisted FK because # save don't do so and, thus, raise constraint errors. That's why save() only gets # called if the object doesn't have a pk and also doesn't hold fk pointers - fks = any([ - fk - for fk in value._meta.fields - if isinstance(fk, ForeignKey) or isinstance(fk, OneToOneField) - ]) + fks = any( + [ + fk + for fk in value._meta.fields + if isinstance(fk, ForeignKey) or isinstance(fk, OneToOneField) + ] + ) if not value.pk and not fks: value.save() diff --git a/tests/generic/models.py b/tests/generic/models.py index 271c7df8..f2d0f381 100755 --- a/tests/generic/models.py +++ b/tests/generic/models.py @@ -457,7 +457,7 @@ class NonStandardManager(models.Model): manager = models.Manager() -### The followin models were added after issue 291 +# The followin models were added after issue 291 # Since they doesn't hold much meaning, they are only numbered ones class Issue291Model1(models.Model): pass @@ -468,5 +468,7 @@ class Issue291Model2(models.Model): class Issue291Model3(models.Model): - fk_model_2 = models.ForeignKey(Issue291Model2, related_name="bazs", on_delete=models.CASCADE) + fk_model_2 = models.ForeignKey( + Issue291Model2, related_name="bazs", on_delete=models.CASCADE + ) name = models.CharField(max_length=32) diff --git a/tests/test_baker.py b/tests/test_baker.py index f02b25e1..2c6b146c 100644 --- a/tests/test_baker.py +++ b/tests/test_baker.py @@ -571,7 +571,9 @@ def test_field_lookup_for_related_field_does_not_work_with_prepare(self): def test_ensure_reverse_fk_for_many_to_one_is_working(self): """This is a regression test to make sure issue 291 is fixed""" - fk1, fk2 = baker.prepare(models.Issue291Model3, fk_model_2=None, name="custom name", _quantity=2) + fk1, fk2 = baker.prepare( + models.Issue291Model3, fk_model_2=None, name="custom name", _quantity=2 + ) obj = baker.make( models.Issue291Model2, m2m_model_1=[baker.make(models.Issue291Model1)], From 225f8cfd8102001864706eb11f4775c82f3894e7 Mon Sep 17 00:00:00 2001 From: berin Date: Thu, 31 Mar 2022 19:33:09 -0300 Subject: [PATCH 4/6] Grammar fix 1 Co-authored-by: Tim Klein --- model_bakery/baker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model_bakery/baker.py b/model_bakery/baker.py index cf82bc13..aeeb2100 100644 --- a/model_bakery/baker.py +++ b/model_bakery/baker.py @@ -607,7 +607,7 @@ def _handle_one_to_many(self, instance: Model, attrs: Dict[str, Any]): for value in values: # Django will handle any operation to persist nested non-persisted FK because - # save don't do so and, thus, raise constraint errors. That's why save() only gets + # save doesn't do so and, thus, raises constraint errors. That's why save() only gets # called if the object doesn't have a pk and also doesn't hold fk pointers fks = any( [ From cfdc6cf5bfe4a7ab57531a5e6f1a9d42d4c495f2 Mon Sep 17 00:00:00 2001 From: berin Date: Thu, 31 Mar 2022 19:33:17 -0300 Subject: [PATCH 5/6] Grammar fix 2 Co-authored-by: Tim Klein --- tests/generic/models.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/generic/models.py b/tests/generic/models.py index f2d0f381..0acf2145 100755 --- a/tests/generic/models.py +++ b/tests/generic/models.py @@ -458,7 +458,7 @@ class NonStandardManager(models.Model): # The followin models were added after issue 291 -# Since they doesn't hold much meaning, they are only numbered ones +# Since they don't hold much meaning, they are only numbered ones class Issue291Model1(models.Model): pass From 1847121241130a237455fa1a674066b271dc85e9 Mon Sep 17 00:00:00 2001 From: Bernardo Fontes Date: Thu, 31 Mar 2022 19:37:01 -0300 Subject: [PATCH 6/6] Code linter --- model_bakery/baker.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/model_bakery/baker.py b/model_bakery/baker.py index aeeb2100..a161d5be 100644 --- a/model_bakery/baker.py +++ b/model_bakery/baker.py @@ -607,8 +607,9 @@ def _handle_one_to_many(self, instance: Model, attrs: Dict[str, Any]): for value in values: # Django will handle any operation to persist nested non-persisted FK because - # save doesn't do so and, thus, raises constraint errors. That's why save() only gets - # called if the object doesn't have a pk and also doesn't hold fk pointers + # save doesn't do so and, thus, raises constraint errors. That's why save() + # only gets called if the object doesn't have a pk and also doesn't hold fk + # pointers. fks = any( [ fk