JonasWanke / rrule

🔁 Recurrence rule parsing & calculation as defined in the iCalendar RFC
https://pub.dev/packages/rrule
Apache License 2.0
52 stars 22 forks source link

toText dont take local time into account #39

Closed loicgeek closed 2 years ago

loicgeek commented 2 years ago

Describe the bug While creating a recurrence, We convert from local time to utc time, but when we try to show to the user with toText(), it shows the utc time but it should show the Local time.

example: My Time zone is UTC+1;

var untilDate = DateTime.parse('2022-06-16 14:57:30'); The generate recurrence is :

var rr = RecurrenceRule (RRULE:FREQ=DAILY;UNTIL=20220616T135700;INTERVAL=1);

When I use rr.toText() is shows until 2022-06-16 13:57:30 but it show show 2022-06-16 14:57:30

Environment:

rrule: 0.2.9

JonasWanke commented 2 years ago

This is intentional due to rrule's handling of DateTime and isUtc: rrule doesn't care about any time-zone-related stuff. All supplied DateTimes must have isUtc set to true (because UTC doesn't have complications like summer and winter time), but the actual time zone is then ignored, e.g., when generating human-readable text.

If you have a DateTime instance in the local time zone, you can use a newly exported helper from v0.2.10: dateTime.copyWith(isUtc: true) keeps the date and time but sets isUtc to true. To convert the generated instances back to your local time zone, you can use dateTime.copyWith(isUtc: false).

I've updated the README to make this clearer as part of v0.2.10.

loicgeek commented 2 years ago

Hello @JonasWanke thanks for your response, but may be I did not explain the issue the right way... Well, I understand that for convenience every dates should be in UTC when passing to rrule, but when printing with toText(), the user is not in the UTC zone, so it should not print the utc date, it should print the correct local time to the user: I dont want to keeps the local time as UTC time, because that same time is used on the Web app also, and there we know that every generated time is in UTC.

loicgeek commented 2 years ago

so basically let me take and example, I want to create an rrule: "every day, until 26 Jun,2022 at 10am"; var untildate = DateTime(2022,6,26,10);

var rr  = ReccurrenceRule( 
      frequency:Frequency.daily,
      until: untilDate.toUtc() 
);

my timezone is UTC+1 if I print rr.toText() the ouput will be 26 Jun,2022 at 9am, but this is UTC Time, not the user Time.

@JonasWanke I hope you can understand with this.

JonasWanke commented 2 years ago

Maybe I should clarify an important difference: isUtc must be true for all DateTimes, but that doesn't mean they should be the UTC representations of dates in your local time zone.

Date and Time Epoch Millis
dateTime (in UTC+1) 2022-06-26 at 10:00:00.000 (in UTC+1) 1654844400000
dateTime.toUtc() 2022-06-26 at 09:00:00.000 (in UTC+0) 1654844400000
dateTime.copyWith(isUtc: true) 2022-06-26 at 10:00:00.000 (in UTC+0) 1654840800000

If you convert your dateTime to UTC by using dateTime.toUtc(), it will represent the same moment in time (epoch millis are the same), but in a different time zone. This will produce the incorrect results you see.

Instead, you should call dateTime.copyWith(isUtc: true) (exported by rrule), which creates a DateTime instance with the same year, month, date, hour, minute, second, and millisecond, but tells Dart to handle it as being in UTC – hence its epoch millis are different if your local time zone doesn't match UTC. But, because rrule doesn't operate on epoch millis but only on date and time, this works fine and produces the correct results.

(The underlying problem is Dart's very limited DateTime: I'm not using DateTime.utc(…) to represent a DateTime in UTC, but to represent a DateTime independent of time zones because these are not relevant for calculations. Other languages like Kotlin/Java or C# have different types for these: Instant is a point in time, whereas LocalDateTime only has a date and time, but no associated time zone and hence doesn't uniquely refer to a point in time. Unfortunately, the only package implementing something similar in Dart which I've also used previously, namely time_machine, isn't really maintained anymore.)

loicgeek commented 2 years ago

I understand very well your point....but using dateTime,copyWith(isUtc:true) will make other calculations (on backend and on website ) fails because it is not the actual Utc date. using dateTime,copyWith(isUtc:true) will lost the actual time if we dont save the timezone used while copying isUtc. @JonasWanke

JonasWanke commented 2 years ago

You don't have to use dateTime.copyWith(isUtc: true) to your backend – this is only intended to be used by rrule. If you want to use the generated iterations in your backend, you can convert them back to the local time zone using dateTime.copyWith(isUtc: false) (or convert them to the actual UTC representation using dateTime.copyWith(isUtc: false).toUtc())

loicgeek commented 2 years ago

@JonasWanke The backend dont hold the local timezone of the user.

loicgeek commented 2 years ago

@JonasWanke the solution I proposed dont alter the rrule, its only render a differrent Output when using toText()

JonasWanke commented 2 years ago

But your proposed change of calling toLocal() when generating a human-readable text is a breaking change that's inconsistent with the rest of this package. You can represent the DateTime on your backend however you like. Only when you use the rrule package, you have to convert the DateTime once using .copyWith(isUtc: true) to instantiate the RecurrenceRule or convert calculated iterations back using .copyWith(isUtc: false)