Fix HTMLFormRenderer to ensure a valid datetime-local format#9365
Fix HTMLFormRenderer to ensure a valid datetime-local format#9365auvipy merged 3 commits intoencode:mainfrom
HTMLFormRenderer to ensure a valid datetime-local format#9365Conversation
|
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. |
auvipy
left a comment
There was a problem hiding this comment.
please address the review comment
|
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.
peterthomassen
left a comment
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! |
6d5cff3 to
0c2b20c
Compare
|
@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! |
Yes!! Of course, I can improve that! |
Tests are prettier now! (I hope so) |
|
I've also renamed the |
5468bca to
89efdab
Compare
|
@peterthomassen I would need your email to add you as |
What makes you think that? -- I believe it is not. There are also multiple functions in DRF that define variables with that name (you can find some via
Thanks, but that's not necessary :-) Anyway, by address is "my first name" at desec dot io. |
|
I meant this builtin function: https://docs.python.org/3/library/functions.html#format I realized when editor highlighted to me |
|
Ahhh correct! Thank you Peter!!! |
89efdab to
dba1179
Compare
Co-authored-by: Peter Thomassen
dba1179 to
d275c59
Compare
HTMLFormRenderer to ensure a valid datetime-local format
| except AttributeError: | ||
| format_ = api_settings.DATETIME_FORMAT | ||
|
|
||
| if format_ is not None: |
There was a problem hiding this comment.
This seems to allow another type of failure if the format is None.
| field = field.as_form_field() | ||
|
|
||
| if style.get('input_type') == 'datetime-local' and isinstance(field.value, str): | ||
| field.value = field.value.rstrip('Z') |
There was a problem hiding this comment.
The previous code allowed blank values, the new one doesn't. This makes rendering fail if no defaults are specified.
| datetime(2024, 12, 24, 0, 55, 30, 345678, tzinfo=ZoneInfo('Asia/Tokyo')), # +09:00 | ||
| "2024-12-23T09:55:30.345" # Rendered in -06:00 | ||
| ) | ||
|
|
There was a problem hiding this comment.
Tests are missing the case where the value is empty (allowed and working in the previous drf release).
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
Please execute
./runtests.py TestDateTimeFieldHTMLFormRenderto run the new testsFix #5363