Skip to content

Commit 06d1091

Browse files
jamesarichclaude
andcommitted
fix(geofence): gate foreign-geofence Edit on connection; test cleanup
Address CodeRabbit review on #6117: - WaypointInfoDialog's Edit button now requires `isConnected` in both map flavors — editing a foreign geofence re-broadcasts it, which needs a live connection, so the affordance is hidden when offline. - MeshDataHandlerTest uses FakeNotificationPrefs instead of a hand-stubbed mock. - Add a NotificationPrefs test for retoggle-refreshes-eviction-order on the capped opt-in set. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 28195a5 commit 06d1091

4 files changed

Lines changed: 26 additions & 9 deletions

File tree

androidApp/src/fdroid/kotlin/org/meshtastic/app/map/MapView.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -833,9 +833,10 @@ fun MapView(
833833
alertsEnabled = waypoint.id in optIns,
834834
onToggleAlerts = { mapViewModel.setGeofenceAlertOptIn(waypoint.id, it) },
835835
onDismissRequest = { showGeofenceInfoDialog = null },
836-
// Unlocked foreign geofences can still be edited/re-broadcast; locked ones stay read-only.
836+
// Unlocked foreign geofences can still be edited/re-broadcast (only while connected, since editing means
837+
// re-sending); locked ones stay read-only.
837838
onEdit =
838-
if (waypoint.locked_to == 0) {
839+
if (waypoint.locked_to == 0 && isConnected) {
839840
{
840841
showGeofenceInfoDialog = null
841842
showEditWaypointDialog = waypoint

androidApp/src/google/kotlin/org/meshtastic/app/map/MapView.kt

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -731,9 +731,10 @@ fun MapView(
731731
alertsEnabled = waypoint.id in optIns,
732732
onToggleAlerts = { mapViewModel.setGeofenceAlertOptIn(waypoint.id, it) },
733733
onDismissRequest = { geofenceInfoWaypoint = null },
734-
// Unlocked foreign geofences can still be edited/re-broadcast; locked ones stay read-only.
734+
// Unlocked foreign geofences can still be edited/re-broadcast (only while connected, since editing
735+
// means re-sending); locked ones stay read-only.
735736
onEdit =
736-
if (waypoint.locked_to == 0) {
737+
if (waypoint.locked_to == 0 && isConnected) {
737738
{
738739
geofenceInfoWaypoint = null
739740
editingWaypoint = waypoint

core/data/src/commonTest/kotlin/org/meshtastic/core/data/manager/MeshDataHandlerTest.kt

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ import org.meshtastic.core.repository.MessageFilter
4848
import org.meshtastic.core.repository.NeighborInfoHandler
4949
import org.meshtastic.core.repository.NodeManager
5050
import org.meshtastic.core.repository.NotificationManager
51-
import org.meshtastic.core.repository.NotificationPrefs
5251
import org.meshtastic.core.repository.PacketHandler
5352
import org.meshtastic.core.repository.PacketRepository
5453
import org.meshtastic.core.repository.PlatformAnalytics
@@ -57,6 +56,7 @@ import org.meshtastic.core.repository.ServiceRepository
5756
import org.meshtastic.core.repository.StoreForwardPacketHandler
5857
import org.meshtastic.core.repository.TelemetryPacketHandler
5958
import org.meshtastic.core.repository.TracerouteHandler
59+
import org.meshtastic.core.testing.FakeNotificationPrefs
6060
import org.meshtastic.proto.ChannelSet
6161
import org.meshtastic.proto.ChannelSettings
6262
import org.meshtastic.proto.Data
@@ -148,10 +148,7 @@ class MeshDataHandlerTest {
148148
nodeManager = nodeManager,
149149
serviceNotifications = serviceNotifications,
150150
crossingStore = GeofenceCrossingStore(),
151-
notificationPrefs =
152-
mock<NotificationPrefs>(MockMode.autofill).apply {
153-
every { geofenceAlertOptIns } returns MutableStateFlow<Set<Int>>(emptySet())
154-
},
151+
notificationPrefs = FakeNotificationPrefs(),
155152
scope = geofenceScope,
156153
),
157154
meshBeaconRepository = meshBeaconRepository,

core/prefs/src/commonTest/kotlin/org/meshtastic/core/prefs/notification/NotificationPrefsTest.kt

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -116,4 +116,22 @@ class NotificationPrefsTest {
116116
assertFalse(2 in ids)
117117
assertTrue(max + 2 in ids) // newest kept
118118
}
119+
120+
@Test
121+
fun `geofenceAlertOptIns retoggle refreshes eviction order`() = testScope.runTest {
122+
val max = NotificationPrefsImpl.MAX_GEOFENCE_OPT_INS
123+
(1..max).forEach { notificationPrefs.setGeofenceAlertOptIn(it, enabled = true) } // 1 = oldest
124+
125+
notificationPrefs.setGeofenceAlertOptIn(
126+
1,
127+
enabled = true,
128+
) // re-toggle → 1 becomes most-recent, 2 now oldest
129+
notificationPrefs.setGeofenceAlertOptIn(max + 1, enabled = true) // forces one eviction
130+
131+
val ids = notificationPrefs.geofenceAlertOptIns.value
132+
assertEquals(max, ids.size)
133+
assertTrue(1 in ids) // retoggled id survived
134+
assertFalse(2 in ids) // 2 became the oldest and was evicted
135+
assertTrue(max + 1 in ids)
136+
}
119137
}

0 commit comments

Comments
 (0)