fix(webhooks): stop GPS ticks overwriting the saved Poracle location#1231
Open
unseenmagik wants to merge 2 commits into
Open
fix(webhooks): stop GPS ticks overwriting the saved Poracle location#1231unseenmagik wants to merge 2 commits into
unseenmagik wants to merge 2 commits into
Conversation
The notification panel listened for Leaflet `locationfound` events and persisted every one of them via the `setLocation` mutation. Because `leaflet.locatecontrol` runs geolocation in watch mode, an active locate control keeps emitting position fixes for as long as it is running, so simply opening the panel was enough to silently rewrite a location the user had deliberately set elsewhere. Users who track by coordinates + distance are hit hardest: Poracle stores one lat/lng per human, so a single background tick moves every distance-based tracking they have. Reported by a user who set alerts for a park, then had them follow him home the next time he edited a tracking. Gate the handler behind a ref that only the "My Location" button arms, and consume it on the first event so a continuing watch cannot write again. `_onClick` toggles the control, so arm from `_active` after the call -- clicking it while active stops locating and no event is coming. Also fix two adjacent issues found while verifying: - The coordinate comparisons required *both* lat and lng to differ, so moving a pin along a single axis never saved. - `human` is empty until the query resolves, so the init effect seeded the store with `[undefined, undefined]`, and its `every(x => x === 0)` guard then never re-fired once real coordinates arrived. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 2818b64969
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
A GPS fix can take many seconds on mobile. If the user clicked My Location and then picked a different location via search or Choose on Map while waiting, the ref stayed armed and the late fix overwrote the newer explicit pick -- reintroducing the silent overwrite through a race. Clear the pending request whenever a location is actually persisted, so the most recent deliberate choice wins. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Contributor
Author
|
resolved with (1f42fc6) |
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.
Problem
A user reported that their Poracle notification location kept changing to wherever they physically were, rather than staying at the location they had deliberately set:
They had set alerts for a park they play at, then edited a tracking from home — and started getting pings for spawns near their house instead.
This is real, and it isn't user error.
Location.jsxlistened for Leaflet'slocationfoundevent and persisted every one of them straight to Poracle via thesetLocationmutation, with no confirmation:leaflet.locatecontrolruns geolocation in watch mode, so once the locate control is active it keeps emitting position fixes for as long as it runs. That means merely opening the notification panel with locate engaged was enough to silently overwrite a saved location — the user sees their current coordinates in the panel, but by then the write has already happened.Users tracking by coordinates + distance are hit hardest. Poracle stores a single lat/lng per human (
POST /api/humans/:id/setLocation), not one per tracking or per profile, so a single stray GPS tick relocates every distance-based tracking they have at once.Fix
Gate the handler behind a ref that only the My Location button arms, and clear it on the first event so a continuing watch can't write a second time. Passive GPS ticks are now ignored; the location only changes when the user asks for it via My Location, the map pin, or the Nominatim search.
_onClicktoggles the control, so the flag is armed fromlc._activeafter the call — clicking it while already active stops locating, and arming beforehand would leave the flag set with no event coming, letting a later stray tick through.Also included
Two adjacent issues found while verifying:
&&should have been||in both coordinate comparisons. They required both latitude and longitude to differ, so moving a pin along a single axis silently didn't save.[undefined, undefined].humanis empty until the query resolves, and theevery(x => x === 0)guard then never re-fires once real coordinates arrive. Harmless under&&, but worth closing now that the save effect is more eager.useLocation's return typedef gains_active?: boolean. The property is real and already relied on inuseStopFollowingOnFly.js, so widening the typedef seemed cleaner than a cast — though it is the same coupling toleaflet.locatecontrolinternals that the existing_onClickusage already has.Testing
node_modules, so I could not run the linter or a build. Worth a manual pass before merging. Suggested check:Note
This doesn't change the underlying design: location remains one value per user, so distance-based trackings still all share it. This only stops it from moving on its own. Tracking by area is still the robust answer for "alert me about this specific place regardless of where I am."