rework the way unread count works

we no longer refresh on each message read, only once the whole
conversation is read.

app/sql.js

@@ -128,6 +128,7 @@ module.exports = {
   saveMessages,
   removeMessage,
   getUnreadByConversation,
+  getUnreadCountByConversation,
   getMessageBySender,
   getMessagesBySender,
   getMessageIdsFromServerIds,
@@ -2488,6 +2489,27 @@ async function getUnreadByConversation(conversationId) {
   return map(rows, row => jsonToObject(row.json));
 }
 
+async function getUnreadCountByConversation(conversationId) {
+  const row = await db.get(
+    `SELECT count(*) from ${MESSAGES_TABLE} WHERE
+    unread = $unread AND
+    conversationId = $conversationId
+    ORDER BY received_at DESC;`,
+    {
+      $unread: 1,
+      $conversationId: conversationId,
+    }
+  );
+
+  if (!row) {
+    throw new Error(
+      `getUnreadCountByConversation: Unable to get unread count of ${conversationId}`
+    );
+  }
+
+  return row['count(*)'];
+}
+
 // Note: Sorting here is necessary for getting the last message (with limit 1)
 async function getMessagesByConversation(
   conversationId,

js/models/conversations.d.ts

@@ -52,6 +52,8 @@ export interface ConversationModel
   isMediumGroup: () => boolean;
   getNickname: () => string | undefined;
   setNickname: (nickname: string | undefined) => Promise<void>;
+  getUnread: () => Promise<Whisper.MessageCollection>;
+  getUnreadCount: () => Promise<number>;
 
   isPublic: () => boolean;
   isClosedGroup: () => boolean;

js/models/conversations.js

@@ -1045,6 +1045,10 @@
       });
     },
 
+    async getUnreadCount() {
+      return window.Signal.Data.getUnreadCountByConversation(this.id);
+    },
+
     validate(attributes) {
       const required = ['id', 'type'];
       const missing = _.filter(required, attr => !attributes[attr]);
@@ -1879,14 +1883,21 @@
 
       // Some messages we're marking read are local notifications with no sender
       read = _.filter(read, m => Boolean(m.sender));
+      const realUnreadCount = await this.getUnreadCount();
       if (read.length === 0) {
-        window.log.info('markRead(): nothing newly read.');
+        const cachedUnreadCountOnConvo = this.get('unreadCount');
+        if (cachedUnreadCountOnConvo !== read.length) {
+          // reset the unreadCount on the convo to the real one coming from markRead messages on the db
+          this.set({ unreadCount: realUnreadCount });
+          this.commit();
+        } else {
+          window.log.info('markRead(): nothing newly read.');
+        }
         return;
       }
       unreadMessages = unreadMessages.filter(m => Boolean(m.isIncoming()));
 
-      const unreadCount = unreadMessages.length - read.length;
-      this.set('unreadCount', unreadCount);
+      this.set({ unreadCount: realUnreadCount });
 
       const mentionRead = (() => {
         const stillUnread = unreadMessages.filter(

js/modules/data.js

@@ -145,6 +145,7 @@ module.exports = {
   removeMessage,
   _removeMessages,
   getUnreadByConversation,
+  getUnreadCountByConversation,
 
   removeAllMessagesInConversation,
 
@@ -1029,6 +1030,10 @@ async function getUnreadByConversation(conversationId, { MessageCollection }) {
   return new MessageCollection(messages);
 }
 
+async function getUnreadCountByConversation(conversationId) {
+  return channels.getUnreadCountByConversation(conversationId);
+}
+
 async function getMessagesByConversation(
   conversationId,
   { limit = 100, receivedAt = Number.MAX_VALUE, MessageCollection, type = '%' }

js/views/conversation_view.js

@@ -977,14 +977,6 @@
         if (this.view.atBottom() || options.scroll) {
           lastSeenEl[0].scrollIntoView();
         }
-
-        // scrollIntoView is an async operation, but we have no way to listen for
-        //   completion of the resultant scroll.
-        setTimeout(() => {
-          if (!this.view.atBottom()) {
-            this.addScrollDownButtonWithCount(unreadCount);
-          }
-        }, 1);
       } else if (this.view.atBottom()) {
         // If we already thought we were at the bottom, then ensure that's the case.
         //   Attempting to account for unpredictable completion of message rendering.
@@ -1131,20 +1123,6 @@
       return null;
     },
 
-    markRead() {
-      let unread;
-
-      if (this.view.atBottom()) {
-        unread = this.model.messageCollection.last();
-      } else {
-        unread = this.findNewestVisibleUnread();
-      }
-
-      if (unread) {
-        this.model.markRead(unread.get('received_at'));
-      }
-    },
-
     async showMembers(e, providedMembers, options = {}) {
       _.defaults(options, { needVerify: false });
 

stylesheets/_session_conversation.scss

@@ -176,20 +176,6 @@
   min-width: 370px;
   scrollbar-width: 4px;
   padding: $session-margin-sm $session-margin-lg $session-margin-lg;
-
-  &__loading {
-    position: absolute;
-    top: 0px;
-    right: 0px;
-    left: 0px;
-    bottom: 0px;
-    z-index: 100;
-    background-color: $session-shade-2;
-    display: flex;
-    flex-grow: 1;
-    align-items: center;
-    justify-content: center;
-  }
 }
 
 .session-message-wrapper {

ts/components/conversation/Message.tsx

@@ -946,12 +946,8 @@ export class Message extends React.PureComponent<Props, State> {
   // tslint:disable-next-line: cyclomatic-complexity
   public render() {
     const {
-      authorPhoneNumber,
       direction,
       id,
-      isKickedFromGroup,
-      isRss,
-      timestamp,
       selected,
       multiSelectMode,
       conversationType,

ts/components/session/LeftPaneMessageSection.tsx

@@ -122,7 +122,6 @@ export class LeftPaneMessageSection extends React.Component<Props, State> {
     }
 
     const conversation = conversations[index];
-    const conversationKey = conversation.id;
 
     return (
       <ConversationListItemWithDetails

ts/components/session/conversation/SessionConversation.tsx

@@ -29,8 +29,6 @@ import { SessionFileDropzone } from './SessionFileDropzone';
 import { ConversationType } from '../../../state/ducks/conversations';
 
 interface State {
-  conversationKey: string;
-
   // Message sending progress
   messageProgressVisible: boolean;
   sendingProgress: number;
@@ -46,7 +44,6 @@ interface State {
   messages: Array<any>;
   selectedMessages: Array<string>;
   isScrolledToBottom: boolean;
-  doneInitialScroll: boolean;
   displayScrollToBottomButton: boolean;
 
   showOverlay: boolean;
@@ -93,13 +90,11 @@ export class SessionConversation extends React.Component<Props, State> {
       sendingProgress: 0,
       prevSendingProgress: 0,
       sendingProgressStatus: 0,
-      conversationKey,
       unreadCount,
       initialFetchComplete: false,
       messages: [],
       selectedMessages: [],
       isScrolledToBottom: !unreadCount,
-      doneInitialScroll: false,
       displayScrollToBottomButton: false,
       showOverlay: false,
       showRecordingView: false,
@@ -134,6 +129,7 @@ export class SessionConversation extends React.Component<Props, State> {
     this.replyToMessage = this.replyToMessage.bind(this);
     this.onClickAttachment = this.onClickAttachment.bind(this);
     this.downloadAttachment = this.downloadAttachment.bind(this);
+    this.refreshMessages = this.refreshMessages.bind(this);
     this.getMessages = _.throttle(
       this.getMessages.bind(this),
       1000 // one second
@@ -153,14 +149,6 @@ export class SessionConversation extends React.Component<Props, State> {
     this.handleDragOut = this.handleDragOut.bind(this);
     this.handleDrag = this.handleDrag.bind(this);
     this.handleDrop = this.handleDrop.bind(this);
-
-    conversationModel.on('change', () => {
-      // reload as much messages as we had before the change.
-      void this.getMessages(
-        this.state.messages.length ||
-          Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT
-      );
-    });
   }
 
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -173,6 +161,11 @@ export class SessionConversation extends React.Component<Props, State> {
   }
 
   public componentWillUnmount() {
+    const { conversationKey } = this.props;
+    const conversationModel = window.ConversationController.getOrThrow(
+      conversationKey
+    );
+    conversationModel.off('change', this.refreshMessages);
     const div = this.messageContainerRef.current;
     div?.removeEventListener('dragenter', this.handleDragIn);
     div?.removeEventListener('dragleave', this.handleDragOut);
@@ -180,7 +173,26 @@ export class SessionConversation extends React.Component<Props, State> {
     div?.removeEventListener('drop', this.handleDrop);
   }
 
+  public componentDidUpdate(prevProps: Props, prevState: State) {
+    const { conversationKey: oldKey } = prevProps;
+    const oldConversationModel = window.ConversationController.getOrThrow(
+      oldKey
+    );
+    oldConversationModel.off('change', this.refreshMessages);
+    const { conversationKey: newKey } = this.props;
+    const newCconversationModel = window.ConversationController.getOrThrow(
+      newKey
+    );
+    newCconversationModel.on('change', this.refreshMessages);
+  }
+
   public componentDidMount() {
+    // reload as much messages as we had before the change.
+    const { conversationKey } = this.props;
+    const conversationModel = window.ConversationController.getOrThrow(
+      conversationKey
+    );
+    conversationModel.on('change', this.refreshMessages);
     // Pause thread to wait for rendering to complete
     setTimeout(() => {
       const div = this.messageContainerRef.current;
@@ -196,7 +208,6 @@ export class SessionConversation extends React.Component<Props, State> {
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   public render() {
     const {
-      conversationKey,
       showRecordingView,
       showOptionsPane,
       quotedMessageProps,
@@ -205,7 +216,7 @@ export class SessionConversation extends React.Component<Props, State> {
     } = this.state;
     const selectionMode = !!selectedMessages.length;
 
-    const { conversation } = this.props;
+    const { conversation, conversationKey } = this.props;
     const conversationModel = window.ConversationController.getOrThrow(
       conversationKey
     );
@@ -314,11 +325,8 @@ export class SessionConversation extends React.Component<Props, State> {
     // After the inital fetch, all new messages are automatically added from onNewMessage
     // in the conversation model.
     // The only time we need to call getMessages() is to grab more messages on scroll.
-    const { conversationKey, initialFetchComplete } = this.state;
-    const conversationModel = window.ConversationController.getOrThrow(
-      conversationKey
-    );
-
+    const { initialFetchComplete } = this.state;
+    const { conversationKey } = this.props;
     if (initialFetchComplete) {
       return;
     }
@@ -331,21 +339,15 @@ export class SessionConversation extends React.Component<Props, State> {
       }
     );
 
-    const messages = messageSet.models;
-
-    this.setState({ messages }, () => {
-      // Add new messages to conversation collection
-      conversationModel.messageCollection = messageSet;
-    });
+    this.setState({ messages: messageSet.models });
   }
 
   public async getMessages(numMessages?: number) {
-    const { conversationKey } = this.state;
-
+    const { unreadCount } = this.state;
+    const { conversationKey } = this.props;
     let msgCount =
       numMessages ||
-      Number(Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT) +
-        this.state.unreadCount;
+      Number(Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT) + unreadCount;
     msgCount =
       msgCount > Constants.CONVERSATION.MAX_MESSAGE_FETCH_COUNT
         ? Constants.CONVERSATION.MAX_MESSAGE_FETCH_COUNT
@@ -381,7 +383,8 @@ export class SessionConversation extends React.Component<Props, State> {
   }
 
   public getHeaderProps() {
-    const { conversationKey, selectedMessages } = this.state;
+    const { conversationKey } = this.props;
+    const { selectedMessages, infoViewState } = this.state;
     const conversation = window.ConversationController.getOrThrow(
       conversationKey
     );
@@ -415,7 +418,7 @@ export class SessionConversation extends React.Component<Props, State> {
       subscriberCount: conversation.get('subscriberCount'),
       isKickedFromGroup: conversation.get('isKickedFromGroup'),
       expirationSettingName,
-      showBackButton: Boolean(this.state.infoViewState),
+      showBackButton: Boolean(infoViewState),
       timerOptions: window.Whisper.ExpirationTimerOptions.map((item: any) => ({
         name: item.getName(),
         value: item.get('seconds'),
@@ -489,17 +492,15 @@ export class SessionConversation extends React.Component<Props, State> {
   public getMessagesListProps() {
     const { conversation } = this.props;
     const {
-      conversationKey,
       messages,
       initialFetchComplete,
       quotedMessageTimestamp,
-      doneInitialScroll,
       selectedMessages,
     } = this.state;
 
     return {
       selectedMessages,
-      conversationKey,
+      conversationKey: conversation.id,
       messages,
       resetSelection: this.resetSelection,
       initialFetchComplete,
@@ -508,7 +509,6 @@ export class SessionConversation extends React.Component<Props, State> {
       selectMessage: this.selectMessage,
       getMessages: this.getMessages,
       replyToMessage: this.replyToMessage,
-      doneInitialScroll,
       onClickAttachment: this.onClickAttachment,
       onDownloadAttachment: this.downloadAttachment,
       messageContainerRef: this.messageContainerRef,
@@ -517,7 +517,7 @@ export class SessionConversation extends React.Component<Props, State> {
   }
 
   public getGroupSettingsProps() {
-    const { conversationKey } = this.state;
+    const { conversationKey } = this.props;
     const conversation = window.ConversationController.getOrThrow(
       conversationKey
     );
@@ -629,7 +629,9 @@ export class SessionConversation extends React.Component<Props, State> {
 
   public async deleteSelectedMessages() {
     // Get message objects
-    const { conversationKey, messages } = this.state;
+    const { messages } = this.state;
+    const { conversationKey } = this.props;
+
     const conversationModel = window.ConversationController.getOrThrow(
       conversationKey
     );
@@ -696,9 +698,6 @@ export class SessionConversation extends React.Component<Props, State> {
           return;
         }
       } else {
-        selectedMessages.forEach(m =>
-          conversationModel.messageCollection.remove(m.id)
-        );
         toDeleteLocally = selectedMessages;
       }
 
@@ -707,7 +706,6 @@ export class SessionConversation extends React.Component<Props, State> {
           await window.Signal.Data.removeMessage(message.id, {
             Message: window.Whisper.Message,
           });
-          message.trigger('unload');
         })
       );
 
@@ -775,8 +773,7 @@ export class SessionConversation extends React.Component<Props, State> {
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   private async replyToMessage(quotedMessageTimestamp?: number) {
     if (!_.isEqual(this.state.quotedMessageTimestamp, quotedMessageTimestamp)) {
-      const { conversationKey } = this.state;
-      const { conversation } = this.props;
+      const { conversation, conversationKey } = this.props;
       const conversationModel = window.ConversationController.getOrThrow(
         conversationKey
       );
@@ -1172,4 +1169,11 @@ export class SessionConversation extends React.Component<Props, State> {
       this.setState({ isDraggingFile: false });
     }
   }
+
+  private refreshMessages() {
+    void this.getMessages(
+      this.state.messages.length ||
+        Constants.CONVERSATION.DEFAULT_MESSAGE_FETCH_COUNT
+    );
+  }
 }

ts/components/session/conversation/SessionConversationMessagesList.tsx

@@ -7,7 +7,6 @@ import { SessionScrollButton } from '../SessionScrollButton';
 import { ResetSessionNotification } from '../../conversation/ResetSessionNotification';
 import { Constants } from '../../../session';
 import _ from 'lodash';
-import { ConversationModel } from '../../../../js/models/conversations';
 import { contextMenu } from 'react-contexify';
 import { AttachmentType } from '../../../types/Attachment';
 import { GroupNotification } from '../../conversation/GroupNotification';
@@ -66,25 +65,31 @@ export class SessionConversationMessagesList extends React.Component<
   public componentDidMount() {
     // Pause thread to wait for rendering to complete
     setTimeout(this.scrollToUnread, 0);
-    this.updateReadMessages();
   }
 
-  public componentDidUpdate() {
-    // Keep scrolled to bottom unless user scrolls up
-    if (this.state.isScrolledToBottom) {
-      this.scrollToBottom();
-      // this.updateReadMessages();
+  public componentDidUpdate(prevProps: Props, _prevState: State) {
+    if (prevProps.conversationKey !== this.props.conversationKey) {
+      // we have a bit of cleaning to do here
+      this.setState(
+        {
+          isScrolledToBottom: false,
+          showScrollButton: false,
+          doneInitialScroll: false,
+        },
+        this.scrollToUnread
+      );
+    } else {
+      // Keep scrolled to bottom unless user scrolls up
+      if (this.state.isScrolledToBottom) {
+        this.scrollToBottom();
+      }
     }
   }
 
   public render() {
     const { messages } = this.props;
+    const { showScrollButton } = this.state;
 
-    const { doneInitialScroll, showScrollButton } = this.state;
-
-    if (!doneInitialScroll) {
-      return <div className="messages-container__loading" />;
-    }
     return (
       <div
         className="messages-container"
@@ -182,24 +187,24 @@ export class SessionConversationMessagesList extends React.Component<
     const { messages, conversationKey } = this.props;
     const { isScrolledToBottom } = this.state;
 
-    const conversation = window.ConversationController.getOrThrow(
-      conversationKey
-    );
+    let unread;
 
-    if (conversation.isBlocked()) {
+    if (!messages || messages.length === 0) {
       return;
     }
 
-    let unread;
+    const conversation = window.ConversationController.getOrThrow(
+      conversationKey
+    );
 
-    if (!messages || messages.length === 0) {
+    if (conversation.isBlocked()) {
       return;
     }
 
     if (isScrolledToBottom) {
       unread = messages[0];
     } else {
-      unread = this.findNewestVisibleUnread();
+      unread = null;
     }
 
     if (unread) {
@@ -207,69 +212,6 @@ export class SessionConversationMessagesList extends React.Component<
     }
   }
 
-  public findNewestVisibleUnread() {
-    const messageContainer = this.messageContainerRef.current;
-    if (!messageContainer) {
-      return null;
-    }
-
-    const { messages, conversation } = this.props;
-    const { length } = messages;
-
-    const viewportBottom =
-      (messageContainer?.clientHeight as number) +
-        (messageContainer?.scrollTop as number) || 0;
-
-    // Start with the most recent message, search backwards in time
-    let foundUnread = 0;
-    for (let i = length - 1; i >= 0; i -= 1) {
-      // Search the latest 30, then stop if we believe we've covered all known
-      //   unread messages. The unread should be relatively recent.
-      // Why? local notifications can be unread but won't be reflected the
-      //   conversation's unread count.
-      if (i > 30 && foundUnread >= conversation.unreadCount) {
-        return null;
-      }
-
-      const message = messages[i];
-
-      if (!message.attributes.unread) {
-        // eslint-disable-next-line no-continue
-        continue;
-      }
-
-      foundUnread += 1;
-
-      const el = document.getElementById(`${message.id}`);
-
-      if (!el) {
-        // eslint-disable-next-line no-continue
-        continue;
-      }
-
-      const top = el.offsetTop;
-
-      // If the bottom fits on screen, we'll call it visible. Even if the
-      //   message is really tall.
-      const height = el.offsetHeight;
-      const bottom = top + height;
-
-      // We're fully below the viewport, continue searching up.
-      if (top > viewportBottom) {
-        // eslint-disable-next-line no-continue
-        continue;
-      }
-
-      if (bottom <= viewportBottom) {
-        return message;
-      }
-
-      // Continue searching up.
-    }
-
-    return null;
-  }
-
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   // ~~~~~~~~~~~~ SCROLLING METHODS ~~~~~~~~~~~~~
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -280,6 +222,10 @@ export class SessionConversationMessagesList extends React.Component<
     }
     contextMenu.hideAll();
 
+    if (!this.state.doneInitialScroll) {
+      return;
+    }
+
     const scrollTop = messageContainer.scrollTop;
     const scrollHeight = messageContainer.scrollHeight;
     const clientHeight = messageContainer.clientHeight;
@@ -307,12 +253,12 @@ export class SessionConversationMessagesList extends React.Component<
     // Scrolled to bottom
     const isScrolledToBottom = scrollOffsetPc === 0;
 
-    // Mark messages read
-    this.updateReadMessages();
-
     // Pin scroll to bottom on new message, unless user has scrolled up
     if (this.state.isScrolledToBottom !== isScrolledToBottom) {
-      this.setState({ isScrolledToBottom });
+      this.setState({ isScrolledToBottom }, () => {
+        // Mark messages read
+        this.updateReadMessages();
+      });
     }
 
     // Fetch more messages when nearing the top of the message list
@@ -336,7 +282,7 @@ export class SessionConversationMessagesList extends React.Component<
 
   public scrollToUnread() {
     const { messages, conversation } = this.props;
-    const message = messages[messages.length - 1 - conversation.unreadCount];
+    const message = messages[conversation.unreadCount];
 
     if (message) {
       this.scrollToMessage(message.id);
@@ -365,6 +311,7 @@ export class SessionConversationMessagesList extends React.Component<
     }
     messageContainer.scrollTop =
       messageContainer.scrollHeight - messageContainer.clientHeight;
+    this.updateReadMessages();
   }
 
   // ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

ts/receiver/errors.ts

@@ -66,6 +66,7 @@ export async function onError(ev: any) {
       id,
       'private'
     );
+    // force conversation unread count to be > 0 so it is highlighted
     conversation.set({
       active_at: Date.now(),
       unreadCount: toNumber(conversation.get('unreadCount')) + 1,

ts/receiver/mediumGroups.ts

@@ -207,10 +207,6 @@ async function handleNewGroup(
 
   // We only set group admins on group creation
   convo.set('groupAdmins', admins);
-  // update the unreadCount for this convo
-  convo.set({
-    unreadCount: Number(convo.get('unreadCount')) + 1,
-  });
   await convo.commit();
 
   const secretKeyHex = toHex(groupPrivateKey);

ts/receiver/queuedJob.ts

@@ -310,7 +310,6 @@ function updateReadStatus(
     conversation.onReadMessage(message);
   } else {
     conversation.set({
-      unreadCount: conversation.get('unreadCount') + 1,
       isArchived: false,
     });
   }
@@ -565,6 +564,9 @@ export async function handleMessageJob(
     //   call it after we have an id for this message, because the jobs refer back
     //   to their source message.
     await queueAttachmentDownloads(message);
+    // this is
+    const unreadCount = await conversation.getUnreadCount();
+    conversation.set({ unreadCount });
     await conversation.commit();
 
     conversation.trigger('newmessage', message);

ts/session/medium_group/index.ts

@@ -540,14 +540,26 @@ export async function addUpdateMessage(
 
   const now = Date.now();
 
+  const markUnread = type === 'incoming';
+
   const message = await convo.addMessage({
     conversationId: convo.get('id'),
     type,
     sent_at: now,
     received_at: now,
     group_update: groupUpdate,
+    unread: markUnread,
   });
 
+  if (markUnread) {
+    // update the unreadCount for this convo
+    const unreadCount = await convo.getUnreadCount();
+    convo.set({
+      unreadCount,
+    });
+    await convo.commit();
+  }
+
   return message;
 }