Refactor synchronized calendar deletion

src/govoplan_calendar/backend/service.py

@@ -485,22 +485,23 @@ def update_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
     return calendar
 
 
-def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, payload: CalendarCollectionDeleteRequest | None = None) -> None:
-    payload = payload or CalendarCollectionDeleteRequest()
-    calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=calendar_id)
-    deleted_at = utcnow()
-    if payload.event_action == "move":
-        if not payload.target_calendar_id:
-            raise CalendarError("Target calendar is required when moving events")
-        if payload.target_calendar_id == calendar_id:
-            raise CalendarError("Target calendar must be different from the deleted calendar")
-        target_calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=payload.target_calendar_id)
-        source = None
-        target_source = None
-        if hasattr(session, "query"):
-            # Source creation takes the collection row lock before linking a
-            # source. Lock both collections in stable order so the mode cannot
-            # change between validation and the local/outbox mutation.
+def _lock_calendar_move_context(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar: CalendarCollection,
+    target_calendar: CalendarCollection,
+) -> tuple[
+    CalendarCollection,
+    CalendarCollection,
+    CalendarSyncSource | None,
+    CalendarSyncSource | None,
+]:
+    if not hasattr(session, "query"):
+        return calendar, target_calendar, None, None
+    # Source creation takes the collection row lock before linking a source.
+    # Lock both collections in stable order so their modes cannot change
+    # between validation and the local/outbox mutation.
     locked_calendars = (
         session.query(CalendarCollection)
         .filter(
@@ -533,7 +534,15 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
         source = locked_sources[source.id]
     if target_source is not None:
         target_source = locked_sources[target_source.id]
+    return calendar, target_calendar, source, target_source
 
+
+def _validate_calendar_move_source_pair(
+    payload: CalendarCollectionDeleteRequest,
+    *,
+    source: CalendarSyncSource | None,
+    target_source: CalendarSyncSource | None,
+) -> None:
     if payload.external_action == "remote_move":
         raise CalendarError(
             "external_action='remote_move' is not supported; no destructive remote move was performed"
@@ -543,15 +552,17 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
             "Events cannot be bulk-moved between synchronized calendars; "
             "external_action='remote_move' remains unsupported"
         )
-        active_events = [event for event in calendar.events if event.deleted_at is None]
-        previous_event_states = (
-            {
-                event.id: calendar_event_change_payload(event, prefix="previous_")
-                for event in active_events
-            }
-            if hasattr(session, "query")
-            else {}
-        )
+
+
+def _prepare_calendar_move_external_action(
+    session: Session,
+    *,
+    tenant_id: str,
+    payload: CalendarCollectionDeleteRequest,
+    source: CalendarSyncSource | None,
+    target_source: CalendarSyncSource | None,
+    deleted_at: datetime,
+) -> None:
     if source is not None:
         if payload.external_action != "detach_keep_remote":
             raise CalendarError(
@@ -559,15 +570,16 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
                 "external_action='detach_keep_remote'"
             )
         # Retirement rejects a live delivery lease before any local event
-            # projection is changed, and cancels all remaining undelivered
-            # desired state. It never creates a remote DELETE operation.
+        # projection is changed, and cancels all remaining undelivered desired
+        # state. It never creates a remote DELETE operation.
         retire_sync_source(
             session,
             tenant_id=tenant_id,
             source=source,
             deleted_at=deleted_at,
         )
-        elif target_source is not None:
+        return
+    if target_source is not None:
         if payload.external_action != "copy_to_remote":
             raise CalendarError(
                 "Moving local events to a synchronized calendar requires "
@@ -582,11 +594,40 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
                 "external_action='copy_to_remote' requires an active two-way CalDAV target"
             )
         assert_sync_mutation_allowed(target_source)
-        elif payload.external_action is not None:
+        return
+    if payload.external_action is not None:
         raise CalendarError(
             "external_action is only valid when moving events to or from a synchronized calendar"
         )
 
+
+def _active_events_with_previous_state(
+    session: Session,
+    calendar: CalendarCollection,
+) -> tuple[list[CalendarEvent], dict[str, dict[str, Any]]]:
+    active_events = [event for event in calendar.events if event.deleted_at is None]
+    previous_event_states = (
+        {
+            event.id: calendar_event_change_payload(event, prefix="previous_")
+            for event in active_events
+        }
+        if hasattr(session, "query")
+        else {}
+    )
+    return active_events, previous_event_states
+
+
+def _move_calendar_events(
+    session: Session,
+    *,
+    tenant_id: str,
+    payload: CalendarCollectionDeleteRequest,
+    target_calendar: CalendarCollection,
+    source: CalendarSyncSource | None,
+    target_source: CalendarSyncSource | None,
+    active_events: list[CalendarEvent],
+    previous_event_states: dict[str, dict[str, Any]],
+) -> None:
     for event in active_events:
         if source is not None or target_source is not None:
             event.source_kind = "local"
@@ -614,6 +655,73 @@ def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, paylo
     if payload.make_target_default:
         clear_default_calendar(session, tenant_id=tenant_id)
         target_calendar.is_default = True
+
+
+def _move_calendar_before_delete(
+    session: Session,
+    *,
+    tenant_id: str,
+    calendar: CalendarCollection,
+    payload: CalendarCollectionDeleteRequest,
+    deleted_at: datetime,
+) -> CalendarCollection:
+    if not payload.target_calendar_id:
+        raise CalendarError("Target calendar is required when moving events")
+    if payload.target_calendar_id == calendar.id:
+        raise CalendarError("Target calendar must be different from the deleted calendar")
+    target_calendar = get_calendar(
+        session,
+        tenant_id=tenant_id,
+        calendar_id=payload.target_calendar_id,
+    )
+    calendar, target_calendar, source, target_source = _lock_calendar_move_context(
+        session,
+        tenant_id=tenant_id,
+        calendar=calendar,
+        target_calendar=target_calendar,
+    )
+    _validate_calendar_move_source_pair(
+        payload,
+        source=source,
+        target_source=target_source,
+    )
+    active_events, previous_event_states = _active_events_with_previous_state(
+        session,
+        calendar,
+    )
+    _prepare_calendar_move_external_action(
+        session,
+        tenant_id=tenant_id,
+        payload=payload,
+        source=source,
+        target_source=target_source,
+        deleted_at=deleted_at,
+    )
+    _move_calendar_events(
+        session,
+        tenant_id=tenant_id,
+        payload=payload,
+        target_calendar=target_calendar,
+        source=source,
+        target_source=target_source,
+        active_events=active_events,
+        previous_event_states=previous_event_states,
+    )
+    return calendar
+
+
+def delete_calendar(session: Session, *, tenant_id: str, calendar_id: str, payload: CalendarCollectionDeleteRequest | None = None) -> None:
+    payload = payload or CalendarCollectionDeleteRequest()
+    calendar = get_calendar(session, tenant_id=tenant_id, calendar_id=calendar_id)
+    deleted_at = utcnow()
+    if payload.event_action == "move":
+        calendar = _move_calendar_before_delete(
+            session,
+            tenant_id=tenant_id,
+            calendar=calendar,
+            payload=payload,
+            deleted_at=deleted_at,
+        )
     elif payload.event_action != "delete":
         raise CalendarError(f"Unsupported calendar delete event action: {payload.event_action}")
     elif payload.external_action is not None: