From d1b7986127a93d7f1447ed2707fc715819ac0b76 Mon Sep 17 00:00:00 2001 From: Jon Petersson Date: Wed, 21 Aug 2024 15:05:23 +0200 Subject: [PATCH] Fix out-of-time notifications --- .../xcshareddata/swiftpm/Package.resolved | 3 +- .../AccountExpiry.swift | 85 +++++++++--- ...countExpiryInAppNotificationProvider.swift | 41 ++++-- ...ountExpirySystemNotificationProvider.swift | 126 ++++++++++++++---- .../NotificationConfiguration.swift | 12 +- .../TunnelManager/SetAccountOperation.swift | 7 +- .../TunnelManager/TunnelManager.swift | 6 +- .../AccountExpiryTests.swift | 64 ++++++--- 8 files changed, 256 insertions(+), 88 deletions(-) diff --git a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved index b7c6470358f7..8f79a7a39c60 100644 --- a/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved +++ b/ios/MullvadVPN.xcodeproj/project.xcworkspace/xcshareddata/swiftpm/Package.resolved @@ -1,4 +1,5 @@ { + "originHash" : "c15149b2d59d9e9c72375f65339c04f41a19943e1117e682df27fc9f943fdc56", "pins" : [ { "identity" : "swift-log", @@ -18,5 +19,5 @@ } } ], - "version" : 2 + "version" : 3 } diff --git a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift index e38b0fd3d57f..ac687a36a247 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift @@ -7,37 +7,80 @@ // import Foundation +import MullvadTypes struct AccountExpiry { + enum Trigger { + case system, inApp + + var dateIntervals: [Int] { + switch self { + case .system: + NotificationConfiguration.closeToExpirySystemTriggerIntervals + case .inApp: + NotificationConfiguration.closeToExpiryInAppTriggerIntervals + } + } + } + + private let calendar = Calendar.current + var expiryDate: Date? - var triggerDate: Date? { - guard let expiryDate else { return nil } + func nextTriggerDate(for trigger: Trigger) -> Date? { + let now = Date().secondsPrecision + let triggerDates = triggerDates(for: trigger) + + // Get earliest trigger date and remove one day. Since we want to count whole days, If first + // notification should trigger 3 days before account expiry, we need to start checking when + // there's (less than) 4 days left. + guard + let expiryDate, + let earliestDate = triggerDates.min(), + let earliestTriggerDate = calendar.date(byAdding: .day, value: -1, to: earliestDate), + now <= expiryDate.secondsPrecision, + now > earliestTriggerDate.secondsPrecision + else { return nil } + + let datesByTimeToTrigger = triggerDates.filter { date in + now.secondsPrecision <= date.secondsPrecision // Ignore dates that have passed. + }.sorted { date1, date2 in + abs(date1.timeIntervalSince(now)) < abs(date2.timeIntervalSince(now)) + } - return Calendar.current.date( - byAdding: .day, - value: -NotificationConfiguration.closeToExpiryTriggerInterval, - to: expiryDate + return datesByTimeToTrigger.first + } + + func daysRemaining(for trigger: Trigger) -> DateComponents? { + let nextTriggerDate = nextTriggerDate(for: trigger) + guard let expiryDate, let nextTriggerDate else { return nil } + + let dateComponents = calendar.dateComponents( + [.day], + from: Date().secondsPrecision, + to: max(nextTriggerDate, expiryDate).secondsPrecision ) + + return dateComponents } - var formattedDuration: String? { - let now = Date() + func triggerDates(for trigger: Trigger) -> [Date] { + guard let expiryDate else { return [] } - guard - let expiryDate, - let triggerDate, - let duration = CustomDateComponentsFormatting.localizedString( - from: now, - to: expiryDate, - unitsStyle: .full - ), - now >= triggerDate, - now < expiryDate - else { - return nil + let dates = trigger.dateIntervals.compactMap { + calendar.date( + byAdding: .day, + value: -$0, + to: expiryDate + ) } - return duration + return dates + } +} + +private extension Date { + var secondsPrecision: Date { + Date(timeIntervalSince1970: TimeInterval(Int(timeIntervalSince1970))) } } diff --git a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift index 04658f0da977..87317866272b 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift @@ -38,24 +38,18 @@ final class AccountExpiryInAppNotificationProvider: NotificationProvider, InAppN // MARK: - InAppNotificationProvider var notificationDescriptor: InAppNotificationDescriptor? { - guard let duration = accountExpiry.formattedDuration else { + guard let durationText = remainingDaysText else { return nil } return InAppNotificationDescriptor( identifier: identifier, style: .warning, - title: NSLocalizedString( - "ACCOUNT_EXPIRY_INAPP_NOTIFICATION_TITLE", - value: "ACCOUNT CREDIT EXPIRES SOON", - comment: "Title for in-app notification, displayed within the last 3 days until account expiry." - ), - body: .init(string: String( - format: NSLocalizedString( - "ACCOUNT_EXPIRY_INAPP_NOTIFICATION_BODY", - value: "%@ left. Buy more credit.", - comment: "Message for in-app notification, displayed within the last 3 days until account expiry." - ), duration + title: durationText, + body: NSAttributedString(string: NSLocalizedString( + "ACCOUNT_EXPIRY_IN_APP_NOTIFICATION_BODY", + value: "You can add more time via the account view or website to continue using the VPN.", + comment: "Title for in-app notification, displayed within the last X days until account expiry." )) ) } @@ -75,7 +69,7 @@ final class AccountExpiryInAppNotificationProvider: NotificationProvider, InAppN private func updateTimer() { timer?.cancel() - guard let triggerDate = accountExpiry.triggerDate else { + guard let triggerDate = accountExpiry.nextTriggerDate(for: .inApp) else { return } @@ -105,3 +99,24 @@ final class AccountExpiryInAppNotificationProvider: NotificationProvider, InAppN invalidate() } } + +extension AccountExpiryInAppNotificationProvider { + private var remainingDaysText: String? { + guard + let expiryDate = accountExpiry.expiryDate, + let nextTriggerDate = accountExpiry.nextTriggerDate(for: .inApp), + let duration = CustomDateComponentsFormatting.localizedString( + from: nextTriggerDate, + to: expiryDate, + unitsStyle: .full + ) + else { return nil } + + return String(format: NSLocalizedString( + "ACCOUNT_EXPIRY_IN_APP_NOTIFICATION_TITLE", + tableName: "AccountExpiry", + value: "%@ left on this account", + comment: "Message for in-app notification, displayed within the last X days until account expiry." + ), duration).uppercased() + } +} diff --git a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift index b360fa9cf5e8..2a718fc3adfe 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift @@ -11,8 +11,9 @@ import MullvadSettings import UserNotifications final class AccountExpirySystemNotificationProvider: NotificationProvider, SystemNotificationProvider { - private var accountExpiry: Date? + private var accountExpiry = AccountExpiry() private var tunnelObserver: TunnelBlockObserver? + private var accountHasRecentlyExpired = false init(tunnelManager: TunnelManager) { super.init() @@ -21,8 +22,16 @@ final class AccountExpirySystemNotificationProvider: NotificationProvider, Syste didLoadConfiguration: { [weak self] tunnelManager in self?.invalidate(deviceState: tunnelManager.deviceState) }, - didUpdateDeviceState: { [weak self] _, deviceState, _ in - self?.invalidate(deviceState: deviceState) + didUpdateDeviceState: { [weak self] tunnelManager, deviceState, previousDeviceState in + guard let self else { return } + + checkAccountExpiry( + tunnelStatus: tunnelManager.tunnelStatus, + deviceState: deviceState, + previousDeviceState: previousDeviceState + ) + + invalidate(deviceState: tunnelManager.deviceState) } ) @@ -38,21 +47,21 @@ final class AccountExpirySystemNotificationProvider: NotificationProvider, Syste // MARK: - SystemNotificationProvider var notificationRequest: UNNotificationRequest? { - guard let trigger else { return nil } + let trigger = accountHasRecentlyExpired ? triggerExpiry : triggerCloseToExpiry + + guard let trigger, let durationText = formattedRemainingDuration else { + return nil + } let content = UNMutableNotificationContent() content.title = NSLocalizedString( "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_TITLE", tableName: "AccountExpiry", value: "Account credit expires soon", - comment: "Title for system account expiry notification, fired 3 days prior to account expiry." - ) - content.body = NSLocalizedString( - "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_BODY", - tableName: "AccountExpiry", - value: "Account credit expires in 3 days. Buy more credit.", - comment: "Message for system account expiry notification, fired 3 days prior to account expiry." + comment: "Title for system account expiry notification, fired X days prior to account expiry." ) + + content.body = durationText content.sound = UNNotificationSound.default return UNNotificationRequest( @@ -74,33 +83,102 @@ final class AccountExpirySystemNotificationProvider: NotificationProvider, Syste // MARK: - Private - private var trigger: UNNotificationTrigger? { - guard let accountExpiry else { return nil } + private var triggerCloseToExpiry: UNNotificationTrigger? { + guard let triggerDate = accountExpiry.nextTriggerDate(for: .system) else { return nil } - guard let triggerDate = Calendar.current.date( - byAdding: .day, - value: -NotificationConfiguration.closeToExpiryTriggerInterval, - to: accountExpiry - ) else { return nil } + let dateComponents = Calendar.current.dateComponents( + [.second, .minute, .hour, .day, .month, .year], + from: triggerDate + ) - // Do not produce notification if less than 3 days left till expiry - guard triggerDate > Date() else { return nil } + return UNCalendarNotificationTrigger(dateMatching: dateComponents, repeats: false) + } - // Create date components for calendar trigger + private var triggerExpiry: UNNotificationTrigger { let dateComponents = Calendar.current.dateComponents( [.second, .minute, .hour, .day, .month, .year], - from: triggerDate + from: Date().addingTimeInterval(1) // Give some leeway. ) return UNCalendarNotificationTrigger(dateMatching: dateComponents, repeats: false) } private var shouldRemovePendingOrDeliveredRequests: Bool { - accountExpiry == nil + return accountExpiry.expiryDate == nil + } + + private func checkAccountExpiry( + tunnelStatus: TunnelStatus, + deviceState: DeviceState, + previousDeviceState: DeviceState + ) { + var blockedStateByExpiredAccount = false + if case .accountExpired = tunnelStatus.observedState.blockedState?.reason { + blockedStateByExpiredAccount = true + } + + let accountHasExpired = deviceState.accountData?.isExpired == true + let accountHasRecentlyExpired = deviceState.accountData?.isExpired != previousDeviceState.accountData?.isExpired + + self.accountHasRecentlyExpired = blockedStateByExpiredAccount && accountHasExpired && accountHasRecentlyExpired } private func invalidate(deviceState: DeviceState) { - accountExpiry = deviceState.accountData?.expiry + accountExpiry.expiryDate = deviceState.accountData?.expiry invalidate() } } + +extension AccountExpirySystemNotificationProvider { + private var formattedRemainingDuration: String? { + if accountHasRecentlyExpired { + return expiredText + } + + switch accountExpiry.daysRemaining(for: .system)?.day { + case .none: + return nil + case 1: + return singleDayText + default: + return multipleDaysText + } + } + + private var expiredText: String { + NSLocalizedString( + "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_BODY", + tableName: "AccountExpiry", + value: "Blocking internet: Your time on this account has expired. To continue using the.", + comment: "Message for in-app notification, displayed on account expiry while connected to VPN." + ) + } + + private var singleDayText: String { + NSLocalizedString( + "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_BODY", + tableName: "AccountExpiry", + value: "You have one day left on this account. Please add more time to continue using the VPN.", + comment: "Message for in-app notification, displayed within the last 1 day until account expiry." + ) + } + + private var multipleDaysText: String? { + guard + let expiryDate = accountExpiry.expiryDate, + let nextTriggerDate = accountExpiry.nextTriggerDate(for: .system), + let duration = CustomDateComponentsFormatting.localizedString( + from: nextTriggerDate, + to: expiryDate, + unitsStyle: .full + ) + else { return nil } + + return String(format: NSLocalizedString( + "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_BODY", + tableName: "AccountExpiry", + value: "You have %@ left on this account.", + comment: "Message for in-app notification, displayed within the last X days until account expiry." + ), duration.lowercased()) + } +} diff --git a/ios/MullvadVPN/Notifications/Notification Providers/NotificationConfiguration.swift b/ios/MullvadVPN/Notifications/Notification Providers/NotificationConfiguration.swift index 98e1913608bf..64c9697e03ec 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/NotificationConfiguration.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/NotificationConfiguration.swift @@ -10,10 +10,16 @@ import Foundation enum NotificationConfiguration { /** - Duration measured in days, before the account expiry, when notification is scheduled to remind user to add more - time on account. + Duration measured in days, before the account expiry, when a system notification is scheduled to remind user + to add more time on account. */ - static let closeToExpiryTriggerInterval = 3 + static let closeToExpirySystemTriggerIntervals = [3, 1] + + /** + Duration measured in days, before the account expiry, when an in-app notification is scheduled to remind user + to add more time on account. + */ + static let closeToExpiryInAppTriggerIntervals: [Int] = [3, 2, 1, 0] /** Time interval measured in seconds at which to refresh account expiry in-app notification, which reformats diff --git a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift index db6e1cc81436..3f66743182f4 100644 --- a/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift +++ b/ios/MullvadVPN/TunnelManager/SetAccountOperation.swift @@ -347,8 +347,11 @@ class SetAccountOperation: ResultOperation { 3. Remove VPN configuration and release an instance of `Tunnel` object. */ private func unsetDeviceState(completion: @escaping () -> Void) { - // Tell the caller to unsubscribe from VPN status notifications. - interactor.prepareForVPNConfigurationDeletion() + completion() + return + + // Tell the caller to unsubscribe from VPN status notifications. + interactor.prepareForVPNConfigurationDeletion() // Reset tunnel and device state. interactor.updateTunnelStatus { tunnelStatus in diff --git a/ios/MullvadVPN/TunnelManager/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift index 2d663a048fb0..78d553bcd4a3 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift @@ -1109,7 +1109,7 @@ final class TunnelManager: StorePaymentObserver { extension TunnelManager { enum AccountExpirySimulationOption { - case closeToExpiry + case closeToExpiry(days: Int) case expired case active @@ -1121,9 +1121,9 @@ extension TunnelManager { case .active: return calendar.date(byAdding: .year, value: 1, to: now) - case .closeToExpiry: + case let .closeToExpiry(days): return calendar.date( - byAdding: DateComponents(day: NotificationConfiguration.closeToExpiryTriggerInterval, second: 5), + byAdding: DateComponents(day: days, second: 5), to: now ) diff --git a/ios/MullvadVPNTests/MullvadVPN/Notifications/NotificationProviders/AccountExpiryTests.swift b/ios/MullvadVPNTests/MullvadVPN/Notifications/NotificationProviders/AccountExpiryTests.swift index cb06ab92783d..08c5ef9153a6 100644 --- a/ios/MullvadVPNTests/MullvadVPN/Notifications/NotificationProviders/AccountExpiryTests.swift +++ b/ios/MullvadVPNTests/MullvadVPN/Notifications/NotificationProviders/AccountExpiryTests.swift @@ -9,40 +9,62 @@ import XCTest class AccountExpiryTests: XCTestCase { - func testNoDateReturnsNoDuration() { + private let calendar = Calendar.current + + func testNoDateDuration() { let accountExpiry = AccountExpiry() - XCTAssertNil(accountExpiry.formattedDuration) + XCTAssertNil(accountExpiry.nextTriggerDate(for: .system)) + XCTAssertNil(accountExpiry.nextTriggerDate(for: .inApp)) } - func testDateNowReturnsNoDuration() { + func testDateNowDuration() { let accountExpiry = AccountExpiry(expiryDate: Date()) - XCTAssertNil(accountExpiry.formattedDuration) + XCTAssertNil(accountExpiry.nextTriggerDate(for: .system)) + XCTAssertNotNil(accountExpiry.nextTriggerDate(for: .inApp)) // In-app expiry triggers on same date as well. } - func testDateInPastReturnsNoDuration() { + func testDateInPastDuration() { let accountExpiry = AccountExpiry(expiryDate: Date().addingTimeInterval(-10)) - XCTAssertNil(accountExpiry.formattedDuration) + XCTAssertNil(accountExpiry.nextTriggerDate(for: .system)) + XCTAssertNil(accountExpiry.nextTriggerDate(for: .inApp)) } - func testDateWithinTriggerIntervalReturnsDuration() { - let date = Calendar.current.date( - byAdding: .day, - value: NotificationConfiguration.closeToExpiryTriggerInterval - 1, - to: Date() - ) + func testDateInFutureDuration() { + let accountExpiry = AccountExpiry(expiryDate: calendar.date(byAdding: .day, value: 1, to: Date())) - let accountExpiry = AccountExpiry(expiryDate: date) - XCTAssertNotNil(accountExpiry.formattedDuration) + XCTAssertNotNil(accountExpiry.nextTriggerDate(for: .system)) + XCTAssertNotNil(accountExpiry.nextTriggerDate(for: .inApp)) } - func testDateNotWithinTriggerIntervalReturnsNoDuration() { - let date = Calendar.current.date( - byAdding: .day, - value: NotificationConfiguration.closeToExpiryTriggerInterval + 1, - to: Date() + func testNumberOfTriggerDates() { + var accountExpiry = AccountExpiry( + expiryDate: calendar.date( + byAdding: .day, + value: AccountExpiry.Trigger.system.dateIntervals.max()!, + to: Date() + ) + ) + XCTAssertEqual(accountExpiry.triggerDates(for: .system).count, AccountExpiry.Trigger.system.dateIntervals.count) + + accountExpiry = AccountExpiry( + expiryDate: calendar.date( + byAdding: .day, + value: AccountExpiry.Trigger.inApp.dateIntervals.max()!, + to: Date() + ) ) + XCTAssertEqual(accountExpiry.triggerDates(for: .inApp).count, AccountExpiry.Trigger.inApp.dateIntervals.count) + } + + func testDaysRemaining() { + AccountExpiry.Trigger.system.dateIntervals.forEach { interval in + let accountExpiry = AccountExpiry(expiryDate: calendar.date(byAdding: .day, value: interval, to: Date())) + XCTAssertEqual(accountExpiry.daysRemaining(for: .system)?.day, interval) + } - let accountExpiry = AccountExpiry(expiryDate: date) - XCTAssertNil(accountExpiry.formattedDuration) + AccountExpiry.Trigger.inApp.dateIntervals.forEach { interval in + let accountExpiry = AccountExpiry(expiryDate: calendar.date(byAdding: .day, value: interval, to: Date())) + XCTAssertEqual(accountExpiry.daysRemaining(for: .inApp)?.day, interval) + } } }