Various fixes for message forwarding

ts/components/ForwardMessageModal.tsx

@@ -27,7 +27,7 @@ import { BodyRangeType, LocalizerType } from '../types/Util';
 import { ModalHost } from './ModalHost';
 import { StagedLinkPreview } from './conversation/StagedLinkPreview';
 import { assert } from '../util/assert';
-import { filterAndSortConversations } from '../util/filterAndSortConversations';
+import { filterAndSortConversationsByRecent } from '../util/filterAndSortConversations';
 
 export type DataPropsType = {
   attachments?: Array<AttachmentType>;
@@ -85,8 +85,8 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
     Array<ConversationType>
   >([]);
   const [searchTerm, setSearchTerm] = useState('');
-  const [filteredContacts, setFilteredContacts] = useState(
-    filterAndSortConversations(candidateConversations, '')
+  const [filteredConversations, setFilteredConversations] = useState(
+    filterAndSortConversationsByRecent(candidateConversations, '')
   );
   const [attachmentsToForward, setAttachmentsToForward] = useState(attachments);
   const [isEditingMessage, setIsEditingMessage] = useState(false);
@@ -148,14 +148,17 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
   const normalizedSearchTerm = searchTerm.trim();
   useEffect(() => {
     const timeout = setTimeout(() => {
-      setFilteredContacts(
-        filterAndSortConversations(candidateConversations, normalizedSearchTerm)
+      setFilteredConversations(
+        filterAndSortConversationsByRecent(
+          candidateConversations,
+          normalizedSearchTerm
+        )
       );
     }, 200);
     return () => {
       clearTimeout(timeout);
     };
-  }, [candidateConversations, normalizedSearchTerm, setFilteredContacts]);
+  }, [candidateConversations, normalizedSearchTerm, setFilteredConversations]);
 
   const contactLookup = useMemo(() => {
     const map = new Map();
@@ -165,7 +168,7 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
     return map;
   }, [candidateConversations]);
 
-  const toggleSelectedContact = useCallback(
+  const toggleSelectedConversation = useCallback(
     (conversationId: string) => {
       let removeContact = false;
       const nextSelectedContacts = selectedContacts.filter(contact => {
@@ -187,9 +190,17 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
     [contactLookup, selectedContacts, setSelectedContacts]
   );
 
-  const rowCount = filteredContacts.length;
+  const handleBackOrClose = useCallback(() => {
+    if (isEditingMessage) {
+      setIsEditingMessage(false);
+    } else {
+      onClose();
+    }
+  }, [isEditingMessage, onClose, setIsEditingMessage]);
+
+  const rowCount = filteredConversations.length;
   const getRow = (index: number): undefined | Row => {
-    const contact = filteredContacts[index];
+    const contact = filteredConversations[index];
     if (!contact) {
       return undefined;
     }
@@ -209,8 +220,18 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
     };
   };
 
+  useEffect(() => {
+    const timeout = setTimeout(() => {
+      inputRef.current?.focus();
+    }, 100);
+
+    return () => {
+      clearTimeout(timeout);
+    };
+  }, []);
+
   return (
-    <ModalHost onClose={onClose}>
+    <ModalHost onEscape={handleBackOrClose} onClose={onClose}>
       <div className="module-ForwardMessageModal">
         <div
           className={classNames('module-ForwardMessageModal__header', {
@@ -328,11 +349,6 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
                     <div
                       className="module-ForwardMessageModal__list-wrapper"
                       ref={measureRef}
-                      onKeyDown={event => {
-                        if (event.key === 'Enter') {
-                          inputRef.current?.focus();
-                        }
-                      }}
                     >
                       <ConversationList
                         dimensions={contentRect.bounds}
@@ -349,7 +365,7 @@ export const ForwardMessageModal: FunctionComponent<PropsType> = ({
                             disabledReason !==
                             ContactCheckboxDisabledReason.MaximumContactsSelected
                           ) {
-                            toggleSelectedContact(conversationId);
+                            toggleSelectedConversation(conversationId);
                           }
                         }}
                         onSelectConversation={shouldNeverBeCalled}

ts/components/ModalHost.tsx

@@ -7,13 +7,14 @@ import { createPortal } from 'react-dom';
 import { Theme, themeClassName } from '../util/theme';
 
 export type PropsType = {
+  readonly onEscape?: () => unknown;
   readonly onClose: () => unknown;
   readonly children: React.ReactElement;
   readonly theme?: Theme;
 };
 
 export const ModalHost = React.memo(
-  ({ onClose, children, theme }: PropsType) => {
+  ({ onEscape, onClose, children, theme }: PropsType) => {
     const [root, setRoot] = React.useState<HTMLElement | null>(null);
 
     useEffect(() => {
@@ -30,7 +31,11 @@ export const ModalHost = React.memo(
     useEffect(() => {
       const handler = (event: KeyboardEvent) => {
         if (event.key === 'Escape') {
-          onClose();
+          if (onEscape) {
+            onEscape();
+          } else {
+            onClose();
+          }
 
           event.preventDefault();
           event.stopPropagation();
@@ -41,7 +46,7 @@ export const ModalHost = React.memo(
       return () => {
         document.removeEventListener('keydown', handler);
       };
-    }, [onClose]);
+    }, [onEscape, onClose]);
 
     // This makes it easier to write dialogs to be hosted here; they won't have to worry
     //   as much about preventing propagation of mouse events.

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

@@ -14,7 +14,7 @@ import { LocalizerType } from '../../../../types/Util';
 import { assert } from '../../../../util/assert';
 import { getOwn } from '../../../../util/getOwn';
 import { missingCaseError } from '../../../../util/missingCaseError';
-import { filterAndSortConversations } from '../../../../util/filterAndSortConversations';
+import { filterAndSortConversationsByTitle } from '../../../../util/filterAndSortConversations';
 import { ConversationType } from '../../../../state/ducks/conversations';
 import { ModalHost } from '../../../ModalHost';
 import { ContactPills } from '../../../ContactPills';
@@ -72,13 +72,16 @@ export const ChooseGroupMembersModal: FunctionComponent<PropsType> = ({
   const canContinue = Boolean(selectedContacts.length);
 
   const [filteredContacts, setFilteredContacts] = useState(
-    filterAndSortConversations(candidateContacts, '')
+    filterAndSortConversationsByTitle(candidateContacts, '')
   );
   const normalizedSearchTerm = searchTerm.trim();
   useEffect(() => {
     const timeout = setTimeout(() => {
       setFilteredContacts(
-        filterAndSortConversations(candidateContacts, normalizedSearchTerm)
+        filterAndSortConversationsByTitle(
+          candidateContacts,
+          normalizedSearchTerm
+        )
       );
     }, 200);
     return () => {

ts/models/conversations.ts

@@ -3324,6 +3324,10 @@ export class ConversationModel extends window.Backbone.Model<
     mentions?: BodyRangesType,
     { dontClearDraft = false } = {}
   ): void {
+    if (this.isGroupV1AndDisabled()) {
+      return;
+    }
+
     this.clearTypingTimers();
 
     const { clearUnreadMetrics } = window.reduxActions.conversations;

ts/state/selectors/conversations.ts

@@ -27,7 +27,7 @@ import { PropsDataType as TimelinePropsType } from '../../components/conversatio
 import { TimelineItemType } from '../../components/conversation/TimelineItem';
 import { assert } from '../../util/assert';
 import { isConversationUnregistered } from '../../util/isConversationUnregistered';
-import { filterAndSortConversations } from '../../util/filterAndSortConversations';
+import { filterAndSortConversationsByTitle } from '../../util/filterAndSortConversations';
 
 import {
   getInteractionMode,
@@ -353,10 +353,13 @@ export const getAllComposableConversations = createSelector(
   getConversationLookup,
   (conversationLookup: ConversationLookupType): Array<ConversationType> =>
     Object.values(conversationLookup).filter(
-      contact =>
-        !contact.isBlocked &&
-        !isConversationUnregistered(contact) &&
-        (isString(contact.name) || contact.profileSharing)
+      conversation =>
+        !conversation.isBlocked &&
+        !conversation.isGroupV1AndDisabled &&
+        !isConversationUnregistered(conversation) &&
+        // All conversation should have a title except in weird cases where
+        // they don't, in that case we don't want to show these for Forwarding.
+        conversation.title
     )
 );
 
@@ -410,7 +413,7 @@ export const getComposeContacts = createSelector(
     searchTerm: string,
     contacts: Array<ConversationType>
   ): Array<ConversationType> => {
-    return filterAndSortConversations(contacts, searchTerm);
+    return filterAndSortConversationsByTitle(contacts, searchTerm);
   }
 );
 
@@ -421,14 +424,14 @@ export const getComposeGroups = createSelector(
     searchTerm: string,
     groups: Array<ConversationType>
   ): Array<ConversationType> => {
-    return filterAndSortConversations(groups, searchTerm);
+    return filterAndSortConversationsByTitle(groups, searchTerm);
   }
 );
 
 export const getCandidateContactsForNewGroup = createSelector(
   getComposableContacts,
   getNormalizedComposerConversationSearchTerm,
-  filterAndSortConversations
+  filterAndSortConversationsByTitle
 );
 
 export const getCantAddContactForModal = createSelector(

ts/test-both/state/selectors/conversations_test.ts

@@ -13,6 +13,7 @@ import {
 import {
   _getConversationComparator,
   _getLeftPaneLists,
+  getAllComposableConversations,
   getCandidateContactsForNewGroup,
   getCantAddContactForModal,
   getComposeContacts,
@@ -450,6 +451,85 @@ describe('both/state/selectors/conversations', () => {
     });
   });
 
+  describe('#getAllComposableConversations', () => {
+    const getRootState = (): StateType => {
+      const rootState = getEmptyRootState();
+      return {
+        ...rootState,
+        conversations: {
+          ...getEmptyState(),
+          conversationLookup: {
+            'our-conversation-id': {
+              ...getDefaultConversation('our-conversation-id'),
+              isMe: true,
+            },
+          },
+        },
+        user: {
+          ...rootState.user,
+          ourConversationId: 'our-conversation-id',
+          i18n,
+        },
+      };
+    };
+
+    const getRootStateWithConversations = (): StateType => {
+      const result = getRootState();
+      Object.assign(result.conversations.conversationLookup, {
+        'convo-1': {
+          ...getDefaultConversation('convo-1'),
+          title: 'A',
+        },
+        'convo-2': {
+          ...getDefaultConversation('convo-2'),
+          type: 'group',
+          isGroupV1AndDisabled: true,
+          title: 'Should Be Dropped (GV1)',
+        },
+        'convo-3': {
+          ...getDefaultConversation('convo-3'),
+          type: 'group',
+          title: 'B',
+        },
+        'convo-4': {
+          ...getDefaultConversation('convo-4'),
+          isBlocked: true,
+          title: 'Should Be Dropped (blocked)',
+        },
+        'convo-5': {
+          ...getDefaultConversation('convo-5'),
+          discoveredUnregisteredAt: new Date(1999, 3, 20).getTime(),
+          title: 'C',
+        },
+        'convo-6': {
+          ...getDefaultConversation('convo-6'),
+          profileSharing: true,
+          name: 'Should Be Droped (no title)',
+          title: null,
+        },
+        'convo-7': {
+          ...getDefaultConversation('convo-7'),
+          discoveredUnregisteredAt: Date.now(),
+          title: 'Should Be Dropped (unregistered)',
+        },
+      });
+      return result;
+    };
+
+    it('returns no gv1, no blocked, no missing titles', () => {
+      const state = getRootStateWithConversations();
+      const result = getAllComposableConversations(state);
+
+      const ids = result.map(contact => contact.id);
+      assert.deepEqual(ids, [
+        'our-conversation-id',
+        'convo-1',
+        'convo-3',
+        'convo-5',
+      ]);
+    });
+  });
+
   describe('#getComposeContacts', () => {
     const getRootState = (searchTerm = ''): StateType => {
       const rootState = getEmptyRootState();
@@ -558,7 +638,7 @@ describe('both/state/selectors/conversations', () => {
       assert.deepEqual(ids, ['convo-0']);
     });
 
-    it('returns not to self when searching for your own name', () => {
+    it('returns note to self when searching for your own name', () => {
       const state = getRootStateWithConversations('Myself');
       const result = getComposeContacts(state);
 

ts/test-both/util/filterAndSortConversations_test.ts

@@ -4,9 +4,12 @@
 import { assert } from 'chai';
 import { getDefaultConversation } from '../helpers/getDefaultConversation';
 
-import { filterAndSortConversations } from '../../util/filterAndSortConversations';
+import {
+  filterAndSortConversationsByTitle,
+  filterAndSortConversationsByRecent,
+} from '../../util/filterAndSortConversations';
 
-describe('filterAndSortConversations', () => {
+describe('filterAndSortConversationsByTitle', () => {
   const conversations = [
     getDefaultConversation({
       title: '+16505551234',
@@ -34,7 +37,7 @@ describe('filterAndSortConversations', () => {
   ];
 
   it('without a search term, sorts conversations by title (but puts no-name contacts at the bottom)', () => {
-    const titles = filterAndSortConversations(conversations, '').map(
+    const titles = filterAndSortConversationsByTitle(conversations, '').map(
       contact => contact.title
     );
     assert.deepEqual(titles, [
@@ -47,16 +50,56 @@ describe('filterAndSortConversations', () => {
   });
 
   it('can search for contacts by title', () => {
-    const titles = filterAndSortConversations(conversations, 'belind').map(
-      contact => contact.title
-    );
+    const titles = filterAndSortConversationsByTitle(
+      conversations,
+      'belind'
+    ).map(contact => contact.title);
     assert.sameMembers(titles, ['Belinda Beetle', 'Belinda Zephyr']);
   });
 
   it('can search for contacts by phone number (and puts no-name contacts at the bottom)', () => {
-    const titles = filterAndSortConversations(conversations, '650555').map(
-      contact => contact.title
-    );
+    const titles = filterAndSortConversationsByTitle(
+      conversations,
+      '650555'
+    ).map(contact => contact.title);
     assert.sameMembers(titles, ['Carlos Santana', '+16505551234']);
   });
 });
+
+describe('filterAndSortConversationsByRecent', () => {
+  const conversations = [
+    getDefaultConversation({
+      title: '+16505551234',
+      activeAt: 1,
+    }),
+    getDefaultConversation({
+      title: 'Abraham Lincoln',
+      activeAt: 4,
+    }),
+    getDefaultConversation({
+      title: 'Boxing Club',
+      activeAt: 3,
+    }),
+    getDefaultConversation({
+      title: 'Not recent',
+    }),
+    getDefaultConversation({
+      title: 'George Washington',
+      e164: '+16505559876',
+      activeAt: 2,
+    }),
+  ];
+
+  it('sorts by recency when no search term is provided', () => {
+    const titles = filterAndSortConversationsByRecent(conversations, '').map(
+      contact => contact.title
+    );
+    assert.sameMembers(titles, [
+      '+16505551234',
+      'George Washington',
+      'Boxing Club',
+      'Abraham Lincoln',
+      'Not recent',
+    ]);
+  });
+});

ts/util/filterAndSortConversations.ts

@@ -32,14 +32,38 @@ const FUSE_OPTIONS: FuseOptions<ConversationType> = {
 
 const collator = new Intl.Collator();
 
-export function filterAndSortConversations(
+function searchConversations(
+  conversations: ReadonlyArray<ConversationType>,
+  searchTerm: string
+): Array<ConversationType> {
+  return new Fuse<ConversationType>(conversations, FUSE_OPTIONS).search(
+    searchTerm
+  );
+}
+
+export function filterAndSortConversationsByRecent(
   conversations: ReadonlyArray<ConversationType>,
   searchTerm: string
 ): Array<ConversationType> {
   if (searchTerm.length) {
-    return new Fuse<ConversationType>(conversations, FUSE_OPTIONS).search(
-      searchTerm
-    );
+    return searchConversations(conversations, searchTerm);
+  }
+
+  return conversations.concat().sort((a, b) => {
+    if (a.activeAt && b.activeAt) {
+      return a.activeAt > b.activeAt ? -1 : 1;
+    }
+
+    return a.activeAt && !b.activeAt ? -1 : 1;
+  });
+}
+
+export function filterAndSortConversationsByTitle(
+  conversations: ReadonlyArray<ConversationType>,
+  searchTerm: string
+): Array<ConversationType> {
+  if (searchTerm.length) {
+    return searchConversations(conversations, searchTerm);
   }
 
   return conversations.concat().sort((a, b) => {