Show in-app notification when new app version is available

[rablador]

Why this needs to be done
If a user is connected and have IAN enabled, and a new app version is available on App Store, we should show an in-app notification to warn them that updating in this state will break their phone's network connectivity. The conditions are the same as for system notifications, described here: IOS-1408.

How do we implement this feature?
See IOS-1408 for logic as to when to trigger the notification. Ideally we would share that code between both system and in-app notifications.

The notification should have a link to App Store, and clicking on it should first alert the user that they need to disable IAN.

If IAN is not enabled, the alert is not presented.

Main view: https://www.figma.com/design/dZvBSfpPVVtVN7OC2zr0Yb/IOS-1417-Design-%22includeAllNetworks%22-activation-flow?node-id=8445-12956&t=OR9HLtt0WFfv5Pol-4

Acceptance criteria
When user has IAN enabled and is connected, if a new app version is available on App Store we should see an in-app notification with a warning text.

Clicking on the notification should open the Mullvad page on App Store.

Before being redirected to App Store the user should be presented with an alert asking them to turn off IAN first.

---

This change is 

[linear]

IOS-1493 Show in-app notification when new app version is available

[github-actions]

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

[waahlnaden]

In-app banner header should be all caps. I also get the same "Disable" dialogue after I've disabled Force all apps, but maybe that's just a debug thing?

[rablador]

Caps fixed.
What dialog are you referring to?

@rablador made 1 comment.
Reviewable status: 0 of 15 files reviewed, all discussions resolved.

[acb-mv]

@acb-mv reviewed 16 files and all commit messages, and made 1 comment.
Reviewable status: complete! all files reviewed, all discussions resolved.

---

ios/MullvadTypes/NotificationProviderIdentifier.swift line 23 at r2 (raw file):

public enum NotificationProviderIdentifier: String {
    case accountExpirySystemNotification = "AccountExpiryNotification"

These names are a bit long. Perhaps we could remove the somewhat redundant word "Notification"?

[mojganii]

@mojganii made 14 comments.
Reviewable status: all files reviewed, 14 unresolved discussions (waiting on rablador).

---

ios/MullvadREST/ApiHandlers/AppStoreMetaDataService.swift line 14 at r2 (raw file):

import UserNotifications

public final class AppStoreMetaDataService: @unchecked Sendable {

According to the new design, I think the only responsibility of this object is to check the app version and inform the upper layer about a new update via the onNewAppVersion handler. The upper layers will then decide whether to publish an in-app or system notification. Having sendSystemNotification here doesn’t quite fit.

---

ios/MullvadREST/ApiHandlers/AppStoreMetaDataService.swift line 104 at r2 (raw file):

    }

    func performVersionCheck() async throws -> Bool {

Shouldn’t it be private?
AppStoreMetaDataServiceTests should act upon the new handler.

---

ios/MullvadTypes/InAppNotificationDescriptor.swift line 13 at r2 (raw file):

/// Struct describing in-app notification.
public struct InAppNotificationDescriptor: Equatable {

can't it reside in main target?

---

ios/MullvadVPN/AppDelegate.swift line 472 at r2 (raw file):

    ) {
        let appStoreMetaDataService = AppStoreMetaDataService(

nit: Since it checks whether a new update is available, I suggest naming it AppVersionService.

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 894 at r2 (raw file):

        guard tunnelManager.settings.includeAllNetworks.includeAllNetworksIsEnabled else {
            navigateToAppStore()

Should we navigate the user to the App Store if IAN is disabled? And should we display a different message than the system notification alert?

we have 2 messages:

• “Force all apps” is enabled, please disable it or disconnect before updating or you will lose network connectivity.
• “Force all apps” is enabled, please disable it before updating or you will lose network connectivity. This will briefly expose your traffic as you reconnect to the VPN.

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 920 at r2 (raw file):

            buttons: [
                AlertAction(
                    title: NSLocalizedString("Disable “Force all apps”", comment: ""),

Since it’s a feature name, we can save it as an individual string named “Force all apps” in the .xcstrings file. This can also reduce the risk of mistranslation. We should also update the code to use this unique key for buttons and other UI elements.

---

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

        else { return nil }

        return String(format: NSLocalizedString("%@ left on this account", comment: ""), duration).localizedUppercase

good catch!

---

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

//
//  NewAppStoreVersionInAppNotificationProvider.swift

AppStore in the name is not necessary

---

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

import UIKit

final class NewAppStoreVersionInAppNotificationProvider:

nit: NewAppVesrionInAppNotificationProvider

---

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

    private var canShowSafeNotification = false
    private var newAppStoreVersionAvailable = false

isNewVersionAvailable

---

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

        didSet {
            if tunnelIsSecured && includeAllNetworksIsEnabled {
                canShowSafeNotification = true

shouldShowNotification

---

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

        self.appStoreMetaDataService.onNewAppVersion = { [weak self] in
            guard let self else { return }

If we set newAppStoreVersionAvailable to true, we can remove it from the invalidate parameter and use the class variable instead to have a single source of truth.

---

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

    private func invalidate(
        newAppStoreVersionAvailable: Bool,
        tunnelIsSecured: Bool,

both are global we can use them directly instead of passing them here

---

ios/PacketTunnel/Notifications/NewAppVersionSystemNoticationHandler.swift line 2 at r2 (raw file):

//
//  NewAppVersionSystemNoticationHandler.swift

typo: Notification

[rablador]

@rablador made 12 comments and resolved 3 discussions.
Reviewable status: all files reviewed, 11 unresolved discussions (waiting on mojganii).

---

ios/MullvadREST/ApiHandlers/AppStoreMetaDataService.swift line 14 at r2 (raw file):

Previously, mojganii wrote…

According to the new design, I think the only responsibility of this object is to check the app version and inform the upper layer about a new update via the onNewAppVersion handler. The upper layers will then decide whether to publish an in-app or system notification. Having sendSystemNotification here doesn’t quite fit.

You're right, I moved the notification part.

---

ios/MullvadREST/ApiHandlers/AppStoreMetaDataService.swift line 104 at r2 (raw file):

Previously, mojganii wrote…

Shouldn’t it be private?
AppStoreMetaDataServiceTests should act upon the new handler.

I think this function can be tested on its own as well.

---

ios/MullvadTypes/InAppNotificationDescriptor.swift line 13 at r2 (raw file):

Previously, mojganii wrote…

can't it reside in main target?

No, it's needed for system notifs in the packet tunnel.

---

ios/MullvadVPN/AppDelegate.swift line 472 at r2 (raw file):

Previously, mojganii wrote…

nit: Since it checks whether a new update is available, I suggest naming it AppVersionService.

Done.

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 894 at r2 (raw file):

Previously, mojganii wrote…

Should we navigate the user to the App Store if IAN is disabled? And should we display a different message than the system notification alert?

we have 2 messages:

• “Force all apps” is enabled, please disable it or disconnect before updating or you will lose network connectivity.
• “Force all apps” is enabled, please disable it before updating or you will lose network connectivity. This will briefly expose your traffic as you reconnect to the VPN.

We should always send user to the app store when the in-app banner is clicked. The difference is that we also show an alert when IAN is enabled.

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 920 at r2 (raw file):

Previously, mojganii wrote…

Since it’s a feature name, we can save it as an individual string named “Force all apps” in the .xcstrings file. This can also reduce the risk of mistranslation. We should also update the code to use this unique key for buttons and other UI elements.

Done.

---

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

Previously, mojganii wrote…

AppStore in the name is not necessary

Agreed, but we should keep "app", so that it's clear what kind of version we're looking at.

---

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

Previously, mojganii wrote…

isNewVersionAvailable

Done.

---

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

Previously, mojganii wrote…

shouldShowNotification

No, setting this flag to true doesn't mean we should display it no matter what, it just means that depending on other state it can (or is allowed) be displayed.

---

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

Previously, mojganii wrote…

If we set newAppStoreVersionAvailable to true, we can remove it from the invalidate parameter and use the class variable instead to have a single source of truth.

Done.

---

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

Previously, mojganii wrote…

both are global we can use them directly instead of passing them here

Done.

---

ios/PacketTunnel/Notifications/NewAppVersionSystemNoticationHandler.swift line 2 at r2 (raw file):

Previously, mojganii wrote…

typo: Notification

Done.

[mojganii]

@mojganii made 3 comments and resolved 9 discussions.
Reviewable status: 7 of 17 files reviewed, 3 unresolved discussions (waiting on acb-mv and rablador).

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 894 at r2 (raw file):

Previously, rablador (Jon Petersson) wrote…

We should always send user to the app store when the in-app banner is clicked. The difference is that we also show an alert when IAN is enabled.

how about the different messages?

---

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

Previously, rablador (Jon Petersson) wrote…

No, setting this flag to true doesn't mean we should display it no matter what, it just means that depending on other state it can (or is allowed) be displayed.

don't we put the computed value for safeNotificationCanBeDisplayed instead:

Code snippet:

 private var safeNotificationCanBeDisplayed : Bool {
        tunnelIsSecured && includeAllNetworksIsEnabled
    }

---

ios/MullvadVPN/Notifications/Notification Providers/NewAppVersionInAppNotificationProvider.swift line 92 at r3 (raw file):

                comment: ""
            )
        } else if safeNotificationCanBeDisplayed {

I'm confused what it's checking. can you elaborate? shouldn't it be if the tunnel is up but IAN isn't enabled?

Code snippet:

  private var safeNotificationCanBeDisplayed : Bool {
        tunnelIsSecured && !includeAllNetworksIsEnabled
    }

[rablador]

@rablador made 3 comments.
Reviewable status: 7 of 17 files reviewed, 3 unresolved discussions (waiting on acb-mv and mojganii).

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 894 at r2 (raw file):

Previously, mojganii wrote…

how about the different messages?

I think the idea for the alert message is that since you can disable IAN with button click there, we should focus on what happens when you do so and that it needs to be reenabled manually afterwards.

---

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

Previously, mojganii wrote…

don't we put the computed value for safeNotificationCanBeDisplayed instead:

Discussed IRL.

---

ios/MullvadVPN/Notifications/Notification Providers/NewAppVersionInAppNotificationProvider.swift line 92 at r3 (raw file):

Previously, mojganii wrote…

I'm confused what it's checking. can you elaborate? shouldn't it be if the tunnel is up but IAN isn't enabled?

Discussed IRL.

[mojganii]

@mojganii made 1 comment and resolved 2 discussions.
Reviewable status: 7 of 21 files reviewed, 2 unresolved discussions (waiting on acb-mv and rablador).

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 61 at r5 (raw file):

    private var outOfTimeTimer: Timer?
    private var tunnelDidConnect: (() -> Void)?

I don't think it's a scaleable approach. we need to listen to the change in the function like below:

Code snippet:

private var tunnelObservers: [String:TunnelObserver] = [:]
    private func handleNewAppVersionInAppNotification() {
        let navigateToAppStore: @Sendable () -> Void = {
            DispatchQueue.main.async {
                let appStoreLink = URL(string: "itms-apps://itunes.apple.com/app/id1488466513")!
                if UIApplication.shared.canOpenURL(appStoreLink) {
                    UIApplication.shared.open(appStoreLink, options: [:], completionHandler: nil)
                }
            }
        }

        // If IAN is disabled, skip the alert and go directly to AppStore.
        guard tunnelManager.settings.includeAllNetworks.includeAllNetworksIsEnabled else {
            navigateToAppStore()
            return
        }

        let message = [
            NSLocalizedString(
                String(
                    format:
                        "“%@“ is enabled, please disable it before updating or you will lose network connectivity. "
                        + "This will briefly expose your traffic as you reconnect to the VPN.",
                    "Force all apps"
                ),
                comment: ""
            ),
            NSLocalizedString(
                String(
                    format: "After updating, you will have to enable “%@” manually again.",
                    "Force all apps"
                ),
                comment: ""
            ),
            NSLocalizedString(
                String(
                    format: "If you do not wish to disable “%@“, you can disconnect from the VPN instead.",
                    "Force all apps"
                ),
                comment: ""
            ),
        ].joinedParagraphs(lineBreaks: 1)
        
 
        let alertPresenter = AlertPresenter(context: self)

        let presentation = AlertPresentation(
            id: "new-app-version-in-app-notification",
            icon: .info,
            message: message,
            buttons: [
                AlertAction(
                    title: NSLocalizedString(
                        String(format: "Disable “%@”", "Force all apps"),
                        comment: ""
                    ),
                    style: .default,
                    interactiveHandler: { [weak self] alertViewController, button in
                        guard let self else { return }

                        // Show loading spinner on button and then navigate to AppStore
                        // when tunnel has reconnected.
                        button.setLoading(true)
                        
                        // Turn off IAN and trigger a tunnel reconnection.
                        let newIncludeAllNetworksSettings = IncludeAllNetworksSettings(
                            includeAllNetworksState: .off,
                            localNetworkSharingState: tunnelManager.settings.includeAllNetworks.localNetworkSharingState
                        )
                        tunnelManager.updateSettings([.includeAllNetworks(newIncludeAllNetworksSettings)])
                    }
                ),
                AlertAction(
                    title: NSLocalizedString("Cancel", comment: ""),
                    style: .default
                ),
            ]
        )
        let id = UUID().uuidString

        let observer = TunnelBlockObserver(
            didUpdateTunnelStatus: { [weak self] _, tunnelStatus in
                guard let self else { return }

                if case .connected = tunnelStatus.state {
                    DispatchQueue.main.async {
                        alertPresenter.dismissAlert(presentation: presentation, animated: true)
                        navigateToAppStore()

                        if let observer = self.tunnelObservers[id] {
                            self.tunnelManager.removeObserver(observer)
                            self.tunnelObservers.removeValue(forKey: id)
                        }
                    }
                }
            }
        )

        tunnelObservers[id] = observer
        tunnelManager.addObserver(observer)
        alertPresenter.showAlert(presentation: presentation, animated: true)
    }

[rablador]

@rablador made 1 comment.
Reviewable status: 7 of 21 files reviewed, 2 unresolved discussions (waiting on acb-mv and mojganii).

---

ios/MullvadVPN/Coordinators/ApplicationCoordinator.swift line 61 at r5 (raw file):

Previously, mojganii wrote…

I don't think it's a scaleable approach. we need to listen to the change in the function like below:

I agree. I did opt for a similar but slightly different approach though, since I don't think we need to bleed the observer stuff outside this function.

[mojganii]

@mojganii made 1 comment.
Reviewable status: 7 of 21 files reviewed, 2 unresolved discussions (waiting on acb-mv).

[github-actions]

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