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

Support rrules with BYDAY frequencies #3273

Merged
merged 1 commit into from
Jan 13, 2025

Conversation

david-venhoff
Copy link
Member

@david-venhoff david-venhoff commented Dec 6, 2024

Short description

This pr is blocked because it depends on #3229, but it can already be reviewed
The blocker is now resolved

This pr adds support for importing events with rrules that contain BYDAY entries with frequencies, for example -1SU (which means repeat every last Sunday).

Unfortunately the icalendar library does not parse these entries fully right now, so this pr also implements some manual parsing. However, in its next major version icalendar will add support for that.

Proposed changes

  • Check if the BYDAY entry has a frequency, and use that frequency as a value for BYSETPOS
  • Handle edge cases
  • Implement some manual parsing for BYDAY weekdays

Side effects

/

Resolved issues

Fixes: #3227


Pull Request Review Guidelines

@david-venhoff david-venhoff added the blocked Blocked by external dependency label Dec 6, 2024
@david-venhoff david-venhoff force-pushed the bugfix/rrule_with_byday_frequency branch from 8e7f231 to 5be5d21 Compare December 6, 2024 13:39
@david-venhoff david-venhoff removed the blocked Blocked by external dependency label Dec 11, 2024
@david-venhoff david-venhoff force-pushed the bugfix/rrule_with_byday_frequency branch 2 times, most recently from 0066075 to 59aa0bd Compare December 11, 2024 13:57
Copy link
Member

@MizukiTemma MizukiTemma left a comment

Choose a reason for hiding this comment

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

Cool 😻 🎉

@JoeyStk JoeyStk mentioned this pull request Jan 6, 2025
@MizukiTemma MizukiTemma added blocker This issue blocks another issue prio: high Needs to be resolved ASAP. deadline Needs to be fixed in the given time labels Jan 7, 2025
@PeterNerlich PeterNerlich self-assigned this Jan 8, 2025
Copy link
Collaborator

@PeterNerlich PeterNerlich left a comment

Choose a reason for hiding this comment

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

RRULE:FREQ=MONTHLY;UNTIL=20250502T220000Z;INTERVAL=1;BYMONTHDAY=3 still produces a ValueError('by_set_pos must not be None') (from outlook.calendar.txt in #3227)

integreat_cms/cms/utils/external_calendar_utils.py Outdated Show resolved Hide resolved
@PeterNerlich PeterNerlich removed their assignment Jan 10, 2025
@david-venhoff
Copy link
Member Author

david-venhoff commented Jan 13, 2025

@PeterNerlich

RRULE:FREQ=MONTHLY;UNTIL=20250502T220000Z;INTERVAL=1;BYMONTHDAY=3 still produces a ValueError('by_set_pos must not be None') (from outlook.calendar.txt in #3227)

Right now, we only support recurrence rules which can be also be created from within the recurrence rule form. However, this recurrence rule repeats every third day of the month, which cannot be expressed in the recurrence rule form (We only support monthly recurrences if we specify the nth-week and a weekday). So this error is intentional

For example, `-1SU`.
Unfortunately this commit contains a manual parsing implementation,
because the `icalendar` package does not support these right now.
(Though it will at the next update)
@david-venhoff david-venhoff force-pushed the bugfix/rrule_with_byday_frequency branch from 28ca313 to 94c01c4 Compare January 13, 2025 13:40
@david-venhoff david-venhoff merged commit 52fcc18 into develop Jan 13, 2025
5 checks passed
@david-venhoff david-venhoff deleted the bugfix/rrule_with_byday_frequency branch January 13, 2025 13:51
charludo pushed a commit that referenced this pull request Jan 13, 2025
For example, `-1SU`.
Unfortunately this commit contains a manual parsing implementation,
because the `icalendar` package does not support these right now.
(Though it will at the next update)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
blocker This issue blocks another issue deadline Needs to be fixed in the given time prio: high Needs to be resolved ASAP.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

External calendars created in outlook with recurring events cannot be imported
3 participants