Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add endpoint for registering alby go notifications #128

Merged
merged 7 commits into from
Dec 19, 2024

Conversation

im-adithya
Copy link
Member

@im-adithya im-adithya commented Nov 7, 2024

To register:

Make a POST call to http://localhost:8080/nip47/notifications/go with body:

{
    // "relayUrl": "wss://relay.getalby.com/v1",
    "pushToken": "ExponentPushToken[<YOUR_DEVICE_TOKEN>]",
    "walletPubkey": <WALLET_PUBKEY>,
    "connectionPubkey": <APP_PUBKEY>
}

It'll then start subscribing to all notifications of the app pubkey on the relay and keeps sending notifications to the mobile app

cmd/server/main.go Outdated Show resolved Hide resolved
internal/nostr/expo.go Outdated Show resolved Hide resolved
go svc.startSubscription(svc.Ctx, &subscription, nil, svc.handleSubscribedEvent)
handleEvent := svc.handleSubscribedEvent
if sub.PushToken != "" {
handleEvent = svc.handleSubscribedExpoNotification
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the naming be improved here? we are still handling a subscribed event, right - we're just notifying in a different way?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changed to be simple - handlePushNotification, wdyt?

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's still inconsistent with handleSubscribedEvent

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A bit long but changed to handleSubscribedEventForPushNotification

@@ -50,6 +50,7 @@ func main() {
e.POST("/nip47", svc.NIP47Handler)
e.POST("/nip47/webhook", svc.NIP47WebhookHandler)
e.POST("/nip47/notifications", svc.NIP47NotificationHandler)
e.POST("/nip47/notifications/go", svc.NIP47ExpoNotificationHandler)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

don't use app specific names here.
You implement an universal API here that can also be used by other applications.
(e.g. /nip47/notifications/expo)

Also try to be consistent.
maybe we move the webhook endpoin then to:
/nip47/notifications/webhook

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done 👍

Regarding webhook, it's a one-time notification endpoint, and it would also be a breaking change. Should we do that?

internal/nostr/models.go Outdated Show resolved Hide resolved
@im-adithya
Copy link
Member Author

Merging this since this is already live (and includes a migration to jsonb)

@im-adithya im-adithya merged commit c973253 into main Dec 19, 2024
1 check passed
@bumi bumi deleted the task-expo-notifs branch December 19, 2024 18:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants