fix: ReactionList Senders is now an array since we want to handle opengroup reactions separately

we no longer need the messagehash and server id for rendering reactions in the UI, ignore reactions using the outdated type
pull/2445/head
William Grant 3 years ago
parent bf7badd2e1
commit 4889cb5b32

@ -69,7 +69,7 @@ export const Reaction = (props: ReactionProps): ReactElement => {
handlePopupClick, handlePopupClick,
} = props; } = props;
const reactionsMap = (reactions && Object.fromEntries(reactions)) || {}; 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 count = reactionsMap[emoji].count;
const showCount = count !== undefined && (count > 1 || inGroup); const showCount = count !== undefined && (count > 1 || inGroup);
@ -138,7 +138,7 @@ export const Reaction = (props: ReactionProps): ReactElement => {
<ReactionPopup <ReactionPopup
messageId={messageId} messageId={messageId}
emoji={popupReaction} emoji={popupReaction}
senders={Object.keys(reactionsMap[popupReaction].senders)} senders={reactionsMap[popupReaction].senders}
tooltipPosition={tooltipPosition} tooltipPosition={tooltipPosition}
onClick={() => { onClick={() => {
if (handlePopupReaction) { if (handlePopupReaction) {

@ -165,7 +165,7 @@ type Props = {
}; };
const handleSenders = (senders: Array<string>, me: string) => { const handleSenders = (senders: Array<string>, me: string) => {
let updatedSenders = senders; let updatedSenders = [...senders];
const blindedMe = updatedSenders.filter(isUsAnySogsFromCache); const blindedMe = updatedSenders.filter(isUsAnySogsFromCache);
let meIndex = -1; let meIndex = -1;
@ -217,7 +217,7 @@ export const ReactListModal = (props: Props): ReactElement => {
let _senders = let _senders =
reactionsMap && reactionsMap[currentReact] && reactionsMap[currentReact].senders reactionsMap && reactionsMap[currentReact] && reactionsMap[currentReact].senders
? Object.keys(reactionsMap[currentReact].senders) ? reactionsMap[currentReact].senders
: null; : null;
if (_senders && !isEqual(senders, _senders)) { if (_senders && !isEqual(senders, _senders)) {

@ -89,12 +89,10 @@ import { addMessagePadding } from '../session/crypto/BufferPadding';
import { getSodiumRenderer } from '../session/crypto'; import { getSodiumRenderer } from '../session/crypto';
import { import {
findCachedOurBlindedPubkeyOrLookItUp, findCachedOurBlindedPubkeyOrLookItUp,
getUsBlindedInThatServer,
isUsAnySogsFromCache, isUsAnySogsFromCache,
} from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys';
import { sogsV3FetchPreviewAndSaveIt } from '../session/apis/open_group_api/sogsv3/sogsV3FetchFile'; import { sogsV3FetchPreviewAndSaveIt } from '../session/apis/open_group_api/sogsv3/sogsV3FetchFile';
import { Reaction } from '../types/Reaction'; import { Reaction } from '../types/Reaction';
import { handleMessageReaction } from '../util/reactions';
export class ConversationModel extends Backbone.Model<ConversationAttributes> { export class ConversationModel extends Backbone.Model<ConversationAttributes> {
public updateLastMessage: () => any; public updateLastMessage: () => any;
@ -738,8 +736,6 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
throw new Error('Only opengroupv2 are supported now'); throw new Error('Only opengroupv2 are supported now');
} }
let sender = UserUtils.getOurPubKeyStrFromCache();
// an OpenGroupV2 message is just a visible message // an OpenGroupV2 message is just a visible message
const chatMessageParams: VisibleMessageParams = { const chatMessageParams: VisibleMessageParams = {
body: '', body: '',
@ -785,20 +781,9 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
const openGroup = OpenGroupData.getV2OpenGroupRoom(this.id); const openGroup = OpenGroupData.getV2OpenGroupRoom(this.id);
const blinded = Boolean(roomHasBlindEnabled(openGroup)); 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 // send with blinding if we need to
await getMessageQueue().sendToOpenGroupV2(chatMessageOpenGroupV2, roomInfos, blinded, []); await getMessageQueue().sendToOpenGroupV2(chatMessageOpenGroupV2, roomInfos, blinded, []);
return; return;
} else {
await handleMessageReaction(reaction, sender, false);
} }
const destinationPubkey = new PubKey(destination); const destinationPubkey = new PubKey(destination);

@ -318,17 +318,12 @@ async function handleSwarmMessage(
void convoToAddMessageTo.queueJob(async () => { void convoToAddMessageTo.queueJob(async () => {
// this call has to be made inside the queueJob! // this call has to be made inside the queueJob!
if (!msgModel.get('isPublic') && rawDataMessage.reaction && rawDataMessage.syncTarget) { // We handle reaction DataMessages separately
await handleMessageReaction( if (!msgModel.get('isPublic') && rawDataMessage.reaction) {
rawDataMessage.reaction, await handleMessageReaction(rawDataMessage.reaction, msgModel.get('source'));
msgModel.get('source'),
false,
messageHash
);
confirm(); confirm();
return; return;
} }
const isDuplicate = await isSwarmMessageDuplicate({ const isDuplicate = await isSwarmMessageDuplicate({
source: msgModel.get('source'), source: msgModel.get('source'),
sentAt, sentAt,

@ -16,7 +16,6 @@ import { GoogleChrome } from '../util';
import { appendFetchAvatarAndProfileJob } from './userProfileImageUpdates'; import { appendFetchAvatarAndProfileJob } from './userProfileImageUpdates';
import { ConversationTypeEnum } from '../models/conversationAttributes'; import { ConversationTypeEnum } from '../models/conversationAttributes';
import { getUsBlindedInThatServer } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys'; import { getUsBlindedInThatServer } from '../session/apis/open_group_api/sogsv3/knownBlindedkeys';
import { handleMessageReaction } from '../util/reactions';
import { Action, Reaction } from '../types/Reaction'; import { Action, Reaction } from '../types/Reaction';
function contentTypeSupported(type: string): boolean { function contentTypeSupported(type: string): boolean {
@ -341,8 +340,6 @@ export async function handleMessageJob(
); );
if (!messageModel.get('isPublic') && regularDataMessage.reaction) { if (!messageModel.get('isPublic') && regularDataMessage.reaction) {
await handleMessageReaction(regularDataMessage.reaction, source, false, messageHash);
if ( if (
regularDataMessage.reaction.action === Action.REACT && regularDataMessage.reaction.action === Action.REACT &&
conversation.isPrivate() && conversation.isPrivate() &&

@ -918,6 +918,16 @@ export const getMessageReactsProps = createSelector(getMessagePropsByMessageId,
]); ]);
if (msgProps.reacts) { 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) => { 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; return a[1].index < b[1].index ? -1 : a[1].index > b[1].index ? 1 : 0;
}); });

@ -54,9 +54,7 @@ describe('ReactionMessage', () => {
// Handling reaction // Handling reaction
const updatedMessage = await handleMessageReaction( const updatedMessage = await handleMessageReaction(
reaction as SignalService.DataMessage.IReaction, reaction as SignalService.DataMessage.IReaction,
ourNumber, ourNumber
false,
originalMessage.get('id')
); );
expect(updatedMessage?.get('reacts'), 'original message should have reacts').to.not.be 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; expect(updatedMessage?.get('reacts')!['😄'], 'reacts should have 😄 key').to.not.be.undefined;
// tslint:disable: no-non-null-assertion // tslint:disable: no-non-null-assertion
expect( expect(
Object.keys(updatedMessage!.get('reacts')!['😄'].senders)[0], updatedMessage!.get('reacts')!['😄'].senders[0],
'sender pubkey should match' 'sender pubkey should match'
).to.be.equal(ourNumber); ).to.be.equal(ourNumber);
expect(updatedMessage!.get('reacts')!['😄'].count, 'count should be 1').to.be.equal(1); expect(updatedMessage!.get('reacts')!['😄'].count, 'count should be 1').to.be.equal(1);
@ -87,9 +85,7 @@ describe('ReactionMessage', () => {
// Handling reaction // Handling reaction
const updatedMessage = await handleMessageReaction( const updatedMessage = await handleMessageReaction(
reaction as SignalService.DataMessage.IReaction, reaction as SignalService.DataMessage.IReaction,
ourNumber, ourNumber
false,
originalMessage.get('id')
); );
expect(updatedMessage?.get('reacts'), 'original message reacts should be undefined').to.be expect(updatedMessage?.get('reacts'), 'original message reacts should be undefined').to.be

@ -123,14 +123,14 @@ export type ReactionList = Record<
{ {
count: number; count: number;
index: number; // relies on reactsIndex in the message model index: number; // relies on reactsIndex in the message model
senders: Record<string, string>; // <sender pubkey, messageHash or serverId> senders: Array<string>;
you?: boolean; // whether you are in the senders because sometimes we dont have the full list of senders yet. 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 // used when rendering reactions to guarantee sorted order using the index
export type SortedReactionList = Array< export type SortedReactionList = Array<
[string, { count: number; index: number; senders: Record<string, string>; you?: boolean }] [string, { count: number; index: number; senders: Array<string>; you?: boolean }]
>; >;
export interface OpenGroupReaction { export interface OpenGroupReaction {

@ -19,16 +19,12 @@ const latestReactionTimestamps: Array<number> = [];
* Retrieves the original message of a reaction * Retrieves the original message of a reaction
*/ */
const getMessageByReaction = async ( const getMessageByReaction = async (
reaction: SignalService.DataMessage.IReaction, reaction: SignalService.DataMessage.IReaction
isOpenGroup: boolean
): Promise<MessageModel | null> => { ): Promise<MessageModel | null> => {
let originalMessage = null; let originalMessage = null;
const originalMessageId = Number(reaction.id); const originalMessageId = Number(reaction.id);
const originalMessageAuthor = reaction.author; const originalMessageAuthor = reaction.author;
if (isOpenGroup) {
originalMessage = await Data.getMessageByServerId(originalMessageId);
} else {
const collection = await Data.getMessagesBySentAt(originalMessageId); const collection = await Data.getMessagesBySentAt(originalMessageId);
originalMessage = collection.find((item: MessageModel) => { originalMessage = collection.find((item: MessageModel) => {
const messageTimestamp = item.get('sent_at'); const messageTimestamp = item.get('sent_at');
@ -40,7 +36,6 @@ const getMessageByReaction = async (
author === originalMessageAuthor author === originalMessageAuthor
); );
}); });
}
if (!originalMessage) { if (!originalMessage) {
window?.log?.warn(`Cannot find the original reacted message ${originalMessageId}.`); window?.log?.warn(`Cannot find the original reacted message ${originalMessageId}.`);
@ -67,6 +62,7 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => {
return; return;
} }
// TODO need to add rate limiting to SOGS function
const timestamp = Date.now(); const timestamp = Date.now();
latestReactionTimestamps.push(timestamp); latestReactionTimestamps.push(timestamp);
@ -80,21 +76,19 @@ export const sendMessageReaction = async (messageId: string, emoji: string) => {
} }
} }
const isOpenGroup = Boolean(found?.get('isPublic')); if (found?.get('isPublic')) {
const id = (isOpenGroup && found.get('serverId')) || Number(found.get('sent_at')); window.log.warn("sendMessageReaction() shouldn't be used in opengroups");
const me = return;
(isOpenGroup && getUsBlindedInThatServer(conversationModel)) || }
UserUtils.getOurPubKeyStrFromCache();
const id = Number(found.get('sent_at'));
const me = UserUtils.getOurPubKeyStrFromCache();
const author = found.get('source'); const author = found.get('source');
let action: Action = Action.REACT; let action: Action = Action.REACT;
const reacts = found.get('reacts'); const reacts = found.get('reacts');
if ( if (reacts && Object.keys(reacts).includes(emoji) && reacts[emoji].senders.includes(me)) {
reacts && window.log.info('Found matching reaction removing it');
Object.keys(reacts).includes(emoji) &&
Object.keys(reacts[emoji].senders).includes(me)
) {
window.log.info('found matching reaction removing it');
action = Action.REMOVE; action = Action.REMOVE;
} else { } else {
const reactions = getRecentReactions(); 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 * Handle reactions on the client by updating the state of the source message
* Do not use for Open Groups
*/ */
export const handleMessageReaction = async ( export const handleMessageReaction = async (
reaction: SignalService.DataMessage.IReaction, reaction: SignalService.DataMessage.IReaction,
sender: string, sender: string
isOpenGroup: boolean,
messageId?: string
) => { ) => {
if (!reaction.emoji) { 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; return;
} }
const originalMessage = await getMessageByReaction(reaction, isOpenGroup); const originalMessage = await getMessageByReaction(reaction);
if (!originalMessage) { if (!originalMessage) {
return; return;
} }
if (originalMessage.get('isPublic')) {
window.log.warn("handleMessageReaction() shouldn't be used in opengroups");
return;
}
const reacts: ReactionList = originalMessage.get('reacts') ?? {}; 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 details = reacts[reaction.emoji] ?? {};
const senders = Object.keys(details.senders); const senders = details.senders;
let count = details.count || 0;
window.log.info( window.log.info(
`${sender} ${reaction.action === Action.REACT ? 'added' : 'removed'} a ${ `${sender} ${reaction.action === Action.REACT ? 'added' : 'removed'} a ${
@ -156,33 +155,30 @@ export const handleMessageReaction = async (
); );
switch (reaction.action) { switch (reaction.action) {
case SignalService.DataMessage.Reaction.Action.REACT: case Action.REACT:
if (senders.includes(sender) && details.senders[sender] !== '') { if (senders.includes(sender)) {
window?.log?.info( window.log.warn('Received duplicate reaction message. Ignoring it', reaction, sender);
'Received duplicate message reaction. Dropping it. id:',
details.senders[sender]
);
return; return;
} }
details.senders[sender] = messageId ?? ''; details.senders.push(sender);
count += 1;
break; break;
case SignalService.DataMessage.Reaction.Action.REMOVE: case Action.REMOVE:
default: default:
if (senders.length > 0) { if (senders && senders.length > 0) {
if (senders.indexOf(sender) >= 0) { const sendersIndex = senders.indexOf(sender);
// tslint:disable-next-line: no-dynamic-delete if (sendersIndex >= 0) {
delete details.senders[sender]; details.senders.splice(sendersIndex, 1);
count -= 1;
} }
} }
} }
const count = Object.keys(details.senders).length;
if (count > 0) { if (count > 0) {
reacts[reaction.emoji].count = count; reacts[reaction.emoji].count = count;
reacts[reaction.emoji].senders = details.senders; reacts[reaction.emoji].senders = details.senders;
// sorting for open groups convos is handled by SOGS if (details && details.index === undefined) {
if (!isOpenGroup && details && details.index === undefined) {
reacts[reaction.emoji].index = originalMessage.get('reactsIndex') ?? 0; reacts[reaction.emoji].index = originalMessage.get('reactsIndex') ?? 0;
originalMessage.set('reactsIndex', (originalMessage.get('reactsIndex') ?? 0) + 1); 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 ( export const handleOpenGroupMessageReactions = async (
reactions: OpenGroupReactionList, reactions: OpenGroupReactionList,
@ -212,6 +208,11 @@ export const handleOpenGroupMessageReactions = async (
return; return;
} }
if (!originalMessage.get('isPublic')) {
window.log.warn('handleOpenGroupMessageReactions() should only be used in opengroups');
return;
}
if (isEmpty(reactions)) { if (isEmpty(reactions)) {
if (originalMessage.get('reacts')) { if (originalMessage.get('reacts')) {
originalMessage.set({ originalMessage.set({
@ -239,9 +240,9 @@ export const handleOpenGroupMessageReactions = async (
} }
} }
const senders: Record<string, string> = {}; const senders: Array<string> = [];
reactions[key].reactors.forEach(reactor => { reactions[key].reactors.forEach(reactor => {
senders[reactor] = String(serverId); senders.push(reactor);
}); });
reacts[emoji] = { reacts[emoji] = {

Loading…
Cancel
Save