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

Update ical comparison to check only for event components #1870

Merged
merged 4 commits into from
May 3, 2023

Conversation

matiasb
Copy link
Contributor

@matiasb matiasb commented May 3, 2023

May be related to #1553.

We got feedback about that happening for Google Calendar imported icals.
Google Calendar exported ics URL was returning different VTIMEZONE components on different requests, triggering differences in the imported ical. Updated the comparison to only consider events (while keep assuming the sequence will reflect if there are any particular event change).

@matiasb matiasb added the pr:no public docs Added to a PR that does not require public documentation updates label May 3, 2023
@matiasb matiasb requested a review from a team May 3, 2023 20:04
@matiasb matiasb added this pull request to the merge queue May 3, 2023
Comment on lines +513 to +518
first_cal_events = {
cmp.get("UID", None): cmp.get("SEQUENCE", None) for cmp in first_subcomponents if cmp.name == "VEVENT"
}
second_cal_events = {
cmp.get("UID", None): cmp.get("SEQUENCE", None) for cmp in second_subcomponents if cmp.name == "VEVENT"
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit consider deduplicating the dict construction logic into a separate function

Merged via the queue into dev with commit 7cf0c4f May 3, 2023
@matiasb matiasb deleted the matiasb/ical-comparison-revisited branch May 3, 2023 20:29
iskhakov pushed a commit that referenced this pull request May 12, 2023
May be related to #1553.

We got feedback about that happening for Google Calendar imported icals.
Google Calendar exported ics URL was returning different VTIMEZONE
components on different requests, triggering differences in the imported
ical. Updated the comparison to only consider events (while keep
assuming the sequence will reflect if there are any particular event
change).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:no public docs Added to a PR that does not require public documentation updates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants