docs(geofences): record why the delete is not feature-gated - #765
Merged
Conversation
The documentation audit reported that disable_user_geofences does not gate
DELETE custom/{id}, which is true: the attribute is on rename, create, submit,
activate, deactivate and import, and the service gates the same five. Delete is
covered by neither.
It should stay that way. Disabling the feature hides the page and refuses new
work, but fences that already exist keep being served in the feed and keep
matching, so a gated delete leaves someone receiving alerts from an area they
can neither edit nor remove.
The alarm types gate their whole controller and get away with it because the
bot is the escape hatch -- !untrack removes an alarm whatever the web says.
Geofences are PoracleWeb-only and none of the bot's 48 commands manages a drawn
polygon, so this endpoint is a user's only route to their own data. Production
holds 42 of them.
No behaviour change. The reason is now in the code, and a test pins the split:
delete ungated, the other six gated, and the class itself must not acquire the
attribute -- which would silently re-gate the delete and the reads with it.
Refs #730
Claude-Session: https://claude.ai/code/session_01Nah4N2sGFs1TU2t7DXfKzJ
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.
Follow-up to the finding in #763's verification pass:
disable_user_geofencesdoesn't gateDELETE custom/{id}.The finding is accurate. The attribute is on rename (line 39), create (71), submit (112), activate (138), deactivate (164) and import (222); the delete at line 89 has none, and
UserGeofenceService.DeleteAsynchas noEnsureEnabledAsynccall either. Neither layer stops it.No behaviour change, because I think the current behaviour is right.
Disabling the feature hides the page and refuses new work, but fences that already exist keep being served in the geofence feed and keep matching. Gate the delete as well and a user is left receiving alerts from an area they can't edit, can't remove, and can only get rid of by asking an admin. Production holds 42 user geofences, so switching the setting off would strand all of them at once.
The alarm controllers gate their whole class, deletes included, and get away with it for a reason that doesn't apply here: the bot is the escape hatch.
!untrackremoves an alarm whatever the web says. Geofences are PoracleWeb-only, and none of the bot's 48 commands manages a drawn polygon — so this endpoint is a user's only route to their own data.What this PR adds is the reason, in the code, plus a test that pins the split: the delete stays ungated, the other six stay gated, and the controller class must not acquire the attribute — which would silently re-gate the delete and the open reads with it.
If you'd rather deletes were refused when the feature is off, say so and it's a one-line change; the docs from #763 already describe the current behaviour accurately either way.
2004 backend tests.
https://claude.ai/code/session_01Nah4N2sGFs1TU2t7DXfKzJ