Fix #82: Filter blocked contacts from notification query - #168
Fix #82: Filter blocked contacts from notification query#168navneetsingh-dev wants to merge 3 commits into
Conversation
The getNotificationQuerySql method was previously fetching all unread incoming messages, including those from blocked contacts. This commit injects a ParticipantColumns.BLOCKED = 0 check to ensure blocked messages do not trigger system notifications or audio alerts.
navneetsingh-dev
left a comment
There was a problem hiding this comment.
all the changes made in the file are functional and had finally resolved
RankoR
left a comment
There was a problem hiding this comment.
The normal SMS path already calculates blocked and stores the message with:
seen = read || messageInObservableConversation || blocked
Therefore blocked SMS messages already fail the existing seen = 0 condition. When the query returns no rows, BugleNotifications.createMessageNotification() enters its state == null branch and independently calls playObservableConversationNotificationSound() whenever the conversation is observable.
I reproduced this with the PR installed as the default SMS app:
- block sender;
- open the blocked conversation;
- receive another SMS;
- database row:
seen=1,blocked=1; - old and new query predicates both match zero rows;
- log reports
No unseen notifications; - there is no active Messaging notification;
MediaPlayer/AudioTrackstill runs and delivers the notification audio.
The issue also covers secondary profiles, but SmsReceiver.onReceive() directly calls postNewMessageSecondaryUserNotification() before this database query and without checking the sender's blocked state.
Please retain the query filter if desired, but gate the observable-conversation sound and secondary-user notification using the receive-time, per-sender blocked decision. ProcessDownloadedMmsAction should also include blockedSender when setting mms.mSeen.
Regression tests should cover blocked/unblocked SMS while focused and in the background, secondary profiles, manually downloaded MMS, and per-sender blocking in group conversations.
|
Thanks for the incredibly detailed review and for mapping out the reproduction steps! I clearly see the architectural gap now: while the SQL filter handles the standard notification query, the observable-conversation audio and the secondary-profile broadcast logic are bypassing it. Here is my plan for the next commit: 1.Retain the current SQL query optimization for standard view consistency. 2.Gate the playObservableConversationNotificationSound in BugleNotifications by checking the sender's blocked status. 3.Gate postNewMessageSecondaryUserNotification in SmsReceiver.onReceive() using the receive-time blocked decision. 4.Update ProcessDownloadedMmsAction so that mms.mSeen accurately includes blockedSender. I will get to work on these additions and make sure to run the full regression testing matrix (foreground/background, secondary profiles, manual MMS, and group thread blocking) before pushing the updated branch. Thanks again for the guidance! |
|
Thanks for the incredibly detailed review and for mapping out the reproduction steps! I clearly see the architectural gap now: while the SQL filter handles the standard notification query, the observable-conversation audio and the secondary-profile broadcast logic are bypassing it. I have pushed a new commit that: 1.Retains the current SQL query optimization for standard view consistency. 2,Gates the playObservableConversationNotificationSound in BugleNotifications by checking the sender's blocked status. 3.Gates postNewMessageSecondaryUserNotification in SmsReceiver.onReceive() using the receive-time blocked decision. 4.Updates ProcessDownloadedMmsAction so that mms.mSeen accurately includes blockedSender. I've run the regression testing on my end and everything is behaving properly. Thanks again for the guidance! |
RankoR
left a comment
There was a problem hiding this comment.
There's also no regression tests. Please add.
| if (messages != null && messages.length > 0) { | ||
| final String senderAddress = messages[0].getOriginatingAddress(); | ||
| if (senderAddress != null) { | ||
| final com.android.messaging.datamodel.DatabaseWrapper db = com.android.messaging.datamodel.DataModel.get().getDatabase(); |
There was a problem hiding this comment.
FQDN here (and in the other places) is redundant
| } | ||
| if (isSenderBlocked) { | ||
| postNewMessageSecondaryUserNotification(); | ||
| }else{ |
|
|
||
| } | ||
| } | ||
| if (isSenderBlocked) { |
There was a problem hiding this comment.
Will it post a notification for a blocked contact for a secondary user?
| final String senderAddress = messages[0].getOriginatingAddress(); | ||
| if (senderAddress != null) { | ||
| final com.android.messaging.datamodel.DatabaseWrapper db = com.android.messaging.datamodel.DataModel.get().getDatabase(); | ||
| isSenderBlocked = com.android.messaging.datamodel.BugleDatabaseOperations.isBlockedDestination(db, senderAddress); |
There was a problem hiding this comment.
isBlockedDestination is annotated @DoesNotRunOnMainThread .
Also:
isBlockedDestination()queriesNORMALIZED_DESTINATION, but the PR supplies rawgetOriginatingAddress(). The production receive action first constructsParticipantDataand passesrawSender.getNormalizedDestination(). Number-format differences can therefore make blocked SMS appear unblocked.- The same code handles
"android.provider.Telephony.MMS_DOWNLOADED"by callingSms.Intents.getMessagesFromIntent(). An MMS-downloaded broadcast is not an SMS-PDU broadcast, so this does not provide the MMS sender. At the current inverted condition, ordinary MMS notifications are suppressed; merely negating the condition would restore notifications but still notify for blocked MMS.
| final Uri ringtoneUri = getNotificationRingtoneUriForConversationId(conversationId); | ||
| playObservableConversationNotificationSound(ringtoneUri); | ||
|
|
||
| boolean isBlocked = false; |
There was a problem hiding this comment.
For an observable conversation, the PR queries every participant and suppresses sound if any participant in the conversation is blocked.
Concrete path:
- Group contains blocked participant A and unblocked participant B.
- B sends an MMS while the group conversation is observable.
- The message is marked seen because the conversation is observable, so notification state is null.
- The new participant query finds blocked A.
- B’s valid soft sound is suppressed.
Blocking is sender-specific; it should not silence unrelated senders in the same group. The existing notification SQL correctly joins the message’s actual SENDER_PARTICIPANT_ID, but the new soft-sound path loses that event-specific identity.
Pass the receive-time blocked decision or actual sender participant ID into the sound decision instead of reducing all conversation participants to any blocked.
| gradlePluginPortal() | ||
| } | ||
| } | ||
| plugins { |
The getNotificationQuerySql method was previously fetching all unread incoming messages, including those from blocked contacts. This commit injects a ParticipantColumns.BLOCKED = 0 check to ensure blocked messages do not trigger system notifications or audio alerts.