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

Can't set the value of a DateTimeField with auto_now or auto_now_add #23 #451

Merged
merged 9 commits into from
Oct 27, 2023

Conversation

rodbv
Copy link
Contributor

@rodbv rodbv commented Oct 7, 2023

Describe the change
A clear and concise description of what the change is.

PR Checklist

@rodbv
Copy link
Contributor Author

rodbv commented Oct 7, 2023

I've faced an issue similar to this and gave it a go. One issue with this implementation is that it only works if either _refresh_after_create is set to True, or the user calls refresh_from_db() directly like I did on the test.

I could call instance.refresh_from_db() here but this would override _refresh_after_create

@amureki
Copy link
Collaborator

amureki commented Oct 14, 2023

@rodbv oh, many thanks for this attempt to tackle an annoying issue! 👍 This is promising!
I would need to dig more into the implementation, but meanwhile, may I ask you to look into the failing tests? I think, there are edge cases here that are causing errors.

Best,
Rust

@rodbv
Copy link
Contributor Author

rodbv commented Oct 19, 2023

@amureki thanks! I've fixed the tests and the linter error. When running the tests workflow on my repo there's an error on the ruff linter but it seems to be a setup issue.

Edit: Some tests are still failing with timezone issues, I'll get to them

@rodbv
Copy link
Contributor Author

rodbv commented Oct 23, 2023

I've dealt with my test failing depending on the use_tz setting. Ruff linter is still failing, is there anything I can do?

@amureki
Copy link
Collaborator

amureki commented Oct 25, 2023

I've dealt with my test failing depending on the use_tz setting. Ruff linter is still failing, is there anything I can do?

Thanks for fixing the tests! I'll take a look into tz stuff before the end of this week. And also will do a proper check on your code.
Ruff part should be resolved if you update your branch on the main.

Sorry for the complications on the contribution way.

@amureki
Copy link
Collaborator

amureki commented Oct 27, 2023

@rodbv I finally got some time and figured the issue. This was happening due to django.conf.settings tampering, which is usually not a good practice (but I know, we still have it in a couple of tests).
I updated your test to use a fixture provided by pytest-django, which solved the issue.

I also went ahead and fixed the last ruff problem, so it is good to go now, I am going to merge it and release now!

@amureki amureki merged commit 2a33cef into model-bakers:main Oct 27, 2023
10 checks passed
@amureki
Copy link
Collaborator

amureki commented Oct 27, 2023

I released it as 1.17.0

@rodbv
Copy link
Contributor Author

rodbv commented Oct 27, 2023

@rodbv I finally got some time and figured the issue. This was happening due to django.conf.settings tampering, which is usually not a good practice (but I know, we still have it in a couple of tests). I updated your test to use a fixture provided by pytest-django, which solved the issue.

I also went ahead and fixed the last ruff problem, so it is good to go now, I am going to merge it and release now!

Awesome! Thanks a lot for the support and for being so welcoming, I'm looking forward to contribute more, I'll have another look at the list of open issues :)

@rodbv rodbv deleted the rodbv/issue23 branch October 27, 2023 13:21
@amureki
Copy link
Collaborator

amureki commented Oct 27, 2023

@rodbv well, thank you for the contribution, patience and kind words, much appreciated! ✨

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.

2 participants