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

Workaround for invalid allday event definition (dtstart = dtend) #58

Merged
merged 1 commit into from
Oct 6, 2019

Conversation

jonas0b1011001
Copy link
Collaborator

Fix for #55 #56 #57

Comment on lines +272 to +275
if (icalEvent.startDate.compare(icalEvent.endDate) == 0 && event.isAllDay){
//Adjust dtend in case dtstart equals dtend as this is not valid for allday events
icalEvent.endDate = icalEvent.endDate.adjust(1,0,0,0);
}
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This is only required for all day events as

For cases where a "VEVENT" calendar component specifies a "DTSTART" property with a DATE-TIME value type but no "DTEND" property, the event ends on the same calendar date and time of day specified by the "DTSTART" property.

@@ -292,12 +296,7 @@ function ConvertToCustomEvent(vevent){
}
else{
event.startTime = icalEvent.startDate.toJSDate();
if (icalEvent.endDate == null){
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Obsolete as icalEvent.endDate will never be null. If the vevent is missing dtend ical.js will calculate an appropriate dtend according duration property (if it exists).

If duration is not specified

  • dtend will be dtstart if Value=Date-Time
  • dtend will be dtstart + 1d if Value=Date

@@ -26,7 +26,6 @@ var removeEventsFromCalendar = true; // If you turn this to "true", any event
var addAlerts = true; // Whether to add the ics/ical alerts as notifications on the Google Calendar events
var addOrganizerToTitle = false; // Whether to prefix the event name with the event organiser for further clarity
var descriptionAsTitles = false; // Whether to use the ics/ical descriptions as titles (true) or to use the normal titles as titles (false)
var defaultDuration = 60; // Default duration (in minutes) in case the event is missing an end specification in the ICS/ICAL file
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

There is no real need for a default Duration as Google Calendar accepts Events with 0min Duration (which is perfectly fine according RFC as well - see other comments).

Copy link
Owner

Choose a reason for hiding this comment

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

My original thought with defaultDuration was that the user could specify a duration in the case of "non-all-day events" where the duration and end datetime are not specified. Do you think we should just allow events to be 0 minutes in that case?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I don't really see the need of defining a own default while the RFC already has one.
I would totally agree if ical did not allow to define a custom duration for events, but as of now i think we should aim for creating an as accurate representation of the input as possible, which includes 0 minute events.

@derekantrican derekantrican merged commit 53736f5 into derekantrican:master Oct 6, 2019
@derekantrican
Copy link
Owner

@jonas0b1011001 I'm assuming you've put these changes into your repo as well?

@jonas0b1011001
Copy link
Collaborator Author

Good point, just did

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants