From cd01c08b5a863121cc9a1cb9e5117be44bc6eecc Mon Sep 17 00:00:00 2001 From: Arkadiusz Fal Date: Tue, 28 Jul 2026 18:54:38 +0200 Subject: [PATCH] Fix #960: scope subscription counts, import/export to active account When an Invidious/Piped account is active, several views read the local SwiftData subscription store instead of the account, so counts contradicted each other (Home tile said 10 while the Channels list showed the server's 2): - Home Channels tile now uses the provider's count via new SubscriptionService.cachedSubscriptionCount (fetched once in the background when the server cache isn't populated yet) - Export footer and export content use the active account's list; export runs async with a spinner and surfaces fetch errors as a toast - CSV/OPML import routes through SubscriptionService.importSubscriptions, subscribing on the server for server accounts instead of silently writing to the invisible local store - ChannelView subscribe-state is corrected via the provider after the optimistic local-store read; unused *Sync write helpers removed - Server-account subscribe/unsubscribe/import now post subscriptionsDidChange so other views refresh - New "Delete Local Subscription Data" section in Subscriptions settings (visible with a server account) clears the local store and queues CloudKit deletions so iCloud doesn't restore it --- Yattee/Data/DataManager+Subscriptions.swift | 39 ++++++ Yattee/Localizable.xcstrings | 88 +++++++++++++ Yattee/Services/SubscriptionService.swift | 70 +++++++--- .../InvidiousSubscriptionProvider.swift | 4 + .../LocalSubscriptionProvider.swift | 4 + .../PipedSubscriptionProvider.swift | 4 + .../Subscriptions/SubscriptionProvider.swift | 4 + Yattee/Views/Channel/ChannelView.swift | 14 +- Yattee/Views/Home/HomeView.swift | 17 ++- .../Settings/SubscriptionsSettingsView.swift | 122 +++++++++++++++++- 10 files changed, 339 insertions(+), 27 deletions(-) diff --git a/Yattee/Data/DataManager+Subscriptions.swift b/Yattee/Data/DataManager+Subscriptions.swift index f22a62a1..e66a1236 100644 --- a/Yattee/Data/DataManager+Subscriptions.swift +++ b/Yattee/Data/DataManager+Subscriptions.swift @@ -291,6 +291,45 @@ extension DataManager { } } + /// Deletes all locally stored subscriptions, including their iCloud copies. + /// Server-account subscriptions (Invidious/Piped) are unaffected. + func deleteAllSubscriptions() { + let allSubscriptions = subscriptions() + guard !allSubscriptions.isEmpty else { return } + + var deleteInfo: [(channelID: String, scope: SourceScope)] = [] + for subscription in allSubscriptions { + let scope = SourceScope.from( + sourceRawValue: subscription.sourceRawValue, + globalProvider: subscription.providerName, + instanceURLString: subscription.instanceURLString, + externalExtractor: nil + ) + deleteInfo.append((subscription.channelID, scope)) + modelContext.delete(subscription) + } + + save() + + for info in deleteInfo { + cloudKitSync?.queueSubscriptionDelete(channelID: info.channelID, scope: info.scope) + } + + SubscriptionFeedCache.shared.invalidate() + + let change = SubscriptionChange( + addedSubscriptions: [], + removedChannelIDs: deleteInfo.map(\.channelID) + ) + NotificationCenter.default.post( + name: .subscriptionsDidChange, + object: nil, + userInfo: [SubscriptionChange.userInfoKey: change] + ) + + LoggingService.shared.info("Deleted all \(deleteInfo.count) local subscriptions", category: .general) + } + /// Returns the total count of subscriptions. var subscriptionCount: Int { let descriptor = FetchDescriptor() diff --git a/Yattee/Localizable.xcstrings b/Yattee/Localizable.xcstrings index a422a23a..592d9244 100644 --- a/Yattee/Localizable.xcstrings +++ b/Yattee/Localizable.xcstrings @@ -14788,6 +14788,17 @@ } } }, + "settings.subscriptions.export.error.title" : { + "comment" : "Title for export error toast", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Export Failed" + } + } + } + }, "settings.subscriptions.export.footer %lld" : { "comment" : "Footer showing subscription count for export", "localizations" : { @@ -14898,6 +14909,83 @@ } } }, + "settings.subscriptions.localData.delete.button" : { + "comment" : "Button to delete locally stored subscriptions", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Delete Local Subscription Data" + } + } + } + }, + "settings.subscriptions.localData.delete.confirmation.action" : { + "comment" : "Confirmation action for deleting local subscriptions", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Delete" + } + } + } + }, + "settings.subscriptions.localData.delete.confirmation.message" : { + "comment" : "Message explaining what deleting local subscriptions does", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Removes subscriptions stored on this device and synced with iCloud. Subscriptions on your Invidious or Piped account are not affected." + } + } + } + }, + "settings.subscriptions.localData.delete.confirmation.title %lld" : { + "comment" : "Confirmation title for deleting local subscriptions", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Delete %lld local subscriptions?" + } + } + } + }, + "settings.subscriptions.localData.deleted.title" : { + "comment" : "Toast shown after local subscriptions were deleted", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Local Subscriptions Deleted" + } + } + } + }, + "settings.subscriptions.localData.footer %lld" : { + "comment" : "Footer showing how many subscriptions are stored locally", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "%lld subscriptions are stored locally on this device. They are not used while a server account is selected." + } + } + } + }, + "settings.subscriptions.localData.title" : { + "comment" : "Header for local subscription data section", + "localizations" : { + "en" : { + "stringUnit" : { + "state" : "translated", + "value" : "Local Data" + } + } + } + }, "settings.subscriptions.title" : { "comment" : "Title for subscriptions settings section", "localizations" : { diff --git a/Yattee/Services/SubscriptionService.swift b/Yattee/Services/SubscriptionService.swift index ab16bfbe..07b8eed6 100644 --- a/Yattee/Services/SubscriptionService.swift +++ b/Yattee/Services/SubscriptionService.swift @@ -114,6 +114,12 @@ final class SubscriptionService { settingsManager.subscriptionAccount.type } + /// Subscription count for the active account, when known without a network fetch. + /// Always available for local accounts; nil for server accounts until their cache is populated. + var cachedSubscriptionCount: Int? { + currentProvider?.cachedSubscriptionCount + } + // MARK: - Subscribe /// Subscribes to a channel using the current provider. @@ -129,6 +135,7 @@ final class SubscriptionService { do { try await provider.subscribe(to: channel) + postChangeNotificationForServerAccount() LoggingService.shared.info( "Subscribed to \(channel.name) via \(provider.accountType)", category: .general @@ -174,6 +181,7 @@ final class SubscriptionService { do { try await provider.unsubscribe(from: channelID) + postChangeNotificationForServerAccount() LoggingService.shared.info( "Unsubscribed from \(channelID) via \(provider.accountType)", category: .general @@ -262,27 +270,53 @@ final class SubscriptionService { } } - // MARK: - Synchronous Helpers (for backwards compatibility) + // MARK: - Import - /// Synchronously checks if subscribed to a channel. - /// Uses cached data from DataManager for instant response. - /// - Parameter channelID: The channel ID to check. - /// - Returns: `true` if subscribed (based on local cache), `false` otherwise. - func isSubscribedSync(to channelID: String) -> Bool { - dataManager.isSubscribed(to: channelID) + /// Imports parsed external subscriptions (YouTube CSV / OPML) into the active account. + /// Local accounts write to SwiftData; server accounts subscribe on the server, + /// one channel at a time. Channels that are already subscribed are skipped. + func importSubscriptions(_ channels: [(channelID: String, name: String)]) async -> (imported: Int, skipped: Int) { + guard currentAccountType != .local else { + return dataManager.importSubscriptionsFromExternal(channels) + } + + guard let provider = currentProvider else { return (0, channels.count) } + + // Populate the cache so already-subscribed channels can be skipped + try? await provider.refreshCache() + + var imported = 0 + var skipped = 0 + for entry in channels { + if await provider.isSubscribed(to: entry.channelID) { + skipped += 1 + continue + } + do { + try await provider.subscribe(to: Channel(id: .global(entry.channelID), name: entry.name)) + imported += 1 + } catch { + skipped += 1 + LoggingService.shared.error( + "Failed to import subscription \(entry.channelID): \(error.localizedDescription)", + category: .general + ) + } + } + + if imported > 0 { + postChangeNotificationForServerAccount() + } + + return (imported, skipped) } - /// Synchronously subscribes to a channel (local provider only). - /// For Invidious provider, this will only update local cache. - /// - Parameter channel: The channel to subscribe to. - func subscribeSync(to channel: Channel) { - dataManager.subscribe(to: channel) - } + // MARK: - Private Helpers - /// Synchronously unsubscribes from a channel (local provider only). - /// For Invidious provider, this will only update local cache. - /// - Parameter channelID: The channel ID to unsubscribe from. - func unsubscribeSync(from channelID: String) { - dataManager.unsubscribe(from: channelID) + /// Posts a subscriptions-changed notification for server accounts. + /// Local accounts already post it from DataManager with a change payload. + private func postChangeNotificationForServerAccount() { + guard currentAccountType != .local else { return } + NotificationCenter.default.post(name: .subscriptionsDidChange, object: nil) } } diff --git a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift index 4efa4fe4..7b3a02ca 100644 --- a/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/InvidiousSubscriptionProvider.swift @@ -109,6 +109,10 @@ final class InvidiousSubscriptionProvider: SubscriptionProvider { _ = try await fetchSubscriptions() } + var cachedSubscriptionCount: Int? { + cachePopulated ? cachedChannels.count : nil + } + // MARK: - Private Helpers /// Gets the authenticated Invidious instance and session ID from account settings. diff --git a/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift b/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift index ea6ddb60..c6a7c469 100644 --- a/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/LocalSubscriptionProvider.swift @@ -63,4 +63,8 @@ final class LocalSubscriptionProvider: SubscriptionProvider { func refreshCache() async throws { // Local provider doesn't need cache refresh - data is already local } + + var cachedSubscriptionCount: Int? { + dataManager.subscriptionCount + } } diff --git a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift index 86191241..29465044 100644 --- a/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/PipedSubscriptionProvider.swift @@ -109,6 +109,10 @@ final class PipedSubscriptionProvider: SubscriptionProvider { _ = try await fetchSubscriptions() } + var cachedSubscriptionCount: Int? { + cachePopulated ? cachedChannels.count : nil + } + // MARK: - Private Helpers /// Gets the authenticated Piped instance and auth token from account settings. diff --git a/Yattee/Services/Subscriptions/SubscriptionProvider.swift b/Yattee/Services/Subscriptions/SubscriptionProvider.swift index 83c1ba8b..144e1a80 100644 --- a/Yattee/Services/Subscriptions/SubscriptionProvider.swift +++ b/Yattee/Services/Subscriptions/SubscriptionProvider.swift @@ -32,6 +32,10 @@ protocol SubscriptionProvider: Sendable { /// - Returns: `true` if subscribed, `false` otherwise. func isSubscribed(to channelID: String) async -> Bool + /// The subscription count known without a network fetch. + /// Returns nil when the provider hasn't populated its cache yet. + var cachedSubscriptionCount: Int? { get } + /// Refreshes the local cache of subscriptions from the remote source. /// For local provider, this is a no-op. func refreshCache() async throws diff --git a/Yattee/Views/Channel/ChannelView.swift b/Yattee/Views/Channel/ChannelView.swift index 7119f3d8..4d988249 100644 --- a/Yattee/Views/Channel/ChannelView.swift +++ b/Yattee/Views/Channel/ChannelView.swift @@ -2097,9 +2097,13 @@ struct ChannelView: View { isLoading = true errorMessage = nil - // Load subscription state + // Load subscription state: optimistic from the local store, then + // corrected by the active provider (server accounts differ from local) subscription = appEnvironment.dataManager.subscription(for: channelID) isSubscribed = subscription != nil + Task { + isSubscribed = await appEnvironment.subscriptionService.isSubscribed(to: channelID) + } // Load cached header data for immediate display cachedHeader = CachedChannelData.load(for: channelID, using: appEnvironment.dataManager) @@ -2212,6 +2216,9 @@ struct ChannelView: View { // Check subscription status using extracted channel ID subscription = appEnvironment.dataManager.subscription(for: fetchedChannel.id.channelID) isSubscribed = subscription != nil + Task { + isSubscribed = await appEnvironment.subscriptionService.isSubscribed(to: fetchedChannel.id.channelID) + } } } catch let error as APIError { await MainActor.run { @@ -2247,6 +2254,11 @@ struct ChannelView: View { let effectiveChannelID = channel?.id.channelID ?? channelID subscription = appEnvironment?.dataManager.subscription(for: effectiveChannelID) isSubscribed = subscription != nil + Task { + if let service = appEnvironment?.subscriptionService { + isSubscribed = await service.isSubscribed(to: effectiveChannelID) + } + } } private func loadMoreVideos() async { diff --git a/Yattee/Views/Home/HomeView.swift b/Yattee/Views/Home/HomeView.swift index 1cb1c7b5..2bf1023b 100644 --- a/Yattee/Views/Home/HomeView.swift +++ b/Yattee/Views/Home/HomeView.swift @@ -1473,7 +1473,22 @@ struct HomeView: View { } private func loadChannelsData() { - channelsCount = dataManager?.subscriptions().count ?? 0 + guard let service = appEnvironment?.subscriptionService else { + channelsCount = 0 + return + } + + // Local accounts always have a count; server accounts only after their + // in-memory cache is populated — fetch it once in the background otherwise. + if let cached = service.cachedSubscriptionCount { + channelsCount = cached + } else { + Task { + if let channels = try? await service.fetchSubscriptions() { + channelsCount = channels.count + } + } + } } private func loadRemoteDevicesData() { diff --git a/Yattee/Views/Settings/SubscriptionsSettingsView.swift b/Yattee/Views/Settings/SubscriptionsSettingsView.swift index 7f7118a2..8bc3d99d 100644 --- a/Yattee/Views/Settings/SubscriptionsSettingsView.swift +++ b/Yattee/Views/Settings/SubscriptionsSettingsView.swift @@ -35,7 +35,13 @@ struct SubscriptionsSettingsView: View { // Export state @State private var selectedExportFormat: SubscriptionExportFormat = .json @State private var exportFile: ExportFile? + @State private var isExporting = false + // Account subscription count (provider-scoped, fetched for server accounts) + @State private var accountSubscriptionCount: Int? + + // Local data deletion state + @State private var showingDeleteLocalConfirmation = false private var dataManager: DataManager? { appEnvironment?.dataManager @@ -49,7 +55,17 @@ struct SubscriptionsSettingsView: View { appEnvironment?.subscriptionAccountValidator } + private var subscriptionService: SubscriptionService? { + appEnvironment?.subscriptionService + } + + /// Subscription count of the active account (local store or server). private var subscriptionCount: Int { + accountSubscriptionCount ?? subscriptionService?.cachedSubscriptionCount ?? 0 + } + + /// Count of subscriptions in the local store, shown in the local-data section. + private var localSubscriptionCount: Int { dataManager?.subscriptionCount ?? 0 } @@ -63,8 +79,12 @@ struct SubscriptionsSettingsView: View { if validator?.hasAvailableAccounts == true { importSection exportSection + localDataSection } } + .task(id: currentAccount) { + await refreshAccountSubscriptionCount() + } .navigationTitle(String(localized: "settings.subscriptions.title")) #if os(iOS) .navigationBarTitleDisplayMode(.inline) @@ -111,6 +131,17 @@ struct SubscriptionsSettingsView: View { } message: { Text(String(localized: "settings.subscriptions.account.switch.message")) } + .confirmationDialog( + String(localized: "settings.subscriptions.localData.delete.confirmation.title \(localSubscriptionCount)"), + isPresented: $showingDeleteLocalConfirmation, + titleVisibility: .visible + ) { + Button(String(localized: "settings.subscriptions.localData.delete.confirmation.action"), role: .destructive) { + deleteLocalSubscriptions() + } + } message: { + Text(String(localized: "settings.subscriptions.localData.delete.confirmation.message")) + } .presentationCompactAdaptation(.sheet) } @@ -225,9 +256,15 @@ struct SubscriptionsSettingsView: View { Button { exportSubscriptions() } label: { - Label(String(localized: "settings.subscriptions.export.button"), systemImage: "square.and.arrow.up") + HStack { + Label(String(localized: "settings.subscriptions.export.button"), systemImage: "square.and.arrow.up") + Spacer() + if isExporting { + ProgressView() + } + } } - .disabled(subscriptionCount == 0) + .disabled(subscriptionCount == 0 || isExporting) } header: { Text(String(localized: "settings.subscriptions.export.title")) } footer: { @@ -235,6 +272,27 @@ struct SubscriptionsSettingsView: View { } } + // MARK: - Local Data Section + + /// Lets the user delete locally stored subscriptions while a server account + /// is active. Hidden in local mode, where the account IS the local store. + @ViewBuilder + private var localDataSection: some View { + if currentAccount.type != .local && localSubscriptionCount > 0 { + Section { + Button(role: .destructive) { + showingDeleteLocalConfirmation = true + } label: { + Label(String(localized: "settings.subscriptions.localData.delete.button"), systemImage: "trash") + } + } header: { + Text(String(localized: "settings.subscriptions.localData.title")) + } footer: { + Text(String(localized: "settings.subscriptions.localData.footer \(localSubscriptionCount)")) + } + } + } + // MARK: - Actions private func showImportPicker() { @@ -281,12 +339,12 @@ struct SubscriptionsSettingsView: View { // Parse subscriptions let parseResult = try SubscriptionImportExport.parseAuto(data) - // Import to database - guard let dataManager else { + // Import into the active account (local store or server) + guard let subscriptionService else { throw SubscriptionImportError.invalidData } - let importStats = dataManager.importSubscriptionsFromExternal(parseResult.channels) + let importStats = await subscriptionService.importSubscriptions(parseResult.channels) await MainActor.run { isImporting = false @@ -294,6 +352,7 @@ struct SubscriptionsSettingsView: View { showingImportResult = true LoggingService.shared.logSubscriptions("Import completed: \(importStats.imported) imported, \(importStats.skipped) skipped") } + await refreshAccountSubscriptionCount() } catch { await MainActor.run { isImporting = false @@ -306,9 +365,36 @@ struct SubscriptionsSettingsView: View { } private func exportSubscriptions() { - guard let dataManager else { return } + isExporting = true + Task { + await performExport() + isExporting = false + } + } - let subscriptions = dataManager.allSubscriptions + /// Exports subscriptions of the active account: the local store in local + /// mode, or the server's list for Invidious/Piped accounts. + private func performExport() async { + let subscriptions: [Subscription] + if currentAccount.type == .local { + guard let dataManager else { return } + subscriptions = dataManager.allSubscriptions + } else { + guard let subscriptionService else { return } + do { + let channels = try await subscriptionService.fetchSubscriptions() + accountSubscriptionCount = channels.count + // Detached model instances used purely as export carriers + subscriptions = channels.map { Subscription.from(channel: $0) } + } catch { + LoggingService.shared.logSubscriptionsError("Failed to fetch account subscriptions for export", error: error) + appEnvironment?.toastManager.showError( + String(localized: "settings.subscriptions.export.error.title"), + subtitle: error.localizedDescription + ) + return + } + } let data: Data? switch selectedExportFormat { @@ -339,6 +425,28 @@ struct SubscriptionsSettingsView: View { #endif } + /// Refreshes the account-scoped subscription count shown in the export footer. + private func refreshAccountSubscriptionCount() async { + guard let subscriptionService else { return } + + if let cached = subscriptionService.cachedSubscriptionCount { + accountSubscriptionCount = cached + } + if subscriptionService.currentAccountType != .local { + if let channels = try? await subscriptionService.fetchSubscriptions() { + accountSubscriptionCount = channels.count + } + } + } + + private func deleteLocalSubscriptions() { + dataManager?.deleteAllSubscriptions() + appEnvironment?.toastManager.showSuccess(String(localized: "settings.subscriptions.localData.deleted.title")) + Task { + await refreshAccountSubscriptionCount() + } + } + #if os(macOS) private func showMacOSSavePanel(data: Data, filename: String) { let panel = NSSavePanel()