Do not notify again for old events #3657
Conversation
Resending the notification here can trigger other system components or apps that listen to new notifications, such as connected smart watches or automation tools. Fixes element-hq#1673
|
@BillCarsonFr WDYT about this change? I though we were building the same notifications each time we want to refresh the navigation drawer, but it's maybe not true |
|
We are a bit scared that this change brings some regressions... |
| if (eventList.isEmpty() || eventList.all { it.isRedacted }) { | ||
| notificationUtils.cancelNotificationMessage(null, SUMMARY_NOTIFICATION_ID) | ||
| } else { | ||
| } else if (hasNewEvent) { |
There was a problem hiding this comment.
can confirm this fixes the issue, great catch!
it might be worth renaming the variable to hasGroupChanged as what this logic is doing is only updating the summary group notification when one of the group children has been notified
I'm also happy to make the change in a separate PR as I'll be in the area attempting to address the double sound issue
for more details #1673 (comment)
There was a problem hiding this comment.
thanks a lot Adam, I think we can merge the PR now!
|
@bmarty from my testing there were no regressions and it fixes #1673, however there is the possibility that the summary count could go out of sync if events are removed as that won't trigger set although... the count is already incorrect~
|
3 participants
Resending the notification here can trigger other system components or
apps that listen to new notifications, such as connected smart watches
or automation tools.
Fixes #1673
Signed-off-by: Tobias Büttner [email protected]
Pull Request Checklist