fix(routes): clearing observers in the edit modal now persists - #319
Merged
Conversation
The route edit modal builds its PUT body with a ternary that collapses
an empty observer list to null:
observer_public_keys: observerPublicKeys.length > 0 ? observerPublicKeys : null,
Pydantic parses null as None, and the PUT handler's guard
if body.observer_public_keys is not None:
_sync_observers(session, route, observer_nodes)
skips the sync entirely when the field is None. So removing all
observers in the modal sent null -> no DB change. Adding observers
worked because a non-empty array passed the guard and _sync_observers
deleted + recreated.
The None-means-skip semantic is correct for true partial updates, so
the fix is on the frontend: the edit modal is a full-form PUT, so it
must always send the array. When empty, it sends [], which Pydantic
parses as [] (not None), the guard passes, and _sync_observers deletes
every existing RouteObserver row.
Tests: add test_update_clear_observers_with_empty_list next to the
existing test_update_observers as a regression guard. It seeds one
observer via PUT, then PUTs observer_public_keys: [] and asserts both
the response and a fresh GET come back with an empty list.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
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.
What
Fixes a bug where removing all observers in the Route edit modal silently failed — the PUT returned 200, but the observers came back on next page load.
Root cause
src/meshcore_hub/web/static/js/spa/pages/routes.js:812built the PUT body with a ternary that collapsed an empty observer list tonull:On the backend, Pydantic parsed
nullasNone, and the PUT handler's guard atapi/routes/routes.py:589skipped the sync:So:
_sync_observersdeletes + recreates → worked.null→ guard failed →_sync_observersskipped → DB unchanged.The
None-means-skip semantic is correct for partial PATCH-style updates, so the bug is purely on the frontend: the route modal is a full-form PUT (every field is sent every time), so it shouldn't use the "send null when empty" pattern.Fix
One-line change in
routes.js— always send the array:When empty, JS serializes
[], Pydantic parses as[](notNone), the guard passes, and_sync_observersdeletes every existingRouteObserverrow.Regression test
tests/test_api/test_routes.py— addedtest_update_clear_observers_with_empty_listnext to the existingtest_update_observers. It:observer_public_keys: []→ asserts responseroute_observersis[].This locks the contract so a future client (frontend or otherwise) can't silently reintroduce the bug by sending
nullfor the empty case.Scope (explicitly unchanged)
RouteUpdate.observer_public_keys: Optional[list[str]] = None— theNone-means-skip semantic stays; it's correct for true partial updates from other clients._sync_observers(routes.py:158) — already does delete-all-then-recreate correctly.nulland[]are equivalent on create (both mean "all observers"), so the same line worked by accident when creating. No change needed.node_public_keys(line 811) — already sends the array unconditionally; no change.Tests
pre-commit run --all-files→ clean (black, flake8, mypy, hooks).Manual verification
Build the stack, then in the running app:
/routes, edit a route, add an observer, save. Confirm it appears.