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

[116] Support Callable default and Always Use default If Present #117

Merged

Conversation

timjklein36
Copy link
Collaborator

What

This change fixes #116, the following bug:

If a field is of a class that does not have a generator (default or provided by the user) also has a default value which is a callable, the generated value is returned as the callable, not the result of calling the same.

For instance:

from django.db import models
from model_bakery import baker


class MyField(models.TextField):
    pass

class DummyModel(models.Model):
    test = MyField(default=lambda: "default value")


dummy = baker.make(DummyModel)

# Before fix
dummy.test  # <function <lambda> at 0x7f0eadb9dca0>

# After fix
dummy.test  # "default value"

This also addresses the fact that the default value for a field was not always being returned from Baker.generate_value since it was lower in the if, elif, else block which also checked for the generator function to use for that field's value.

Note: This PR takes the stance that: if default is present, that value should be used unless an explicit kwarg for that field is supplied to make.

Why

This will allow fields of classes that are unknown to model_bakery, yet have a desired default value, to be used without error.

- Also, always generate default value if `field.default` is supplied
  - Previously, this would not happen, since the `generator` determination
    was being done earlier in the same `if, elif` block

- Add unit tests for generating default value of callables of varying kinds
@anapaulagomes anapaulagomes self-requested a review October 10, 2020 14:29
Copy link
Contributor

@anapaulagomes anapaulagomes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 👍🏽

@timjklein36 timjklein36 changed the base branch from master to main October 12, 2020 20:28
Copy link
Member

@berinhard berinhard left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your PR @timjklein36

Comment on lines +57 to +58
assert person.enjoy_jards_macale is enjoy_jards_macale_field.default
assert person.like_metal_music is like_metal_music_field.default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for this catch! I didn't notice that the assert was wrong before

@berinhard berinhard merged commit a7e723e into model-bakers:main Oct 13, 2020
amureki pushed a commit that referenced this pull request Mar 9, 2021
#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).
berinhard pushed a commit that referenced this pull request Mar 12, 2021
* 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
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.

Support callables as default values in django model fields
4 participants