diff --git a/Yattee/Core/AccountRemoval.swift b/Yattee/Core/AccountRemoval.swift new file mode 100644 index 00000000..f96521a8 --- /dev/null +++ b/Yattee/Core/AccountRemoval.swift @@ -0,0 +1,42 @@ +// +// AccountRemoval.swift +// Yattee +// +// Taking an account off this device by hand. +// + +import Foundation + +/// The removal cascade for an account whose instance is gone. +/// +/// Kept out of `AppEnvironment` so the ordering can be exercised directly: an +/// orphan is exactly the case where the usual lookups return nothing, and a +/// cascade that quietly skips a step leaves a secret behind with no screen left +/// to reach it from. +@MainActor +enum AccountRemoval { + /// Removes an orphaned account and the credential filed under its ID. + /// + /// `AppEnvironment.accountCredentialsManager(for:)` resolves the store from + /// the account's instance, and an orphan has none — going through it would + /// return `nil` and delete nothing. Every account-keyed store is asked to + /// drop the ID instead; at most one of them holds anything, and the scoped + /// key needs only the ID. The unscoped legacy item is deliberately left + /// alone: identifying it needs the instance, which is precisely what is + /// missing. + static func removeOrphan( + _ accountID: UUID, + stores: [any AccountCredentialsManager], + accountsManager: AccountsManager, + discardData: (UUID) -> Void + ) { + for store in stores { + store.deleteCredential(forAccountID: accountID) + } + + accountsManager.removeAccount(accountID) + // Whatever the account had in memory goes with it, or its subscriptions + // and playlists would keep answering for an account nobody has. + discardData(accountID) + } +} diff --git a/Yattee/Core/AppEnvironment.swift b/Yattee/Core/AppEnvironment.swift index ac7be099..6cf48f11 100644 --- a/Yattee/Core/AppEnvironment.swift +++ b/Yattee/Core/AppEnvironment.swift @@ -878,6 +878,40 @@ final class AppEnvironment { instancesManager.remove(instance) } + /// Signs an account out and removes it, with everything keyed to it. + /// + /// The one cascade behind every by-hand sign-out, so a place that has to be + /// cleared is added here rather than in each screen that lists accounts. + func signOutAndRemoveAccount(_ account: Account) { + let instance = accountsManager.instance(for: account) + + // Deleting the account is what authorizes removing the legacy item, and + // only when this device adopted it and it still holds that same secret. + accountCredentialsManager(for: account)?.deleteCredential(for: account) + accountCredentialsManager(for: account)?.deleteLegacyCredentialIfOwned(for: account) + accountsManager.removeAccount(account.id) + // Drop what the account had in memory too, or its subscriptions and + // playlists would keep answering for an account that no longer exists. + discardAccountData(for: account.id) + + if let instance, accountsManager.accounts(forInstance: instance.id).isEmpty { + instancesManager.setUsesAccountLogin(false, for: instance) + } + } + + /// Removes an account whose instance is gone. + /// + /// Separate from `signOutAndRemoveAccount(_:)` because none of the lookups + /// that cascade depends on can resolve without an instance. + func removeOrphanedAccount(_ accountID: UUID) { + AccountRemoval.removeOrphan( + accountID, + stores: [invidiousCredentialsManager, pipedCredentialsManager], + accountsManager: accountsManager, + discardData: { discardAccountData(for: $0) } + ) + } + /// The account-keyed credentials manager for an account's instance. /// `nil` when the instance is unknown or its type has no account concept. func accountCredentialsManager(for account: Account) -> (any AccountCredentialsManager)? { diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index b637d394..fe1bce62 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -153,6 +153,90 @@ } } }, + "accounts.capability.history" : { + "comment" : "Capability hint on an account row", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Watch history" + } + } + } + }, + "accounts.capability.playlists" : { + "comment" : "Capability hint on an account row", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Playlists" + } + } + } + }, + "accounts.capability.subscriptions" : { + "comment" : "Capability hint on an account row", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Subscriptions" + } + } + } + }, + "accounts.empty.description" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Sign in to an Invidious or Piped source to add an account." + } + } + } + }, + "accounts.empty.title" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "No Accounts" + } + } + } + }, + "accounts.orphan.footer" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "These accounts belong to a source that is not on this device. It may still arrive from iCloud — removing an account here deletes its saved login." + } + } + } + }, + "accounts.orphan.section" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Source Missing" + } + } + } + }, + "accounts.orphan.unnamed" : { + "comment" : "Title of an account whose source is gone and which has no label", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Account" + } + } + } + }, "accounts.playlists.error.entryMismatch" : { "localizations" : { "en" : { @@ -193,6 +277,37 @@ } } }, + "accounts.remove.confirm.message" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The saved login is deleted, and the account is removed from your other devices too." + } + } + } + }, + "accounts.remove.confirm.message.orphan" : { + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The saved login is deleted, and the account is removed from your other devices too. Its source is already gone from this device." + } + } + } + }, + "accounts.remove.confirm.title %@" : { + "comment" : "Confirmation title with the account name", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Remove \"%@\"?" + } + } + } + }, "accounts.signIn.identityChanged" : { "localizations" : { "en" : { @@ -304,6 +419,17 @@ } } }, + "accounts.title" : { + "comment" : "Title of the accounts screen", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Accounts" + } + } + } + }, "accounts.useThisAccount" : { "localizations" : { "en" : { @@ -1499,6 +1625,17 @@ } } }, + "common.skip" : { + "comment" : "Button that passes over an optional step", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Skip" + } + } + } + }, "common.unit.points" : { "localizations" : { "en" : { @@ -5447,6 +5584,17 @@ } } }, + "login.footer.optionalStep" : { + "comment" : "Footer of the optional sign-in step shown after adding a source", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "The source has been added. You can sign in later from Settings." + } + } + } + }, "login.footer.pipedAccount %@" : { "localizations" : { "en" : { @@ -16427,6 +16575,29 @@ } } }, + "sources.delete.accountsWarning %lld" : { + "comment" : "Lead-in to the accounts an instance deletion signs out, followed by their names", + "localizations" : { + "en" : { + "variations" : { + "plural" : { + "one" : { + "stringUnit" : { + "state" : "translated", + "value" : "This also signs out and removes 1 account:" + } + }, + "other" : { + "stringUnit" : { + "state" : "translated", + "value" : "This also signs out and removes %lld accounts:" + } + } + } + } + } + } + }, "sources.delete.confirmation" : { "comment" : "Generic delete confirmation title", "localizations" : { diff --git a/Yattee/Views/Components/AccountStatusBadge.swift b/Yattee/Views/Components/AccountStatusBadge.swift new file mode 100644 index 00000000..90f1560c --- /dev/null +++ b/Yattee/Views/Components/AccountStatusBadge.swift @@ -0,0 +1,47 @@ +// +// AccountStatusBadge.swift +// Yattee +// +// How an account's status reads, wherever accounts are listed. +// + +import SwiftUI + +/// An account's status as one line of caption text. +/// +/// Shared so a status says the same thing in every list it appears in — the +/// per-instance section, the accounts screen and the context switcher all name +/// the same states, and one added to `AccountStatus` is named once rather than +/// three times. +struct AccountStatusBadge: View { + let status: AccountStatus + + var body: some View { + Text(Self.text(for: status)) + .font(.caption) + .foregroundStyle(Self.color(for: status)) + } + + static func text(for status: AccountStatus) -> String { + switch status { + case .checking: String(localized: "accounts.status.checking") + case .signedIn: String(localized: "accounts.status.signedIn") + case .credentialMissing: String(localized: "accounts.status.credentialMissing") + case .credentialRejected: String(localized: "accounts.status.credentialRejected") + case .unreachable: String(localized: "accounts.status.unreachable") + } + } + + /// Only the states the user has to act on are coloured. A credential that + /// never arrived on this device is ordinary — tvOS never receives them at + /// all — so it reads as secondary rather than as a fault. + static func color(for status: AccountStatus) -> Color { + switch status { + case .checking: .secondary + case .signedIn: .secondary + case .credentialMissing: .secondary + case .credentialRejected: .red + case .unreachable: .orange + } + } +} diff --git a/Yattee/Views/Components/DataContextSwitcherMenu.swift b/Yattee/Views/Components/DataContextSwitcherMenu.swift index 86c4fa76..0b23533a 100644 --- a/Yattee/Views/Components/DataContextSwitcherMenu.swift +++ b/Yattee/Views/Components/DataContextSwitcherMenu.swift @@ -84,7 +84,7 @@ enum DataContextSwitcherLabels { let status = appEnvironment.accountsManager.status(for: id) guard status != .signedIn else { return title } - return "\(title) — \(statusText(for: status))" + return "\(title) — \(AccountStatusBadge.text(for: status))" } @MainActor @@ -93,16 +93,6 @@ enum DataContextSwitcherLabels { .filter { appEnvironment.dataContextValidator.displayName(for: $0) == name } .count > 1 } - - static func statusText(for status: AccountStatus) -> String { - switch status { - case .checking: String(localized: "accounts.status.checking") - case .signedIn: String(localized: "accounts.status.signedIn") - case .credentialMissing: String(localized: "accounts.status.credentialMissing") - case .credentialRejected: String(localized: "accounts.status.credentialRejected") - case .unreachable: String(localized: "accounts.status.unreachable") - } - } } // MARK: - Menu diff --git a/Yattee/Views/Settings/AccountSignInHandler.swift b/Yattee/Views/Settings/AccountSignInHandler.swift new file mode 100644 index 00000000..f5d21c6a --- /dev/null +++ b/Yattee/Views/Settings/AccountSignInHandler.swift @@ -0,0 +1,104 @@ +// +// AccountSignInHandler.swift +// Yattee +// +// What happens after a login succeeds, for every screen that offers one. +// + +import Foundation + +/// Files a successful login against an account, creating one if this was a +/// fresh sign-in. +/// +/// One implementation for every place a login can be started — the per-instance +/// accounts section, the accounts screen, and adding a source — because the +/// order it does things in is what keeps a half-written login from looking like +/// a real one, and that order is not something a second copy would preserve. +@MainActor +struct AccountSignInHandler { + let appEnvironment: AppEnvironment + /// The instance being signed in to. A new account is created on it; an + /// existing one is expected to already live there. + let instance: Instance + + /// The identifier is stored beside the credential in the Keychain rather + /// than on the account record, so it never travels as synced metadata. + /// - Returns: whether the credential was stored. A login the server + /// accepted is not the same as one this device could keep. + @discardableResult + func storeLogin(credential: String, username: String, target: AccountLoginTarget) -> Bool { + // Credential storage refuses to write under an unresolved identity + // scope, so creating the account first would leave one with no + // credential — and in a namespace that is discarded the moment the real + // scope arrives. Fail visibly instead. + guard appEnvironment.credentialIdentityScope.isResolved else { + appEnvironment.toastManager.showError( + String(localized: "accounts.signIn.identityUnresolved") + ) + return false + } + + // Re-resolved rather than trusted: signing in takes a round trip, and + // the account can be deleted on another device while it is happening. + // Writing the credential then would file a secret under an account that + // no longer exists, and report success for it. + if let existing = target.account, appEnvironment.accountsManager.account(id: existing.id) == nil { + appEnvironment.toastManager.showError( + String(localized: "accounts.signIn.storeFailed") + ) + return false + } + + let account = target.account + ?? appEnvironment.accountsManager.addAccount(instanceID: instance.id) + + guard let credentials = appEnvironment.accountCredentialsManager(for: account) else { + // Its instance may have gone in the meantime, which is what makes + // the backend unresolvable. A provisional account created moments + // ago must not be left behind. + if target.account == nil { + appEnvironment.accountsManager.rollbackAccountCreation(account.id) + } + return false + } + + credentials.setCredential(credential, for: account) + + // Confirm the write landed before reporting success: a Keychain failure + // would otherwise leave an account that looks signed in but has nothing + // behind it. + guard credentials.credential(for: account) != nil else { + if target.account == nil { + // Rolled back, not deleted: creating it also queued the marker + // that stops other devices synthesizing an account for this + // instance's pre-revamp login, and a sign-in that never + // completed must not leave that behind. + appEnvironment.accountsManager.rollbackAccountCreation(account.id) + } + appEnvironment.toastManager.showError( + String(localized: "accounts.signIn.storeFailed") + ) + return false + } + if !username.isEmpty { + credentials.setUsername(username, for: account) + } + + appEnvironment.accountsManager.setStatus(.signedIn, for: account.id) + + // `usesAccountLogin` is deliberately not set. It is pre-revamp state + // that migration only reads, and it syncs through iCloud key-value + // storage independently of the account records — a device receiving the + // flag before the account would synthesize a migrated account for a + // login that was never legacy, and the marker that arrives later does + // not retract it. Accounts describe logins now; the flag only describes + // the world before they existed. + + // Signing in again keeps the same account, so nothing downstream sees a + // change of context — but the credential behind it may now belong to + // somebody else, and their data must not survive the swap. + appEnvironment.accountCredentialDidChange(for: account) + + return true + } +} diff --git a/Yattee/Views/Settings/AccountsListView.swift b/Yattee/Views/Settings/AccountsListView.swift new file mode 100644 index 00000000..0ea1e202 --- /dev/null +++ b/Yattee/Views/Settings/AccountsListView.swift @@ -0,0 +1,384 @@ +// +// AccountsListView.swift +// Yattee +// +// Every account on this device, grouped by the source it lives on. +// + +import SwiftUI + +/// A login started from this screen. +/// +/// Unlike the per-instance section, this one spans every source — so the +/// instance the sheet is for travels with the target rather than being a +/// property of the view. +private struct AccountsListLogin: Identifiable { + let id: UUID + let instance: Instance + let target: AccountLoginTarget +} + +/// An account the user asked to remove, held until the confirmation comes back. +private struct PendingAccountRemoval: Identifiable { + let id: UUID + let name: String + let isOrphan: Bool +} + +/// The accounts screen: what is signed in where, and what to do about it. +/// +/// The per-instance section in the source editor shows one server's logins +/// while the user is already editing it. This is the other direction — every +/// login the app knows about, including the ones whose source is gone and which +/// no source editor can reach. +struct AccountsListView: View { + @Environment(\.appEnvironment) private var appEnvironment + + @State private var login: AccountsListLogin? + @State private var pendingRemoval: PendingAccountRemoval? + + /// Bumped whenever a credential is replaced, purely to re-read the login + /// identifier the rows display. + /// + /// That identifier lives in the Keychain beside the secret, not in any + /// observable state, so signing in again as somebody else moves nothing + /// SwiftUI watches. The rest of the screen is driven by observation: the + /// account list, the statuses and the instances are all observable, and + /// rebuilding rows on every status change would cost tvOS its focus. + @State private var credentialRevision = 0 + + private var accountsManager: AccountsManager? { + appEnvironment?.accountsManager + } + + // MARK: - Body + + var body: some View { + Group { + #if os(tvOS) + TVSidebarDetailContainer( + systemImage: "person.crop.circle", + title: String(localized: "accounts.title") + ) { + content + } + #else + content + #endif + } + #if !os(tvOS) + .navigationTitle(String(localized: "accounts.title")) + #endif + #if os(iOS) + .navigationBarTitleDisplayMode(.inline) + #endif + #if os(tvOS) + .fullScreenCover(item: $login) { login in + loginSheet(login) + } + #else + .sheet(item: $login) { login in + loginSheet(login) + } + #endif + .confirmationDialog( + pendingRemoval.map { String(localized: "accounts.remove.confirm.title \($0.name)") } ?? "", + isPresented: Binding( + get: { pendingRemoval != nil }, + set: { if !$0 { pendingRemoval = nil } } + ), + titleVisibility: .visible + ) { + Button(String(localized: "common.remove"), role: .destructive) { + confirmRemoval() + } + Button(String(localized: "common.cancel"), role: .cancel) { + pendingRemoval = nil + } + } message: { + Text( + String( + localized: pendingRemoval?.isOrphan == true + ? "accounts.remove.confirm.message.orphan" + : "accounts.remove.confirm.message" + ) + ) + } + #if !os(tvOS) + .presentationCompactAdaptation(.sheet) + #endif + .onReceive(NotificationCenter.default.publisher(for: .accountCredentialDidChange)) { _ in + credentialRevision &+= 1 + } + .accessibilityIdentifier("accounts.view") + } + + @ViewBuilder + private var content: some View { + if accountsManager?.accounts.isEmpty ?? true { + emptyState + } else { + Form { + ForEach(groups, id: \.instance.id) { group in + instanceSection(group.instance, accounts: group.accounts) + } + + orphanSection + } + #if os(macOS) + .formStyle(.grouped) + #endif + .opaqueSettingsFormBackground() + } + } + + private var emptyState: some View { + ContentUnavailableView { + Label(String(localized: "accounts.empty.title"), systemImage: "person.crop.circle") + } description: { + Text(String(localized: "accounts.empty.description")) + } + .frame(maxWidth: .infinity, maxHeight: .infinity) + } + + // MARK: - Sections + + /// Instances that hold at least one account, oldest source first. + /// + /// A source with no logins is left out: the place to make one is the source + /// itself, and listing every server here would bury the accounts. + private var groups: [(instance: Instance, accounts: [Account])] { + guard let appEnvironment else { return [] } + + return appEnvironment.instancesManager.instances + .sorted { $0.dateAdded < $1.dateAdded } + .compactMap { instance in + let accounts = sorted(appEnvironment.accountsManager.accounts(forInstance: instance.id)) + return accounts.isEmpty ? nil : (instance, accounts) + } + } + + /// Stable regardless of the order the accounts arrived from iCloud in. + private func sorted(_ accounts: [Account]) -> [Account] { + accounts.sorted { + ($0.dateAdded, $0.id.uuidString) < ($1.dateAdded, $1.id.uuidString) + } + } + + @ViewBuilder + private func instanceSection(_ instance: Instance, accounts: [Account]) -> some View { + Section { + ForEach(accounts) { account in + accountRow(account, instance: instance) + } + // Read here on purpose, so the rows genuinely depend on it rather + // than relying on a state write invalidating a body that never + // mentions it. + .id(credentialRevision) + } header: { + Text(instance.displayName) + } + } + + @ViewBuilder + private var orphanSection: some View { + let orphans = sorted(accountsManager?.orphanedAccounts ?? []) + + if !orphans.isEmpty { + Section { + ForEach(orphans) { account in + accountRow(account, instance: nil) + } + } header: { + Text(String(localized: "accounts.orphan.section")) + } footer: { + Text(String(localized: "accounts.orphan.footer")) + } + } + } + + // MARK: - Rows + + @ViewBuilder + private func accountRow(_ account: Account, instance: Instance?) -> some View { + VStack(alignment: .leading, spacing: 4) { + Text(title(for: account, instance: instance)) + .lineLimit(1) + + if instance == nil { + // An orphan has no status worth reporting: its capabilities are + // unknown and no request can be made on its behalf. What it has + // instead is the reason. + Text(String(localized: "accounts.status.instanceMissing")) + .font(.caption) + .foregroundStyle(.orange) + } else { + AccountStatusBadge(status: status(for: account)) + } + + if let capabilities = capabilityHints(for: account) { + Text(capabilities) + .font(.caption2) + .foregroundStyle(.tertiary) + } + } + .frame(maxWidth: .infinity, alignment: .leading) + #if !os(tvOS) + .contextMenu { + rowActions(account, instance: instance) + } + #endif + + #if os(tvOS) + // A context menu is not reachable on tvOS, and a Form row is a single + // focusable unit — so the actions are rows of their own. + rowActions(account, instance: instance) + #endif + } + + @ViewBuilder + private func rowActions(_ account: Account, instance: Instance?) -> some View { + if let instance { + if isSelectable(account), !isCurrentContext(account) { + Button { + appEnvironment?.switchDataContext(to: .account(account.id)) + } label: { + Label(String(localized: "accounts.useThisAccount"), systemImage: "checkmark.circle") + } + #if os(tvOS) + .buttonStyle(TVSettingsButtonStyle()) + #endif + } + + Button { + login = AccountsListLogin( + id: account.id, + instance: instance, + target: .existing(account) + ) + } label: { + Label(String(localized: "login.signInAgain"), systemImage: "person.badge.key") + } + #if os(tvOS) + .buttonStyle(TVSettingsButtonStyle()) + #endif + } + + Button(role: .destructive) { + pendingRemoval = PendingAccountRemoval( + id: account.id, + name: title(for: account, instance: instance), + isOrphan: instance == nil + ) + } label: { + Label( + String(localized: instance == nil ? "common.remove" : "login.signOutAndRemove"), + systemImage: instance == nil ? "trash" : "rectangle.portrait.and.arrow.forward" + ) + } + #if os(tvOS) + .buttonStyle(TVSettingsButtonStyle()) + #endif + } + + private func loginSheet(_ login: AccountsListLogin) -> some View { + InstanceLoginView(instance: login.instance) { credential, username in + guard let appEnvironment else { return false } + return AccountSignInHandler(appEnvironment: appEnvironment, instance: login.instance) + .storeLogin(credential: credential, username: username, target: login.target) + } + } + + // MARK: - Presentation + + /// The orphan's own label, never the validator's: with no instance to + /// resolve, its last-resort fallback is the words "Source missing", which + /// the row already says right underneath. + private func title(for account: Account, instance: Instance?) -> String { + guard instance != nil else { + if let name = account.displayName, !name.isEmpty { return name } + return String(localized: "accounts.orphan.unnamed") + } + + return appEnvironment?.dataContextValidator.displayName(for: account) + ?? String(localized: "accounts.orphan.unnamed") + } + + private func status(for account: Account) -> AccountStatus { + accountsManager?.status(for: account.id) ?? .checking + } + + /// Asked of the validator rather than recomputed here: an account on a + /// disabled source passes the capability test but is not something the + /// switcher offers, and a screen that offered it would switch the app into + /// a context that immediately falls back to local. + private func isSelectable(_ account: Account) -> Bool { + appEnvironment?.dataContextValidator.availableContexts.contains(.account(account.id)) == true + } + + private func isCurrentContext(_ account: Account) -> Bool { + accountsManager?.currentContext == .account(account.id) + } + + /// What the backend behind this account lets it do. Deliberately a short + /// sentence rather than a row of glyphs — the difference between two Piped + /// logins is who they are, not what they support. + private func capabilityHints(for account: Account) -> String? { + guard let capabilities = accountsManager?.capabilities(for: account) else { return nil } + + var parts: [String] = [] + if capabilities.supportsSubscriptions { + parts.append(String(localized: "accounts.capability.subscriptions")) + } + if capabilities.supportsPlaylistReads { + parts.append(String(localized: "accounts.capability.playlists")) + } + if capabilities.supportsWatchHistory { + parts.append(String(localized: "accounts.capability.history")) + } + + guard !parts.isEmpty else { return nil } + return parts.joined(separator: " · ") + } + + /// Whether the settings entry should carry an attention badge. + /// + /// Both states the user can only clear from this screen: an orphan nothing + /// else lists, and a login the server has started refusing. + @MainActor + static func needsAttention(in appEnvironment: AppEnvironment?) -> Bool { + guard let appEnvironment else { return false } + + if !appEnvironment.accountsManager.orphanedAccounts.isEmpty { return true } + + return appEnvironment.accountsManager.accounts.contains { + appEnvironment.accountsManager.status(for: $0.id) == .credentialRejected + } + } + + // MARK: - Actions + + private func confirmRemoval() { + guard let appEnvironment, let pending = pendingRemoval else { return } + defer { pendingRemoval = nil } + + // Re-resolved rather than trusted: the dialog can outlive the account, + // which another device may have deleted while it was up. + guard let account = appEnvironment.accountsManager.account(id: pending.id) else { return } + + if pending.isOrphan { + appEnvironment.removeOrphanedAccount(account.id) + } else { + appEnvironment.signOutAndRemoveAccount(account) + } + } +} + +// MARK: - Preview + +#Preview { + NavigationStack { + AccountsListView() + } + .appEnvironment(.preview) +} diff --git a/Yattee/Views/Settings/AddSource/AddRemoteServerView.swift b/Yattee/Views/Settings/AddSource/AddRemoteServerView.swift index 7a121734..eba7a568 100644 --- a/Yattee/Views/Settings/AddSource/AddRemoteServerView.swift +++ b/Yattee/Views/Settings/AddSource/AddRemoteServerView.swift @@ -68,6 +68,13 @@ struct AddRemoteServerView: View { @State private var showingYatteeServerWarning = false @State private var pendingYatteeServerInstance: Instance? + /// The instance whose optional sign-in step is on screen. + /// + /// The source is already added by then, so dismissing this sheet — by + /// signing in, by skipping, or by pulling it down — is what finishes + /// adding. Only instances that have accounts at all get this far. + @State private var signInInstance: Instance? + // Closure to dismiss the parent sheet var dismissSheet: DismissAction? @@ -131,6 +138,9 @@ struct AddRemoteServerView: View { Text(String(localized: "sources.yatteeServer.warning.message")) } .presentationCompactAdaptation(.sheet) + .fullScreenCover(item: $signInInstance, onDismiss: finishAdding) { instance in + signInStep(for: instance) + } #else formContent .navigationTitle(String(localized: "sources.addRemoteServer")) @@ -147,20 +157,30 @@ struct AddRemoteServerView: View { Text(String(localized: "sources.yatteeServer.warning.message")) } .presentationCompactAdaptation(.sheet) + .sheet(item: $signInInstance, onDismiss: finishAdding) { instance in + signInStep(for: instance) + } #endif } + /// The optional login offered right after a source that supports accounts + /// is added, so the common case does not need a second trip through + /// settings to become useful. + private func signInStep(for instance: Instance) -> some View { + InstanceLoginView(instance: instance, isOptionalStep: true) { credential, username in + guard let appEnvironment else { return false } + return AccountSignInHandler(appEnvironment: appEnvironment, instance: instance) + .storeLogin(credential: credential, username: username, target: .newAccount) + } + } + @ViewBuilder private var yatteeServerWarningButtons: some View { Button(String(localized: "sources.yatteeServer.warning.disableOthers"), role: .destructive) { if let instance = pendingYatteeServerInstance { appEnvironment?.instancesManager.disableOtherYatteeServerInstances(except: instance.id) appEnvironment?.instancesManager.add(instance) - if let dismissSheet { - dismissSheet() - } else { - dismiss() - } + finishAdding() } } Button(String(localized: "common.cancel"), role: .cancel) { @@ -768,11 +788,7 @@ struct AddRemoteServerView: View { showingYatteeServerWarning = true } else { appEnvironment.instancesManager.add(instance) - if let dismissSheet { - dismissSheet() - } else { - dismiss() - } + finishAdding() } } else { credentialValidationError = String(localized: "sources.error.invalidCredentials") @@ -802,6 +818,20 @@ struct AddRemoteServerView: View { } appEnvironment.instancesManager.add(instance) + + // A source that can hold accounts is worth offering a login for while + // the user is still here. Everything else is done. + if instance.supportsAuthentication { + signInInstance = instance + } else { + finishAdding() + } + } + + /// Closes the add flow. Reached directly for sources with no login step, + /// and as the sign-in sheet's dismissal for the ones that have it — + /// signing in and skipping end the same way. + private func finishAdding() { if let dismissSheet { dismissSheet() } else { diff --git a/Yattee/Views/Settings/EditSourceView.swift b/Yattee/Views/Settings/EditSourceView.swift index e85b255a..446e57b4 100644 --- a/Yattee/Views/Settings/EditSourceView.swift +++ b/Yattee/Views/Settings/EditSourceView.swift @@ -312,6 +312,12 @@ private struct EditRemoteServerContent: View { dismiss() } Button(String(localized: "common.cancel"), role: .cancel) {} + } message: { + // Deleting a source has always taken its logins with it; only the + // question was silent about it. + if let accounts = InstanceDeletionDisclosure.message(for: instance, in: appEnvironment) { + Text(accounts) + } } .presentationCompactAdaptation(.sheet) #if os(tvOS) @@ -389,91 +395,19 @@ private struct EditRemoteServerContent: View { // MARK: - Actions - /// Files a successful login against an account, creating one if this was a - /// fresh sign-in. - /// - /// The identifier is stored beside the credential in the Keychain rather - /// than on the account record, so it never travels as synced metadata. - /// - Returns: whether the credential was stored. A login the server - /// accepted is not the same as one this device could keep. + /// Files a successful login against an account, and reflects it in the + /// authenticated actions this sheet shows. @discardableResult private func storeLogin(credential: String, username: String, target: AccountLoginTarget) -> Bool { guard let appEnvironment else { return false } - // Credential storage refuses to write under an unresolved identity - // scope, so creating the account first would leave one with no - // credential — and in a namespace that is discarded the moment the real - // scope arrives. Fail visibly instead. - guard appEnvironment.credentialIdentityScope.isResolved else { - appEnvironment.toastManager.showError( - String(localized: "accounts.signIn.identityUnresolved") - ) - return false + let stored = AccountSignInHandler(appEnvironment: appEnvironment, instance: instance) + .storeLogin(credential: credential, username: username, target: target) + + if stored { + isLoggedIn = true } - - // Re-resolved rather than trusted: signing in takes a round trip, and - // the account can be deleted on another device while it is happening. - // Writing the credential then would file a secret under an account that - // no longer exists, and report success for it. - if let existing = target.account, appEnvironment.accountsManager.account(id: existing.id) == nil { - appEnvironment.toastManager.showError( - String(localized: "accounts.signIn.storeFailed") - ) - return false - } - - let account = target.account - ?? appEnvironment.accountsManager.addAccount(instanceID: instance.id) - - guard let credentials = appEnvironment.accountCredentialsManager(for: account) else { - // Its instance may have gone in the meantime, which is what makes - // the backend unresolvable. A provisional account created moments - // ago must not be left behind. - if target.account == nil { - appEnvironment.accountsManager.rollbackAccountCreation(account.id) - } - return false - } - - credentials.setCredential(credential, for: account) - - // Confirm the write landed before reporting success: a Keychain failure - // would otherwise leave an account that looks signed in but has nothing - // behind it. - guard credentials.credential(for: account) != nil else { - if target.account == nil { - // Rolled back, not deleted: creating it also queued the marker - // that stops other devices synthesizing an account for this - // instance's pre-revamp login, and a sign-in that never - // completed must not leave that behind. - appEnvironment.accountsManager.rollbackAccountCreation(account.id) - } - appEnvironment.toastManager.showError( - String(localized: "accounts.signIn.storeFailed") - ) - return false - } - if !username.isEmpty { - credentials.setUsername(username, for: account) - } - - appEnvironment.accountsManager.setStatus(.signedIn, for: account.id) - - // `usesAccountLogin` is deliberately not set. It is pre-revamp state - // that migration only reads, and it syncs through iCloud key-value - // storage independently of the account records — a device receiving the - // flag before the account would synthesize a migrated account for a - // login that was never legacy, and the marker that arrives later does - // not retract it. Accounts describe logins now; the flag only describes - // the world before they existed. - - // Signing in again keeps the same account, so nothing downstream sees a - // change of context — but the credential behind it may now belong to - // somebody else, and their data must not survive the swap. - appEnvironment.accountCredentialDidChange(for: account) - - isLoggedIn = true - return true + return stored } private func loadServerInfo() async { diff --git a/Yattee/Views/Settings/InstanceAccountsSection.swift b/Yattee/Views/Settings/InstanceAccountsSection.swift index b427ee43..44c73de9 100644 --- a/Yattee/Views/Settings/InstanceAccountsSection.swift +++ b/Yattee/Views/Settings/InstanceAccountsSection.swift @@ -40,6 +40,11 @@ struct InstanceAccountsSection: View { /// one instance apart. @State private var credentialRevision = 0 + /// The account a sign-out was asked for, held until the confirmation comes + /// back. Removing an account deletes its credential from every device + /// sharing the Keychain, which is not something to do on one tap. + @State private var pendingSignOut: Account? + private var accounts: [Account] { appEnvironment?.accountsManager.accounts(forInstance: instance.id) ?? [] } @@ -83,20 +88,15 @@ struct InstanceAccountsSection: View { Text(displayName(for: account)) .lineLimit(1) - Text(statusText(for: account)) - .font(.caption) - .foregroundStyle(statusColor(for: account)) + AccountStatusBadge(status: status(for: account)) } .frame(maxWidth: .infinity, alignment: .leading) .contextMenu { - // Without this a second login on an instance cannot be reached: the - // legacy picker lists one entry per instance, so it always resolves - // to whichever account was signed in first. The switcher that makes - // this a first-class choice everywhere comes later; until then this - // is what makes a second login usable at all. + // Selecting an account from the source that owns it, rather than + // hunting for it in the global switcher. if !isCurrentContext(account) { Button { - appEnvironment?.accountsManager.setContext(.account(account.id)) + appEnvironment?.switchDataContext(to: .account(account.id)) } label: { Label(String(localized: "accounts.useThisAccount"), systemImage: "checkmark.circle") } @@ -109,11 +109,36 @@ struct InstanceAccountsSection: View { } Button(role: .destructive) { - signOut(account) + pendingSignOut = account } label: { Label(String(localized: "login.signOutAndRemove"), systemImage: "rectangle.portrait.and.arrow.forward") } } + // Attached to the row that raised it rather than to the section: a + // presentation modifier on a `Section` inside a `Form` has no view of + // its own to present from. + .confirmationDialog( + String(localized: "accounts.remove.confirm.title \(displayName(for: account))"), + isPresented: Binding( + // By ID: a label edit arriving from another device while the + // dialog is up would otherwise close it. + get: { pendingSignOut?.id == account.id }, + set: { if !$0 { pendingSignOut = nil } } + ), + titleVisibility: .visible + ) { + Button(String(localized: "common.remove"), role: .destructive) { + confirmSignOut() + } + Button(String(localized: "common.cancel"), role: .cancel) { + pendingSignOut = nil + } + } message: { + Text(String(localized: "accounts.remove.confirm.message")) + } + #if !os(tvOS) + .presentationCompactAdaptation(.sheet) + #endif #if os(tvOS) // tvOS never syncs app Keychain items, so an account that arrived from @@ -141,46 +166,20 @@ struct InstanceAccountsSection: View { appEnvironment?.accountsManager.status(for: account.id) ?? .checking } - private func statusText(for account: Account) -> String { - switch status(for: account) { - case .checking: String(localized: "accounts.status.checking") - case .signedIn: String(localized: "accounts.status.signedIn") - case .credentialMissing: String(localized: "accounts.status.credentialMissing") - case .credentialRejected: String(localized: "accounts.status.credentialRejected") - case .unreachable: String(localized: "accounts.status.unreachable") - } - } - private func isCurrentContext(_ account: Account) -> Bool { appEnvironment?.accountsManager.currentContext == .account(account.id) } - private func statusColor(for account: Account) -> Color { - switch status(for: account) { - case .checking: .secondary - case .signedIn: .secondary - case .credentialMissing: .secondary - case .credentialRejected: .red - case .unreachable: .orange - } - } - // MARK: - Actions - private func signOut(_ account: Account) { - guard let appEnvironment else { return } + private func confirmSignOut() { + guard let appEnvironment, let account = pendingSignOut else { return } + defer { pendingSignOut = nil } - // Deleting the account is what authorizes removing the legacy item, and - // only when this device adopted it and it still holds that same secret. - appEnvironment.accountCredentialsManager(for: account)?.deleteCredential(for: account) - appEnvironment.accountCredentialsManager(for: account)?.deleteLegacyCredentialIfOwned(for: account) - appEnvironment.accountsManager.removeAccount(account.id) - // Drop what the account had in memory too, or its subscriptions and - // playlists would keep answering for an account that no longer exists. - appEnvironment.discardAccountData(for: account.id) + // Re-resolved rather than trusted: the dialog can outlive the account, + // which another device may have deleted while it was up. + guard let account = appEnvironment.accountsManager.account(id: account.id) else { return } - if appEnvironment.accountsManager.accounts(forInstance: instance.id).isEmpty { - appEnvironment.instancesManager.setUsesAccountLogin(false, for: instance) - } + appEnvironment.signOutAndRemoveAccount(account) } } diff --git a/Yattee/Views/Settings/InstanceDeletionDisclosure.swift b/Yattee/Views/Settings/InstanceDeletionDisclosure.swift new file mode 100644 index 00000000..6da2755d --- /dev/null +++ b/Yattee/Views/Settings/InstanceDeletionDisclosure.swift @@ -0,0 +1,35 @@ +// +// InstanceDeletionDisclosure.swift +// Yattee +// +// What deleting a source takes with it, said before it happens. +// + +import Foundation + +/// Names the accounts an instance deletion would sign out and remove. +/// +/// The cascade has always taken them; only the confirmation was silent about +/// it. Deleting a source the user set up months ago is otherwise the one action +/// in the app that quietly destroys a login. +enum InstanceDeletionDisclosure { + /// The accounts paragraph for the confirmation, or `nil` when the instance + /// has none and the plain "delete this?" question already says everything. + static func message(accountNames: [String]) -> String? { + guard !accountNames.isEmpty else { return nil } + + let warning = String(localized: "sources.delete.accountsWarning \(accountNames.count)") + return ([warning] + accountNames).joined(separator: "\n") + } + + @MainActor + static func message(for instance: Instance, in appEnvironment: AppEnvironment?) -> String? { + guard let appEnvironment else { return nil } + + let names = appEnvironment.accountsManager + .accounts(forInstance: instance.id) + .map { appEnvironment.dataContextValidator.displayName(for: $0) } + + return message(accountNames: names) + } +} diff --git a/Yattee/Views/Settings/InstanceLoginView.swift b/Yattee/Views/Settings/InstanceLoginView.swift index c50c26c7..9e33cf08 100644 --- a/Yattee/Views/Settings/InstanceLoginView.swift +++ b/Yattee/Views/Settings/InstanceLoginView.swift @@ -9,6 +9,16 @@ import SwiftUI struct InstanceLoginView: View { let instance: Instance + + /// Whether signing in is optional here. + /// + /// Set when the sheet follows an action that already succeeded — adding a + /// source, which is saved before this appears. The way out is then "Skip" + /// rather than "Cancel", and it is offered as a button in the form as well: + /// tvOS has no cancellation affordance at all, so an optional step without + /// one would be a dead end there. + var isOptionalStep: Bool = false + /// Receives the credential **and** the identifier the user signed in with, /// and reports whether it was stored. /// @@ -47,7 +57,7 @@ struct InstanceLoginView: View { #endif .toolbar { ToolbarItem(placement: .cancellationAction) { - Button(String(localized: "common.cancel")) { + Button(String(localized: isOptionalStep ? "common.skip" : "common.cancel")) { dismiss() } } @@ -113,6 +123,22 @@ struct InstanceLoginView: View { .buttonStyle(TVSettingsButtonStyle()) #endif } + + if isOptionalStep { + Section { + Button { + dismiss() + } label: { + Text(String(localized: "common.skip")) + } + .accessibilityIdentifier("instance.login.skipButton") + #if os(tvOS) + .buttonStyle(TVSettingsButtonStyle()) + #endif + } footer: { + Text(String(localized: "login.footer.optionalStep")) + } + } } #if os(iOS) .scrollDismissesKeyboard(.interactively) diff --git a/Yattee/Views/Settings/SettingsView.swift b/Yattee/Views/Settings/SettingsView.swift index 698d761d..4007278b 100644 --- a/Yattee/Views/Settings/SettingsView.swift +++ b/Yattee/Views/Settings/SettingsView.swift @@ -35,8 +35,16 @@ struct SettingsView: View { NavigationSplitView { VStack(spacing: 0) { List(SettingsSection.allCases, selection: $selectedSection) { section in - Label(section.title, systemImage: section.icon) - .tag(section) + HStack { + Label(section.title, systemImage: section.icon) + + if section == .accounts, AccountsListView.needsAttention(in: appEnvironment) { + Spacer() + Image(systemName: "exclamationmark.circle.fill") + .foregroundStyle(.orange) + } + } + .tag(section) } .listStyle(.sidebar) @@ -51,6 +59,8 @@ struct SettingsView: View { switch selectedSection { case .sources: SourcesListView() + case .accounts: + AccountsListView() case .icloud: iCloudSettingsView() case .appearance: @@ -128,6 +138,20 @@ struct SettingsView: View { } .accessibilityIdentifier("settings.row.sources") + NavigationLink { + AccountsListView() + } label: { + HStack { + Label(String(localized: "accounts.title"), systemImage: "person.crop.circle") + Spacer() + if AccountsListView.needsAttention(in: appEnvironment) { + Image(systemName: "exclamationmark.circle.fill") + .foregroundStyle(.orange) + } + } + } + .accessibilityIdentifier("settings.row.accounts") + DataContextSwitcherSettingsRow() NavigationLink { @@ -232,6 +256,20 @@ struct SettingsView: View { } .accessibilityIdentifier("settings.row.sources") + NavigationLink { + AccountsListView() + } label: { + HStack { + Label(String(localized: "accounts.title"), systemImage: "person.crop.circle") + Spacer() + if AccountsListView.needsAttention(in: appEnvironment) { + Image(systemName: "exclamationmark.circle.fill") + .foregroundStyle(.orange) + } + } + } + .accessibilityIdentifier("settings.row.accounts") + DataContextSwitcherSettingsRow() } @@ -360,6 +398,7 @@ struct SettingsView: View { enum SettingsSection: String, CaseIterable, Identifiable { case sources + case accounts case icloud case appearance case layoutNavigation @@ -377,6 +416,7 @@ enum SettingsSection: String, CaseIterable, Identifiable { var title: String { switch self { case .sources: return String(localized: "sources.title") + case .accounts: return String(localized: "accounts.title") case .icloud: return String(localized: "settings.icloud.title") case .appearance: return String(localized: "settings.appearance.sectionTitle") case .layoutNavigation: return String(localized: "settings.layoutNavigation.title") @@ -394,6 +434,7 @@ enum SettingsSection: String, CaseIterable, Identifiable { var icon: String { switch self { case .sources: return "server.rack" + case .accounts: return "person.crop.circle" case .icloud: return "icloud" case .appearance: return "paintbrush" case .layoutNavigation: return "hand.tap" diff --git a/Yattee/Views/Settings/SourcesListView.swift b/Yattee/Views/Settings/SourcesListView.swift index da3e703c..9b2e5e34 100644 --- a/Yattee/Views/Settings/SourcesListView.swift +++ b/Yattee/Views/Settings/SourcesListView.swift @@ -128,6 +128,12 @@ struct SourcesListView: View { pendingDeleteInstance = nil pendingDeleteSource = nil } + } message: { + // Deleting a source has always taken its logins with it; only the + // question was silent about it. + if let accounts = deleteConfirmationAccounts { + Text(accounts) + } } #if !os(tvOS) .presentationCompactAdaptation(.sheet) @@ -603,6 +609,11 @@ struct SourcesListView: 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) + } } #if os(tvOS) diff --git a/YatteeTests/AccountsScreenTests.swift b/YatteeTests/AccountsScreenTests.swift new file mode 100644 index 00000000..d23fdae6 --- /dev/null +++ b/YatteeTests/AccountsScreenTests.swift @@ -0,0 +1,254 @@ +// +// AccountsScreenTests.swift +// YatteeTests +// +// The accounts screen's two pieces of logic that are not presentation: +// removing an account whose source is gone, and disclosing what deleting a +// source takes with it. +// + +import Testing +import Foundation +import SwiftUI +@testable import Yattee + +@Suite("Orphaned account removal") +@MainActor +struct OrphanedAccountRemovalTests { + private func makeDefaults() -> UserDefaults { + UserDefaults(suiteName: "yattee.tests.\(UUID().uuidString)")! + } + + private func makeScope() -> CredentialIdentityScope { + CredentialIdentityScope(fixedScope: "identity-\(UUID().uuidString)") + } + + private func makeInstance(_ type: InstanceType) -> Instance { + Instance(type: type, url: URL(string: "https://\(UUID().uuidString).example.com")!) + } + + @Test("An account outlives its source, and knows nothing about it") + func orphanHasNothingToResolveThrough() { + let scope = makeScope() + let instances = InstancesManager() + let instance = makeInstance(.invidious) + instances.add(instance) + + let accountsManager = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: makeDefaults() + ) + let account = accountsManager.addAccount(instanceID: instance.id) + + instances.remove(instance) + + // Everything the ordinary cascade resolves through is gone with it — + // which is why removal has to go by account ID rather than by instance. + #expect(accountsManager.instance(for: account) == nil) + #expect(accountsManager.capabilities(for: account) == nil) + #expect(accountsManager.orphanedAccounts.contains { $0.id == account.id }) + #expect(!accountsManager.selectableAccounts.contains { $0.id == account.id }) + + accountsManager.removeAccount(account.id) + } + + @Test("Removing an orphan deletes the credential filed under its ID") + func removalDeletesTheCredential() { + let scope = makeScope() + let invidious = InvidiousCredentialsManager(identityScope: scope) + let piped = PipedCredentialsManager(identityScope: scope) + + let instances = InstancesManager() + let instance = makeInstance(.invidious) + instances.add(instance) + + let accountsManager = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: makeDefaults() + ) + let account = accountsManager.addAccount(instanceID: instance.id) + invidious.setCredential("sid-1", for: account) + #expect(invidious.credential(for: account) != nil) + + instances.remove(instance) + + var discarded: [UUID] = [] + AccountRemoval.removeOrphan( + account.id, + stores: [invidious, piped], + accountsManager: accountsManager, + discardData: { discarded.append($0) } + ) + + #expect(invidious.credential(for: account) == nil) + #expect(accountsManager.account(id: account.id) == nil) + #expect(!accountsManager.orphanedAccounts.contains { $0.id == account.id }) + // The in-memory data goes too, or the account's subscriptions and + // playlists keep answering for an account nobody has. + #expect(discarded == [account.id]) + } + + @Test("Every store is asked, because the backend is no longer knowable") + func removalAsksBothStores() { + let scope = makeScope() + let invidious = InvidiousCredentialsManager(identityScope: scope) + let piped = PipedCredentialsManager(identityScope: scope) + + let instances = InstancesManager() + let instance = makeInstance(.piped) + instances.add(instance) + + let accountsManager = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: makeDefaults() + ) + let account = accountsManager.addAccount(instanceID: instance.id) + piped.setCredential("token-1", for: account) + + instances.remove(instance) + + AccountRemoval.removeOrphan( + account.id, + stores: [invidious, piped], + accountsManager: accountsManager, + discardData: { _ in } + ) + + // The instance's type is what said which store to look in, and it is + // gone — so a Piped orphan must be cleared by a caller that cannot know + // it was Piped. + #expect(piped.credential(for: account) == nil) + } + + @Test("A deletion queued for the server survives the removal") + func removalQueuesTheTombstone() { + let scope = makeScope() + let instances = InstancesManager() + let instance = makeInstance(.invidious) + instances.add(instance) + + let accountsManager = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: makeDefaults() + ) + let account = accountsManager.addAccount(instanceID: instance.id) + + instances.remove(instance) + + AccountRemoval.removeOrphan( + account.id, + stores: [], + accountsManager: accountsManager, + discardData: { _ in } + ) + + // Removing an orphan is a real deletion, not a local tidy-up: without + // the tombstone every other device would keep the account and send it + // back. + #expect(accountsManager.pendingDeletions.contains(account.id)) + } + + @Test("The pre-revamp instance-keyed item is deliberately left alone") + func removalLeavesTheLegacyItem() { + let scope = makeScope() + let invidious = InvidiousCredentialsManager(identityScope: scope) + + let instances = InstancesManager() + let instance = makeInstance(.invidious) + instances.add(instance) + + let accountsManager = AccountsManager( + identityScope: scope, + instancesManager: instances, + defaults: makeDefaults() + ) + let account = accountsManager.addAccount(instanceID: instance.id) + + // The unscoped legacy item is keyed by the instance, and identifying it + // needs the instance — which is exactly what an orphan is missing. + invidious.setCredential("legacy-sid", for: instance) + defer { invidious.deleteCredential(for: instance) } + + instances.remove(instance) + + AccountRemoval.removeOrphan( + account.id, + stores: [invidious], + accountsManager: accountsManager, + discardData: { _ in } + ) + + #expect(invidious.credential(for: instance) == "legacy-sid") + } +} + +@Suite("Instance deletion disclosure") +struct InstanceDeletionDisclosureTests { + @Test("A source with no accounts adds nothing to the question") + func noAccountsSaysNothing() { + #expect(InstanceDeletionDisclosure.message(accountNames: []) == nil) + } + + @Test("A single account is counted and named") + func oneAccountIsNamed() throws { + let message = try #require(InstanceDeletionDisclosure.message(accountNames: ["me@inv.example.com"])) + + #expect(message.contains("me@inv.example.com")) + #expect(message.contains("1")) + // The lead-in would leak its key if the plural variation were malformed. + #expect(!message.contains("accountsWarning")) + } + + @Test("Several accounts are counted and all named, one per line") + func severalAccountsAreNamed() throws { + let names = ["first@inv.example.com", "second@inv.example.com", "third@inv.example.com"] + let message = try #require(InstanceDeletionDisclosure.message(accountNames: names)) + + for name in names { + #expect(message.contains(name)) + } + #expect(message.contains("3")) + #expect(message.split(separator: "\n").count == names.count + 1) + } + + @Test("The count reads differently for one than for many") + func countVariesWithTheNumberOfAccounts() throws { + let single = try #require(InstanceDeletionDisclosure.message(accountNames: ["a"])) + let several = try #require(InstanceDeletionDisclosure.message(accountNames: ["a", "b"])) + + let singleLeadIn = try #require(single.split(separator: "\n").first) + let severalLeadIn = try #require(several.split(separator: "\n").first) + + #expect(singleLeadIn != severalLeadIn) + } +} + +@Suite("Account status badge") +struct AccountStatusBadgeTests { + @Test("Every status is named, and no two the same") + func statusesAreNamed() { + let statuses: [AccountStatus] = [ + .checking, .signedIn, .credentialMissing, .credentialRejected, .unreachable + ] + let texts = statuses.map { AccountStatusBadge.text(for: $0) } + + #expect(texts.allSatisfy { !$0.isEmpty }) + #expect(Set(texts).count == statuses.count) + } + + @Test("Only the states the user has to act on are coloured") + func onlyActionableStatesAreColoured() { + #expect(AccountStatusBadge.color(for: .credentialRejected) == .red) + #expect(AccountStatusBadge.color(for: .unreachable) == .orange) + + // A credential that never arrived on this device is ordinary — tvOS + // never receives them at all — so it must not read as a fault. + #expect(AccountStatusBadge.color(for: .credentialMissing) == .secondary) + #expect(AccountStatusBadge.color(for: .signedIn) == .secondary) + #expect(AccountStatusBadge.color(for: .checking) == .secondary) + } +}