ReviewOS

also looking at this

craft-native/craft

fix(ios): schedule a notification with the data the page gave it

#262
Merged glennmichael123 wants to merge fix/ios-notification-data into main
8 files +242 -22
9 days ago

Closes #258.

The SDK's NotificationOptions.data never made it into a scheduled notification. Neither iOS scheduler read it, so content.userInfo stayed empty. A tap on the notification (#255), or its arrival while the app was open (#256), then handed the page {}, leaving nothing to route on. That's the "Try it →" case: a reminder that should open a specific screen opened nothing in particular.

Change

  • Swift (scheduleLocalNotification): if let info = data["data"] as? [String: Any] { content.userInfo = info }.
  • Zig (bridge_mobile_notifications.zig): parseSchedule keeps an object data as JSON text on the Schedule, so parsing stays pure and unit-testable. buildRequest reads it back through NSJSONSerialization into the same Foundation types WebKit hands the Swift side, then calls setUserInfo:. If that JSON can't be read back, scheduling fails with an error rather than silently delivering {} again. JSON the page sent always reads, so in practice this shouldn't happen.

Only an object counts, matching the Swift side's as? [String: Any]. A string, array or null data leaves userInfo unset on both sides.

Tests

  • Zig unit test: parsing keeps nested data intact and ignores anything that isn't an object. It fails when the field isn't carried (error.DataDropped).
  • index.test.ts: the generated Swift sets userInfo on the content before the request is built from it.
  • End to end: a third stage of the notification UI test. A link has the page schedule a local notification with data: { craftE2E, stage: 'local', nested: { kept: true } } eight seconds out. The test kills the app before it fires, taps the banner, and the judge requires that data back once, nested values included. The probe now enables local notifications, so the shim leg exercises Swift's scheduler and the runtime leg exercises Zig's. Both pass locally.

Each fix was removed on its own and the whole harness rerun:

Removedshim legruntime leg
Swift's content.userInfo = infofails: a tap on a scheduled notification handed the page {}: the data it was scheduled with never reached userInfopasses
Zig's setUserInfo: callpassesfails with the same message

Each leg fails only on its own side, which also proves the runtime leg's scheduleNotification goes through Zig rather than being handed back to Swift.

protocol.test.ts gains 5 judge cases, for 72 in total.

Filed while here: #261. notifications.schedule({ scheduleAt }) fires immediately, because neither scheduler reads scheduleAt. They read the bridge's timestamp and delay instead.

8 changed files on the files tab, with 0 review threads.

Sign in to comment on this pull request.