refactor: generate notification incoming/missed call - WPB-11664 #2397
Conversation
echoes-hq
bot
added
echoes: technical-roadmap
jullianm
changed the title
Test Results 3 files 4 suites 4m 41s ⏱️ Results for commit 834df05. |
Datadog ReportBranch report: ✅ 0 Failed, 3203 Passed, 28 Skipped, 2m 16.62s Total Time |
echoes-hq
bot
removed
the
echoes: technical-roadmap
| static func decode(from calling: Calling) -> Self? { | ||
| let decoder = JSONDecoder() | ||
|
|
||
| guard let data = calling.content.data(using: .utf8) else { | ||
| return nil | ||
| } | ||
|
|
||
| do { | ||
| return try decoder.decode(Self.self, from: data) | ||
| } catch { | ||
| return nil | ||
| } | ||
| } |
There was a problem hiding this comment.
question: why do we silence error here?
| let isAVSReady = userDefaults.bool(forKey: "isAVSReady") | ||
| let isCallKitReady = userDefaults.bool(forKey: "isCallKitAvailable") | ||
| let loadedUserSessions = userDefaults.object(forKey: "loadedUserSessions") as? [String] ?? [] |
| let isStartCall = callContent.type.isOne(of: ["SETUP", "GROUPSTART", "CONFSTART"]) | ||
| let isIncomingCall = isStartCall && !callContent.resp | ||
| let isEndCall = callContent.type.isOne(of: ["CANCEL", "GROUPEND", "CONFEND"]) |
There was a problem hiding this comment.
suggestion: this is common to callkitNOtificationBuilder, using constants for types could help making sure we use the same
| case .missedCall: | ||
| buildMissedCallNotification() | ||
| case .unhandled: | ||
| fatalError() |
There was a problem hiding this comment.
suggestion: let's log a warning here instead of crashing
| // if not we fallback to the regular call notification builder. | ||
| if let callKitBuilder, await callKitBuilder.shouldBuildNotification() { | ||
| return callKitBuilder | ||
| } else if let callNotifBuilder = await makeCallRegularNotificationBuilder( |
There was a problem hiding this comment.
nitpic: don't shorten variable
| } else if let callNotifBuilder = await makeCallRegularNotificationBuilder( | |
| } else if let callNotificationBuilder = await makeCallRegularNotificationBuilder( |
| ) | ||
| } | ||
|
|
||
| func shouldBuildNotification() async -> Bool { | ||
| !context.isMessageSilenced | ||
| !context.isMessageSilenced && !context.isConversationReadOnly |
There was a problem hiding this comment.
thought: is it related? it makes me think about WPB-8946 (bug: notifications shown even though availability is busy or away).
|
|
||
| } | ||
|
|
||
| func testGenerateCallNotification_Should_Build_Notification_Returns_False() async { |
There was a problem hiding this comment.
| func testGenerateCallNotification_Should_Build_Notification_Returns_False() async { | |
| func testGenerateCallNotification_IsOneOnOne_Team_Should_Build_Notification_Returns_False() async { |
Key points
This PR is about generating notification content related to calling events (incoming call, missed call) which can be either handled by the
CallKitNotificationBuilderwhich will show the built-in CallKit notification or by theCallNotificationBuilderin case we only need to show a regular push notification.Logic to decide which builder to use is done through the
shouldBuildNotification()method in order to be consistent with the new NSE structure.Testing
CallKitnotification is correctly generated.Checklist
[WPB-XXX].