Coming from #1460058: iCal feed not honoring timezone or Daylight Saving Time

  • Drupal's theme system is being used to assemble structured data, the iCal feed. You wouldn't use it to assemble a JSON object. We should use a library to create an iCal fed for us, rather than trying to do complex string manipulation to assemble a massively complicated file format.
  • There is a such an iCal library for PHP, distributed under the LGPL: iCalcreator.
  • I've re-written the views style plugin to use this library instead of Drupal's theme layer. The integration requires the Libraries module, version 2.0 or higher.
  • I've re-implemented everything the current module does with the iCalcreator library, including repeating dates, but crucially time zone information goes into the created iCal feed.

We should leverage a library to create iCal feeds, or fork it and make it better in Drupal-land.

Sadly the library requires PHP 5.3 for the last point above, generating timezone information for use in the iCal feed.

CommentFileSizeAuthor
#10 date_ical-patched-1833362-7.tar_.gz20 KBsteven jones
#7 date_ical-use-ical-library-1833362-7.patch46.83 KBcoredumperror
#1 date_ical-1833362-use-ical-library.patch23.93 KBsteven jones

Comments

steven jones’s picture

steven jones commented
StatusFileSize
new date_ical-1833362-use-ical-library.patch23.93 KB

This is the patch from #1460058-69: iCal feed not honoring timezone or Daylight Saving Time plus a couple of bug fixes (one double escaping and one documentation issue)

steven jones’s picture

steven jones commented
Status: Active » Needs review

Sorry, some testing tips from the other issue:

Testing

To test this patch, you'll need to do the following:

  1. Install and enable version 2.0 of the Libraries module.
  2. Download the iCalcreator module from http://kigkonsult.se/downloads/index.php#icalcreator and extract to sites/all/libraries/iCalcreator, so that sites/all/libraries/iCalcreator/iCalcreator.class.php exists.
  3. Apply the patch, and test away!
dfelicia’s picture

dfelicia commented

Steven, what, if anything, will happen to existing entries if I test your patch?

steven jones’s picture

steven jones commented

@dfelicia I'm not sure, it depends if you're doing anything custom with the templates already. If you're not, then the output should be compatible if not 100% the same. You'll want to test it on a test site, and compare the iCal output.

aarailfan’s picture

aarailfan commented

I like this idea and think it's probably the correct way to move forward. My concern is are we going to have 2 different development paths going on? Also, I'd like to see something get finalized and moved into production since I have a lot of need for this to work properly on multiple sites. To that end, I can begin working to test things, but I'm not a coder so I'll need some help getting this going...to be specific, what version do I need to patch and how does one apply the patch?

coredumperror’s picture

coredumperror commented

I need this for my company's site, so I'm going to be spending the next several work days polishing it and adding new features.

I'm still getting to know the code; so far, I've added the ability to include a LAST-MODIFIED value, as specified in RFC 2445 (http://www.ietf.org/rfc/rfc2445.txt).

I'd be happy to work with anyone who wants to provide input.

coredumperror’s picture

coredumperror commented
StatusFileSize
new date_ical-use-ical-library-1833362-7.patch46.83 KB

OK, I've done a bunch of work to improve this patch, and used it myself in my own test site, to great effect. The major additions and changes I made to Steve Jones' initial work are:

* Added LAST-MODIFIED field output for VEVENTs.
* Implemented the view modes fix suggested in #1793868: Problem with adding iCal views mode.
* Implemented the div removal suggested in #1863448: Remove unnecessary div around the feed icon.
* Moved the .inc files into the includes/ subfolder to tidy up the root module folder.
* Switched date_ical_plugin_style_ical_feed to 'uses fields' => FALSE. Date iCal doesn't use fields, so offering them to the user is confusing. I also added the ability for users to override the Calendar Name that we output as the X-WR-CALNAME field in the VCALENDAR.
* Renamed date_ical_plugin_row_ical_feed to date_ical_plugin_row_ical_entity, since it generates individual VEVENTS from entities.
* Since our style plugin can really only work with our row plugin, I set it up to output an error if the user tries to use a different row plugin.
* Fixed the settings dialog for the row plugin, so that the long text blurb describing how Date iCal interacts with the 'iCal' view mode doesn't push the date field select off the bottom of the dialog.

SyneX’s picture

SyneX commented

The patch of #7 works for me. I only tried the normal export.

Why you say this library needs php >=5.3? The homepage says "iCalcreator now requires PHP >= 5.2".

And could you please add support for more fields like the location field? (#1488946: Location information)

edit: This fixes also #1439058: Repeating events cause duplicates in iCal feeds

chris.gr’s picture

chris.gr commented

After applying the patch, I am getting the information "missing style plugin". Is it possible to provide the patched module so that I know if all the files are correct?

Thanks!

steven jones’s picture

steven jones commented
StatusFileSize
new date_ical-patched-1833362-7.tar_.gz20 KB

@SyneX We need PHP 5.3 because iCalcreator uses PHP 5.3 only stuff to do the repeating event timezone handling, which was basically the entire point of using the library in the first place, otherwise, yes we could have gotten away with PHP 5.2.

@chris.gr Find a dev version of date module with the patch from #7 applied, I've not validated that that version of the patch works however.

SyneX’s picture

SyneX commented

Ah ok.

It would be nice when more people review this patch so that we can change the status. This patch resolves some strange issues. But I don't know whether this module is active maintained anymore.

coredumperror’s picture

coredumperror commented

@SyneX - I've requested to be added as a maintainer for this module, but the current maintainer hasn't gotten back to me. It looks like I'll have to go through the workflow for taking over an abandoned project, which I should be able to get to later this week. I'll also see about adding the ability to export to the LOCATION field.

coredumperror’s picture

coredumperror commented
Status: Needs review » Closed (won't fix)

Because this is a major re-write of the entire code base, I've moved this functionality into the 7.x-2.x branch. I'm closing this ticket, since this functionality is implemented in 2.x, and this issue no longer applies to 7.x-1.x.