fix(notifications): align dual schedule config with Android plugin + bump DNP

- 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
2026-03-20 21:13:50 +08:00
parent 1389a166fa
commit e121db5fcf
5 changed files with 330 additions and 22 deletions
--- a/doc/plugin-feedback-android-parseUserNotificationConfig-optional-fields.md
+++ b/doc/plugin-feedback-android-parseUserNotificationConfig-optional-fields.md
@@ -0,0 +1,99 @@
+# 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`
--- a/doc/plugin-feedback-android-scheduleDualNotification-contentFetch-json.md
+++ b/doc/plugin-feedback-android-scheduleDualNotification-contentFetch-json.md
@@ -0,0 +1,117 @@
+# Plugin feedback: Android `scheduleDualNotification` — `JSONException: No value for timeout`
+
+**Date:** 2026-03-20 18:21 PST  
+**Target repo:** `@timesafari/daily-notification-plugin` (daily-notification-plugin)  
+**Consuming app:** crowd-funder-for-time-pwa (TimeSafari)  
+**Platform:** Android (Kotlin)  
+**Plugin version observed:** 2.1.2 (from app `node_modules`)
+
+---
+
+## Summary
+
+Scheduling the **New Activity** dual notification on Android fails with a native `JSONException` because `DailyNotificationPlugin.parseContentFetchConfig()` uses **`JSONObject.getInt()`** for `timeout`, `retryAttempts`, and `retryDelay`. Those keys are **absent** from the app’s `contentFetch` object built by `buildDualScheduleConfig()`. The plugin’s own TypeScript `ContentFetchConfig` marks those fields as **optional**, so the Android parser is stricter than the published contract.
+
+**Recommended direction:**
+
+1. **Plugin (primary):** Parse optional numeric fields with defaults (e.g. `optInt` / nullable + defaults) so payloads that omit them do not crash and match `definitions.d.ts`.
+2. **App (secondary / compatibility):** Include explicit `timeout`, `retryAttempts`, and `retryDelay` on `contentFetch` so older plugin versions that still use `getInt` continue to work.
+
+**Does it make sense to change both sides?** **Yes.** Fixing the plugin aligns behavior with the documented API and protects any consumer that omits those fields. Fixing the app is still valuable for **older shipped plugin builds** and makes network behavior explicit. Together you get backward compatibility, clearer intent, and no silent reliance on undocumented defaults.
+
+---
+
+## Symptoms (consuming app)
+
+- In-app toast: *“Could not schedule New Activity notification. Please try again.”* (generic error path after `scheduleDualNotification` rejects.)
+- Logcat (filtered on DNP / plugin tags):
+
+```text
+E DNP-PLUGIN: Schedule dual notification error
+E DNP-PLUGIN: org.json.JSONException: No value for timeout
+E DNP-PLUGIN:   at org.json.JSONObject.getInt(JSONObject.java:487)
+E DNP-PLUGIN:   at org.timesafari.dailynotification.DailyNotificationPlugin.parseContentFetchConfig(DailyNotificationPlugin.kt:2403)
+E DNP-PLUGIN:   at org.timesafari.dailynotification.DailyNotificationPlugin.scheduleDualNotification(DailyNotificationPlugin.kt:1391)
+```
+
+---
+
+## Root cause
+
+### Call path
+
+`scheduleDualNotification` reads `config.contentFetch` and passes it to `parseContentFetchConfig`:
+
+- File: `android/.../DailyNotificationPlugin.kt`
+- `scheduleDualNotification` ~1391: `parseContentFetchConfig(contentFetchObj)`
+- `parseContentFetchConfig` ~2397–2411: uses `getInt` for three keys.
+
+### Strict Android parsing
+
+Illustrative (exact line numbers may shift between releases):
+
+```kotlin
+// parseContentFetchConfig — timeout / retry fields are required via getInt()
+timeout = configJson.getInt("timeout"),
+retryAttempts = configJson.getInt("retryAttempts"),
+retryDelay = configJson.getInt("retryDelay"),
+```
+
+`getInt` throws if the key is missing → first missing key in practice is `timeout` → `JSONException: No value for timeout`.
+
+### App payload today (consuming app)
+
+File: `src/services/notifications/dualScheduleConfig.ts` — `buildDualScheduleConfig()` sets `contentFetch` to:
+
+- `enabled`, `schedule`, `callbacks` only (no `timeout`, `retryAttempts`, `retryDelay`, no `url`).
+
+That matches the **TypeScript** contract in the plugin’s `dist/esm/definitions.d.ts`, where `timeout`, `retryAttempts`, and `retryDelay` are **optional** on `ContentFetchConfig`.
+
+### Contract mismatch
+
+| Layer | `timeout` / `retryAttempts` / `retryDelay` |
+|--------|--------------------------------------------|
+| TS `ContentFetchConfig` | Optional (`?`) |
+| Android `parseContentFetchConfig` | Required (`getInt` — throws if absent) |
+
+The consuming app followed the TS API; Android rejected it at runtime.
+
+---
+
+## Plugin-side recommendations
+
+1. **Use optional reads with defaults** for `timeout`, `retryAttempts`, and `retryDelay` (and any similar fields), e.g. Kotlin/Capacitor equivalents of `optInt` or `getInteger` with fallbacks documented in `ContentFetchConfig`.
+2. **Document defaults** in the plugin README or API docs if they are applied on native when omitted.
+3. **Consider tests** that call `scheduleDualNotification` with a minimal `contentFetch` (only `enabled`, `schedule`, `callbacks`) and assert scheduling succeeds on Android.
+4. **Optional:** If `url` is also read in a way that assumes presence, align with TS (`url?`) the same way.
+
+---
+
+## App-side recommendations (later; crowd-funder-for-time-pwa)
+
+When you implement the app fix:
+
+- Extend `contentFetch` in `buildDualScheduleConfig()` (`src/services/notifications/dualScheduleConfig.ts`) to include explicit integers, for example aligned with existing app/network conventions (the app’s `capacitor.config.ts` already uses a `timeout` value in one place — reuse or document chosen values).
+- Ensure **both** code paths that build dual config stay in sync (e.g. `AccountViewView.vue` uses `buildDualScheduleConfig` for New Activity scheduling and for `updateDualScheduleConfig` fallback).
+
+This unblocks users on **current** plugin versions that still require those keys.
+
+---
+
+## References (paths in consuming app workspace)
+
+- App config builder: `src/services/notifications/dualScheduleConfig.ts`
+- Native scheduling entry: `node_modules/@timesafari/daily-notification-plugin/android/.../DailyNotificationPlugin.kt` (`scheduleDualNotification`, `parseContentFetchConfig`)
+
+---
+
+## Answer: change both plugin and app?
+
+**Yes, it makes sense to change both**, for different reasons:
+
+| Side | Why |
+|------|-----|
+| **Plugin** | Fixes the real bug: native behavior must match the published optional TS fields; avoids breaking any client that sends a minimal `contentFetch`. |
+| **App** | Defense in depth and support for **already-shipped** plugin binaries that will not get the Kotlin fix until users update the app. Explicit values also document intended fetch/retry behavior in one place. |
+
+If you only fix the plugin, new app releases still need users to update the **native** binary. If you only fix the app, any other consumer of the plugin or future minimal payloads can hit the same crash until the plugin is fixed.
--- a/doc/plugin-fix-android-DailyNotificationWorker-duplicate-scheduleId.md
+++ b/doc/plugin-fix-android-DailyNotificationWorker-duplicate-scheduleId.md
@@ -0,0 +1,96 @@
+# Plugin fix: Android compile error — duplicate `scheduleId` in `handleDisplayNotification`
+
+**Date:** 2026-03-20  
+**Target repo:** `@timesafari/daily-notification-plugin` (daily-notification-plugin)  
+**Consuming app:** crowd-funder-for-time-pwa (TimeSafari)  
+**Platform:** Android (Java)
+
+---
+
+## Summary
+
+The Android module fails to compile with **two** `javac` errors: `variable scheduleId is already defined in method handleDisplayNotification(String)`. The method already declares `String scheduleId` at the start of the `try` block; two nested blocks incorrectly **redeclare** `String scheduleId`, which Java forbids in the same method scope. Remove the redundant declarations and reuse the existing variable (or assign without `String` if you ever need to refresh it).
+
+---
+
+## Problem
+
+- **File:** `android/src/main/java/org/timesafari/dailynotification/DailyNotificationWorker.java`
+- **Method:** `private Result handleDisplayNotification(String notificationId)`
+
+**Compiler output (representative):**
+
+```text
+DailyNotificationWorker.java:162: error: variable scheduleId is already defined in method handleDisplayNotification(String)
+                String scheduleId = inputData.getString("schedule_id");
+                       ^
+DailyNotificationWorker.java:193: error: variable scheduleId is already defined in method handleDisplayNotification(String)
+                String scheduleId = inputData.getString("schedule_id");
+                       ^
+```
+
+**Root cause:** At the top of the `try` block, the code already has:
+
+```java
+Data inputData = getInputData();
+String scheduleId = inputData.getString("schedule_id");
+```
+
+Later, inside:
+
+1. The `if (isStaticReminder) { ... }` branch — a line like `String scheduleId = inputData.getString("schedule_id");` (around line 162).
+2. The `else { ... }` branch — the same pattern (around line 193).
+
+In Java, a local variable name cannot be declared again in nested blocks that share the enclosing method’s scope for that name. These inner `String scheduleId` lines are **illegal** and break `:timesafari-daily-notification-plugin:compileDebugJavaWithJavac`.
+
+**Functional note:** Both inner reads use the same key (`"schedule_id"`) as the outer declaration, so they add **no** new information; the fix is to **delete** those inner declarations and keep using `scheduleId` from the first assignment.
+
+---
+
+## Required change
+
+**Option A (recommended):** Delete the two redundant lines entirely:
+
+- Remove the inner `String scheduleId = inputData.getString("schedule_id");` in the **static reminder** branch (post-reboot/rollover comment block).
+- Remove the inner `String scheduleId = inputData.getString("schedule_id");` in the **regular notification** branch (rollover/notify_* comment block).
+
+All subsequent uses of `scheduleId` in those branches should continue to refer to the variable declared immediately after `getInputData()`.
+
+**Option B (only if you must re-read input later):** Replace redeclaration with assignment:
+
+```java
+scheduleId = inputData.getString("schedule_id");
+```
+
+Do **not** prefix with `String` again inside the same method.
+
+---
+
+## Verification
+
+1. **Compile:** From the plugin repo, run the Android Java compile for the library (or assemble debug). Expect **zero** errors for `DailyNotificationWorker.java`.
+2. **Consuming app:** Bump/publish the plugin version, update `package.json` in TimeSafari, `npm install`, `npx cap sync android`, then run the usual Android debug build (e.g. `./scripts/build-android.sh --test` or `assembleDebug`). The task `:timesafari-daily-notification-plugin:compileDebugJavaWithJavac` must succeed.
+3. **Behavior:** No intended behavior change: `schedule_id` is still read once per worker run from `getInputData()` and used for dual-prefix checks, static reminder DB fallback, and canonical content by `schedule_id` in the non-static path.
+
+---
+
+## Context (how this was found)
+
+- Observed when running `npm run build:android:test:run` on crowd-funder-for-time-pwa; Vite/TypeScript succeeded; Gradle failed on the plugin’s Java sources under `node_modules/.../DailyNotificationWorker.java`.
+- Line numbers in published packages may drift slightly; search for `handleDisplayNotification` and duplicate `String scheduleId` inside that method.
+
+---
+
+## Cursor prompt (paste into plugin repo)
+
+You can paste the block below into Cursor in the **daily-notification-plugin** workspace:
+
+```text
+Fix Android compile errors in DailyNotificationWorker.java: in handleDisplayNotification(String notificationId), scheduleId is declared once after getInputData(). Remove the two illegal inner redeclarations "String scheduleId = inputData.getString(\"schedule_id\");" (static reminder branch and else branch). Reuse the outer scheduleId variable. Do not shadow or redeclare String scheduleId in the same method. Verify compileDebugJavaWithJavac passes.
+```
+
+---
+
+## After the fix
+
+Release a new plugin version and update the consuming app’s dependency so `node_modules` is not hand-edited (edits there are lost on `npm install`).
--- a/package-lock.json
+++ b/package-lock.json
@@ -8685,8 +8685,8 @@
      }
    },
    "node_modules/@timesafari/daily-notification-plugin": {
-      "version": "2.1.1",
-      "resolved": "git+https://gitea.anomalistdesign.com/trent_larson/daily-notification-plugin.git#539b011fa812571f39e4f26c66805b6d0a1eeaef",
+      "version": "2.1.3",
+      "resolved": "git+https://gitea.anomalistdesign.com/trent_larson/daily-notification-plugin.git#4dd1aea00224e4edc79a1fa3aa5767985a771c25",
      "license": "MIT",
      "workspaces": [
        "packages/*"
--- a/src/services/notifications/dualScheduleConfig.ts
+++ b/src/services/notifications/dualScheduleConfig.ts
@@ -3,6 +3,15 @@
 * Used for API-driven "New Activity" notifications (prefetch + notify).
 */

+import type { DualScheduleConfiguration } from "@timesafari/daily-notification-plugin";
+
+/** Matches `plugins.DailyNotification.networkConfig` in capacitor.config.ts */
+const CONTENT_FETCH_NETWORK = {
+  timeout: 30_000,
+  retryAttempts: 3,
+  retryDelay: 1_000,
+} as const;
+
 /**
 * Convert "HH:mm" (24h) to cron expression "minute hour * * *" (daily at that time).
 */
@@ -42,26 +51,9 @@ export interface DualScheduleConfigInput {
 * Build plugin DualScheduleConfiguration for scheduleDualNotification().
 * contentFetch runs 5 minutes before notifyTime; userNotification at notifyTime.
 */
-export function buildDualScheduleConfig(input: DualScheduleConfigInput): {
-  contentFetch: {
-    enabled: boolean;
-    schedule: string;
-    callbacks: Record<string, unknown>;
-  };
-  userNotification: {
-    enabled: boolean;
-    schedule: string;
-    title: string;
-    body: string;
-    sound: boolean;
-    priority: "high" | "normal" | "low";
-  };
-  relationship?: {
-    autoLink: boolean;
-    contentTimeout: number;
-    fallbackBehavior: "skip" | "show_default" | "retry";
-  };
-} {
+export function buildDualScheduleConfig(
+  input: DualScheduleConfigInput,
+): DualScheduleConfiguration {
  const notifyTime = input.notifyTime || "09:00";
  const fetchCron = timeToCronFiveMinutesBefore(notifyTime);
  const notifyCron = timeToCron(notifyTime);
@@ -69,6 +61,9 @@ export function buildDualScheduleConfig(input: DualScheduleConfigInput): {
    contentFetch: {
      enabled: true,
      schedule: fetchCron,
+      timeout: CONTENT_FETCH_NETWORK.timeout,
+      retryAttempts: CONTENT_FETCH_NETWORK.retryAttempts,
+      retryDelay: CONTENT_FETCH_NETWORK.retryDelay,
      callbacks: {},
    },
    userNotification: {
@@ -77,6 +72,7 @@ export function buildDualScheduleConfig(input: DualScheduleConfigInput): {
      title: input.title ?? "New Activity",
      body: input.body ?? "Check your starred projects and offers for updates.",
      sound: true,
+      vibration: true,
      priority: "normal",
    },
    relationship: {