Skip to content

Commit

Permalink
Improve open action handling for notifications
Browse files Browse the repository at this point in the history 
Push notifications were not opened reliably. To improve robustness, the
following changes were introduced:
1. The notification opening logic was updated to become more similar to
   URL handling, in a way that uses better defined interfaces and
   functions that provide better result guarantees, by separating
   complex handling logic, and the side-effects/mutations that
   are made after computing the open action — instead of relying on a
   complex logic function that produces side-effects as a result, which
   obfuscates the actual behavior of the function.
2. The LoadableThreadView was expanded and renamed to
   LoadableNostrEventView, to reflect that it can also handle non-thread
   nostr events, such as DMs, which is a necessity for handling push
   notifications.
3. A new type of Notify object, the `QueueableNotify` was introduced, to
   address issues where the listener/handler is not instantiated at the
   time the app notifies that there is a push notification to be opened.
   This was implemented using async streams, which simplifies the usage
   of this down to a simple "for-in" loop.

Closes: #2825
Changelog-Fixed: Fixed issue where some push notifications would not open in the app and leave users confused
Signed-off-by: Daniel D’Aquino <[email protected]>
  • Loading branch information
@danieldaquino
danieldaquino committed Feb 21, 2025
1 parent 1ab9b30 commit 4324b18
Show file tree
Hide file tree
Showing 9 changed files with 453 additions and 291 deletions.
24 changes: 16 additions & 8 deletions damus.xcodeproj/project.pbxproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1048,6 +1048,9 @@
D706C5AF2D5D31C20027C627 /* AutoSaveIndicatorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5AE2D5D31B20027C627 /* AutoSaveIndicatorView.swift */; };
D706C5B02D5D31C20027C627 /* AutoSaveIndicatorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5AE2D5D31B20027C627 /* AutoSaveIndicatorView.swift */; };
D706C5B12D5D31C20027C627 /* AutoSaveIndicatorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5AE2D5D31B20027C627 /* AutoSaveIndicatorView.swift */; };
D706C5B72D602A110027C627 /* QueueableNotify.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5B62D602A050027C627 /* QueueableNotify.swift */; };
D706C5B82D602A110027C627 /* QueueableNotify.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5B62D602A050027C627 /* QueueableNotify.swift */; };
D706C5B92D602A110027C627 /* QueueableNotify.swift in Sources */ = {isa = PBXBuildFile; fileRef = D706C5B62D602A050027C627 /* QueueableNotify.swift */; };
D70A3B172B02DCE5008BD568 /* NotificationFormatter.swift in Sources */ = {isa = PBXBuildFile; fileRef = D70A3B162B02DCE5008BD568 /* NotificationFormatter.swift */; };
D70D90982CDED61800CD0534 /* CodeScanner in Frameworks */ = {isa = PBXBuildFile; productRef = D70D90972CDED61800CD0534 /* CodeScanner */; };
D70D909C2CDED7B200CD0534 /* CodeScanner in Frameworks */ = {isa = PBXBuildFile; productRef = D70D909B2CDED7B200CD0534 /* CodeScanner */; };
Expand Down Expand Up @@ -1451,9 +1454,9 @@
D74EA08F2D2E271E002290DD /* ErrorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA08D2D2E271E002290DD /* ErrorView.swift */; };
D74EA0902D2E271E002290DD /* ErrorView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA08D2D2E271E002290DD /* ErrorView.swift */; };
D74EA0912D2E3464002290DD /* URLHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = D767066E2C8BB3CE00F09726 /* URLHandler.swift */; };
D74EA0932D2E77B9002290DD /* LoadableThreadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableThreadView.swift */; };
D74EA0942D2E77B9002290DD /* LoadableThreadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableThreadView.swift */; };
D74EA0952D2E77B9002290DD /* LoadableThreadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableThreadView.swift */; };
D74EA0932D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableNostrEventView.swift */; };
D74EA0942D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableNostrEventView.swift */; };
D74EA0952D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74EA0922D2E77B9002290DD /* LoadableNostrEventView.swift */; };
D74F430A2B23F0BE00425B75 /* DamusPurple.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74F43092B23F0BE00425B75 /* DamusPurple.swift */; };
D74F430C2B23FB9B00425B75 /* StoreObserver.swift in Sources */ = {isa = PBXBuildFile; fileRef = D74F430B2B23FB9B00425B75 /* StoreObserver.swift */; };
D753CEAA2BE9DE04001C3A5D /* MutingTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D753CEA92BE9DE04001C3A5D /* MutingTests.swift */; };
Expand Down Expand Up @@ -2420,6 +2423,7 @@
D703D7262C66E47100A400EA /* highlighter action extension.entitlements */ = {isa = PBXFileReference; lastKnownFileType = text.plist.entitlements; path = "highlighter action extension.entitlements"; sourceTree = "<group>"; };
D703D72A2C66F29500A400EA /* getSelection.js */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.javascript; path = getSelection.js; sourceTree = "<group>"; };
D706C5AE2D5D31B20027C627 /* AutoSaveIndicatorView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = AutoSaveIndicatorView.swift; sourceTree = "<group>"; };
D706C5B62D602A050027C627 /* QueueableNotify.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = QueueableNotify.swift; sourceTree = "<group>"; };
D70A3B162B02DCE5008BD568 /* NotificationFormatter.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NotificationFormatter.swift; sourceTree = "<group>"; };
D7100C552B76F8E600C59298 /* PurpleViewPrimitives.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = PurpleViewPrimitives.swift; sourceTree = "<group>"; };
D7100C572B76FC8400C59298 /* MarketingContentView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MarketingContentView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -2453,7 +2457,7 @@
D74AAFD32B155ECB006CF0F4 /* Zaps+.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "Zaps+.swift"; sourceTree = "<group>"; };
D74AAFD52B155F0C006CF0F4 /* WalletConnect+.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = "WalletConnect+.swift"; sourceTree = "<group>"; };
D74EA08D2D2E271E002290DD /* ErrorView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ErrorView.swift; sourceTree = "<group>"; };
D74EA0922D2E77B9002290DD /* LoadableThreadView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableThreadView.swift; sourceTree = "<group>"; };
D74EA0922D2E77B9002290DD /* LoadableNostrEventView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = LoadableNostrEventView.swift; sourceTree = "<group>"; };
D74F43092B23F0BE00425B75 /* DamusPurple.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = DamusPurple.swift; sourceTree = "<group>"; };
D74F430B2B23FB9B00425B75 /* StoreObserver.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = StoreObserver.swift; sourceTree = "<group>"; };
D753CEA92BE9DE04001C3A5D /* MutingTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MutingTests.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -3177,7 +3181,7 @@
D783A63E2AD4E53D00658DDA /* SuggestedHashtagsView.swift */,
D77BFA0A2AE3051200621634 /* ProfileActionSheetView.swift */,
D71AD8FC2CEC176A002E2C3C /* AppAccessibilityIdentifiers.swift */,
D74EA0922D2E77B9002290DD /* LoadableThreadView.swift */,
D74EA0922D2E77B9002290DD /* LoadableNostrEventView.swift */,
);
path = Views;
sourceTree = "<group>";
Expand Down Expand Up @@ -3355,6 +3359,7 @@
4CA3529C2A76AE47003BB08B /* Notify */ = {
isa = PBXGroup;
children = (
D706C5B62D602A050027C627 /* QueueableNotify.swift */,
D7EB00AF2CD59C8300660C07 /* PresentFullScreenItemNotify.swift */,
4C86F7C52A76C51100EC0817 /* AttachedWalletNotify.swift */,
4C9D6D152B1AA9C6004E5CD9 /* DisplayTabBarNotify.swift */,
Expand Down Expand Up @@ -4624,6 +4629,7 @@
9CA876E229A00CEA0003B9A3 /* AttachMediaUtility.swift in Sources */,
D734B1452CCC19B1000B5C97 /* DamusFullScreenCover.swift in Sources */,
4C4E137D2A76D63600BDD832 /* UnmuteThreadNotify.swift in Sources */,
D706C5B72D602A110027C627 /* QueueableNotify.swift in Sources */,
4CE4F0F829DB7399005914DB /* ThiccDivider.swift in Sources */,
4CFF8F5929C9FD1E008DB934 /* DamusPurpleView.swift in Sources */,
4CE0E2B629A3ED5500DB4CA2 /* InnerTimelineView.swift in Sources */,
Expand Down Expand Up @@ -4718,7 +4724,7 @@
4CA927612A290E340098A105 /* EventShell.swift in Sources */,
4C363AA428296DEE006E126D /* SearchModel.swift in Sources */,
4C8D00CC29DF92DF0036AF10 /* Hashtags.swift in Sources */,
D74EA0942D2E77B9002290DD /* LoadableThreadView.swift in Sources */,
D74EA0942D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */,
4CEE2AF3280B25C500AB5EEF /* ProfilePicView.swift in Sources */,
4CC7AAF6297F1A6A00430951 /* EventBody.swift in Sources */,
D76556D62B1E6C08001B0CCC /* DamusPurpleWelcomeView.swift in Sources */,
Expand Down Expand Up @@ -4989,7 +4995,7 @@
82D6FAEC2CD99F7900C925F4 /* OnlyZapsNotify.swift in Sources */,
82D6FAED2CD99F7900C925F4 /* PostNotify.swift in Sources */,
82D6FAEE2CD99F7900C925F4 /* PresentSheetNotify.swift in Sources */,
D74EA0932D2E77B9002290DD /* LoadableThreadView.swift in Sources */,
D74EA0932D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */,
82D6FAEF2CD99F7900C925F4 /* ProfileUpdatedNotify.swift in Sources */,
82D6FAF02CD99F7900C925F4 /* ReportNotify.swift in Sources */,
82D6FAF12CD99F7900C925F4 /* ScrollToTopNotify.swift in Sources */,
Expand Down Expand Up @@ -5350,6 +5356,7 @@
82D6FC522CD99F7900C925F4 /* DMView.swift in Sources */,
82D6FC532CD99F7900C925F4 /* EmptyTimelineView.swift in Sources */,
82D6FC542CD99F7900C925F4 /* EmptyUserSearchView.swift in Sources */,
D706C5B82D602A110027C627 /* QueueableNotify.swift in Sources */,
82D6FC552CD99F7900C925F4 /* EventView.swift in Sources */,
82D6FC562CD99F7900C925F4 /* EventDetailView.swift in Sources */,
82D6FC572CD99F7900C925F4 /* FollowButtonView.swift in Sources */,
Expand Down Expand Up @@ -5526,7 +5533,7 @@
D73E5E9C2C6A97F4007EB227 /* Reply.swift in Sources */,
D73E5E9D2C6A97F4007EB227 /* SearchModel.swift in Sources */,
D73E5E9E2C6A97F4007EB227 /* NostrFilter+Hashable.swift in Sources */,
D74EA0952D2E77B9002290DD /* LoadableThreadView.swift in Sources */,
D74EA0952D2E77B9002290DD /* LoadableNostrEventView.swift in Sources */,
D73E5F912C6AA71B007EB227 /* InputDismissKeyboard.swift in Sources */,
D73E5E9F2C6A97F4007EB227 /* CreateAccountModel.swift in Sources */,
D73E5EA12C6A97F4007EB227 /* SignalModel.swift in Sources */,
Expand All @@ -5553,6 +5560,7 @@
D73E5EB42C6A97F4007EB227 /* NoteContent.swift in Sources */,
D73E5EB52C6A97F4007EB227 /* LongformEvent.swift in Sources */,
D73E5EB62C6A97F4007EB227 /* PushNotificationClient.swift in Sources */,
D706C5B92D602A110027C627 /* QueueableNotify.swift in Sources */,
D71AD8FD2CEC176A002E2C3C /* AppAccessibilityIdentifiers.swift in Sources */,
D73E5EB72C6A97F4007EB227 /* HighlightEvent.swift in Sources */,
D73E5EB82C6A97F4007EB227 /* RelayConnection.swift in Sources */,
Expand Down
98 changes: 54 additions & 44 deletions damus/ContentView.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -222,12 +222,6 @@ struct ContentView: View {
navigationCoordinator.push(route: Route.Script(script: model))
}

func open_profile(pubkey: Pubkey) {
let profile_model = ProfileModel(pubkey: pubkey, damus: damus_state!)
let followers = FollowersModel(damus_state: damus_state!, target: pubkey)
navigationCoordinator.push(route: Route.Profile(profile: profile_model, followers: followers))
}

func open_search(filt: NostrFilter) {
let search = SearchModel(state: damus_state!, search: filt)
navigationCoordinator.push(route: Route.Search(search: search))
Expand Down Expand Up @@ -312,6 +306,9 @@ struct ContentView: View {
hasSeenOnboardingSuggestions = true
}
self.appDelegate?.state = damus_state
Task { // We probably don't need this to be a detached task. According to https://docs.swift.org/swift-book/documentation/the-swift-programming-language/concurrency/#Defining-and-Calling-Asynchronous-Functions, awaits are only suspension points that do not block the thread.
await self.listenAndHandleLocalNotifications()
}
}
.sheet(item: $active_sheet) { item in
switch item {
Expand Down Expand Up @@ -513,27 +510,6 @@ struct ContentView: View {
@unknown default:
break
}
}
.onReceive(handle_notify(.local_notification)) { local in
guard let damus_state else { return }

switch local.mention {
case .pubkey(let pubkey):
open_profile(pubkey: pubkey)

case .note(let noteId):
openEvent(noteId: noteId, notificationType: local.type)
case .nevent(let nevent):
openEvent(noteId: nevent.noteid, notificationType: local.type)
case .nprofile(let nprofile):
open_profile(pubkey: nprofile.author)
case .nrelay(_):
break
case .naddr(let naddr):
break
}


}
.onReceive(handle_notify(.onlyzaps_mode)) { hide in
home.filter_events()
Expand Down Expand Up @@ -643,6 +619,28 @@ struct ContentView: View {
self.selected_timeline = timeline
}

/// Listens to requests to open a push/local user notification
///
/// This function never returns, it just keeps streaming
func listenAndHandleLocalNotifications() async {
for await notification in await QueueableNotify<LossyLocalNotification>.shared.stream {
self.handleNotification(notification: notification)
}
}

func handleNotification(notification: LossyLocalNotification) {
Log.info("ContentView is handling a notification", for: .push_notifications)
guard let damus_state else {
// This should never happen because `listenAndHandleLocalNotifications` is called after damus state is initialized in `onAppear`
assertionFailure("DamusState not loaded when ContentView (new handler) was handling a notification")
Log.error("DamusState not loaded when ContentView (new handler) was handling a notification", for: .push_notifications)
return
}
let local = notification
let openAction = local.toViewOpenAction()
self.execute_open_action(openAction)
}

func connect() {
// nostrdb
var mndb = Ndb()
Expand Down Expand Up @@ -748,23 +746,6 @@ struct ContentView: View {
damus_state.postbox.send(ev)
}
}

private func openEvent(noteId: NoteId, notificationType: LocalNotificationType) {
guard let target = damus_state.events.lookup(noteId) else {
return
}

switch notificationType {
case .dm:
selected_timeline = .dms
damus_state.dms.set_active_dm(target.pubkey)
navigationCoordinator.push(route: Route.DMChat(dms: damus_state.dms.active_model))
case .like, .zap, .mention, .repost, .reply, .tagged:
open_event(ev: target)
case .profile_zap:
break
}
}

/// An open action within the app
/// This is used to model, store, and communicate a desired view action to be taken as a result of opening an object,
Expand Down Expand Up @@ -1218,6 +1199,35 @@ func handle_post_notification(keypair: FullKeypair, postbox: PostBox, events: Ev
}
}

extension LossyLocalNotification {
/// Computes a view open action from a mention reference.
/// Use this when opening a user-presentable interface to a specific mention reference.
func toViewOpenAction() -> ContentView.ViewOpenAction {
switch self.mention {
case .pubkey(let pubkey):
return .route(.ProfileByKey(pubkey: pubkey))
case .note(let noteId):
return .route(.LoadableNostrEvent(note_reference: .note_id(noteId)))
case .nevent(let nEvent):
// TODO: Improve this by implementing a route that handles nevents with their relay hints.
return .route(.LoadableNostrEvent(note_reference: .note_id(nEvent.noteid)))
case .nprofile(let nProfile):
// TODO: Improve this by implementing a profile route that handles nprofiles with their relay hints.
return .route(.ProfileByKey(pubkey: nProfile.author))
case .nrelay(let string):
// We do not need to implement `nrelay` support, it has been deprecated.
// See https://github.com/nostr-protocol/nips/blob/6e7a618e7f873bb91e743caacc3b09edab7796a0/BREAKING.md?plain=1#L21
return .sheet(.error(ErrorView.UserPresentableError(
user_visible_description: NSLocalizedString("You opened an invalid link. The link you tried to open refers to \"nrelay\", which has been deprecated and is not supported.", comment: "User-visible error description for a user who tries to open a deprecated \"nrelay\" link."),
tip: NSLocalizedString("Please contact the person who provided the link, and ask for another link.", comment: "User-visible tip on what to do if a link contains a deprecated \"nrelay\" reference."),
technical_info: "`MentionRef.toViewOpenAction` detected deprecated `nrelay` contents"
)))
case .naddr(let nAddr):
return .route(.LoadableNostrEvent(note_reference: .naddr(nAddr)))
}
}
}


func logout(_ state: DamusState?)
{
Expand Down
4 changes: 2 additions & 2 deletions damus/Models/URLHandler.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ struct DamusURLHandler {
let thread = await ThreadModel(event: nostrEvent, damus_state: damus_state)
return .route(.Thread(thread: thread))
case .event_reference(let event_reference):
return .route(.ThreadFromReference(note_reference: event_reference))
return .route(.LoadableNostrEvent(note_reference: event_reference))
case .wallet_connect(let walletConnectURL):
damus_state.wallet.new(walletConnectURL)
return .route(.Wallet(wallet: damus_state.wallet))
Expand Down Expand Up @@ -99,7 +99,7 @@ struct DamusURLHandler {
case profile(Pubkey)
case filter(NostrFilter)
case event(NostrEvent)
case event_reference(LoadableThreadModel.NoteReference)
case event_reference(LoadableNostrEventViewModel.NoteReference)
case wallet_connect(WalletConnectURL)
case script([UInt8])
case purple(DamusPurpleURL)
Expand Down
22 changes: 7 additions & 15 deletions damus/Notify/LocalNotificationNotify.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,11 @@

import Foundation

struct LocalNotificationNotify: Notify {
typealias Payload = LossyLocalNotification
var payload: Payload
}

extension NotifyHandler {
static var local_notification: NotifyHandler<LocalNotificationNotify> {
.init()
}
}

extension Notifications {
static func local_notification(_ payload: LossyLocalNotification) -> Notifications<LocalNotificationNotify> {
.init(.init(payload: payload))
}
extension QueueableNotify<LossyLocalNotification> {
/// A shared singleton for opening local and push user notifications
///
/// ## Implementation notes
///
/// - The queue can only hold one element. This is done because if the user hypothetically opened 10 push notifications and there was a lag, we wouldn't want the app to suddenly open 10 different things.
static let shared = QueueableNotify(maxQueueItems: 1)
}
Loading

0 comments on commit 4324b18

Please sign in to comment.