Skip to content
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

Duplicate events returned on a day when one instance of a recurring event changes time #45

Closed
carloshanson opened this issue Mar 9, 2019 · 11 comments · Fixed by #80
Closed

Comments

@carloshanson
Copy link
Contributor

carloshanson commented Mar 9, 2019

A recurring event was created in the past.
One instance had a time change.
On the date of the event, two events are returned.

The combination of the UID and the SEQUENCE needs to be observed. In this case, both events have the same UID in the calendar, so the one with the higher SEQUENCE should be returned.

Right now, recurring events are copied with each new date and the UID is changed for each by appending a random integer. I don't think the UID should be changed, but I need to test some different recurring scenarios to be sure that keeping the same UID will be okay.

I will work on this, but feel free to add any suggestions while I do.

@carloshanson
Copy link
Contributor Author

Using Google Calendar, recurrence is limited by day, so leaving the UID the same for each recurring event would allow checking UID and SEQUENCE for each day before returning found events.

@fvox13
Copy link

fvox13 commented Mar 12, 2019

@carloshanson I'm having the same issue, but with an Apple iCalendar feed. Do you have any workaround, or thoughts as to code or pseudocode for a patch? I'm keen to fix this and I don't want to start from scratch if you had something already.

@carloshanson
Copy link
Contributor Author

@fvox13 My initial though is the found events should be a dictionary instead of a list. The key could be a tuple of the UID and the date. The value could be a tuple of the SEQUENCE and the event. I haven't had a chance to play with it further yet, but I hope to work on it this week. When adding a found event, only add it if the SEQUENCE is greater than the one already found.

We have to prevent changing the UID of the recurring events, and add SEQUENCE to an Event.

I haven't had a chance to play with it yet, but I hope to do so this week.

@jc1518
Copy link

jc1518 commented May 10, 2019

@carloshanson I am seeing the same issue in Confluence team calendar. Have you made any progress on this one?

@carloshanson
Copy link
Contributor Author

My pull request should resolve the issue, but it breaks the tests. I had hoped that it would be looked and and the tests changed. I consider changing the tests myself. I'll look at it again.

@jc1518
Copy link

jc1518 commented May 10, 2019

@carloshanson Thanks! I have downloaded the parser source code from your branch, and tested against the confluence team calendar. It looks good to me!

@jc1518
Copy link

jc1518 commented May 11, 2019

@carloshanson Just found out that the fix you put in could not handle some cases which need "UID", "Sequence" and "Reference-id" to identify the event instance.

Reference: https://icalendar.org/iCalendar-RFC-5545/3-8-4-4-recurrence-id.html

@jc1518
Copy link

jc1518 commented May 11, 2019

Here is a example. The code returns event id: 68170 and 69069, which are supposed to 83168 and 83169.

BEGIN:VEVENT
DTSTAMP:20190510T042124Z
DTSTART;TZID=Australia/Sydney:20181015T090000
DTEND;TZID=Australia/Sydney:20181022T085900
SUMMARY:Primary on-call
X-CONFLUENCE-CUSTOM-TYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
CATEGORIES:On-call support
SUBCALENDAR-ID:036ce37a-a1d4-402f-b5ee-22b0cd46fca8
PARENT-CALENDAR-ID:648843fb-68a3-4840-a0f6-c59e851c4188
PARENT-CALENDAR-NAME:
SUBSCRIPTION-ID:
SUBCALENDAR-TZ-ID:Australia/Sydney
SUBCALENDAR-NAME:Production Support
EVENT-ID:68170
EVENT-ALLDAY:false
CUSTOM-EVENTTYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
UID:20181018T122154Z-201879622@confluence.mydomain.com
DESCRIPTION:
ORGANIZER;X-CONFLUENCE-USER-KEY=8a0b193243717ee90143adbdf61c0028;CN=Staff 
 Leader;CUTYPE=INDIVIDUAL:mailto:Staff.Leader@mydomain.com
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO
CREATED:20181018T122154Z
LAST-MODIFIED:20190318T042532Z
SEQUENCE:6
X-CONFLUENCE-SUBCALENDAR-TYPE:custom
STATUS:CONFIRMED
END:VEVENT

BEGIN:VEVENT
DTSTAMP:20190510T042124Z
DTSTART;TZID=Australia/Sydney:20181105T090000
DTEND;TZID=Australia/Sydney:20181112T085900
SUMMARY:Secondary on-call
X-CONFLUENCE-CUSTOM-TYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
CATEGORIES:On-call support
SUBCALENDAR-ID:036ce37a-a1d4-402f-b5ee-22b0cd46fca8
PARENT-CALENDAR-ID:648843fb-68a3-4840-a0f6-c59e851c4188
PARENT-CALENDAR-NAME:
SUBSCRIPTION-ID:
SUBCALENDAR-TZ-ID:Australia/Sydney
SUBCALENDAR-NAME:Production Support
EVENT-ID:69069
EVENT-ALLDAY:false
CUSTOM-EVENTTYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
UID:20181018T122802Z-201879622@confluence.mydomain.com
DESCRIPTION:
LOCATION:Fernando Renteria
ORGANIZER;X-CONFLUENCE-USER-KEY=8a0b193243717ee90143adbdf61c0028;CN=Staff 
 Leader;CUTYPE=INDIVIDUAL:mailto:Staff.Leader@mydomain.com
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO
RECURRENCE-ID;TZID=Australia/Sydney:20181105T090000
CREATED:20181029T054341Z
LAST-MODIFIED:20181106T021557Z
ATTENDEE;X-CONFLUENCE-USER-KEY=8a0b19324eccfb81014ecd06d8230002;CN=Staff 
 Two;CUTYPE=INDIVIDUAL:mailto:staff.two@mydomain.com
SEQUENCE:2
X-CONFLUENCE-SUBCALENDAR-TYPE:custom
STATUS:CONFIRMED
END:VEVENT


BEGIN:VEVENT
DTSTAMP:20190510T053432Z
DTSTART;TZID=Australia/Sydney:20190506T090000
DTEND;TZID=Australia/Sydney:20190513T085900
SUMMARY:Primary on-call
X-CONFLUENCE-CUSTOM-TYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
CATEGORIES:On-call support
SUBCALENDAR-ID:036ce37a-a1d4-402f-b5ee-22b0cd46fca8
PARENT-CALENDAR-ID:648843fb-68a3-4840-a0f6-c59e851c4188
PARENT-CALENDAR-NAME:
SUBSCRIPTION-ID:
SUBCALENDAR-TZ-ID:Australia/Sydney
SUBCALENDAR-NAME:Production Support
EVENT-ID:83168
EVENT-ALLDAY:false
CUSTOM-EVENTTYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
UID:20181018T122154Z-201879622@confluence.mydomain.com
DESCRIPTION:
ORGANIZER;X-CONFLUENCE-USER-KEY=8a0b193243717ee90143adbdf61c0028;CN=Staff 
 Leader;CUTYPE=INDIVIDUAL:mailto:Staff.Leader@mydomain.com
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO
RECURRENCE-ID;TZID=Australia/Sydney:20190506T090000
CREATED:20190318T053843Z
LAST-MODIFIED:20190318T053843Z
ATTENDEE;X-CONFLUENCE-USER-KEY=8a0b193244d606920144f0437e38016b;CN=Staff
 Three;CUTYPE=INDIVIDUAL:mailto:staff.three@mydomain.com
SEQUENCE:1
X-CONFLUENCE-SUBCALENDAR-TYPE:custom
STATUS:CONFIRMED
END:VEVENT

BEGIN:VEVENT
DTSTAMP:20190510T053432Z
DTSTART;TZID=Australia/Sydney:20190506T090000
DTEND;TZID=Australia/Sydney:20190513T085900
SUMMARY:Secondary on-call
X-CONFLUENCE-CUSTOM-TYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
CATEGORIES:On-call support
SUBCALENDAR-ID:036ce37a-a1d4-402f-b5ee-22b0cd46fca8
PARENT-CALENDAR-ID:648843fb-68a3-4840-a0f6-c59e851c4188
PARENT-CALENDAR-NAME:
SUBSCRIPTION-ID:
SUBCALENDAR-TZ-ID:Australia/Sydney
SUBCALENDAR-NAME:Production Support
EVENT-ID:83169
EVENT-ALLDAY:false
CUSTOM-EVENTTYPE-ID:58a6aacc-936c-4cfd-9618-83cebb4c8b4a
UID:20181018T122802Z-201879622@confluence.mydomain.com
DESCRIPTION:
ORGANIZER;X-CONFLUENCE-USER-KEY=8a0b193243717ee90143adbdf61c0028;CN=Staff 
 Leader;CUTYPE=INDIVIDUAL:mailto:Staff.Leader@mydomain.com
RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO
RECURRENCE-ID;TZID=Australia/Sydney:20190506T090000
CREATED:20190318T053855Z
LAST-MODIFIED:20190318T053855Z
ATTENDEE;X-CONFLUENCE-USER-KEY=8a0b193243717ee90143982d569e001b;CN=Staff
 One;CUTYPE=INDIVIDUAL:mailto:staff.one@mydomain.com
SEQUENCE:1
X-CONFLUENCE-SUBCALENDAR-TYPE:custom
STATUS:CONFIRMED
END:VEVENT

@jc1518
Copy link

jc1518 commented May 13, 2019

I made some changes, and it works for me now. May need more testings.

    def add_event(found, event):
        key = (event.uid, event.start.date())
        original_event = found.get(key)
        if original_event is None or event.recurrence_id > original_event.recurrence_id:
            found.update({key: event})
        elif original_event is None or event.sequence > original_event.sequence:
            found.update({key: event})
    def __init__(self):
        ...
        self.recurrence_id = None
    def copy_to(self, new_start=None, uid=None):
        ...
        ne.recurrence_id = self.recurrence_id
    def create_event(component, tz=UTC):
        ...
        if component.get('rrule'):
            event.recurring = True
                if component.get('RECURRENCE-ID'):
                    event.recurrence_id = normalize(component.get('RECURRENCE-ID').dt, tz=tz)
                else:
                    event.recurrence_id = event.start

@carloshanson
Copy link
Contributor Author

Cool. I'm still trying to find time to work on this again, so I can get my changes accepted.

@jc1518
Copy link

jc1518 commented May 28, 2019

One more thing, the event with RECURRENCE-ID should not be treated a recurring event. As it is just a instance of a recurring event. So it should be:

def parse_rrule(component, tz=UTC):
    ...
    if component.get('rrule') and component.get('RECURRENCE-ID') is None:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants