Why doesn't _fill_optional actually fill the optional fields?

[kilicluka]

Let's take this model for example:

class User(models.Model):
    name = models.CharField(max_length=64, blank=True, default="")
    last_name = models.CharField(max_length=64, blank=True, default="")
    dob = models.DateField(blank=True, null=True, default=None)

using

baker.make("user.User", _fill_optional=True)

or

baker.make("user.User")

will both give the same results. In both cases the user.name == "", user.last_name == "" and user.dob == None. My understanding after reading _fill_optional docs was that these values would all get randomly generated strings if I provided the argument.

Is there a specific reason for this or was it just an oversight?

[amureki]

Hey @kilicluka, thank you for your question!

_fill_optional indeed should set values for optional fields in the Django model. But there is a small quirk in your snippet - you are setting default values, which model bakery prioritizes over filling anything else:

model_bakery/model_bakery/baker.py

Lines 665 to 667 in a408873

	if field.has_default() and field.name not in self.rel_fields:
	if callable(field.default):
	return field.default()

We probably need to mention this in the documentation, as it is not quite explicit.

One way to solve I see is to explicitly point fields to use certain generators (either during test object creation or in the recipe):

User = Recipe(
    "User",
    name=random_gen.gen_string(64),
    last_name=random_gen.gen_string(64),
    dob=random_gen.gen_datetime(),
)

In theory, we could think of another flag, something like _override_default=True, that will handle such cases, but that'd require some agreeing within @model-bakers/core and a bit of developing if we'd want to continue.

What do you think?

[kilicluka]

Hey @amureki, thanks for the reply!

In theory, we could think of another flag, something like _override_default=True, that will handle such cases

I think this would be the ideal solution. If it's not feasible to implement, a more detailed documentation about this would suffice :)

[amureki]

@berinhard @timjklein36 what do you think, would such feature fit the package?

[timjklein36]

@amureki I haven't thought about it too deeply, however, at face value it seems like a fine idea. (As long as the meaning is clear, the default behavior remains unchanged, and the override behavior is well documented)