fix(browser): Don't create web notifications for old notifications #1943
Conversation
Sometimes a notification got "newly mounted" while being old. This can happen when a user has many notifications (100-1). The UI first only loads (100-76), if any notification is then resolved (e.g. by deleting or reading a chat), further old notifications (75+74) would be added to the UI and triggered a web notification (including call sound) in the past. This threshold ID is therefore updated to only higher values, before each pulling of notifications to ensure that we only ever web-notify on new notifications and not newly loaded old notifications. Signed-off-by: Joas Schilling <coding@schilljs.com>
|
/compile / |
|
/backport 6a54367 to stable29 |
|
/backport 6a54367 to stable28 |
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
| if (this.shouldNotify && this.$parent.$parent.$parent.showBrowserNotifications) { | ||
| if (this.shouldNotify | ||
| && this.$parent.$parent.$parent.showBrowserNotifications | ||
| && this.$parent.$parent.$parent.webNotificationsThresholdId < this.notificationId) { |
There was a problem hiding this comment.
Changes look good in general. I'm just not sure about this place:
- you changing
webNotificationsThresholdIdbefore and right after the _fetch, so Vue components for new notifications might not be mounted yet. Wouldn't we skip this block of code for the very last notification?
| && this.$parent.$parent.$parent.webNotificationsThresholdId < this.notificationId) { | |
| && this.$parent.$parent.$parent.webNotificationsThresholdId <= this.notificationId) { |
There was a problem hiding this comment.
you changing webNotificationsThresholdId before and right after the _fetch, so Vue components for new notifications might not be mounted yet.
Before was added "later" and should not be an issue.
After is no problem as the first run (!this.backgroundFetching which is wrapping it) is not generating notifications anyway. I was just afraid to remove this, in case someone would manually delete/resolve all of the initial load. Then the first background fetch would have an empty notification list and therefore there would still be sound+notifications for old things that are loaded with the first background fetch
|
/backport 6a54367 to stable27 |
Sometimes a notification got "newly mounted" while being old. This can happen when a user has many notifications (100-1). The UI first only loads (100-76), if any notification is then resolved (e.g. by deleting or reading a chat), further old notifications (75+74) would be added to the UI and triggered a web notification (including call sound) in the past.
This threshold ID is therefore updated to only higher values, before each pulling of notifications to ensure that we only ever web-notify on new notifications and not newly loaded old notifications.