feat: First checkin for the ical direct features by dploeger · Pull Request #189 · ColdTrick/event_manager

dploeger · 2026-01-09T14:15:01Z

This implements a feature I called "ical direct" offering ical export and import features without the need of an external provider.

Features:

Exporting an event (with currently only basic information) is the first implementation done (i18n wasn't yet done)
Exporting single events as well as a (optionall filtered) list of events
Importing single events and complete calendars from ical

dploeger · 2026-02-04T10:16:35Z

@jdalsem can you find some time checking this out? Thanks.

jdalsem · 2026-02-09T11:04:25Z

i enabled the automated checks on this PR. You might want to fix them first. After that i will do a detailed review of the code.

At first glance it looks ok (with some minor remarks).

I was wondering how you see this working together with the AddThisEvent feature (which also supports ical downloads). Should users choose on or the other?

Maybe you could squash all your commits into one.. we do not need all the development commits in our history

dploeger · 2026-02-09T16:41:38Z

i enabled the automated checks on this PR. You might want to fix them first. After that i will do a detailed review of the code.

Sure. Will do, thanks.

I was wondering how you see this working together with the AddThisEvent feature (which also supports ical downloads). Should users choose on or the other?

I've made it selectable in the code. AddThisEvent doesn't support iCal imports and is a US-based company so many people will favor a local embedded version (at least we would 😅)

Maybe you could squash all your commits into one.. we do not need all the development commits in our history

Yes, of course. I usually do a rebase with fixups once everything is settled.

jdalsem · 2026-02-09T18:23:00Z

lol 4 comments... something went wrong probably

If this can provide the same feature as AddThisEvent, we might choose to drop that. It is just an 'easy' solution (from the past). No specific needs... maybe the links in the mail footer is special

dploeger · 2026-02-09T19:34:26Z

Shouldn't use the GitHub app on an unstable 5G connection apparently. 😂

Sure, the iCal export also works fine on mobile with direct import into the calendar app.

Although users who use the AddThisEvent app already might want to keep it.

dploeger · 2026-02-16T09:14:19Z

@jdalsem Could you please approve the workflow again. I've fixed some things.

jdalsem · 2026-02-16T09:37:05Z

we also expect the composer.lock file to be present (and updated with your changes)

dploeger · 2026-02-16T09:39:07Z

we also expect the composer.lock file to be present (and updated with your changes)

Hm. Actually did a composer update in the module file and added the composer.lock. 🤔

Was that wrong?

dploeger · 2026-02-16T09:40:52Z

Hm. Maybe I copied the wrong composer.lock. 😆 I'll take a look

dploeger · 2026-02-16T09:43:05Z

Ah. Found the right composer.lock. Sorry.

jdalsem · 2026-02-16T09:44:42Z

best option is to reset the lock and composer.json and do:

composer install
composer require your package

best practice is to not do composer update as it will update all (unrelated) packages... composer update should be something that we do in a separate commit / PR... not a big issue / blocker for now, but it might introduce unrelated issues

jdalsem · 2026-02-16T09:46:08Z

still not ok...

dploeger · 2026-02-16T09:48:44Z

Yeah, resetted the lock like you suggested now.

jdalsem · 2026-02-16T09:52:18Z

Now it installs correctly... some views/actions seem to be assuming too much... or not casting input correctly.. i will have a look at the rest of the PR

dploeger · 2026-02-16T09:58:00Z

Thanks. I'll take a look at the unit test result.

dploeger · 2026-02-16T10:15:15Z

If I read the result correctly, it just having a problem with calling the import ical action without specifying a correct input file. So the fix would be to check for a proper file and exiting, right?

jdalsem

It was a big review, but i think you are on the right track... do not let the amount of comments discourage you. I am merely trying to make sure it all works as expected.

Main concerns:

forms (import/export) should not be part of a listing resource (next to all/mine)
actions should do some more sanity checks and expect users to misbehave
importing of attendees feels a privacy issue (as users might join events they did not subscribe for themselves)
importing to random users/groups is possible. You might need to validate if that is possible / allowed

If we can get this PR merged, this will be added in a minor version of Event Manager as 'experimental'. If all works fine we might consider dropping AddEvent in the version for Elgg 7 and completely switch to this ical feature.

jdalsem · 2026-02-17T11:00:39Z

actions/event_manager/export/ical.php

+$date_format = elgg_get_config('date_format', elgg_echo('input:date_format'));
+
+$start_date = get_input('start_date', '');
+if ($start_date == '') {


loosely comparing is not recommended. use triple === if you know it is a string or just use empty($start_date)

you do this everywhere... just commenting once about it

Do you mean every double == comparison? Because I see that outside my changed files as well. Or just comparing empty strings?

there might be some legacy code that still does check with ==, but === is better. See https://www.php.net/manual/en/language.operators.comparison.php#:~:text=Note%3A%20Be%20aware%20that%20PHP's,in%20most%20cases.

jdalsem · 2026-02-17T11:01:55Z

actions/event_manager/export/ical.php

+$calendar_type = get_input('calendar_type', 'all');
+$date_format = elgg_get_config('date_format', elgg_echo('input:date_format'));
+
+$start_date = get_input('start_date', '');


defaulting to empty string is not recommended. If you need it to always be a string, better to cast it:

$start_date = (string) get_input('start_date');

you do this everywhere... just commenting once about it

I wanted to default to an empty string so I can later default to the current time. A better alternative would be probably to check, if the start_date input was given at all. Is there a "has_input" function or something?

no nothing other than get_input... has_input is just as tricky as you do not know if the input is valid. Personally just cast and check the validity with empty() or elgg_is_empty() and then decide if you need a special default.

jdalsem · 2026-02-17T11:03:19Z

actions/event_manager/export/ical.php

+	]
+];
+
+if ($region != '') {


also a loosely compared condition... use !empty($region)

actions/event_manager/export/ical.php

views/default/resources/event/export/ical.php

jdalsem · 2026-02-17T12:25:45Z

views/default/resources/event/import/ical.php

@@ -0,0 +1,27 @@
+<?php


i think the import/export resource could be replaced by utilizing ajax forms

replace the tabs in the filter menu by title menu buttons that popup with an ajax form (look at reported content for an example)

I'll take a look.

What do you mean with "reported content"?

Oh i meant... take a look at the reported_content plugin. That plugin also has ajax lightbox form for reporting content.. maybe you can get inspiration there

Ah okay. Thanks

views/ical/resources/event/export.php

jdalsem · 2026-02-17T12:29:38Z

composer.json

 	"conflict": {
 		"elgg/elgg": "<6.3.1"
+	},
+	"require-dev": {


please remove this, we do not want it in our code....

we have sniffs configured in Elgg (the base repo) and in our IDE... we do not want the administration in our plugins... might need to update your lock file (composer update --lock) if you touch this

Oh whoops. Got in there while trying to simulate the lint checks.

jdalsem · 2026-02-17T12:54:02Z

elgg-plugin.php

 			'path' => '/event',
 			'resource' => 'event/upcoming',
 		],
+		'collection:ical:export' => [


remove this if you decided to switch to ajax forms... as mentioned before

dploeger

I've fixed the first batches of comments. I'll take a look at the rest.

dploeger · 2026-02-18T11:47:27Z

actions/event_manager/export/ical.php

+$calendar_type = get_input('calendar_type', 'all');
+$date_format = elgg_get_config('date_format', elgg_echo('input:date_format'));
+
+$start_date = get_input('start_date', '');


dploeger · 2026-02-18T11:47:37Z

actions/event_manager/export/ical.php

+$date_format = elgg_get_config('date_format', elgg_echo('input:date_format'));
+
+$start_date = get_input('start_date', '');
+if ($start_date == '') {


dploeger · 2026-02-18T11:47:49Z

actions/event_manager/export/ical.php

+	]
+];
+
+if ($region != '') {


actions/event_manager/export/ical.php

dploeger · 2026-02-18T12:22:18Z

actions/event_manager/import/ical.php

+try {
+	$vcalendar->parse($file->getContent());
+} catch (Exception $e) {
+	return elgg_error_response('Error parsing calendar: ' . $e);


dploeger · 2026-02-18T12:24:29Z

actions/event_manager/import/ical.php

+		$event = Event::fromVEvent($component);
+
+		switch ($calendar_type) {
+			case 'group':


dploeger · 2026-02-18T12:25:26Z

classes/Event.php

+			foreach ($vevent->getAllAttendee() as $attendee) {
+				$attendee_object = elgg_get_user_by_email($attendee);
+				if (!is_null($attendee_object)) {
+					$event->addRelationship($attendee_object->guid, 'event_attending');


Removed it.

views/ical/resources/event/export.php

dploeger · 2026-02-18T12:46:47Z

composer.json

 	"conflict": {
 		"elgg/elgg": "<6.3.1"
+	},
+	"require-dev": {


dploeger · 2026-02-19T08:56:04Z

Ah yes, the missing file-upload check. Thank you, PHPUnit. 😆

Missing German error translations

dploeger · 2026-02-19T09:09:16Z

@jdalsem Could you please approve the workflows again? I hope the test is fixed now.

I tried to run them locally, but I get this problem:

# ./vendor/bin/phpunit ./vendor/elgg/elgg/engine/tests/phpunit/plugins_integration
PHP Fatal error:  Uncaught Error: Class "Elgg\Actions\RegistrationIntegrationTestCase" not found in /var/www/localhost/htdocs/vendor/elgg/elgg/engine/tests/phpunit/plugins_integration/Elgg/Plugins/ActionsRegistrationIntegrationTest.php:7
Stack trace:
#0 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/Util/FileLoader.php(66): include_once()
#1 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/Util/FileLoader.php(49): PHPUnit\Util\FileLoader::load()
#2 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/Framework/TestSuite.php(398): PHPUnit\Util\FileLoader::checkAndLoad()
#3 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/Framework/TestSuite.php(537): PHPUnit\Framework\TestSuite->addTestFile()
#4 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/Runner/BaseTestRunner.php(98): PHPUnit\Framework\TestSuite->addTestFiles()
#5 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/TextUI/Command.php(121): PHPUnit\Runner\BaseTestRunner->getTest()
#6 /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/TextUI/Command.php(99): PHPUnit\TextUI\Command->run()
#7 /var/www/localhost/htdocs/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#8 /var/www/localhost/htdocs/vendor/bin/phpunit(122): include('...')
#9 {main}

Next PHPUnit\TextUI\RuntimeException: Class "Elgg\Actions\RegistrationIntegrationTestCase" not found in /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/TextUI/Command.php:101
Stack trace:
#0 /var/www/localhost/htdocs/vendor/phpunit/phpunit/phpunit(107): PHPUnit\TextUI\Command::main()
#1 /var/www/localhost/htdocs/vendor/bin/phpunit(122): include('...')
#2 {main}
  thrown in /var/www/localhost/htdocs/vendor/phpunit/phpunit/src/TextUI/Command.php on line 101

jdalsem · 2026-02-19T09:24:45Z

is your phpunit.xml configured correctly? did you copy it from elgg?

dploeger · 2026-02-19T09:33:36Z

is your phpunit.xml configured correctly? did you copy it from elgg?

Sorry, no. I just tried copying the phpunit.xml.dist to phpunit.xml, but it didn't work either. Do you have any hints on how to properly setup the test tools? I mainly tried to interpret your github workflow. 😄

(My PHP development skills are also rather rusted, so please bear with me 🙂 )

jdalsem · 2026-02-19T09:49:51Z

it depends probably on how you installed... my enviroments is a github pull of elgg/elgg into a folder (your_website). In the /mod folder i have pulled my individual projects (event_manager).

If you start a phpunit make sure you start it in the root folder of your website. You can use phpunit --filter StaticConfigIntegrationTest to limit to a specific test (not sure how to limit to a single plugin for your usecase)

dploeger · 2026-02-19T09:53:30Z

I've used the composer-way of installing elgg, but other than that it's quite the same. Maybe the error is because of another plugin I've installed.

dploeger · 2026-02-19T09:56:14Z

But about the test suite error:

1) Elgg\Plugins\StaticConfigIntegrationTest::testRouteRegistrations with data set #0 (ElggPlugin Object (...), 'event_manager')
Resource resources/event/export for route default:object:event:export does not exist
Failed asserting that false is true.

I believe it's because the ical export only exists in the ics viewtype and not the default. Should I add a stub for the default view or can this test be controlled to ignore the default view?

jdalsem · 2026-02-19T10:03:20Z

The test says the resource view is not found... which is true... the resource is configured for 'event/export', but you have the export resource in 'event/export/ical'... or is an obsolete route? As you also have 'collection:ical:export'... check if and why you need the route 'default:object:event:export'

dploeger · 2026-02-19T12:54:25Z

I'm using that route in Entity.php while providing the view type. Or should I use ical:object:event:export as the rouote name?

jdalsem · 2026-02-19T13:13:56Z

I'm using that route in Entity.php while providing the view type. Or should I use ical:object:event:export as the rouote name?

i would suggest you use just one ('collection:ical:export' makes the most sense)... but if you decide to move to ajax views... these would both become obsolete (no routes needed anymore)

dploeger · 2026-02-19T13:15:26Z

Okay. I'll check out that path then first.

dploeger marked this pull request as draft January 9, 2026 14:15

dploeger mentioned this pull request Jan 9, 2026

Questions about replacing iionly's event_calendar with this one #179

Closed

dploeger marked this pull request as ready for review January 27, 2026 14:11

dploeger force-pushed the feature/dpr/icaldirect branch from 82b58db to 18e497f Compare February 16, 2026 06:50

added: Integrated iCal import and export features

c0bbb1e

dploeger force-pushed the feature/dpr/icaldirect branch from 18e497f to c0bbb1e Compare February 16, 2026 09:13

fixed: Updated composer.lock

9a2baf8

fixed: Updated composer.lock and composer.json

01bf912

jdalsem requested changes Feb 17, 2026

View reviewed changes

fixed: First fixes of PR

d8f7138

dploeger commented Feb 18, 2026

View reviewed changes

fixed: Sanity Check import

2ada5f8

Missing German error translations

Uh oh!

Comments

Conversation

dploeger commented Jan 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dploeger commented Feb 4, 2026

Uh oh!

jdalsem commented Feb 9, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dploeger commented Feb 9, 2026

Uh oh!

jdalsem commented Feb 9, 2026

Uh oh!

dploeger commented Feb 9, 2026

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

jdalsem commented Feb 16, 2026

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

jdalsem commented Feb 16, 2026

Uh oh!

jdalsem commented Feb 16, 2026

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

jdalsem commented Feb 16, 2026

Uh oh!

dploeger commented Feb 16, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

dploeger commented Feb 16, 2026

Uh oh!

jdalsem left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

dploeger commented Jan 9, 2026 •

edited

Loading

jdalsem commented Feb 9, 2026 •

edited

Loading

dploeger commented Feb 16, 2026 •

edited

Loading