Fixed #5363 -- HTML5 datetime-local valid format HTMLFormRenderer

[mgaligniana]

Description

Hi!

This PR delete everything after the first 3 digits of the miliseconds part of a DateTimeField input at HTMLFormRenderer to avoid break in browsers:

https://developer.mozilla.org/en-US/docs/Web/HTML/Reference/Elements/input/datetime-local#try_it

Please execute ./runtests.py TestDateTimeFieldHTMLFormRender to run the new tests

Fix #5363

[mgaligniana]

Hi @auvipy! Following your comment on #5363 (comment) I ask if you could do the review! Thank you!

[stale]

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]

please address the review comment

[mgaligniana]

Thanks @auvipy! I'll do it!

[mgaligniana]

Just an update here. I'm still active! I'll do as soon as possible

[mgaligniana]

I finally did it, sorry for the long delay!

[Copilot]

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]

Thanks. I have two more remarks:

• Timezones currently get dropped, but I assume that's fine because this is about datetime-local which "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?

[mgaligniana]

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

[peterthomassen]

do you mean use the datetime value instead of the input.value (string)?

Yes, that's what I meant. Sorry for being unclear!

[browniebroke]

I tried to use this branch on a Django project with TIME_ZONE="Europe/Paris" and I'm getting a similar error as the original report, bacause the timezone is not Z but +02:00 at the end. While it feels a bit out of scope of what this fix is trying to do, this last suggestion could address it as well?

[mgaligniana]

Correct! I'll go for that approach and add one more test for that case Bruno! Thanks!

stale

[mgaligniana]

@browniebroke and @peterthomassen thank you so much for your quick reviews and guidance! 🙏

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

Sure, I'll add a new test with timezone!

I'm not sure. Is it guaranteed that there is always a parent?

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!

[Copilot]

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[Copilot]

Pull Request Overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.

---

_{Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.}

[mgaligniana]

Hello team! I went for a new approach: get datetime string and transform to datetime object to better manipulation.

I've added 7 tests in total! I think I covered all possible cases!

[peterthomassen]

I like the new approach, but I don't understand the typing considerations. You probably have thought about that -- any insights on my comments?

Also, the tests have quite some code duplication (I'm not sure if that is a problem).

[mgaligniana]

Also, the tests have quite some code duplication (I'm not sure if that is a problem).

Yes!! Of course, I can improve that!

[mgaligniana]

Also, the tests have quite some code duplication (I'm not sure if that is a problem).

Yes!! Of course, I can improve that!

Tests are prettier now! (I hope so)

[mgaligniana]

I've also renamed the format to format_ as it is a Python reserved word

[mgaligniana]

@peterthomassen I would need your email to add you as Co-authored-by I think (I rebased it to make it only 1 commit)

[peterthomassen]

I've also renamed the format to format_ as it is a Python reserved word

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 git grep 'def .*format=').

@peterthomassen I would need your email to add you as Co-authored-by I think (I rebased it to make it only 1 commit)

Thanks, but that's not necessary :-) Anyway, by address is "my first name" at desec dot io.

[mgaligniana]

I meant this builtin function: https://docs.python.org/3/library/functions.html#format I realized when editor highlighted to me

[peterthomassen]

Ah, I understand what you mean. (That's not reserved, but a built-in, which is shadowed when you redeclare it.)

Anyway, looks great to me now!

[mgaligniana]

Ahhh correct! Thank you Peter!!!

[browniebroke]

I think I wasn't clear enough when I wrote "naive datetime". I was talking about the naive in the pure Python sense, which is outline in this paragraph of the Python docs: https://docs.python.org/3.9/library/datetime.html#aware-and-naive-objects

In practical sense, I think that this test should not only set the TIME_ZONE Django setting, but also set the tzinfo attribute on the datetime object itself:

from zoneinfo import ZoneInfo

datetime(2024, 12, 24, 0, 55, 30, 345678, tz_info=ZoneInfo('Asia/Tokyo'))

[mgaligniana]

Thank you @browniebroke !

I've modified and re-named as this: test_datetime_field_rendering_timezone_aware_datetime

Let me if its what you had in mind!

[browniebroke]

Yes, looks good. Not sure why the tests fail on Django 4.2 though... 🤔

[mgaligniana]

Mmm, I found it! In Django 4.1 is returing value="2024-12-23T15:55:30.345" instead of value="2024-12-23T09:55:30.345"

[mgaligniana]

Should we skip the test for that version as it is no longer maintenable? 🤔 That is Django 4.1

[mgaligniana]

I FOUND IT!

In django 4.1, 4.2 USE_TZ was False! https://docs.djangoproject.com/en/4.2/ref/settings/#std-setting-USE_TZ

[browniebroke]

Do you still need to remove the UTC timezone manually (with .rstrip('Z'))? It works as well without it, so I think that datetime.datetime.fromisoformat handles it out of the box.

The TZ info is stripped line 362 with field.value.replace(tzinfo=None) and that should handle any timezone.

[mgaligniana]

Thanks for your fast response!

Mmm if I remove the .rstrip('Z') test_datetime_field_rendering_utc fails

[mgaligniana]

>>> import datetime
>>> value = '2024-12-24T00:55:30.345678Z'
>>> datetime.datetime.fromisoformat(value).replace(tzinfo=None)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
ValueError: Invalid isoformat string: '2024-12-24T00:55:30.345678Z'