Timeline: Use messageChangeCounter to mark messages read less often

Co-authored-by: Scott Nonnenberg <scott@signal.org>

ts/components/conversation/Timeline.stories.tsx

@@ -533,6 +533,7 @@ const useProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
     overrideProps.isIncomingMessageRequest === true
   ),
   items: overrideProps.items || Object.keys(items),
+  messageChangeCounter: 0,
   scrollToIndex: overrideProps.scrollToIndex,
   scrollToIndexCounter: 0,
   totalUnseen: number('totalUnseen', overrideProps.totalUnseen || 0),

ts/components/conversation/Timeline.tsx

@@ -86,6 +86,7 @@ export type ContactSpoofingReviewPropType =
 export type PropsDataType = {
   haveNewest: boolean;
   haveOldest: boolean;
+  messageChangeCounter: number;
   messageLoadingState?: TimelineMessageLoadingState;
   isNearBottom?: boolean;
   items: ReadonlyArray<string>;
@@ -647,12 +648,14 @@ export class Timeline extends React.Component<
   ): void {
     const {
       items: oldItems,
+      messageChangeCounter: previousMessageChangeCounter,
       messageLoadingState: previousMessageLoadingState,
     } = prevProps;
     const {
       discardMessages,
       id,
       items: newItems,
+      messageChangeCounter,
       messageLoadingState,
     } = this.props;
 
@@ -711,7 +714,8 @@ export class Timeline extends React.Component<
           numberToKeepAtTop,
         });
       }
-    } else {
+    }
+    if (previousMessageChangeCounter !== messageChangeCounter) {
       this.markNewestBottomVisibleMessageRead();
     }
   }

ts/state/ducks/conversations.ts

@@ -244,6 +244,7 @@ export type MessageLookupType = {
 };
 export type ConversationMessageType = {
   isNearBottom?: boolean;
+  messageChangeCounter: number;
   messageIds: Array<string>;
   messageLoadingState?: undefined | TimelineMessageLoadingState;
   metrics: MessageMetricsType;
@@ -2502,8 +2503,18 @@ export function reducer(
       return state;
     }
 
+    const toIncrement = data.reactions?.length ? 1 : 0;
+
     return {
       ...state,
+      messagesByConversation: {
+        ...state.messagesByConversation,
+        [conversationId]: {
+          ...existingConversation,
+          messageChangeCounter:
+            (existingConversation.messageChangeCounter || 0) + toIncrement,
+        },
+      },
       messagesLookup: {
         ...state.messagesLookup,
         [id]: {
@@ -2582,6 +2593,7 @@ export function reducer(
       messagesByConversation: {
         ...messagesByConversation,
         [conversationId]: {
+          messageChangeCounter: 0,
           scrollToMessageId,
           scrollToMessageCounter: existingConversation
             ? existingConversation.scrollToMessageCounter + 1

ts/state/selectors/conversations.ts

@@ -828,6 +828,7 @@ export function _conversationMessagesSelector(
 ): TimelinePropsType {
   const {
     isNearBottom,
+    messageChangeCounter,
     messageIds,
     messageLoadingState,
     metrics,
@@ -860,6 +861,7 @@ export function _conversationMessagesSelector(
     haveOldest,
     isNearBottom,
     items,
+    messageChangeCounter,
     messageLoadingState,
     oldestUnseenIndex:
       isNumber(oldestUnseenIndex) && oldestUnseenIndex >= 0
@@ -899,6 +901,7 @@ export const getConversationMessagesSelector = createSelector(
         return {
           haveNewest: false,
           haveOldest: false,
+          messageChangeCounter: 0,
           messageLoadingState: TimelineMessageLoadingState.DoingInitialLoad,
           scrollToIndexCounter: 0,
           totalUnseen: 0,

ts/test-electron/state/ducks/conversations_test.ts

@@ -55,21 +55,22 @@ const {
   conversationStoppedByMissingVerification,
   createGroup,
   discardMessages,
+  messageChanged,
   openConversationInternal,
   repairNewestMessage,
   repairOldestMessage,
+  resetAllChatColors,
+  reviewGroupMemberNameCollision,
+  reviewMessageRequestNameCollision,
   setComposeGroupAvatar,
   setComposeGroupName,
   setComposeSearchTerm,
   setPreJoinConversation,
   showArchivedConversations,
+  showChooseGroupMembers,
   showInbox,
   startComposing,
-  showChooseGroupMembers,
   startSettingGroupMetadata,
-  resetAllChatColors,
-  reviewGroupMemberNameCollision,
-  reviewMessageRequestNameCollision,
   toggleConversationInChooseMembers,
 } = actions;
 
@@ -317,7 +318,7 @@ describe('both/state/ducks/conversations', () => {
     function getDefaultMessage(id: string): MessageType {
       return {
         attachments: [],
-        conversationId: 'conversationId',
+        conversationId,
         id,
         received_at: previousTime,
         sent_at: previousTime,
@@ -331,6 +332,7 @@ describe('both/state/ducks/conversations', () => {
 
     function getDefaultConversationMessage(): ConversationMessageType {
       return {
+        messageChangeCounter: 0,
         messageIds: [],
         metrics: {
           totalUnseen: 0,
@@ -1317,6 +1319,7 @@ describe('both/state/ducks/conversations', () => {
         },
         messagesByConversation: {
           [conversationId]: {
+            messageChangeCounter: 0,
             metrics: {
               totalUnseen: 0,
             },
@@ -1387,6 +1390,139 @@ describe('both/state/ducks/conversations', () => {
       });
     });
 
+    describe('MESSAGE_CHANGED', () => {
+      const startState: ConversationsStateType = {
+        ...getEmptyState(),
+        conversationLookup: {
+          [conversationId]: {
+            ...getDefaultConversation(),
+            id: conversationId,
+            groupVersion: 2,
+            groupId: 'dGhpc2lzYWdyb3VwaWR0aGlzaXNhZ3JvdXBpZHRoaXM=',
+          },
+        },
+        messagesByConversation: {
+          [conversationId]: {
+            messageChangeCounter: 0,
+            messageIds: [messageId, messageIdTwo, messageIdThree],
+            metrics: {
+              totalUnseen: 0,
+            },
+            scrollToMessageCounter: 0,
+          },
+        },
+        messagesLookup: {
+          [messageId]: {
+            ...getDefaultMessage(messageId),
+            displayLimit: undefined,
+          },
+          [messageIdTwo]: {
+            ...getDefaultMessage(messageIdTwo),
+            displayLimit: undefined,
+          },
+          [messageIdThree]: {
+            ...getDefaultMessage(messageIdThree),
+            displayLimit: undefined,
+          },
+        },
+      };
+      const changedMessage = {
+        ...getDefaultMessage(messageId),
+        body: 'changed',
+        displayLimit: undefined,
+      };
+
+      it('updates message data', () => {
+        const state = reducer(
+          startState,
+          messageChanged(messageId, conversationId, changedMessage)
+        );
+
+        assert.deepEqual(state.messagesLookup[messageId], changedMessage);
+        assert.strictEqual(
+          state.messagesByConversation[conversationId]?.messageChangeCounter,
+          0
+        );
+      });
+
+      it('does not update lookup if it is a story reply', () => {
+        const state = reducer(
+          startState,
+          messageChanged(messageId, conversationId, {
+            ...changedMessage,
+            storyId: 'story-id',
+          })
+        );
+
+        assert.deepEqual(
+          state.messagesLookup[messageId],
+          startState.messagesLookup[messageId]
+        );
+        assert.strictEqual(
+          state.messagesByConversation[conversationId]?.messageChangeCounter,
+          0
+        );
+      });
+
+      it('increments message change counter if new message has reactions', () => {
+        const changedMessageWithReaction: MessageType = {
+          ...changedMessage,
+          reactions: [
+            {
+              emoji: '🎁',
+              fromId: 'some-other-id',
+              timestamp: 2222,
+              targetTimestamp: 1111,
+              targetAuthorUuid: 'author-uuid',
+            },
+          ],
+        };
+        const state = reducer(
+          startState,
+          messageChanged(messageId, conversationId, changedMessageWithReaction)
+        );
+
+        assert.deepEqual(
+          state.messagesLookup[messageId],
+          changedMessageWithReaction
+        );
+        assert.strictEqual(
+          state.messagesByConversation[conversationId]?.messageChangeCounter,
+          1
+        );
+      });
+
+      it('does not increment message change counter if only old message had reactions', () => {
+        const updatedStartState = {
+          ...startState,
+          messagesLookup: {
+            [messageId]: {
+              ...startState.messagesLookup[messageId],
+              reactions: [
+                {
+                  emoji: '🎁',
+                  fromId: 'some-other-id',
+                  timestamp: 2222,
+                  targetTimestamp: 1111,
+                  targetAuthorUuid: 'author-uuid',
+                },
+              ],
+            },
+          },
+        };
+        const state = reducer(
+          updatedStartState,
+          messageChanged(messageId, conversationId, changedMessage)
+        );
+
+        assert.deepEqual(state.messagesLookup[messageId], changedMessage);
+        assert.strictEqual(
+          state.messagesByConversation[conversationId]?.messageChangeCounter,
+          0
+        );
+      });
+    });
+
     describe('SHOW_ARCHIVED_CONVERSATIONS', () => {
       it('is a no-op when already at the archive', () => {
         const state = {