diff --git a/Forji/Forji/App/AppDelegate.swift b/Forji/Forji/App/AppDelegate.swift index f17b9c1..dc96d90 100644 --- a/Forji/Forji/App/AppDelegate.swift +++ b/Forji/Forji/App/AppDelegate.swift @@ -59,6 +59,8 @@ extension AppDelegate: UNUserNotificationCenterDelegate { if response.actionIdentifier == "MARK_READ" { await markNotificationAsRead(userInfo: userInfo, serverURL: serverURL, username: username) } else { + // Opening the notification clears it, the same as tapping it inside the app. + await markNotificationAsRead(userInfo: userInfo, serverURL: serverURL, username: username) await MainActor.run { NavigationState.shared.navigateToNotifications(serverURL: serverURL, username: username) } diff --git a/Forji/Forji/Views/MergedNotificationsOverviewView.swift b/Forji/Forji/Views/MergedNotificationsOverviewView.swift index 6ae7a68..64476c2 100644 --- a/Forji/Forji/Views/MergedNotificationsOverviewView.swift +++ b/Forji/Forji/Views/MergedNotificationsOverviewView.swift @@ -110,7 +110,10 @@ struct MergedNotificationsOverviewView: View { let notification = tagged.item let destination = navigationDestination(for: tagged) if let destination { - NavigationLink { destination } label: { + NavigationLink { + destination + .onAppear { Task { await markReadOnOpen(tagged) } } + } label: { MergedNotificationRow(notification: notification, instanceName: tagged.instanceName) } } else { @@ -193,14 +196,22 @@ struct MergedNotificationsOverviewView: View { await setNotificationRead(tagged) } - private func setNotificationRead(_ tagged: TaggedItem) async { + /// Marks a notification read when the user opens it, mirroring Mail and News. + /// The row is updated in place rather than removed so navigation into the + /// detail view is not interrupted while the list is mutating. + private func markReadOnOpen(_ tagged: TaggedItem) async { + guard tagged.item.unread else { return } + await setNotificationRead(tagged, removeFromUnread: false) + } + + private func setNotificationRead(_ tagged: TaggedItem, removeFromUnread: Bool = true) async { guard let client = tagged.authService.client else { return } let service = NotificationService(client: client) let notification = tagged.item do { try await service.markAsRead(id: notification.id) withAnimation { - if statusFilter == "unread" { + if removeFromUnread, statusFilter == "unread" { pagination.items.removeAll { $0.id == tagged.id } } else if let index = pagination.items.firstIndex(where: { $0.id == tagged.id }) { let updated = NotificationThread( diff --git a/Forji/Forji/Views/NotificationsOverviewView.swift b/Forji/Forji/Views/NotificationsOverviewView.swift index 6c475ee..ae4c3f6 100644 --- a/Forji/Forji/Views/NotificationsOverviewView.swift +++ b/Forji/Forji/Views/NotificationsOverviewView.swift @@ -114,7 +114,10 @@ struct NotificationsOverviewView: View { private func notificationRow(_ notification: NotificationThread) -> some View { let destination = navigationDestination(for: notification) if let destination { - NavigationLink { destination } label: { + NavigationLink { + destination + .onAppear { Task { await markReadOnOpen(notification) } } + } label: { NotificationRow(notification: notification) } } else { @@ -182,12 +185,20 @@ struct NotificationsOverviewView: View { await setNotificationRead(notification) } - private func setNotificationRead(_ notification: NotificationThread) async { + /// Marks a notification read when the user opens it, mirroring Mail and News. + /// The row is updated in place rather than removed so navigation into the + /// detail view is not interrupted while the list is mutating. + private func markReadOnOpen(_ notification: NotificationThread) async { + guard notification.unread else { return } + await setNotificationRead(notification, removeFromUnread: false) + } + + private func setNotificationRead(_ notification: NotificationThread, removeFromUnread: Bool = true) async { guard let notificationService else { return } do { try await notificationService.markAsRead(id: notification.id) withAnimation { - if statusFilter == "unread" { + if removeFromUnread, statusFilter == "unread" { pagination.items.removeAll { $0.id == notification.id } } else if let index = pagination.items.firstIndex(where: { $0.id == notification.id }) { pagination.items[index] = NotificationThread( diff --git a/Forji/ForjiUITests/NotificationsUITests.swift b/Forji/ForjiUITests/NotificationsUITests.swift index 3ef71a3..185f4c0 100644 --- a/Forji/ForjiUITests/NotificationsUITests.swift +++ b/Forji/ForjiUITests/NotificationsUITests.swift @@ -49,4 +49,67 @@ final class NotificationsUITests: ForgejoUITestBase { } } } + + // MARK: - Open-to-clear (#32) + + /// Opening an unread notification marks it read on the server, so it leaves + /// the Unread filter on the next refresh. + /// + /// Covers the in-app NotificationsOverviewView path: the tap gesture on the + /// row calls markReadOnOpen, which marks the thread read with + /// removeFromUnread: false so the row stays put during navigation and only + /// drops out of Unread on the next server fetch. The multi-instance and + /// system-notification open paths apply the same fix but aren't reachable + /// from this single-instance harness. + /// + /// Sorts after testNotifications and opens whichever unread row is first + /// rather than a fixed one, so it's unaffected by the two notifications that + /// test consumes (the seed leaves several more). + @MainActor + func testOpeningNotificationMarksItRead() throws { + loginAndWaitForHome() + app.tabBars.buttons["Notifications"].tap() + + let notificationsList = app.collectionViews.firstMatch + XCTAssertTrue(notificationsList.waitForExistence(timeout: 10)) + + // Make sure the Unread filter is active. + let unreadButton = app.buttons["Unread"] + XCTAssertTrue(unreadButton.waitForExistence(timeout: 5)) + unreadButton.tap() + XCTAssertTrue(notificationsList.waitForExistence(timeout: 10)) + + // Remember the first unread notification's title (the row's first text) + // so we can assert that specific one leaves the Unread list. + let firstCell = notificationsList.cells.firstMatch + XCTAssertTrue(firstCell.waitForExistence(timeout: 10), "Expected at least one unread notification") + let openedTitle = firstCell.staticTexts.firstMatch.label + XCTAssertFalse(openedTitle.isEmpty, "Could not read the unread notification's title") + + // Open it. Navigating into the detail proves the row was tappable; the + // nav bar title changes away from "Notifications" once it's on screen. + firstCell.tap() + let detailNavBar = app.navigationBars.element(boundBy: 0) + XCTAssertTrue(detailNavBar.waitForExistence(timeout: 10), "Detail view did not open") + XCTAssertNotEqual(detailNavBar.identifier, "Notifications", "Did not navigate into the notification detail") + + // Back to the list. + app.navigationBars.buttons.element(boundBy: 0).tap() + XCTAssertTrue(notificationsList.waitForExistence(timeout: 10)) + + // Force a server refresh of Unread by toggling the filter. Opening marks + // the thread read but keeps the row visible until the next fetch, so this + // round-trip is what surfaces the open-to-clear behaviour. + app.buttons["Read"].tap() + XCTAssertTrue(notificationsList.waitForExistence(timeout: 10)) + unreadButton.tap() + XCTAssertTrue(notificationsList.waitForExistence(timeout: 10)) + + // The opened notification should no longer be under Unread. + let openedRow = notificationsList.staticTexts[openedTitle] + XCTAssertFalse( + openedRow.waitForExistence(timeout: 5), + "Opened notification \"\(openedTitle)\" should have left the Unread filter after refresh", + ) + } }