[Freeipa-devel] [PATCH] 546 webui: Datetime parsing and formatting

[Misnyovszki Adam]

On Tue, 25 Feb 2014 18:05:28 +0100
Petr Vobornik <pvoborni at redhat.com> wrote:

> prerequisite for patch 547, 548
> depends on tbabej's datetime patch
> 
> this patch implements:
> - output_formatter in field. It should be used in par with formatter. 
> Formatter serves for datasource->widget conversion, output_formatter
> for widget->datasource format conversion.
> - datetime module which parses/format strings in subset of ISO 8601
> and LDAP generalized time format to Date.
> - utc formatter replaced with new datetime formatter
> - datetime_validator introduced
> - new datetime field, extension of text field, which by default uses 
> datetime formatter and validator
> 
> Dojo was regenerated to include dojo/string module
> 
> https://fedorahosted.org/freeipa/ticket/4194

Hi,
these are the results of my review:
- if incorrect number specified for any of the parts(ie 2013-01-01
  25:00:00), then it counts forward(result: 2013-01-02 01:00:00), does
  it supposed to work this way? at least some warning should be given
  to the user, that the date is incorrect(imho)
- couldn't test non utc datetime input(no test cases in the ui yet),
  but other tests(integration and ui) passed which are connected to
  this issue
- validity fields accept non existing timeframe(ie start: 2013-01-01
  00:00:00Z, end: 2012-01-01 00:00:00Z)
- validity fields only accept UTC time, it's good

so besides that timeframe issue(which the api should handle i think),
it's an ACK

Be careful, it should be pushed to master with pvoborni's 531-541 and
546-548, wait for the review of those!

Greets:
Adam