fix(node): remove every unheard node from the banner's Remove

[jamesarich]

Remove on the unheard-nodes banner only removed one node. It asked for a confirmation per node, and AlertManager holds a single alert, so each request replaced the last and only the final node's dialog was left to confirm.

🐛 Bug Fixes

• The banner asks once for the whole list ("N nodes will be removed…") and removes every node on confirm.
• NodeManagementActions.requestRemoveNodes is the batch entry point; a single node still gets the existing one-node dialog.

Testing Performed

• NodeManagementActionsTest.requestRemoveNodes_removes_every_node_after_one_confirmation

Summary by CodeRabbit

• New Features
  • Added a single confirmation for removing multiple unheard nodes. Confirming removes all selected nodes; removing one node continues to use its existing confirmation.
• Bug Fixes
  • Ensured bulk removal leaves nodes outside the selection unchanged.

[coderabbitai]

📝 Walkthrough

Node removal now accepts lists of nodes. Multiple nodes use one pluralized confirmation, while a single node uses the existing removal action. The node-list view model and screen pass node lists to the management action.

Changes: Bulk node removal

Layer / File(s)	Summary
Plural confirmation and removal .skills/compose-ui/strings-index.txt, core/resources/src/commonMain/composeResources/values/strings.xml, feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/NodeManagementActions.kt, feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/detail/NodeManagementActionsTest.kt	The plural resource provides singular and plural messages with a formatted count. requestRemoveNodes skips empty lists, delegates a single node to requestRemoveNode, and displays one confirmation for multiple nodes. Confirming removes each requested node. A test verifies that unrelated nodes remain.
Node-list removal entry points feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/list/NodeListViewModel.kt, feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/list/NodeListScreen.kt	The view model now delegates node lists through removeNodes. The context menu passes one node, and the unheard-nodes banner passes the full list.

Priority: ➖ Normal

Merge Risk: 🔵 Low · up to 40cfd

If removal stops partway through, the remaining nodes may not be offered again until the screen is reopened. This is a bounded retry issue that should be fixed or accepted before merging.

Pre-merge checks: 6 · 2

Failed checks: 2 warnings

Check name	Status	Explanation	Resolution
Tests Prove The Path, Not The End State		The added test checks only the final FakeNodeRepository map. It seeds the fake at lines 114–115 and asserts nodeDBbyNum.value.keys at line 121. FakeRadioController.removeByNodenum is a no-op and…	Add a path-specific assertion. Use a mock or recording RadioController, stub packet IDs, confirm the single alert once, and verify removeByNodenum was called for node numbers 1, 2, and 3. Keep the exact surviving-node assertion if repos…
Regression Coverage For Changed Behavior		Coverage is incomplete. The added test covers the successful batch action at NodeManagementActions, but it does not cover the changed banner wiring in NodeListScreen.kt:350; reverting the banner t…	Add a regression test for the unheard-nodes banner with multiple nodes that invokes Remove once and confirms all nodes are requested. Add tests for requestRemoveNodes with one node and with an empty list. Verify the singleton route uses t…

Passed checks: 6 passed

Check name	Status	Explanation
Description Check		Check skipped - CodeRabbit’s high-level summary is enabled.
Title check		The title clearly describes the main change: removing every unheard node from the banner action. It is specific and related to the pull request.
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.
Sibling Call Sites And Presence Semantics		PASS: The pull request changes node-removal requests and string resources only. It does not make a field nullable, remove a zero-guard, add a presence check, or add a physical measurement field with a…
Moved Code Diffed Against Its Original		No moved type or function was introduced. NodeManagementActions.requestRemoveNode and removeNode remain in place with their original scope, defaults, exception handling, and mutation behavior. The…

Full details: Sibling Call Sites And Presence Semantics

Explanation

PASS: The pull request changes node-removal requests and string resources only. It does not make a field nullable, remove a zero-guard, add a presence check, or add a physical measurement field with a default of 0. Therefore, sibling call-site and presence semantics checks are not applicable.

Full details: Moved Code Diffed Against Its Original

Explanation

No moved type or function was introduced. NodeManagementActions.requestRemoveNode and removeNode remain in place with their original scope, defaults, exception handling, and mutation behavior. The only removed declaration is the NodeListViewModel.removeNode wrapper, replaced in the same file by removeNodes; both callers now use the new API. The context-menu caller passes a singleton, and the banner passes the full list. NodeDetailViewModel still uses the original single-node API and callback.

Full details: Tests Prove The Path, Not The End State

Explanation

The added test checks only the final FakeNodeRepository map. It seeds the fake at lines 114–115 and asserts nodeDBbyNum.value.keys at line 121. FakeRadioController.removeByNodenum is a no-op and records no removal request, so an implementation that deletes the nodes directly without issuing the intended radio removals would pass. The assertion does distinguish the surviving node from a size-only check, but it does not prove the required path.

Resolution

Add a path-specific assertion. Use a mock or recording RadioController, stub packet IDs, confirm the single alert once, and verify removeByNodenum was called for node numbers 1, 2, and 3. Keep the exact surviving-node assertion if repository deletion also requires coverage.

Full details: Regression Coverage For Changed Behavior

Explanation

Coverage is incomplete. The added test covers the successful batch action at NodeManagementActions, but it does not cover the changed banner wiring in NodeListScreen.kt:350; reverting the banner to per-node requests would still pass. It also does not cover the singleton and empty-list branches in NodeManagementActions.kt:74-76, the NodeListScreen.kt:275 context-menu route, each radio removal call, or the plural confirmation text.

Resolution

Add a regression test for the unheard-nodes banner with multiple nodes that invokes Remove once and confirms all nodes are requested. Add tests for requestRemoveNodes with one node and with an empty list. Verify the singleton route uses the existing confirmation. Verify the radio receives one removal request per node, and add a Compose alert test that checks the plural confirmation text.

• Fix all pre-merge checks with AI

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 1

---

ℹ️ Review info

⚙️ Run configuration

• Configuration used: Repository: meshtastic/Meshtastic-Android/.coderabbit.yaml
• Review profile: CHILL
• Plan: Advanced
• Run ID: 966a79fb-a820-4e54-90b3-4ad360f1ff7c

📥 Commits

Reviewing files that changed from the base of the PR and between 7f01ed0 and 40cfd8d.

📒 Files selected for processing: 6

• .skills/compose-ui/strings-index.txt
• core/resources/src/commonMain/composeResources/values/strings.xml
• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/detail/NodeManagementActions.kt
• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/list/NodeListScreen.kt
• feature/node/src/commonMain/kotlin/org/meshtastic/feature/node/list/NodeListViewModel.kt
• feature/node/src/commonTest/kotlin/org/meshtastic/feature/node/detail/NodeManagementActionsTest.kt

Included review availability: This review used your included allowance. Your plan provides up to 4 included reviews per hour; 3 remain after this review.