From 795edf198787c7b9fcab48ef1fa6b3abe29264d6 Mon Sep 17 00:00:00 2001 From: automated-signal <37887102+automated-signal@users.noreply.github.com> Date: Fri, 22 Nov 2024 17:40:13 -0600 Subject: [PATCH] Smarter incoming message handling for filter by unread (#9366) Co-authored-by: yash-signal --- ts/state/ducks/conversations.ts | 22 ++-- ts/state/ducks/search.ts | 171 +++++++++++++++++++++++++- ts/util/filterAndSortConversations.ts | 9 +- 3 files changed, 185 insertions(+), 17 deletions(-) diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index da36f07ff0dd..d08220add37c 100644 --- a/ts/state/ducks/conversations.ts +++ b/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])); }; } diff --git a/ts/state/ducks/search.ts b/ts/state/ducks/search.ts index f96c734d0f49..2c7d1566e6e3 100644 --- a/ts/state/ducks/search.ts +++ b/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; + 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 +): 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 +): 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 { @@ -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; + conversationToInject?: ConversationType; } ): Promise<{ contactIds: Array; conversationIds: Array; }> { 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 = getEmptyState(), action: Readonly ): 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, diff --git a/ts/util/filterAndSortConversations.ts b/ts/util/filterAndSortConversations.ts index fe69dcdfd614..e5f20a4954df 100644 --- a/ts/util/filterAndSortConversations.ts +++ b/ts/util/filterAndSortConversations.ts @@ -165,12 +165,17 @@ export function filterAndSortConversations( conversations: ReadonlyArray, searchTerm: string, regionCode: string | undefined, - filterByUnread: boolean = false + filterByUnread: boolean = false, + conversationToInject?: ConversationType ): Array { - 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(