Add basic support for parsing VALARM

[kipcole9]

This very presumptuous PR adds basic support for parsing VALARM entries in events. It is currently missing:

• Serialization
• Tests

I'd rather get feedback early than go down a wrong path too far, hence this initial PR. I've tested it on a fairly large .ics file (my personal calendar) and everything appears to parse correctly but I haven't added tests yet.

[kipcole9]

I should also note that alarm triggers can be a duration or a datetime. As a result, the PR uses Duration.from_iso8601!/1 which is only available from Elixir 1.18 1.17. Maybe thats still ok?

[jonathanballs]

Thanks for using Magical and thank you for this PR!

This looks great and I can see that you've been through the codebase and actually kept things quite consistent with the design that I initially created. I have a small comment about your use of Duration but otherwise all is fine. As for your question as to whether we should support Duration given that it's a newer addition to the Elixir stdlib my opinion is that it's fine - and indeed within the spirit of this library to parse these durations into native elixir types!

[jonathanballs]

The convention that I created within the DateParser was that if a field was not parseable that it should be left as nil

Suggested change

	Duration.from_iso8601!(duration_string)
	Duration.from_iso8601(duration_string)
	|> case do
	{:ok, duration} -> duration
	error -> nil
	end

[jonathanballs]

Also I know that I've been slow to respond to this PR but I will be much more on top of it now! I did actually start work on this feature last year when an issue was raised (on which you commented) however I did not get round to finishing it. I'm happy to lend a hand if need be otherwise I'll let you get onto it!

Thanks again :)

[jonathanballs]

I have updated the github workflow to run tests on PRs and new branches so if you rebase onto main you should be able to get CI working within the PR

[madjar]

Hey folks!

I'm testing this branch on an ics export of my google calendar, and I'm getting a failure which may be interesting to you. It dies on

BEGIN:VALARM
ACTION:EMAIL
ATTENDEE:mailto:my@email.com
TRIGGER:-P0DT7H10M0S
DESCRIPTION:This is an event reminder
SUMMARY:Alarm notification
END:VALARM

This looks legit according to the RFC, since a VALARM can contain some email-related fields when the ACTION is EMAIL.

[kipcole9]

Good callout - I'll update my PR and resubmit. I'll also recheck against the RFC (I so far only focused on calendar exports too).

[madjar]

I went ahead and fixed it in a branch, then I found out that SUMMARY is also troublesome. The commit is here, for reference (sorry about the indentation changes): 52fb900