-
Notifications
You must be signed in to change notification settings - Fork 17
Register push tokens with the 5calls API instead of OneSignal #321
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -89,8 +89,8 @@ dependencies { | |
| implementation 'com.github.PhilJay:MPAndroidChart:v3.0.3' | ||
| implementation 'com.jjoe64:graphview:4.2.2' | ||
|
|
||
| // OneSignal | ||
| implementation 'com.onesignal:OneSignal:5.1.28' | ||
| // Push notifications, sent by our own api | ||
| implementation 'com.google.firebase:firebase-messaging' | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. remove onesignal from 5calls/app/src/main/assets/licenses.html also maybe add firebase if needed? |
||
|
|
||
| // Plausible | ||
| implementation 'com.github.OneBusAway:plausible-android-sdk:3.3' | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,15 +1,16 @@ | ||
| package org.a5calls.android.a5calls.controller; | ||
|
|
||
| import android.Manifest; | ||
| import android.app.Dialog; | ||
| import android.content.DialogInterface; | ||
| import android.os.Bundle; | ||
|
|
||
| import androidx.activity.result.ActivityResultLauncher; | ||
| import androidx.annotation.NonNull; | ||
| import androidx.annotation.Nullable; | ||
| import androidx.appcompat.app.AlertDialog; | ||
| import androidx.fragment.app.DialogFragment; | ||
|
|
||
| import com.onesignal.Continue; | ||
| import com.onesignal.OneSignal; | ||
|
|
||
| import org.a5calls.android.a5calls.FiveCallsApplication; | ||
| import org.a5calls.android.a5calls.R; | ||
|
|
@@ -27,6 +28,17 @@ public static NotificationSettingsDialog newInstance() { | |
|
|
||
| private int mSelectedOption = 0; | ||
|
|
||
| // has to be registered before the fragment starts, so it can't wait until | ||
| // the save button is tapped. Null when the permission isn't needed. | ||
| private ActivityResultLauncher<String> mPermissionRequest; | ||
|
|
||
| @Override | ||
| public void onCreate(@Nullable Bundle savedInstanceState) { | ||
| super.onCreate(savedInstanceState); | ||
| mPermissionRequest = SettingsActivity.createNotificationPermissionRequest( | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| this, isGranted -> {}); | ||
| } | ||
|
|
||
| public NotificationSettingsDialog() { | ||
|
|
||
| } | ||
|
|
@@ -49,9 +61,8 @@ public void onClick(DialogInterface dialogInterface, int i) { | |
| builder.setPositiveButton(R.string.save, new DialogInterface.OnClickListener() { | ||
| @Override | ||
| public void onClick(DialogInterface dialogInterface, int i) { | ||
| if (mSelectedOption == 0) { | ||
| OneSignal.getUser().getPushSubscription().optIn(); | ||
| OneSignal.getNotifications().requestPermission(true, Continue.none()); | ||
| if (mSelectedOption == 0 && mPermissionRequest != null) { | ||
| mPermissionRequest.launch(Manifest.permission.POST_NOTIFICATIONS); | ||
| // TODO(#139): Do not turn on notifications preference if they did not enable | ||
| // permissions. | ||
| } | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,7 +16,6 @@ | |
| import com.google.gson.Gson; | ||
| import com.google.gson.GsonBuilder; | ||
| import com.google.gson.reflect.TypeToken; | ||
| import com.onesignal.OneSignal; | ||
|
|
||
| import org.a5calls.android.a5calls.BuildConfig; | ||
| import org.a5calls.android.a5calls.model.AccountManager; | ||
|
|
@@ -296,9 +295,10 @@ public void onResponse(JSONObject response) { | |
| AccountManager.Instance.setDistrict(mContext, district); | ||
|
|
||
| districtId = state + "-" + district; | ||
| if (OneSignal.isInitialized()) { | ||
| OneSignal.getUser().addTag("districtID", districtId); | ||
| } | ||
| // the api targets notifications by district, so | ||
| // it needs to hear about a change the same way | ||
| // the onesignal tag used to | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. no need to reference onesignal, since we are removing it. |
||
| PushRegistration.INSTANCE.updateDistrict(mContext); | ||
| } | ||
| } catch (JSONException e) { | ||
| e.printStackTrace(); | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| package org.a5calls.android.a5calls.net | ||
|
|
||
| import android.app.Notification | ||
| import android.app.NotificationChannel | ||
| import android.app.NotificationManager | ||
| import android.app.PendingIntent | ||
| import android.content.Context | ||
| import android.content.Intent | ||
| import android.os.Build | ||
| import android.util.Log | ||
| import androidx.core.content.ContextCompat | ||
| import com.google.firebase.messaging.FirebaseMessagingService | ||
| import com.google.firebase.messaging.RemoteMessage | ||
| import org.a5calls.android.a5calls.R | ||
| import org.a5calls.android.a5calls.controller.MainActivity | ||
|
|
||
| /** | ||
| * 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 | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. again remove reference to onesignal, but fine to keep "this does token handling and display" |
||
| * our own Firebase project and our own API. | ||
| */ | ||
| class FiveCallsMessagingService : FirebaseMessagingService() { | ||
| companion object { | ||
| private const val TAG = "FiveCallsMessaging" | ||
|
|
||
| /** | ||
| * Separate from the call reminder channel so people can silence vote | ||
| * alerts without losing their own reminders. | ||
| */ | ||
| const val CHANNEL_ID = "5calls_push_channel" | ||
|
|
||
| private const val NOTIFICATION_ID = 8288 | ||
|
|
||
| /** Custom data the API sends so a tap can open the right message. */ | ||
| const val MESSAGE_ID_KEY = "messageid" | ||
| } | ||
|
|
||
| /** | ||
| * FCM hands us a new token on install, reinstall, restore, and whenever it | ||
| * decides to rotate one. Passing it straight to the API is what keeps our | ||
| * token list from going stale. | ||
| */ | ||
| override fun onNewToken(token: String) { | ||
| super.onNewToken(token) | ||
| PushRegistration.register(applicationContext, token) | ||
| } | ||
|
|
||
| override fun onMessageReceived(message: RemoteMessage) { | ||
| super.onMessageReceived(message) | ||
|
|
||
| // notifications from our API carry a notification block; fall back to | ||
| // the data payload so a data-only send still shows something | ||
| val title = message.notification?.title ?: message.data["title"] | ||
| val body = message.notification?.body ?: message.data["body"] | ||
|
|
||
| if (title.isNullOrEmpty() && body.isNullOrEmpty()) { | ||
| Log.w(TAG, "push with no title or body, nothing to show") | ||
| return | ||
| } | ||
|
|
||
| showNotification(title, body, message.data[MESSAGE_ID_KEY]) | ||
| } | ||
|
|
||
| private fun showNotification(title: String?, body: String?, messageId: String?) { | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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? |
||
| createChannel() | ||
|
|
||
| val intent = Intent(this, MainActivity::class.java).apply { | ||
| putExtra(MainActivity.EXTRA_FROM_NOTIFICATION, true) | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 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. |
||
| if (!messageId.isNullOrEmpty()) { | ||
| putExtra(MESSAGE_ID_KEY, messageId) | ||
| } | ||
| } | ||
|
|
||
| val pendingIntent = PendingIntent.getActivity( | ||
| this, | ||
| MainActivity.NOTIFICATION_REQUEST, | ||
| intent, | ||
| PendingIntent.FLAG_IMMUTABLE or PendingIntent.FLAG_UPDATE_CURRENT | ||
| ) | ||
|
|
||
| val builder = Notification.Builder(this) | ||
| .setContentTitle(title ?: getString(R.string.app_name)) | ||
| .setContentText(body) | ||
| .setStyle(Notification.BigTextStyle().bigText(body)) | ||
| .setContentIntent(pendingIntent) | ||
| .setAutoCancel(true) | ||
| .setSmallIcon(R.drawable.app_icon_bw) | ||
|
|
||
| if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { | ||
| builder.setChannelId(CHANNEL_ID) | ||
| } | ||
|
|
||
| builder.setColor(ContextCompat.getColor(this, R.color.colorPrimary)) | ||
|
|
||
| val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | ||
| manager.notify(NOTIFICATION_ID, builder.build()) | ||
| } | ||
|
|
||
| private fun createChannel() { | ||
| if (Build.VERSION.SDK_INT < Build.VERSION_CODES.O) { | ||
| return | ||
| } | ||
|
|
||
| val manager = getSystemService(Context.NOTIFICATION_SERVICE) as NotificationManager | ||
| val channel = NotificationChannel( | ||
| CHANNEL_ID, | ||
| getString(R.string.push_notification_channel_name), | ||
| NotificationManager.IMPORTANCE_DEFAULT | ||
| ) | ||
|
|
||
| manager.createNotificationChannel(channel) | ||
| } | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this need to change anything about the privacy statement we link to from play store, https://5calls.org/privacy/?