On Thu, 06 Mar 2014 13:26:03 +0100 Petr Vobornik <[email protected]> wrote:
> On 6.3.2014 13:01, Misnyovszki Adam wrote: > > On Tue, 25 Feb 2014 18:05:28 +0100 > > Petr Vobornik <[email protected]> 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 _______________________________________________ Freeipa-devel mailing list [email protected] https://www.redhat.com/mailman/listinfo/freeipa-devel
