Sort by recency then alphabetically everywhere

ts/components/AddUserToAnotherGroupModal.tsx

@@ -8,7 +8,7 @@ import type { ListRowProps } from 'react-virtualized';
 import type { ConversationType } from '../state/ducks/conversations';
 import type { LocalizerType } from '../types/Util';
 import { ToastType } from '../types/Toast';
-import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../util/filterAndSortConversations';
 import { ConfirmationDialog } from './ConfirmationDialog';
 import type { GroupListItemConversationType } from './conversationList/GroupListItem';
 import {
@@ -56,7 +56,7 @@ export function AddUserToAnotherGroupModal({
 }: Props): JSX.Element | null {
   const [searchTerm, setSearchTerm] = React.useState('');
   const [filteredConversations, setFilteredConversations] = React.useState(
-    filterAndSortConversationsByRecent(candidateConversations, '', undefined)
+    filterAndSortConversations(candidateConversations, '', undefined)
   );
 
   const [selectedGroupId, setSelectedGroupId] = React.useState<
@@ -78,7 +78,7 @@ export function AddUserToAnotherGroupModal({
   React.useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredConversations(
-        filterAndSortConversationsByRecent(
+        filterAndSortConversations(
           candidateConversations,
           normalizedSearchTerm,
           regionCode

ts/components/CallsNewCall.tsx

@@ -9,7 +9,7 @@ import { List } from 'react-virtualized';
 import type { ConversationType } from '../state/ducks/conversations';
 import type { LocalizerType } from '../types/I18N';
 import { SearchInput } from './SearchInput';
-import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../util/filterAndSortConversations';
 import { NavSidebarSearchHeader } from './NavSidebar';
 import { ListTile } from './ListTile';
 import { strictAssert } from '../util/assert';
@@ -89,11 +89,7 @@ export function CallsNewCall({
     if (query === '') {
       return activeConversations;
     }
-    return filterAndSortConversationsByRecent(
-      activeConversations,
-      query,
-      regionCode
-    );
+    return filterAndSortConversations(activeConversations, query, regionCode);
   }, [activeConversations, query, regionCode]);
 
   const [groupConversations, directConversations] = useMemo(() => {

ts/components/ForwardMessagesModal.tsx

@@ -23,7 +23,7 @@ import type { LocalizerType, ThemeType } from '../types/Util';
 import type { SmartCompositionTextAreaProps } from '../state/smart/CompositionTextArea';
 import { SearchInput } from './SearchInput';
 import { StagedLinkPreview } from './conversation/StagedLinkPreview';
-import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../util/filterAndSortConversations';
 import {
   shouldNeverBeCalled,
   asyncShouldNeverBeCalled,
@@ -96,7 +96,7 @@ export function ForwardMessagesModal({
   >([]);
   const [searchTerm, setSearchTerm] = useState('');
   const [filteredConversations, setFilteredConversations] = useState(
-    filterAndSortConversationsByRecent(candidateConversations, '', regionCode)
+    filterAndSortConversations(candidateConversations, '', regionCode)
   );
   const [isEditingMessage, setIsEditingMessage] = useState(false);
   const [cannotMessage, setCannotMessage] = useState(false);
@@ -169,7 +169,7 @@ export function ForwardMessagesModal({
   useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredConversations(
-        filterAndSortConversationsByRecent(
+        filterAndSortConversations(
           candidateConversations,
           normalizedSearchTerm,
           regionCode

ts/components/SendStoryModal.tsx

@@ -5,7 +5,7 @@ import React, { useEffect, useMemo, useState } from 'react';
 import { noop, sortBy } from 'lodash';
 
 import { SearchInput } from './SearchInput';
-import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../util/filterAndSortConversations';
 
 import type { ConversationType } from '../state/ducks/conversations';
 import type { ConversationWithStoriesType } from '../state/selectors/conversations';
@@ -175,11 +175,7 @@ export function SendStoryModal({
   const [searchTerm, setSearchTerm] = useState('');
 
   const [filteredConversations, setFilteredConversations] = useState(
-    filterAndSortConversationsByRecent(
-      groupConversations,
-      searchTerm,
-      undefined
-    )
+    filterAndSortConversations(groupConversations, searchTerm, undefined)
   );
 
   const normalizedSearchTerm = searchTerm.trim();
@@ -187,7 +183,7 @@ export function SendStoryModal({
   useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredConversations(
-        filterAndSortConversationsByRecent(
+        filterAndSortConversations(
           groupConversations,
           normalizedSearchTerm,
           undefined

ts/components/StoriesSettingsModal.tsx

@@ -27,7 +27,7 @@ import { MY_STORY_ID, getStoryDistributionListName } from '../types/Stories';
 import { PagedModal, ModalPage } from './Modal';
 import { SearchInput } from './SearchInput';
 import { StoryDistributionListName } from './StoryDistributionListName';
-import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../util/filterAndSortConversations';
 import { isNotNil } from '../util/isNotNil';
 import {
   shouldNeverBeCalled,
@@ -89,7 +89,7 @@ function filterConversations(
   conversations: ReadonlyArray<ConversationType>,
   searchTerm: string
 ) {
-  return filterAndSortConversationsByRecent(
+  return filterAndSortConversations(
     conversations,
     searchTerm,
     undefined

ts/components/conversation/conversation-details/AddGroupMembersModal/ChooseGroupMembersModal.tsx

@@ -19,7 +19,7 @@ import { missingCaseError } from '../../../../util/missingCaseError';
 import type { LookupConversationWithoutServiceIdActionsType } from '../../../../util/lookupConversationWithoutServiceId';
 import { parseAndFormatPhoneNumber } from '../../../../util/libphonenumberInstance';
 import type { ParsedE164Type } from '../../../../util/libphonenumberInstance';
-import { filterAndSortConversationsByRecent } from '../../../../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../../../../util/filterAndSortConversations';
 import type { ConversationType } from '../../../../state/ducks/conversations';
 import type {
   UUIDFetchStateKeyType,
@@ -140,13 +140,13 @@ export function ChooseGroupMembersModal({
   const canContinue = Boolean(selectedContacts.length);
 
   const [filteredContacts, setFilteredContacts] = useState(
-    filterAndSortConversationsByRecent(candidateContacts, '', regionCode)
+    filterAndSortConversations(candidateContacts, '', regionCode)
   );
   const normalizedSearchTerm = searchTerm.trim();
   useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredContacts(
-        filterAndSortConversationsByRecent(
+        filterAndSortConversations(
           candidateContacts,
           normalizedSearchTerm,
           regionCode

ts/state/ducks/search.ts

@@ -6,7 +6,7 @@ import { debounce, omit, reject } from 'lodash';
 
 import type { ReadonlyDeep } from 'type-fest';
 import type { StateType as RootStateType } from '../reducer';
-import { filterAndSortConversationsByRecent } from '../../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../../util/filterAndSortConversations';
 import type {
   ClientSearchResultMessageType,
   ClientInterface,
@@ -337,12 +337,11 @@ async function queryConversationsAndContacts(
     }
   );
 
-  const searchResults: Array<ConversationType> =
-    filterAndSortConversationsByRecent(
-      visibleConversations,
-      normalizedQuery,
-      regionCode
-    );
+  const searchResults: Array<ConversationType> = filterAndSortConversations(
+    visibleConversations,
+    normalizedQuery,
+    regionCode
+  );
 
   // Split into two groups - active conversations and items just from address book
   let conversationIds: Array<string> = [];

ts/state/selectors/conversations.ts

@@ -30,10 +30,7 @@ import { deconstructLookup } from '../../util/deconstructLookup';
 import type { PropsDataType as TimelinePropsType } from '../../components/conversation/Timeline';
 import { assertDev } from '../../util/assert';
 import { isConversationUnregistered } from '../../util/isConversationUnregistered';
-import {
-  filterAndSortConversationsAlphabetically,
-  filterAndSortConversationsByRecent,
-} from '../../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../../util/filterAndSortConversations';
 import type { ContactNameColorType } from '../../types/Colors';
 import { ContactNameColors } from '../../types/Colors';
 import type { AvatarDataType } from '../../types/Avatar';
@@ -724,11 +721,7 @@ export const getFilteredComposeContacts = createSelector(
     contacts: ReadonlyArray<ConversationType>,
     regionCode: string | undefined
   ): Array<ConversationType> => {
-    return filterAndSortConversationsAlphabetically(
-      contacts,
-      searchTerm,
-      regionCode
-    );
+    return filterAndSortConversations(contacts, searchTerm, regionCode);
   }
 );
 
@@ -750,18 +743,16 @@ export const getFilteredComposeGroups = createSelector(
       }>;
     }
   > => {
-    return filterAndSortConversationsAlphabetically(
-      groups,
-      searchTerm,
-      regionCode
-    ).map(group => ({
-      ...group,
-      // we don't disable groups when composing, already filtered
-      disabledReason: undefined,
-      // should always be populated for a group
-      membersCount: group.membersCount ?? 0,
-      memberships: group.memberships ?? [],
-    }));
+    return filterAndSortConversations(groups, searchTerm, regionCode).map(
+      group => ({
+        ...group,
+        // we don't disable groups when composing, already filtered
+        disabledReason: undefined,
+        // should always be populated for a group
+        membersCount: group.membersCount ?? 0,
+        memberships: group.memberships ?? [],
+      })
+    );
   }
 );
 
@@ -769,7 +760,7 @@ export const getFilteredCandidateContactsForNewGroup = createSelector(
   getCandidateContactsForNewGroup,
   getNormalizedComposerConversationSearchTerm,
   getRegionCode,
-  filterAndSortConversationsByRecent
+  filterAndSortConversations
 );
 
 const getGroupCreationComposerState = createSelector(

ts/state/smart/CallsTab.tsx

@@ -14,7 +14,7 @@ import {
   getAllConversations,
   getConversationSelector,
 } from '../selectors/conversations';
-import { filterAndSortConversationsByRecent } from '../../util/filterAndSortConversations';
+import { filterAndSortConversations } from '../../util/filterAndSortConversations';
 import type {
   CallHistoryFilter,
   CallHistoryFilterOptions,
@@ -44,7 +44,7 @@ function getCallHistoryFilter(
       return conversation.removalStage == null;
     });
 
-    const filteredConversations = filterAndSortConversationsByRecent(
+    const filteredConversations = filterAndSortConversations(
       currentConversations,
       query,
       regionCode

ts/test-both/util/filterAndSortConversations_test.ts

@@ -2,90 +2,138 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import { assert } from 'chai';
+import { pick } from 'lodash';
 import { getDefaultConversation } from '../helpers/getDefaultConversation';
+import { filterAndSortConversations } from '../../util/filterAndSortConversations';
+import type { ConversationType } from '../../state/ducks/conversations';
 
-import {
-  filterAndSortConversationsAlphabetically,
-  filterAndSortConversationsByRecent,
-} from '../../util/filterAndSortConversations';
+type CheckProps = Pick<ConversationType, 'title' | 'activeAt' | 'e164'>;
+
+function check({
+  searchTerm,
+  input,
+  expected,
+}: {
+  searchTerm: string;
+  input: Array<CheckProps>;
+  expected: Array<CheckProps>;
+}) {
+  const conversations = input.map(props => {
+    return getDefaultConversation(props);
+  });
+  const results = filterAndSortConversations(conversations, searchTerm, 'US');
+  const actual = results.map(convo => {
+    return pick(convo, 'title', 'activeAt');
+  });
+  assert.sameDeepMembers(actual, expected);
+}
 
 describe('filterAndSortConversations', () => {
-  const conversations = [
-    getDefaultConversation({
-      title: '+16505551234',
-      activeAt: 1,
-    }),
-    getDefaultConversation({
-      title: 'The Abraham Lincoln Club',
-      activeAt: 4,
-    }),
-    getDefaultConversation({
-      title: 'Boxing Club',
-      activeAt: 3,
-    }),
-    getDefaultConversation({
-      title: 'Not recent',
-    }),
-    getDefaultConversation({
-      title: 'George Washington',
-      e164: '+16505559876',
-      activeAt: 2,
-    }),
-    getDefaultConversation({
-      title: 'A long long long title ending with burrito',
-    }),
-  ];
-
-  it('filterAndSortConversationsByRecent sorts by recency when no search term is provided', () => {
-    const titles = filterAndSortConversationsByRecent(
-      conversations,
-      '',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameOrderedMembers(titles, [
-      'The Abraham Lincoln Club',
-      'Boxing Club',
-      'George Washington',
-      '+16505551234',
-      'Not recent',
-      'A long long long title ending with burrito',
-    ]);
-  });
-
-  it('filterAndSortConversationsAlphabetically sorts by title when no search term is provided', () => {
-    const titles = filterAndSortConversationsAlphabetically(
-      conversations,
-      '',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameOrderedMembers(titles, [
-      'A long long long title ending with burrito',
-      'Boxing Club',
-      'George Washington',
-      'Not recent',
-      'The Abraham Lincoln Club',
-      '+16505551234',
-    ]);
-  });
-
-  it('filterAndSortConversationsAlphabetically sorts by title when a search term is provided', () => {
-    const titles = filterAndSortConversationsAlphabetically(
-      conversations,
-      'club',
-      'US'
-    ).map(contact => contact.title);
-    assert.sameOrderedMembers(titles, [
-      'Boxing Club',
-      'The Abraham Lincoln Club',
-    ]);
+  it('finds a conversation by title', () => {
+    check({
+      searchTerm: 'yes',
+      input: [{ title: 'no' }, { title: 'yes' }, { title: 'no' }],
+      expected: [{ title: 'yes' }],
+    });
   });
 
   it('finds a conversation when the search term is at the end of a long title', () => {
-    const titles = filterAndSortConversationsByRecent(
-      conversations,
-      'burrito',
-      'US'
-    ).map(convo => convo.title);
-    assert.deepEqual(titles, ['A long long long title ending with burrito']);
+    check({
+      searchTerm: 'burrito',
+      input: [
+        { title: 'no' },
+        {
+          title: 'A long long long title ending with burrito',
+        },
+        { title: 'no' },
+      ],
+      expected: [
+        {
+          title: 'A long long long title ending with burrito',
+        },
+      ],
+    });
+  });
+
+  it('finds a conversation by phone number', () => {
+    check({
+      searchTerm: '9876',
+      input: [
+        { title: 'no' },
+        { title: 'yes', e164: '+16505559876' },
+        { title: 'no' },
+      ],
+      expected: [{ title: 'yes' }],
+    });
+  });
+
+  describe('no search term', () => {
+    it('sorts by recency first', () => {
+      check({
+        searchTerm: '',
+        input: [
+          { title: 'B', activeAt: 2 },
+          { title: 'A', activeAt: 1 },
+          { title: 'C', activeAt: 3 },
+        ],
+        expected: [
+          { title: 'C', activeAt: 3 },
+          { title: 'B', activeAt: 2 },
+          { title: 'A', activeAt: 1 },
+        ],
+      });
+    });
+
+    it('falls back to alphabetically', () => {
+      check({
+        searchTerm: '',
+        input: [
+          { title: 'B', activeAt: 2 },
+          { title: 'A', activeAt: 2 },
+          { title: 'C', activeAt: 3 },
+        ],
+        expected: [
+          { title: 'C', activeAt: 3 },
+          { title: 'A', activeAt: 2 },
+          { title: 'B', activeAt: 2 },
+        ],
+      });
+    });
+  });
+
+  describe('with search term', () => {
+    it('sorts by recency first', () => {
+      check({
+        searchTerm: 'yes',
+        input: [
+          { title: 'no' },
+          { title: 'yes B', activeAt: 2 },
+          { title: 'yes A', activeAt: 1 },
+          { title: 'yes C', activeAt: 3 },
+        ],
+        expected: [
+          { title: 'yes C', activeAt: 3 },
+          { title: 'yes B', activeAt: 2 },
+          { title: 'yes A', activeAt: 1 },
+        ],
+      });
+    });
+
+    it('falls back to alphabetically', () => {
+      check({
+        searchTerm: 'yes',
+        input: [
+          { title: 'no' },
+          { title: 'yes B', activeAt: 2 },
+          { title: 'yes A', activeAt: 2 },
+          { title: 'yes C', activeAt: 3 },
+        ],
+        expected: [
+          { title: 'yes C', activeAt: 3 },
+          { title: 'yes A', activeAt: 2 },
+          { title: 'yes B', activeAt: 2 },
+        ],
+      });
+    });
   });
 });

ts/util/filterAndSortConversations.ts

@@ -2,7 +2,6 @@
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import type Fuse from 'fuse.js';
-
 import type { ConversationType } from '../state/ducks/conversations';
 import { parseAndFormatPhoneNumber } from './libphonenumberInstance';
 import { WEEK } from './durations';
@@ -129,7 +128,27 @@ function searchConversations(
   return index.search(extendedSearchTerm);
 }
 
-export function filterAndSortConversationsByRecent(
+function startsWithLetter(title: string) {
+  // Uses \p, the unicode character class escape, to check if a the first character is a
+  // letter
+  return /^\p{Letter}/u.test(title);
+}
+
+function sortAlphabetically(a: ConversationType, b: ConversationType) {
+  // Sort alphabetically with conversations starting with a letter first (and phone
+  // numbers last)
+  const aStartsWithLetter = startsWithLetter(a.title);
+  const bStartsWithLetter = startsWithLetter(b.title);
+  if (aStartsWithLetter && !bStartsWithLetter) {
+    return -1;
+  }
+  if (!aStartsWithLetter && bStartsWithLetter) {
+    return 1;
+  }
+  return a.title.localeCompare(b.title);
+}
+
+export function filterAndSortConversations(
   conversations: ReadonlyArray<ConversationType>,
   searchTerm: string,
   regionCode: string | undefined
@@ -156,53 +175,22 @@ export function filterAndSortConversationsByRecent(
           (b.score ?? 0) +
           (bLeft ? LEFT_GROUP_PENALTY : 0);
 
-        return aScore - bScore;
+        const activeScore = aScore - bScore;
+        if (activeScore !== 0) {
+          return activeScore;
+        }
+        return sortAlphabetically(a.item, b.item);
       })
       .map(result => result.item);
   }
 
   return conversations.concat().sort((a, b) => {
-    if (a.activeAt && b.activeAt) {
-      return a.activeAt > b.activeAt ? -1 : 1;
+    const aScore = a.activeAt ?? 0;
+    const bScore = b.activeAt ?? 0;
+    const score = bScore - aScore;
+    if (score !== 0) {
+      return score;
     }
-
-    return a.activeAt && !b.activeAt ? -1 : 1;
+    return sortAlphabetically(a, b);
   });
 }
-
-function startsWithLetter(title: string) {
-  // Uses \p, the unicode character class escape, to check if a the first character is a
-  // letter
-  return /^\p{Letter}/u.test(title);
-}
-
-function sortAlphabetically(a: ConversationType, b: ConversationType) {
-  // Sort alphabetically with conversations starting with a letter first (and phone
-  // numbers last)
-  const aStartsWithLetter = startsWithLetter(a.title);
-  const bStartsWithLetter = startsWithLetter(b.title);
-  if (aStartsWithLetter && !bStartsWithLetter) {
-    return -1;
-  }
-  if (!aStartsWithLetter && bStartsWithLetter) {
-    return 1;
-  }
-  return a.title.localeCompare(b.title);
-}
-
-export function filterAndSortConversationsAlphabetically(
-  conversations: ReadonlyArray<ConversationType>,
-  searchTerm: string,
-  regionCode: string | undefined
-): Array<ConversationType> {
-  if (searchTerm.length) {
-    const withoutUnknown = conversations.filter(item => item.titleNoDefault);
-
-    return searchConversations(withoutUnknown, searchTerm, regionCode)
-      .slice()
-      .map(result => result.item)
-      .sort(sortAlphabetically);
-  }
-
-  return conversations.concat().sort(sortAlphabetically);
-}