chore(android): remove unreachable Saved Peers screen and other dead code - #1123
Merged
Conversation
The Saved Peers screen was never wired into navigation (no AppDestination entry, no nav-graph reference), leaving it dead code. Removing it cascades to the only other references to the favorites listing surface: - Delete SavedPeersScreen, SavedPeersViewModel, SavedPeersViewModelTest - AnnounceDao: drop getFavoriteAnnounces, searchFavoriteAnnounces, getFavoriteCount, getEnrichedFavoriteAnnounces, searchEnrichedFavoriteAnnounces (no remaining callers) - AnnounceRepository: drop getFavoriteAnnounces, searchFavoriteAnnounces, toggleFavorite, getFavoriteCount - PeerCard: drop the showFavoriteToggle param, only the deleted screen set it to false; star state is now simply announce.isFavorite - PeerCardTest: remove the saved-screen case and param references - AnnounceStreamViewModel: fix stale comment referencing toggleFavorite Kept: setFavorite/updateFavoriteStatus (Announce Stream star sync), isFavorite column + stale-cleanup protection, getAnnounceFlow.
Two more unreachable code clusters found while tracing the Saved Peers removal: - MainScreen + MainViewModel + UiState: the original 'Hello, Reticulum!' demo scaffold. MainActivity renders the NavHost and never references MainScreen; no AppDestination, nav-graph, or injection site exists. Remove the screen, its viewmodel, the UiState sealed class, and MainViewModelTest. - LocationSharingViewModel: extracted from MessagingViewModel for single responsibility but never wired into any screen (MessagingViewModel and MapViewModel handle location sharing inline via LocationSharingManager). Only its own test referenced it. Remove it and LocationSharingViewModelTest. Both removals orphan no shared code: LocationSharingManager, LocationSharingState, SharingDuration, ContactRepository, and RnsCore all remain in use by live screens and viewmodels.
Contributor
Greptile SummaryThe PR removes unreachable Android screens, unused view models, their orphaned favorite-peer data APIs, and associated tests while retaining the live contact and location-sharing paths.
Confidence Score: 5/5The PR appears safe to merge. No blocking failure remains. Important Files Changed
Reviews (3): Last reviewed commit: "test(contacts): make retryIdentityResolu..." | Re-trigger Greptile |
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Fix a pre-existing flake that failed CI on this PR's first run (Kotlin
Tests Shard 1/4) in NomadNetBrowserViewModelTest.'identifyToNode does
nothing when already identified'.
identifyToNode() dispatches its work onto the real Dispatchers.IO, but
this test only replaces Dispatchers.Main with a TestDispatcher, so
advanceUntilIdle() cannot observe the IO coroutine's completion. The
coVerify(exactly=1) then raced the IO thread; under CI load the mock
call was not yet recorded ('was not called').
Poll for the isIdentified flag the IO coroutine sets (bounded at 2s)
before asserting, instead of relying on advanceUntilIdle alone. The
production guard (_identifyInProgress) was already correct.
Verified: 10/10 consecutive runs green + detekt/ktlint clean.
Fix the second pre-existing flake hit on this PR (Kotlin Tests Shard 2/4):
ContactsViewModelTest.'retryIdentityResolution - resets contact with fresh
timestamp'.
retryIdentityResolution() launches on the real Dispatchers.IO, but the
test only replaces Dispatchers.Main, so the bare coVerify/verify raced
the IO thread ('was not called' under CI load). The two sibling tests in
the same group already use verify(timeout=1000) for exactly this reason;
apply the same pattern here.
Verified: 10/10 consecutive runs green + detekt/ktlint clean.
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
1 participant
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 this removes
Dead code found by tracing the 'Saved Peers' notification-settings bug. Two commits, each independently safe.
Commit 1: Unreachable Saved Peers screen + orphan cascade
The Saved Peers screen was never wired into navigation (no
AppDestinationentry, no nav-graph reference). It and its exclusive dependencies:SavedPeersScreen.kt,SavedPeersViewModel.kt+ test (deleted)AnnounceDao:getFavoriteAnnounces,searchFavoriteAnnounces,getFavoriteCount,getEnrichedFavoriteAnnounces,searchEnrichedFavoriteAnnouncesAnnounceRepository:getFavoriteAnnounces,searchFavoriteAnnounces,toggleFavorite,getFavoriteCountPeerCard.showFavoriteToggleparam - only the deleted screen set it false; star state is now simplyannounce.isFavoriteKept (still live):
setFavorite/updateFavoriteStatus(Announce Stream star writes both the contacts table and the announce flag in sync), theannounces.isFavoritecolumn + stale-announce cleanup protection,getAnnounceFlow.Commit 2: Dead MainScreen demo + unwired LocationSharingViewModel
MainScreen+MainViewModel+UiState: the original 'Hello, Reticulum!' demo scaffold. MainActivity renders the NavHost and never references it; no destination, nav entry, or injection site (all 38 registered destinations verified reachable).LocationSharingViewModel: extracted from MessagingViewModel in a refactor but never attached to any screen (full-historygit log -Sconfirms zero references in any commit). The live location-sharing feature is intact - it runs throughLocationSharingManagerdirectly from MessagingViewModel / MapViewModel / SettingsViewModel.Verification (both commits, on head ff77b8a)
Net: 13 files, +4 / -1891 lines. No behavioral change to any reachable feature.
Known non-issues
detekt-baseline-debug.xmlretains one stale ID for the deleted MainViewModel.kt. Inert (stale baseline entries are ignored); not hand-edited per generated-file convention.