model_bakery
model_bakery copied to clipboard
model-bakers
Can't set the value of a DateTimeField with auto_now or auto_now_add
To reproduce
# models.py
from django.db import models
class MyModel(models.Model):
some_date = models.DateTimeField(auto_now_add=True)
# test code
from model_mommy import mommy
thing = mommy.make(MyModel, some_date="1985-10-21")
print(thing.some_date)
Expected:
- The specified date should be printed with time set to midnight
Actual:
- The current datetime is printed
This is to do with the DateTimeField's pre_save method, which overrides the initial value with the current date if auto_now or auto_now_add are true.
In my opinion, since it is internal Django functionality that was made for certain purpose (like for created/modified fields) we should not mess around it.
To be more specific: See docs: https://docs.djangoproject.com/en/3.0/ref/models/fields/#datefield There is even a next statement:
The auto_now and auto_now_add options will always use the date in the default timezone at the moment of creation or update. If you need something different, you may want to consider using your own callable default or overriding save() instead of using auto_now or auto_now_add; or using a DateTimeField instead of a DateField and deciding how to handle the conversion from datetime to date at display time.
So I would be in for being explicit and consistent behavior, so there would be no difference between using this Django internal or custom app logic which pre_saves some fields values. This logic should be seen and handled in tests. So I would be interested in seeing @ukch needs here and probably would suggest using some helper libraries for such cases, like https://github.com/spulec/freezegun
Now, regarding model_bakery here, I think, if this is something that is not explicit, we might want to track that users are overriding auto_now/auto_now_add values, that would be ignored by Django and throw a warning. But then again, we would not do this for any custom field logic that might appear there in the wild. So I do not know if we want to even do this. Or maybe just documenting this behavior would be enough?
Makes sense, @amureki. Of course, we can always add this as a note in the docs but, as you said, model_bakery must have the same behavior as Django. cc @berinhard
Thanks for the thoughtful explanation @amureki! Sorry for taking to long to read it, but I was changing jobs during these last 2 months and I had to stay a little bit away from the project.
I agree with both strategies, the warning one and also improving our docs to leave this question more explicit. But I think the warning should only be raised if:
The field has auto_now or auto_now_add enabled and the user is passing a custom value for this field via baker.make or baker.prepare. We can even suggest the developer to use freezegun to test the auto_now stuff in the warning message.
Can I close this issue and open a new one with proper instruction on this work @amureki and @anapaulagomes ?
Hey Bernardo, no worries! I am also quite overwhelmed and was not very active here.
So, I'd be happy about the documentation warning about signals and this behaviour.
We could add an extra check for auto_now or auto_now_add also. This brings (in my opinion) a small inconsistency (if I see the lib is covering some signals and throws a warning, I'd expect it to cover all of them - but this is super-tricky), but might be fine, because auto_now or auto_now_add is commonly used and might bring some pain to developers to debug what is going on.
In my case this little feature with explicit overriding fields with auto_now_add will be very helpful in tests.
E.g. test which checks that custom filter by date (created recently) works as expected: I created a few instances of a model with different created_at and run filtering against it. Using freezgun is painful in the case when you need to create many models with different values at once.
Besides the point raised by @b0g3r, the freezegun approach would be problematic when auto_now is paired with null=True, which is exactly the case I have on hand right now, so I had to do the following:
# models.py
from django.db import models
class MyModel(models.Model):
created_at = models.DateTimeField(auto_now=True, null=True)
def ignore_future(today, instances):
from_future = Q(created_at__gt=today)
return instances.exclude(from_future)
# test code
from datetime import datetime
from model_bakery.baker import make
class TestMyModel
def make_my_model(self, **kwargs):
created_at = kwargs.pop("created_at", None)
my_model = make(MyModel, **kwargs)
my_model.created_at = created_at
my_model.save()
return my_model
def test_ignore_the_future(self):
self.make_my_model()
self.make_my_model(datetime(2001, 1, 2))
result = ignore_future(today=datetime(2001, 1, 1), instances=MyModel.objects.all())
assert [] == list(result)
It makes sense for me to override the created_at value, given that it has been explicitly passed. I don't see how user intention could be different in that case.
What do you all think? Should I go ahead with an implementation proposal?
Hey @israelst thank you for following up on this one and providing another use case! 👍
If you have something in mind, we'd be happy to see your implementation proposal, sure! If you want, feel free to open a PR with new failing test cases and do a proposal right there. Otherwise, we can discuss it here if you don't feel confident enough about the codebase.
Cheers, Rust
You can set the value of created_at with queryset update function.
Breed.objects.filter(pk=8).update(created_at =a)
