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

[Petr Viktorin]

On 03/13/2014 03:45 PM, Misnyovszki Adam wrote:
> On Thu, 06 Mar 2014 13:26:03 +0100
> Petr Vobornik <pvoborni at redhat.com> wrote:
>
>> On 6.3.2014 13:01, Misnyovszki Adam wrote:
>>> 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)
>>
>> It's standard behavior of JavaScript Date object's setUTCFullYear
>> method. I did not find better methods which would not require pulling
>> third-party lib or do real evaluation of the dates.
>>
>> In the end it's not that bad.
>>
>> https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/setUTCFullYear
>>
>>> - 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
>>
>> Non UTC are not supported therefore it's disabled. But there are unit
>> tests in test/utils_tests.js
>>
>>> - validity fields accept non existing timeframe(ie start: 2013-01-01
>>>     00:00:00Z, end: 2012-01-01 00:00:00Z)
>>
>> I don't think it's checked even on a server. Maybe it should be.
>>
>>> - 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
>>
>>
>
>
> ACK,
> can push to master.
> Adam

Pushed to master: 870a5daf24c165069fe6f546bb3710f9b3880538



-- 
Petr³