Skip to content

Commit

Permalink
Fix creation of model instances with related model fields (#164)
Browse files Browse the repository at this point in the history
* Allow id to be used as FK default (Fix #136)

#117 introduced an issue for cases where `id` values is provided as a default for foreign keys.
This PR attempts to fix that issue by checking if generated value for FK field is an instance of this model - if not it uses `_id` as a field name (https://docs.djangoproject.com/en/3.1/ref/models/fields/#database-representation).

* Another fix idea: use default unless given custom argument

* Remove old way, do some cleanup

* Update CHANGELOG.md
  • Loading branch information
amureki authored Mar 12, 2021
1 parent 4318ef8 commit 297745b
Show file tree
Hide file tree
Showing 4 changed files with 68 additions and 7 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
### Changed

- Type hinting fixed for Recipe "_model" parameter [PR #124](https://github.com/model-bakers/model_bakery/pull/124)
- Modify `setup.py` to not import the whole module for package data, but get it from `__about__.py` [PR #142](https://github.com/model-bakers/model_bakery/pull/142)
- Fixed a bug (introduced in 1.2.1) that was breaking creation of model instances with related model fields [PR #164](https://github.com/model-bakers/model_bakery/pull/164)
- [dev] Modify `setup.py` to not import the whole module for package data, but get it from `__about__.py` [PR #142](https://github.com/model-bakers/model_bakery/pull/142)
- [dev] Add Dependabot config file [PR #146](https://github.com/model-bakers/model_bakery/pull/146)
- [dev] Update Dependabot config file to support GH Actions and auto-rebase [PR #160](https://github.com/model-bakers/model_bakery/pull/160)

Expand Down
13 changes: 7 additions & 6 deletions model_bakery/baker.py
Original file line number Diff line number Diff line change
Expand Up @@ -531,10 +531,11 @@ def generate_value(self, field: Field, commit: bool = True) -> Any:
"""Call the associated generator with a field passing all required args.
Generator Resolution Precedence Order:
-- attr_mapping - mapping per attribute name
-- choices -- mapping from avaiable field choices
-- type_mapping - mapping from user defined type associated generators
-- default_mapping - mapping from pre-defined type associated
-- `field.default` - model field default value, unless explicitly overwritten during baking
-- `attr_mapping` - mapping per attribute name
-- `choices` -- mapping from available field choices
-- `type_mapping` - mapping from user defined type associated generators
-- `default_mapping` - mapping from pre-defined type associated
generators
`attr_mapping` and `type_mapping` can be defined easily overwriting the
Expand All @@ -543,8 +544,8 @@ def generate_value(self, field: Field, commit: bool = True) -> Any:
is_content_type_fk = isinstance(field, ForeignKey) and issubclass(
self._remote_field(field).model, contenttypes.models.ContentType
)

if field.has_default():
# we only use default unless the field is overwritten in `self.rel_fields`
if field.has_default() and field.name not in self.rel_fields:
if callable(field.default):
return field.default()
return field.default
Expand Down
28 changes: 28 additions & 0 deletions tests/generic/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,10 @@ class LonelyPerson(models.Model):
only_friend = models.OneToOneField(Person, on_delete=models.CASCADE)


class Cake(models.Model):
name = models.CharField(max_length=64)


class RelatedNamesModel(models.Model):
name = models.CharField(max_length=256)
one_to_one = models.OneToOneField(
Expand All @@ -191,6 +195,30 @@ class RelatedNamesModel(models.Model):
)


def get_default_cake_id():
instance, _ = Cake.objects.get_or_create(name="Muffin")
return instance.id


class RelatedNamesWithDefaultsModel(models.Model):
name = models.CharField(max_length=256, default="Bravo")
cake = models.ForeignKey(
Cake,
on_delete=models.CASCADE,
default=get_default_cake_id,
)


class RelatedNamesWithEmptyDefaultsModel(models.Model):
name = models.CharField(max_length=256, default="Bravo")
cake = models.ForeignKey(
Cake,
on_delete=models.CASCADE,
null=True,
default=None,
)


class ModelWithOverridedSave(Dog):
def save(self, *args, **kwargs):
self.owner = kwargs.pop("owner")
Expand Down
31 changes: 31 additions & 0 deletions tests/test_filling_fields.py
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,37 @@ def test_filling_content_type_field(self):
assert dummy.content_type.model_class() is not None


@pytest.mark.django_db
class TestFillingForeignKeyFieldWithDefaultFunctionReturningId:
def test_filling_foreignkey_with_default_id(self):
dummy = baker.make(models.RelatedNamesWithDefaultsModel)
assert dummy.cake.__class__.objects.count() == 1
assert dummy.cake.id == models.get_default_cake_id()
assert dummy.cake.name == "Muffin"

def test_filling_foreignkey_with_default_id_with_custom_arguments(self):
dummy = baker.make(
models.RelatedNamesWithDefaultsModel, cake__name="Baumkuchen"
)
assert dummy.cake.__class__.objects.count() == 1
assert dummy.cake.id == dummy.cake.__class__.objects.get().id
assert dummy.cake.name == "Baumkuchen"


@pytest.mark.django_db
class TestFillingOptionalForeignKeyField:
def test_not_filling_optional_foreignkey(self):
dummy = baker.make(models.RelatedNamesWithEmptyDefaultsModel)
assert dummy.cake is None

def test_filling_optional_foreignkey_implicitly(self):
dummy = baker.make(
models.RelatedNamesWithEmptyDefaultsModel, cake__name="Carrot cake"
)
assert dummy.cake.__class__.objects.count() == 1
assert dummy.cake.name == "Carrot cake"


@pytest.mark.django_db
class TestsFillingFileField:
def test_filling_file_field(self):
Expand Down

0 comments on commit 297745b

Please sign in to comment.