Event timing issue #1198
Event timing issue #1198
Conversation
📝 WalkthroughWalkthroughThe sync logic in the Apple Calendar plugin was refactored to batch-fetch events for all selected calendars in a single API call, improving efficiency. Calendar upserts now preserve existing IDs, and event synchronization logic was clarified and corrected to ensure accurate event updates, additions, and deletions. Changes
Sequence Diagram(s)sequenceDiagram
participant UserDatabase
participant AppleCalendarPlugin
participant EventKitAPI
UserDatabase->>AppleCalendarPlugin: sync_events(user_id)
AppleCalendarPlugin->>UserDatabase: Fetch selected calendar tracking IDs
AppleCalendarPlugin->>EventKitAPI: list_system_events_for_calendars(tracking_ids)
EventKitAPI-->>AppleCalendarPlugin: {tracking_id: [events], ...}
AppleCalendarPlugin->>UserDatabase: Map system events to DB calendar IDs
AppleCalendarPlugin->>UserDatabase: Upsert calendars (preserve IDs if matched)
AppleCalendarPlugin->>UserDatabase: Update existing events, add new, mark deleted
Estimated code review effort2 (~15 minutes) Possibly related PRs
✨ Finishing Touches
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. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![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
🧹 Nitpick comments (1)
plugins/apple-calendar/src/sync.rs (1)
277-326: Good batch fetching implementation with minor improvements needed.The batch event fetching function effectively optimizes API calls and includes proper rate limiting. The runtime detection and error handling are robust.
Consider these minor improvements:
- .unwrap_or_else(|e| std::collections::HashMap::new()) + .unwrap_or_else(|_| std::collections::HashMap::new())Also consider reducing the verbosity of logging inside the spawn_blocking closure, as excessive logging in tight loops can impact performance.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
plugins/apple-calendar/src/sync.rs(6 hunks)
📓 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:
plugins/apple-calendar/src/sync.rs
🧰 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:
plugins/apple-calendar/src/sync.rs
🔇 Additional comments (6)
plugins/apple-calendar/src/sync.rs (6)
34-51: Excellent performance optimization with batch API calls.The refactoring from individual calendar event fetches to a single batch API call significantly improves efficiency. The mapping logic correctly converts from tracking IDs to database IDs while maintaining proper error handling.
95-95: Good fix for preserving calendar IDs during upserts.This change correctly preserves existing calendar IDs when matching system calendars are found, preventing unnecessary ID regeneration and maintaining referential integrity.
138-138: Critical bug fix for calendar ID comparison.This correctly fixes the comparison to use the calendar's database ID instead of tracking ID when checking if a database event belongs to a selected calendar. The previous logic would have failed to properly identify events belonging to selected calendars.
150-150: Proper ID preservation during event updates.These changes correctly preserve database event IDs and URLs during sync operations, maintaining referential integrity and external links while updating event data from the system calendar.
Also applies to: 158-158, 171-171
185-191: Clean and correct event deletion logic.The simplified logic properly handles all deletion scenarios: events not found in system calendars, events from non-existent calendars, and events without calendar associations.
196-228: Robust new event addition with proper duplicate prevention.The implementation includes excellent safety checks and duplicate prevention at multiple levels. The logic correctly identifies genuinely new events while avoiding unnecessary database operations for already-handled or existing events.
@yujonglee
not to merge, for review at least now