search

adamgibbons / ics

iCalendar (ics) file generator for node.js
ISC License
732 stars 155 forks source link

Add support for CUTYPE and X-Num-Guests fields #237

Closed AdrianCurtin closed 1 year ago

AdrianCurtin commented 1 year ago

Allegedly these help with gmail compatibility

AdrianCurtin commented 1 year ago

I have confirmed that adding these fields allows gmail to recognize the events.

The fields were added to address a field order issue mentioned here : https://stackoverflow.com/questions/66102584/when-i-add-method-request-to-icalendar-gmail-stops-recognizing-as-event

and should help address https://github.com/adamgibbons/ics/issues/199

I will note that gmail does not recognize this as a request if you are both the organizer and an attendee (but it works for other people)

AdrianCurtin commented 1 year ago

@adamgibbons

hughgardiner commented 1 year ago

@AdrianCurtin thank for your contribution, much appreciate it! I'm trying to verify your changes, unfortunately, Google Calendar still isn't picking up on any changes. I've attached the ics files that are working wonderfully in outlook, but not google. Anything obvious I'm missing here? ics_files.zip

AdrianCurtin commented 1 year ago

@hughgardiner I managed to get it to work, but try PARTSTAT=NEEDS-ACTION and adding in CREATED:20230313T220600Z LAST-MODIFIED:20230313T220600Z

the only other major differences in my working examples are I have also specified a category, and a location but i'm not sure that matters. I also modified the organizer MAILTO to mailto, but i'm not sure if that matters either

jlapier commented 1 year ago

I did some experimenting with these fields after seeing them in an ics attachment from lu.ma. I was able to hand-build ics files that were recognized by gmail, would accept updates, and would accept a cancellation.

Regardless of gmail though, this PR seems innocuous enough - I don't see a reason not to merge it. CUTYPE is in the RFC. I confess I don't know the origin of X-NUM-GUESTS, but in any case, it's not a breaking change because it's adding new attributes.

adamgibbons commented 1 year ago

Thanks for this! It initially broke four tests related to RSVP, but those were easy to fix. Before merging this, though, would you please add tests for the two fields you introduced, CUTYPE and xNumGuests, and include them in the docs? Happy to merge after that.

jlapier commented 1 year ago

I went ahead and resynced my fork and re-added these changes, along with my own tweaks (which include tests for the new fields): #244

adamgibbons commented 1 year ago

Thanks @AdrianCurtin and @jlapier for your contributions! I've merged #244 which builds on this PR.