Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -1142,6 +1142,12 @@
"OpeningCheckout" = "Opening checkout…";
"UpgradePending" = "Upgrade pending";
"YourUpgradeIsBeingProcessedDescriptionLong" = "Your upgrade is being processed. You'll remain on the free plan until it's complete. Sync now to check, or continue and it will update automatically.";
/* Generic button label for declining an action without dismissing it permanently. */
"NotNow" = "Not now";
/* Alert title shown when a sync attempt for a pending Premium upgrade fails. */
"SyncUnsuccessful" = "Sync unsuccessful";
/* Alert message shown when a sync attempt for a pending Premium upgrade fails. */
"WeCouldntSyncYourVaultWithTheServerDescriptionLong" = "We couldn't sync your vault with the server. Your vault may not reflect your latest changes. You can try again now or it will sync automatically later.";
"UpgradedToPremium" = "Upgraded to Premium";
"SubscriptionNeedsAttention" = "Your subscription needs attention";
"CheckYourPlanForDetails" = "Check your plan for details.";
Expand Down
31 changes: 28 additions & 3 deletions BitwardenShared/Core/Billing/Services/BillingService.swift
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,19 @@ class DefaultBillingService: BillingService {

self.currentSyncSubscriber = Task {
guard let publisher = try? await self.stateService.lastSyncTimePublisher() else { return }
for await _ in publisher.values {
// Snapshot the last-known sync time directly, rather than relying on
// whichever value the publisher happens to deliver first: its
// `CurrentValueSubject` backing replays the existing cached value
// immediately on subscribe (not evidence that a sync just happened), and a
// *different* account's sync can also re-emit this shared store without this
// account's own value having changed. Comparing each emission against the
// last value actually seen β€” rather than its position in the stream β€” means
// only a genuinely new sync for this account reaches
// `reconcilePendingUpgradeIfNeeded()`, regardless of subscription timing.
var lastSeenDate = try? await self.stateService.getLastSyncTime()
for await date in publisher.values {
guard date != lastSeenDate else { continue }
lastSeenDate = date
await self.reconcilePendingUpgradeIfNeeded()
}
}
Expand All @@ -398,11 +410,24 @@ class DefaultBillingService: BillingService {
return
}

guard await stateService.doesActiveAccountHavePremium() else { return }
// Reaching this point at all means a *new* sync just completed successfully β€” `start()`
// filters out both the initial replay and same-value re-emissions before invoking this
// method, and `lastSyncTimePublisher` never fires on failure β€” so the most recent
// attempt did not fail. Clear that regardless of whether the account has become Premium
// yet, so a stale failure doesn't linger after a later, unrelated sync has succeeded.
do {
try await billingStateService.setPremiumUpgradeLastSyncAttemptFailed(false)
} catch {
errorReporter.log(error: error)
}

guard await stateService.doesActiveAccountHavePremium() else {
await refreshPremiumUpgradePendingStateSubject()
return
}

do {
try await billingStateService.setPremiumUpgradePending(false)
try await billingStateService.setPremiumUpgradeLastSyncAttemptFailed(false)
try await billingStateService.setUpgradedToPremiumActionCardVisible(true)
} catch {
errorReporter.log(error: error)
Expand Down
55 changes: 55 additions & 0 deletions BitwardenShared/Core/Billing/Services/BillingServiceTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -608,6 +608,59 @@ struct BillingServiceTests { // swiftlint:disable:this type_body_length

// MARK: start()

/// `start()` clears a stale `lastAttemptFailed` flag as soon as any sync succeeds, even
/// when the active account still isn't Premium yet β€” a later, unrelated sync succeeding
/// means the most recent attempt did not fail, regardless of whether Premium has been
/// granted.
@Test
func start_clearsLastAttemptFailedOnGenericSyncEvenWithoutPremium() async throws {
stateService.activeAccount = .fixture()
stateService.doesActiveAccountHavePremiumResult = false

await subject.start()
// Wait for the subscription's baseline snapshot before sending a "new" value below β€”
// `start()` reads `getLastSyncTime()` exactly once, before subscribing to the publisher.
// Without this, the publisher's `CurrentValueSubject` backing could coalesce an
// immediate send with the not-yet-taken snapshot, making it ambiguous whether the send
// counts as a change (both nested `Task`s run on the cooperative pool, so a fixed
// number of `Task.yield()` calls from this `@MainActor` test isn't a reliable proxy).
try await waitForAsync { stateService.getLastSyncTimeCallCount == 1 }

stateService.premiumUpgradePendingResult = true
stateService.premiumUpgradeLastSyncAttemptFailedResult = true
stateService.lastSyncTimeSubject.send(Date())

try await waitForAsync { stateService.premiumUpgradeLastSyncAttemptFailedResult == false }
#expect(stateService.premiumUpgradePendingResult == true)
#expect(stateService.upgradedToPremiumActionCardVisibleResult == false)
}

/// `start()` does not clear a stale `lastAttemptFailed` flag just from subscribing to the
/// last-sync-time publisher β€” its `CurrentValueSubject` backing replays the existing cached
/// value immediately on subscribe, and that replay must not be mistaken for a new sync
/// completing.
@Test
func start_doesNotClearLastAttemptFailedOnInitialSubscriptionReplay() async throws {
stateService.activeAccount = .fixture()
stateService.doesActiveAccountHavePremiumResult = true
stateService.lastSyncTimeSubject.send(Date())
stateService.premiumUpgradePendingResult = true
stateService.premiumUpgradeLastSyncAttemptFailedResult = true

await subject.start()
try await waitForAsync { stateService.getLastSyncTimeCallCount == 1 }

#expect(stateService.premiumUpgradeLastSyncAttemptFailedResult == true)
#expect(stateService.setPremiumUpgradeLastSyncAttemptFailedCallCount == 0)

// A genuinely new sync, sent only once the initial subscription has settled, still
// clears the flag as expected.
stateService.lastSyncTimeSubject.send(Date())

try await waitForAsync { stateService.premiumUpgradeLastSyncAttemptFailedResult == false }
#expect(stateService.setPremiumUpgradeLastSyncAttemptFailedCallCount == 1)
}

/// `start()` resolves a pending Premium upgrade when a generic sync completes and the
/// active account has since become Premium, by any means (not just the original checkout
/// attempt's own subscription).
Expand All @@ -617,6 +670,8 @@ struct BillingServiceTests { // swiftlint:disable:this type_body_length
stateService.doesActiveAccountHavePremiumResult = false

await subject.start()
// See the comment in `start_clearsLastAttemptFailedOnGenericSyncEvenWithoutPremium()`.
try await waitForAsync { stateService.getLastSyncTimeCallCount == 1 }

stateService.premiumUpgradePendingResult = true
stateService.premiumUpgradeLastSyncAttemptFailedResult = true
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS
var premiumUpgradeLastSyncAttemptFailedResult: Bool = false // swiftlint:disable:this identifier_name
var premiumUpgradePendingResult: Bool = false
// swiftlint:disable:next identifier_name
var setPremiumUpgradeLastSyncAttemptFailedCallCount = 0
// swiftlint:disable:next identifier_name
var setPremiumUpgradeLastSyncAttemptFailedResult: Result<Void, Error> = .success(())
var setPremiumUpgradePendingResult: Result<Void, Error> = .success(())
var setSubscriptionAttentionCardResult: Result<Void, Error> = .success(())
Expand Down Expand Up @@ -93,6 +95,7 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS
var getAccountHasBeenUnlockedInteractivelyResult: Result<Bool, Error> = .success(false)
var getActiveAccountIdError: Error?
var getBiometricAuthenticationEnabledResult: Result<Void, Error> = .success(())
var getLastSyncTimeCallCount = 0
var lastRequestToTurnOnCredentialProvider: Date?
var lastSyncMonotonicTimeByUserId = [String: TimeInterval?]()
var lastSyncTimeByUserId = [String: Date]()
Expand Down Expand Up @@ -389,8 +392,13 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS
}

func getLastSyncTime(userId: String?) async throws -> Date? {
getLastSyncTimeCallCount += 1
let userId = try unwrapUserId(userId)
return lastSyncTimeByUserId[userId]
// Falls back to `lastSyncTimeSubject` for tests that drive it directly via `.send(_:)`
// instead of going through `setLastSyncTime(_:userId:)`, which keeps both stores in
// sync (mirroring `DefaultStateService`, where both calls read from the same
// underlying store).
return lastSyncTimeByUserId[userId] ?? lastSyncTimeSubject.value
}

func getLastSyncMonotonicTime(userId: String?) async throws -> TimeInterval? {
Expand Down Expand Up @@ -663,6 +671,7 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS
}

func setPremiumUpgradeLastSyncAttemptFailed(_ failed: Bool) async throws {
setPremiumUpgradeLastSyncAttemptFailedCallCount += 1
try setPremiumUpgradeLastSyncAttemptFailedResult.get()
premiumUpgradeLastSyncAttemptFailedResult = failed
}
Expand Down Expand Up @@ -775,6 +784,7 @@ class MockStateService: StateService, ActiveAccountStateProvider, AutofillStateS
func setLastSyncTime(_ date: Date?, userId: String?) async throws {
let userId = try unwrapUserId(userId)
lastSyncTimeByUserId[userId] = date
lastSyncTimeSubject.value = date
}

func getLastUserShouldConnectToWatch() async -> Bool {
Expand Down
27 changes: 27 additions & 0 deletions BitwardenShared/UI/Billing/Extensions/Alert+Billing.swift
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,33 @@ extension Alert {
)
}

/// An alert shown when a sync attempt for a pending Premium upgrade fails.
///
/// - Parameter tryAgainHandler: A closure called when the user taps "Try again".
/// - Returns: An `Alert` with "Not now" and "Try again" actions.
///
static func syncUnsuccessful(
tryAgainHandler: @escaping () async -> Void,
) -> Alert {
Alert(
title: Localizations.syncUnsuccessful,
message: Localizations.weCouldntSyncYourVaultWithTheServerDescriptionLong,
alertActions: [
AlertAction(
title: Localizations.notNow,
style: .cancel,
),
AlertAction(
title: Localizations.tryAgain,
style: .default,
handler: { _, _ in
await tryAgainHandler()
},
),
],
)
}

/// An alert shown when a Premium upgrade is still being processed.
///
/// - Parameter syncNowHandler: A closure called when the user taps "Sync now".
Expand Down
18 changes: 18 additions & 0 deletions BitwardenShared/UI/Billing/Extensions/Alert+BillingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,24 @@ class AlertBillingTests: BitwardenTestCase {
XCTAssertEqual(tryAgainAction.style, .default)
}

/// `syncUnsuccessful(tryAgainHandler:)` builds an `Alert` with the correct title, message, and actions.
func test_syncUnsuccessful() {
let subject = Alert.syncUnsuccessful {}

XCTAssertEqual(subject.title, Localizations.syncUnsuccessful)
XCTAssertEqual(subject.message, Localizations.weCouldntSyncYourVaultWithTheServerDescriptionLong)
XCTAssertEqual(subject.preferredStyle, .alert)
XCTAssertEqual(subject.alertActions.count, 2)

let notNowAction = subject.alertActions[0]
XCTAssertEqual(notNowAction.title, Localizations.notNow)
XCTAssertEqual(notNowAction.style, .cancel)

let tryAgainAction = subject.alertActions[1]
XCTAssertEqual(tryAgainAction.title, Localizations.tryAgain)
XCTAssertEqual(tryAgainAction.style, .default)
}

/// `upgradePending(syncNowHandler:)` builds an `Alert` with the correct title, message, and actions.
func test_upgradePending() {
let subject = Alert.upgradePending {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -703,11 +703,30 @@ extension VaultListProcessor {
/// Streams live updates to the Premium upgrade pending state, hiding or re-evaluating the
/// upsell action card immediately rather than waiting for the next screen appearance β€”
/// needed because dismissing the "Upgrade Pending" alert returns to this same screen
/// without a fresh `.appeared`.
/// without a fresh `.appeared`. Also shows the "Sync Unsuccessful" alert the first time a
/// sync attempt for the pending upgrade fails.
///
private func streamPremiumUpgradePendingState() async {
var lastAttemptFailed = false
for await pendingState in services.billingService.premiumUpgradePendingStatePublisher().values {
await updatePremiumUpgradeActionCardVisibility(isPending: pendingState.isPending)

// Only show the alert on a false-to-true transition β€” the publisher replays its
// current value to this subscription immediately (covering a stale failure still
// persisted from a previous app session), but without tracking the transition, any
// later emission that still has `lastAttemptFailed: true` (e.g. triggered by an
// unrelated `isPending` change) would incorrectly re-show the alert.
if pendingState.lastAttemptFailed, !lastAttemptFailed {
coordinator.showAlert(.syncUnsuccessful { [weak self] in
// Reset the tracker before retrying β€” if the retry's own sync also fails,
// the resulting `lastAttemptFailed: true` emission must be treated as a new
// 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()
})
}
lastAttemptFailed = pendingState.lastAttemptFailed
}
}

Expand Down
Loading
Loading