iCal Aggregator module extends Core module Aggregator to support iCal formatted data through RSS.
Project page: http://drupal.org/sandbox/TimBrandin/1207150

Reason for module:
iCal can be read as an RSS feed,
for example when the need to list the upcoming lectures or events in a block, you only need the feed temporarily.
Instead of using Feeds, where items are imported as Node, Taxonomy etc, Aggregator module stores the data temporarily which is good enough for this scope of use.

It wouldn't make sense to deliver this feature with Aggregator, as it is a core module and this module uses a GPL Third Party project.

The module does not also add another interface to use.

Why import iCal as RSS through Aggregator is to solve a simple problem.

iCal can be imported with Drupal 6 through Feeds and Job scheduler and iCal feed parser,
to simplify the UI for this task and also not actually import any items, I've created iCal Aggregator.

The module builds on the ical2rss -project by Arnout Engelen.
http://sourceforge.net/projects/ical2rss/
- Address to GPL licence for the project is in the code.
- - It also be found here: https://sourceforge.net/softwaremap/?&fq%5B%5D=trove%3A15

I'm not sure how to include the ical2rss in the project, but I'm assuming it's OK to add it in the project folder as it has a standard GPL license.

I've included a copy of standard GPL v2 also, for iCal Aggregator project.
Not sure if that's how you do it, or if its generated.

My Background:
Studied Computer Science bachelor, and now studying Interaction Designer master at Chalmers University of Technology.
I've worked with web since 1999, php since 2004 and Drupal since 2009.
I am new to module development and git, but have read through most of the documentation about this.
- and gone through the steps of module approval tips & tricks etc.

CommentFileSizeAuthor
#12 ical2rss-error.png89.97 KBkriskhaira

Comments

ccardea’s picture

ccardea commented

The waiting time for a project application review is currently approaching six weeks. Please consider shortening your wait time by contributing to the code review process. All it takes is basic module writing skills, plus it is a great way to add to your knowledge of Drupal. Please visit http://groups.drupal.org/code-review for details on how to participate.

alexreinhart’s picture

alexreinhart commented
Status: Needs review » Needs work

You don't need to include the LICENSE.txt, since the Drupal.org packaging script adds it automatically when you make a release.

Since ical2rss is an external project I don't think it's my role to review it. Taking a look through your module, though, here's a couple comments:

  • As far as I know, you shouldn't need to declare "global $base_url" in your .module file when you're not in a function -- the $base_url should be accessible anyway.
  • In ical_aggregator_aggregator_fetch, there should be spaces around the !=.
  • Your @file comment at the top of the file should actually contain something, like a brief description of what the file does.

And that's it. Once you've fixed those issues, set the status here back to "needs review" and I'll try to take another look through.

timbrandin’s picture

timbrandin commented
Status: Needs work » Needs review

Made those changes you suggested.

About $base_url, a got some Warnings about it not being defined when I removed it. Could it be that globals are not loaded when constants are being defined?

alexreinhart’s picture

alexreinhart commented
Status: Needs review » Reviewed & tested by the community

Hm, I suppose the global does make sense if modules are included from inside a function, since the included file would get all variables in that function's scope only.

I don't see any other issues in the code. The ical2rss folder has some unnecessary _notes folders which contain DreamWeaver sync information (which isn't necessary for your module to work), but that's the ical2rss maintainer's problem, really.

Otherwise, nicely done! A short and simple module.

timbrandin’s picture

timbrandin commented

Hmm, yes they are not needed there.
Removed the _notes folders anyway.

Thanks

alexreinhart’s picture

alexreinhart commented

Hm; I apologize for not noticing this earlier, but the Drupal.org policy strongly encourages having your users download the ical2rss library separately:

http://drupal.org/node/422996

You can make this easier by using the Libraries API module, or you can just specify in the README to download ical2rss and extract it to the right directory.

timbrandin’s picture

timbrandin commented

Okey, but what about this text here:


"unless if they are licensed under GPL or GPL v2+ or they have a broader license and can be re-licensed under the GPL."

-- http://drupal.org/node/422996

Doesn't it say that it's OK with GPL licensed code?

And I believe it is:
ical2rss -license: http://sourceforge.net/projects/ical2rss/

Or am I wrong? // TB

alexreinhart’s picture

alexreinhart commented

I'm not sure. I think it strongly discourages external libraries either way, but I don't know enough about the rule or its reasoning to be sure.

ccardea’s picture

ccardea commented

I think the reasoning for not allowing 3rd party libraries in the repository, aside from licensing issues, is namespacing. I don't want to step on anybody's toes by changing this back to 'needs work', but I think you might have a hard time getting this approved if you don't change this. Or you could leave it as is and see how it goes, which might be interesting.

tim.plunkett’s picture

tim.plunkett
he/him
Primary language English
Location Philadelphia
commented
Status: Reviewed & tested by the community » Needs work

Per #6 and #9, need work. Use the libraries module.

Also, please conform to coding standards, especially in your inline comments.
files[] = ical_aggregator.module should be removed.

Move the module_load_include's inside ical_aggregator_aggregator_fetch, assuming they are actually necessary.

kriskhaira’s picture

kriskhaira commented

I'm getting an error for several ICS links from different sources whenever I update the feed:

The feed from Test ICS seems to be broken, because of error "Not well-formed (invalid token)" on line 1.

I tried these ICS links, which I can subscribe to without any problems in both iCal and Google Calendar:
https://www.google.com/calendar/ical/en.malaysia%23holiday%40group.v.cal...
http://ibew177.org/civicrm/event/ical

With the second feed, I get this additional error:

Notice: Undefined index: extension in ical_aggregator_aggregator_fetch() (line 55 of /var/www/sites/all/modules/contrib/ical_aggregator/ical_aggregator.module).

kriskhaira’s picture

kriskhaira commented
StatusFileSize
new ical2rss-error.png89.97 KB

It might be a problem with ical2rss because if I run it by itself, I get this error.

error on line 1 at column 1: Document is empty

Check out the attached file.

misc’s picture

misc commented

@TimBrandin has been contacted to ask if the application is abandoned.

After ten weeks with a status of needs work: the applicant may be contacted by a reviewer to determine whether the application was indeed abandoned. The action taken by the reviewer should be documented in the project application issue.

http://drupal.org/node/894256

misc’s picture

misc commented
Status: Needs work » Closed (won't fix)

The application has been closed. If you would like to reopen it, you are free to do so.
See http://drupal.org/node/894256#abandonedtwoweekscontact