Send iCal notifications task #452
Conversation
| $hostaddress = str_replace('http://', '', $CFG->wwwroot); | ||
| $hostaddress = str_replace('https://', '', $hostaddress); | ||
|
|
||
| $ev = new \iCalendar_event; // To export in ical format. | ||
| $ev->add_property('uid', $zoom_event->id.'@'.$hostaddress); | ||
|
|
||
| // Set iCal event summary from event name. | ||
| $ev->add_property('summary', format_string($zoom_event->name, true, ['context' => $cal_event->context])); | ||
|
|
||
| $ev->add_property('description', html_to_text($description, 0)); | ||
|
|
||
| $ev->add_property('class', 'PUBLIC'); // PUBLIC / PRIVATE / CONFIDENTIAL | ||
|
|
||
| // Since we don't cater for modified invites, the created and last modified dates are the same | ||
| $ev->add_property('created', \Bennu::timestamp_to_datetime($zoom_event->timemodified)); | ||
| $ev->add_property('last-modified', \Bennu::timestamp_to_datetime($zoom_event->timemodified)); | ||
|
|
||
| $no_reply_user = \core_user::get_noreply_user(); | ||
| $ev->add_property('organizer', 'mailto:' . $no_reply_user->email, array('cn' => $this->get_lms_site_name())); | ||
| // Need to strip out the double quotations around the 'organizer' values - probably a bug in the core code | ||
| $ev->properties['ORGANIZER'][0]->value = substr($ev->properties['ORGANIZER'][0]->value, 1, -1); | ||
| $ev->properties['ORGANIZER'][0]->parameters['CN'] = substr($ev->properties['ORGANIZER'][0]->parameters['CN'], 1, -1); | ||
|
|
||
| $ev->add_property('dtstamp', \Bennu::timestamp_to_datetime()); // now | ||
| if ($zoom_event->timeduration > 0) { | ||
| //dtend is better than duration, because it works in Microsoft Outlook and works better in Korganizer | ||
| $ev->add_property('dtstart', \Bennu::timestamp_to_datetime($zoom_event->timestart)); // when event starts. | ||
| $ev->add_property('dtend', \Bennu::timestamp_to_datetime($zoom_event->timestart + $zoom_event->timeduration)); | ||
| } else if ($zoom_event->timeduration == 0) { | ||
| // When no duration is present, the event is instantaneous event, ex - Due date of a module. | ||
| // Moodle doesn't support all day events yet. See MDL-56227. | ||
| $ev->add_property('dtstart', \Bennu::timestamp_to_datetime($zoom_event->timestart)); | ||
| $ev->add_property('dtend', \Bennu::timestamp_to_datetime($zoom_event->timestart)); | ||
| } else { | ||
| // This can be used to represent all day events in future. | ||
| throw new \coding_exception("Negative duration is not supported yet."); | ||
| } |
There was a problem hiding this comment.
It seems like most of these are already handled by the zoom_helper_icalendar_event() helper function. I would be interested to know if any of the properties that are not being set by the helper function are necessary (because we may want to consider adding them to the helper function itself).
There was a problem hiding this comment.
Regression testing done and no issues found
There was a problem hiding this comment.
@jrchamp Will do some tests to see if the zoom_helper_icalendar_event() helper function is sufficient.
| mtrace('[Zoom ical Notifications] Checking to see if zoom ID ' . $zoom->id . ' was notified before.'); | ||
| $executiontime = $this->get_notification_execution_time($zoom->id); | ||
| // Only run if it hasn't run before. | ||
| if ($executiontime === 0) { |
There was a problem hiding this comment.
If this is only sending when there is no entry in the notifications table, then this notifications task will never notify users about updated event information. Probably need to change this to something like $executiontime < $lasttaskcompletiontime not that that variable exists, but it's pseudo-code.
There was a problem hiding this comment.
Hi @jrchamp , yes, for our purposes we had that buffer time period (think it's 15 minutes or so) just after the Zoom activity was created before sending the ical invitation. However, please feel free to add this functionality for updated events as well. Please let me know if you require more information.
There was a problem hiding this comment.
Hello @jrchamp
I hope you are doing well. I'm finally in the process of looking at this again. I'm in the process of creating a brand new PR for this ical notification functionality, but in the meantime I'd thought I try to give a better answer to your question above.
We give a buffer before sending out the ical event to avoid sending multiple event while the user is maybe making some last minute adjustments to the Zoom event.
After that initial buffer we thought it might not make sense to send additional updates, since we wouldn't have access to each users ical invite and it would result in another additional/duplicate invite being sent. But maybe that is unavoidable and would be the appropriate action to take.
Send iCal notifications task
Make use of new 'registration' instead of 'registration_required'. Fix task name catalog retrieval.
jrchamp
force-pushed
the
OpenCollab-ical_email_feature
branch
from
3b17662 to
bc22b3e
Compare
| $record = new \stdClass(); | ||
| $record->zoomid = $zoomid; | ||
| $record->executiontime = time(); | ||
|
|
||
| $DB->insert_record('zoom_ical_notifications', $record); |
There was a problem hiding this comment.
This handles inserting new records, but we may need it to be able to update existing records.
There was a problem hiding this comment.
@jrchamp The same logic applies here in the sense that since we wouldn't have access to each user's ical object (after it has been sent to them) we wouldn't necessarily want sent an additional ical invite. But maybe that is unavoidable and the better approach.
| $sql = 'SELECT * | ||
| FROM {zoom} | ||
| WHERE timemodified >= ' . (time() - 3600) . ' | ||
| AND timemodified <= ' . (time() - 600); |
There was a problem hiding this comment.
If the goal here is to notify users about events, we may want to check for Zoom events instead and index the notifications based on the event IDs instead of the zoom IDs.
Update send_ical_notifications.php
|
This PR can be closed. |
Send iCal notifications task