Fix blind read reset on received messages

packages/mgt-chat/src/components/ChatList/ChatList.tsx

@@ -4,7 +4,7 @@ import { MgtTemplateProps, ProviderState, Providers, Spinner, log } from '@micro
 import { makeStyles, Button, FluentProvider, shorthands, webLightTheme } from '@fluentui/react-components';
 import { FluentThemeProvider } from '@azure/communication-react';
 import { FluentTheme } from '@fluentui/react';
-import { Chat as GraphChat, ChatMessage } from '@microsoft/microsoft-graph-types';
+import { ChatMessage } from '@microsoft/microsoft-graph-types';
 import {
   StatefulGraphChatListClient,
   GraphChatListClient,
@@ -20,8 +20,8 @@ import { PleaseSignIn } from '../Error/PleaseSignIn';
 import { OpenTeamsLinkError } from '../Error/OpenTeams';
 
 export interface IChatListProps {
-  onSelected: (e: GraphChat) => void;
-  onUnselected?: (e: GraphChat) => void;
+  onSelected: (e: GraphChatThread) => void;
+  onUnselected?: (e: GraphChatThread) => void;
   onLoaded?: (e: GraphChatThread[]) => void;
   onAllMessagesRead: (e: string[]) => void;
   buttonItems?: ChatListButtonItem[];
@@ -94,21 +94,22 @@ export const ChatList = ({
   onAllMessagesRead,
   onLoaded,
   onConnectionChanged,
+  onSelected,
+  onUnselected,
   chatThreadsPerPage,
   ...props
 }: MgtTemplateProps & IChatListProps & IChatListMenuItemsProps) => {
   const styles = useStyles();
 
   const [chatListClient, setChatListClient] = useState<StatefulGraphChatListClient | undefined>();
   const [chatListState, setChatListState] = useState<GraphChatListClient | undefined>();
-  const [internalSelectedChatId, setInternalSelectedChatId] = useState<string | undefined>();
   const loadingRef = useRef(false);
   // wait for provider to be ready before setting client and state
   useEffect(() => {
     const provider = Providers.globalProvider;
     const conditionalLoad = (state: ProviderState | undefined) => {
       if (state === ProviderState.SignedIn && !chatListClient) {
-        const client = new StatefulGraphChatListClient(chatThreadsPerPage);
+        const client = new StatefulGraphChatListClient(chatThreadsPerPage, selectedChatId);
         setChatListClient(client);
         setChatListState(client.getState());
       }
@@ -117,30 +118,20 @@ export const ChatList = ({
       conditionalLoad(evt.detail);
     });
     conditionalLoad(provider?.state);
-  }, [chatListClient, chatThreadsPerPage]);
-
-  // if selected chat id is changed, update the internal state
-  // NOTE: Decoupling this ensures that the app can change the selection but the chat
-  // list can also change the selection without a full re - render.
-  useEffect(() => {
-    setInternalSelectedChatId(selectedChatId);
-  }, [selectedChatId]);
+  }, [chatListClient, chatThreadsPerPage, selectedChatId]);
 
   // Store last read time in cache so that when the user comes back to the chat list,
   // we know what messages they are likely to have not read. This is not perfect because
   // the user could have read messages in another client (for instance, the Teams client).
   useEffect(() => {
     const timer = setInterval(() => {
-      if (internalSelectedChatId) {
-        log(`caching the last-read timestamp of now to chat ID '${internalSelectedChatId}'...`);
-        chatListClient?.cacheLastReadTime([internalSelectedChatId]);
-      }
+      chatListClient?.cacheLastReadTime('selected');
     }, lastReadTimeInterval);
 
     return () => {
       clearInterval(timer);
     };
-  }, [chatListClient, internalSelectedChatId, lastReadTimeInterval]);
+  }, [chatListClient, lastReadTimeInterval]);
 
   useEffect(() => {
     // shortcut if we don't have a chat list client
@@ -151,10 +142,20 @@ export const ChatList = ({
     // handle state changes
     chatListClient.onStateChange(setChatListState);
     chatListClient.onStateChange(state => {
-      if (state.status === 'chat message received') {
-        if (onMessageReceived && state.chatMessage) {
-          onMessageReceived(state.chatMessage);
-        }
+      if (state.status === 'chat message received' && onMessageReceived && state.chatMessage) {
+        onMessageReceived(state.chatMessage);
+      }
+
+      if (state.status === 'chat selected' && onSelected && state.internalSelectedChat) {
+        onSelected(state.internalSelectedChat);
+      }
+
+      if (state.status === 'chat unselected' && onUnselected && state.internalPrevSelectedChat) {
+        onUnselected(state.internalPrevSelectedChat);
+      }
+
+      if (state.status === 'chats read' && onAllMessagesRead && state.chatThreads) {
+        onAllMessagesRead(state.chatThreads.map(c => c.id!));
       }
 
       if (state.status === 'chats loaded' && onLoaded) {
@@ -163,6 +164,10 @@ export const ChatList = ({
         loadingRef.current = false;
       }
 
+      if (state.status === 'chats loaded' && state.initialSelectedChatSet && onSelected && state.internalSelectedChat) {
+        onSelected(state.internalSelectedChat);
+      }
+
       if (state.status === 'no chats' && onLoaded) {
         onLoaded([]);
       }
@@ -175,7 +180,7 @@ export const ChatList = ({
         onConnectionChanged(false);
       }
     });
-  }, [chatListClient, onMessageReceived, onLoaded, onConnectionChanged]);
+  }, [chatListClient, onLoaded, onMessageReceived, onSelected, onUnselected, onAllMessagesRead, onConnectionChanged]);
 
   // this only runs once when the component is unmounted
   useEffect(() => {
@@ -190,44 +195,15 @@ export const ChatList = ({
     }
   }, []);
 
-  const markThreadAsRead = (chatThread: string) => {
-    const markedChatThreads = chatListClient?.markChatThreadsAsRead([chatThread]);
-    if (markedChatThreads) {
-      chatListClient?.cacheLastReadTime(markedChatThreads);
-    }
-  };
-
-  const onClickChatListItem = (chatListItem: GraphChat) => {
-    // trigger an unselect event for the previously selected item
-    if (internalSelectedChatId && props.onUnselected) {
-      const previouslySelectedChatListItem = chatListState?.chatThreads.filter(c => c.id === internalSelectedChatId);
-      if (previouslySelectedChatListItem?.length === 1) {
-        props.onUnselected(previouslySelectedChatListItem[0]);
-      }
-    }
-
-    // select a new item
-    markThreadAsRead(chatListItem.id!);
-    setInternalSelectedChatId(chatListItem.id);
-    props.onSelected(chatListItem);
+  const onClickChatListItem = (chat: GraphChatThread) => {
+    chatListClient?.setInternalSelectedChat(chat);
   };
 
   const chatListButtonItems = props.buttonItems === undefined ? [] : props.buttonItems;
 
-  const markAllThreadsAsRead = (chatThreads: GraphChat[] | undefined) => {
-    if (!chatThreads) {
-      return;
-    }
-    const readChatThreads = chatThreads.map(c => c.id!);
-    const markedChatThreads = chatListClient?.markChatThreadsAsRead(readChatThreads);
-    if (markedChatThreads) {
-      chatListClient?.cacheLastReadTime(markedChatThreads);
-      onAllMessagesRead(markedChatThreads);
-    }
-  };
   const markAllAsRead = {
     displayText: 'Mark all as read',
-    onClick: () => markAllThreadsAsRead(chatListState?.chatThreads)
+    onClick: () => chatListClient?.markAllChatThreadsAsRead()
   };
 
   const isLoading = ['creating server connections', 'subscribing to notifications', 'loading messages'].includes(
@@ -284,8 +260,8 @@ export const ChatList = ({
                       key={c.id}
                       chat={c}
                       myId={chatListState.userId}
-                      isSelected={c.id === internalSelectedChatId}
-                      isRead={c.id === internalSelectedChatId || c.isRead}
+                      isSelected={c.id === chatListState?.internalSelectedChat?.id}
+                      isRead={c.isRead}
                     />
                   </Button>
                 ))}

packages/mgt-chat/src/components/ChatListItem/ChatListItem.tsx

@@ -15,9 +15,10 @@ import { rewriteEmojiContentToText } from '../../utils/rewriteEmojiContent';
 import { convert } from 'html-to-text';
 import { loadChatWithPreview, loadAppsInChat } from '../../statefulClient/graph.chat';
 import { DefaultProfileIcon } from './DefaultProfileIcon';
+import { GraphChatThread } from '../../statefulClient/StatefulGraphChatListClient';
 
 interface IChatListItemProps {
-  chat: Chat;
+  chat: GraphChatThread;
   myId: string | undefined;
   isSelected: boolean;
   isRead: boolean;
@@ -153,7 +154,11 @@ export const ChatListItem = ({ chat, myId, isSelected, isRead }: IChatListItemPr
         };
         load(chatInternal.id!).then(
           c => {
-            setChatInternal(c);
+            const chatThread = {
+              ...(c as GraphChatThread),
+              isRead: chat.isRead
+            };
+            setChatInternal(chatThread);
           },
           e => error(e)
         );

packages/mgt-chat/src/statefulClient/StatefulGraphChatListClient.ts

@@ -76,10 +76,15 @@ export type GraphChatListClient = Pick<MessageThreadProps, 'userId'> & {
     | 'no chats'
     | 'chats loaded'
     | 'chat message received'
-    | 'chats read';
+    | 'chats read'
+    | 'chat selected'
+    | 'chat unselected';
   chatThreads: GraphChatThread[];
   chatMessage: ChatMessage | undefined;
   moreChatThreadsToLoad: boolean | undefined;
+  internalSelectedChat: GraphChatThread | undefined;
+  internalPrevSelectedChat: GraphChatThread | undefined;
+  initialSelectedChatSet: boolean;
 } & Pick<ErrorBarProps, 'activeErrorMessages'>;
 
 interface StatefulClient<T> {
@@ -106,15 +111,18 @@ interface StatefulClient<T> {
    */
   loadMoreChatThreads(): void;
   /**
-   * Method for marking chat threads as read
-   * @param chatThreads - chat threads to mark as read
+   * Method for marking all chat threads as read
    */
-  markChatThreadsAsRead(chatThreads: string[]): string[];
+  markAllChatThreadsAsRead(): void;
   /**
    * Method for caching last read time for all included chat threads
-   * @param chatThreads - chat threads to cache last read time for
    */
-  cacheLastReadTime(chatThreads: string[]): void;
+  cacheLastReadTime(action: 'all' | 'selected'): void;
+  /**
+   * Method for setting the currently selected chat
+   * @param chatThread - currently selected chat
+   */
+  setInternalSelectedChat(chatThread: GraphChatThread): void;
 }
 
 type MessageEventType =
@@ -150,16 +158,18 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
   private readonly _notificationClient: GraphNotificationUserClient;
   private readonly _eventEmitter: ThreadEventEmitter;
   private readonly _cache: LastReadCache;
-  private _stateSubscribers: ((state: GraphChatListClient) => void)[] = [];
   private readonly _graph: IGraph;
-  constructor(chatThreadsPerPage: number) {
+  private _stateSubscribers: ((state: GraphChatListClient) => void)[] = [];
+  private _initialSelectedChatId: string | undefined;
+  constructor(chatThreadsPerPage: number, initialSelectedChatId: string | undefined) {
     this.userId = currentUserId();
     Providers.globalProvider.onActiveAccountChanged(this.onActiveAccountChanged);
     this._eventEmitter = new ThreadEventEmitter();
     this.registerEventListeners();
     this._cache = new LastReadCache();
     this._graph = graph('mgt-chat', GraphConfig.version);
     this.chatThreadsPerPage = chatThreadsPerPage;
+    this._initialSelectedChatId = initialSelectedChatId;
     this._notificationClient = new GraphNotificationUserClient(this._eventEmitter, this._graph);
 
     void this.updateUserSubscription(this.userId);
@@ -223,14 +233,38 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
     }
   }
 
-  public markChatThreadsAsRead = (readChatThreads: string[]): string[] => {
+  public setInternalSelectedChat = (chatThread: GraphChatThread): void => {
+    const state = this.getState();
+
+    if (state.internalSelectedChat) {
+      this.notifyStateChange((draft: GraphChatListClient) => {
+        draft.status = 'chat unselected';
+        draft.internalPrevSelectedChat = state.internalSelectedChat;
+        draft.internalSelectedChat = undefined;
+      });
+    }
+
+    this.notifyStateChange((draft: GraphChatListClient) => {
+      draft.status = 'chat selected';
+      draft.internalSelectedChat = {
+        ...chatThread,
+        isRead: true
+      };
+      draft.chatThreads = state.chatThreads.map((c: GraphChatThread) => {
+        if (c.id === chatThread.id && draft.internalSelectedChat) {
+          return draft.internalSelectedChat;
+        }
+        return c;
+      });
+    });
+  };
+
+  public markAllChatThreadsAsRead = () => {
     // mark as read after chat thread is found in current state
-    const markedChatThreads: string[] = [];
     this.notifyStateChange((draft: GraphChatListClient) => {
       draft.status = 'chats read';
-      draft.chatThreads = this._state.chatThreads.map((chatThread: GraphChatThread) => {
-        if (chatThread.id && readChatThreads.includes(chatThread.id) && !chatThread.isRead) {
-          markedChatThreads.push(chatThread.id);
+      draft.chatThreads = this.getState().chatThreads.map((chatThread: GraphChatThread) => {
+        if (chatThread.id && !chatThread.isRead) {
           return {
             ...chatThread,
             isRead: true
@@ -240,16 +274,28 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
       });
     });
 
-    return markedChatThreads;
+    this.cacheLastReadTime('all');
   };
 
-  public cacheLastReadTime = (readChatThreads: string[]) => {
+  public cacheLastReadTime = (action: 'all' | 'selected') => {
+    const state = this.getState();
+    // cache last read time after all chat threads
+    if (action === 'all') {
+      state.chatThreads.forEach((chatThread: GraphChatThread) => {
+        if (chatThread.id) {
+          void this._cache.cacheLastReadTime(chatThread.id, new Date());
+        }
+      });
+    }
+
     // cache last read time after chat thread is found in current state
-    this._state.chatThreads.forEach((chatThread: GraphChatThread) => {
-      if (chatThread.id && readChatThreads.includes(chatThread.id)) {
-        void this._cache.cacheLastReadTime(chatThread.id, new Date());
+    if (action === 'selected') {
+      const selectedChatId = state.internalSelectedChat?.id;
+      if (selectedChatId) {
+        log(`caching the last-read timestamp of now to chat ID '${selectedChatId}'...`);
+        void this._cache.cacheLastReadTime(selectedChatId, new Date());
       }
-    });
+    }
   };
 
   // check whether to mark the chat as read or not
@@ -311,7 +357,10 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
     userId: '',
     chatThreads: [],
     moreChatThreadsToLoad: undefined,
-    chatMessage: undefined
+    chatMessage: undefined,
+    internalSelectedChat: undefined,
+    internalPrevSelectedChat: undefined,
+    initialSelectedChatSet: false
   };
 
   /**
@@ -414,7 +463,13 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
         // update the last message preview and bring to the top
         chatThread.lastMessagePreview = event.message as ChatMessageInfo;
         chatThread.lastUpdatedDateTime = event.message.lastModifiedDateTime;
-        chatThread.isRead = false;
+        // this resets the chat thread read state for all chats including the active chat
+        if (draft.internalSelectedChat && draft.internalSelectedChat.id === draft.chatMessage.chatId) {
+          chatThread.isRead = true;
+        } else {
+          chatThread.isRead = false;
+        }
+
         bringToTop();
       } else if (event.type === 'chatMessageReceived' && event.message?.chatId) {
         draft.status = 'chat message received';
@@ -504,6 +559,11 @@ class StatefulGraphChatListClient implements StatefulClient<GraphChatListClient>
       draft.status = chatThreads.length > 0 ? 'chats loaded' : 'no chats';
       draft.chatThreads = chatThreads;
       draft.moreChatThreadsToLoad = nextLink !== undefined && nextLink !== '';
+      if (this._initialSelectedChatId) {
+        draft.internalSelectedChat = chatThreads.filter(c => c.id === this._initialSelectedChatId)[0];
+        this._initialSelectedChatId = undefined;
+        draft.initialSelectedChatSet = true;
+      }
     });
   };