Smarter incoming message handling for filter by unread (#9366)

Co-authored-by: yash-signal <yash@signal.org>

ts/state/ducks/conversations.ts

@@ -201,7 +201,6 @@ import { markCallHistoryReadInConversation } from './callHistory';
 import type { CapabilitiesType } from '../../textsecure/WebAPI';
 import { actions as searchActions } from './search';
 import type { SearchActionType } from './search';
-
 // State
 
 export type DBConversationType = ReadonlyDeep<{
@@ -2689,14 +2688,8 @@ function conversationsUpdated(
       calling.groupMembersChanged(conversation.id);
     }
 
-    const { conversationLookup } = getState().conversations;
-
-    const someConversationsHaveNewMessages = data.some(conversation => {
-      return (
-        conversationLookup[conversation.id]?.lastMessageReceivedAt !==
-        conversation.lastMessageReceivedAt
-      );
-    });
+    const { conversationLookup: oldConversationLookup } =
+      getState().conversations;
 
     dispatch({
       type: 'CONVERSATIONS_UPDATED',
@@ -2705,9 +2698,12 @@ function conversationsUpdated(
       },
     });
 
-    if (someConversationsHaveNewMessages) {
-      dispatch(searchActions.refreshSearch());
-    }
+    dispatch(
+      searchActions.updateSearchResultsOnConversationUpdate(
+        oldConversationLookup,
+        data
+      )
+    );
   };
 }
 
@@ -4627,6 +4623,8 @@ function onConversationClosed(
         conversationId,
       },
     });
+
+    dispatch(searchActions.maybeRemoveReadConversations([conversationId]));
   };
 }
 

ts/state/ducks/search.ts

@@ -23,6 +23,7 @@ import type {
   TargetedConversationChangedActionType,
   ShowArchivedConversationsActionType,
   MessageType,
+  ConversationLookupType,
 } from './conversations';
 import {
   getFilterByUnread,
@@ -45,6 +46,10 @@ import { removeDiacritics } from '../../util/removeDiacritics';
 import * as log from '../../logging/log';
 import { searchConversationTitles } from '../../util/searchConversationTitles';
 import { isDirectConversation } from '../../util/whatTypeOfConversation';
+import {
+  countConversationUnreadStats,
+  hasUnread,
+} from '../../util/countUnreadStats';
 
 const { searchMessages: dataSearchMessages } = DataReader;
 
@@ -135,6 +140,14 @@ type RefreshSearchActionType = ReadonlyDeep<{
   payload: null;
 }>;
 
+type MaybeRemoveReadConversationsActionType = ReadonlyDeep<{
+  type: 'MAYBE_REMOVE_READ_CONVERSATIONS';
+  payload: {
+    conversations: Array<ConversationType>;
+    selectedConversationId: string | undefined;
+  };
+}>;
+
 export type SearchActionType = ReadonlyDeep<
   | SearchMessagesResultsFulfilledActionType
   | SearchDiscussionsResultsFulfilledActionType
@@ -152,6 +165,7 @@ export type SearchActionType = ReadonlyDeep<
   | ConversationUnloadedActionType
   | UpdateFilterByUnreadActionType
   | RefreshSearchActionType
+  | MaybeRemoveReadConversationsActionType
 >;
 
 // Action Creators
@@ -166,6 +180,8 @@ export const actions = {
   updateSearchTerm,
   updateFilterByUnread,
   refreshSearch,
+  maybeRemoveReadConversations,
+  updateSearchResultsOnConversationUpdate,
 };
 
 export const useSearchActions = (): BoundActionCreatorsMapObject<
@@ -248,6 +264,98 @@ function refreshSearch(): ThunkAction<
   };
 }
 
+function updateSearchResultsOnConversationUpdate(
+  oldConversationLookup: ConversationLookupType,
+  updatedConversations: Array<ConversationType>
+): ThunkAction<
+  void,
+  RootStateType,
+  unknown,
+  MaybeRemoveReadConversationsActionType
+> {
+  return (dispatch, getState) => {
+    const state = getState();
+
+    if (!getIsActivelySearching(getState())) {
+      return;
+    }
+
+    const someConversationsHaveNewMessages = updatedConversations.some(
+      conversation => {
+        const oldConversation = oldConversationLookup[conversation.id];
+
+        return (
+          !oldConversation ||
+          oldConversation.lastMessageReceivedAt !==
+            conversation.lastMessageReceivedAt
+        );
+      }
+    );
+
+    if (someConversationsHaveNewMessages) {
+      dispatch(refreshSearch());
+      // A new search will automatically remove read conversations
+      return;
+    }
+
+    dispatch({
+      type: 'MAYBE_REMOVE_READ_CONVERSATIONS',
+      payload: {
+        conversations: updatedConversations,
+        selectedConversationId: state.conversations.selectedConversationId,
+      },
+    });
+  };
+}
+
+function shouldRemoveConversationFromUnreadList(
+  conversation: ConversationType,
+  selectedConversationId: string | undefined,
+  state: SearchStateType
+): boolean {
+  if (
+    state.filterByUnread &&
+    state.conversationIds.includes(conversation.id) &&
+    conversation &&
+    (selectedConversationId == null ||
+      selectedConversationId !== conversation.id) &&
+    !hasUnread(
+      countConversationUnreadStats(conversation, { includeMuted: true })
+    )
+  ) {
+    return true;
+  }
+
+  return false;
+}
+
+function maybeRemoveReadConversations(
+  conversationIds: Array<string>
+): ThunkAction<
+  void,
+  RootStateType,
+  unknown,
+  MaybeRemoveReadConversationsActionType
+> {
+  return (dispatch, getState) => {
+    const {
+      conversations: { selectedConversationId, conversationLookup },
+    } = getState();
+
+    const conversations = conversationIds
+      .map(id => conversationLookup[id])
+      .filter(isNotNil);
+
+    dispatch({
+      type: 'MAYBE_REMOVE_READ_CONVERSATIONS',
+      payload: {
+        conversations,
+        selectedConversationId,
+      },
+    });
+  };
+}
+
 function updateFilterByUnread(
   filterByUnread: boolean
 ): ThunkAction<void, RootStateType, unknown, UpdateFilterByUnreadActionType> {
@@ -308,6 +416,8 @@ const doSearch = debounce(
     const ourConversationId = getUserConversationId(state);
     const searchConversationId = getSearchConversation(state)?.id;
 
+    const { selectedConversationId } = state.conversations;
+
     strictAssert(ourConversationId, 'doSearch our conversation is missing');
 
     // Limit the number of contacts to something reasonable
@@ -354,6 +464,10 @@ const doSearch = debounce(
 
     if (!searchConversationId) {
       void (async () => {
+        const selectedConversation: ConversationType | undefined =
+          selectedConversationId
+            ? state.conversations.conversationLookup[selectedConversationId]
+            : undefined;
         const { conversationIds, contactIds } =
           await queryConversationsAndContacts(query, {
             filterByUnread,
@@ -361,6 +475,24 @@ const doSearch = debounce(
             noteToSelf,
             regionCode,
             allConversations,
+            /**
+             * If filter by unread is enabled, the selected conversation
+             * is read, and it's already in the list, we don't want to remove it
+             * from the list. It will be removed when the user switches to
+             * a different conversation.
+             */
+            conversationToInject:
+              filterByUnread &&
+              selectedConversationId &&
+              selectedConversation &&
+              state.search.conversationIds.includes(selectedConversationId) &&
+              !hasUnread(
+                countConversationUnreadStats(selectedConversation, {
+                  includeMuted: true,
+                })
+              )
+                ? selectedConversation
+                : undefined,
           });
 
         dispatch({
@@ -416,12 +548,14 @@ async function queryConversationsAndContacts(
     noteToSelf: string;
     regionCode: string | undefined;
     allConversations: ReadonlyArray<ConversationType>;
+    conversationToInject?: ConversationType;
   }
 ): Promise<{
   contactIds: Array<string>;
   conversationIds: Array<string>;
 }> {
   const {
+    conversationToInject,
     filterByUnread,
     ourConversationId,
     noteToSelf,
@@ -462,7 +596,8 @@ async function queryConversationsAndContacts(
     visibleConversations,
     normalizedQuery,
     regionCode,
-    filterByUnread
+    filterByUnread,
+    conversationToInject
   );
 
   // Split into two groups - active conversations and items just from address book
@@ -479,8 +614,11 @@ async function queryConversationsAndContacts(
     }
   }
 
-  // Inject synthetic Note to Self entry if query matches localized 'Note to Self'
-  if (noteToSelf.indexOf(query.toLowerCase()) !== -1) {
+  // If it's a query search and query matches part of localized "Note to Self",
+  // inject synthetic Note to Self only in the contacts list.
+  // If we're filtering by unread, no contacts are shown anyway, so we show it in the
+  // normal flow of the conversations list.
+  if (!filterByUnread && noteToSelf.indexOf(query.toLowerCase()) !== -1) {
     // ensure that we don't have duplicates in our results
     contactIds = contactIds.filter(id => id !== ourConversationId);
     conversationIds = conversationIds.filter(id => id !== ourConversationId);
@@ -551,6 +689,33 @@ export function reducer(
   state: Readonly<SearchStateType> = getEmptyState(),
   action: Readonly<SearchActionType>
 ): SearchStateType {
+  if (action.type === 'MAYBE_REMOVE_READ_CONVERSATIONS') {
+    if (!state.filterByUnread) {
+      return state;
+    }
+    const { conversations, selectedConversationId } = action.payload;
+
+    const conversationIdsToRemove = conversations
+      .filter(conversation =>
+        shouldRemoveConversationFromUnreadList(
+          conversation,
+          selectedConversationId,
+          state
+        )
+      )
+      .map(conversation => conversation.id);
+
+    if (conversationIdsToRemove.length === 0) {
+      return state;
+    }
+
+    return {
+      ...state,
+      conversationIds: state.conversationIds.filter(
+        id => !conversationIdsToRemove.includes(id)
+      ),
+    };
+  }
   if (action.type === 'FILTER_BY_UNREAD_UPDATE') {
     return handleSearchUpdate(state, {
       filterByUnread: action.payload.enabled,

ts/util/filterAndSortConversations.ts

@@ -165,12 +165,17 @@ export function filterAndSortConversations(
   conversations: ReadonlyArray<ConversationType>,
   searchTerm: string,
   regionCode: string | undefined,
-  filterByUnread: boolean = false
+  filterByUnread: boolean = false,
+  conversationToInject?: ConversationType
 ): Array<ConversationType> {
-  const filteredConversations = filterByUnread
+  let filteredConversations = filterByUnread
     ? filterConversationsByUnread(conversations, true)
     : conversations;
 
+  if (conversationToInject) {
+    filteredConversations = [...filteredConversations, conversationToInject];
+  }
+
   if (searchTerm.length) {
     const now = Date.now();
     const withoutUnknownAndFiltered = filteredConversations.filter(