killbill / killbill-plugin-framework-ruby

Framework to write Kill Bill plugins in Ruby
http://killbill.io
8 stars 11 forks source link

Improve timezone conversion or error reporting #54

Closed pierre closed 5 years ago

pierre commented 8 years ago

Consider the following:

jruby-1.7.21 > jtz = Java::org.joda.time.DateTimeZone.forID('CET')
 => #<Java::OrgJodaTimeTz::CachedDateTimeZone:0x6bf26637>
jruby-1.7.21 > TZInfo::Timezone.get(jtz.get_id)
 => #<TZInfo::DataTimezone: CET>

jruby-1.7.21 > jtz = Java::org.joda.time.DateTimeZone.forID('+01:00')
 => #<Java::OrgJodaTimeTz::FixedDateTimeZone:0x1980077e>
jruby-1.7.21 > TZInfo::Timezone.get(jtz.get_id)
TZInfo::InvalidTimezoneIdentifier: Invalid identifier

The issue is that "+0100" does not have to be in Central European Time, as other time zones could have the same UTC offset.

neptoon commented 8 years ago

As mentioned in the associated mailing list thread (https://groups.google.com/forum/#!topic/killbilling-users/oDUmYbzmhSs) it might be more of an issue in Kaui which uses the offset format instead of timezone identifiers.

pierre commented 8 years ago

See https://github.com/killbill/killbill-admin-ui/issues/62 for tracking the Kaui bug.

pierre commented 7 years ago

While the Kaui bug has been fixed (and https://github.com/killbill/killbill-plugin-framework-ruby/blob/4681bfb8caafda0b3192ce368cb2e2e12cb17659/lib/killbill/gen/api/account.rb#L66 is correct now), the generator still needs to be fixed.

Specifically https://github.com/killbill/killbill-plugin-framework-ruby/blob/4681bfb8caafda0b3192ce368cb2e2e12cb17659/lib/killbill/gen/api/account.rb#L77 is wrong and will always fail since fixed_offset_time_zone is computed using DateTimeZone.forOffsetMillis. We should probably just return a string (e.g. -08:00) in Ruby.