From 18fb2b806e91c770c320742fc94ad0002f9e8718 Mon Sep 17 00:00:00 2001 From: Fedor Indutny <79877362+indutny-signal@users.noreply.github.com> Date: Wed, 3 Mar 2021 11:03:11 -0800 Subject: [PATCH] Remove notification on reaction remove/change --- js/notifications.js | 43 ++++++++++++++++++++++++++++++++++--------- ts/models/messages.ts | 26 ++++++++++++++++++++++++-- 2 files changed, 58 insertions(+), 11 deletions(-) diff --git a/js/notifications.js b/js/notifications.js index 0b2796925..13685bb59 100644 --- a/js/notifications.js +++ b/js/notifications.js @@ -47,6 +47,7 @@ // isExpiringMessage: boolean; // reaction: { // emoji: string; + // fromId: string; // }; // } notificationData: null, @@ -56,16 +57,40 @@ this.update(); }, - removeBy({ conversationId, messageId }) { - const shouldClear = - Boolean(this.notificationData) && - ((conversationId && - this.notificationData.conversationId === conversationId) || - (messageId && this.notificationData.messageId === messageId)); - if (shouldClear) { - this.clear(); - this.update(); + // Remove the last notification if both conditions hold: + // + // 1. Either `conversationId` or `messageId` matches (if present) + // 2. `reactionFromId` matches (if present) + removeBy({ conversationId, messageId, reactionFromId }) { + if (!this.notificationData) { + return; } + + let shouldClear = false; + if ( + conversationId && + this.notificationData.conversationId === conversationId + ) { + shouldClear = true; + } + if (messageId && this.notificationData.messageId === messageId) { + shouldClear = true; + } + + if (!shouldClear) { + return; + } + + if ( + reactionFromId && + this.notificationData.reaction && + this.notificationData.reaction.fromId !== reactionFromId + ) { + return; + } + + this.clear(); + this.update(); }, fastUpdate() { diff --git a/ts/models/messages.ts b/ts/models/messages.ts index 8f340a1f4..a7aa2e13b 100644 --- a/ts/models/messages.ts +++ b/ts/models/messages.ts @@ -3642,6 +3642,12 @@ export class MessageModel extends window.Backbone.Model { const messageId = this.idForLogging(); const count = reactions.length; + const conversation = window.ConversationController.get( + this.get('conversationId') + ); + + let staleReactionFromId: string | undefined; + if (reaction.get('remove')) { window.log.info('Removing reaction for message', messageId); const newReactions = reactions.filter( @@ -3650,6 +3656,8 @@ export class MessageModel extends window.Backbone.Model { re.fromId !== reaction.get('fromId') ); this.set({ reactions: newReactions }); + + staleReactionFromId = reaction.get('fromId'); } else { window.log.info('Adding reaction for message', messageId); const newReactions = reactions.filter( @@ -3658,9 +3666,12 @@ export class MessageModel extends window.Backbone.Model { newReactions.push(reaction.toJSON()); this.set({ reactions: newReactions }); - const conversation = window.ConversationController.get( - this.get('conversationId') + const oldReaction = reactions.find( + re => re.fromId === reaction.get('fromId') ); + if (oldReaction) { + staleReactionFromId = oldReaction.fromId; + } // Only notify for reactions to our own messages if (conversation && this.isOutgoing() && !reaction.get('fromSync')) { @@ -3668,6 +3679,10 @@ export class MessageModel extends window.Backbone.Model { } } + if (staleReactionFromId) { + this.clearNotifications(reaction.get('fromId')); + } + const newCount = this.get('reactions').length; window.log.info( `Done processing reaction for message ${messageId}. Went from ${count} to ${newCount} reactions.` @@ -3704,6 +3719,13 @@ export class MessageModel extends window.Backbone.Model { // eslint-disable-next-line @typescript-eslint/no-non-null-assertion this.getConversation()!.updateLastMessage(); } + + clearNotifications(reactionFromId?: string): void { + window.Whisper.Notifications.removeBy({ + messageId: this.id, + reactionFromId, + }); + } } window.Whisper.Message = MessageModel as typeof window.WhatIsThis;