Repository navigation
fix(push): "rest over" rings only the device that rested - #348
Merged
Merged
Conversation
With push switched on in two browsers, a rest timer started on one could ring on both, and stay that way. sendPush aims a rest alert at the rows carrying the resting device's id and falls back to every subscription of the account when none does. Three paths left a row without the id: - The service worker's pushsubscriptionchange handler re-registers a rotated subscription without a deviceId: it has no localStorage to read gym_device from. - A subscription stored before device ids existed never gains one: the boot sync asks GET /api/push/status, sees the endpoint stored, and writes nothing. - POST /api/push/subscribe rebuilt the row from the request alone, so a re-send with no usable id dropped the one the row had. The page now leaves its device id in a cache of its own (opengym-device, under /opengym-device-id) whenever this browser holds a subscription, and the worker sends it with the rotated one. The activate sweep keeps that cache. With no Cache API the worker sends the subscription alone, as before. GET /api/push/status names the deviceId a stored row carries, null for none, and the boot sync sends the same subscription again when that is not this browser's id; the upsert keeps the row's created date. An api without the field (web and api ship separately) gets no re-send, nor does a browser with no id to give. deviceId() replaces a stored id the server would not keep (the deviceIdOf shape), which would otherwise be re-sent on every boot. Subscribe keeps the id the row already has when the request carries none. The spec and website/api.html say so. Tests: server-push-status.test.js 5 (3 fail on the old server.js), push.test.js 22 and sw.test.js 11 (13 of those 33 fail on the old push.js and sw.js). Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
DuarteSantos8
added a commit
that referenced
this pull request
Oct 7, 2026
When the device that started a rest had no subscription of its own, the alert fell back to every subscription of the account, so the phone rang for a rest timed on the desktop. It now goes to that device's subscriptions and to rows from older clients that carry no device id, nobody else.
Owner
|
Merged and released in v1.3.10, thanks @kurktchiev! https://github.com/DuarteSantos8/openGym/releases/tag/v1.3.10 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
With push notifications on in two browsers, a rest timer started on one can ring on both, and keep doing it.
sendPushaims a rest alert at the subscriptions stored with the resting device's id and falls back to every subscription of the account when none has it. Three paths leave a row without the id:pushsubscriptionchangehandler (sw.js ~189) re-registers a rotated subscription with nodeviceId. A worker cannot read localStorage, wheregym_devicelives.GET /api/push/status, sees the endpoint stored, and writes nothing.POST /api/push/subscriberebuilds the row from the request alone, so a re-send without a usable id drops the one the row had.The fixes, in the same order:
opengym-device, key/opengym-device-id) whenever this browser holds a subscription: on each signed-in boot's sync and when the switch is turned on. The worker reads it and sends it with the rotated subscription. The activate sweep of old build caches leaves that cache alone. Without a Cache API (private mode) the worker sends the subscription alone, as before. Handing the new subscription to an open page would mostly reach nobody, becausepushsubscriptionchangeusually fires with no page open. IndexedDB would do the same job with a lot more code on both sides, and a query string on the worker's URL would reinstall it and put the id in access logs.GET /api/push/statusalso returns thedeviceIda stored row carries,nullfor none. When that is not this browser's id, the boot sync sends the same subscription again, and the server's upsert stores it with the same endpoint, keys andcreated. An API that does not return the field gets no re-send, because web and api ship as separate images and it would otherwise mean a db.json write on every boot. A browser with no id to give (storage refused) skips it too.deviceId()now replaces a stored id the server would not keep (notdeviceIdOf's 8–64 of[A-Za-z0-9_-]); before, such an id went out as it was, the row stayed without one, and the new re-send would repeat on every boot.openapi.yaml describes the new status field and the subscribe rule, and website/api.html is regenerated from it. Running the generator again leaves it unchanged.
Tests:
deviceId: nulland is repaired by the owner's re-send; a re-send with"bad!", an object or no id leaves the row's id alone; a rest started on a device whose row has no id reaches both subscriptions, and only its own once repaired. 3 of the 5 fail on the old server.js.No open PR touches push.js, sw.js or the push routes. #312, #326 and #167 edit other parts of api/openapi.yaml and merge cleanly with this.
🤖 Generated with Claude Code