From 18dcdb148c65937834de5dd957f4c21c61038dd7 Mon Sep 17 00:00:00 2001 From: Anatoly Rugalev Date: Mon, 1 Feb 2021 13:59:40 +0100 Subject: [PATCH] Proper sync of reaction data in messages --- src/channel.ts | 8 +- src/channel_state.ts | 187 +++++++++++++--------------------- test/integration/messages.js | 67 +----------- test/integration/reactions.js | 102 ++++++++++++++++++- test/integration/utils.js | 59 +++++++++++ test/unit/channel_state.js | 63 +++++++++++- 6 files changed, 298 insertions(+), 188 deletions(-) diff --git a/src/channel.ts b/src/channel.ts index a062beb489..85ca3ecb8e 100644 --- a/src/channel.ts +++ b/src/channel.ts @@ -1616,19 +1616,19 @@ export class Channel< } break; case 'reaction.new': - if (event.reaction) { - channelState.addReaction(event.reaction, event.message); + if (event.message && event.reaction) { + event.message = channelState.addReaction(event.reaction, event.message); } break; case 'reaction.deleted': if (event.reaction) { - channelState.removeReaction(event.reaction, event.message); + event.message = channelState.removeReaction(event.reaction, event.message); } break; case 'reaction.updated': if (event.reaction) { // assuming reaction.updated is only called if enforce_unique is true - channelState.addReaction(event.reaction, event.message, true); + event.message = channelState.addReaction(event.reaction, event.message, true); } break; case 'channel.hidden': diff --git a/src/channel_state.ts b/src/channel_state.ts index c575cee7ab..002d6f61a1 100644 --- a/src/channel_state.ts +++ b/src/channel_state.ts @@ -319,46 +319,17 @@ export class ChannelState< >, enforce_unique?: boolean, ) { - const { messages } = this; if (!message) return; - const { parent_id, show_in_channel } = message; - - if (parent_id && this.threads[parent_id]) { - const thread = this.threads[parent_id]; - - for (let i = 0; i < thread.length; i++) { - const msg = thread[i]; - const messageWithReaction = this._addReactionToMessage( - msg, - reaction, - enforce_unique, - ); - if (!messageWithReaction) { - continue; - } - - thread[i] = messageWithReaction; - this.threads[parent_id] = thread; - break; - } - } - - if ((!show_in_channel && !parent_id) || show_in_channel) { - for (let i = 0; i < messages.length; i++) { - const msg = messages[i]; - const messageWithReaction = this._addReactionToMessage( - msg, - reaction, - enforce_unique, - ); - if (!messageWithReaction) { - continue; - } - - this.messages[i] = messageWithReaction; - break; - } - } + let messageWithReaction = message; + this._updateMessage(message, (msg) => { + const newMsg = this._addReactionToMessage(msg, reaction, enforce_unique); + messageWithReaction = { + ...messageWithReaction, + own_reactions: newMsg.own_reactions, + }; + return newMsg; + }); + return messageWithReaction; } _addReactionToMessage( @@ -376,33 +347,22 @@ export class ChannelState< reaction: ReactionResponse, enforce_unique?: boolean, ) { - const idMatch = !!message.id && message.id === reaction.message_id; - - if (!idMatch) { - return false; + const newMessage = message; + if (enforce_unique) { + newMessage.own_reactions = []; + } else { + this._removeOwnReactionFromMessage(message, reaction); } - const newMessage = this._removeReactionFromMessage(message, reaction, enforce_unique); - if (this._channel.getClient().userID === reaction.user?.id) { - const ownReactions = newMessage.own_reactions; - if (ownReactions) { - newMessage.own_reactions = [...ownReactions, reaction]; - } + newMessage.own_reactions = newMessage.own_reactions || []; + if (this._channel.getClient().userID === reaction.user_id) { + newMessage.own_reactions.push(reaction); } - const latestReactions = newMessage.latest_reactions; - if (latestReactions) { - newMessage.latest_reactions = [...latestReactions, reaction]; - } - - const oldReactionCounts = newMessage.reaction_counts || {}; - const oldReactionType = oldReactionCounts[reaction.type]; - oldReactionCounts[reaction.type] = oldReactionType ? oldReactionType + 1 : 1; - newMessage.reaction_counts = oldReactionCounts; return newMessage; } - _removeReactionFromMessage( + _removeOwnReactionFromMessage( message: ReturnType< ChannelState< AttachmentType, @@ -415,32 +375,12 @@ export class ChannelState< >['formatMessage'] >, reaction: ReactionResponse, - enforce_unique?: boolean, ) { - const filterReaction = (old: ReactionResponse[]) => - old.filter((item) => - enforce_unique - ? item.user?.id !== reaction.user?.id - : item.type !== reaction.type || item.user?.id !== reaction.user?.id, - ); if (message.own_reactions) { - message.own_reactions = filterReaction(message.own_reactions); - } - if (message.latest_reactions) { - message.latest_reactions = filterReaction(message.latest_reactions); - } - if (enforce_unique) { - const oldReaction = message.own_reactions?.find( - ({ type }) => type === reaction.type, + message.own_reactions = message.own_reactions.filter( + (item) => item.user_id !== reaction.user_id || item.type !== reaction.type, ); - if (oldReaction && message.reaction_counts) { - const oldReactionCount = message.reaction_counts[oldReaction.type]; - message.reaction_counts[oldReaction.type] = oldReactionCount - ? oldReactionCount - 1 - : 0; - } } - return message; } // eslint-disable-next-line sonarjs/cognitive-complexity @@ -455,50 +395,65 @@ export class ChannelState< UserType >, ) { - const { messages } = this; if (!message) return; + let messageWithReaction = message; + this._updateMessage(message, (msg) => { + this._removeOwnReactionFromMessage(msg, reaction); + messageWithReaction = { + ...messageWithReaction, + own_reactions: msg.own_reactions, + }; + return msg; + }); + return messageWithReaction; + } + + /** + * Updates all instances of given message in channel state + * @param message + * @param updateFunc + */ + _updateMessage( + message: { id?: string; parent_id?: string; show_in_channel?: boolean }, + updateFunc: ( + msg: ReturnType< + ChannelState< + AttachmentType, + ChannelType, + CommandType, + EventType, + MessageType, + ReactionType, + UserType + >['formatMessage'] + >, + ) => ReturnType< + ChannelState< + AttachmentType, + ChannelType, + CommandType, + EventType, + MessageType, + ReactionType, + UserType + >['formatMessage'] + >, + ) { const { parent_id, show_in_channel } = message; if (parent_id && this.threads[parent_id]) { const thread = this.threads[parent_id]; - for (let i = 0; i < thread.length; i++) { - const msg = thread[i]; - const idMatch = !!msg.id && msg.id === reaction.message_id; - - if (!idMatch) { - continue; - } - const messageWithReaction = this._removeReactionFromMessage(msg, reaction); - const oldReactionCounts = messageWithReaction.reaction_counts; - if (oldReactionCounts) { - const oldReactionType = oldReactionCounts[reaction.type]; - oldReactionCounts[reaction.type] = oldReactionType ? oldReactionType - 1 : 0; - messageWithReaction.reaction_counts = oldReactionCounts; - } - - thread[i] = messageWithReaction; + const msgIndex = thread.findIndex((msg) => msg.id === message.id); + if (msgIndex !== -1) { + thread[msgIndex] = updateFunc(thread[msgIndex]); this.threads[parent_id] = thread; - break; } } + if ((!show_in_channel && !parent_id) || show_in_channel) { - for (let i = 0; i < messages.length; i++) { - const msg = messages[i]; - const idMatch = !!msg.id && msg.id === reaction.message_id; - - if (!idMatch) { - continue; - } - const messageWithReaction = this._removeReactionFromMessage(msg, reaction); - const oldReactionCounts = messageWithReaction.reaction_counts; - if (oldReactionCounts) { - const oldReactionType = oldReactionCounts[reaction.type]; - oldReactionCounts[reaction.type] = oldReactionType ? oldReactionType - 1 : 0; - messageWithReaction.reaction_counts = oldReactionCounts; - } - - this.messages[i] = messageWithReaction; - break; + const msgIndex = this.messages.findIndex((msg) => msg.id === message.id); + if (msgIndex !== -1) { + this.messages[msgIndex] = updateFunc(this.messages[msgIndex]); } } } diff --git a/test/integration/messages.js b/test/integration/messages.js index 0f7d1fd71a..67c76a27f0 100644 --- a/test/integration/messages.js +++ b/test/integration/messages.js @@ -1,11 +1,4 @@ -import { v4 as uuidv4 } from 'uuid'; - -import { - expectHTTPErrorCode, - getServerTestClient, - getTestClientForUser, - sleep, -} from './utils'; +import { expectHTTPErrorCode, sleep, setupTestChannel } from './utils'; import chai from 'chai'; import chaiAsPromised from 'chai-as-promised'; @@ -25,64 +18,6 @@ Promise.config({ }, }); -// setupTestChannel sets up all possible client and channel objects for given channel type -// set createOnServerSide=true to execute channel creation from the server side -async function setupTestChannel(type, createOnServerSide, custom = {}) { - const ownerID = uuidv4(); - const memberID = uuidv4(); - const modID = uuidv4(); - const userID = uuidv4(); - const ownerClient = await getTestClientForUser(ownerID); - const memberClient = await getTestClientForUser(memberID); - const modClient = await getTestClientForUser(modID); - const userClient = await getTestClientForUser(userID); - const serverSideClient = getServerTestClient(); - const ownerChannel = ownerClient.channel(type, uuidv4(), { - ...custom, - members: [ownerID, modID, memberID], - }); - const memberChannel = memberClient.channel(ownerChannel.type, ownerChannel.id); - const modChannel = modClient.channel(ownerChannel.type, ownerChannel.id); - const serverSideChannel = serverSideClient.channel( - ownerChannel.type, - ownerChannel.id, - { created_by_id: ownerID }, - ); - const userChannel = userClient.channel(ownerChannel.type, ownerChannel.id); - if (createOnServerSide) { - await serverSideChannel.create(); - } else { - await ownerChannel.create(); - } - await serverSideChannel.addModerators([modID]); - return { - owner: { - id: ownerID, - client: ownerClient, - channel: ownerChannel, - }, - member: { - id: memberID, - client: memberClient, - channel: memberChannel, - }, - mod: { - id: modID, - client: modClient, - channel: modChannel, - }, - user: { - id: userID, - client: await userClient, - channel: userChannel, - }, - serverSide: { - client: serverSideClient, - channel: serverSideChannel, - }, - }; -} - function testMessagePinPermissions(type, createOnServerSide, matrix) { let chat; before(async () => { diff --git a/test/integration/reactions.js b/test/integration/reactions.js index 87a2e4e0a8..2f49b0e899 100644 --- a/test/integration/reactions.js +++ b/test/integration/reactions.js @@ -2,7 +2,12 @@ import chai from 'chai'; import chaiAsPromised from 'chai-as-promised'; -import { expectHTTPErrorCode, getTestClient, getTestClientForUser } from './utils'; +import { + expectHTTPErrorCode, + getTestClient, + getTestClientForUser, + setupTestChannel, +} from './utils'; import { v4 as uuidv4 } from 'uuid'; const expect = chai.expect; @@ -408,3 +413,98 @@ describe('Reactions', () => { }); }); }); + +describe('When reacting to a message', () => { + let chat; + let message; + before(async () => { + chat = await setupTestChannel('messaging'); + const resp = await chat.owner.channel.sendMessage({ + text: 'Hello', + }); + message = resp.message; + }); + it('Reaction interactions between 2 users', async () => { + let memberEvent; + let ownerEvent; + await chat.member.channel.watch(); + await chat.owner.channel.watch(); + const member = new Promise((resolve) => { + let count = 0; + chat.member.channel.on('reaction.new', (e) => { + count++; + if (count === 2) { + memberEvent = e; + resolve(); + } + }); + }); + const owner = new Promise((resolve) => { + let count = 0; + chat.owner.channel.on('reaction.new', (e) => { + count++; + if (count === 2) { + ownerEvent = e; + resolve(); + } + }); + }); + await chat.member.channel.sendReaction(message.id, { + type: 'like', + }); + await chat.owner.channel.sendReaction(message.id, { + type: 'like', + score: 10, + }); + await Promise.all([member, owner]); + expect(ownerEvent.message).is.not.undefined; + expect(ownerEvent.message.own_reactions.length).to.be.equal(1); + expect(ownerEvent.message.own_reactions[0].user_id).to.be.equal(chat.owner.id); + expect(memberEvent.message.own_reactions.length).to.be.equal(1); + expect(memberEvent.message.own_reactions[0].user_id).to.be.equal(chat.member.id); + + expect(ownerEvent.message.reaction_counts).has.key('like'); + expect(ownerEvent.message.reaction_counts['like']).to.be.equal(2); + expect(ownerEvent.message.reaction_scores).has.key('like'); + expect(ownerEvent.message.reaction_scores['like']).to.be.equal(11); + expect(ownerEvent.message.latest_reactions.length).to.be.equal(2); + + expect(memberEvent.message.reaction_counts).has.key('like'); + expect(memberEvent.message.reaction_counts['like']).to.be.equal(2); + expect(memberEvent.message.reaction_scores).has.key('like'); + expect(memberEvent.message.reaction_scores['like']).to.be.equal(11); + expect(memberEvent.message.latest_reactions.length).to.be.equal(2); + }); + it('Owner reacts with enforce_unique, member sees no own_reactions', async () => { + let memberEvent; + let ownerEvent; + await chat.member.channel.watch(); + await chat.owner.channel.watch(); + const member = new Promise((resolve) => { + chat.member.channel.on('reaction.updated', (e) => { + memberEvent = e; + resolve(); + }); + }); + const owner = new Promise((resolve) => { + chat.owner.channel.on('reaction.updated', (e) => { + ownerEvent = e; + resolve(); + }); + }); + await chat.owner.channel.sendReaction(message.id, { + type: 'like', + }); + await chat.owner.channel.sendReaction( + message.id, + { + type: 'repost', + }, + { enforce_unique: true }, + ); + await Promise.all([member, owner]); + expect(ownerEvent.message.own_reactions.length).to.be.equal(1); + expect(ownerEvent.message.own_reactions[0].user_id).to.be.equal(chat.owner.id); + expect(memberEvent.message.own_reactions.length).to.be.equal(0); + }); +}); diff --git a/test/integration/utils.js b/test/integration/utils.js index 7d18380050..59ab25c74d 100644 --- a/test/integration/utils.js +++ b/test/integration/utils.js @@ -1,6 +1,7 @@ import { StreamChat } from '../../src'; import chai from 'chai'; import http from 'http'; +import { v4 as uuidv4 } from 'uuid'; const expect = chai.expect; require('dotenv').config(); const apiKey = process.env.STREAM_API_KEY; @@ -210,3 +211,61 @@ export function randomUnicodeString(length) { return azAZ09[Math.floor(Math.random() * azAZ09.length)]; }).join(''); } + +// setupTestChannel sets up all possible client and channel objects for given channel type +// set createOnServerSide=true to execute channel creation from the server side +export async function setupTestChannel(type, createOnServerSide, custom = {}) { + const ownerID = uuidv4(); + const memberID = uuidv4(); + const modID = uuidv4(); + const userID = uuidv4(); + const ownerClient = await getTestClientForUser(ownerID); + const memberClient = await getTestClientForUser(memberID); + const modClient = await getTestClientForUser(modID); + const userClient = await getTestClientForUser(userID); + const serverSideClient = getServerTestClient(); + const ownerChannel = ownerClient.channel(type, uuidv4(), { + ...custom, + members: [ownerID, modID, memberID], + }); + const memberChannel = memberClient.channel(ownerChannel.type, ownerChannel.id); + const modChannel = modClient.channel(ownerChannel.type, ownerChannel.id); + const serverSideChannel = serverSideClient.channel( + ownerChannel.type, + ownerChannel.id, + { created_by_id: ownerID }, + ); + const userChannel = userClient.channel(ownerChannel.type, ownerChannel.id); + if (createOnServerSide) { + await serverSideChannel.create(); + } else { + await ownerChannel.create(); + } + await serverSideChannel.addModerators([modID]); + return { + owner: { + id: ownerID, + client: ownerClient, + channel: ownerChannel, + }, + member: { + id: memberID, + client: memberClient, + channel: memberChannel, + }, + mod: { + id: modID, + client: modClient, + channel: modChannel, + }, + user: { + id: userID, + client: await userClient, + channel: userChannel, + }, + serverSide: { + client: serverSideClient, + channel: serverSideChannel, + }, + }; +} diff --git a/test/unit/channel_state.js b/test/unit/channel_state.js index 9f2172a538..9a12b1005a 100644 --- a/test/unit/channel_state.js +++ b/test/unit/channel_state.js @@ -1,5 +1,5 @@ import chai from 'chai'; -import { ChannelState } from '../../src/channel_state'; +import { ChannelState, StreamChat, Channel } from '../../src'; import { generateMsg } from './test-utils/generateMessage'; const expect = chai.expect; @@ -252,3 +252,64 @@ describe('ChannelState addMessagesSorted', function () { expect(state.pinnedMessages[2].id).to.be.equal('2'); }); }); + +describe('ChannelState reactions', () => { + const message = generateMsg(); + let state; + beforeEach(() => { + const client = new StreamChat(); + client.userID = 'observer'; + state = new ChannelState(new Channel(client, 'live', 'stream', {})); + state.addMessageSorted(message); + }); + it('Add one reaction', () => { + const newMessage = state.addReaction( + { + user_id: 'observer', + type: 'like', + score: 1, + }, + message, + ); + expect(newMessage.own_reactions.length).to.be.eq(1); + }); + it('Add same reaction twice', () => { + let newMessage = state.addReaction( + { + user_id: 'observer', + type: 'like', + score: 1, + }, + message, + ); + newMessage = state.addReaction( + { + user_id: 'observer', + type: 'like', + score: 1, + }, + newMessage, + ); + expect(newMessage.own_reactions.length).to.be.eq(1); + }); + it('Add two reactions', () => { + let newMessage = state.addReaction( + { + user_id: 'observer', + type: 'like', + score: 1, + }, + message, + ); + newMessage = state.addReaction( + { + user_id: 'user2', + type: 'like', + score: 4, + }, + newMessage, + ); + expect(newMessage.own_reactions.length).to.be.eq(1); + expect(newMessage.own_reactions[0].user_id).to.be.eq('observer'); + }); +});