arborrow / montserrat

A retreat management application written in Laravel
12 stars 4 forks source link

Improve Implementation of Google Calendar Integration to avoid orphans #482

Closed arborrow closed 2 years ago

arborrow commented 2 years ago

Current implementation of Google Calendar integration is a bit sloppy and prone to producing orphans. An event is created; however, no exception is generated if the actual creation of the event fails making it possible that there would be calendar_id values in the event table that have no actual corresponding event on the Google calendar.

Further, if a Google Calendar event were to be deleted directly from the calendar you could have an abandoned calendar_id in the event table that would cause an error when trying to update that record. At the moment, there is no way to remove the abandoned calendar_id. Again, trying and returning an exception would be helpful here. If the event is not found, it would probably be better to remove the calendar_id from the event record. I would want to double check that the change from a value to null value is recorded in the audit table. In addition, it would be good to have a way for a site admin to be able to manually edit the value of the calendar_id field on the retreat.edit blade which would also be recorded in the audit table. An additional check should be made before saving manually entered data that it does not conflict with an existing value in the table (in other words - each calendar_id should correspond to one and only one event).

Another helpful addition would be to use the htmlLink - see https://developers.google.com/calendar/api/v3/reference/events - to be able to link directly to the Google calendar event from within Polanco. Currently, the code creates a link in the calendar description that links back to the Polanco event.

All exceptions should generate an email to the site admin notifying of the issue so that appropriate action can be taken. To ensure that all existing data is valid, an export of the calendar.ics isolating the UIDs for each event could be compared against all of the existing calendar_ids. Any missing ids should be deleted from the event table.

Finally, it never hurts to review the unit tests to make sure that the Google Calendar integration is functioning as intended in the case of where there is calendar integration and when there is not calendar integration. In fact, when there is no calendar integration the calendar_id field should be omitted on the retreat.edit blade.

arborrow commented 2 years ago

I have opted not to make the calendar_id field editable. Polanco will either be able to find the record or not. Polanco first ensures that Google Calendar Events are configured for use. If not, it will ignore attempts to create, find and update Google Calendar events.

During creation, it will attempt to create an event and if it fails it will flash a notice to the user that a Google calendar event was not created and save the calendar_id as null so that during updates it will try to create a new event if possible.

On the retreat.show blade, I have accessed the Google Calendar Event htmlLink property and appended it to the $retreat object so that if Google Calendar Events are configured the link to the Calendar Event will be available. If not, the value of calendar_id will be displayed.

On the retreat.edit blade, the field will be displayed as readonly. The update method will not change the value unless it is blank or if it fails to find an existing value. If the existing value does not exist, then Polanco will attempt to create a new Google Calendar Event. If the existing event is found it will be updated. If anything fails, then flash notices are displayed to the user letting them know of the issue. If failing to find and create, the calendar_id will be set to null. This should help to ensure that all values are functional.

arborrow commented 2 years ago

Just a note that if an event is removed/deleted from the Google calendar - the associated calendar_id is still visible; however, the status is changed to cancelled from confirmed. Hence, I have explicitly set the status to confirmed when updating to handle the case where a calendar event was deleted from the calendar but not in Polanco; otherwise, the calendar event is in fact being updated properly but does not display because the status continues to be cancelled.

arborrow commented 2 years ago

I am reopening this issue as the functionality of when an incorrect calendar_id is recorded it does not correct the bad data and seems to unsuccessfully create the new entry. Further testing is needed on that usage case. In addition, I would like to review the remaining bad data and make sure that they get processed accordingly. The following query displays the results of those that are in the event table but did not seem to be in the calendar. This may be a little dated so the mjrhgooglecalendar table may need to be updated:

SELECT e.id, e.title, e.start_date, e.calendar_id, e.deleted_at FROM event as e LEFT JOIN mjrhmastercalendar as m ON (m.uid = e.calendar_id) WHERE m.uid IS NULL AND e.calendar_id IS NOT NULL AND e.deleted_at IS NULL
ORDER BY e.start_date DESC;

arborrow commented 2 years ago

The update method was getting tripped up because it was not adequately distinguishing between creating a new event and saving an existing event. I added a check to see if the event calendar_id field matched the Google calendar event id. If they are the same then we are updating an existing event and if not then we are creating/inserting a new event.

arborrow commented 2 years ago

For events in 2017 that had invalid calendar_id values I just deleted those. If those events are updated in the future then a new id will be created for them. For events that were inactive with invalid calendar_id values I also deleted those calendar_id values. There were five active events that had invalid calendar_id values. I updated those manually in Polanco to allow Polanco to generate a new Google calendar event for those records and it handled them correctly. So all existing data should be on the calendar.