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

Display MayHaveMessagesNotification on push with locked db #238

Merged
merged 4 commits into from
Nov 16, 2023

Conversation

p1gp1g
Copy link
Member

@p1gp1g p1gp1g commented Nov 11, 2023

This works with FCM : If the remote message's priority is high, then the push notification should require a notification :)

Copy link

@pixincreate pixincreate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Other than that, looks good to me.

Just a quick question, this implementation only works for FCM and not UnifiedPush and websockets, right?

@@ -28,6 +28,10 @@ public class FcmReceiveService extends FirebaseMessagingService {
@Override
public void onMessageReceived(RemoteMessage remoteMessage) {
if (KeyCachingService.isLocked()) {
if (remoteMessage != null && remoteMessage.getPriority() == RemoteMessage.PRIORITY_HIGH) {
Log.d(TAG, "New urgent message received while the db is locked");

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
Log.d(TAG, "New urgent message received while the db is locked");
Log.d(TAG, "New high priority message received while the database is locked");

@p1gp1g
Copy link
Member Author

p1gp1g commented Nov 12, 2023

Thanks for you review. For information, urgent is a signal term which is converted by the server to FCM term.

Just a quick question, this implementation only works for FCM and not UnifiedPush and websockets, right?

I've done this for UnifiedPush too : mollyim@9f9d882 but it requires the version 1.1.0 of mollysocket I will publish during the day

@valldrac valldrac merged commit 89ba25a into mollyim:main Nov 16, 2023
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants