Conversation
Ongoing notifications can now set lock-screen visibility with a public version for the private copy, an accent color, a notification category, a timeout that retires a stale notification, local-only delivery, and a group with a sort key. A single update can alert again with `alert`, and a timestamp or chronometer can be kept for ordering without being shown with `showWhen` and `chronometerCountDown`. Lifetime fields are options: they are stored with the record, survive every update, and are cleared by name with an explicit null. The lock-screen copy is payload, so an update replaces it like any other text. ADR 0008 records that split and the fields this issue refused. Client-side validation lives in one module keyed by every option, so an option cannot be added to the public type without saying how it is validated, and an unresolvable color rejects the promise instead of escaping the TurboModule method.
16b3f29 to
f57f274
Compare
Adversarial reviewVerdict. On the primary question — API-level safety across 24…36 — this holds up: I walked every platform call this PR adds or touches against its true Blocking
const updateOptions = { ...optionsRef.current, ...options }
lastUpdateOptionsRef.current = updateOptionsvoid update(lastUpdateOptionsRef.current)Call Minimal fix: strip the per-post key before caching, e.g. in Correctness1.
Requirement 13 says the upsert update branch must "merge with the three states and honour Fix: have 2. The new wrapper is the right shape and correctly converts the 3. Low-end Robolectric coverage is thin enough that the 24/25 claims are untested, not just unproven. Never executed at 24 or 25:
Fix: add Nits
Checked and looks correct
Generated by Claude Code |
…cation hook replays
The hook cached every update option for the autoUpdate effect, including
`alert`, so one call to `update({ alert: true })` made every later content
change alert again. `alert` describes the single post it arrives with and is
never stored natively, so it is dropped before caching. A react-test-renderer
test drives the real hook and pins that the replayed update carries no alert
while the other options still replay.
The native upsert update branch already merges the three states and honours `alert`, and the Kotlin test drove it directly, but the JS upsert entry point took start-shaped options and refused both, so no public caller could reach that behavior and a push could not clear what an earlier push stored. Upsert options are now the start shape plus what only an update can use: clearable presentation options and `alert`, validated the way an update validates them. The example background task forwards them from a push, the testing screen drives both toggles through upsert, and the docs no longer promise that a remote upsert cannot clear or re-alert.
…ication is unavailable resolveOngoingNotificationMutation only mapped IllegalArgumentException, so the IllegalStateException thrown for requestPromotedOngoing combined with fallbackBehavior 'error' still escaped the TurboModule method with a generic error. Map it to its own VOLTRA_NOTIFICATION_UNAVAILABLE rejection, so every refusal an ongoing notification call can hit arrives as a coded promise rejection.
…24 and 25 The public-version path is the only place a second Notification.Builder is built and it takes the deprecated Builder(Context) branch on old releases; setChronometerCountDown sits exactly on minSdk; and visibility, color, localOnly, group and sortKey were never read back below API 26. Each now has a pinned @config run, including API 25, which no test used to touch.
…ests type-invalid The validators reject these values at runtime, but the test file itself did not type-check: the rejected literals contradicted the public option types, twelve errors in total. Route them through one small helper that says out loud what they are - values the types forbid, handed to the guard on purpose - the same way the dynamic-widget tests cast their malformed specs.
…er text field The content's own text, subText and bigText fields accept an empty string - it is how a caller blanks a line - but publicVersion.text alone rejected it, even though the type allows it and the native side handles it. A public version still needs a usable title; its text now follows the same rule as its siblings.
… too The ADR places publicVersion under 'it is text', but buildPublicVersion also mirrors when, showWhen and the chronometer with its count-down onto the lock-screen copy. The behavior is deliberate - a timestamp reveals no content, and dropping the timer the real notification started would be stranger than showing it - but it is a deviation from the rule, so it now says so where the rule lives.
… ticker as refused The rejected-fields test checked every refusal except the channel-owned alert fields and the ticker. Sound and vibration live on fields the modern SDK stops naming, so the assertions read them the way the runtime keeps them; lights and defaults are read as delivered by the platform builder, ticker as its plain field. Now the test covers the full refused list from the ADR.
57729ca to
764942e
Compare
|
All review items are addressed, one commit each: Blocking — sticky Correctness 1 — Correctness 2 — Correctness 3 — low-end behavior was argued, not proven — Nit 1 — Nit 2 — Nit 3 — public version mirroring the chronometer was undocumented — Nit 4 — rejected-fields test missed the alert-family fields — Also: the lockfile in the first commit had been produced by a mismatched pnpm (9.7.0 vs the pinned 11.5.0), which dragged recently-published transitive entries into the lockfile and tripped the Validation: JS lint/format/typecheck green; android-client jest 28/28; android node tests 24/24; Robolectric 33/33 incl. the new 24/25 matrix; strict Android lint clean; changeset status clean. The two ios-client jest failures seen locally are an artifact of this worktree sitting inside another checkout (an undeclared @V3RON — PTAL. |
📱 On-device validation (emulator, API 35)Ran the PR end-to-end on a bootful Android 15 (API 35) emulator using the example app's ongoing-notification testing screen ( All checks passed:
Notes (not PR issues):
Combined with CI green on 764942e (jest 28/28, Robolectric 33/33 incl. API 24/25 matrix, node tests 24/24, strict Kotlin lint clean), the PR is validated both in CI and on a real emulator. |
…-options Resolves the conflicts with the Live Updates work (#325) and the Metric layout (#326), which landed overlapping pieces of this PR: - A countdown is expressed with #325's `chronometer: 'countDown'`; the separate `chronometerCountDown` prop is dropped. The payload key is unchanged, so the Kotlin side and remote payloads are unaffected, and `showWhen` layers on top of #325's timestamp handling. - Errors go through #325's VoltraNotificationException path. An unresolvable `color` now rejects with VOLTRA_NOTIFICATION_INVALID_OPTIONS; the promoted-unavailable case is #325's VOLTRA_NOTIFICATION_NOT_PROMOTABLE. - `showWhen` and `publicVersion` move into normalizeCommonDisplayFields and the Metric payload, so every kind carries them. - This PR's manager tests move to VoltraNotificationManagerPresentationTest, since #325 took VoltraNotificationManagerTest for its per-API suite. - The field-placement ADR is renumbered to 0010: main took 0008 and #332 holds 0009.
# Conflicts: # docs/adr/README.md
…notification-presentation-options # Conflicts: # example/screens/testing-grounds/AndroidOngoingNotificationTestingScreen.tsx # packages/android-client/android/src/main/java/voltra/VoltraNotificationManager.kt # packages/android/test/ongoing-notification.test.js
This branch has not been deployed
1 participant
What is this?
Android ongoing notifications can now be presented the way the platform allows. An app that shows a
ride, a delivery or a workout used to be able to choose only the channel, the small icon, the deep
link and whether to ask for promoted presentation; everything else about how the notification behaves
in the shade and on the lock screen was fixed by Voltra.
Starting, updating and upserting an ongoing notification now also accept
visibility,color,category,timeoutMs,localOnly,group,sortKeyandallowSystemGeneratedContextualActions. The content accepts apublicVersionlock-screen copy, andshowWhennext to the existingwhenandchronometer(a countdown is #325'schronometer: 'countDown'). An update canalso
alertonce, instead of every update after the first being silent by construction.Closes #317.
How does it work?
Every field went through one test, recorded in ADR 0010: if a server that renders payloads but knows
nothing about the app's policy could reasonably send it on every update, it is payload; if losing it
on an update would be a bug, it is an option.
publicVersionis the pair that shows the splitworking — the lock-screen line is text and travels with the rest of the text, while
visibilityisthe app's privacy policy and is stored with the notification, so the copy can change per update
without the visibility ever having to be re-sent.
Stored options need to tell three things apart, so
AndroidOngoingNotificationOptionis a three-casesealed type read from the bridge with
hasKeyandisNull, and merged into the record withmergedWith. The pre-existing?:merge could not express clearing, which is why the presentationoptions merge on the record's own type;
smallIconand its neighbours keep their old two-statebehaviour, and the docs say so instead of implying the whole options object behaves the same way.
alertis neither merged nor stored: it describes the post it arrives with.Validation is split by what each layer can know. JavaScript checks shape before any native call, for
local calls and for the
optionsobject of a push alike, in a module holding one validator per key ofthe options type — a new option cannot be added to the public type without saying how it is validated,
which is the trap the issue found, where a new option had to be added in four places and was silently
dropped when it was not. Native checks the one thing JavaScript cannot: whether a color string is a
static color rather than a dynamic theme token. That rejects the promise rather than throwing out of
the TurboModule method, which also fixes the pre-existing
channelIdcase that had the same shape. Anunknown category or visibility is ignored with a warning, because the alternative is a push written
for a newer release failing on the device that has to show it today.
Coverage is a Robolectric suite that reads back the notification from the
NotificationManagershadow at API 24, 26, 28, 29, 31 and 35 — nothing is mocked, so what is asserted is what
Notification.Builderproduced — plus renderer tests for the new payload keys and their validation,and tests for the option validator and the three-state bridge shaping. The example's ongoing
notification screen now drives every new option, and the push envelope it generates carries them, so
the same checks can be run through a real remote push.
What still needs a device to prove. Robolectric cannot show what a real system does with these
fields, and its pinned version cannot reach SDK 36, so the following is what an E2E pass has to
confirm, mostly on the testing-grounds screen:
visibility: 'private'and apublicVersion, the secured lock screen shows only the publiccopy, and the private content appears once unlocked;
'secret'hides the notification there.colorandcategoryvisibly reach the heads-up and shade presentation.timeoutMsreally removes a notification that stops being updated, whether the delete intent fireswhen the system does it (unlike a swipe), and that
startthen needsstopbefore that id works.localOnly: truestops the notification reaching a Wear companion or Android Auto, and does notwhen left off.
groupandsortKeybundle and order the notifications in the shade.alert: truemakes one update audibly alert on a high-importance channel, and the next one issilent again.
showWhen: falsehides an age that would never change while keeping the timestamp for ordering, andchronometer: 'countDown'counts down.getAndroidOngoingNotificationStatus().hasPromotableCharacteristicsis stilltrue with the new fields set, and promotion still happens. This is the one that would be a real
regression rather than a missing feature.
optionscarry the new keys reaches the device through the registeredbackground task, since the example handler now forwards them.
Why is this useful?
Apps can build the notification they mean instead of the one Voltra picked: private content with a
copy of the app's own choosing on the lock screen, an accent color that matches the brand, a category
that places it with the right kind of notification, and a timeout so a ride that stopped being
updated does not sit in the shade claiming to be in progress forever. Re-alerting on one update is
what "your driver is waiting outside" needs from an ongoing notification that is otherwise silent.
The layering is the other half of the value. Updates cannot quietly drop an app's privacy settings, a
server can vary lock-screen copy per update without learning anything about the app, and adding the
next option costs one validator entry in the client and one read and one merge line natively rather
than four edits and a silent drop. What was refused is written down with its reason, in the docs and
in ADR 0010, so the next person does not have to relitigate why there is no badge count.
Merge with main (after #325 and #326)
#325 landed overlapping pieces of this PR, so the merge commit reconciles them:
chronometer: 'countDown', and this PR's separatechronometerCountDownprop is gone. The payload key is the same, so remote payloads are unaffected.VoltraNotificationException. An unresolvablecolorrejects withVOLTRA_NOTIFICATION_INVALID_OPTIONS. The promoted-unavailable case is feat(android): Android 16 Live Updates polish for ongoing notifications #325'sVOLTRA_NOTIFICATION_NOT_PROMOTABLE, soVOLTRA_INVALID_NOTIFICATION_OPTIONSandVOLTRA_NOTIFICATION_UNAVAILABLEno longer exist.showWhenandpublicVersionapply to the Metric layout too.