-
Notifications
You must be signed in to change notification settings - Fork 395
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix recurring events with unsupported rrules. #1465
Conversation
The android calendar provider does not support RRULE's with BYSETPOS or BYWEEKNO properly, so additional processing has to be done to ensure that duplicate events are not added to the calendar.
Date date = new java.util.Date(startTime); | ||
|
||
// We'll use SimpleDateFormat to get the D/M/Y but we also need to set the timezone. | ||
SimpleDateFormat sdf = new java.text.SimpleDateFormat(); | ||
sdf.setTimeZone(java.util.TimeZone.getTimeZone("GMT")); | ||
|
||
sdf.applyPattern("yyyy"); | ||
int startYear = Integer.parseInt(sdf.format(date)); | ||
sdf.applyPattern("MM"); | ||
int startMonth = Integer.parseInt(sdf.format(date)) - 1; | ||
sdf.applyPattern("dd"); | ||
int startDay = Integer.parseInt(sdf.format(date)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't it be preferrable to use java.time.LocalDateTime
instead and work with epoch, since you have no associated timezone?
Untested and uncompiled:
long firstInstance = LocalDateTime
.ofEpochSecond(startTime / 1000, (startTime % 1000) * 1000000)
.toLocalDate() // clamping to day start
.toInstant()
.toEpochMillis();
// comparison:
if (instance.getTimestamp() != firstInstance) {
return 0;
} // etc.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It needs to resolve out to a DateTime
object to pass into RecurrenceSet()
, so I could work with epoch's in some of the code, but in the end it still needs to be a DateTime
object, so no real point in doing both.
thanks for your contribution, this looks fine to me, but I have not tested it. |
Tested and works. Thanks for solving the issue. |
The android calendar provider does not support RRULE's with BYSETPOS or BYWEEKNO properly, so additional processing has to be done to ensure that duplicate events are not added to the calendar.
This is accomplished by using a separate RRULE processor, lib-recur, to double check that the instance is valid.
Resolves #1132