Stop relying on the NotificationCenter for removing delivered push notifications:

- this will improve the architecture but more importantly will remove some very flakey unit tests

ElementX.xcodeproj/project.pbxproj

@@ -827,7 +827,6 @@
 		C413D36D44F89DE63D3ADFA4 /* ReportContentScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = A433BE28B40D418237BE37B5 /* ReportContentScreen.swift */; };
 		C4180F418235DAD9DD173951 /* TemplateScreenUITests.swift in Sources */ = {isa = PBXBuildFile; fileRef = 9873076F224E4CE09D8BD47D /* TemplateScreenUITests.swift */; };
 		C49FCC766673006B6D299F1C /* RoomDetailsEditScreenViewModelProtocol.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1DF8F7A3AD83D04C08D75E01 /* RoomDetailsEditScreenViewModelProtocol.swift */; };
-		C4C84901ABAC9B17564AB7EB /* NotificationName.swift in Sources */ = {isa = PBXBuildFile; fileRef = 4151163F666ED94FD959475A /* NotificationName.swift */; };
 		C4D2BCAA54E2C62B94B24AF4 /* InviteUsersScreen.swift in Sources */ = {isa = PBXBuildFile; fileRef = C2E9B841EE4878283ECDB554 /* InviteUsersScreen.swift */; };
 		C4E0D03DF88242697545A9B7 /* UserIndicatorController.swift in Sources */ = {isa = PBXBuildFile; fileRef = FD1275D9CE0FFBA6E8E85426 /* UserIndicatorController.swift */; };
 		C4F69156C31A447FEFF2A47C /* DTHTMLElement+AttributedStringBuilder.swift in Sources */ = {isa = PBXBuildFile; fileRef = 1E508AB0EDEE017FF4F6F8D1 /* DTHTMLElement+AttributedStringBuilder.swift */; };
@@ -1396,7 +1395,6 @@
 		40076C770A5FB83325252973 /* VoiceMessageMediaManager.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = VoiceMessageMediaManager.swift; sourceTree = "<group>"; };
 		40316EFFEAC7B206EE9A55AE /* SecureBackupScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SecureBackupScreenViewModelTests.swift; sourceTree = "<group>"; };
 		40B21E611DADDEF00307E7AC /* String.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = String.swift; sourceTree = "<group>"; };
-		4151163F666ED94FD959475A /* NotificationName.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationName.swift; sourceTree = "<group>"; };
 		4176C3E20C772DE8D182863C /* LegalInformationScreen.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LegalInformationScreen.swift; sourceTree = "<group>"; };
 		41BB37D96C3EA18F3CE8675D /* RoomDirectorySearchScreenModels.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomDirectorySearchScreenModels.swift; sourceTree = "<group>"; };
 		41D041A857614A9AE13C7795 /* RoomChangePermissionsScreenViewModelTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RoomChangePermissionsScreenViewModelTests.swift; sourceTree = "<group>"; };
@@ -3009,7 +3007,6 @@
 				045253F9967A535EE5B16691 /* Label.swift */,
 				C14D83B2B7CD5501A0089EFC /* LayoutDirection.swift */,
 				23AA3F4B285570805CB0CCDD /* MapTiler.swift */,
-				4151163F666ED94FD959475A /* NotificationName.swift */,
 				F72EFC8C634469F9262659C7 /* NSItemProvider.swift */,
 				95BAC0F6C9644336E9567EE6 /* NSRegularExpresion.swift */,
 				62B07B296D7A9D2F09120853 /* OrderedSet.swift */,
@@ -6201,7 +6198,6 @@
 				FD4C21F8DA1E273DE94FCD1A /* NotificationItemProxyProtocol.swift in Sources */,
 				652ACCF104A8CEF30788963C /* NotificationManager.swift in Sources */,
 				06D3942496E9E0E655F14D21 /* NotificationManagerProtocol.swift in Sources */,
-				C4C84901ABAC9B17564AB7EB /* NotificationName.swift in Sources */,
 				5139F4BD5A5DF6F8D11A9BDE /* NotificationPermissionsScreen.swift in Sources */,
 				454311EAC17D778E19F46592 /* NotificationPermissionsScreenCoordinator.swift in Sources */,
 				3E7B65C2C97748D5D65AAA8B /* NotificationPermissionsScreenModels.swift in Sources */,

ElementX/Sources/FlowCoordinators/UserSessionFlowCoordinator.swift

@@ -32,6 +32,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
     private let bugReportService: BugReportServiceProtocol
     private let appSettings: AppSettings
     private let analytics: AnalyticsService
+    private let notificationManager: NotificationManagerProtocol
     
     private let stateMachine: UserSessionFlowCoordinatorStateMachine
     
@@ -82,6 +83,7 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
         self.roomTimelineControllerFactory = roomTimelineControllerFactory
         self.appSettings = appSettings
         self.analytics = analytics
+        self.notificationManager = notificationManager
         
         navigationSplitCoordinator = NavigationSplitCoordinator(placeholderCoordinator: PlaceholderScreenCoordinator())
         
@@ -467,7 +469,9 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
         }
         
         Task {
-            await userSession.clientProxy.trackRecentlyVisitedRoom(roomID)
+            let _ = await userSession.clientProxy.trackRecentlyVisitedRoom(roomID)
+            
+            await notificationManager.removeDeliveredMessageNotifications(for: roomID)
         }
     }
     
@@ -527,6 +531,10 @@ class UserSessionFlowCoordinator: FlowCoordinatorProtocol {
         sidebarNavigationStackCoordinator.push(coordinator, animated: animated) { [weak self] in
             self?.stateMachine.processEvent(.dismissedInvitesScreen)
         }
+        
+        Task {
+            await notificationManager.removeDeliveredInviteNotifications()
+        }
     }
     
     private func dismissInvitesList(animated: Bool) {

ElementX/Sources/Mocks/Generated/GeneratedMocks.swift

@@ -5329,6 +5329,80 @@ class NotificationManagerMock: NotificationManagerProtocol {
         requestAuthorizationCallsCount += 1
         requestAuthorizationClosure?()
     }
+    //MARK: - removeDeliveredMessageNotifications
+
+    var removeDeliveredMessageNotificationsForUnderlyingCallsCount = 0
+    var removeDeliveredMessageNotificationsForCallsCount: Int {
+        get {
+            if Thread.isMainThread {
+                return removeDeliveredMessageNotificationsForUnderlyingCallsCount
+            } else {
+                var returnValue: Int? = nil
+                DispatchQueue.main.sync {
+                    returnValue = removeDeliveredMessageNotificationsForUnderlyingCallsCount
+                }
+
+                return returnValue!
+            }
+        }
+        set {
+            if Thread.isMainThread {
+                removeDeliveredMessageNotificationsForUnderlyingCallsCount = newValue
+            } else {
+                DispatchQueue.main.sync {
+                    removeDeliveredMessageNotificationsForUnderlyingCallsCount = newValue
+                }
+            }
+        }
+    }
+    var removeDeliveredMessageNotificationsForCalled: Bool {
+        return removeDeliveredMessageNotificationsForCallsCount > 0
+    }
+    var removeDeliveredMessageNotificationsForReceivedRoomID: String?
+    var removeDeliveredMessageNotificationsForReceivedInvocations: [String] = []
+    var removeDeliveredMessageNotificationsForClosure: ((String) async -> Void)?
+
+    func removeDeliveredMessageNotifications(for roomID: String) async {
+        removeDeliveredMessageNotificationsForCallsCount += 1
+        removeDeliveredMessageNotificationsForReceivedRoomID = roomID
+        removeDeliveredMessageNotificationsForReceivedInvocations.append(roomID)
+        await removeDeliveredMessageNotificationsForClosure?(roomID)
+    }
+    //MARK: - removeDeliveredInviteNotifications
+
+    var removeDeliveredInviteNotificationsUnderlyingCallsCount = 0
+    var removeDeliveredInviteNotificationsCallsCount: Int {
+        get {
+            if Thread.isMainThread {
+                return removeDeliveredInviteNotificationsUnderlyingCallsCount
+            } else {
+                var returnValue: Int? = nil
+                DispatchQueue.main.sync {
+                    returnValue = removeDeliveredInviteNotificationsUnderlyingCallsCount
+                }
+
+                return returnValue!
+            }
+        }
+        set {
+            if Thread.isMainThread {
+                removeDeliveredInviteNotificationsUnderlyingCallsCount = newValue
+            } else {
+                DispatchQueue.main.sync {
+                    removeDeliveredInviteNotificationsUnderlyingCallsCount = newValue
+                }
+            }
+        }
+    }
+    var removeDeliveredInviteNotificationsCalled: Bool {
+        return removeDeliveredInviteNotificationsCallsCount > 0
+    }
+    var removeDeliveredInviteNotificationsClosure: (() async -> Void)?
+
+    func removeDeliveredInviteNotifications() async {
+        removeDeliveredInviteNotificationsCallsCount += 1
+        await removeDeliveredInviteNotificationsClosure?()
+    }
 }
 class NotificationSettingsProxyMock: NotificationSettingsProxyProtocol {
     var callbacks: PassthroughSubject<NotificationSettingsProxyCallback, Never> {

ElementX/Sources/Other/Extensions/NotificationName.swift

@@ -1,22 +0,0 @@
-//
-// Copyright 2023 New Vector Ltd
-//
-// Licensed under the Apache License, Version 2.0 (the "License");
-// you may not use this file except in compliance with the License.
-// You may obtain a copy of the License at
-//
-// http://www.apache.org/licenses/LICENSE-2.0
-//
-// Unless required by applicable law or agreed to in writing, software
-// distributed under the License is distributed on an "AS IS" BASIS,
-// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
-// See the License for the specific language governing permissions and
-// limitations under the License.
-//
-
-import Foundation
-
-extension Notification.Name {
-    static let roomMarkedAsRead = Notification.Name("roomMarkedAsRead")
-    static let invitesScreenAppeared = Notification.Name("invitesScreenAppeared")
-}

ElementX/Sources/Screens/InvitesScreen/InvitesScreenModels.swift

@@ -43,5 +43,4 @@ struct InvitesScreenRoomDetails: Identifiable {
 enum InvitesScreenViewAction {
     case accept(InvitesScreenRoomDetails)
     case decline(InvitesScreenRoomDetails)
-    case appeared
 }

ElementX/Sources/Screens/InvitesScreen/InvitesScreenViewModel.swift

@@ -57,8 +57,6 @@ class InvitesScreenViewModel: InvitesScreenViewModelType, InvitesScreenViewModel
             accept(invite: invite)
         case .decline(let invite):
             startDeclineFlow(invite: invite)
-        case .appeared:
-            notificationCenterProtocol.post(name: .invitesScreenAppeared, object: nil)
         }
     }
     

ElementX/Sources/Screens/InvitesScreen/View/InvitesScreen.swift

@@ -45,9 +45,6 @@ struct InvitesScreen: View {
                 noInvitesContent
             }
         }
-        .onAppear {
-            context.send(viewAction: .appeared)
-        }
     }
 
     private var noInvitesContent: some View {

ElementX/Sources/Screens/JoinRoomScreen/JoinRoomScreenViewModel.swift

@@ -60,7 +60,7 @@ class JoinRoomScreenViewModel: JoinRoomScreenViewModelType, JoinRoomScreenViewMo
         switch await clientProxy.joinRoom(state.roomID) {
         case .success:
             actionsSubject.send(.joined)
-        case .failure(let error):
+        case .failure:
             state.bindings.alertInfo = .init(id: .joinFailed)
         }
     }

ElementX/Sources/Screens/RoomScreen/RoomScreenViewModel.swift

@@ -416,11 +416,6 @@ class RoomScreenViewModel: RoomScreenViewModelType, RoomScreenViewModelProtocol
     private func sendReadReceiptIfNeeded(for lastVisibleItemID: TimelineItemIdentifier) async {
         guard application.applicationState == .active else { return }
                 
-        // Clear any notifications from notification center.
-        if lastVisibleItemID.timelineID == state.timelineViewState.timelineIDs.last {
-            notificationCenter.post(name: .roomMarkedAsRead, object: roomProxy.id)
-        }
-        
         await timelineController.sendReadReceipt(for: lastVisibleItemID)
     }
 

ElementX/Sources/Services/Notification/Manager/NotificationManager.swift

@@ -33,7 +33,6 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
         self.notificationCenter = notificationCenter
         self.appSettings = appSettings
         super.init()
-        addObservers()
     }
 
     // MARK: NotificationManagerProtocol
@@ -100,8 +99,8 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
             guard let self else { return }
             
             if await notificationCenter.authorizationStatus() == .authorized {
-                await MainActor.run {
+                await MainActor.run { [weak self] in
-                    delegate?.registerForRemoteNotifications()
+                    self?.delegate?.registerForRemoteNotifications()
                 }
             }
         }
@@ -128,6 +127,22 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
         }
     }
     
+    func removeDeliveredMessageNotifications(for roomID: String) async {
+        let notificationsIdentifiers = await notificationCenter
+            .deliveredNotifications()
+            .filter { $0.request.content.roomID == roomID }
+            .map(\.request.identifier)
+        notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
+    }
+
+    func removeDeliveredInviteNotifications() async {
+        let notificationsIdentifiers = await notificationCenter
+            .deliveredNotifications()
+            .filter { $0.request.content.categoryIdentifier == NotificationConstants.Category.invite }
+            .map(\.request.identifier)
+        notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
+    }
+    
     private func setPusher(with deviceToken: Data, clientProxy: ClientProxyProtocol) async -> Bool {
         do {
             let defaultPayload = APNSPayload(aps: APSInfo(mutableContent: 1,
@@ -177,47 +192,6 @@ final class NotificationManager: NSObject, NotificationManagerProtocol {
             delegate?.unregisterForRemoteNotifications()
         }
     }
-
-    private func addObservers() {
-        NotificationCenter
-            .default
-            .publisher(for: .roomMarkedAsRead)
-            .sink { [weak self] notification in
-                guard let roomID = notification.object as? String else {
-                    return
-                }
-                self?.removeMessageNotifications(for: roomID)
-            }
-            .store(in: &cancellables)
-
-        NotificationCenter
-            .default
-            .publisher(for: .invitesScreenAppeared)
-            .sink { [weak self] _ in
-                self?.removeInviteNotifications()
-            }
-            .store(in: &cancellables)
-    }
-
-    private func removeMessageNotifications(for roomID: String) {
-        Task {
-            let notificationsIdentifiers = await notificationCenter
-                .deliveredNotifications()
-                .filter { $0.request.content.roomID == roomID }
-                .map(\.request.identifier)
-            notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
-        }
-    }
-
-    private func removeInviteNotifications() {
-        Task {
-            let notificationsIdentifiers = await notificationCenter
-                .deliveredNotifications()
-                .filter { $0.request.content.categoryIdentifier == NotificationConstants.Category.invite }
-                .map(\.request.identifier)
-            notificationCenter.removeDeliveredNotifications(withIdentifiers: notificationsIdentifiers)
-        }
-    }
 }
 
 // MARK: - UNUserNotificationCenterDelegate

ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift

@@ -40,4 +40,7 @@ protocol NotificationManagerProtocol: AnyObject {
     func setUserSession(_ userSession: UserSessionProtocol?)
     
     func requestAuthorization()
+    
+    func removeDeliveredMessageNotifications(for roomID: String) async
+    func removeDeliveredInviteNotifications() async
 }

UnitTests/Sources/InvitesScreenViewModelTests.swift

@@ -86,14 +86,6 @@ class InvitesScreenViewModelTests: XCTestCase {
         XCTAssertNotNil(context.alertInfo)
     }
     
-    func testHasAppeared() async {
-        setupViewModel()
-        context.send(viewAction: .appeared)
-
-        await Task.yield()
-        XCTAssertEqual(mockNotificationCenter.postNameObjectReceivedArguments?.aName, .invitesScreenAppeared)
-    }
-    
     // MARK: - Private
     
     private func setupViewModel(roomSummaries: [RoomSummary]? = nil) {

UnitTests/Sources/NotificationManager/NotificationManagerTests.swift

@@ -204,55 +204,6 @@ final class NotificationManagerTests: XCTestCase {
         await notificationManager.userNotificationCenter(UNUserNotificationCenter.current(), didReceive: response)
         XCTAssertTrue(notificationTappedDelegateCalled)
     }
-
-    func test_MessageNotificationsRemoval() async throws {
-        notificationCenter.deliveredNotificationsClosure = {
-            XCTFail("deliveredNotifications should not be called if the object is nil or of the wrong type")
-            return []
-        }
-        // No interaction if the object is nil or of the wrong type
-        NotificationCenter.default.post(name: .roomMarkedAsRead, object: nil)
-        try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
-        
-        XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 0)
-        XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 0)
-
-        NotificationCenter.default.post(name: .roomMarkedAsRead, object: 1)
-        try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
-        
-        XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 0)
-        XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 0)
-
-        // The center calls the delivered and the removal functions when an id is passed
-        notificationCenter.deliveredNotificationsClosure = nil
-        notificationCenter.deliveredNotificationsReturnValue = []
-        let expectation = expectation(description: "Notification should be removed")
-        notificationCenter.removeDeliveredNotificationsWithIdentifiersClosure = { _ in
-            expectation.fulfill()
-        }
-        
-        NotificationCenter.default.post(name: .roomMarkedAsRead, object: "RoomID")
-        try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
-        
-        await fulfillment(of: [expectation])
-        XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 1)
-        XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 1)
-    }
-
-    func test_InvitesNotificationsRemoval() async {
-        let expectation = expectation(description: "Notification should be removed")
-        notificationCenter.deliveredNotificationsReturnValue = []
-        notificationCenter.removeDeliveredNotificationsWithIdentifiersClosure = { _ in
-            expectation.fulfill()
-        }
-        
-        NotificationCenter.default.post(name: .invitesScreenAppeared, object: nil)
-        try? await Task.sleep(for: .seconds(0.25)) // Wait for the notification to be delivered
-        
-        await fulfillment(of: [expectation])
-        XCTAssertEqual(notificationCenter.deliveredNotificationsCallsCount, 1)
-        XCTAssertEqual(notificationCenter.removeDeliveredNotificationsWithIdentifiersCallsCount, 1)
-    }
 }
 
 extension NotificationManagerTests: NotificationManagerDelegate {

UnitTests/Sources/RoomScreenViewModelTests.swift

@@ -336,11 +336,6 @@ class RoomScreenViewModelTests: XCTestCase {
         let arguments = timelineProxy.sendReadReceiptForTypeReceivedArguments
         XCTAssertEqual(arguments?.eventID, "t3")
         XCTAssertEqual(arguments?.type, .read)
-        
-        // And the notifications should be cleared.
-        XCTAssertEqual(notificationCenter.postNameObjectReceivedArguments?.aName, .roomMarkedAsRead)
-        let roomID = notificationCenter.postNameObjectReceivedArguments?.anObject as? String
-        XCTAssertEqual(roomID, roomProxy.id)
     }
     
     func testSendMoreReadReceipts() async throws {

UnitTests/Sources/UserSessionFlowCoordinatorTests.swift

@@ -23,6 +23,8 @@ import Combine
 class UserSessionFlowCoordinatorTests: XCTestCase {
     var userSessionFlowCoordinator: UserSessionFlowCoordinator!
     var navigationRootCoordinator: NavigationRootCoordinator!
+    var notificationManager: NotificationManagerMock!
+    
     var cancellables = Set<AnyCancellable>()
     
     var detailCoordinator: CoordinatorProtocol? {
@@ -45,6 +47,8 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
         
         navigationRootCoordinator = NavigationRootCoordinator()
         
+        notificationManager = NotificationManagerMock()
+        
         userSessionFlowCoordinator = UserSessionFlowCoordinator(userSession: userSession,
                                                                 navigationRootCoordinator: navigationRootCoordinator,
                                                                 windowManager: WindowManagerMock(),
@@ -53,7 +57,7 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
                                                                 roomTimelineControllerFactory: MockRoomTimelineControllerFactory(),
                                                                 appSettings: ServiceLocator.shared.settings,
                                                                 analytics: ServiceLocator.shared.analytics,
-                                                                notificationManager: NotificationManagerMock(),
+                                                                notificationManager: notificationManager,
                                                                 isNewLogin: false)
         
         let deferred = deferFulfillment(userSessionFlowCoordinator.statePublisher) { $0 == .roomList(selectedRoomID: nil) }
@@ -81,6 +85,8 @@ class UserSessionFlowCoordinatorTests: XCTestCase {
         try await process(route: .roomList, expectedState: .roomList(selectedRoomID: nil))
         XCTAssertNil(detailNavigationStack?.rootCoordinator)
         XCTAssertNil(detailCoordinator)
+        
+        XCTAssertEqual(notificationManager.removeDeliveredMessageNotificationsForReceivedInvocations, ["1", "1", "2"])
     }
     
     func testRoomDetailsPresentation() async throws {