fix(addToMessageList): prevent messages shifting when adding reactions

src/utils.ts

@@ -346,22 +346,23 @@ export const findIndexInSortedArray = <T, L>({
     middle = Math.round((left + right) / 2);
   };
 
-  const actualNeedle = selectValueToCompare(needle);
-  recalculateMiddle();
+  const comparableNeedle = selectValueToCompare(needle);
 
   while (left <= right) {
-    // if (actualNeedle === selectValueToCompare(sortedArray[middle])) return middle;
+    recalculateMiddle();
+
+    const comparableMiddle = selectValueToCompare(sortedArray[middle]);
+
+    // if (comparableNeedle === comparableMiddle) return middle;
 
     if (
-      (sortDirection === 'ascending' && actualNeedle < selectValueToCompare(sortedArray[middle])) ||
-      (sortDirection === 'descending' && actualNeedle > selectValueToCompare(sortedArray[middle]))
+      (sortDirection === 'ascending' && comparableNeedle < comparableMiddle) ||
+      (sortDirection === 'descending' && comparableNeedle > comparableMiddle)
     ) {
       right = middle - 1;
     } else {
       left = middle + 1;
     }
-
-    recalculateMiddle();
   }
 
   return left;
@@ -405,7 +406,7 @@ export function addToMessageList<T extends FormatMessageResponse>(
   // find the closest index to push the new message
   const insertionIndex = findIndexInSortedArray({
     needle: newMessage,
-    sortedArray: messages,
+    sortedArray: newMessages,
     sortDirection: 'ascending',
     // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
     selectValueToCompare: (m) => m[sortBy]!.getTime(),

test/unit/threads.test.ts

@@ -107,25 +107,29 @@ describe('Threads 2.0', () => {
           const optimisticMessage = generateMsg({
             parent_id: parentMessageResponse.id,
             text: 'aaa',
-            date: '2020-01-01T00:00:00Z',
+            created_at: '2020-01-01T00:00:00Z',
           }) as MessageResponse;
 
           const message = generateMsg({
             parent_id: parentMessageResponse.id,
             text: 'bbb',
-            date: '2020-01-01T00:00:10Z',
+            created_at: '2020-01-01T00:00:10Z',
           }) as MessageResponse;
 
           const thread = createTestThread({ latest_replies: [optimisticMessage, message] });
-          const udpatedMessage = { ...optimisticMessage, text: 'ccc', date: '2020-01-01T00:00:20Z' };
+          const updatedMessage: MessageResponse = {
+            ...optimisticMessage,
+            text: 'ccc',
+            created_at: '2020-01-01T00:00:20Z',
+          };
 
           const stateBefore = thread.state.getLatestValue();
           expect(stateBefore.replies).to.have.lengthOf(2);
           expect(stateBefore.replies[0].id).to.equal(optimisticMessage.id);
           expect(stateBefore.replies[0].text).to.equal('aaa');
           expect(stateBefore.replies[1].id).to.equal(message.id);
 
-          thread.upsertReplyLocally({ message: udpatedMessage, timestampChanged: true });
+          thread.upsertReplyLocally({ message: updatedMessage, timestampChanged: true });
 
           const stateAfter = thread.state.getLatestValue();
           expect(stateAfter.replies).to.have.lengthOf(2);

test/unit/utils.test.ts

@@ -0,0 +1,107 @@
+import { expect } from 'chai';
+import { v4 as uuidv4 } from 'uuid';
+
+import { generateMsg } from './test-utils/generateMessage';
+
+import { addToMessageList, formatMessage } from '../../src/utils';
+
+import type { FormatMessageResponse, MessageResponse } from '../../src';
+
+describe('addToMessageList', () => {
+  const timestamp = new Date('2024-09-18T15:30:00.000Z').getTime();
+  // messages with each created_at 10 seconds apart
+  let messagesBefore: FormatMessageResponse[];
+
+  const getNewFormattedMessage = ({ timeOffset, id = uuidv4() }: { timeOffset: number; id?: string }) =>
+    formatMessage(
+      generateMsg({
+        id,
+        created_at: new Date(timestamp + timeOffset),
+      }) as MessageResponse,
+    );
+
+  beforeEach(() => {
+    messagesBefore = Array.from({ length: 5 }, (_, index) =>
+      formatMessage(generateMsg({ created_at: new Date(timestamp + index * 10 * 1000) }) as MessageResponse),
+    );
+  });
+
+  it('new message is inserted at the correct index', () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 25 * 1000 });
+
+    const messagesAfter = addToMessageList(messagesBefore, newMessage);
+
+    expect(messagesAfter).to.not.equal(messagesBefore);
+    expect(messagesAfter).to.have.length(6);
+    expect(messagesAfter).to.contain(newMessage);
+    expect(messagesAfter[3]).to.equal(newMessage);
+  });
+
+  it('replaces the message which created_at changed to a server response created_at', () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 33 * 1000, id: messagesBefore[2].id });
+
+    expect(newMessage.id).to.equal(messagesBefore[2].id);
+
+    const messagesAfter = addToMessageList(messagesBefore, newMessage, true);
+
+    expect(messagesAfter).to.not.equal(messagesBefore);
+    expect(messagesAfter).to.have.length(5);
+    expect(messagesAfter).to.contain(newMessage);
+    expect(messagesAfter[3]).to.equal(newMessage);
+  });
+
+  it('adds a new message to an empty message list', () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 0 });
+
+    const emptyMessagesBefore = [];
+
+    const messagesAfter = addToMessageList(emptyMessagesBefore, newMessage);
+
+    expect(messagesAfter).to.have.length(1);
+    expect(messagesAfter).to.contain(newMessage);
+  });
+
+  it("doesn't add a new message to an empty message list if timestampChanged & addIfDoesNotExist are false", () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 0 });
+
+    const emptyMessagesBefore = [];
+
+    const messagesAfter = addToMessageList(emptyMessagesBefore, newMessage, false, 'created_at', false);
+
+    expect(messagesAfter).to.have.length(0);
+  });
+
+  it("adds message to the end of the list if it's the newest one", () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 50 * 1000 });
+
+    const messagesAfter = addToMessageList(messagesBefore, newMessage);
+
+    expect(messagesAfter).to.have.length(6);
+    expect(messagesAfter).to.contain(newMessage);
+    expect(messagesAfter.at(-1)).to.equal(newMessage);
+  });
+
+  it("doesn't add a newest message to a message list if timestampChanged & addIfDoesNotExist are false", () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 50 * 1000 });
+
+    const messagesAfter = addToMessageList(messagesBefore, newMessage, false, 'created_at', false);
+
+    expect(messagesAfter).to.have.length(5);
+    // FIXME: it'd be nice if the function returned old
+    // unchanged array in case of no modification such as this one
+    expect(messagesAfter).to.deep.equal(messagesBefore);
+  });
+
+  it("updates an existing message that wasn't filtered due to changed timestamp (timestampChanged)", () => {
+    const newMessage = getNewFormattedMessage({ timeOffset: 30 * 1000, id: messagesBefore[4].id });
+
+    expect(messagesBefore[4].id).to.equal(newMessage.id);
+    expect(messagesBefore[4].text).to.not.equal(newMessage.text);
+    expect(messagesBefore[4]).to.not.equal(newMessage);
+
+    const messagesAfter = addToMessageList(messagesBefore, newMessage, false, 'created_at', false);
+
+    expect(messagesAfter).to.have.length(5);
+    expect(messagesAfter[4]).to.equal(newMessage);
+  });
+});