Fix out-of-time notifications

[rablador]

Currently the iOS app sends but a single notification about running out of time.

We should schedule 4 in-app notifications, with improved copy:

• "You have 3 days left on this account."
• “You have 2 days left on this account.”
• “You have 1 day left on this account.”
• "You have less than a day left on this account. Please add more time to continue using the VPN.”

We should also schedule 3 system notifications, with improved copy:

• "You have 3 days left on this account. Please add more time to continue using the VPN.."
• “You have one day left on this account. Please add more time to continue using the VPN.”
• "Blocking internet: Your time on this account has expired. To continue using the internet, please add more time or disconnect the VPN.”

---

This change is 

[linear]

IOS-743 Fix out-of-time notifications

[rablador]

Can be tested by invoking eg. the snippet below (please modify as needed) somewhere in initTunnelManagerOperation (AppDelegate), after tunnel manager is fully initialized. Account expired system notification cannot be tested this way due to it working in a different manner.

Task {
            try await delay(seconds: 5)

            print("Simulate close to expiry, 3 days")
            self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry(days: 3))
            try await delay(seconds: 5)

            print("Simulate close to expiry, 2 days")
            self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry(days: 2))
            try await delay(seconds: 5)

            print("Simulate close to expiry, 1 day")
            self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry(days: 1))
            try await delay(seconds: 5)

            print("Simulate close to expiry, 0 days")
            self.tunnelManager.simulateAccountExpiration(option: .closeToExpiry(days: 0))
            try await delay(seconds: 5)

            print("Simulate expired account")
            self.tunnelManager.simulateAccountExpiration(option: .expired)
            try await delay(seconds: 5)

            print("Simulate active account")
            self.tunnelManager.simulateAccountExpiration(option: .active)
        }

Reviewable status: 0 of 8 files reviewed, all discussions resolved

[buggmagnet]

It looks like you forgot to provide the implementation for delay, Task.sleep did the trick however.

Reviewed 6 of 6 files at r1, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @rablador)

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift line 85 at r1 (raw file):

    // Used to compare dates with a precision of a minimum of seconds.
    var secondsPrecision: Date {
        Date(timeIntervalSince1970: TimeInterval(Int(timeIntervalSince1970)))

I have to say, it feels really convoluted to do this. I think we could have done entirely the same thing just using a single point in time, and calendar operations.

But it seems to work pretty well, so I'll leave it at that.

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 100 at r1 (raw file):

        let dateComponents = Calendar.current.dateComponents(
            [.second, .minute, .hour, .day, .month, .year],
            from: Date().addingTimeInterval(1) // Giving some leeway.

Can we clarify what "giving some leeway" means here ?

[rablador]

Ah, right!

Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @buggmagnet)

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpiry.swift line 85 at r1 (raw file):

Previously, buggmagnet wrote…

I have to say, it feels really convoluted to do this. I think we could have done entirely the same thing just using a single point in time, and calendar operations.

But it seems to work pretty well, so I'll leave it at that.

Indeed, done.

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 100 at r1 (raw file):

Previously, buggmagnet wrote…

Can we clarify what "giving some leeway" means here ?

Yep, done.

[buggmagnet]

Reviewed 1 of 2 files at r2.
Reviewable status: 5 of 6 files reviewed, 1 unresolved discussion

[mojganii]

Reviewable status: 5 of 6 files reviewed, 4 unresolved discussions (waiting on @buggmagnet and @rablador)

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 117 at r2 (raw file):

        }

        let accountHasExpired = deviceState.accountData?.isExpired == true

nit : we can remove the equity expression. an use the variable for the next line

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 120 at r2 (raw file):

        let accountHasRecentlyExpired = deviceState.accountData?.isExpired != previousDeviceState.accountData?.isExpired

        self.accountHasRecentlyExpired = blockedStateByExpiredAccount && accountHasExpired && accountHasRecentlyExpired

isn't enough to compare the previous one state with the new one to realise if the account has been expired recently or not? must we evaluate the tunnel state for blocking state with the reason to be sure it's blocked? I feel we are over-evaluating to figure out if it's expired recently or not.

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 146 at r2 (raw file):

    }

    private var formattedRemainingDurationBody: String? {

nit : I think we can make it better :

Code snippet:

     private var formattedRemainingDurationBody: String? {
        guard !accountHasRecentlyExpired, let daysRemaining = accountExpiry.daysRemaining(for: .system)?.day else {
            return expiredText
        }
        return daysRemaining == 1 ? singleDayText : multipleDaysText
    }

[rablador]

Reviewable status: 5 of 6 files reviewed, 4 unresolved discussions (waiting on @buggmagnet and @mojganii)

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 120 at r2 (raw file):

Previously, mojganii wrote…

isn't enough to compare the previous one state with the new one to realise if the account has been expired recently or not? must we evaluate the tunnel state for blocking state with the reason to be sure it's blocked? I feel we are over-evaluating to figure out if it's expired recently or not.

I'm adding extra security here to make sure we don't accidentally spam users with notifications. Basically making sure that:

1. Blocked state is because of account expiry, so that we don't trigger for any kind of blocked state.
2. Current state is blocked state.
3. Previous expiry state is not the same as current expiry state. This last check alone can't decide if we went from expired -> not expired, or the other way around. That's why I combine 2) and 3).

[rablador]

Reviewable status: 4 of 6 files reviewed, 3 unresolved discussions (waiting on @buggmagnet and @mojganii)

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 117 at r2 (raw file):

Previously, mojganii wrote…

nit : we can remove the equity expression. an use the variable for the next line

Done.

---

ios/MullvadVPN/Notifications/Notification Providers/AccountExpirySystemNotificationProvider.swift line 146 at r2 (raw file):

Previously, mojganii wrote…

nit : I think we can make it better :

This doesn't work since no .day should return nil rather than expiredText. This will in turn make notificationRequest return nil, which will prevent a notification being sent. accountHasRecentlyExpired will check that account expiry was recent, but for all other cases (account expired sometime in the past and no longer relevant) there should be no notification date to trigger.

[rablador]

Did some updates to fix the last system notification on account expiry. Code got less convoluted as well.

Reviewable status: 3 of 6 files reviewed, 3 unresolved discussions (waiting on @buggmagnet and @mojganii)

[buggmagnet]

Reviewed 1 of 2 files at r3, 2 of 2 files at r4, all commit messages.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @mojganii)

[mojganii]

Reviewable status: all files reviewed, 1 unresolved discussion

[github-actions]

🚨 End to end tests failed. Please check the failed workflow run.