maybe_start_location_tracking/2 matched %{track_location: true} in the
track_settings argument. No caller passes that key -- all five call sites
send %{map_id: _, track: true | false} -- so the clause never matched and
the function could not turn location tracking on. maybe_start_ship_tracking/2
had the identical defect.
Match on the state's active_maps instead: a character on at least one map
should be polling their location.
Without this, a character is stranded by an event they never initiated. When
a browser's presence lapses past the grace period the character is untracked
server-side, active_maps empties, and maybe_stop_tracking/2 clears
track_location. The character is still online in EVE throughout, and the only
other writer -- update_online/1 -- is gated on an online-status transition
that a character who stays logged in never produces. So the browser
reconnecting cannot restore it. update_location/1 then falls through to its
catch-all on every tick, and the character stops moving on the map while
every liveness signal keeps reporting healthy.
This is why #620 reports that toggling tracking off and on does not help but
relogging in EVE does: toggling routes through the dead clause, while
relogging produces the online transition that restores the flag as a side
effect.
Fixes#620
- correct the Jita/Hek comment to match the systems the tests actually use
- document the sandbox contract on IntegrationCase and make shared mode an
explicit opt-in that is reverted on exit
- drop the blanket rescue around Sandbox.allow/4: it returns
{:already, :owner | :allowed} rather than raising, so the rescue only
masked genuine infrastructure faults
- use Cache.insert/3 and document the :online option
Review of the previous commit found that Ash returns NotFound wrapped
inside Ash.Error.Invalid, not as a bare struct. Verified against a live
query: `Api.Map.by_id(<missing uuid>)` returns
{:error, %Ash.Error.Invalid{errors: [%Ash.Error.Query.NotFound{}]}}
so the bare-NotFound clause never matched. A genuinely missing map would
have propagated as a generic error and logged a spurious error line.
Match the wrapped shape as well. Confirmed `MapRepo.get/2` now returns
{:error, :not_found} for a missing map with no error log. No caller
depends on the :not_found atom specifically (all use wildcard or
with/else), so propagation of real faults is unaffected.
Also make track_character_on_map/2 clear stale per-map location caches,
mirroring TrackingUtils.track_character/4, which the helper claimed to
mirror but did not.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Four independent causes, each confirmed by instrumentation rather than
inference.
1. Sandbox access. MapPool GenServers are spawned dynamically and load
map state from the DB during init (six Task.async loaders in
do_init_state/1). IntegrationCase's one-shot supervision-tree grant
runs before any pool exists, and its polling monitor granted only
mock ownership, never Ecto sandbox access -- and would lose the race
regardless. Every loader died with DBConnection.OwnershipError.
Fixed with an opt-in @moduletag :shared_sandbox. Shared mode is the
only mechanism covering a process that queries immediately upon
spawn. It is opt-in rather than a global flip because Ecto shared
mode is node-global, and it is now reverted to :manual on exit --
not reverting is why an earlier global-flip attempt broke
CommonAPIControllerTest.
2. Characters were never tracked. update_characters/1 iterates
get_tracked_character_ids/1, which needs the character in
map.characters AND a tracking_start_time cache key. The tests set
only presence_character_ids, which that path never reads, so the
tracked list was empty and the whole body was a no-op. Added
track_character_on_map/2.
3. Characters were offline. With online: false the :character_online
handler deletes the solar_system_id cache key that
check_location_update just wrote, so no movement was ever observed.
4. Map scopes. get_effective_scopes/1 prefers the scopes array over
scope, and scopes defaults to [:wormholes]. The tests set only
scope: :all, so hi-sec movement was rejected as an invalid
connection.
Separately, these tests asserted that Jita is added to the map. Jita is
hardcoded in @prohibited_systems and can never be added, so those
assertions could never pass. Rather than weaken them, the start system
is now Hek (not prohibited), which preserves each test's intent, and a
new regression test pins the Jita prohibition itself.
Verified by set-diff against merge-base b7ddbc48 at full-suite scope
(two runs each, counts are unstable so sets were compared): zero new
failures, all 9 target failures fixed. Suite is 15 tests, 0 failures
across seeds 0, 1 and 42, with no OwnershipError and no map-start
timeout remaining.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
LiveView event handlers were trusting client-supplied record IDs without
verifying they belong to the user's current map/user scope. A logged-in
user could push another map's record UUID over the LV socket and act on
it. Affected handlers:
- cancel-subscription / edit-subscription / update_subscription
(cancel or upgrade any map's paid subscription by ID)
- characters_live.ex "delete"
(soft-delete any user's character and null their user_id)
- cancel_ping
(cross-map rally-ping cancellation broadcast)
- deleteSystemComment
(cross-map comment deletion)
- update_passage_mass
(cross-map wormhole passage mass corruption)
- updateCharacterTracking
(untrack another user's character on a shared map)
Also fixes a related numeric-trust bug in subscribe/update_subscription
where `period`, `characters_limit`, and `hubs_limit` were parsed without
bounds, allowing `period: "-1"` to produce a back-dated subscription
with a negative price.
Adds WandererAppWeb.HandlerAuth with focused helpers each handler now
routes through, plus 26 regression tests covering both the IDOR auth
checks and the bounded-int parsers.
Guarzo