Conversation
📝 WalkthroughWalkthroughAdds is_recurring to Event and threads it through database update/upsert paths. Updates Apple Calendar sync to handle migration from non-recurring to recurring events, introducing session transfer logic and reordering sync steps: create/upsert, update, transfer sessions, then delete. Changes
Sequence Diagram(s)sequenceDiagram
participant Calendar as Apple Calendar
participant Sync as Sync Engine
participant DB as Event DB
participant Sessions as Session Store
Calendar->>Sync: Fetch system events
Sync->>Sync: Detect recurring vs non-recurring
alt Recurring replacing old non-recurring
Sync->>DB: Find matching non-recurring event (base_id/date/name)
Sync->>DB: Upsert new event (new id, composite tracking_id, is_recurring=true)
Sync->>Sessions: Record session transfer (old_session -> new_event)
end
Sync->>DB: Upsert new events
Sync->>DB: Update existing events (incl. is_recurring)
Sync->>Sessions: Relink sessions to new events
Sync->>DB: Delete obsolete events
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Tip 🔌 Remote MCP (Model Context Protocol) integration is now available!Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats. ✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
Status, Documentation and Community
|
![cubic-dev-ai[bot]](https://avatars.githubusercontent.com/in/1082092?s=60&v=4){kind=small}
There was a problem hiding this comment.
cubic analysis
No issues found across 2 files. Review in cubic
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🔭 Outside diff range comments (1)
plugins/apple-calendar/src/sync.rs (1)
255-271: Critical: Session transfer is skipped when the recurring instance already existsThe early continue on "already_exists" prevents the backward-compatibility session transfer from running. If the recurring instance is already in DB and the old non-recurring event has a session, you’ll delete the old event and orphan the session.
Move the backward-compatibility branch before the “already exists” check and, when the recurring instance already exists, re-link the session to that existing event instead of creating a duplicate.
Apply this diff:
- // Skip if this event already exists in the database with the same tracking_id - let already_exists = db_events_with_session - .iter() - .any(|(db_event, _)| db_event.tracking_id == composite_tracking_id); - if already_exists { - continue; - } - - // Check for backward compatibility: recurring event replacing old non-recurring - if system_event.is_recurring { - // Look for old format event with session - if let Some((_, session)) = - db_events_with_session.iter().find(|(db_event, session)| { - db_event.tracking_id == system_event.id && // Old format used base ID only - db_event.start_date == system_event.start_date && - db_event.name == system_event.name && - !db_event.is_recurring && // Was stored as non-recurring - session.is_some() // Has a session - }) - { - // Store session ID for transfer after new event is created - if let Some(session) = session { - let new_event_id = uuid::Uuid::new_v4().to_string(); - state - .session_transfers - .push((session.id.clone(), new_event_id.clone())); - - let new_event = hypr_db_user::Event { - id: new_event_id, - tracking_id: composite_tracking_id, - user_id: user_id.clone(), - calendar_id: Some(db_calendar.id.clone()), - name: system_event.name.clone(), - note: system_event.note.clone(), - start_date: system_event.start_date, - end_date: system_event.end_date, - google_event_url: None, - participants: Some( - serde_json::to_string(&system_event.participants) - .unwrap_or_else(|_| "[]".to_string()), - ), - is_recurring: system_event.is_recurring, - }; - - state.to_upsert.push(new_event); - continue; - } - } - } + // Backward-compatibility: if a recurring instance replaces a previously stored + // non-recurring event that had a session, transfer the session. + if system_event.is_recurring { + // Locate the old-format event with a session + if let Some((_, Some(session))) = db_events_with_session.iter().find(|(db_event, session)| { + db_event.tracking_id == system_event.id && // old base ID + db_event.name == system_event.name && + // allow small clock skew (<= 1 minute) + (db_event.start_date - system_event.start_date).num_minutes().abs() <= 1 && + !db_event.is_recurring && + session.is_some() + }) { + // If the recurring instance already exists, just re-link the session to it. + if let Some((existing_recurring, _)) = db_events_with_session + .iter() + .find(|(db_event, _)| db_event.tracking_id == composite_tracking_id) + { + state + .session_transfers + .push((session.id.clone(), existing_recurring.id.clone())); + continue; + } + + // Otherwise, create the recurring instance and re-link the session to it. + let new_event_id = uuid::Uuid::new_v4().to_string(); + state + .session_transfers + .push((session.id.clone(), new_event_id.clone())); + + let new_event = hypr_db_user::Event { + id: new_event_id, + tracking_id: composite_tracking_id, + user_id: user_id.clone(), + calendar_id: Some(db_calendar.id.clone()), + name: system_event.name.clone(), + note: system_event.note.clone(), + start_date: system_event.start_date, + end_date: system_event.end_date, + google_event_url: None, + participants: Some( + serde_json::to_string(&system_event.participants) + .unwrap_or_else(|_| "[]".to_string()), + ), + is_recurring: system_event.is_recurring, + }; + state.to_upsert.push(new_event); + continue; + } + } + + // Skip if this event already exists in the database with the same tracking_id + if db_events_with_session + .iter() + .any(|(db_event, _)| db_event.tracking_id == composite_tracking_id) + { + continue; + }Also applies to: 272-313
🧹 Nitpick comments (4)
crates/db-user/src/events_ops.rs (1)
250-251: Add assertion for is_recurring persistence in testsYou initialize is_recurring but don’t assert it. Add a check to ensure persistence is wired correctly.
You can extend the test like:
// after upsert assert_eq!(event.is_recurring, false);plugins/apple-calendar/src/sync.rs (3)
276-281: Relax exact start_date equality for robust matchingExact equality on start_date is brittle due to timezone and rounding differences. A small tolerance (e.g., <= 1 minute) is safer for the BC matching condition.
The diff above already applies this relaxation:
- db_event.start_date == system_event.start_date && + (db_event.start_date - system_event.start_date).num_minutes().abs() <= 1 &&
245-253: Consider extracting composite tracking ID logicThis formatting appears in multiple places. A small helper reduces duplication and drift.
For example:
fn compose_tracking_id(id: &str, start: chrono::DateTime<chrono::Utc>, is_recurring: bool) -> String { if is_recurring { format!("{}__HYPR__{}", id, start.format("%Y%m%d%H%M%S")) } else { id.to_string() } }
609-623: Tests don’t cover the BC session transfer pathAdd a case where a non-recurring DB event with a session is replaced by a recurring system event instance; assert the session is re-linked and the old event ends up in to_delete.
I can draft a focused unit test harnessing in-memory state to simulate this scenario. Want me to add it?
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these settings in your CodeRabbit configuration.
📒 Files selected for processing (2)
crates/db-user/src/events_ops.rs(6 hunks)plugins/apple-calendar/src/sync.rs(4 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.{js,ts,tsx,rs}
⚙️ CodeRabbit Configuration File
**/*.{js,ts,tsx,rs}: 1. No error handling.
2. No unused imports, variables, or functions.
3. For comments, keep it minimal. It should be about "Why", not "What".
Files:
crates/db-user/src/events_ops.rsplugins/apple-calendar/src/sync.rs
🔇 Additional comments (6)
crates/db-user/src/events_ops.rs (2)
43-45: is_recurring column & migration verified — OK to propagate in UPDATEConfirmed in repo: a migration adds the column and code already binds/uses it. No code changes required.
- crates/db-user/src/events_migration_2.sql —
ALTER TABLE events ADD COLUMN is_recurring BOOLEAN DEFAULT FALSE;- crates/db-user/src/events_migration.sql — original CREATE TABLE (does not include is_recurring; migration covers the addition)
- crates/db-user/src/events_ops.rs — UPDATE/UPSERT use
is_recurring = :is_recurringand pass":is_recurring": event.is_recurring(both occurrences updated)- crates/db-user/src/events_types.rs —
pub is_recurring: bool- plugins/db/js/bindings.gen.ts —
Eventtype includesis_recurring: booleanRecommendation: ensure the migration has been applied in each deployment environment (operational check).
89-91: No change required — tracking_id is globally UNIQUE, ON CONFLICT(tracking_id) is correctVerified: crates/db-user/src/events_migration.sql declares
tracking_id TEXT NOT NULL UNIQUE, soON CONFLICT(tracking_id)matches the schema and will not collide across users.Files checked:
- crates/db-user/src/events_migration.sql —
tracking_id TEXT NOT NULL UNIQUE- crates/db-user/src/events_ops.rs —
ON CONFLICT(tracking_id) DO UPDATE ...(around line ~103)- crates/db-user/src/calendars_migration.sql —
tracking_id TEXT NOT NULL UNIQUE- crates/db-user/src/calendars_ops.rs —
ON CONFLICT(tracking_id) DO UPDATE ...(around line ~99)Action: ignore the suggested diff to switch to
ON CONFLICT(user_id, tracking_id)— no change required.plugins/apple-calendar/src/sync.rs (4)
544-545: Session transfer state is a clean extensionAdding session_transfers to EventSyncState is appropriate and keeps execution concerns localized.
565-571: Reordering: upsert before update is soundCreating new events before updates avoids foreign-key issues with session re-linking.
579-585: Session transfers sequenced after upsertsGood call to transfer sessions after ensuring the target event exists.
586-590: Delete last to preserve referential integrityDeleting old events after transfers prevents dangling references.
1 participant
Summary by cubic
Fixed recurring event handling in Apple Calendar sync to transfer sessions from old non-recurring events and preserve data integrity. Improved is_recurring field support when saving events to the database.