JodliDev / calendar

GNU Affero General Public License v3.0
16 stars 12 forks source link

Fix #31: .ICS attachment Convert DateTimeImmutable before formatting it #32

Open dalgwen opened 1 year ago

dalgwen commented 1 year ago

closes #31

JodliDev commented 1 year ago

Hello Thank you so much for your fix. I have two concerns though: -) Unfortunately DateTime::createFromImmutable is not available before PHP 7.3 and the original project still has php 5.5 as their minimal requirement and I would like to stay as close as possible to their project to be able to incorporate future updates easily. A solution for that problem is to just recreate the object by using the timestamp (as I did here: https://github.com/JodliDev/libcalendaring/blob/f96299dbd7fd26599c01c4e64183a90f69bed645/libcalendaring.php#L199)

-) But more importantly: I have a feeling that we are trying to fix an underlying issue in https://github.com/JodliDev/libcalendaring As far as I can see, your changes correct a faulty output from either load_events() https://github.com/JodliDev/calendar/blob/f0a90df1d3395ae86b229f21dbb3a7c862a2cf40/drivers/caldav/caldav_driver.php#L1374 or (which I think is the culprit here) get_mail_ical_objects() in libcalendaring https://github.com/JodliDev/libcalendaring/blob/f96299dbd7fd26599c01c4e64183a90f69bed645/libcalendaring.php#L1172

If I am not mistaken, the output from load_events() is coming from the database, so it should not be Immutable - meaning we dont have to correct it. So in theory, if we fix get_mail_ical_objects() everything should work as expected. I will continue to investigate and see if my theory is correct.

dalgwen commented 1 year ago

OK, I see what you mean and why you would like to fix it at the source. I try to read the code of get_mail_ical_objects() in libcalendaring but php is DEFINITELY not my type 😅 Let me know if I can help you.