From cb72055d01920c277ac362cb8814646791957d28 Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Tue, 11 Aug 2026 16:00:15 -0500 Subject: [PATCH 1/3] [PM-39767] fix: Show Upgraded to Premium celebration screen after Sync Now succeeds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Tapping "Sync Now" on the Upgrade Pending alert and having the retry succeed showed no celebration screen — only the existing Upgraded to Premium action card, indistinguishable from a delayed background sync resolving later. The celebration screen (PremiumUpgradeCompleteView) already existed and was already presented correctly (a full-screen card-style modal, matching Figma) for the synchronous confirm-while-upgrade-screen-is-open path; it just had no way to be reached from the Sync Now retry, since that path dismisses the upgrade screen (and its nested BillingCoordinator) before the retry even runs. Adds a new BillingRoute.premiumUpgradeCompleteStandalone, reachable from all five screens that can initiate an upgrade via a new PremiumUpgradeRoute.premiumUpgradeComplete requirement, mirroring the existing premiumUpgrade wiring in each. After a Sync Now retry resolves, DefaultPremiumUpgradeHelper checks whether the upgrade actually succeeded (not still pending, not failed) before navigating — deliberately not reusing the durable premiumUpgradePendingStatePublisher signal, since that also fires for delayed syncs that must never show this screen. --- .../UI/Billing/BillingCoordinator.swift | 22 ++++++++++- .../UI/Billing/BillingCoordinatorTests.swift | 28 +++++++++++++ BitwardenShared/UI/Billing/BillingRoute.swift | 6 +++ .../UI/Billing/PremiumUpgradeHelper.swift | 18 ++++++++- .../Billing/PremiumUpgradeHelperTests.swift | 39 +++++++++++++++++++ .../Settings/SettingsCoordinator.swift | 15 +++++++ .../Settings/SettingsCoordinatorTests.swift | 12 ++++++ .../UI/Platform/Settings/SettingsRoute.swift | 5 +++ .../UI/Tools/Send/Send/SendCoordinator.swift | 13 +++++++ .../Send/Send/SendCoordinatorTests.swift | 12 ++++++ .../UI/Tools/Send/Send/SendRoute.swift | 5 +++ .../Send/SendItem/SendItemCoordinator.swift | 13 +++++++ .../SendItem/SendItemCoordinatorTests.swift | 11 ++++++ .../Tools/Send/SendItem/SendItemRoute.swift | 5 +++ .../UI/Vault/Vault/VaultCoordinator.swift | 13 +++++++ .../Vault/Vault/VaultCoordinatorTests.swift | 11 ++++++ .../UI/Vault/Vault/VaultRoute.swift | 5 +++ .../VaultItem/VaultItemCoordinator.swift | 13 +++++++ .../VaultItem/VaultItemCoordinatorTests.swift | 11 ++++++ .../UI/Vault/VaultItem/VaultItemRoute.swift | 5 +++ 20 files changed, 259 insertions(+), 3 deletions(-) diff --git a/BitwardenShared/UI/Billing/BillingCoordinator.swift b/BitwardenShared/UI/Billing/BillingCoordinator.swift index 7b07a319b4..a1326476c8 100644 --- a/BitwardenShared/UI/Billing/BillingCoordinator.swift +++ b/BitwardenShared/UI/Billing/BillingCoordinator.swift @@ -64,6 +64,8 @@ class BillingCoordinator: NSObject, Coordinator, HasStackNavigator { } case .premiumUpgradeComplete: showPremiumUpgradeComplete() + case .premiumUpgradeCompleteStandalone: + showPremiumUpgradeCompleteStandalone() case let .premiumPlan(subscription): showPremiumPlan(subscription: subscription) case .premiumUpgrade: @@ -75,7 +77,10 @@ class BillingCoordinator: NSObject, Coordinator, HasStackNavigator { // MARK: Private Methods - /// Shows the Premium upgrade complete screen. + /// Shows the Premium upgrade complete screen on top of the currently-visible + /// `PremiumUpgradeView`, for the synchronous-confirm-while-upgrade-screen-is-open path. Use + /// `showPremiumUpgradeCompleteStandalone()` instead when no `PremiumUpgradeView` has been + /// shown in this coordinator instance. /// private func showPremiumUpgradeComplete() { premiumUpgradeCompleteOnClose = isUpgradeAsModalRoot @@ -94,6 +99,21 @@ class BillingCoordinator: NSObject, Coordinator, HasStackNavigator { stackNavigator?.present(view) } + /// Shows the Premium upgrade complete screen as the sole content of this coordinator's + /// stack, with no `PremiumUpgradeView` shown first. Use this when a Premium upgrade + /// resolves outside of the upgrade screen itself — e.g. a "Sync Now" retry succeeding after + /// the upgrade screen has already been dismissed — so `.dismiss` can simply close the whole + /// modal without any `isUpgradeAsModalRoot`/Settings-plan branching. + /// + private func showPremiumUpgradeCompleteStandalone() { + let processor = PremiumUpgradeCompleteProcessor( + coordinator: asAnyCoordinator(), + services: services, + ) + let view = PremiumUpgradeCompleteView(store: Store(processor: processor)) + stackNavigator?.replace(view) + } + /// Shows the Premium plan screen. /// /// - Parameter subscription: An already-fetched subscription; pass `nil` to let the plan screen fetch it. diff --git a/BitwardenShared/UI/Billing/BillingCoordinatorTests.swift b/BitwardenShared/UI/Billing/BillingCoordinatorTests.swift index 4dab363ad0..4665cd7d30 100644 --- a/BitwardenShared/UI/Billing/BillingCoordinatorTests.swift +++ b/BitwardenShared/UI/Billing/BillingCoordinatorTests.swift @@ -117,6 +117,34 @@ struct BillingCoordinatorTests { #expect(action.view is PremiumUpgradeCompleteView) } + /// `navigate(to:)` with `.premiumUpgradeCompleteStandalone` replaces the stack's root with the + /// Premium upgrade complete view, rather than presenting it as a child of an existing screen. + @Test + func navigate_premiumUpgradeCompleteStandalone() throws { + subject.navigate(to: .premiumUpgradeCompleteStandalone) + + #expect(stackNavigator.actions.count == 1) + let action = try #require(stackNavigator.actions.last) + #expect(action.type == .replaced) + #expect(action.view is PremiumUpgradeCompleteView) + } + + /// `navigate(to:)` with `.dismiss` after `.premiumUpgradeCompleteStandalone` dismisses the + /// entire modal directly, without any of the modal-root/settings-context branching that + /// `.premiumUpgradeComplete` needs. + @Test + func navigate_dismiss_afterPremiumUpgradeCompleteStandalone() throws { + subject.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator.actions.removeAll() + + stackNavigator.isPresenting = false + // viewControllersToPop is empty by default, so pop() returns nil. + subject.navigate(to: .dismiss) + + let action = try #require(stackNavigator.actions.last) + #expect(action.type == .dismissed) + } + /// `navigate(to:)` with `.premiumPlan` pushes the Premium plan view. @Test func navigate_premiumPlan() throws { diff --git a/BitwardenShared/UI/Billing/BillingRoute.swift b/BitwardenShared/UI/Billing/BillingRoute.swift index afb0cbc5fb..fff343b93f 100644 --- a/BitwardenShared/UI/Billing/BillingRoute.swift +++ b/BitwardenShared/UI/Billing/BillingRoute.swift @@ -14,4 +14,10 @@ enum BillingRoute: Equatable { /// A route to the Premium upgrade complete screen. case premiumUpgradeComplete + + /// A route to the Premium upgrade complete screen, presented as the sole content of a + /// freshly-created modal (no `PremiumUpgradeView` shown first in this coordinator instance). + /// Used when a Premium upgrade resolves outside of the upgrade screen itself — e.g. a + /// "Sync Now" retry succeeding after the upgrade screen has already been dismissed. + case premiumUpgradeCompleteStandalone } diff --git a/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift b/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift index b0d6a72d11..1aa6a70834 100644 --- a/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift +++ b/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift @@ -11,6 +11,11 @@ protocol PremiumUpgradeRoute { /// The route to the Premium upgrade screen. static var premiumUpgrade: Self { get } + /// The route to a standalone Premium upgrade complete screen, shown when an upgrade + /// resolves outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after + /// the upgrade screen has already been dismissed). + static var premiumUpgradeComplete: Self { get } + /// The route to dismiss the current screen with an optional action. /// /// - Parameter action: The action to perform on dismiss. @@ -157,12 +162,21 @@ class DefaultPremiumUpgradeHelper: PremiumUpg // MARK: Private Methods /// Calls `onPendingDismiss`, then shows the upgrade pending alert with "Sync Now" wired to - /// `reconcileCheckoutSuccess()`. + /// `reconcileCheckoutSuccess()` — navigating to the standalone Premium upgrade complete + /// screen if the retry actually resolves the upgrade. A still-pending (not yet Premium) or + /// still-failed outcome gets no further UI here: the CTA and the "Sync unsuccessful" alert + /// already react to those independently via the durable `premiumUpgradePendingStatePublisher()` + /// signal. /// private func showUpgradePendingAlert() { onPendingDismiss?() coordinator.showAlert(.upgradePending { [weak self] in - await self?.services.billingService.reconcileCheckoutSuccess() + guard let self else { return } + await services.billingService.reconcileCheckoutSuccess() + let state = await services.billingService.premiumUpgradePendingState() + if !state.isPending, !state.lastAttemptFailed { + coordinator.navigate(to: .premiumUpgradeComplete) + } }) } diff --git a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift index bb699d77e2..631f609904 100644 --- a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift +++ b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift @@ -370,6 +370,45 @@ struct PremiumUpgradeHelperTests { // swiftlint:disable:this type_body_length }) == 1) } + /// Tapping "Sync Now" on the upgrade pending alert navigates to the standalone Premium + /// upgrade complete screen only when the retry actually resolves the pending upgrade — not + /// when it's still pending (not yet Premium) or the retry itself failed, both of which are + /// covered by PR2's CTA and PR3's "Sync unsuccessful" alert independently, via the durable + /// `premiumUpgradePendingStatePublisher()` signal. + @Test(arguments: [ + (PremiumUpgradePendingState(isPending: false, lastAttemptFailed: false), true), + (PremiumUpgradePendingState(isPending: true, lastAttemptFailed: false), false), + (PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true), false), + ]) + func subscribeToPremiumCheckoutStatus_pending_syncNow( + pendingState: PremiumUpgradePendingState, + expectedNavigatesToComplete: Bool, + ) async throws { + billingRepository.isInAppUpgradeAvailableReturnValue = true + let statusSubject = PassthroughSubject() + billingService.premiumCheckoutStatusPublisherReturnValue = statusSubject.eraseToAnyPublisher() + billingService.premiumUpgradePendingStateReturnValue = pendingState + let subject = makeSubject() + await subject.navigateToPremiumUpgrade() + + statusSubject.send(.pending) + try await waitForAsync { + guard case let .dismiss(action) = coordinator.routes.last else { return false } + return action != nil + } + guard case let .dismiss(action) = coordinator.routes.last else { + Issue.record("Expected .dismiss route") + return + } + action?.action() + + let alert = try #require(coordinator.alertShown.last) + try await alert.tapAction(title: Localizations.syncNow) + + #expect(billingService.reconcileCheckoutSuccessCalled) + #expect((coordinator.routes.last == .premiumUpgradeComplete) == expectedNavigatesToComplete) + } + /// When the billing service emits `.syncing`, nothing happens (the loading overlay is shown /// by `PremiumUpgradeProcessor`). @Test diff --git a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift index ef71195967..b4991b2a8b 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift @@ -210,6 +210,8 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d showPremiumPlan(subscription: subscription) case .premiumUpgrade: showPremiumUpgrade() + case .premiumUpgradeComplete: + showPremiumUpgradeCompleteScreen() case let .selectLanguage(currentLanguage: currentLanguage): showSelectLanguage(currentLanguage: currentLanguage, delegate: context as? SelectLanguageDelegate) case let .settings(presentationMode): @@ -518,6 +520,19 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d coordinator.navigate(to: .premiumUpgrade) } + /// Shows a standalone Premium upgrade complete screen, for when an upgrade resolves outside + /// of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the upgrade screen + /// has already been dismissed). Unlike `showPremiumUpgrade()`, this presents its own fresh + /// modal rather than pushing onto Settings' existing stack, matching every other origin + /// screen's treatment of this same screen. + /// + private func showPremiumUpgradeCompleteScreen() { + let navigationController = module.makeNavigationController() + let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) + coordinator.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator?.present(navigationController) + } + /// Shows the select language screen. /// private func showSelectLanguage( diff --git a/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift b/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift index 52582e9976..3a53c4cbbc 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift @@ -400,6 +400,18 @@ class SettingsCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this ty XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgrade]) } + /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade + /// complete screen via the billing coordinator, unlike `.premiumUpgrade` which pushes onto + /// Settings' own existing stack. + @MainActor + func test_navigateTo_premiumUpgradeComplete() throws { + subject.navigate(to: .premiumUpgradeComplete) + + let action = try XCTUnwrap(stackNavigator.actions.last) + XCTAssertEqual(action.type, .presented) + XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) + } + /// `navigate(to:)` with `.selectLanguage()` presents the select language view. @MainActor func test_navigateTo_selectLanguage() throws { diff --git a/BitwardenShared/UI/Platform/Settings/SettingsRoute.swift b/BitwardenShared/UI/Platform/Settings/SettingsRoute.swift index 2a54a51feb..713fadbed3 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsRoute.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsRoute.swift @@ -80,6 +80,11 @@ public enum SettingsRoute: Equatable, Hashable { /// A route to the Premium upgrade screen. case premiumUpgrade + /// A route to a standalone Premium upgrade complete screen, shown when an upgrade resolves + /// outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the + /// upgrade screen has already been dismissed). + case premiumUpgradeComplete + /// A route to view the select language view. /// /// - Parameter currentLanguage: The currently selected language option. diff --git a/BitwardenShared/UI/Tools/Send/Send/SendCoordinator.swift b/BitwardenShared/UI/Tools/Send/Send/SendCoordinator.swift index 2749978b4f..c9843c2829 100644 --- a/BitwardenShared/UI/Tools/Send/Send/SendCoordinator.swift +++ b/BitwardenShared/UI/Tools/Send/Send/SendCoordinator.swift @@ -87,6 +87,8 @@ final class SendCoordinator: Coordinator, HasStackNavigator { showList() case .premiumUpgrade: showPremiumUpgrade() + case .premiumUpgradeComplete: + showPremiumUpgradeCompleteScreen() case let .share(url): showShareSheet(for: [url]) case let .viewItem(sendView): @@ -169,6 +171,17 @@ final class SendCoordinator: Coordinator, HasStackNavigator { stackNavigator?.present(navigationController) } + /// Shows a standalone Premium upgrade complete screen, for when an upgrade resolves outside + /// of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the upgrade screen + /// has already been dismissed). + /// + private func showPremiumUpgradeCompleteScreen() { + let navigationController = module.makeNavigationController() + let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) + coordinator.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator?.present(navigationController) + } + /// Presents the system share sheet for the specified items. /// /// - Parameter items: The items to share using the system share sheet. diff --git a/BitwardenShared/UI/Tools/Send/Send/SendCoordinatorTests.swift b/BitwardenShared/UI/Tools/Send/Send/SendCoordinatorTests.swift index 7428319896..672e2d5153 100644 --- a/BitwardenShared/UI/Tools/Send/Send/SendCoordinatorTests.swift +++ b/BitwardenShared/UI/Tools/Send/Send/SendCoordinatorTests.swift @@ -166,6 +166,18 @@ class SendCoordinatorTests: BitwardenTestCase { XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgrade]) } + /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade + /// complete screen via the billing coordinator. + @MainActor + func test_navigateTo_premiumUpgradeComplete() throws { + subject.navigate(to: .premiumUpgradeComplete) + + let action = try XCTUnwrap(stackNavigator.actions.last) + XCTAssertEqual(action.type, .presented) + XCTAssertTrue(action.view is UINavigationController) + XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) + } + /// `navigate(to:)` with `.share` presents the share sheet. @MainActor func test_navigateTo_share() throws { diff --git a/BitwardenShared/UI/Tools/Send/Send/SendRoute.swift b/BitwardenShared/UI/Tools/Send/Send/SendRoute.swift index bfa1cd96a1..beb6592e0d 100644 --- a/BitwardenShared/UI/Tools/Send/Send/SendRoute.swift +++ b/BitwardenShared/UI/Tools/Send/Send/SendRoute.swift @@ -31,6 +31,11 @@ public enum SendRoute: Equatable { /// A route to the Premium upgrade screen. case premiumUpgrade + /// A route to a standalone Premium upgrade complete screen, shown when an upgrade resolves + /// outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the + /// upgrade screen has already been dismissed). + case premiumUpgradeComplete + /// A route to share the provided URL. /// /// - Parameter url: The `URL` to share. diff --git a/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinator.swift b/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinator.swift index b2abb0bdb9..dd78ce5f6d 100644 --- a/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinator.swift +++ b/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinator.swift @@ -106,6 +106,8 @@ final class SendItemCoordinator: Coordinator, HasStackNavigator, ProfileSwitcher showGenerator(delegate: delegate) case .premiumUpgrade: showPremiumUpgrade() + case .premiumUpgradeComplete: + showPremiumUpgradeCompleteScreen() case let .share(url): showShareSheet(for: [url]) case let .view(sendView): @@ -236,6 +238,17 @@ final class SendItemCoordinator: Coordinator, HasStackNavigator, ProfileSwitcher stackNavigator?.present(navigationController) } + /// Shows a standalone Premium upgrade complete screen, for when an upgrade resolves outside + /// of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the upgrade screen + /// has already been dismissed). + /// + private func showPremiumUpgradeCompleteScreen() { + let navigationController = module.makeNavigationController() + let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) + coordinator.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator?.present(navigationController) + } + /// Presents the system share sheet for the specified items. /// /// - Parameter items: The items to share using the system share sheet. diff --git a/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinatorTests.swift b/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinatorTests.swift index 37e49815b3..e0396ecf5a 100644 --- a/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinatorTests.swift +++ b/BitwardenShared/UI/Tools/Send/SendItem/SendItemCoordinatorTests.swift @@ -209,6 +209,17 @@ class SendItemCoordinatorTests: BitwardenTestCase { XCTAssertNil(stackNavigator.actions.last) } + /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade + /// complete screen via the billing coordinator. + @MainActor + func test_navigateTo_premiumUpgradeComplete() throws { + subject.navigate(to: .premiumUpgradeComplete) + + let action = try XCTUnwrap(stackNavigator.actions.last) + XCTAssertEqual(action.type, .presented) + XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) + } + /// `navigate(to:)` with `.view` shows the view send screen. @MainActor func test_navigateTo_view() throws { diff --git a/BitwardenShared/UI/Tools/Send/SendItem/SendItemRoute.swift b/BitwardenShared/UI/Tools/Send/SendItem/SendItemRoute.swift index a29c8c7c6c..8e4e0cb97c 100644 --- a/BitwardenShared/UI/Tools/Send/SendItem/SendItemRoute.swift +++ b/BitwardenShared/UI/Tools/Send/SendItem/SendItemRoute.swift @@ -65,6 +65,11 @@ public enum SendItemRoute: Equatable, Hashable { /// A route to the Premium upgrade screen. case premiumUpgrade + /// A route to a standalone Premium upgrade complete screen, shown when an upgrade resolves + /// outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the + /// upgrade screen has already been dismissed). + case premiumUpgradeComplete + /// A route to share the provided URL. /// /// - Parameter url: The `URL` to share. diff --git a/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift b/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift index c554d04890..8f9b5b6b9b 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultCoordinator.swift @@ -269,6 +269,8 @@ final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disa delegate?.switchToSettingsTab(route: .premiumPlan(nil)) case .premiumUpgrade: showPremiumUpgrade() + case .premiumUpgradeComplete: + showPremiumUpgradeCompleteScreen() case let .vaultItemSelection(totpKeyModel): showVaultItemSelection(totpKeyModel: totpKeyModel) case let .viewItem(id, masterPasswordRepromptCheckCompleted): @@ -431,6 +433,17 @@ final class VaultCoordinator: Coordinator, HasStackNavigator { // swiftlint:disa stackNavigator?.present(navigationController) } + /// Shows a standalone Premium upgrade complete screen, for when an upgrade resolves outside + /// of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the upgrade screen + /// has already been dismissed). + /// + private func showPremiumUpgradeCompleteScreen() { + let navigationController = module.makeNavigationController() + let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) + coordinator.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator?.present(navigationController) + } + /// Shows the vault list screen. /// private func showList() { diff --git a/BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift b/BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift index 23dceef66c..c06e012b46 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultCoordinatorTests.swift @@ -413,6 +413,17 @@ class VaultCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this type_ XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgrade]) } + /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade + /// complete screen via the billing coordinator. + @MainActor + func test_navigateTo_premiumUpgradeComplete() throws { + subject.navigate(to: .premiumUpgradeComplete) + + let action = try XCTUnwrap(stackNavigator.actions.last) + XCTAssertEqual(action.type, .presented) + XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) + } + /// `navigate(to:)` with `.switchAccount(userId:, isUnlocked: isUnlocked)`calls the associated delegate method. @MainActor func test_navigateTo_switchAccount() throws { diff --git a/BitwardenShared/UI/Vault/Vault/VaultRoute.swift b/BitwardenShared/UI/Vault/Vault/VaultRoute.swift index bccb72e461..76750ed4bd 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultRoute.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultRoute.swift @@ -77,6 +77,11 @@ public enum VaultRoute: Equatable, Hashable { /// A route to the Premium upgrade view. case premiumUpgrade + /// A route to a standalone Premium upgrade complete screen, shown when an upgrade resolves + /// outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the + /// upgrade screen has already been dismissed). + case premiumUpgradeComplete + /// A route to switch accounts. /// /// - Parameter userId: The user id of the selected account. diff --git a/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinator.swift b/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinator.swift index 7eeaf3e26b..fea8dd7f7a 100644 --- a/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinator.swift +++ b/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinator.swift @@ -143,6 +143,8 @@ class VaultItemCoordinator: NSObject, Coordinator, HasStackNavigator { // swiftl showPasswordHistory(passwordHistory) case .premiumUpgrade: showPremiumUpgrade() + case .premiumUpgradeComplete: + showPremiumUpgradeCompleteScreen() case let .saveFile(temporaryUrl): showSaveFile(temporaryUrl) case .setupTotpManual: @@ -462,6 +464,17 @@ class VaultItemCoordinator: NSObject, Coordinator, HasStackNavigator { // swiftl stackNavigator?.present(navigationController) } + /// Shows a standalone Premium upgrade complete screen, for when an upgrade resolves outside + /// of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the upgrade screen + /// has already been dismissed). + /// + private func showPremiumUpgradeCompleteScreen() { + let navigationController = module.makeNavigationController() + let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) + coordinator.navigate(to: .premiumUpgradeCompleteStandalone) + stackNavigator?.present(navigationController) + } + /// Present the `UIDocumentPickerViewController` that allows users to save the newly downloaded file. /// /// - Parameter temporaryUrl: The temporary url where the file is currently stored. diff --git a/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinatorTests.swift b/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinatorTests.swift index 3691ee813c..4a13176e80 100644 --- a/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinatorTests.swift +++ b/BitwardenShared/UI/Vault/VaultItem/VaultItemCoordinatorTests.swift @@ -456,6 +456,17 @@ class VaultItemCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this t XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgrade]) } + /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade + /// complete screen via the billing coordinator. + @MainActor + func test_navigateTo_premiumUpgradeComplete() throws { + subject.navigate(to: .premiumUpgradeComplete) + + let action = try XCTUnwrap(stackNavigator.actions.last) + XCTAssertEqual(action.type, .presented) + XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) + } + /// `navigate(to:)` with `.setupTotpCamera` with context without conformance fails to present. @MainActor func test_navigateTo_setupTotpCamera_noConformance() async throws { diff --git a/BitwardenShared/UI/Vault/VaultItem/VaultItemRoute.swift b/BitwardenShared/UI/Vault/VaultItem/VaultItemRoute.swift index 05b9fca649..befaecf781 100644 --- a/BitwardenShared/UI/Vault/VaultItem/VaultItemRoute.swift +++ b/BitwardenShared/UI/Vault/VaultItem/VaultItemRoute.swift @@ -88,6 +88,11 @@ enum VaultItemRoute: Equatable, Hashable { /// A route to the Premium upgrade screen. case premiumUpgrade + /// A route to a standalone Premium upgrade complete screen, shown when an upgrade resolves + /// outside of the upgrade screen itself (e.g. a "Sync Now" retry succeeding after the + /// upgrade screen has already been dismissed). + case premiumUpgradeComplete + /// A route to the file saving view. /// /// - Parameter temporaryUrl: The url where the file is currently stored. From 6816378681adb2d9eaa21351a7790b078bb46963 Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Tue, 11 Aug 2026 16:24:48 -0500 Subject: [PATCH 2/3] Pop stale upgrade screen on Settings origin; test the failed-retry boundary MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Settings is the only origin that pushes PremiumUpgradeView rather than presenting it as a fresh modal root, and SettingsCoordinator's .dismiss only dismisses presented view controllers, never pops. The standalone celebration was being presented over a still-visible, now-stale PremiumUpgradeView that has no way to notice premium was granted (its .task-driven self-dismiss check only runs once). Pop it before presenting the celebration so closing returns to the real Settings screen. Also added the missing (isPending: false, lastAttemptFailed: true) case to the Sync Now success test — the existing cases were satisfied by the isPending clause alone, leaving the lastAttemptFailed clause unverified. --- .../UI/Billing/PremiumUpgradeHelperTests.swift | 1 + .../UI/Platform/Settings/SettingsCoordinator.swift | 6 ++++++ .../Platform/Settings/SettingsCoordinatorTests.swift | 12 +++++++----- 3 files changed, 14 insertions(+), 5 deletions(-) diff --git a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift index 631f609904..f2e117d02a 100644 --- a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift +++ b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift @@ -379,6 +379,7 @@ struct PremiumUpgradeHelperTests { // swiftlint:disable:this type_body_length (PremiumUpgradePendingState(isPending: false, lastAttemptFailed: false), true), (PremiumUpgradePendingState(isPending: true, lastAttemptFailed: false), false), (PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true), false), + (PremiumUpgradePendingState(isPending: false, lastAttemptFailed: true), false), ]) func subscribeToPremiumCheckoutStatus_pending_syncNow( pendingState: PremiumUpgradePendingState, diff --git a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift index b4991b2a8b..b1276f4087 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsCoordinator.swift @@ -527,6 +527,12 @@ final class SettingsCoordinator: Coordinator, HasStackNavigator { // swiftlint:d /// screen's treatment of this same screen. /// private func showPremiumUpgradeCompleteScreen() { + // Unlike every other origin, Settings pushes `PremiumUpgradeView` (rather than + // presenting it as a fresh modal root) — reaching this method always means it's still + // the top of this stack, since nothing else pops it. Pop it before presenting the + // celebration so closing the celebration reveals the real Settings screen underneath, + // not a now-stale "Upgrade now" screen that has no way to notice premium was granted. + stackNavigator?.pop(animated: false) let navigationController = module.makeNavigationController() let coordinator = module.makeBillingCoordinator(stackNavigator: navigationController) coordinator.navigate(to: .premiumUpgradeCompleteStandalone) diff --git a/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift b/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift index 3a53c4cbbc..6b88fdf904 100644 --- a/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift +++ b/BitwardenShared/UI/Platform/Settings/SettingsCoordinatorTests.swift @@ -400,15 +400,17 @@ class SettingsCoordinatorTests: BitwardenTestCase { // swiftlint:disable:this ty XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgrade]) } - /// `navigate(to:)` with `.premiumUpgradeComplete` presents a standalone Premium upgrade - /// complete screen via the billing coordinator, unlike `.premiumUpgrade` which pushes onto - /// Settings' own existing stack. + /// `navigate(to:)` with `.premiumUpgradeComplete` pops the pushed Premium upgrade screen — + /// unlike every other origin, Settings pushes rather than presents it, so it's otherwise + /// still visible underneath the celebration and never re-checks premium status on its own — + /// then presents a standalone Premium upgrade complete screen via the billing coordinator. @MainActor func test_navigateTo_premiumUpgradeComplete() throws { subject.navigate(to: .premiumUpgradeComplete) - let action = try XCTUnwrap(stackNavigator.actions.last) - XCTAssertEqual(action.type, .presented) + XCTAssertEqual(stackNavigator.actions.count, 2) + XCTAssertEqual(stackNavigator.actions[0].type, .popped) + XCTAssertEqual(stackNavigator.actions[1].type, .presented) XCTAssertEqual(module.billingCoordinator.routes, [.premiumUpgradeCompleteStandalone]) } From 2bc2ca427a43dea34f4776431a2f9486d0ca311c Mon Sep 17 00:00:00 2001 From: Katherine Bertelsen Date: Wed, 12 Aug 2026 11:45:34 -0500 Subject: [PATCH 3/3] Show celebration screen when Try Again resolves the upgrade, not just Sync Now MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit An end-to-end walkthrough of the whole stack surfaced an inconsistency: the original "Sync Now" button (on the upgrade pending alert) checked whether the retry actually resolved the upgrade and showed the celebration screen on success, but "Try Again" on the "Sync unsuccessful" alert (PR3) only called premiumStatusChanged() and did nothing further — a retry that succeeded there produced only a silent CTA update, with no celebration, even though it's the same kind of explicit, user-initiated retry. Extracted the shared "retry then show celebration if resolved" logic into PremiumUpgradeRetry so both entry points behave identically. --- .../UI/Billing/PremiumUpgradeHelper.swift | 42 ++++++++++++++++-- .../Billing/PremiumUpgradeHelperTests.swift | 12 ++++- .../Vault/VaultList/VaultListProcessor.swift | 8 +++- .../VaultList/VaultListProcessorTests.swift | 44 +++++++++++++++++++ 4 files changed, 99 insertions(+), 7 deletions(-) diff --git a/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift b/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift index 1aa6a70834..82943feb17 100644 --- a/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift +++ b/BitwardenShared/UI/Billing/PremiumUpgradeHelper.swift @@ -29,6 +29,39 @@ extension SettingsRoute: PremiumUpgradeRoute {} extension VaultItemRoute: PremiumUpgradeRoute {} extension VaultRoute: PremiumUpgradeRoute {} +// MARK: - PremiumUpgradeRetry + +/// Shared logic for the two explicit, user-initiated "retry the pending upgrade's sync" entry +/// points — the "Sync Now" button on the upgrade pending alert, and "Try Again" on the "Sync +/// unsuccessful" alert — so both react identically to the retry actually resolving the upgrade. +/// +enum PremiumUpgradeRetry { + /// Calls `retry`, then, if the pending upgrade resolved successfully (no longer pending, and + /// the retry itself didn't fail), navigates to the standalone Premium upgrade complete + /// screen. A still-pending (not yet Premium) or still-failed outcome gets no further UI + /// here — the CTA and the "Sync unsuccessful" alert already react to those independently via + /// the durable `premiumUpgradePendingStatePublisher()` signal. + /// + /// - Parameters: + /// - billingService: The service used to check the resulting state after `retry` runs. + /// - coordinator: The coordinator to navigate on success. + /// - retry: The retry action to perform — `reconcileCheckoutSuccess()` for the "Sync Now" + /// alert, `premiumStatusChanged()` for the "Sync unsuccessful" alert's "Try Again". + /// + @MainActor + static func retryAndShowCompleteIfResolved( + billingService: BillingService, + coordinator: any Coordinator, + retry: () async -> Void, + ) async { + await retry() + let state = await billingService.premiumUpgradePendingState() + if !state.isPending, !state.lastAttemptFailed { + coordinator.navigate(to: .premiumUpgradeComplete) + } + } +} + // MARK: - PremiumUpgradeHelper /// A helper that centralizes the Premium upgrade navigation flow. @@ -172,10 +205,11 @@ class DefaultPremiumUpgradeHelper: PremiumUpg onPendingDismiss?() coordinator.showAlert(.upgradePending { [weak self] in guard let self else { return } - await services.billingService.reconcileCheckoutSuccess() - let state = await services.billingService.premiumUpgradePendingState() - if !state.isPending, !state.lastAttemptFailed { - coordinator.navigate(to: .premiumUpgradeComplete) + await PremiumUpgradeRetry.retryAndShowCompleteIfResolved( + billingService: services.billingService, + coordinator: coordinator, + ) { + await self.services.billingService.reconcileCheckoutSuccess() } }) } diff --git a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift index f2e117d02a..d977448e8f 100644 --- a/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift +++ b/BitwardenShared/UI/Billing/PremiumUpgradeHelperTests.swift @@ -388,9 +388,16 @@ struct PremiumUpgradeHelperTests { // swiftlint:disable:this type_body_length billingRepository.isInAppUpgradeAvailableReturnValue = true let statusSubject = PassthroughSubject() billingService.premiumCheckoutStatusPublisherReturnValue = statusSubject.eraseToAnyPublisher() - billingService.premiumUpgradePendingStateReturnValue = pendingState + // Not pending yet for `startInAppPremiumUpgrade`'s own already-pending check, so it + // navigates to `.premiumUpgrade` normally; `pendingState` only reflects the result of the + // "Sync Now" retry itself, checked afterward. + billingService.premiumUpgradePendingStateReturnValue = PremiumUpgradePendingState( + isPending: false, + lastAttemptFailed: false, + ) let subject = makeSubject() await subject.navigateToPremiumUpgrade() + try await waitForAsync { coordinator.routes.last == .premiumUpgrade } statusSubject.send(.pending) try await waitForAsync { @@ -401,6 +408,7 @@ struct PremiumUpgradeHelperTests { // swiftlint:disable:this type_body_length Issue.record("Expected .dismiss route") return } + billingService.premiumUpgradePendingStateReturnValue = pendingState action?.action() let alert = try #require(coordinator.alertShown.last) @@ -436,4 +444,4 @@ struct PremiumUpgradeHelperTests { // swiftlint:disable:this type_body_length #expect(coordinator.routes.count == routeCountBeforeSend) } -} +} // swiftlint:disable:this file_length diff --git a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift index a78f74af0d..5211a16f69 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessor.swift @@ -763,7 +763,13 @@ extension VaultListProcessor { // transition rather than a duplicate, or this explicit, user-initiated // retry would fail with no feedback at all. lastAttemptFailed = false - await self?.services.billingService.premiumStatusChanged() + guard let self else { return } + await PremiumUpgradeRetry.retryAndShowCompleteIfResolved( + billingService: services.billingService, + coordinator: coordinator, + ) { + await self.services.billingService.premiumStatusChanged() + } }) } lastAttemptFailed = pendingState.lastAttemptFailed diff --git a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift index dab1da89ae..865efc71b0 100644 --- a/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift +++ b/BitwardenShared/UI/Vault/Vault/VaultList/VaultListProcessorTests.swift @@ -1321,6 +1321,10 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true), ) billingService.premiumUpgradePendingStatePublisherReturnValue = pendingStateSubject.eraseToAnyPublisher() + billingService.premiumUpgradePendingStateReturnValue = PremiumUpgradePendingState( + isPending: true, + lastAttemptFailed: false, + ) let task = Task { await subject.perform(.streamPremiumUpgradePendingState) @@ -1334,6 +1338,41 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ try await alert.tapAction(title: Localizations.tryAgain) } waitFor(billingService.premiumStatusChangedCallsCount == 1) + + // Still pending (not yet Premium) — no celebration screen, matching "Sync Now"'s own + // still-pending boundary. + XCTAssertNotEqual(coordinator.routes.last, .premiumUpgradeComplete) + } + + /// `perform(_:)` with `.streamPremiumUpgradePendingState` navigates to the standalone Premium + /// upgrade complete screen when the user's own "Try again" retry actually resolves the + /// pending upgrade — matching "Sync Now"'s own behavior on the pending alert, so the two + /// explicit retry entry points behave identically. + @MainActor + func test_perform_streamPremiumUpgradePendingState_syncUnsuccessfulAlert_tryAgainResolves() throws { + let pendingStateSubject = CurrentValueSubject( + PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true), + ) + billingService.premiumUpgradePendingStatePublisherReturnValue = pendingStateSubject.eraseToAnyPublisher() + billingService.premiumUpgradePendingStateReturnValue = PremiumUpgradePendingState( + isPending: false, + lastAttemptFailed: false, + ) + + let task = Task { + await subject.perform(.streamPremiumUpgradePendingState) + } + defer { task.cancel() } + + waitFor(!coordinator.alertShown.isEmpty) + let alert = try XCTUnwrap(coordinator.alertShown.last) + + Task { + try await alert.tapAction(title: Localizations.tryAgain) + } + waitFor(coordinator.routes.last == .premiumUpgradeComplete) + + XCTAssertEqual(billingService.premiumStatusChangedCallsCount, 1) } /// `perform(_:)` with `.streamPremiumUpgradePendingState` re-shows the "Sync Unsuccessful" @@ -1345,6 +1384,10 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true), ) billingService.premiumUpgradePendingStatePublisherReturnValue = pendingStateSubject.eraseToAnyPublisher() + billingService.premiumUpgradePendingStateReturnValue = PremiumUpgradePendingState( + isPending: true, + lastAttemptFailed: true, + ) billingService.premiumStatusChangedClosure = { pendingStateSubject.send(PremiumUpgradePendingState(isPending: true, lastAttemptFailed: true)) } @@ -1363,6 +1406,7 @@ class VaultListProcessorTests: BitwardenTestCase { // swiftlint:disable:this typ waitFor(coordinator.alertShown.count == 2) XCTAssertEqual(coordinator.alertShown.last?.title, Localizations.syncUnsuccessful) + XCTAssertNotEqual(coordinator.routes.last, .premiumUpgradeComplete) } /// `perform(_:)` with `.streamShowWebIcons` requests the value of the show