-
Notifications
You must be signed in to change notification settings - Fork 2
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
Notification service for Android #737
Conversation
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.
Skeleton looks good. I think it is ready to fill in with details.
Also I think since in contrary to iOS here we get silent push so perhaps we can raise a local notification at the end with the same message we do in iOS.
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.
Looks good.
override fun onMessageReceived(remoteMessage: RemoteMessage) { | ||
Log.d(TAG, "From: ${remoteMessage.from}") | ||
|
||
// Check if message contains a data payload. |
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.
Is this logic in Breez mobile needed to still be able to handle notifications in flutter? https://github.com/breez/breezmobile/blob/master/android/app/src/main/java/com/breez/client/BreezFirebaseMessagingService.java#L48
return ForegroundInfo( | ||
NOTIFICATION_ID_PAYMENT_RECEIVED, | ||
createNotification( | ||
"Incoming payment", |
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.
Is this showing during the payment? If so perhaps "Receiving payment..." ?
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.
Yes, this is the initial message of notification. I was referencing iOS notification, we should change it there as well.
a4e0f56
to
baedeed
Compare
fcfdcde
to
ee0e07e
Compare
- Handle notifications only if app is in the background or killed - Create BreezNotificationService - Create separate notification for each paymentHash - Handle status notification separately - Create JobManager - Rename BreezFCMService to BreezFcmService - Prevent running job for existing payment - Read mnemonics from Keystore - Increase minSdkVersion to 24 - Use shared instance for BreezSdk - Stop job after 1 minute - Declare ndkVersion as flutter.ndkVersion
ee0e07e
to
fc098b5
Compare
ctd. Remove Flutter implementation for background messages & workmanager
Set hydrated_bloc to fix 9.1.2 version due to a regression on 9.1.3
fc098b5
to
0dcbd51
Compare
@roeierez Created a draft PR to make it easier to review progress.