forked from trent_larson/crowd-funder-for-time-pwa
e121db5fcf
- buildDualScheduleConfig: set contentFetch timeout/retryAttempts/retryDelay (match capacitor DailyNotification networkConfig), userNotification.vibration, return type DualScheduleConfiguration - @timesafari/daily-notification-plugin 2.1.1 → 2.1.3 (package-lock) - doc: plugin feedback (contentFetch JSON, parseUserNotificationConfig optional fields) and Android DailyNotificationWorker duplicate scheduleId note
100 lines
5.5 KiB
Markdown
100 lines
5.5 KiB
Markdown
# Plugin feedback: Android `parseUserNotificationConfig` — optional fields vs `getBoolean` / `getString`
|
|
|
|
**Date:** 2026-03-20 21:11 PST
|
|
**Target repo:** `@timesafari/daily-notification-plugin` (daily-notification-plugin)
|
|
**Consuming app:** crowd-funder-for-time-pwa (TimeSafari)
|
|
**Platform:** Android (Kotlin)
|
|
**Related:** Same class of issue as [plugin-feedback-android-scheduleDualNotification-contentFetch-json.md](./plugin-feedback-android-scheduleDualNotification-contentFetch-json.md) (`contentFetch` / `parseContentFetchConfig`).
|
|
|
|
---
|
|
|
|
## Summary
|
|
|
|
`DailyNotificationPlugin.parseUserNotificationConfig()` uses **`JSObject` / `JSONObject` strict getters** for fields that the published TypeScript **`UserNotificationConfig`** marks as **optional** (`sound?`, `vibration?`, `priority?`, `title?`, `body?`). If a key is omitted, Android throws **`JSONException`** (e.g. *No value for vibration*), and `scheduleDualNotification` fails before scheduling.
|
|
|
|
**Recommended direction (plugin):** Align Kotlin parsing with `dist/esm/definitions.d.ts` by using **optional reads + defaults**, consistent with the fix already applied for `parseContentFetchConfig` (e.g. `optIntOrNull`, or Capacitor/JSON equivalents for booleans and strings).
|
|
|
|
**Recommended direction (app / already done in TimeSafari):** Send explicit `sound`, `vibration`, and `priority` (and title/body) in `buildDualScheduleConfig()` so **older plugin builds** that still use strict getters continue to work.
|
|
|
|
**Does it make sense to change both sides?** **Yes** — same reasoning as for `contentFetch`: the plugin should match its public contract; the app can stay explicit for compatibility and clarity.
|
|
|
|
---
|
|
|
|
## Symptoms (consuming app)
|
|
|
|
- In-app toast: *“Could not schedule New Activity notification. Please try again.”* (generic catch after `scheduleDualNotification` rejects.)
|
|
- Logcat:
|
|
|
|
```text
|
|
E DNP-PLUGIN: Schedule dual notification error
|
|
E DNP-PLUGIN: org.json.JSONException: No value for vibration
|
|
E DNP-PLUGIN: at org.json.JSONObject.getBoolean(JSONObject.java:419)
|
|
E DNP-PLUGIN: at org.timesafari.dailynotification.DailyNotificationPlugin.parseUserNotificationConfig(DailyNotificationPlugin.kt:2428)
|
|
E DNP-PLUGIN: at org.timesafari.dailynotification.DailyNotificationPlugin.scheduleDualNotification(DailyNotificationPlugin.kt:1392)
|
|
```
|
|
|
|
(First failure observed after `contentFetch` timeouts were fixed was **`vibration`**; the same pattern can affect **`sound`** or **`priority`** if those keys are omitted.)
|
|
|
|
---
|
|
|
|
## Root cause
|
|
|
|
### Published TypeScript contract (`UserNotificationConfig`)
|
|
|
|
From `definitions.d.ts` (representative):
|
|
|
|
- `title?`, `body?`, `sound?`, `vibration?`, `priority?` — all optional.
|
|
|
|
### Current Android implementation (strict)
|
|
|
|
In `DailyNotificationPlugin.kt`, `parseUserNotificationConfig` (line numbers approximate; search for `parseUserNotificationConfig`):
|
|
|
|
```kotlin
|
|
private fun parseUserNotificationConfig(configJson: JSObject): UserNotificationConfig {
|
|
return UserNotificationConfig(
|
|
enabled = configJson.getBoolean("enabled") ?: true,
|
|
schedule = configJson.getString("schedule") ?: "0 9 * * *",
|
|
title = configJson.getString("title"),
|
|
body = configJson.getString("body"),
|
|
sound = configJson.getBoolean("sound"),
|
|
vibration = configJson.getBoolean("vibration"),
|
|
priority = configJson.getString("priority")
|
|
)
|
|
}
|
|
```
|
|
|
|
- **`getBoolean("vibration")`** (and **`getBoolean("sound")`**) throw if the key is **missing** — optional in TS, required at runtime on Android.
|
|
- **`getString("title")`**, **`getString("body")`**, **`getString("priority")`** likewise throw if missing (depending on `JSObject` / `JSONObject` behavior for absent keys).
|
|
|
|
So minimal or TS-faithful payloads omit `vibration` → immediate `JSONException`.
|
|
|
|
---
|
|
|
|
## Plugin-side recommendations
|
|
|
|
1. **Treat `UserNotificationConfig` optional fields as optional on Android**, mirroring `definitions.d.ts`:
|
|
- **`vibration`:** e.g. `optBoolean` / nullable + default **`true`** (or `false` if that matches product default — document the default).
|
|
- **`sound`:** same pattern; default **`true`** is typical for notifications.
|
|
- **`priority`:** optional string with default **`"normal"`** (or map from TS union).
|
|
- **`title` / `body`:** if TS allows omission, use optional reads + defaults consistent with dual-schedule UX (or reject with a clear `call.reject` message instead of a raw `JSONException`).
|
|
|
|
2. **Reuse the same helper style** as `parseContentFetchConfig` after the timeout fix (`optIntOrNull`, etc.) so one codebase convention applies to all dual-schedule JSON parsing.
|
|
|
|
3. **Tests:** Unit or integration test that calls `scheduleDualNotification` with a **minimal** `userNotification` object (only what TS strictly requires, if anything) and asserts scheduling succeeds on Android.
|
|
|
|
4. **iOS parity:** If iOS already accepts omitted `vibration` / `sound`, Android should match; if not, align both platforms to the same `UserNotificationConfig` rules.
|
|
|
|
---
|
|
|
|
## App-side note (TimeSafari)
|
|
|
|
`src/services/notifications/dualScheduleConfig.ts` — `buildDualScheduleConfig()` now includes **`vibration: true`** (with `sound: true`) so current native code paths succeed. Keeping this explicit is still recommended even after the plugin is fixed.
|
|
|
|
---
|
|
|
|
## References
|
|
|
|
- Plugin: `android/.../DailyNotificationPlugin.kt` — `parseUserNotificationConfig`
|
|
- TS: `dist/esm/definitions.d.ts` — `UserNotificationConfig`, `DualScheduleConfiguration`
|
|
- App: `src/services/notifications/dualScheduleConfig.ts` — `buildDualScheduleConfig`
|