DateTime field: allow timestamp 0 #2264
Conversation
| @@ -576,7 +577,7 @@ def test_timestamp_field_deserialization(self, fmt, value, expected): | |||
| @pytest.mark.parametrize("fmt", ["timestamp", "timestamp_ms"]) | |||
| @pytest.mark.parametrize( | |||
| "in_value", | |||
| ["", "!@#", 0, -1, dt.datetime(2013, 11, 10, 1, 23, 45)], | |||
| ["", "!@#", -1, dt.datetime(2013, 11, 10, 1, 23, 45)], | |||
There was a problem hiding this comment.
Nit: might be worth testing None here
There was a problem hiding this comment.
None fails validation before reaching _deserialize.
ValidationError: Field may not be null.
| @@ -422,6 +422,7 @@ def test_field_toggle_show_invalid_value_in_error_message(self): | |||
| [ | |||
| "not-a-datetime", | |||
| 42, | |||
| 0, | |||
There was a problem hiding this comment.
thought(non-blocking): I believe this will also make False a valid value which isn't totally intuitive. But I'm not sure there's a way around that
There was a problem hiding this comment.
I added a condition to reject boolean inputs, copied from numeric fields above.
There was a problem hiding this comment.
I would be tempted to inherit avoid duplicating that logic. Float to match datetime.fromtimestamp()'s type signature andThat would also allow removing value = float(value) from utils.from_timestamp() which is equally guilty for the unexpected bool to int promotion.
There was a problem hiding this comment.
On second thought inheritance wouldn't really work with the other date formats.
I would push this logic down to utils.from_timestamp() since all of the other date time utils.from_* properly error on bools already.
There was a problem hiding this comment.
Right. I added your commit to the PR.
Fixes #2133.