diff --git a/bitchat/App/AppRuntime.swift b/bitchat/App/AppRuntime.swift index d21d0507..15b8e88a 100644 --- a/bitchat/App/AppRuntime.swift +++ b/bitchat/App/AppRuntime.swift @@ -171,11 +171,27 @@ final class AppRuntime: ObservableObject { /// recent-chats section, so the last conversation is one tap away rather /// than lost. private func restoreLastActiveConversationOnLaunch() { - if conversations.restoreLastActiveConversation() == .conversationList { + if Self.shouldPresentConversationList(for: conversations.restoreLastActiveConversation()) { appChromeModel.showSidebar = true } } + /// The launch decision itself, split out as a pure function so the mapping + /// is testable without standing up an `AppRuntime`. It is one comparison, + /// but it is the comparison that decides whether a public timeline or a + /// list ends up on screen, and inverting it silently is exactly the kind of + /// mistake the store-level tests cannot see. + static func shouldPresentConversationList( + for presentation: ConversationStore.LaunchPresentation + ) -> Bool { + switch presentation { + case .conversationList: + return true + case .deferToChannelRestore: + return false + } + } + /// Drops media that has outlived the retention window, then applies the /// explicit protection class to files that older builds wrote without /// one. Expiry runs first so the migration never touches files the diff --git a/bitchatTests/AppArchitectureTests.swift b/bitchatTests/AppArchitectureTests.swift index 270ad76d..ca4fd621 100644 --- a/bitchatTests/AppArchitectureTests.swift +++ b/bitchatTests/AppArchitectureTests.swift @@ -506,6 +506,12 @@ struct AppArchitectureTests { // left set covers the root timeline exactly like the others. Launch // restore (#1064) can raise it without the person touching anything. chromeModel.showSidebar = true + // The OTHER arm of `isPeopleSheetPresented`. Asserting only the latch + // would leave the test green while a selected DM kept the sheet over + // the wipe result — the selection is cleared by pre-existing code in + // ChatViewModel.panicClearAllData, so this pins a cross-model + // invariant that neither half owns alone. + viewModel.startPrivateChat(with: PeerID(str: "1122334455667788")) chromeModel.panicClearAllData() @@ -514,6 +520,7 @@ struct AppArchitectureTests { #expect(!chromeModel.isNoticesSheetPresented) #expect(chromeModel.showingFingerprintFor == nil) #expect(!chromeModel.showSidebar) + #expect(viewModel.selectedPrivateChatPeer == nil) } @Test("Recent chats list direct conversations with people absent from the rosters") diff --git a/bitchatTests/ConversationStoreLastActiveTests.swift b/bitchatTests/ConversationStoreLastActiveTests.swift index b67bc992..d04d6908 100644 --- a/bitchatTests/ConversationStoreLastActiveTests.swift +++ b/bitchatTests/ConversationStoreLastActiveTests.swift @@ -135,9 +135,15 @@ final class ConversationStoreLastActiveTests: XCTestCase { // beginPanicWipe → setSelectedPrivatePeer(nil) → setActiveChannel(.mesh) → finishPanicWipe // Without the fix those resets re-persist a `.mesh` record, and the next // launch would `.deferToChannelRestore` instead of `.conversationList`. - // That is what makes this assertion load-bearing rather than trivially - // true: `.conversationList` is also the no-record answer, so the test - // distinguishes "pointer absent" from "pointer rewritten as .mesh". + // + // The presentation assertion alone is NOT enough, and the earlier + // version of this comment overclaimed. `.conversationList` is the + // answer for an absent record AND for a surviving `.direct` one, so a + // regression where `finishPanicWipe` stopped removing the key while + // suppression still worked would leave the peer id on disk and this + // test would stay green. For a panic wipe that is the whole point of + // the feature, so assert the stored key is gone, not just that launch + // looks right. let storage = makeStorage() let session = ConversationStore(storage: storage) @@ -149,14 +155,32 @@ final class ConversationStoreLastActiveTests: XCTestCase { session.setActiveChannel(.mesh) // re-persist trigger #2 (suppressed) session.finishPanicWipe() // removes the pointer once - // A fresh store on the SAME storage finds NO record and falls back to - // the conversation list — the pointer is truly absent, not `.mesh`. + // The pointer is truly absent, not rewritten as `.mesh` and not left + // behind as `.direct`. Read the key directly: this is the only + // assertion that separates "erased" from "still on disk", since both + // present as `.conversationList` at launch. + XCTAssertNil(storage.data(forKey: "conversation.lastActive")) + + // And launch presents the list rather than deferring to a resurrected + // `.mesh` record — the suppression half of the same fix. XCTAssertEqual( ConversationStore(storage: storage).restoreLastActiveConversation(), .conversationList ) } + // MARK: - Launch decision + + func test_launchPresentsTheListForAConversationListPresentation() { + XCTAssertTrue(AppRuntime.shouldPresentConversationList(for: .conversationList)) + } + + func test_launchLeavesAPublicChannelRestoreAlone() { + // Inverting this is how a geohash restore would end up buried under the + // people sheet, or a DM launch would land on the public timeline. + XCTAssertFalse(AppRuntime.shouldPresentConversationList(for: .deferToChannelRestore)) + } + func test_firstLaunchPresentsConversationList() { let storage = makeStorage()