More helpful error message when loading fixture with invalid date

[Knut Nesheim]

It would be extremely helpful to have a more helpful error message when loading a fixture an invalid date. This would allow me to actually see the invalid data and I may then correct the data.

Attached is a very simple patch that will include the invalid data in the exception string. It would be extremely helpful to see more data, like the model and pk.

[Raúl Cumplido]

Pull Request provided ​here at github with tests.

[Julien Phalip]

The patch looks good. While we're at it, we should also do the same for DateField's "invalid_date" and DateTimeField's "invalid" messages. See amended patch attached.

[Simon Charette]

Replying to julien:

The patch looks good. While we're at it, we should also do the same for DateField's "invalid_date" and DateTimeField's "invalid" messages. See amended patch attached.

That would be a great addition. I have one question concerning the attached patch.

How are the ValueError risen by datetime.date calls supposed to be translated? I see you're calling _(str(e)) before formating the invalid_date message but I can't find where django is supposed to catch i.e. _('month must be in 1..12') when makemessages logic is invoked.

[Julien Phalip]

That's a good question. One way would be to purposely trigger exceptions while the module is loaded to gather all the various error messages from datetime.datetime(), but that feels dirty... This is also dangerous as there's no guarantee that the messages will be the same on every version of Python.

The most complete way would be for Django to do the same tests as datetime.datetime() and then provide its own messages. However, this wouldn't be very robust and would be annoying to maintain.

Perhaps the safest way is to simply not give any granular error message and just say that it's an invalid date, as suggested in the new patch attached.

[Julien Phalip]

In [16966]:

Fixed #12467 -- Made the model data validation for DateField and DateTimeField more useful by actually telling what was the value that failed. Also did a bit of PEP8 cleanup in the area. Thanks to knutin for the report, to raulcd for the initial patch and to charettes for the review.

[Anssi Kääriäinen]

The commit made a couple of always True asserts:

/home/akaj/Django/django_test/django/db/models/fields/__init__.py:513: SyntaxWarning: assertion is always true, perhaps remove parentheses?
  assert (kwargs.get('primary_key', False) is True,
/home/akaj/Django/django_test/django/db/models/fields/__init__.py:539: SyntaxWarning: assertion is always true, perhaps remove parentheses?

[Julien Phalip]

Well spotted. This is weird, I didn't see those warnings when running the tests. I'll fix that in a minute. Thanks!

[Julien Phalip]

In [16971]:

Fixed a couple of assert syntax warnings mistakingly introduced in r16966 and added some tests to prevent future regressions. Thanks to Anssi Kääriäinen for the report.
Refs #12467.