Keep outgoing pending activity at same position

Author: compulim

packages/core/src/reducers/createActivitiesReducer.ts

@@ -129,8 +129,6 @@ function upsertActivityWithSort(
 
   const { channelData: { clientActivityID: incomingClientActivityID } = {} } = incomingActivity;
 
-  let incomingPosition: number | undefined = incomingActivity.channelData['webchat:internal:position'];
-
   const nextActivities = activities.filter(
     ({ channelData: { clientActivityID } = {}, id }) =>
       // We will remove all "sending messages" activities and activities with same ID
@@ -139,102 +137,93 @@ function upsertActivityWithSort(
       !(id && id === incomingActivity.id)
   );
 
-  // If `channelData['webchat:internal:position']` is already set, use that to find where to insert the activity.
-  // For example, when receiving the echoback, we will want to keep the existing position.
-  if (typeof incomingPosition !== 'number') {
-    const incomingEntityPosition = incomingActivity.channelData?.['webchat:entity-position'];
-    const incomingPartOf = incomingActivity.channelData?.['webchat:entity-part-of'];
-    const incomingSequenceId = getSequenceIdOrDeriveFromTimestamp(incomingActivity, ponyfill);
-
-    // TODO: [P0] Turn (activity) => boolean into comparer (x, y) => number.
-    //       It is not trivial to write in current form.
-    //       We can use comparer for insertion sort too, so let's rewrite in comparer form.
-    let indexToInsert = nextActivities.findIndex(activity => {
-      // TODO: [P1] #3953 We should move this patching logic to a DLJS wrapper for simplicity.
-      // If the message does not have sequence ID, use these fallback values:
-      // 1. `entities.position` where `entities.isPartOf[@type === 'HowTo']`
-      //    - If they are not of same set, ignore `entities.position`
-      // 2. `channelData.streamSequence` field for same session IDk
-      // 3. `channelData['webchat:sequence-id']`
-      //    - If not available, it will fallback to `+new Date(timestamp)`
-      //    - Outgoing activity will not have `timestamp` field
-      const { channelData = {} } = activity;
-      const currentPosition = channelData['webchat:entity-position'];
-      const currentPartOf = channelData['webchat:entity-part-of'];
-
-      const bothHavePosition = typeof currentPosition === 'number' && typeof incomingEntityPosition === 'number';
-      const bothArePartOf = typeof currentPartOf === 'string' && currentPartOf === incomingPartOf;
-
-      // For activities in the same creative work part, position is primary sort key
-      if (bothHavePosition && bothArePartOf) {
-        return currentPosition > incomingEntityPosition;
-      }
+  const incomingEntityPosition = incomingActivity.channelData?.['webchat:entity-position'];
+  const incomingPartOf = incomingActivity.channelData?.['webchat:entity-part-of'];
+  const incomingSequenceId = getSequenceIdOrDeriveFromTimestamp(incomingActivity, ponyfill);
+
+  // TODO: [P0] Turn (activity) => boolean into comparer (x, y) => number.
+  //       It is not trivial to write in current form.
+  //       We can use comparer for insertion sort too, so let's rewrite in comparer form.
+  let indexToInsert = nextActivities.findIndex(activity => {
+    // TODO: [P1] #3953 We should move this patching logic to a DLJS wrapper for simplicity.
+    // If the message does not have sequence ID, use these fallback values:
+    // 1. `entities.position` where `entities.isPartOf[@type === 'HowTo']`
+    //    - If they are not of same set, ignore `entities.position`
+    // 2. `channelData.streamSequence` field for same session IDk
+    // 3. `channelData['webchat:sequence-id']`
+    //    - If not available, it will fallback to `+new Date(timestamp)`
+    //    - Outgoing activity will not have `timestamp` field
+    const { channelData = {} } = activity;
+    const currentEntityPosition = channelData['webchat:entity-position'];
+    const currentEntityPartOf = channelData['webchat:entity-part-of'];
+
+    const bothHavePosition = typeof currentEntityPosition === 'number' && typeof incomingEntityPosition === 'number';
+    const bothArePartOf = typeof currentEntityPartOf === 'string' && currentEntityPartOf === incomingPartOf;
+
+    // For activities in the same creative work part, position is primary sort key
+    if (bothHavePosition && bothArePartOf) {
+      return currentEntityPosition > incomingEntityPosition;
+    }
 
-      const currentLivestreamingMetadata = getActivityLivestreamingMetadata(activity);
+    const currentLivestreamingMetadata = getActivityLivestreamingMetadata(activity);
 
-      if (
-        incomingLivestreamingMetadata &&
-        currentLivestreamingMetadata &&
-        incomingLivestreamingMetadata.sessionId === currentLivestreamingMetadata.sessionId
-      ) {
-        return currentLivestreamingMetadata.sequenceNumber > incomingLivestreamingMetadata.sequenceNumber;
-      }
+    if (
+      incomingLivestreamingMetadata &&
+      currentLivestreamingMetadata &&
+      incomingLivestreamingMetadata.sessionId === currentLivestreamingMetadata.sessionId
+    ) {
+      return currentLivestreamingMetadata.sequenceNumber > incomingLivestreamingMetadata.sequenceNumber;
+    }
 
-      const currentSequenceId = getSequenceIdOrDeriveFromTimestamp(activity, ponyfill);
+    const currentSequenceId = getSequenceIdOrDeriveFromTimestamp(activity, ponyfill);
 
-      if (typeof incomingSequenceId === 'number') {
-        if (typeof currentSequenceId === 'number') {
-          return currentSequenceId > incomingSequenceId;
-        }
-
-        // Always insert activity whose has sequence ID before those whose doesn't have sequence ID.
-        return true;
-      } else if (typeof currentSequenceId === 'number') {
-        return false;
+    if (typeof incomingSequenceId === 'number') {
+      if (typeof currentSequenceId === 'number') {
+        return currentSequenceId > incomingSequenceId;
       }
 
-      // No more properties can be used to find a good insertion spot.
-      // Return `false` so the activity will append to the end.
+      // Always insert activity whose has sequence ID before those whose doesn't have sequence ID.
+      return true;
+    } else if (typeof currentSequenceId === 'number') {
       return false;
-    });
-
-    if (!~indexToInsert) {
-      // If no right place can be found, append it.
-      indexToInsert = nextActivities.length;
     }
 
-    const prevActivity: WebChatActivity = nextActivities.at(indexToInsert - 1);
-    const nextActivity: WebChatActivity = nextActivities.at(indexToInsert);
+    // No more properties can be used to find a good insertion spot.
+    // Return `false` so the activity will append to the end.
+    return false;
+  });
 
-    if (prevActivity) {
-      const prevPosition = prevActivity.channelData['webchat:internal:position'];
+  if (!~indexToInsert) {
+    // If no right place can be found, append it.
+    indexToInsert = nextActivities.length;
+  }
 
-      if (nextActivity) {
-        const nextSequenceId = nextActivity.channelData['webchat:internal:position'];
+  const prevActivity: WebChatActivity = indexToInsert === 0 ? undefined : nextActivities.at(indexToInsert - 1);
+  const nextActivity: WebChatActivity = nextActivities.at(indexToInsert);
+  let incomingPosition: number;
 
-        // eslint-disable-next-line no-magic-numbers
-        incomingPosition = (prevPosition + nextSequenceId) / 2;
-      } else {
-        incomingPosition = prevPosition + 1;
-      }
-    } else if (nextActivity) {
+  if (prevActivity) {
+    const prevPosition = prevActivity.channelData['webchat:internal:position'];
+
+    if (nextActivity) {
       const nextSequenceId = nextActivity.channelData['webchat:internal:position'];
 
-      incomingPosition = nextSequenceId - 1;
+      // eslint-disable-next-line no-magic-numbers
+      incomingPosition = (prevPosition + nextSequenceId) / 2;
     } else {
-      incomingPosition = 0;
+      incomingPosition = prevPosition + 1;
     }
+  } else if (nextActivity) {
+    const nextSequenceId = nextActivity.channelData['webchat:internal:position'];
+
+    incomingPosition = nextSequenceId - 1;
+  } else {
+    incomingPosition = 1;
   }
 
-  const indexToInsert = nextActivities.findIndex(
-    activity => activity.channelData['webchat:internal:position'] > incomingPosition
-  );
+  incomingActivity = updateIn(incomingActivity, ['channelData', 'webchat:internal:position'], () => incomingPosition);
 
-  nextActivities.splice(
-    ~indexToInsert ? indexToInsert : nextActivities.length,
-    0,
-    updateIn(incomingActivity, ['channelData', 'webchat:internal:position'], () => incomingPosition)
-  );
+  nextActivities.splice(indexToInsert, 0, incomingActivity);
 
   return nextActivities;
 }
@@ -273,6 +262,16 @@ export default function createActivitiesReducer(
           activity = updateIn(activity, ['channelData', 'state'], () => SENDING);
           activity = updateIn(activity, ['channelData', 'webchat:send-status'], () => SENDING);
 
+          // Assume the message was sent immediately after the very last message.
+          // This helps to maintain the order of the outgoing message before the server respond.
+          activity = updateIn(activity, ['timestamp'], () => {
+            const lastTimestamp = state.at(-1)?.timestamp;
+
+            return (
+              lastTimestamp ? new ponyfill.Date(+new ponyfill.Date(lastTimestamp) + 1) : new ponyfill.Date(1)
+            ).toISOString();
+          });
+
           state = upsertActivityWithSort(state, activity, ponyfill);
         }
 
@@ -394,11 +393,6 @@ export default function createActivitiesReducer(
               } = existingActivity;
 
               activity = updateIn(activity, ['channelData', 'webchat:internal:id'], () => permanentId);
-              activity = updateIn(
-                activity,
-                ['channelData', 'webchat:internal:position'],
-                () => existingActivity.channelData['webchat:internal:position']
-              );
 
               if (sendStatus === SENDING || sendStatus === SEND_FAILED || sendStatus === SENT) {
                 activity = updateIn(activity, ['channelData', 'webchat:send-status'], () => sendStatus);