diff --git a/Yattee/Core/AccountsManager.swift b/Yattee/Core/AccountsManager.swift index aa9783a2..c5cb0e57 100644 --- a/Yattee/Core/AccountsManager.swift +++ b/Yattee/Core/AccountsManager.swift @@ -353,6 +353,14 @@ final class AccountsManager { /// - Returns: `true` when the selection actually moved. @discardableResult func correctContextIfAccountDeleted() -> Bool { + // The namespace moved and the reload has not refilled memory yet, so + // the selection and the tombstones weighed against each other here both + // belong to the previous identity. Fenced on the loaded namespace for + // the same reason `save()` is: acting on that state repairs — and + // announces — a choice the identity being switched to never made. The + // reload replaces both sides, and the next activation asks again. + guard loadedNamespace == namespace else { return false } + guard case let .account(id) = storedContext.context else { return false } guard deletedAccountIDs.contains(id) else { return false } diff --git a/Yattee/Views/Components/SubscriptionDestinationMenu.swift b/Yattee/Views/Components/SubscriptionDestinationMenu.swift index 601357b0..884fff26 100644 --- a/Yattee/Views/Components/SubscriptionDestinationMenu.swift +++ b/Yattee/Views/Components/SubscriptionDestinationMenu.swift @@ -145,6 +145,14 @@ struct SubscriptionDestinationRows: View { .onReceive(NotificationCenter.default.publisher(for: .accountsDidChange)) { _ in invalidateMembership() } + // An instance switched on adds a row to `destinations` on the spot, and + // the round that would have asked about it has already run. Unknown + // reads as unsubscribed, so an account the channel is already on gets a + // checkmark-less row whose tap subscribes again instead of doing the + // unsubscribe it promises. + .onReceive(NotificationCenter.default.publisher(for: .instancesDidChange)) { _ in + invalidateMembership() + } } /// Forgets what is known and asks again. diff --git a/Yattee/Views/MediaBrowser/MediaSourcesView.swift b/Yattee/Views/MediaBrowser/MediaSourcesView.swift index ac0facbb..11572f14 100644 --- a/Yattee/Views/MediaBrowser/MediaSourcesView.swift +++ b/Yattee/Views/MediaBrowser/MediaSourcesView.swift @@ -127,6 +127,13 @@ struct MediaSourcesView: View { pendingDeleteInstance = nil pendingDeleteSource = nil } + } message: { + // The same cascade runs from here as from Settings, so the same + // logins go with it — this dialog was simply the one still silent + // about it. + if let accounts = deleteConfirmationAccounts { + Text(accounts) + } } #if !os(tvOS) .presentationCompactAdaptation(.sheet) @@ -163,6 +170,11 @@ struct MediaSourcesView: View { return String(localized: "sources.delete.confirmation") } + private var deleteConfirmationAccounts: String? { + guard let instance = pendingDeleteInstance else { return nil } + return InstanceDeletionDisclosure.message(for: instance, in: appEnvironment) + } + private func confirmDelete() { if let instance = pendingDeleteInstance { appEnvironment?.removeInstance(instance) diff --git a/Yattee/Views/Settings/SubscriptionsSettingsView.swift b/Yattee/Views/Settings/SubscriptionsSettingsView.swift index b2b32789..0450ebc1 100644 --- a/Yattee/Views/Settings/SubscriptionsSettingsView.swift +++ b/Yattee/Views/Settings/SubscriptionsSettingsView.swift @@ -353,14 +353,23 @@ struct SubscriptionsSettingsView: View { } /// Refreshes the account-scoped subscription count shown in the export footer. + /// + /// The context the fetch was started for is rechecked before its answer is + /// applied. A selection change would already have been fenced by the + /// service's provider generation, but the fallback to local is driven by the + /// account's availability and announces nothing — so a count still in flight + /// would otherwise land under the local heading it never came from. private func refreshAccountSubscriptionCount() async { guard let subscriptionService else { return } if let cached = subscriptionService.cachedSubscriptionCount { accountSubscriptionCount = cached } - if !effectiveContext.isLocal { + + let context = effectiveContext + if !context.isLocal { if let channels = try? await subscriptionService.fetchSubscriptions() { + guard effectiveContext == context else { return } accountSubscriptionCount = channels.count } } diff --git a/YatteeTests/AccountsManagerTests.swift b/YatteeTests/AccountsManagerTests.swift index 957c6825..defa0b05 100644 --- a/YatteeTests/AccountsManagerTests.swift +++ b/YatteeTests/AccountsManagerTests.swift @@ -666,6 +666,53 @@ struct DataContextValidatorTests { #expect(!accounts.pendingContextUpload) } + @Test("Nothing is repaired while the namespace is reloading") + func correctionIsFencedToTheLoadedNamespace() { + let instances = InstancesManager() + let instance = Instance(type: .invidious, url: URL(string: "https://inv-reload.example.com")!) + instances.add(instance) + defer { instances.remove(instance) } + + let scope = CredentialIdentityScope(fixedScope: "identity-A") + let toasts = ToastManager() + let accounts = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: UserDefaults(suiteName: "yattee.tests.\(UUID().uuidString)")! + ) + let validator = DataContextValidator( + accountsManager: accounts, + instancesManager: instances, + toastManager: toasts + ) + + let deleted = accounts.addAccount(instanceID: instance.id) + let sibling = accounts.addAccount(instanceID: instance.id) + accounts.setStatus(.signedIn, for: sibling.id) + accounts.setContext(.local) + accounts.applyRemoteTombstone(deleted.id) + accounts.setContext(.account(deleted.id)) + + // The scope moved before the observer that reloads the manager ran, so + // the selection and the tombstone are both still identity-A's — and the + // identity now resolved is identity-B, which chose neither. + scope.setFixedScope("identity-B", announcing: false) + validator.correctIfContextRemoved() + + #expect(accounts.currentContext == .account(deleted.id)) + // Least of all announced: a toast here tells the user their account was + // switched during a reload that has not shown them an account yet. + #expect(toasts.activeToasts.isEmpty) + + // Back under the namespace the state in memory came from, the same + // repair runs exactly as it always did. + scope.setFixedScope("identity-A", announcing: false) + validator.correctIfContextRemoved() + + #expect(accounts.currentContext == .account(sibling.id)) + #expect(!toasts.activeToasts.isEmpty) + } + @Test("Only accounts the app can actually resolve are offered") func availableContextsAreFiltered() { let instances = InstancesManager()