Allow `organiser` to contain comma, ensure that new lines in values don't break output, and various other tweaks

tjenkinson commented 1 year ago

Thanks for this great library!

We had a bug where we were providing an organiser where the name included a ,, and the fix was to wrap the name in quotes. Essentially take the quoted-string option for param-value in the spec

So the I decided to make the same change to the other param-value's so that they don't have the same issue.

And then I added some logic to encode new lines to\n in a lot more places because otherwise it's possible for user input containing new lines to break things

And as I was going through I spotted a few other things so here's a list of all the tweaks/fixes:

exclusionDates was missing in the schema so I don't think it could be used before? Now it can be and I changed it to be an array of date/time entries
date time entries can now be a single number which is the unix time, which is easy to get from date.getTime(), or a string which is then just passed through
the default values are now built with a fn at runtime as this means the default date will be the invoke time and not the time when the library was loaded, and also means the uuid is unique every time and assignUniqueId was no longer needed
some of the regex patterns in the schema check weren't bounded. I.e. oopsCANCELLED would be allowed
I split up the event into header/event/footer (the second commit). This makes it a bit cleaner when there are no events as don't need to do any string searching anymore. It still supports taking the calendar name etc from the first event object, ~but in the future it might be worth updating the public api to support passing the header fields separately to the event ones, so that then if you are creating a calendar feed where the are no events it's still possible to set the header fields and not get the defaults~
Added an optional second param to getEvents which can take header attributes, so it's possible to still have the correct header attributes even when there are no events
added encodeParamValue which is used to encode param-value's. The spec doesn't appear to say that "'s are allowed in the string, but escaping them with \ does seem to work when I tested in the apple calendar app

tjenkinson commented 1 year ago

Just realised I still need to update the d.ts file

adamgibbons commented 9 months ago

@tjenkinson thanks for all this great work - I checked out your branch and all tests were passing, but when I merged into main tests were breaking. Would love to merge this in if we can straighten that out.

tjenkinson commented 9 months ago

@adamgibbons will have a look now

tjenkinson commented 9 months ago

I'm on node v18.18.0, npm 9.8.1 and npm ci && npm t works fine or me on feee468da713c92603eb4cd0faf03c0d9b7b5df0 🤔

What errors are you seeing?

adamgibbons commented 9 months ago

@tjenkinson would you reopen this PR against master when you have a chance? 🙏

tjenkinson commented 9 months ago

Done in https://github.com/adamgibbons/ics/pull/261

adamgibbons / ics

Allow `organiser` to contain comma, ensure that new lines in values don't break output, and various other tweaks #252