Skip to content

Commit b82500c

Browse files
committed
feat(notifications): surface tapback/reaction notifications (#2039)
Receiving a tapback/reaction previously produced no local notification: both notification branches in textMessageAppPacket gated on isEmoji == false, so a reaction was stored but only visible with the conversation already open. Now, when a reaction arrives and the sender/channel isn't muted, look up the reacted-to message locally by messageId == replyID and schedule an iMessage-style notification (e.g. `Alice reacted 👍 to "See you soon"`). If the original message isn't known locally (a "phantom" tapback whose replyID matches nothing), the reaction is still stored but no notification is shown — mirroring Android's rememberReaction guard. - Add MeshPackets.reactionNotificationBody(replyID:emoji:senderName:context:) (static + context-injected so it's unit-testable) which returns nil for phantom tapbacks and formats the localized body otherwise. - Add makeMessageNotification(...) factory to collapse the DM/channel by regular/reaction Notification construction into one path. - Add the reaction body format key to Localizable.xcstrings. Out of scope (unchanged): unread-count/badge behavior. The isEmoji == false filters in the *EntityExtension unread counts stay, so a reaction notifies without bumping the conversation's unread badge (matching iMessage/Google Messages). Adds ReactionNotificationTests (7 tests) covering the phantom-tapback guard (unknown/zero/negative replyID), the formatted body, emoji fallbacks, and the nil-original-payload-still-notifies edge.
1 parent 8e0eaaf commit b82500c

4 files changed

Lines changed: 200 additions & 40 deletions

File tree

Localizable.xcstrings

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
{
22
"sourceLanguage" : "en",
33
"strings" : {
4+
"%1$@ reacted %2$@ to \"%3$@\"" : {
5+
"comment" : "Tapback/reaction notification body. %1$@ is the sender name, %2$@ is the reaction emoji, %3$@ is the original message text that was reacted to."
6+
},
47
"%@ entered %@" : {
58
"comment" : "Geofence notification body. First %@ is the node name, second %@ is the waypoint name."
69
},

Meshtastic.xcodeproj/project.pbxproj

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -333,6 +333,7 @@
333333
DD00003C000SNAP00000001 /* SnapshotTesting in Frameworks */ = {isa = PBXBuildFile; productRef = DD00003B000SNAP00000000 /* SnapshotTesting */; };
334334
DD0000CHSETST0000000001 /* ChannelSetSaveTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD0000CHSETST0000000000 /* ChannelSetSaveTests.swift */; };
335335
DD0000MQTTFWDFLTTST00001 /* MqttForwardFilterTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD0000MQTTFWDFLTTST00000 /* MqttForwardFilterTests.swift */; };
336+
DD0000REACTNOTIFTST00001 /* ReactionNotificationTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD0000REACTNOTIFTST00000 /* ReactionNotificationTests.swift */; };
336337
DD0000DEVLNKTST000000001 /* DeviceLinkTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD0000DEVLNKTST000000000 /* DeviceLinkTests.swift */; };
337338
DD1827FB0000000000000002 /* RegionCodesFirmwareLocaleTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD1827FA0000000000000002 /* RegionCodesFirmwareLocaleTests.swift */; };
338339
DD007BAE2AA4E91200F5FA12 /* MyInfoEntityExtension.swift in Sources */ = {isa = PBXBuildFile; fileRef = DD007BAD2AA4E91200F5FA12 /* MyInfoEntityExtension.swift */; };
@@ -946,6 +947,7 @@
946947
DD00003ASUPPORTLVL0000000 /* SupportLevel.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SupportLevel.swift; sourceTree = "<group>"; };
947948
DD0000CHSETST0000000000 /* ChannelSetSaveTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ChannelSetSaveTests.swift; sourceTree = "<group>"; };
948949
DD0000MQTTFWDFLTTST00000 /* MqttForwardFilterTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MqttForwardFilterTests.swift; sourceTree = "<group>"; };
950+
DD0000REACTNOTIFTST00000 /* ReactionNotificationTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ReactionNotificationTests.swift; sourceTree = "<group>"; };
949951
DD0000DEVLNKTST000000000 /* DeviceLinkTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DeviceLinkTests.swift; sourceTree = "<group>"; };
950952
DD1827FA0000000000000002 /* RegionCodesFirmwareLocaleTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RegionCodesFirmwareLocaleTests.swift; sourceTree = "<group>"; };
951953
DD007BAD2AA4E91200F5FA12 /* MyInfoEntityExtension.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MyInfoEntityExtension.swift; sourceTree = "<group>"; };
@@ -1696,6 +1698,7 @@
16961698
BEEF00102F00000100000003 /* MeshtasticChannelURLTests.swift */,
16971699
DD0000CHSETST0000000000 /* ChannelSetSaveTests.swift */,
16981700
DD0000MQTTFWDFLTTST00000 /* MqttForwardFilterTests.swift */,
1701+
DD0000REACTNOTIFTST00000 /* ReactionNotificationTests.swift */,
16991702
DD1827FA0000000000000002 /* RegionCodesFirmwareLocaleTests.swift */,
17001703
DD000031STATMSGDISP00000 /* StatusMessageDisplayTests.swift */,
17011704
);
@@ -2780,6 +2783,7 @@
27802783
DD0000DEVLNKTST000000001 /* DeviceLinkTests.swift in Sources */,
27812784
DD0000CHSETST0000000001 /* ChannelSetSaveTests.swift in Sources */,
27822785
DD0000MQTTFWDFLTTST00001 /* MqttForwardFilterTests.swift in Sources */,
2786+
DD0000REACTNOTIFTST00001 /* ReactionNotificationTests.swift in Sources */,
27832787
DD1827FB0000000000000002 /* RegionCodesFirmwareLocaleTests.swift in Sources */,
27842788
DD000031STATMSGDISP00001 /* StatusMessageDisplayTests.swift in Sources */,
27852789
AA009MCT0000002000000001 /* MarkdownConverterTests.swift in Sources */,

Meshtastic/Helpers/MeshPackets.swift

Lines changed: 103 additions & 40 deletions
Original file line numberDiff line numberDiff line change
@@ -1121,6 +1121,55 @@ actor MeshPackets {
11211121
}
11221122
}
11231123

1124+
/// Builds the notification body for a received tapback/reaction, or returns `nil` when the
1125+
/// reacted-to message isn't known locally (a "phantom" tapback). The reaction is still stored
1126+
/// as it is today — we just don't surface a notification for something there's nothing to show
1127+
/// for, matching Android's guard (`MeshDataHandlerImpl.rememberReaction`).
1128+
///
1129+
/// Made `static` and context-injected (rather than reading `self.modelContext`) so the
1130+
/// phantom-tapback guard and body formatting can be exercised directly in unit tests.
1131+
static func reactionNotificationBody(replyID: Int64, emoji: String?, senderName: String, context: ModelContext) -> String? {
1132+
guard replyID > 0 else { return nil }
1133+
let descriptor = FetchDescriptor<MessageEntity>(predicate: #Predicate { $0.messageId == replyID })
1134+
guard let original = try? context.fetch(descriptor).first else { return nil }
1135+
let originalText = original.messagePayload ?? ""
1136+
let reactionEmoji = (emoji?.isEmpty == false) ? emoji! : "❤️"
1137+
// Mirrors how iMessage phrases a tapback notification ("Name liked …").
1138+
return String.localizedStringWithFormat(
1139+
"%1$@ reacted %2$@ to \"%3$@\"".localized,
1140+
senderName, reactionEmoji, originalText
1141+
)
1142+
}
1143+
1144+
/// Builds a message/reaction local notification from a (context-bound) `MessageEntity`,
1145+
/// synchronously, so callers can hand the resulting value to a deferred `@MainActor` Task
1146+
/// without capturing the entity itself. Collapses the DM/channel × regular/reaction variants
1147+
/// that differ only in `content`, deep-link `path`, and `userNum`.
1148+
private func makeMessageNotification(
1149+
message: MessageEntity,
1150+
content: String,
1151+
path: String,
1152+
userNum: Int64?,
1153+
critical: Bool
1154+
) -> Notification {
1155+
var notification = Notification(
1156+
id: ("notification.id.\(message.messageId)"),
1157+
title: "\(message.fromUser?.longName ?? "Unknown".localized)",
1158+
subtitle: "AKA \(message.fromUser?.shortName ?? "?")",
1159+
content: content,
1160+
target: "messages",
1161+
path: path,
1162+
messageId: message.messageId,
1163+
channel: message.channel,
1164+
userNum: userNum,
1165+
critical: critical
1166+
)
1167+
#if os(iOS)
1168+
notification.senderIntent = CarPlayIntentDonation.incomingMessageIntent(from: message)
1169+
#endif
1170+
return notification
1171+
}
1172+
11241173
func textMessageAppPacket(
11251174
packet: MeshPacket,
11261175
wantRangeTestPackets: Bool,
@@ -1340,33 +1389,41 @@ actor MeshPackets {
13401389
appState?.unreadDirectMessages = unreadCount
13411390
}
13421391
}
1343-
if !(newMessage.fromUser?.mute ?? false) && newMessage.isEmoji == false {
1392+
if !(newMessage.fromUser?.mute ?? false) {
13441393
// Build the notification from the model objects now, while this context is valid,
13451394
// and capture only the resulting value — a deferred Task must not hold `newMessage`
13461395
// (a context-bound model), since a device switch can reset this context first
13471396
// ("destroyed by ModelContext.reset").
13481397
let senderName = newMessage.fromUser?.longName ?? "Unknown".localized
1349-
var dmNotification = Notification(
1350-
id: ("notification.id.\(newMessage.messageId)"),
1351-
title: "\(senderName)",
1352-
subtitle: "AKA \(newMessage.fromUser?.shortName ?? "?")",
1353-
content: messageText!,
1354-
target: "messages",
1355-
path: "meshtastic:///messages?userNum=\(newMessage.fromUser?.num ?? 0)&messageId=\(newMessage.isEmoji ? newMessage.replyID : newMessage.messageId)",
1356-
messageId: newMessage.messageId,
1357-
channel: newMessage.channel,
1358-
userNum: Int64(packet.from),
1359-
critical: critical
1360-
)
1361-
#if os(iOS)
1362-
dmNotification.senderIntent = CarPlayIntentDonation.incomingMessageIntent(from: newMessage)
1363-
#endif
1364-
let notification = dmNotification
1365-
Task {@MainActor in
1366-
let manager = LocalNotificationManager()
1367-
manager.notifications = [notification]
1368-
manager.schedule()
1369-
Logger.services.debug("iOS Notification Scheduled for text message from \(senderName, privacy: .public)")
1398+
let dmUserNum = Int64(packet.from)
1399+
var dmNotification: Notification?
1400+
if newMessage.isEmoji == false {
1401+
dmNotification = makeMessageNotification(
1402+
message: newMessage,
1403+
content: messageText!,
1404+
path: "meshtastic:///messages?userNum=\(newMessage.fromUser?.num ?? 0)&messageId=\(newMessage.messageId)",
1405+
userNum: dmUserNum,
1406+
critical: critical
1407+
)
1408+
} else if let reactionBody = MeshPackets.reactionNotificationBody(replyID: newMessage.replyID, emoji: messageText, senderName: senderName, context: modelContext) {
1409+
// Tapback/reaction: only notify when the reacted-to message is known locally.
1410+
// A "phantom" tapback (replyID with no matching local message) is stored but not
1411+
// surfaced — reactionNotificationBody returns nil in that case.
1412+
dmNotification = makeMessageNotification(
1413+
message: newMessage,
1414+
content: reactionBody,
1415+
path: "meshtastic:///messages?userNum=\(newMessage.fromUser?.num ?? 0)&messageId=\(newMessage.replyID)",
1416+
userNum: dmUserNum,
1417+
critical: critical
1418+
)
1419+
}
1420+
if let notification = dmNotification {
1421+
Task {@MainActor in
1422+
let manager = LocalNotificationManager()
1423+
manager.notifications = [notification]
1424+
manager.schedule()
1425+
Logger.services.debug("iOS Notification Scheduled for direct message from \(senderName, privacy: .public)")
1426+
}
13701427
}
13711428
}
13721429
} else if newMessage.fromUser != nil && newMessage.toUser == nil {
@@ -1384,25 +1441,31 @@ actor MeshPackets {
13841441
// to ~1/sec — the badge tolerates brief lag and resyncs on app-active and on read.
13851442
let recountUnread = shouldRecomputeChannelUnread()
13861443
let connectedNodeNum = connectedNode
1387-
let shouldNotify = UserDefaults.channelMessageNotifications && newMessage.isEmoji == false && myInfo.channels.contains(where: { $0.index == newMessage.channel && !$0.mute })
1444+
let channelNotificationsEnabled = UserDefaults.channelMessageNotifications && myInfo.channels.contains(where: { $0.index == newMessage.channel && !$0.mute })
1445+
let senderName = newMessage.fromUser?.longName ?? "Unknown".localized
1446+
let channelUserNum = Int64(newMessage.fromUser?.userId ?? "0")
13881447
var channelNotification: Notification?
1389-
if shouldNotify {
1390-
var chNotification = Notification(
1391-
id: ("notification.id.\(newMessage.messageId)"),
1392-
title: "\(newMessage.fromUser?.longName ?? "Unknown".localized)",
1393-
subtitle: "AKA \(newMessage.fromUser?.shortName ?? "?")",
1394-
content: messageText!,
1395-
target: "messages",
1396-
path: "meshtastic:///messages?channelId=\(newMessage.channel)&messageId=\(newMessage.isEmoji ? newMessage.replyID : newMessage.messageId)",
1397-
messageId: newMessage.messageId,
1398-
channel: newMessage.channel,
1399-
userNum: Int64(newMessage.fromUser?.userId ?? "0"),
1400-
critical: critical
1401-
)
1402-
#if os(iOS)
1403-
chNotification.senderIntent = CarPlayIntentDonation.incomingMessageIntent(from: newMessage)
1404-
#endif
1405-
channelNotification = chNotification
1448+
if channelNotificationsEnabled {
1449+
if newMessage.isEmoji == false {
1450+
channelNotification = makeMessageNotification(
1451+
message: newMessage,
1452+
content: messageText!,
1453+
path: "meshtastic:///messages?channelId=\(newMessage.channel)&messageId=\(newMessage.messageId)",
1454+
userNum: channelUserNum,
1455+
critical: critical
1456+
)
1457+
} else if let reactionBody = MeshPackets.reactionNotificationBody(replyID: newMessage.replyID, emoji: messageText, senderName: senderName, context: modelContext) {
1458+
// Tapback/reaction: only notify when the reacted-to message is known
1459+
// locally. A "phantom" tapback is stored but not surfaced — the helper
1460+
// returns nil in that case, per Android's guard.
1461+
channelNotification = makeMessageNotification(
1462+
message: newMessage,
1463+
content: reactionBody,
1464+
path: "meshtastic:///messages?channelId=\(newMessage.channel)&messageId=\(newMessage.replyID)",
1465+
userNum: channelUserNum,
1466+
critical: critical
1467+
)
1468+
}
14061469
}
14071470
let notification = channelNotification
14081471
Task {@MainActor in
Lines changed: 90 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,90 @@
1+
//
2+
// ReactionNotificationTests.swift
3+
// MeshtasticTests
4+
//
5+
// Coverage for issue #2039: receiving a tapback/reaction should surface a local
6+
// notification (it previously produced none), *except* for a "phantom" tapback whose
7+
// replyID doesn't match a locally-known message — that is stored but must not notify,
8+
// matching Android's guard (MeshDataHandlerImpl.rememberReaction).
9+
//
10+
// These exercise `MeshPackets.reactionNotificationBody(replyID:emoji:senderName:context:)`
11+
// directly against an in-memory SwiftData context, so both the phantom guard and the
12+
// iMessage-style body formatting are locked in without spinning up the whole packet path.
13+
//
14+
15+
import Testing
16+
import Foundation
17+
import SwiftData
18+
@testable import Meshtastic
19+
20+
@Suite("Reaction notifications (#2039)")
21+
@MainActor
22+
struct ReactionNotificationTests {
23+
24+
private var context: ModelContext { TestContainerProvider.shared.mainContext }
25+
26+
/// Insert an original (reacted-to) message with a unique id and the given payload.
27+
@discardableResult
28+
private func insertOriginal(id: Int64, payload: String?) -> MessageEntity {
29+
let msg = MessageEntity()
30+
context.insert(msg)
31+
msg.messageId = id
32+
msg.messagePayload = payload
33+
msg.isEmoji = false
34+
try? context.save()
35+
return msg
36+
}
37+
38+
private func body(replyID: Int64, emoji: String?, sender: String = "Bob") -> String? {
39+
MeshPackets.reactionNotificationBody(
40+
replyID: replyID,
41+
emoji: emoji,
42+
senderName: sender,
43+
context: context
44+
)
45+
}
46+
47+
// MARK: - Phantom-tapback guard (the core of the fix)
48+
49+
@Test func phantomTapback_unknownReplyID_returnsNil() {
50+
// No message with id 990001 exists in the store.
51+
#expect(body(replyID: 990_001, emoji: "👍") == nil)
52+
}
53+
54+
@Test func replyIDZero_returnsNil() {
55+
// replyID == 0 means "not a reply/reaction to anything" — never notify.
56+
#expect(body(replyID: 0, emoji: "👍") == nil)
57+
}
58+
59+
@Test func negativeReplyID_returnsNil() {
60+
#expect(body(replyID: -5, emoji: "👍") == nil)
61+
}
62+
63+
// MARK: - Found original -> formatted iMessage-style body
64+
65+
@Test func knownReplyID_formatsBody() {
66+
insertOriginal(id: 990_010, payload: "See you soon")
67+
let result = body(replyID: 990_010, emoji: "👍", sender: "Alice")
68+
#expect(result == "Alice reacted 👍 to \"See you soon\"")
69+
}
70+
71+
@Test func knownReplyID_emptyEmoji_fallsBackToHeart() {
72+
insertOriginal(id: 990_020, payload: "Hello there")
73+
let result = body(replyID: 990_020, emoji: "")
74+
#expect(result == "Bob reacted ❤️ to \"Hello there\"")
75+
}
76+
77+
@Test func knownReplyID_nilEmoji_fallsBackToHeart() {
78+
insertOriginal(id: 990_030, payload: "Hello there")
79+
let result = body(replyID: 990_030, emoji: nil)
80+
#expect(result == "Bob reacted ❤️ to \"Hello there\"")
81+
}
82+
83+
@Test func knownReplyID_nilOriginalPayload_stillNotifies() {
84+
// The original message exists (so it's NOT a phantom tapback) but has no text.
85+
// We still surface a notification; the quoted text is simply empty.
86+
insertOriginal(id: 990_040, payload: nil)
87+
let result = body(replyID: 990_040, emoji: "🎉", sender: "Carol")
88+
#expect(result == "Carol reacted 🎉 to \"\"")
89+
}
90+
}

0 commit comments

Comments
 (0)