From 52b411e95420b34841035e2dedd79b525e8920e5 Mon Sep 17 00:00:00 2001 From: Stefan Ceriu Date: Fri, 2 Oct 2026 15:22:48 +0300 Subject: [PATCH] Revert "Open the room live when tapping the notification for its newest message" This reverts commit 4c71cae0f. Comparing against other delivered notifications doesn't reflect the timeline: in rooms set to mentions and keywords, after swiping away newer notifications or with messages sent from another device, newer messages can exist without a notification, so the room opened live with the tapped event out of view. --- .../Sources/Application/AppCoordinator.swift | 5 ++-- .../Manager/NotificationManager.swift | 12 +------- .../Manager/NotificationManagerProtocol.swift | 3 +- .../NotificationManagerTests.swift | 29 +------------------ .../UNNotification+Creator.swift | 5 +--- ...extInputNotificationResponse+Creator.swift | 10 +++++-- 6 files changed, 14 insertions(+), 50 deletions(-) diff --git a/ElementX/Sources/Application/AppCoordinator.swift b/ElementX/Sources/Application/AppCoordinator.swift index ff6d7e999..ed0813488 100644 --- a/ElementX/Sources/Application/AppCoordinator.swift +++ b/ElementX/Sources/Application/AppCoordinator.swift @@ -413,7 +413,7 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg return !userSessionFlowCoordinator.isDisplayingRoomScreen(withRoomID: roomID) } - func notificationTapped(content: UNNotificationContent, isNewestInRoom: Bool) async { + func notificationTapped(content: UNNotificationContent) async { MXLog.info("Tapped Notification") guard let roomID = content.roomID, @@ -431,12 +431,11 @@ class AppCoordinator: AppCoordinatorProtocol, AuthenticationFlowCoordinatorDeleg handleAppRoute(.room(roomID: roomID, via: []), windowType: nil) } else if appSettings.threadsEnabled, let threadRootEventID = content.threadRootEventID { handleAppRoute(.thread(roomID: roomID, threadRootEventID: threadRootEventID, focusEventID: eventID), windowType: nil) - } else if let eventID, !isNewestInRoom { + } else if let eventID { // Only track main timeline event deeplinking analyticsService.signpost.startTransaction(.notificationToMessage) handleAppRoute(.event(eventID: eventID, roomID: roomID, via: []), windowType: nil) } else { - // The live timeline already ends with the newest message, so there's no need to focus on it. handleAppRoute(.room(roomID: roomID, via: []), windowType: nil) } } diff --git a/ElementX/Sources/Services/Notification/Manager/NotificationManager.swift b/ElementX/Sources/Services/Notification/Manager/NotificationManager.swift index 132a729e7..9b8bf8188 100644 --- a/ElementX/Sources/Services/Notification/Manager/NotificationManager.swift +++ b/ElementX/Sources/Services/Notification/Manager/NotificationManager.swift @@ -259,21 +259,11 @@ extension NotificationManager: UNUserNotificationCenterDelegate { content: response.notification.request.content, replyText: response.userText) case UNNotificationDefaultActionIdentifier: - await delegate?.notificationTapped(content: response.notification.request.content, - isNewestInRoom: isNewestInRoom(response.notification)) + await delegate?.notificationTapped(content: response.notification.request.content) default: break } } - - /// Whether no other delivered notification for the same room is more recent. - private func isNewestInRoom(_ notification: UNNotification) async -> Bool { - guard let roomID = notification.request.content.roomID else { return false } - - return await notificationCenter.deliveredNotifications().allSatisfy { deliveredNotification in - deliveredNotification.request.content.roomID != roomID || deliveredNotification.date <= notification.date - } - } } extension UNUserNotificationCenter: UserNotificationCenterProtocol { } diff --git a/ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift b/ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift index a34032c22..98d1f7d59 100644 --- a/ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift +++ b/ElementX/Sources/Services/Notification/Manager/NotificationManagerProtocol.swift @@ -11,8 +11,7 @@ import UserNotifications protocol NotificationManagerDelegate: AnyObject { func shouldDisplayInAppNotification(content: UNNotificationContent) -> Bool - /// - Parameter isNewestInRoom: Whether no other delivered notification for the same room is more recent. - func notificationTapped(content: UNNotificationContent, isNewestInRoom: Bool) async + func notificationTapped(content: UNNotificationContent) async func handleInlineReply(_ service: NotificationManagerProtocol, content: UNNotificationContent, replyText: String) async diff --git a/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift b/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift index a2dba2590..c2247acdf 100644 --- a/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift +++ b/UnitTests/Sources/NotificationManager/NotificationManagerTests.swift @@ -21,7 +21,6 @@ final class NotificationManagerTests { private var shouldDisplayInAppNotificationReturnValue = false private var handleInlineReplyDelegateCalled = false private var notificationTappedDelegateCalled = false - private var notificationTappedIsNewestInRoom: Bool? private var registerForRemoteNotificationsDelegateCalled: (() -> Void)? private let appSettings: AppSettings @@ -239,31 +238,6 @@ final class NotificationManagerTests { #expect(notificationTappedDelegateCalled) } - @Test - func whenTappedNotificationIsTheNewestInItsRoom_delegateIsToldItIsNewest() async throws { - notificationManager.delegate = self - let date = Date.now - notificationCenter.deliveredNotificationsReturnValue = try [UNNotification.with(userInfo: [NotificationConstants.UserInfoKey.roomIdentifier: "!room:matrix.org"], date: date - 60), - UNNotification.with(userInfo: [NotificationConstants.UserInfoKey.roomIdentifier: "!other:matrix.org"], date: date + 60)] - - let response = try UNTextInputNotificationResponse.with(userInfo: [NotificationConstants.UserInfoKey.roomIdentifier: "!room:matrix.org"], date: date) - await notificationManager.userNotificationCenter(UNUserNotificationCenter.current(), didReceive: response) - - #expect(notificationTappedIsNewestInRoom == true) - } - - @Test - func whenTappedNotificationIsOlderThanAnotherInItsRoom_delegateIsToldItIsNotNewest() async throws { - notificationManager.delegate = self - let date = Date.now - notificationCenter.deliveredNotificationsReturnValue = try [UNNotification.with(userInfo: [NotificationConstants.UserInfoKey.roomIdentifier: "!room:matrix.org"], date: date + 60)] - - let response = try UNTextInputNotificationResponse.with(userInfo: [NotificationConstants.UserInfoKey.roomIdentifier: "!room:matrix.org"], date: date) - await notificationManager.userNotificationCenter(UNUserNotificationCenter.current(), didReceive: response) - - #expect(notificationTappedIsNewestInRoom == false) - } - @Test func updatingAppBadgeCountUsesTheClientSideCount() async { clientProxy.totalUnreadNotifications = 7 @@ -298,9 +272,8 @@ extension NotificationManagerTests: @MainActor NotificationManagerDelegate { shouldDisplayInAppNotificationReturnValue } - func notificationTapped(content: UNNotificationContent, isNewestInRoom: Bool) async { + func notificationTapped(content: UNNotificationContent) async { notificationTappedDelegateCalled = true - notificationTappedIsNewestInRoom = isNewestInRoom } func handleInlineReply(_ service: ElementX.NotificationManagerProtocol, content: UNNotificationContent, replyText: String) async { diff --git a/UnitTests/Sources/NotificationManager/UNNotification+Creator.swift b/UnitTests/Sources/NotificationManager/UNNotification+Creator.swift index 1f8deaf1e..eadfe3e29 100644 --- a/UnitTests/Sources/NotificationManager/UNNotification+Creator.swift +++ b/UnitTests/Sources/NotificationManager/UNNotification+Creator.swift @@ -11,7 +11,7 @@ import UserNotifications import XCTest extension UNNotification { - static func with(userInfo: [AnyHashable: Any], date: Date? = nil) throws -> UNNotification { + static func with(userInfo: [AnyHashable: Any]) throws -> UNNotification { let content = UNMutableNotificationContent() content.userInfo = userInfo let request = UNNotificationRequest(identifier: "", @@ -20,9 +20,6 @@ extension UNNotification { let archiver = MockCoder(requiringSecureCoding: false) let notification = try XCTUnwrap(UNNotification(coder: archiver)) notification.setValue(request, forKey: "request") - if let date { - notification.setValue(date, forKey: "date") - } return notification } } diff --git a/UnitTests/Sources/NotificationManager/UNTextInputNotificationResponse+Creator.swift b/UnitTests/Sources/NotificationManager/UNTextInputNotificationResponse+Creator.swift index a0544f9c9..e5ba5be6c 100644 --- a/UnitTests/Sources/NotificationManager/UNTextInputNotificationResponse+Creator.swift +++ b/UnitTests/Sources/NotificationManager/UNTextInputNotificationResponse+Creator.swift @@ -10,9 +10,15 @@ import UserNotifications import XCTest extension UNTextInputNotificationResponse { - static func with(userInfo: [AnyHashable: Any], date: Date? = nil, actionIdentifier: String = UNNotificationDefaultActionIdentifier) throws -> UNNotificationResponse { - let notification = try UNNotification.with(userInfo: userInfo, date: date) + static func with(userInfo: [AnyHashable: Any], actionIdentifier: String = UNNotificationDefaultActionIdentifier) throws -> UNNotificationResponse { + let content = UNMutableNotificationContent() + content.userInfo = userInfo + let request = UNNotificationRequest(identifier: "", + content: content, + trigger: nil) let archiver = MockCoder(requiringSecureCoding: false) + let notification = try XCTUnwrap(UNNotification(coder: archiver)) + notification.setValue(request, forKey: "request") let response = try XCTUnwrap(UNTextInputNotificationResponse(coder: archiver)) response.setValue(notification, forKey: "notification")