collective / collective/icalendar

Mandatory properties are neither enforced nor set to a default

Open
#596 10 comments 0 reactions 0 assignees View on GitHub
feature
Dominant language
Python
Stars
1.2k
Forks
413
Avg merge
3d 21h
Merged PRs (30d)
45

Description

According to RFC 5545, every VCAL needs a `PRODID` and `VERSION`. If those are not explicitly added, `Calendar.to_ical()` produces non-conforming output because they are missing.

It could be a deliberate design choice to leave this open. But I wonder, how often do people want to generate invalid output, and shouldn't the library enforce a valid calendar object somehow?

Just some suggestions here.
### VERSION
As this library claims to be implementing RFC 5545, I think this should just be `2.0` by default. If you add it again with a different value, that's fine, it will be replaced.

### PRODID
What about setting this to something like `-//python.collective.icalendar//NONSGML python.collective.icalendar.app.event//EN`? (I'm not sure how exactly these strings should be constructed, I just wrote something based on a test case I saw in this repo). Same here, if you add it again with a different value, it will be replaced.

Both of above improvements are fully backwards-compatible, AFAIK. Please let me know your thoughts.

---

TODO list by @niccokunzmann

To close:

- [ ] add `make_valid()` with the mandatory fields to these classes:
- [ ] Alarm, respecting recommendations from RFC 9074, so we can claim compatibility, RFC 5545
- [ ] Availability, RFC 7953
- [ ] Available, RFC 7953
- [ ] Calendar RFC 5545
- [ ] Event, RFC 5545
- [ ] FreeBusy, RFC 5545
- [ ] Journal, RFC 5545
- [ ] Todo, RFC 5545
- [ ] use `make_valid()` in `new()` instead of setting the attributes directly to avoid code duplication

Contributor guide

Open the contributing guide

Research direction

Start by reading the TODO list and RFC references in the issue, then inspect the existing new() behavior for Alarm, Availability, Available, Calendar, Event, FreeBusy, Journal, and Todo. Define how make_valid() should add or preserve mandatory fields, including RFC 9074 recommendations for Alarm, and use it from new() without duplicating attribute setup. Done means each listed class has the required mandatory properties and new() uses make_valid().

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend-api-design
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Mostly clear
Newbie friendliness
32/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.