Proper apple calendar support in new bundle#2232
Conversation
📝 WalkthroughWalkthroughThis PR introduces a comprehensive Apple Calendar synchronization system spanning database schema extensions, a scheduled task-based event sync mechanism, updated Apple Calendar plugin with improved error handling and modularization, and UI components for calendar selection and refresh controls. Changes include adding Core Graphics dependency, extending the calendar data model with tracking IDs and source/provider fields, implementing periodic event fetching via a chunked query system, refactoring the Rust plugin with granular extraction helpers and result-based error handling, and relocating task management infrastructure. Changes
Sequence DiagramsequenceDiagram
participant UI as Timeline UI
participant TaskMgr as Task Manager
participant Sync as useCalendarSyncTask
participant Plugin as Apple Calendar Plugin
participant Store as Calendar Store
participant AppleOS as Apple System
UI->>TaskMgr: Initialize
TaskMgr->>Sync: Execute useCalendarSyncTask(store, userId, calendars)
Sync->>Plugin: Query permission via useQuery
Plugin->>AppleOS: Check calendar access
AppleOS-->>Plugin: Permission status
Plugin-->>Sync: Access granted
Sync->>Sync: Generate calendar chunks<br/>(7d past, current, 7d future, 7-14d)
Sync->>TaskMgr: Schedule recurring task<br/>(CALENDAR_FETCH_TASK_ID)
TaskMgr->>Sync: Task triggered (periodic)
Sync->>Plugin: listEvents(currentCalendar,<br/>currentChunk)
Plugin->>AppleOS: Fetch calendar events
AppleOS-->>Plugin: Events for chunk
Plugin-->>Sync: AppleEvent[]
Sync->>Sync: Map to internal calendar IDs<br/>via calendarMap
Sync->>Store: storeEvents(events)<br/>→ persist rows
Store-->>Sync: Stored
Sync->>Sync: Advance to next chunk/<br/>calendar
Sync->>TaskMgr: scheduleNextChunk()<br/>via callback
Note over UI,Store: User clicks Refresh<br/>in Timeline
UI->>Sync: triggerCalendarSync()
Sync->>Sync: Reset chunk iteration
Sync->>TaskMgr: Run task immediately
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes
Possibly related PRs
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
✅ Deploy Preview for hyprnote ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
✅ Deploy Preview for hyprnote-storybook ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (11)
plugins/apple-calendar/src/contact_resolver.rs (2)
22-93: Consider improving error handling for the contacts query.At line 54, the
errorparameter is passed asstd::ptr::null_mut(), which means any query errors will be silently ignored. While the function returnsOptionand handles theNonecase gracefully, explicit error handling could improve debugging.Consider capturing the error parameter to log failures:
+ let mut error: *mut NSError = std::ptr::null_mut(); let contacts: Option<Retained<NSArray<CNContact>>> = unsafe { msg_send![ &*contact_store, unifiedContactsMatchingPredicate: &*predicate, keysToFetch: &*keys_to_fetch, - error: std::ptr::null_mut::<*mut NSError>() + error: &mut error ] }; + + if !error.is_null() { + // Log error if needed + }
95-99: Consider using a constant for the authorization status.The hardcoded
3value is documented with a comment, but using a constant would be more maintainable.+const CN_AUTHORIZATION_STATUS_AUTHORIZED: i64 = 3; + fn has_contacts_access() -> bool { let status = unsafe { CNContactStore::authorizationStatusForEntityType(CNEntityType::Contacts) }; - status.0 == 3 // CNAuthorizationStatus::Authorized + status.0 == CN_AUTHORIZATION_STATUS_AUTHORIZED }apps/desktop/src/components/tasks/index.tsx (1)
5-14: Type assertion on line 11 could be improved.The cast to
CalendarStore | undefinedsuggests a type mismatch between the main store and the calendar-specific store type. Consider either:
- Exporting proper store types that align between modules, or
- Adding a runtime check before the cast
The component structure is correct for a side-effect-only component that registers background tasks.
Consider adding a type guard:
export function TaskManager() { const store = main.UI.useStore(main.STORE_ID); const { user_id } = main.UI.useValues(main.STORE_ID); const calendars = main.UI.useTable("calendars", main.STORE_ID); useUpdateCheckTask(); - useCalendarSyncTask(store as CalendarStore | undefined, user_id, calendars); + useCalendarSyncTask( + store && 'getTables' in store ? (store as CalendarStore) : undefined, + user_id, + calendars + ); return null; }plugins/apple-calendar/src/lib.rs (1)
89-122: Consider adding assertions to tests.The tests currently only print results without assertions. While this is useful for manual verification, adding assertions would improve test reliability and catch regressions automatically.
Example refactor for
test_list_calendars:#[test] fn test_list_calendars() { let app = create_app(tauri::test::mock_builder()); - let calendars = app.list_calendars(); - println!("calendars: {:?}", calendars); + let calendars = app.list_calendars().expect("Failed to list calendars"); + assert!(!calendars.is_empty(), "Expected at least one calendar"); + println!("calendars: {:?}", calendars); }apps/desktop/src/components/main/sidebar/timeline/index.tsx (1)
22-23: Duplicate task ID constant.The task ID
"fetchAppleCalendarChunk"is also defined inapps/desktop/src/components/tasks/calendar-sync/index.ts(line 23). Consider exporting it from a shared location to avoid potential drift.-const CALENDAR_FETCH_TASK_ID = "fetchAppleCalendarChunk"; +import { TASK_ID as CALENDAR_FETCH_TASK_ID } from "../../tasks/calendar-sync";Alternatively, export the constant from
calendar-sync/index.tsand import it here.apps/desktop/src/components/settings/calendar/configure/shared.tsx (1)
84-95: Consider adding accessibility label to the Switch.The
Switchlacks an explicit accessibility label. While the adjacent text provides visual context, screen readers may not associate them.function CalendarToggleRow({ calendar, enabled, onToggle, }: { calendar: CalendarItem; enabled: boolean; onToggle: (enabled: boolean) => void; }) { + const labelId = `calendar-label-${calendar.id}`; return ( <div className="flex items-center justify-between gap-3 py-1"> <div className="flex items-center gap-2 flex-1 min-w-0"> <div className="size-3 rounded-full shrink-0" style={{ backgroundColor: calendar.color ?? "#888" }} /> - <span className="text-sm truncate">{calendar.title}</span> + <span id={labelId} className="text-sm truncate">{calendar.title}</span> </div> - <Switch checked={enabled} onCheckedChange={onToggle} /> + <Switch checked={enabled} onCheckedChange={onToggle} aria-labelledby={labelId} /> </div> ); }apps/desktop/src/components/tasks/calendar-sync/index.ts (2)
83-95: Add defensive error handling for JSON.parse.
JSON.parse(arg)can throw on malformed input. While this is an internal task argument, defensive handling prevents silent failures.const { chunks, calendarTrackingIds: storedCalendarIds, currentChunkIndex, currentCalendarIndex, - }: ChunkArg = arg - ? JSON.parse(arg) - : { + }: ChunkArg = (() => { + if (!arg) { + return { chunks: generateCalendarChunks(), calendarTrackingIds, currentChunkIndex: 0, currentCalendarIndex: 0, }; + } + try { + return JSON.parse(arg); + } catch { + console.warn("[Calendar] Invalid task arg, resetting to default"); + return { + chunks: generateCalendarChunks(), + calendarTrackingIds, + currentChunkIndex: 0, + currentCalendarIndex: 0, + }; + } + })();
23-24: Export TASK_ID for reuse.The timeline component (
timeline/index.tsx) duplicates this constant. Export it to ensure consistency.-const TASK_ID = "fetchAppleCalendarChunk"; -const SYNC_INTERVAL = 5 * 60 * 1000; +export const TASK_ID = "fetchAppleCalendarChunk"; +export const SYNC_INTERVAL = 5 * 60 * 1000;apps/desktop/src/components/settings/calendar/configure/apple.tsx (1)
220-232: Consider using theisLoadingstate.The
useAppleCalendarSelectionhook returnsisLoadingbut it's not destructured or used. Consider showing a loading indicator while calendars are being fetched.function AppleCalendarSelection() { - const { groups, isCalendarEnabled, handleToggle, handleRefresh } = + const { groups, isLoading, isCalendarEnabled, handleToggle, handleRefresh } = useAppleCalendarSelection(); + if (isLoading) { + return ( + <div className="py-4 text-center text-sm text-neutral-500"> + Loading calendars... + </div> + ); + } + return ( <CalendarSelection groups={groups} isCalendarEnabled={isCalendarEnabled} onToggle={handleToggle} onRefresh={handleRefresh} /> ); }apps/desktop/src/components/tasks/calendar-sync/utils.ts (1)
64-76: Tighten runtime checks ingetEnabledAppleCalendarMap(unknown-typed row data).
dataisRecord<string, unknown>, but the code assumesprovideris a string,enabledis1, andtracking_idis a string. Addtypeofguards to avoid surprising matches (e.g.,enabled: true,tracking_id: 0).for (const [rowId, data] of Object.entries(calendars)) { - if (data.provider === "apple" && data.enabled === 1 && data.tracking_id) { - result[data.tracking_id as string] = rowId; - } + const provider = data.provider; + const enabled = data.enabled; + const trackingId = data.tracking_id; + if ( + provider === "apple" && + enabled === 1 && + typeof trackingId === "string" && + trackingId.length > 0 + ) { + result[trackingId] = rowId; + } }plugins/apple-calendar/src/apple.rs (1)
658-698:extract_color_components: small cleanup + preserve alpha in fallback.
#[allow(unused_variables)]looks stale (everything is used now).- In the fallback branches, consider using the computed
alphainstead of hardcoding1.0so translucent calendar colors don’t get lost.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (6)
Cargo.lockis excluded by!**/*.lockplugins/apple-calendar/js/bindings.gen.tsis excluded by!**/*.gen.tsplugins/permissions/Cargo.tomlis excluded by!plugins/**/permissions/**plugins/permissions/src/commands.rsis excluded by!plugins/**/permissions/**plugins/permissions/src/ext.rsis excluded by!plugins/**/permissions/**plugins/permissions/src/lib.rsis excluded by!plugins/**/permissions/**
📒 Files selected for processing (24)
Cargo.toml(1 hunks)apps/desktop/src/components/devtool/seed/shared/calendar.ts(1 hunks)apps/desktop/src/components/main/sidebar/timeline/index.tsx(5 hunks)apps/desktop/src/components/settings/calendar/configure/apple.tsx(3 hunks)apps/desktop/src/components/settings/calendar/configure/cloud.tsx(1 hunks)apps/desktop/src/components/settings/calendar/configure/index.tsx(1 hunks)apps/desktop/src/components/settings/calendar/configure/shared.tsx(1 hunks)apps/desktop/src/components/task-manager.tsx(0 hunks)apps/desktop/src/components/tasks/calendar-sync/index.ts(1 hunks)apps/desktop/src/components/tasks/calendar-sync/types.ts(1 hunks)apps/desktop/src/components/tasks/calendar-sync/utils.ts(1 hunks)apps/desktop/src/components/tasks/index.tsx(1 hunks)apps/desktop/src/components/tasks/update-check.ts(1 hunks)apps/desktop/src/main.tsx(1 hunks)apps/desktop/src/store/tinybase/main.ts(4 hunks)packages/db/src/schema.ts(1 hunks)packages/store/src/schema-external.ts(2 hunks)plugins/apple-calendar/Cargo.toml(1 hunks)plugins/apple-calendar/src/apple.rs(11 hunks)plugins/apple-calendar/src/contact_resolver.rs(1 hunks)plugins/apple-calendar/src/error.rs(1 hunks)plugins/apple-calendar/src/ext.rs(1 hunks)plugins/apple-calendar/src/lib.rs(2 hunks)plugins/apple-calendar/src/model.rs(1 hunks)
💤 Files with no reviewable changes (1)
- apps/desktop/src/components/task-manager.tsx
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx}: Avoid creating a bunch of types/interfaces if they are not shared. Especially for function props, just inline them instead.
Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
If there are many classNames with conditional logic, usecn(import from@hypr/utils). It is similar toclsx. Always pass an array and split by logical grouping.
Usemotion/reactinstead offramer-motion.
Files:
apps/desktop/src/components/settings/calendar/configure/index.tsxapps/desktop/src/components/tasks/update-check.tsapps/desktop/src/components/tasks/index.tsxapps/desktop/src/main.tsxapps/desktop/src/components/tasks/calendar-sync/types.tsapps/desktop/src/components/tasks/calendar-sync/index.tsapps/desktop/src/components/devtool/seed/shared/calendar.tsapps/desktop/src/store/tinybase/main.tsapps/desktop/src/components/settings/calendar/configure/cloud.tsxapps/desktop/src/components/main/sidebar/timeline/index.tsxpackages/store/src/schema-external.tspackages/db/src/schema.tsapps/desktop/src/components/tasks/calendar-sync/utils.tsapps/desktop/src/components/settings/calendar/configure/shared.tsxapps/desktop/src/components/settings/calendar/configure/apple.tsx
**/*.ts
📄 CodeRabbit inference engine (CLAUDE.md)
**/*.ts: Agent implementations should use TypeScript and follow the established architectural patterns defined in the agent framework
Agent communication should use defined message protocols and interfaces
Files:
apps/desktop/src/components/tasks/update-check.tsapps/desktop/src/components/tasks/calendar-sync/types.tsapps/desktop/src/components/tasks/calendar-sync/index.tsapps/desktop/src/components/devtool/seed/shared/calendar.tsapps/desktop/src/store/tinybase/main.tspackages/store/src/schema-external.tspackages/db/src/schema.tsapps/desktop/src/components/tasks/calendar-sync/utils.ts
plugins/*/src/lib.rs
📄 CodeRabbit inference engine (plugins/AGENTS.md)
After updating commands in
plugins/<NAME>/src/lib.rs, runcodegen, updateplugins/<NAME>/permissions/default.toml, andapps/desktop/src-tauri/capabilities/default.json
Files:
plugins/apple-calendar/src/lib.rs
🧠 Learnings (1)
📚 Learning: 2025-11-24T16:32:23.055Z
Learnt from: CR
Repo: fastrepl/hyprnote PR: 0
File: AGENTS.md:0-0
Timestamp: 2025-11-24T16:32:23.055Z
Learning: Applies to **/*.{ts,tsx} : Never do manual state management for form/mutation. Use useForm (from tanstack-form) and useQuery/useMutation (from tanstack-query) instead for 99% of cases. Avoid patterns like setError.
Applied to files:
apps/desktop/src/components/settings/calendar/configure/apple.tsx
🧬 Code graph analysis (12)
apps/desktop/src/components/settings/calendar/configure/index.tsx (3)
apps/desktop/src/hooks/usePlatform.ts (1)
useIsMacos(13-15)apps/desktop/src/components/settings/calendar/configure/apple.tsx (1)
AppleCalendarProviderCard(234-285)apps/desktop/src/components/settings/calendar/configure/cloud.tsx (1)
DisabledProviderCard(9-38)
plugins/apple-calendar/src/model.rs (1)
plugins/apple-calendar/src/apple.rs (1)
default(81-84)
apps/desktop/src/components/tasks/update-check.ts (1)
apps/desktop/src/components/main/sidebar/profile/ota/task.ts (1)
checkForUpdate(8-28)
plugins/apple-calendar/src/ext.rs (1)
plugins/apple-calendar/src/apple.rs (2)
default(81-84)list_events(145-170)
apps/desktop/src/components/tasks/calendar-sync/index.ts (2)
apps/desktop/src/components/tasks/calendar-sync/utils.ts (3)
getEnabledAppleCalendarMap(64-76)generateCalendarChunks(19-37)storeEvents(39-62)apps/desktop/src/components/tasks/calendar-sync/types.ts (1)
ChunkArg(1-6)
apps/desktop/src/components/devtool/seed/shared/calendar.ts (1)
apps/desktop/src/utils/index.ts (1)
DEFAULT_USER_ID(20-20)
apps/desktop/src/components/settings/calendar/configure/cloud.tsx (2)
extensions/shared/types/hypr-extension.d.ts (2)
AccordionItem(120-120)AccordionTrigger(121-121)packages/utils/src/cn.ts (1)
cn(20-22)
apps/desktop/src/components/main/sidebar/timeline/index.tsx (2)
packages/db/src/schema.ts (1)
calendars(203-214)extensions/shared/types/hypr-extension.d.ts (1)
Button(159-159)
packages/store/src/schema-external.ts (1)
packages/db/src/schema.ts (1)
calendarSchema(335-335)
apps/desktop/src/components/settings/calendar/configure/shared.tsx (1)
extensions/shared/types/hypr-extension.d.ts (2)
Button(159-159)Switch(437-437)
apps/desktop/src/components/settings/calendar/configure/apple.tsx (2)
apps/desktop/src/components/settings/calendar/configure/shared.tsx (2)
CalendarItem(6-10)CalendarSelection(24-73)packages/db/src/schema.ts (1)
calendars(203-214)
plugins/apple-calendar/src/apple.rs (2)
plugins/apple-calendar/src/model.rs (1)
default(70-76)plugins/apple-calendar/src/contact_resolver.rs (2)
safe_participant_schedule_status(155-168)resolve_participant_contact(9-20)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (8)
- GitHub Check: Redirect rules - hyprnote
- GitHub Check: Header rules - hyprnote
- GitHub Check: Pages changed - hyprnote
- GitHub Check: desktop_ci (macos, depot-macos-14)
- GitHub Check: desktop_ci (linux, depot-ubuntu-22.04-8)
- GitHub Check: fmt
- GitHub Check: ci
- GitHub Check: desktop_ci (linux, depot-ubuntu-24.04-8)
🔇 Additional comments (27)
apps/desktop/src/components/devtool/seed/shared/calendar.ts (2)
21-27: Good variety for seed data.The randomly selected source values provide useful test coverage across different calendar types. This will help ensure the UI handles various calendar sources correctly.
33-37: The provider and source field relationship is intentional and correctly designed. Apple Calendar natively aggregates calendars from multiple sources (iCloud, Exchange, Google, Local, Subscribed), which is reflected in the seed data. The hardcodedprovider: "apple"with variablesourcevalues mirrors real usage inapps/desktop/src/components/settings/calendar/configure/apple.tsx, where actual Apple Calendar data follows the same pattern. No changes needed.plugins/apple-calendar/src/model.rs (1)
69-77: LGTM!The Default implementation follows Rust idioms and provides sensible defaults (empty strings and
Localsource type). This aligns with the pattern used elsewhere in the codebase.apps/desktop/src/components/tasks/update-check.ts (1)
1-16: LGTM!The update check task hook is well-implemented using the tinytick task system. The 30-second interval is reasonable for background update checks, and the hook properly registers the async task with scheduled execution.
apps/desktop/src/components/tasks/calendar-sync/types.ts (1)
1-6: LGTM!The
ChunkArgtype clearly models the state needed for chunked calendar synchronization. Field names are descriptive and the structure supports tracking progress through both chunks and calendars.plugins/apple-calendar/src/contact_resolver.rs (5)
9-20: LGTM!The function provides a clean fallback strategy: attempt to fetch full contact details, then parse email from URL if contact fetch fails. The return type appropriately handles both cases.
101-144: LGTM!The helper functions properly handle optional values and empty strings. The use of
unsafeblocks is necessary for Objective-C interop, and the logic correctly navigates the nested CNContact structures.
146-153: LGTM!Simple and effective mailto: URL parser with appropriate None handling.
155-168: LGTM!Proper exception handling using
catchwithAssertUnwindSafe. The function safely extracts the participant schedule status and handles errors by returningNone.
170-184: LGTM!Comprehensive mapping of schedule status codes with appropriate handling of unknown values.
Cargo.toml (1)
243-243: LGTM!The
objc2-core-graphicsdependency addition is consistent with the existing objc2 ecosystem (version 0.3) and supports the Apple Calendar color handling features.apps/desktop/src/main.tsx (1)
20-20: LGTM!Simple import path update that aligns with the new task-based architecture. The TaskManager usage remains unchanged at line 111.
plugins/apple-calendar/src/ext.rs (1)
37-37: LGTM! Idiomatic use of Default trait.The refactor from
Handle::new()toHandle::default()is an idiomatic Rust pattern improvement. This aligns with the Default implementation inplugins/apple-calendar/src/apple.rs.Also applies to: 43-43
apps/desktop/src/store/tinybase/main.ts (1)
444-455: LGTM! Calendar query and index definitions are well-structured.The new
visibleCalendarsquery andcalendarsBySourceindex are consistent with existing patterns and align with the extended calendar schema fields (tracking_id,source,provider,enabled).Also applies to: 548-553, 685-685, 698-698
plugins/apple-calendar/src/lib.rs (1)
6-8: LGTM! Module organization is appropriate.The conditional inclusion of
contact_resolverandrecurrencemodules for macOS is properly structured.plugins/apple-calendar/Cargo.toml (1)
36-37: LGTM! Core Graphics dependency properly configured.The addition of
objc2-core-graphicsand its integration withobjc2-event-kitfeatures is correctly implemented.apps/desktop/src/components/settings/calendar/configure/index.tsx (1)
1-29: LGTM! Clean component structure with proper platform filtering.The
ConfigureProviderscomponent correctly filters providers by platform and conditionally renders the appropriate card components. The implementation follows React best practices.apps/desktop/src/components/settings/calendar/configure/cloud.tsx (1)
9-38: LGTM! Follows coding guidelines for props and className handling.The component correctly uses inline props typing and the
cnutility for className composition, adhering to the coding guidelines.packages/store/src/schema-external.ts (1)
47-50: LGTM! Schema extensions are consistent and type-safe.The calendar schema updates correctly reflect the database schema changes, maintaining type safety through the
InferTinyBaseSchemaconstraint.Also applies to: 265-269
plugins/apple-calendar/src/error.rs (1)
11-26: LGTM!The new error variants are well-structured and follow thiserror conventions. The
#[from]attributes forIoErrorandSerdeErrorenable ergonomic error propagation via the?operator. The variants cover the expected failure modes for calendar operations.apps/desktop/src/components/main/sidebar/timeline/index.tsx (1)
119-129: LGTM!The refresh button is conditionally rendered when calendars are enabled, with proper accessibility via
aria-label. The callback correctly triggers the calendar sync task.apps/desktop/src/components/settings/calendar/configure/shared.tsx (1)
6-22: LGTM!The type definitions are clean and appropriately exported since they're shared across components (used in
apple.tsx). Props interface is inlined as per coding guidelines.apps/desktop/src/components/tasks/calendar-sync/index.ts (2)
137-150: LGTM!The effect correctly listens for calendar changes and triggers sync. The Promise-based cleanup pattern is correct for async event listeners.
153-191: LGTM!The chunked scheduling logic correctly advances through calendars first, then chunks, with a small delay (100ms) to allow the UI to remain responsive.
apps/desktop/src/components/settings/calendar/configure/apple.tsx (2)
130-218: LGTM!The hook cleanly separates concerns: fetching Apple calendars, grouping by source, managing toggle state, and refresh. The store integration correctly creates new calendar rows or updates existing ones.
125-128: LGTM!The color conversion correctly handles Apple's normalized 0-1 color values to CSS rgba format with proper rounding.
apps/desktop/src/components/tasks/calendar-sync/utils.ts (1)
19-37: Potential duplicate fetch at chunk boundaries (inclusive range APIs).
to: nowand next chunkfrom: nowcan double-include events exactly atnowif the underlying query treats endpoints as inclusive. If you’ve seen duplicates, consider making the intervals half-open (e.g., add 1ms to the nextfrom, or subtract 1ms from the previousto).
| function getEventUniqueId(event: AppleEvent): string { | ||
| // For recurring events, event_identifier is shared across all occurrences. | ||
| // occurrence_date distinguishes specific occurrences. | ||
| // Fall back to start_date which is always unique per occurrence. | ||
| const occurrenceKey = event.occurrence_date ?? event.start_date; | ||
| return `${event.event_identifier}:${occurrenceKey}`; | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
Avoid event row ID collisions when event_identifier is missing/empty.
In the Rust plugin, event_identifier can become "" (unwrap_or_default()), so getEventUniqueId() can collide across events and cause overwrites. Consider falling back to calendar_item_identifier (or another always-present identifier) when event_identifier is falsy.
function getEventUniqueId(event: AppleEvent): string {
// For recurring events, event_identifier is shared across all occurrences.
// occurrence_date distinguishes specific occurrences.
// Fall back to start_date which is always unique per occurrence.
const occurrenceKey = event.occurrence_date ?? event.start_date;
- return `${event.event_identifier}:${occurrenceKey}`;
+ const baseId =
+ event.event_identifier && event.event_identifier.length > 0
+ ? event.event_identifier
+ : event.calendar_item_identifier;
+ return `${baseId}:${occurrenceKey}`;
}Also applies to: 49-60
🤖 Prompt for AI Agents
In apps/desktop/src/components/tasks/calendar-sync/utils.ts around lines 11 to
17 (and similarly update the logic at lines 49 to 60), getEventUniqueId
currently uses event.event_identifier which can be an empty string from the Rust
plugin and cause ID collisions; change the construction to treat a falsy/empty
event_identifier as missing and fall back to event.calendar_item_identifier (or
another always-present identifier) before combining with
occurrence_date/start_date, i.e., pick event.event_identifier if non-empty else
event.calendar_item_identifier, then append the occurrence key so each
occurrence remains unique.
| export function storeEvents( | ||
| store: StoreSetRow, | ||
| events: AppleEvent[], | ||
| calendarMap: Record<string, string>, | ||
| userId: string, | ||
| ) { | ||
| for (const event of events) { | ||
| const calendarId = calendarMap[event.calendar.id]; | ||
| if (!calendarId) continue; | ||
|
|
||
| store.setRow("events", getEventUniqueId(event), { | ||
| user_id: userId, | ||
| created_at: event.creation_date ?? new Date().toISOString(), | ||
| calendar_id: calendarId, | ||
| title: event.title, | ||
| started_at: event.start_date, | ||
| ended_at: event.end_date, | ||
| location: event.location ?? "", | ||
| meeting_link: event.url ?? "", | ||
| description: event.notes ?? "", | ||
| note: "", | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
created_at semantics may drift across re-syncs.
If event.creation_date is absent, created_at becomes “time of sync” and will change on subsequent writes (since setRow overwrites). If created_at is intended to represent record creation in your DB, consider using a dedicated synced_at field (or preserve existing created_at when updating).
| tracking_id: text("tracking_id").notNull(), | ||
| name: text("name").notNull(), | ||
| source: text("source").notNull(), | ||
| provider: text("provider").notNull(), | ||
| enabled: integer("enabled").notNull().default(1), |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
find . -type f \( -name "*.sql" -o -name "*migration*" \) | head -20Repository: fastrepl/hyprnote
Length of output: 992
🏁 Script executed:
fd -e sql | grep -i migrRepository: fastrepl/hyprnote
Length of output: 4337
🏁 Script executed:
rg -l 'tracking_id|source.*provider' -g '*.sql'Repository: fastrepl/hyprnote
Length of output: 143
🏁 Script executed:
cat -n crates/db-user/src/calendars_migration.sqlRepository: fastrepl/hyprnote
Length of output: 332
🏁 Script executed:
cat -n crates/db-user/src/calendars_migration_1.sqlRepository: fastrepl/hyprnote
Length of output: 138
🏁 Script executed:
cat -n packages/db/src/schema.ts | sed -n '200,220p'Repository: fastrepl/hyprnote
Length of output: 823
🏁 Script executed:
rg -n 'provider|enabled' crates/db-user/src/calendars_migration*.sqlRepository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
ls -la crates/db-user/src/calendars_migration*.sqlRepository: fastrepl/hyprnote
Length of output: 244
🏁 Script executed:
rg -n 'ALTER TABLE.*calendars' crates/db-user/src/Repository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
rg -n 'calendars' supabase/migrations/ | head -20Repository: fastrepl/hyprnote
Length of output: 43
🏁 Script executed:
rg -n 'provider|enabled' packages/db/src/schema.ts | grep -A2 -B2 'calendars\|provider\|enabled'Repository: fastrepl/hyprnote
Length of output: 300
Add missing migrations for provider, enabled columns and update source column constraint.
The schema defines tracking_id, source, provider, and enabled as notNull(), but migrations are incomplete:
providerandenabledcolumns are missing entirely from migration filessourcecolumn added incalendars_migration_1.sqlis nullable, conflicting with thenotNull()requirement in schema.ts
Create migrations to:
- Add
providerandenabledcolumns with appropriate defaults for existing rows - Alter
sourcecolumn to add NOT NULL constraint or update schema definition to match migration reality
🤖 Prompt for AI Agents
In packages/db/src/schema.ts around lines 207 to 211, the schema marks provider
and enabled as notNull but migrations never add those columns and the source
column was created nullable in calendards_migration_1.sql; add a new migration
that (1) alters the calendars table to add a provider column (text) with a
sensible default for existing rows (e.g., 'unknown' or the repo's chosen
default) and a NOT NULL constraint, (2) adds an enabled column (integer/boolean)
with default 1 and NOT NULL for existing rows, and (3) alter the source column
to set NOT NULL (or if you prefer to keep it nullable, update schema.ts to allow
nullable source); ensure the migration uses safe ALTER TABLE statements and
backfills values before adding NOT NULL constraints and update migration
versioning and README/changelog as appropriate.
| fn transform_event(event: &EKEvent) -> Result<AppleEvent, Error> { | ||
| let identifiers = extract_event_identifiers(event); | ||
| let calendar_ref = extract_event_calendar_ref(event); | ||
| let basic_info = extract_event_basic_info(event); | ||
| let dates = extract_event_dates(event); | ||
| let status_info = extract_event_status_info(event); | ||
| let flags = extract_event_flags(event); | ||
| let participants = extract_event_participants(event); | ||
| let location_info = extract_event_location_info(event); | ||
| let recurrence_info = extract_event_recurrence_info(event, flags.has_recurrence_rules); | ||
| let alarm_info = extract_event_alarm_info(event); | ||
| let birthday_info = extract_event_birthday_info(event, &calendar_ref); | ||
|
|
||
| Ok(AppleEvent { | ||
| event_identifier: identifiers.event_identifier, | ||
| calendar_item_identifier: identifiers.calendar_item_identifier, | ||
| external_identifier: identifiers.external_identifier, | ||
| calendar: calendar_ref, | ||
| title: basic_info.title, | ||
| location: basic_info.location, | ||
| url: basic_info.url, | ||
| notes: basic_info.notes, | ||
| creation_date: basic_info.creation_date, | ||
| last_modified_date: basic_info.last_modified_date, | ||
| time_zone: basic_info.time_zone, | ||
| start_date: dates.start_date, | ||
| end_date: dates.end_date, | ||
| is_all_day: dates.is_all_day, | ||
| availability: status_info.availability, | ||
| status: status_info.status, | ||
| has_alarms: flags.has_alarms, | ||
| has_attendees: flags.has_attendees, | ||
| has_notes: flags.has_notes, | ||
| has_recurrence_rules: flags.has_recurrence_rules, | ||
| organizer: participants.organizer, | ||
| attendees: participants.attendees, | ||
| structured_location: location_info.structured_location, | ||
| recurrence: recurrence_info.recurrence, | ||
| occurrence_date: recurrence_info.occurrence_date, | ||
| is_detached: recurrence_info.is_detached, | ||
| alarms: alarm_info.alarms, | ||
| birthday_contact_identifier: birthday_info.birthday_contact_identifier, | ||
| is_birthday: birthday_info.is_birthday, | ||
| }) | ||
| } |
There was a problem hiding this comment.
Avoid panics: unwrap() in event extraction should be error-propagating.
extract_event_calendar_ref and extract_event_birthday_info can panic on unexpected nil calendars. Since transform_event already returns Result, prefer threading the EKCalendar (or at least CalendarRef) from list_events into the transform path instead of re-fetching + unwrapping.
Minimal direction:
- Change
extract_event_calendar_ref(calendar: &EKCalendar) -> CalendarRef - In
list_events, you already havelet calendar = unsafe { event.calendar() }?;— pass&calendardown.
Also applies to: 291-297, 448-461
🤖 Prompt for AI Agents
In plugins/apple-calendar/src/apple.rs around lines 227-271 (and similarly at
291-297 and 448-461), transform_event currently calls extract_event_calendar_ref
and extract_event_birthday_info which can panic by unwrapping nil calendars;
change extract_event_calendar_ref to accept a &EKCalendar (e.g.,
extract_event_calendar_ref(calendar: &EKCalendar) -> Result<CalendarRef, Error>
or CalendarRef without unsafe unwrapping), update extract_event_birthday_info to
take the calendar reference instead of fetching/unwraping internally, and modify
list_events to pass the already-obtained calendar (you already have let calendar
= unsafe { event.calendar() }?;) into transform_event so errors are propagated
through Result rather than panicking; remove any unwraps and propagate or map
errors appropriately in all referenced lines.
| // Helper function for getting URL string from Objective-C objects | ||
| fn get_url_string<T>(obj: &T, selector: &str) -> Option<String> | ||
| where | ||
| T: objc2::Message + ?Sized, | ||
| { | ||
| unsafe { | ||
| let sel = objc2::sel!(selector); | ||
| let url_obj: Option<Retained<NSURL>> = msg_send![obj, sel]; | ||
| url_obj.and_then(|u| u.absoluteString().map(|s| s.to_string())) | ||
| } | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n plugins/apple-calendar/src/apple.rs | sed -n '1,50p'Repository: fastrepl/hyprnote
Length of output: 2148
🏁 Script executed:
cat -n plugins/apple-calendar/src/apple.rs | sed -n '737,747p'Repository: fastrepl/hyprnote
Length of output: 510
🏁 Script executed:
cat -n plugins/apple-calendar/src/apple.rs | sed -n '309,327p'Repository: fastrepl/hyprnote
Length of output: 1023
🏁 Script executed:
cat -n plugins/apple-calendar/src/apple.rs | sed -n '472,475p'Repository: fastrepl/hyprnote
Length of output: 266
🏁 Script executed:
cat plugins/apple-calendar/Cargo.toml | grep -A 5 "objc2"Repository: fastrepl/hyprnote
Length of output: 657
🌐 Web query:
objc2 sel! macro runtime string selector
💡 Result:
-
sel! is a Rust macro from the objc2 crate that produces a static Objective‑C selector (type objc2::runtime::Sel) from a method-name token or string-like token list (e.g. sel!(alloc), sel!(setObject:forKey:), sel!(storyboardWithName:bundle:)). It ignores whitespace and supports internal/multiple colons. [1][2]
-
Use sel! when the selector is known at compile time for efficient, static registration. For dynamic cases you can call Sel::register(&CStr) to register a selector at runtime. Sel values are interned (cheap equality) and provide Sel::name() to get the &'static CStr name. [1][2]
-
Implementation/details/examples and edge cases (whitespace, invalid selectors) are documented in the objc2 macros/source docs. [2][3]
Sources:
[1] objc2::runtime::Sel documentation (docs.rs).
[2] objc2 macro docs / sel! examples and rules (docs.rs source).
🏁 Script executed:
# Check if there's any objc2 sel!() usage documentation or examples in comments
rg "sel!\(" plugins/apple-calendar/src/ -A 2 -B 2Repository: fastrepl/hyprnote
Length of output: 436
🏁 Script executed:
# Look for other msg_send usage patterns to understand the macro usage
rg "msg_send!\[" plugins/apple-calendar/src/apple.rs | head -20Repository: fastrepl/hyprnote
Length of output: 1517
🏁 Script executed:
rg "get_url_string" plugins/apple-calendar/src/Repository: fastrepl/hyprnote
Length of output: 239
🌐 Web query:
objc2 Sel::register runtime selector CStr
💡 Result:
Short answer: use a &CStr (from std::ffi::CStr or the cstr_literal crate) and call Sel::register; but prefer the sel! / sel! macro for static selectors. Sel::register will register the selector with the ObjC runtime and returns a Sel (panics on allocation failure).
Example:
use std::ffi::CStr;
use objc2::runtime::Sel;
let name = CStr::from_bytes_with_nul(b"doSomething:\0").unwrap();
let sel = Sel::register(name);
Or, for a static selector (preferred):
let sel = sel!(doSomething:);
References: Sel::register documentation and runtime module (objc2). [1][2]
Sources
[1] Sel::register (objc2 docs). [2] sel! macro / runtime docs (objc2).
🏁 Script executed:
# Verify the actual code structure around the function
cat -n plugins/apple-calendar/src/apple.rs | sed -n '730,750p'Repository: fastrepl/hyprnote
Length of output: 669
🏁 Script executed:
# Check if there are any compilation scripts or test runs that would catch this
ls -la plugins/apple-calendar/ | grep -E "(Makefile|build|test)"Repository: fastrepl/hyprnote
Length of output: 119
objc2::sel!(selector) won't compile: sel!() requires compile-time selectors, not runtime &str.
Line 743 attempts let sel = objc2::sel!(selector); where selector is a runtime parameter. The sel!() macro only accepts compile-time identifiers. Since this function is called only once with the literal "URL" (line 313), the simplest fix is to inline and use a compile-time selector:
- url: get_url_string(event, "URL"),
+ url: unsafe {
+ let url_obj: Option<Retained<NSURL>> = msg_send![event, URL];
+ url_obj.and_then(|u| u.absoluteString().map(|s| s.to_string()))
+ },If dynamic selectors are genuinely needed, use Sel::register() instead:
let sel = objc2::runtime::Sel::register(std::ffi::CStr::from_bytes_with_nul(b"URL\0").unwrap());🤖 Prompt for AI Agents
In plugins/apple-calendar/src/apple.rs around lines 737-747, the code calls
objc2::sel!(selector) with a runtime &str which fails because sel! only accepts
compile-time selectors; since this helper is only called with the literal "URL"
at its call site, change the function to use a compile-time selector (replace
the runtime parameter and call objc2::sel!(URL) directly inside), update the
call site at line ~313 to match the new signature, or if you truly need dynamic
selectors, accept or construct an objc2::runtime::Sel (e.g., Sel::register from
a CStr) and use that instead of sel!.
No description provided.