Register push tokens with the 5calls API instead of OneSignal - #321
Register push tokens with the 5calls API instead of OneSignal#321nickoneill wants to merge 3 commits into
Conversation
OneSignal is capping the free plan at 1,000 mobile MAU on Oct 1 and we have far more than that, so the API stores push tokens itself now and will send notifications directly. The FCM tokens were always issued under our own Firebase project rather than OneSignal's, so they keep working once we're the ones sending to them. FiveCallsMessagingService takes over the two jobs the OneSignal SDK was doing: onNewToken hands new tokens to the API, and onMessageReceived builds and shows the notification, which OneSignal used to render for us. It uses its own channel so vote alerts can be silenced separately from call reminders. PushRegistration posts to /v1/push/register with the caller's district, which AccountManager already stores, and re-sends when the district changes the same way the OneSignal tag was updated. Turning notifications off deletes the token rather than just opting out locally. Only firebase-messaging is added; the Firebase BOM, the google-services plugin and google-services.json were already here for auth and analytics. New files are Kotlin, existing Java left as Java. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxrjMYu6big5x2XrR7HyTy
onNewToken only fires when FCM rotates a token, so a registration that failed once, or a district that changed while the device had no network, would stay wrong until the token happened to change. iOS already re-registers on launch via registerIfAuthorized; this brings Android in line. Skipped when notifications are turned off, otherwise the next launch would undo the unregister we send on opt-out. Also skipped when the OS permission isn't granted, which is the existing branch that resets the preference. refreshToken now returns early when Firebase isn't initialized. Calling it from Application.onCreate meant FirebaseMessaging.getInstance() ran in unit tests, where FirebaseApp doesn't exist, and threw through 140 of them. The api upserts on the token, so repeating this costs one request. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxrjMYu6big5x2XrR7HyTy
"Vote alerts" described the iOS inbox, which shows how a rep voted. Android has no inbox; its notifications are alerts about new issues worth calling about, which is how the app already describes them in the opt-in dialog and in disable_new_issues_notification. The channel name is only the label in system notification settings, and it updates on the next createNotificationChannel call, so this reaches existing installs without a new channel. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UxrjMYu6big5x2XrR7HyTy
| @Override | ||
| public void onCreate(@Nullable Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
| mPermissionRequest = SettingsActivity.createNotificationPermissionRequest( |
There was a problem hiding this comment.
android can have some trickiness around permissions -- for example if the user says "no" the first time they see this dialog, what happens the second time? sometimes the system won't show any dialog and the user can't figure it out -- then the 5calls app may need to say "go into your settings and find this app and approve permission for notifications".
That's a long-winded way to say, does this do what we want if the user denies permission the first time and then tries to change add notifications again later?
| } | ||
| // the api targets notifications by district, so | ||
| // it needs to hear about a change the same way | ||
| // the onesignal tag used to |
There was a problem hiding this comment.
no need to reference onesignal, since we are removing it.
|
|
||
| // OneSignal | ||
| implementation 'com.onesignal:OneSignal:5.1.28' | ||
| // Push notifications, sent by our own api |
There was a problem hiding this comment.
does this need to change anything about the privacy statement we link to from play store, https://5calls.org/privacy/?
|
|
||
| <!-- Description of a setting to show an example call in the calls list [CHAR_LIMIT=NONE]--> | ||
| <string name="settings_reshow_demo_description">Show an example issue in the list with hints about how to use 5 Calls.</string> | ||
| <!-- Name of the notification channel for alerts about new issues to call about --> |
There was a problem hiding this comment.
Please add a Spanish lang version too
|
|
||
| /** | ||
| * Receives push notifications from FCM and shows them. OneSignal's SDK used to | ||
| * do both the token handling and the display; this does the same work against |
There was a problem hiding this comment.
again remove reference to onesignal, but fine to keep "this does token handling and display"
| createChannel() | ||
|
|
||
| val intent = Intent(this, MainActivity::class.java).apply { | ||
| putExtra(MainActivity.EXTRA_FROM_NOTIFICATION, true) |
There was a problem hiding this comment.
Optional: we could consider a different extra to track reminders (which currently use extra_from_notification) and these one-off alerts. Would allow us to differentiate in Plausible.
| showNotification(title, body, message.data[MESSAGE_ID_KEY]) | ||
| } | ||
|
|
||
| private fun showNotification(title: String?, body: String?, messageId: String?) { |
There was a problem hiding this comment.
what happens if one of these shows at the same time as the reminder is showing? would the user see 2 at the same time, would the most recent one win, a crash?
|
|
||
| /** | ||
| * Tells the 5calls API about this device's FCM token so it can send us | ||
| * notifications directly, which OneSignal used to do. |
| // OneSignal | ||
| implementation 'com.onesignal:OneSignal:5.1.28' | ||
| // Push notifications, sent by our own api | ||
| implementation 'com.google.firebase:firebase-messaging' |
There was a problem hiding this comment.
remove onesignal from 5calls/app/src/main/assets/licenses.html
also maybe add firebase if needed?
| return "$state-$district" | ||
| } | ||
|
|
||
| private fun send(context: Context, method: Int, body: JSONObject) { |
There was a problem hiding this comment.
so far we've sent all HTTP requests through FiveCallsApi, so that's the only class that needs to know about how to make HTTP requests. Consider having this delegate the actual request/response there?
OneSignal is essentially cutting us off from using the free plan on October 1. We don't really use push regularly right now but to continue ensuring we have access to the data needed to potentially send pushes we need to migrate the new token sending to our own API and away from OneSignal. I wouldn't use OneSignal if we were starting this from scratch anyway since it's so trivial to manage this data and develop a UI for sending.
Claude summary:
OneSignal is capping the free plan at 1,000 mobile MAU on Oct 1 and we have far more than that, so the API stores push tokens itself now and will send notifications directly. The FCM tokens were always issued under our own Firebase project rather than OneSignal's, so they keep working once we're the ones sending to them.
FiveCallsMessagingService takes over the two jobs the OneSignal SDK was doing: onNewToken hands new tokens to the API, and onMessageReceived builds and shows the notification, which OneSignal used to render for us. It uses its own channel so vote alerts can be silenced separately from call reminders.
PushRegistration posts to /v1/push/register with the caller's district, which AccountManager already stores, and re-sends when the district changes the same way the OneSignal tag was updated. Turning notifications off deletes the token rather than just opting out locally.
Only firebase-messaging is added; the Firebase BOM, the google-services plugin and google-services.json were already here for auth and analytics.
New files are Kotlin, existing Java left as Java.
What type of PR is this? (check all applicable)
Were the changes tested?
have not been included