-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Add Airship Notifications for Mobile Clients #288
Comments
To clarify, this is both to deliver notifications that users should see and tap on, but also to just push out background updates whenever there are new messages (so they are all stored locally and ready to go), right? |
Or are we just counting on re-synchronizing all the reports when you open up the app and reconnect? |
I'm less certain if Airship lets us add data to a React Native app like that. But we can look into it. |
There seem to be ways to background fetch data so maybe it ends up being a combination of two libs if we can't do it with Airship alone. Uncharted waters for me! Maybe @roryabraham has some ideas? https://github.com/transistorsoft/react-native-background-fetch |
I think Airship alone would probably be sufficient. From the Airship docs:
There is an event that is triggered when a push-notification is received, so if we wanted to silently update the app's internal data without notifying the user, we could send an "invisible notification", and setup a callback for the PushReceived event which updates the Ion data while the app is backgrounded. We could do the same thing when visible push-notifications are received, if we wanted to. The only thing I'm unsure about is what type of data can be passed with a push-notification (i.e: if could send a report JSON with an invisible push-notification) |
Yes! There are limits to the size of the payload. We currently unset the report action when its too large. However, we still send the reportID so we know that the app needs to fetch new info from the server. So one idea would be to send our notification with a simple instruction like |
@roryabraham - How are things going with this one? |
@AndrewGable Working on it now |
Ok awesome, if you need any API keys and/or configuration help in UrbanAirship I am a ping away! |
So far, I've added the airship library, and followed the setup instructions for iOS and Android. I just took the API keys from Mobile, so we should be good there. So now, I'm at the point where I'll actually start writing code, and this is my plan: For background info, check out this PR on Browser (web/desktop) notifications, but TL;DR Currently, there is some desktop-specific notification code in Instead, I'll port the code from |
To clarify, is the goal to do as @marcaaron suggests here:
If so, can you elaborate more precisely on what happens? A couple questions:
Given those assumptions, am I correct in assuming the basic flow is:
Is all that about right? |
Answers to the first 6 questions in slack here.
I was thinking that, more generally, each silent notification can just trigger any action, providing the name of the action and any parameters. So, if a new report action was created, we could send a push notification with the data: {
"action": "REPORT_FETCH_ACTIONS",
"reportID": 746,
} Which would trigger Report::fetchActions. If a new report was created, we could just send: {
"action": "REPORT_FETCH_ALL",
}
This isn't set up, but sounds like a good idea 👍 |
@roryabraham do you have an update on this? It's one of the only CRITICAL issues on the milestone and @chiragsalian is waiting for it so he can work on #331 (comment) |
Yeah, so this is prettymuch ready in that the Airship library has been added to the codebase. I've been holding off on writing more code, pending some consensus from people on the points outlined in the TODO/RequestForComments on the PR. Also see this issue for discussion of the main decision that needs to be made. |
Similar to #285
But for getting Airship device notifications to work on native devices.
The text was updated successfully, but these errors were encountered: