fix: read messages not marked read

Fixes #32. Opening a notification now marks its thread read (via the detail's `.onAppear`) so the tab and icon badges clear; swipe actions unchanged. Adds a UI regression test covering open-to-clear.

I grant Stefan Hausotte an irrevocable, worldwide, royalty-free license to use, sublicense, and distribute my contribution, including through Apple's App Store under the project's App Store exception.
This commit is contained in:
systemBlue 2026-06-02 18:01:07 +02:00 committed by secana
parent 992c628abd
commit c6dda2cd70
4 changed files with 93 additions and 6 deletions

View file

@ -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)
}

View file

@ -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<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(_ tagged: TaggedItem<NotificationThread>) async {
guard tagged.item.unread else { return }
await setNotificationRead(tagged, removeFromUnread: false)
}
private func setNotificationRead(_ tagged: TaggedItem<NotificationThread>, 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(

View file

@ -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(

View file

@ -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",
)
}
}