Fixed #5363 -- HTML5 datetime-local valid format HTMLFormRenderer #9365
Conversation
|
Hi @auvipy! Following your comment on #5363 (comment) I ask if you could do the review! Thank you! |
|
This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
Thanks @auvipy! I'll do it! |
|
Just an update here. I'm still active! I'll do as soon as possible |
|
I finally did it, sorry for the long delay! |
There was a problem hiding this comment.
Pull Request Overview
This PR fixes HTML5 datetime-local input formatting by truncating milliseconds to 3 digits to ensure browser compatibility. The HTML5 datetime-local input type specification only supports up to 3 digits of milliseconds precision, but DRF was potentially outputting more digits which caused browser validation errors.
- Modifies the datetime-local field value formatting in HTMLFormRenderer to limit milliseconds to 3 digits
- Adds a test case to verify the correct datetime-local input formatting
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| rest_framework/renderers.py | Updates datetime-local field formatting to truncate milliseconds to 3 digits |
| tests/test_renderers.py | Adds test case to verify datetime-local input formatting with milliseconds |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Thanks. I have two more remarks:
- Timezones currently get dropped, but I assume that's fine because this is about
datetime-localwhich "represent[s] a local date and time, with no time-zone offset information". - The transformation assumes that the string format is iso-8601. It doesn't work if I specify, for example,
appointment = serializers.DateTimeField(format='%a %d %b %Y, %I:%M%p'). Shouldn't the internal value be used as an input?
|
Thank you Peter! Not sure if I'm following: do you mean use the datetime value instead of the input.value (string)? So let me know if you want to fix this in this pull request |
Yes, that's what I meant. Sorry for being unclear! |
|
I tried to use this branch on a Django project with |
|
Correct! I'll go for that approach and add one more test for that case Bruno! Thanks! |
| datetime_value = field._field.parent.validated_data.get(field.field_name) | ||
| field.value = datetime_value.replace(tzinfo=None).isoformat(timespec="milliseconds").rstrip('Z') |
There was a problem hiding this comment.
It was harder than I thought!
Let me know if this is the correct way to get the datetime value.
There was a problem hiding this comment.
I'm not sure. Is it guaranteed that there is always a parent?
db93f6f to
6d5cff3
Compare
6d5cff3 to
0c2b20c
Compare
| @@ -488,6 +489,68 @@ class TestSerializer(serializers.Serializer): | |||
| assert rendered == '' | |||
|
|
|||
|
|
|||
| class TestDateTimeFieldHTMLFormRender(TestCase): | |||
There was a problem hiding this comment.
It would be nice to add a couple of test cases with some non-naive datetimes, with a timezone specified. Could try with UTC and another timezone where the offset is non-zero
|
@browniebroke and @peterthomassen thank you so much for your quick reviews and guidance! 🙏
Sure, I'll add a new test with timezone!
Good question. I didn't fully understand what is parent so I'll read more code to be sure and add new cases if needed! |
Description
Hi!
This PR delete everything after the first 3 digits of the miliseconds part of a
DateTimeFieldinput atHTMLFormRendererto avoid break in browsers:https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/datetime-local#try_it
Fix #5363