Refactor command handling, enhance tests, and improve discovery logic (#4878)

Signed-off-by: James Rich <2199651+jamesarich@users.noreply.github.com>

feature/node/src/androidMain/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreen.kt

@@ -229,34 +229,6 @@ private fun NodeDetailBottomSheet(onDismiss: () -> Unit, content: @Composable ()
     ModalBottomSheet(onDismissRequest = onDismiss, sheetState = sheetState) { content() }
 }
 
-private fun handleNodeAction(
-    action: NodeDetailAction,
-    uiState: NodeDetailUiState,
-    navigateToMessages: (String) -> Unit,
-    onNavigateUp: () -> Unit,
-    onNavigate: (Route) -> Unit,
-    viewModel: NodeDetailViewModel,
-) {
-    when (action) {
-        is NodeDetailAction.Navigate -> onNavigate(action.route)
-        is NodeDetailAction.TriggerServiceAction -> viewModel.onServiceAction(action.action)
-        is NodeDetailAction.HandleNodeMenuAction -> {
-            when (val menuAction = action.action) {
-                is NodeMenuAction.DirectMessage -> {
-                    val route = viewModel.getDirectMessageRoute(menuAction.node, uiState.ourNode)
-                    navigateToMessages(route)
-                }
-                is NodeMenuAction.Remove -> {
-                    viewModel.handleNodeMenuAction(menuAction)
-                    onNavigateUp()
-                }
-                else -> viewModel.handleNodeMenuAction(menuAction)
-            }
-        }
-        else -> {}
-    }
-}
-
 @Preview(showBackground = true)
 @Composable
 private fun NodeDetailListPreview(@PreviewParameter(NodePreviewParameterProvider::class) node: Node) {

feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/HandleNodeAction.kt

@@ -0,0 +1,55 @@
+/*
+ * Copyright (c) 2025-2026 Meshtastic LLC
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program.  If not, see <https://www.gnu.org/licenses/>.
+ */
+package org.meshtastic.feature.node.detail
+
+import org.meshtastic.core.navigation.Route
+import org.meshtastic.feature.node.component.NodeMenuAction
+import org.meshtastic.feature.node.model.NodeDetailAction
+
+/**
+ * Shared handler for [NodeDetailAction]s that are common across all platforms.
+ *
+ * Platform-specific actions (e.g. [NodeDetailAction.ShareContact], [NodeDetailAction.OpenCompass]) are ignored by this
+ * handler and should be handled by the platform-specific caller.
+ */
+internal fun handleNodeAction(
+    action: NodeDetailAction,
+    uiState: NodeDetailUiState,
+    navigateToMessages: (String) -> Unit,
+    onNavigateUp: () -> Unit,
+    onNavigate: (Route) -> Unit,
+    viewModel: NodeDetailViewModel,
+) {
+    when (action) {
+        is NodeDetailAction.Navigate -> onNavigate(action.route)
+        is NodeDetailAction.TriggerServiceAction -> viewModel.onServiceAction(action.action)
+        is NodeDetailAction.HandleNodeMenuAction -> {
+            when (val menuAction = action.action) {
+                is NodeMenuAction.DirectMessage -> {
+                    val route = viewModel.getDirectMessageRoute(menuAction.node, uiState.ourNode)
+                    navigateToMessages(route)
+                }
+                is NodeMenuAction.Remove -> {
+                    viewModel.handleNodeMenuAction(menuAction)
+                    onNavigateUp()
+                }
+                else -> viewModel.handleNodeMenuAction(menuAction)
+            }
+        }
+        else -> {}
+    }
+}

feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/detail/NodeManagementActionsTest.kt

@@ -23,14 +23,13 @@ import dev.mokkery.verify
 import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.test.StandardTestDispatcher
 import kotlinx.coroutines.test.TestScope
-import kotlinx.coroutines.test.runTest
-import org.junit.Test
 import org.meshtastic.core.model.Node
 import org.meshtastic.core.model.RadioController
 import org.meshtastic.core.repository.NodeRepository
 import org.meshtastic.core.repository.ServiceRepository
 import org.meshtastic.core.ui.util.AlertManager
 import org.meshtastic.proto.User
+import kotlin.test.Test
 
 @OptIn(ExperimentalCoroutinesApi::class)
 class NodeManagementActionsTest {
@@ -51,7 +50,7 @@ class NodeManagementActionsTest {
         )
 
     @Test
-    fun `requestRemoveNode shows confirmation alert`() {
+    fun requestRemoveNode_shows_confirmation_alert() {
         val node = Node(num = 123, user = User(long_name = "Test Node"))
 
         actions.requestRemoveNode(testScope, node)
@@ -70,11 +69,4 @@ class NodeManagementActionsTest {
             )
         }
     }
-
-    @Test
-    fun `requestFavoriteNode shows confirmation alert`() = runTest(testDispatcher) {
-        // This test might fail due to getString() not being mocked easily
-        // but let's see if we can at least get requestRemoveNode passing.
-        // Actually, if getString() fails, the coroutine will fail.
-    }
 }

feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/metrics/EnvironmentMetricsStateTest.kt

@@ -1,5 +1,5 @@
 /*
- * Copyright (c) 2026 Meshtastic LLC
+ * Copyright (c) 2025-2026 Meshtastic LLC
  *
  * This program is free software: you can redistribute it and/or modify
  * it under the terms of the GNU General Public License as published by
@@ -16,17 +16,17 @@
  */
 package org.meshtastic.feature.node.metrics
 
-import org.junit.Assert.assertEquals
-import org.junit.Assert.assertTrue
-import org.junit.Test
 import org.meshtastic.core.common.util.nowSeconds
 import org.meshtastic.proto.EnvironmentMetrics
 import org.meshtastic.proto.Telemetry
+import kotlin.test.Test
+import kotlin.test.assertEquals
+import kotlin.test.assertTrue
 
 class EnvironmentMetricsStateTest {
 
     @Test
-    fun `environmentMetricsForGraphing correctly calculates times`() {
+    fun environmentMetricsForGraphing_correctly_calculates_times() {
         val now = nowSeconds.toInt()
         val metrics =
             listOf(
@@ -42,7 +42,7 @@ class EnvironmentMetricsStateTest {
     }
 
     @Test
-    fun `environmentMetricsForGraphing handles valid zero temperatures`() {
+    fun environmentMetricsForGraphing_handles_valid_zero_temperatures() {
         val now = nowSeconds.toInt()
         val metrics = listOf(Telemetry(time = now, environment_metrics = EnvironmentMetrics(temperature = 0.0f)))
         val state = EnvironmentMetricsState(metrics)

feature/node/src/jvmMain/kotlin/org/meshtastic/feature/node/detail/NodeDetailScreens.kt

@@ -23,8 +23,6 @@ import androidx.compose.ui.Modifier
 import androidx.lifecycle.compose.collectAsStateWithLifecycle
 import org.meshtastic.core.navigation.Route
 import org.meshtastic.feature.node.compass.CompassViewModel
-import org.meshtastic.feature.node.component.NodeMenuAction
-import org.meshtastic.feature.node.model.NodeDetailAction
 
 @Composable
 actual fun NodeDetailScreen(
@@ -45,24 +43,14 @@ actual fun NodeDetailScreen(
         uiState = uiState,
         modifier = modifier,
         onAction = { action ->
-            when (action) {
-                is NodeDetailAction.Navigate -> onNavigate(action.route)
-                is NodeDetailAction.TriggerServiceAction -> viewModel.onServiceAction(action.action)
-                is NodeDetailAction.HandleNodeMenuAction -> {
-                    when (val menuAction = action.action) {
-                        is NodeMenuAction.DirectMessage -> {
-                            val route = viewModel.getDirectMessageRoute(menuAction.node, uiState.ourNode)
-                            navigateToMessages(route)
-                        }
-                        is NodeMenuAction.Remove -> {
-                            viewModel.handleNodeMenuAction(menuAction)
-                            onNavigateUp()
-                        }
-                        else -> viewModel.handleNodeMenuAction(menuAction)
-                    }
-                }
-                else -> {}
-            }
+            handleNodeAction(
+                action = action,
+                uiState = uiState,
+                navigateToMessages = navigateToMessages,
+                onNavigateUp = onNavigateUp,
+                onNavigate = onNavigate,
+                viewModel = viewModel,
+            )
         },
         onFirmwareSelect = { /* No-op on desktop for now */ },
         onSaveNotes = { num, notes -> viewModel.setNodeNotes(num, notes) },

feature/node/src/jvmMain/kotlin/org/meshtastic/feature/node/metrics/PositionLogScreens.kt

@@ -16,21 +16,10 @@
  */
 package org.meshtastic.feature.node.metrics
 
-import androidx.compose.foundation.layout.Box
-import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.material3.MaterialTheme
-import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.Alignment
-import androidx.compose.ui.Modifier
+import org.meshtastic.core.ui.component.PlaceholderScreen
 
 @Composable
 actual fun PositionLogScreen(viewModel: MetricsViewModel, onNavigateUp: () -> Unit) {
-    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
-        Text(
-            text = "Position Log",
-            style = MaterialTheme.typography.headlineMedium,
-            color = MaterialTheme.colorScheme.onSurfaceVariant,
-        )
-    }
+    PlaceholderScreen(name = "Position Log")
 }

feature/node/src/jvmMain/kotlin/org/meshtastic/feature/node/navigation/TracerouteMapScreens.kt

@@ -16,27 +16,11 @@
  */
 package org.meshtastic.feature.node.navigation
 
-import androidx.compose.foundation.layout.Box
-import androidx.compose.foundation.layout.fillMaxSize
-import androidx.compose.material3.MaterialTheme
-import androidx.compose.material3.Text
 import androidx.compose.runtime.Composable
-import androidx.compose.ui.Alignment
-import androidx.compose.ui.Modifier
+import org.meshtastic.core.ui.component.PlaceholderScreen
 
 @Composable
 actual fun TracerouteMapScreen(destNum: Int, requestId: Int, logUuid: String?, onNavigateUp: () -> Unit) {
     // Desktop placeholder for now
     PlaceholderScreen(name = "Traceroute Map")
 }
-
-@Composable
-internal fun PlaceholderScreen(name: String) {
-    Box(modifier = Modifier.fillMaxSize(), contentAlignment = Alignment.Center) {
-        Text(
-            text = name,
-            style = MaterialTheme.typography.headlineMedium,
-            color = MaterialTheme.colorScheme.onSurfaceVariant,
-        )
-    }
-}