diff --git a/ts/components/conversation/message/reactions/Reaction.tsx b/ts/components/conversation/message/reactions/Reaction.tsx index d0d6af1a7..675d24afd 100644 --- a/ts/components/conversation/message/reactions/Reaction.tsx +++ b/ts/components/conversation/message/reactions/Reaction.tsx @@ -69,7 +69,7 @@ export const Reaction = (props: ReactionProps): ReactElement => { handlePopupClick, } = props; const reactionsMap = (reactions && Object.fromEntries(reactions)) || {}; - const senders = reactionsMap[emoji].senders ? Object.keys(reactionsMap[emoji].senders) : []; + const senders = reactionsMap[emoji].senders || []; const count = reactionsMap[emoji].count; const showCount = count !== undefined && (count > 1 || inGroup); @@ -138,7 +138,7 @@ export const Reaction = (props: ReactionProps): ReactElement => { { if (handlePopupReaction) { diff --git a/ts/components/dialog/ReactListModal.tsx b/ts/components/dialog/ReactListModal.tsx index debc9fc70..fc1787297 100644 --- a/ts/components/dialog/ReactListModal.tsx +++ b/ts/components/dialog/ReactListModal.tsx @@ -165,7 +165,7 @@ type Props = { }; const handleSenders = (senders: Array, me: string) => { - let updatedSenders = senders; + let updatedSenders = [...senders]; const blindedMe = updatedSenders.filter(isUsAnySogsFromCache); let meIndex = -1; @@ -217,7 +217,7 @@ export const ReactListModal = (props: Props): ReactElement => { let _senders = reactionsMap && reactionsMap[currentReact] && reactionsMap[currentReact].senders - ? Object.keys(reactionsMap[currentReact].senders) + ? reactionsMap[currentReact].senders : null; if (_senders && !isEqual(senders, _senders)) { diff --git a/ts/models/conversation.ts b/ts/models/conversation.ts index bc14d2ccf..3c1aeaa7e 100644 --- a/ts/models/conversation.ts +++ b/ts/models/conversation.ts @@ -89,12 +89,10 @@ import { addMessagePadding } from '../session/crypto/BufferPadding'; import { getSodiumRenderer } from '../session/crypto'; import { findCachedOurBlindedPubkeyOrLookItUp, - getUsBlindedInThatServer, isUsAnySogsFromCache, } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; import { sogsV3FetchPreviewAndSaveIt } from '../session/apis/open_group_api/sogsv3/sogsV3FetchFile'; import { Reaction } from '../types/Reaction'; -import { handleMessageReaction } from '../util/reactions'; export class ConversationModel extends Backbone.Model { public updateLastMessage: () => any; @@ -738,8 +736,6 @@ export class ConversationModel extends Backbone.Model { throw new Error('Only opengroupv2 are supported now'); } - let sender = UserUtils.getOurPubKeyStrFromCache(); - // an OpenGroupV2 message is just a visible message const chatMessageParams: VisibleMessageParams = { body: '', @@ -785,20 +781,9 @@ export class ConversationModel extends Backbone.Model { const openGroup = OpenGroupData.getV2OpenGroupRoom(this.id); const blinded = Boolean(roomHasBlindEnabled(openGroup)); - if (blinded) { - const blindedSender = getUsBlindedInThatServer(this); - if (blindedSender) { - sender = blindedSender; - } - } - - await handleMessageReaction(reaction, sender, true); - // send with blinding if we need to await getMessageQueue().sendToOpenGroupV2(chatMessageOpenGroupV2, roomInfos, blinded, []); return; - } else { - await handleMessageReaction(reaction, sender, false); } const destinationPubkey = new PubKey(destination); diff --git a/ts/receiver/dataMessage.ts b/ts/receiver/dataMessage.ts index e2c8d09a4..362639f40 100644 --- a/ts/receiver/dataMessage.ts +++ b/ts/receiver/dataMessage.ts @@ -318,17 +318,12 @@ async function handleSwarmMessage( void convoToAddMessageTo.queueJob(async () => { // this call has to be made inside the queueJob! - if (!msgModel.get('isPublic') && rawDataMessage.reaction && rawDataMessage.syncTarget) { - await handleMessageReaction( - rawDataMessage.reaction, - msgModel.get('source'), - false, - messageHash - ); + // We handle reaction DataMessages separately + if (!msgModel.get('isPublic') && rawDataMessage.reaction) { + await handleMessageReaction(rawDataMessage.reaction, msgModel.get('source')); confirm(); return; } - const isDuplicate = await isSwarmMessageDuplicate({ source: msgModel.get('source'), sentAt, diff --git a/ts/receiver/queuedJob.ts b/ts/receiver/queuedJob.ts index 443ce6789..03b4a67dc 100644 --- a/ts/receiver/queuedJob.ts +++ b/ts/receiver/queuedJob.ts @@ -16,7 +16,6 @@ import { GoogleChrome } from '../util'; import { appendFetchAvatarAndProfileJob } from './userProfileImageUpdates'; import { ConversationTypeEnum } from '../models/conversationAttributes'; import { getUsBlindedInThatServer } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; -import { handleMessageReaction } from '../util/reactions'; import { Action, Reaction } from '../types/Reaction'; function contentTypeSupported(type: string): boolean { @@ -341,8 +340,6 @@ export async function handleMessageJob( ); if (!messageModel.get('isPublic') && regularDataMessage.reaction) { - await handleMessageReaction(regularDataMessage.reaction, source, false, messageHash); - if ( regularDataMessage.reaction.action === Action.REACT && conversation.isPrivate() && diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index e3f0d8081..91e87d3dc 100644 --- a/ts/state/selectors/conversations.ts +++ b/ts/state/selectors/conversations.ts @@ -918,6 +918,16 @@ export const getMessageReactsProps = createSelector(getMessagePropsByMessageId, ]); if (msgProps.reacts) { + // TODO we don't want to render reactions that have 'senders' as an object this is a deprecated type used during development 25/08/2022 + const oldReactions = Object.values(msgProps.reacts).filter( + reaction => !Array.isArray(reaction.senders) + ); + + if (oldReactions.length > 0) { + msgProps.reacts = undefined; + return msgProps; + } + const sortedReacts = Object.entries(msgProps.reacts).sort((a, b) => { return a[1].index < b[1].index ? -1 : a[1].index > b[1].index ? 1 : 0; }); diff --git a/ts/test/session/unit/reactions/ReactionMessage_test.ts b/ts/test/session/unit/reactions/ReactionMessage_test.ts index cf5a1150d..dfd2fd1aa 100644 --- a/ts/test/session/unit/reactions/ReactionMessage_test.ts +++ b/ts/test/session/unit/reactions/ReactionMessage_test.ts @@ -54,9 +54,7 @@ describe('ReactionMessage', () => { // Handling reaction const updatedMessage = await handleMessageReaction( reaction as SignalService.DataMessage.IReaction, - ourNumber, - false, - originalMessage.get('id') + ourNumber ); expect(updatedMessage?.get('reacts'), 'original message should have reacts').to.not.be @@ -65,7 +63,7 @@ describe('ReactionMessage', () => { expect(updatedMessage?.get('reacts')!['😄'], 'reacts should have 😄 key').to.not.be.undefined; // tslint:disable: no-non-null-assertion expect( - Object.keys(updatedMessage!.get('reacts')!['😄'].senders)[0], + updatedMessage!.get('reacts')!['😄'].senders[0], 'sender pubkey should match' ).to.be.equal(ourNumber); expect(updatedMessage!.get('reacts')!['😄'].count, 'count should be 1').to.be.equal(1); @@ -87,9 +85,7 @@ describe('ReactionMessage', () => { // Handling reaction const updatedMessage = await handleMessageReaction( reaction as SignalService.DataMessage.IReaction, - ourNumber, - false, - originalMessage.get('id') + ourNumber ); expect(updatedMessage?.get('reacts'), 'original message reacts should be undefined').to.be diff --git a/ts/types/Reaction.ts b/ts/types/Reaction.ts index 22dbad543..617999541 100644 --- a/ts/types/Reaction.ts +++ b/ts/types/Reaction.ts @@ -123,14 +123,14 @@ export type ReactionList = Record< { count: number; index: number; // relies on reactsIndex in the message model - senders: Record; // + senders: Array; you?: boolean; // whether you are in the senders because sometimes we dont have the full list of senders yet. } >; // used when rendering reactions to guarantee sorted order using the index export type SortedReactionList = Array< - [string, { count: number; index: number; senders: Record; you?: boolean }] + [string, { count: number; index: number; senders: Array; you?: boolean }] >; export interface OpenGroupReaction { diff --git a/ts/util/reactions.ts b/ts/util/reactions.ts index f558af080..c3c0d59cd 100644 --- a/ts/util/reactions.ts +++ b/ts/util/reactions.ts @@ -19,28 +19,23 @@ const latestReactionTimestamps: Array = []; * Retrieves the original message of a reaction */ const getMessageByReaction = async ( - reaction: SignalService.DataMessage.IReaction, - isOpenGroup: boolean + reaction: SignalService.DataMessage.IReaction ): Promise => { let originalMessage = null; const originalMessageId = Number(reaction.id); const originalMessageAuthor = reaction.author; - if (isOpenGroup) { - originalMessage = await Data.getMessageByServerId(originalMessageId); - } else { - const collection = await Data.getMessagesBySentAt(originalMessageId); - originalMessage = collection.find((item: MessageModel) => { - const messageTimestamp = item.get('sent_at'); - const author = item.get('source'); - return Boolean( - messageTimestamp && - messageTimestamp === originalMessageId && - author && - author === originalMessageAuthor - ); - }); - } + const collection = await Data.getMessagesBySentAt(originalMessageId); + originalMessage = collection.find((item: MessageModel) => { + const messageTimestamp = item.get('sent_at'); + const author = item.get('source'); + return Boolean( + messageTimestamp && + messageTimestamp === originalMessageId && + author && + author === originalMessageAuthor + ); + }); if (!originalMessage) { window?.log?.warn(`Cannot find the original reacted message ${originalMessageId}.`); @@ -67,6 +62,7 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => { return; } + // TODO need to add rate limiting to SOGS function const timestamp = Date.now(); latestReactionTimestamps.push(timestamp); @@ -80,21 +76,19 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => { } } - const isOpenGroup = Boolean(found?.get('isPublic')); - const id = (isOpenGroup && found.get('serverId')) || Number(found.get('sent_at')); - const me = - (isOpenGroup && getUsBlindedInThatServer(conversationModel)) || - UserUtils.getOurPubKeyStrFromCache(); + if (found?.get('isPublic')) { + window.log.warn("sendMessageReaction() shouldn't be used in opengroups"); + return; + } + + const id = Number(found.get('sent_at')); + const me = UserUtils.getOurPubKeyStrFromCache(); const author = found.get('source'); let action: Action = Action.REACT; const reacts = found.get('reacts'); - if ( - reacts && - Object.keys(reacts).includes(emoji) && - Object.keys(reacts[emoji].senders).includes(me) - ) { - window.log.info('found matching reaction removing it'); + if (reacts && Object.keys(reacts).includes(emoji) && reacts[emoji].senders.includes(me)) { + window.log.info('Found matching reaction removing it'); action = Action.REMOVE; } else { const reactions = getRecentReactions(); @@ -127,27 +121,32 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => { /** * Handle reactions on the client by updating the state of the source message + * Do not use for Open Groups */ export const handleMessageReaction = async ( reaction: SignalService.DataMessage.IReaction, - sender: string, - isOpenGroup: boolean, - messageId?: string + sender: string ) => { if (!reaction.emoji) { - window?.log?.warn(`There is no emoji for the reaction ${messageId}.`); + window?.log?.warn(`There is no emoji for the reaction ${reaction}.`); return; } - const originalMessage = await getMessageByReaction(reaction, isOpenGroup); + const originalMessage = await getMessageByReaction(reaction); if (!originalMessage) { return; } + if (originalMessage.get('isPublic')) { + window.log.warn("handleMessageReaction() shouldn't be used in opengroups"); + return; + } + const reacts: ReactionList = originalMessage.get('reacts') ?? {}; - reacts[reaction.emoji] = reacts[reaction.emoji] || { count: null, senders: {} }; + reacts[reaction.emoji] = reacts[reaction.emoji] || { count: null, senders: [] }; const details = reacts[reaction.emoji] ?? {}; - const senders = Object.keys(details.senders); + const senders = details.senders; + let count = details.count || 0; window.log.info( `${sender} ${reaction.action === Action.REACT ? 'added' : 'removed'} a ${ @@ -156,33 +155,30 @@ export const handleMessageReaction = async ( ); switch (reaction.action) { - case SignalService.DataMessage.Reaction.Action.REACT: - if (senders.includes(sender) && details.senders[sender] !== '') { - window?.log?.info( - 'Received duplicate message reaction. Dropping it. id:', - details.senders[sender] - ); + case Action.REACT: + if (senders.includes(sender)) { + window.log.warn('Received duplicate reaction message. Ignoring it', reaction, sender); return; } - details.senders[sender] = messageId ?? ''; + details.senders.push(sender); + count += 1; break; - case SignalService.DataMessage.Reaction.Action.REMOVE: + case Action.REMOVE: default: - if (senders.length > 0) { - if (senders.indexOf(sender) >= 0) { - // tslint:disable-next-line: no-dynamic-delete - delete details.senders[sender]; + if (senders && senders.length > 0) { + const sendersIndex = senders.indexOf(sender); + if (sendersIndex >= 0) { + details.senders.splice(sendersIndex, 1); + count -= 1; } } } - const count = Object.keys(details.senders).length; if (count > 0) { reacts[reaction.emoji].count = count; reacts[reaction.emoji].senders = details.senders; - // sorting for open groups convos is handled by SOGS - if (!isOpenGroup && details && details.index === undefined) { + if (details && details.index === undefined) { reacts[reaction.emoji].index = originalMessage.get('reactsIndex') ?? 0; originalMessage.set('reactsIndex', (originalMessage.get('reactsIndex') ?? 0) + 1); } @@ -200,7 +196,7 @@ export const handleMessageReaction = async ( }; /** - * Handle all updates to messages reactions from the SOGS API + * Handles all message reaction updates for opengroups */ export const handleOpenGroupMessageReactions = async ( reactions: OpenGroupReactionList, @@ -212,6 +208,11 @@ export const handleOpenGroupMessageReactions = async ( return; } + if (!originalMessage.get('isPublic')) { + window.log.warn('handleOpenGroupMessageReactions() should only be used in opengroups'); + return; + } + if (isEmpty(reactions)) { if (originalMessage.get('reacts')) { originalMessage.set({ @@ -239,9 +240,9 @@ export const handleOpenGroupMessageReactions = async ( } } - const senders: Record = {}; + const senders: Array = []; reactions[key].reactors.forEach(reactor => { - senders[reactor] = String(serverId); + senders.push(reactor); }); reacts[emoji] = {