feat: Allow persistent GCal deletions #482

jamesvillarrubia · 2025-03-01T01:49:03Z

Type: feat

What kind of change does this PR introduce?
- This PR introduces a new feature for tracking manually deleted events to prevent them from being re-added during future synchronizations.
What is the current behavior?
- Previously, the application did not track deleted events, which could lead to events being re-added if they were deleted manually by the user within GCal.
What is the new behavior?
- The application now tracks manually deleted events and via a state-machine. This includes both single and recurring event instances. Events that are identified as manually deleted will not be re-added during the synchronization process.
Does this PR introduce a breaking change?
- No breaking changes are introduced.
Testing Process
1. Fully delete test calendar.
2. Sync calendar.
3. Delete two recurring instances of the same event. Delete one instance of a different recurring event. Delete one non recurring event.
4. Sync again.
5. Confirm events stay deleted.

jonas0b1011001 · 2025-03-01T07:48:47Z

Thank you very much for contributing.

Upon first inspection, the code does not distinguish between
events that have previously been added but are no longer part of the target calendar
and
events that are newly added to the source and are to be synced for the first time.

So a freshly set up script would never add any events cause the script treats all events as having been manually deleted.

Am i missing something?

jamesvillarrubia · 2025-03-03T15:34:09Z

@jonas0b1011001 Oof, yeah, refreshed this morning and ran into that. Came here this mornign to convert back to draft and saw your comment. So thanks for the speedy and thorough review!

Not sure why my original manual tests didn't catch this. I'll come back with a revision.

jamesvillarrubia · 2025-03-03T19:25:23Z

@jonas0b1011001 Totally revised the approach. Now status (from untracked, tracked, to deleted) is managed via a simple state machine. States are stored in memory.

I also updated my test process (see PR desc.) to fully wipe the test calendar and deleted multiple different types of events.

Let me know if you find any other issues or if there's something you think should be added.

jonas0b1011001 · 2025-03-03T19:53:57Z

Great, will take a look on the weekend!

Lonestarjeepin · 2025-03-04T20:51:26Z

@jonas0b1011001 Totally revised the approach. Now status (from untracked, tracked, to deleted) is managed via a simple state machine. States are stored in memory.

I also updated my test process (see PR desc.) to fully wipe the test calendar and deleted multiple different types of events.

Let me know if you find any other issues or if there's something you think should be added.

I've sometimes wished I had this feature, but from a user standpoint is there any way to get an event back after deletion? Or what if the source event changes, would it then repopulate? I'm not a good enough programmer to review your code so maybe you've already accounted for this!?! Just some thoughts from a user. Thanks!

jamesvillarrubia · 2025-03-04T21:22:18Z

@Lonestarjeepin they can add a duplicate event in the event source and it'll add back in since the originating ID will be different. Other than that, I'm open to ideas. If the source event is just updated, I don't believe it will not repopulate since the ID doesn't change. Perhaps there's an edge case for an event being deleted (for a conflict), but then moved... on the downstream GCal I think it would still be deleted. Not sure how to handle that.

jonas0b1011001 · 2025-03-05T15:28:30Z

I've sometimes wished I had this feature, but from a user standpoint is there any way to get an event back after deletion? Or what if the source event changes, would it then repopulate?

Good questions, just some quick thoughts on it (still have not had a closer look at the new code):

is there any way to get an event back after deletion

Google Calendar has a trash bin were deleted events sit for 30 days. The user can restore events from there.
manually_deleted events that reappear in calendarEvents would have to transition back to tracked.

what if the source event changes, would it then repopulate?

First we would need a way to track that it changed. I guess that would only be possible if we not only track the state of an event but also it's extendedProperties.private["MD5"].
Once we know that it changed, we can

silently readd the event
notify the user that a deleted event was updated
readd and notify
do nothing

jonas0b1011001 · 2025-03-09T11:46:12Z

I did some testing, for me the current version fails the test procedure you described in your first comment. Normal events are getting readded on every run.
This also reintroduces bugs with general recurrence handling, for example:
create a recurring event, rename only the first instance of the series. The instance gets duplicated on first sync.

I'll need more time for a full review/test.

jamesvillarrubia · 2025-03-09T15:08:37Z

Hmmmm... is there a public ics calendar you are using for tests? Maybe it's specific to the source calendar I'm using.

I've got calendars from my work outlook, but it doesn't give me much flexibility on test cases. Perhaps there's an ics file somewhere with a broad enough set of test events that we can both work from?

I'll step through the code again to see if there's a logic gap I missed.

Also any consideration for adding a test suite for the repo?

jonas0b1011001 · 2025-03-09T15:59:39Z

Hmmmm... is there a public ics calendar you are using for tests? Maybe it's specific to the source calendar I'm using.

I was using this simple ics.
Single will be readded on each run.
rec2 and rec10 get improperly synced because case EVENT_STATE.UNTRACKED: (L880) is unreachable, if i fix this (setEventState(fullEventId, EVENT_STATE.TRACKED); in L1438 only if recurrenceId is null) i get duplicates.

I'll step through the code again to see if there's a logic gap I missed.

Do you see my comments on the code or are they invisible unless i finish my review?

Also any consideration for adding a test suite for the repo?

Nothing planned atm.

jamesvillarrubia · 2025-03-10T16:52:19Z

I don't think I can see the comments until you close the review. Obviously close without approval and I'll debug some more and push some updates. Thanks for working with me on this1

jonas0b1011001

Will need to take a closer look at the recurrence part of it as i'm 100% sure it will cause issues at some point (as it does with every feature of the script)

jonas0b1011001 · 2025-03-09T08:08:08Z

Helpers.gs

+    case EVENT_STATE.TRACKED:
+      // Check if the event is still in the source
+      if (!icsEventsIds.includes(fullEventId)) {
+        setEventState(fullEventId, EVENT_STATE.REMOVED);


This code is unreachable, we are iterating over all source events.
Removed would have to be set in the cleanup function where we check for events in the target calendar that are not in the source file.

jonas0b1011001 · 2025-03-09T08:20:30Z

Helpers.gs

+
+    case EVENT_STATE.REMOVED:
+      // If the event reappears in the source, re-add it
+      if (icsEventsIds.includes(fullEventId)) {


Unneccessary if statement, at this point we know it reappeared.

jonas0b1011001 · 2025-03-09T08:32:34Z

Helpers.gs

+      break;
+
+    case EVENT_STATE.MANUALLY_DELETED:
+      // Do nothing, as the event should not be re-added


//Allow recovering manually deleted events if (calendarEventsIds.includes(fullEventId)) { setEventState(fullEventId, EVENT_STATE.TRACKED); if (isRecurringInstance) { processEventInstance(event); } else { processEvent(event, calendarTz); } }

jonas0b1011001 · 2025-03-09T08:52:17Z

Helpers.gs

+      break;
+
+    case EVENT_STATE.MANUALLY_DELETED:
+      // Do nothing, as the event should not be re-added


This would also be the place to check if the manually deleted event was updated.

We would have to

save to latest hash in processEvent/processEventInstance to know which hash the event had when deleted

calculate the current hash of the event in the source

event.removeProperty('dtstamp'); let digest = Utilities.computeDigest(Utilities.DigestAlgorithm.MD5, event.toString(), Utilities.Charset.UTF_8).toString();

compare saved and current hash

decide what to do in case the hash changed. readd yes/no, notify the user yes/no

jonas0b1011001 · 2025-03-09T09:06:02Z

Helpers.gs

+      setEventState(fullEventId, EVENT_STATE.TRACKED);
+      break;
+
+    case EVENT_STATE.TRACKED:


How do non-recurring events transition from TRACKED to MANUALLY_DELETED?

You are missing

if (!calendarEventsIds.includes(fullEventId)) { setEventState(fullEventId, EVENT_STATE.MANUALLY_DELETED); }

jonas0b1011001 · 2025-03-09T11:41:53Z

Helpers.gs

+ * @param {string} fullEventId - The full event ID to search for, in the format "eventId_recurrenceId"
+ * @return {Array.<Calendar.Event>} An array with a single element if a matching event instance is found, or an empty array if no match is found.
+ */
+function findEventInstanceToPatch(recEvent) {


This neither matches the doc nor the function call in Line 876.

jonas0b1011001 · 2025-03-10T17:01:23Z

Code.gs

    Logger.log("Processing " + recurringEvents.length + " Recurrence Instances!");
    for (var recEvent of recurringEvents){
-      processEventInstance(recEvent);
+      processEventWithState(recEvent, calendarTz, true); // Use the state machine for recurring instances


Is there a need for instances to go through the state machine(s) 3 times?

jonas0b1011001 · 2025-03-10T17:04:16Z

Helpers.gs

+
+  // Check the current state of the event instance
+  switch (currentState) {
+    case EVENT_STATE.UNTRACKED:


Unreachable - all instances will be set to TRACKED the first time they enter the statemachine in processEventWithState().

jonas0b1011001 · 2025-03-10T17:14:32Z

Helpers.gs

+ * @param {string} fullEventId - The full event ID to search for, in the format "eventId_recurrenceId"
+ * @return {Array.<Calendar.Event>} An array with a single element if a matching event instance is found, or an empty array if no match is found.
+ */
+function findEventInstanceToPatch(recEvent) {


This function is incomplete and will only ever match instances that where patched by the script (privateExtendedProperty : "rec-id="), however we need this to be able to match 'all' instances, i.e. instances that are created by google from a recurring event (recurrence rules are 'expanded' by google and not by the script)

Need to test:

function findEventInstanceToPatch(recEvent) { if (recEvent.recurringEventId.length > 10 && recEvent.recurringEventId.substr(-1) !== "Z"){ recEvent.recurringEventId += "Z"; } eventInstanceToPatch = callWithBackoff(function(){ return Calendar.Events.list(targetCalendarId, { singleEvents : true, orderBy : "startTime", timeZone: "etc/GMT", privateExtendedProperty : "fromGAS=true", privateExtendedProperty : "id=" + recEvent.extendedProperties.private["id"] }).items; }, defaultMaxRetries); // If the event instance is not found, it may be deleted. return eventInstanceToPatch.filter(function(e){ return ((e.originalStartTime.dateTime || e.originalStartTime.date) == recEvent.recurringEventId); }); }

jamesvillarrubia marked this pull request as draft March 3, 2025 15:30

Refactor to use State Machine for events to track deleted status.

b76d255

jamesvillarrubia force-pushed the feature/allow-persistent-gcal-deletions branch from d1baee8 to b76d255 Compare March 3, 2025 19:19

jamesvillarrubia marked this pull request as ready for review March 3, 2025 19:23

jonas0b1011001 requested changes Mar 10, 2025

View reviewed changes

Uh oh!

feat: Allow persistent GCal deletions #482

Are you sure you want to change the base?

feat: Allow persistent GCal deletions #482

Uh oh!

Conversation

jamesvillarrubia commented Mar 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonas0b1011001 commented Mar 1, 2025

Uh oh!

jamesvillarrubia commented Mar 3, 2025

Uh oh!

jamesvillarrubia commented Mar 3, 2025

Uh oh!

jonas0b1011001 commented Mar 3, 2025

Uh oh!

Lonestarjeepin commented Mar 4, 2025

Uh oh!

jamesvillarrubia commented Mar 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jonas0b1011001 commented Mar 5, 2025

Uh oh!

jonas0b1011001 commented Mar 9, 2025

Uh oh!

jamesvillarrubia commented Mar 9, 2025

Uh oh!

jonas0b1011001 commented Mar 9, 2025

Uh oh!

jamesvillarrubia commented Mar 10, 2025

Uh oh!

jonas0b1011001 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!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

jamesvillarrubia commented Mar 1, 2025 •

edited

Loading

jamesvillarrubia commented Mar 4, 2025 •

edited

Loading