From 2e2c31801d2795e076985b758936a4700b69f159 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 --- .../AccountExpiry.swift | 91 ++++++++--- ...countExpiryInAppNotificationProvider.swift | 50 +++--- ...ountExpirySystemNotificationProvider.swift | 146 ++++++++++++++---- .../NotificationConfiguration.swift | 12 +- .../TunnelManager/TunnelManager.swift | 6 +- .../AccountExpiryTests.swift | 64 +++++--- 6 files changed, 274 insertions(+), 95 deletions(-) diff --git a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift index e38b0fd3d57f..32fec36a4f67 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift @@ -7,37 +7,86 @@ // 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) - return Calendar.current.date( - byAdding: .day, - value: -NotificationConfiguration.closeToExpiryTriggerInterval, - to: expiryDate + // 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 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 { + // Used to compare dates with a precision of a minimum of seconds. + var secondsPrecision: Date { + let dateComponents = Calendar.current.dateComponents( + [.second, .minute, .hour, .day, .month, .year, .calendar], + from: self + ) + + return dateComponents.date ?? self } } diff --git a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift index 04658f0da977..6c43b9d7c1fe 100644 --- a/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift +++ b/ios/MullvadVPN/Notifications/Notification Providers/AccountExpiryInAppNotificationProvider.swift @@ -22,6 +22,9 @@ final class AccountExpiryInAppNotificationProvider: NotificationProvider, InAppN didLoadConfiguration: { [weak self] tunnelManager in self?.invalidate(deviceState: tunnelManager.deviceState) }, + didUpdateTunnelStatus: { [weak self] tunnelManager, _ in + self?.invalidate(deviceState: tunnelManager.deviceState) + }, didUpdateDeviceState: { [weak self] _, deviceState, _ in self?.invalidate(deviceState: deviceState) } @@ -38,24 +41,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." )) ) } @@ -63,19 +60,15 @@ final class AccountExpiryInAppNotificationProvider: NotificationProvider, InAppN // MARK: - Private private func invalidate(deviceState: DeviceState) { - updateExpiry(deviceState: deviceState) + accountExpiry.expiryDate = deviceState.accountData?.expiry updateTimer() invalidate() } - private func updateExpiry(deviceState: DeviceState) { - accountExpiry.expiryDate = deviceState.accountData?.expiry - } - private func updateTimer() { timer?.cancel() - guard let triggerDate = accountExpiry.triggerDate else { + guard let triggerDate = accountExpiry.nextTriggerDate(for: .inApp) else { return } @@ -105,3 +98,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..16987abc287f 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 accountHasExpired = 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) }, + didUpdateTunnelStatus: { [weak self] tunnelManager, _ in + self?.checkAccountExpiry( + tunnelStatus: tunnelManager.tunnelStatus, + deviceState: tunnelManager.deviceState + ) + }, didUpdateDeviceState: { [weak self] _, deviceState, _ in - self?.invalidate(deviceState: deviceState) + if self?.accountHasExpired == false { + self?.invalidate(deviceState: deviceState) + } } ) @@ -38,22 +47,16 @@ final class AccountExpirySystemNotificationProvider: NotificationProvider, Syste // MARK: - SystemNotificationProvider var notificationRequest: UNNotificationRequest? { - guard let trigger else { return nil } + let trigger = accountHasExpired ? triggerExpiry : triggerCloseToExpiry + + guard let trigger, let formattedRemainingDurationBody 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." - ) - content.sound = UNNotificationSound.default + content.title = formattedRemainingDurationTitle + content.body = formattedRemainingDurationBody + content.sound = .default return UNNotificationRequest( identifier: identifier.domainIdentifier, @@ -74,33 +77,118 @@ 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 { + // When scheduling a user notification we need to make sure that the date has not passed + // when it's actually added to the system. Giving it a one second leeway lets us be sure + // that this is the case. let dateComponents = Calendar.current.dateComponents( [.second, .minute, .hour, .day, .month, .year], - from: triggerDate + from: Date().addingTimeInterval(1) ) return UNCalendarNotificationTrigger(dateMatching: dateComponents, repeats: false) } private var shouldRemovePendingOrDeliveredRequests: Bool { - accountExpiry == nil + return accountExpiry.expiryDate == nil + } + + private func checkAccountExpiry(tunnelStatus: TunnelStatus, deviceState: DeviceState) { + if !accountHasExpired { + if case .accountExpired = tunnelStatus.observedState.blockedState?.reason { + accountHasExpired = true + } + + if accountHasExpired { + invalidate(deviceState: deviceState) + } + } } private func invalidate(deviceState: DeviceState) { - accountExpiry = deviceState.accountData?.expiry + accountExpiry.expiryDate = deviceState.accountData?.expiry invalidate() } } + +extension AccountExpirySystemNotificationProvider { + private var formattedRemainingDurationTitle: String { + accountHasExpired + ? NSLocalizedString( + "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_TITLE", + tableName: "AccountExpiry", + value: "Account credit has expired", + comment: "Title for system account expiry notification, fired on account expiry." + ) + : NSLocalizedString( + "ACCOUNT_EXPIRY_SYSTEM_NOTIFICATION_TITLE", + tableName: "AccountExpiry", + value: "Account credit expires soon", + comment: "Title for system account expiry notification, fired X days prior to account expiry." + ) + } + + private var formattedRemainingDurationBody: String? { + guard !accountHasExpired else { 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 internet, \ + please add more time or disconnect the VPN. + """, + 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/TunnelManager.swift b/ios/MullvadVPN/TunnelManager/TunnelManager.swift index 9f5cc98b6221..758be0bcb615 100644 --- a/ios/MullvadVPN/TunnelManager/TunnelManager.swift +++ b/ios/MullvadVPN/TunnelManager/TunnelManager.swift @@ -1112,7 +1112,7 @@ final class TunnelManager: StorePaymentObserver { extension TunnelManager { enum AccountExpirySimulationOption { - case closeToExpiry + case closeToExpiry(days: Int) case expired case active @@ -1124,9 +1124,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) + } } }