feat: clear delivered iOS notifications when a channel or DM is read#2069
feat: clear delivered iOS notifications when a channel or DM is read#2069rancur wants to merge 1 commit into
Conversation
Incoming message notifications remained in iOS Notification Center after the user read the messages, so they piled up indefinitely. Reading a conversation only cancelled still-pending notification requests, never the ones already delivered. When a channel or DM is opened and its messages are marked read, also remove the matching delivered notifications from Notification Center. Notifications are matched by the deep-link path already stored in userInfo (?channelId= vs ?userNum=) plus the channel index / node number, so channels and DMs are cleared independently and only the opened conversation is affected. The app icon badge continues to be driven by the unread counts updated in the same mark-as-read path, so muted-channel badge behavior is unchanged.
|
OpenClaw seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
1 similar comment
|
OpenClaw seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account. You have signed the CLA already but the status is still pending? Let us recheck it. |
📝 WalkthroughWalkthroughDelivered notifications are now removed for channel and direct-message conversations when their messages are marked as read. Matching uses notification metadata and conversation identifiers. ChangesNotification cleanup
Estimated code review effort: 2 (Simple) | ~10 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
Meshtastic/Helpers/LocalNotificationManager.swift (2)
148-189: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate filtering logic between the two new methods.
removeDeliveredNotifications(forChannel:)andremoveDeliveredNotifications(forDirectMessageUserNum:)share nearly identical structure (fetch delivered notifications, filter by path substring + numeric field match, log, remove). Consider extracting a shared private helper to reduce duplication and centralize future changes (e.g., to the fetch/removal mechanism).♻️ Proposed refactor
+ private func removeDeliveredNotifications(matching predicate: `@escaping` (UNNotification) -> Bool, logLabel: String) { + UNUserNotificationCenter.current().getDeliveredNotifications { notifications in + let identifiers = notifications.filter(predicate).map { $0.request.identifier } + if !identifiers.isEmpty { + Logger.services.debug("Removing \(identifiers.count, privacy: .public) delivered \(logLabel, privacy: .public) notifications") + UNUserNotificationCenter.current().removeDeliveredNotifications(withIdentifiers: identifiers) + } + } + } + func removeDeliveredNotifications(forChannel channelIndex: Int32) { - UNUserNotificationCenter.current().getDeliveredNotifications { notifications in - let identifiers = notifications.compactMap { notification -> String? in - let userInfo = notification.request.content.userInfo - guard let path = userInfo["path"] as? String, path.contains("?channelId="), - let channel = (userInfo["channel"] as? NSNumber)?.int32Value, - channel == channelIndex else { - return nil - } - return notification.request.identifier - } - - if !identifiers.isEmpty { - Logger.services.debug("Removing \(identifiers.count, privacy: .public) delivered channel notifications") - UNUserNotificationCenter.current().removeDeliveredNotifications(withIdentifiers: identifiers) - } - } + removeDeliveredNotifications(logLabel: "channel") { notification in + let userInfo = notification.request.content.userInfo + guard let path = userInfo["path"] as? String, path.contains("?channelId="), + let channel = (userInfo["channel"] as? NSNumber)?.int32Value else { + return false + } + return channel == channelIndex + } }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Helpers/LocalNotificationManager.swift` around lines 148 - 189, Extract the shared fetch, filtering, logging, and removal flow from removeDeliveredNotifications(forChannel:) and removeDeliveredNotifications(forDirectMessageUserNum:) into a private helper that accepts the path marker, numeric userInfo key, expected value, and notification-type description. Update both public methods to supply their specific matching criteria while preserving the existing channel and direct-message behavior.
148-165: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winPrefer the async notifications API here. Both methods can use
await UNUserNotificationCenter.current().deliveredNotifications()instead ofgetDeliveredNotifications(completionHandler:), which fits the project’s async/await convention and is available on the supported targets.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@Meshtastic/Helpers/LocalNotificationManager.swift` around lines 148 - 165, Update removeDeliveredNotifications(forChannel:) and the other delivered-notifications method to use await UNUserNotificationCenter.current().deliveredNotifications() instead of getDeliveredNotifications(completionHandler:). Propagate async as needed through callers while preserving the existing filtering, logging, and removal behavior.Source: Coding guidelines
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Meshtastic/Views/Messages/ChannelMessageList.swift`:
- Around line 94-96: Update docs/user/messages.md to document that opening or
catching up on a channel removes its already-delivered notifications from
Notification Center, reflecting the behavior implemented by
removeDeliveredNotifications(forChannel:) in ChannelMessageList.
In `@Meshtastic/Views/Messages/UserMessageList.swift`:
- Around line 86-88: Document the new user-visible behavior in
docs/user/messages.md: explain that opening a direct-message thread clears its
already-delivered Notification Center notifications, alongside the existing
channel behavior documentation. Update the relevant messages documentation
section without changing the implementation in UserMessageList.
---
Nitpick comments:
In `@Meshtastic/Helpers/LocalNotificationManager.swift`:
- Around line 148-189: Extract the shared fetch, filtering, logging, and removal
flow from removeDeliveredNotifications(forChannel:) and
removeDeliveredNotifications(forDirectMessageUserNum:) into a private helper
that accepts the path marker, numeric userInfo key, expected value, and
notification-type description. Update both public methods to supply their
specific matching criteria while preserving the existing channel and
direct-message behavior.
- Around line 148-165: Update removeDeliveredNotifications(forChannel:) and the
other delivered-notifications method to use await
UNUserNotificationCenter.current().deliveredNotifications() instead of
getDeliveredNotifications(completionHandler:). Propagate async as needed through
callers while preserving the existing filtering, logging, and removal behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 012350d1-0524-4295-95c3-64eba8fd3a39
📒 Files selected for processing (3)
Meshtastic/Helpers/LocalNotificationManager.swiftMeshtastic/Views/Messages/ChannelMessageList.swiftMeshtastic/Views/Messages/UserMessageList.swift
| // Also clear any notifications for this channel that were already delivered to Notification | ||
| // Center, so catching up on the channel wipes its piled-up notifications. | ||
| notificationManager.removeDeliveredNotifications(forChannel: channelIndex) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update docs/user/messages.md for this user-visible behavior change.
This change makes delivered notifications disappear from Notification Center when a channel is read — a user-visible behavior change in Meshtastic/Views/Messages/.
As per path instructions, Meshtastic/Views/Messages/**/*.swift: "Update docs/user/messages.md when code in Meshtastic/Views/Messages/ changes in a user-visible way."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/Views/Messages/ChannelMessageList.swift` around lines 94 - 96,
Update docs/user/messages.md to document that opening or catching up on a
channel removes its already-delivered notifications from Notification Center,
reflecting the behavior implemented by removeDeliveredNotifications(forChannel:)
in ChannelMessageList.
Source: Path instructions
| // Also clear any notifications for this conversation that were already delivered to | ||
| // Notification Center, so opening the DM wipes its piled-up notifications. | ||
| notificationManager.removeDeliveredNotifications(forDirectMessageUserNum: user.num) |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟠 Major | ⚡ Quick win
Update docs/user/messages.md for this user-visible behavior change.
Same as the channel-side change: clearing delivered notifications when a DM thread is opened is a user-visible behavior change under Meshtastic/Views/Messages/.
As per path instructions, Meshtastic/Views/Messages/**/*.swift: "Update docs/user/messages.md when code in Meshtastic/Views/Messages/ changes in a user-visible way."
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Meshtastic/Views/Messages/UserMessageList.swift` around lines 86 - 88,
Document the new user-visible behavior in docs/user/messages.md: explain that
opening a direct-message thread clears its already-delivered Notification Center
notifications, alongside the existing channel behavior documentation. Update the
relevant messages documentation section without changing the implementation in
UserMessageList.
Source: Path instructions
|
Looks good, can merge it once the CLA is signed. |
3 participants
What
Incoming message notifications stayed in iOS Notification Center even after the user had read the messages, so they piled up indefinitely. Reading a conversation only cancelled still-pending notification requests (
removePendingNotificationRequests) — it never removed the ones that had already been delivered.This makes reading a conversation clear its already-delivered notifications:
Why
Users see Meshtastic notifications accumulate on the lock screen / Notification Center long after they've read the messages in-app. Catching up in the app should clear the corresponding notifications.
How
Message notifications are already tagged in
userInfowhen scheduled (scheduleNotificationsinLocalNotificationManager) with the deep-linkpath,channelindex, and senderuserNum. Two new helpers query delivered notifications and remove only the matching ones:removeDeliveredNotifications(forChannel:)— matches channel notifications via their?channelId=path plus the channel index.removeDeliveredNotifications(forDirectMessageUserNum:)— matches DM notifications via their?userNum=path plus the remote node number.Because both channel and DM notifications carry a
channeland auserNumvalue, the deep-linkpathis used to tell the two conversation types apart so a channel and a DM are never cleared for one another. Other notification types (waypoint, low-battery, etc.) use different paths and are untouched.They're invoked from the existing
markMessagesAsRead()paths inChannelMessageListandUserMessageList, right after the existing pending-cancel call.Notes
markMessagesAsRead()path, so the muted-channel badge behavior from fix: muted ("Hide Alerts") channels & DMs must not badge the app icon or Messages tab #2050 is not affected.userInfovalues are read back asNSNumber.Testing
I was not able to build or run this on a device/simulator in my environment (no full Xcode available), so this has not yet been verified at runtime — relying on CI to compile. The change is small, follows the existing
cancelNotificationsForMessageIdspattern in the same file, and reusesuserInfotags that are already populated. Runtime confirmation on a device (generate channel + DM notifications, background the app, open each conversation, confirm only that conversation's notifications clear) would be appreciated before merge. Automated coverage ofUNUserNotificationCenterdelivery isn't practical in the unit-test target.Summary by CodeRabbit