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

Direct notifications from apps to omi with mentor example #1560

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

tiagoefreitas
Copy link

@tiagoefreitas tiagoefreitas commented Dec 19, 2024

Implements #1434

The example in the mentor app sends a reminder to the user 1 minute after the last mentor notification saying "Hey! How's it going with my previous suggestion? Have you had a chance to try it out?".

This could be improved further by merging PR #1440 , that way the app could save the last notification sent and create a personalized message reminding the user about the previous suggestion and asking how its going.

For authentication:
We send the app if to the app transcriptions webhook, the app should save the id.

When the apps call the /notification webhook on the omi api, they need to provide the app base url (same as configure in the webhook) as the http referrer.
And they need to send the uid and the aid (app id).
If the app is not installed on that user or the url doesn't match, the request fails.

We could implement more robust api key authentication, but it would make it more complex for the developers, and this is secure now because only the app can receive the app id from the backend. Effectively the app id serves as api key, that can be revoked by just uninstalling the app.

@tiagoefreitas
Copy link
Author

tiagoefreitas commented Dec 19, 2024

One improvement that can be made is making an Omi SDK for the backend, currently there is one implemented as class FriendClient in plugins/example/zapier/client.py but it should be moved to /plugins and renamed to OmiClient or OmiSDK so apps can use all available backend functions, and add more like getting user facts etc.

Also the auth needs to be implemented like I did in this PR, the current FriendClient SDK for Zapier uses a hardcoded WORKFLOW_API_KEY in the backed, so it cannot be used by app developers like I did in this PR.

I suggest a bounty to make a proper Omi SDK for a wider backed API, using my auth method or an API key generator.

@mdmohsin7
Copy link
Collaborator

Hi @tiagoefreitas, can we pls have your notification logic as a completely different endpoint and function instead of mixing it in send_notification_to_user. Current endpoint is being used in our admin dashboard for specific communications to the user. I guess keeping both separate will make more sense and also easy to understand

@tiagoefreitas
Copy link
Author

@mdmohsin7 I moved it to /v1//integrations/notifications because integrations has other public apis used by the zapier app.
I still think we should create an Omi SDK for apps to use instead of calling the endpoints directly so that we can handle auth in the SDK and change it later if needed, and make it easier for app developers to see all available public APIs they can use.

@kodjima33 if you want me to do the SDK, please create a separate issue

Also should I add the ability to use prompt templates in the direct notifications? Right now it only supports direct messages, which gives more control to devs, but prompts with the args may be easier for simpler apps.

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.

2 participants