fixes and tests

ts/components/ConversationListItem.tsx

@@ -155,7 +155,7 @@ export interface ConversationListItemHeaderProps {
   unreadCount: number;
   mentionedUs: boolean;
   activeAt?: number;
-  isPinned?: boolean;
+  isPinned: boolean;
 
   name?: string;
   phoneNumber: string;

ts/components/session/LeftPaneMessageSection.tsx

@@ -77,17 +77,12 @@ export class LeftPaneMessageSection extends React.Component<Props, State> {
   }
 
   public renderRow = ({ index, key, style }: RowRendererParamsType): JSX.Element => {
-    const { openConversationExternal } = this.props;
-    let { conversations } = this.props;
+    const { conversations, openConversationExternal } = this.props;
 
     if (!conversations) {
       throw new Error('renderRow: Tried to render without conversations');
     }
 
-    conversations = _.sortBy([...conversations], convo => {
-      return convo.isPinned ? -1 : 1;
-    });
-
     const conversation = conversations[index];
 
     return (

ts/components/session/menu/Menu.tsx

@@ -135,7 +135,7 @@ export const MenuItemPinConversation = (
 ): JSX.Element | null => {
   const { conversationId } = props;
   const conversation = getConversationController().get(conversationId);
-  const isPinned = conversation.getIsPinned();
+  const isPinned = conversation.isPinned();
   const nbOfAlreadyPinnedConvos = useSelector(getNumberOfPinnedConversations);
 
   const togglePinConversation = async () => {

ts/models/conversation.ts

@@ -91,7 +91,7 @@ export interface ConversationAttributes {
   accessKey?: any;
   triggerNotificationsFor: ConversationNotificationSettingType;
   isTrustedForAttachmentDownload: boolean;
-  isPinned?: boolean;
+  isPinned: boolean;
 }
 
 export interface ConversationAttributesOptionals {
@@ -129,7 +129,7 @@ export interface ConversationAttributesOptionals {
   accessKey?: any;
   triggerNotificationsFor?: ConversationNotificationSettingType;
   isTrustedForAttachmentDownload?: boolean;
-  isPinned?: boolean;
+  isPinned: boolean;
 }
 
 /**
@@ -407,7 +407,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
       left: !!this.get('left'),
       groupAdmins,
       members,
-      isPinned: this.getIsPinned(),
+      isPinned: this.isPinned(),
     };
   }
 
@@ -1234,7 +1234,7 @@ export class ConversationModel extends Backbone.Model<ConversationAttributes> {
     return this.get('name') || window.i18n('unknown');
   }
 
-  public getIsPinned() {
+  public isPinned() {
     return this.get('isPinned');
   }
 

ts/state/ducks/conversations.ts

@@ -82,7 +82,7 @@ export interface ConversationType {
   avatarPath?: string; // absolute filepath to the avatar
   groupAdmins?: Array<string>; // admins for closed groups and moderators for open groups
   members?: Array<string>; // members for closed groups only
-  isPinned?: boolean;
+  isPinned: boolean;
 }
 
 export type ConversationLookupType = {

ts/state/selectors/conversations.ts

@@ -64,6 +64,14 @@ const collator = new Intl.Collator();
 
 export const _getConversationComparator = (testingi18n?: LocalizerType) => {
   return (left: ConversationType, right: ConversationType): number => {
+    // Pin is the first criteria to check
+    if (left.isPinned && !right.isPinned) {
+      return -1;
+    }
+    if (!left.isPinned && right.isPinned) {
+      return 1;
+    }
+    // Then if none is pinned, check other criteria
     const leftActiveAt = left.activeAt;
     const rightActiveAt = right.activeAt;
     if (leftActiveAt && !rightActiveAt) {

ts/test/session/unit/selectors/conversations_test.ts

@@ -0,0 +1,206 @@
+import { assert } from 'chai';
+import { ConversationTypeEnum } from '../../../../models/conversation';
+
+import { ConversationLookupType } from '../../../../state/ducks/conversations';
+import {
+  _getConversationComparator,
+  _getLeftPaneLists,
+} from '../../../../state/selectors/conversations';
+
+describe('state/selectors/conversations', () => {
+  describe('#getLeftPaneList', () => {
+    it('sorts conversations based on timestamp then by intl-friendly title', () => {
+      const i18n = (key: string) => key;
+      const data: ConversationLookupType = {
+        id1: {
+          id: 'id1',
+          activeAt: 0,
+          name: 'No timestamp',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id2: {
+          id: 'id2',
+          activeAt: 20,
+          name: 'B',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id3: {
+          id: 'id3',
+          activeAt: 20,
+          name: 'C',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id4: {
+          id: 'id4',
+          activeAt: 20,
+          name: 'Á',
+          phoneNumber: 'notused',
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id5: {
+          id: 'id5',
+          activeAt: 30,
+          name: 'First!',
+          phoneNumber: 'notused',
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+      };
+      const comparator = _getConversationComparator(i18n);
+      const { conversations } = _getLeftPaneLists(data, comparator);
+
+      assert.strictEqual(conversations[0].name, 'First!');
+      assert.strictEqual(conversations[1].name, 'Á');
+      assert.strictEqual(conversations[2].name, 'B');
+      assert.strictEqual(conversations[3].name, 'C');
+    });
+  });
+
+  describe('#getLeftPaneListWithPinned', () => {
+    it('sorts conversations based on pin, timestamp then by intl-friendly title', () => {
+      const i18n = (key: string) => key;
+      const data: ConversationLookupType = {
+        id1: {
+          id: 'id1',
+          activeAt: 0,
+          name: 'No timestamp',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id2: {
+          id: 'id2',
+          activeAt: 20,
+          name: 'B',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+        id3: {
+          id: 'id3',
+          activeAt: 20,
+          name: 'C',
+          phoneNumber: 'notused',
+
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: true,
+        },
+        id4: {
+          id: 'id4',
+          activeAt: 20,
+          name: 'Á',
+          phoneNumber: 'notused',
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: true,
+        },
+        id5: {
+          id: 'id5',
+          activeAt: 30,
+          name: 'First!',
+          phoneNumber: 'notused',
+          type: ConversationTypeEnum.PRIVATE,
+          isMe: false,
+          unreadCount: 1,
+          mentionedUs: false,
+          isSelected: false,
+          isTyping: false,
+          isBlocked: false,
+          isKickedFromGroup: false,
+          left: false,
+          isPinned: false,
+        },
+      };
+      const comparator = _getConversationComparator(i18n);
+      const { conversations } = _getLeftPaneLists(data, comparator);
+
+      assert.strictEqual(conversations[0].name, 'Á');
+      assert.strictEqual(conversations[1].name, 'C');
+      assert.strictEqual(conversations[2].name, 'First!');
+      assert.strictEqual(conversations[3].name, 'B');
+    });
+  });
+});

ts/test/state/selectors/conversations_test.ts

@@ -1,103 +0,0 @@
-import { assert } from 'chai';
-import { ConversationTypeEnum } from '../../../models/conversation';
-
-import { ConversationLookupType } from '../../../state/ducks/conversations';
-import {
-  _getConversationComparator,
-  _getLeftPaneLists,
-} from '../../../state/selectors/conversations';
-
-describe('state/selectors/conversations', () => {
-  describe('#getLeftPaneList', () => {
-    it('sorts conversations based on timestamp then by intl-friendly title', () => {
-      const i18n = (key: string) => key;
-      const data: ConversationLookupType = {
-        id1: {
-          id: 'id1',
-          activeAt: 0,
-          name: 'No timestamp',
-          phoneNumber: 'notused',
-
-          type: ConversationTypeEnum.PRIVATE,
-          isMe: false,
-          unreadCount: 1,
-          mentionedUs: false,
-          isSelected: false,
-          isTyping: false,
-          isBlocked: false,
-          isKickedFromGroup: false,
-          left: false,
-        },
-        id2: {
-          id: 'id2',
-          activeAt: 20,
-          name: 'B',
-          phoneNumber: 'notused',
-
-          type: ConversationTypeEnum.PRIVATE,
-          isMe: false,
-          unreadCount: 1,
-          mentionedUs: false,
-          isSelected: false,
-          isTyping: false,
-          isBlocked: false,
-          isKickedFromGroup: false,
-          left: false,
-        },
-        id3: {
-          id: 'id3',
-          activeAt: 20,
-          name: 'C',
-          phoneNumber: 'notused',
-
-          type: ConversationTypeEnum.PRIVATE,
-          isMe: false,
-          unreadCount: 1,
-          mentionedUs: false,
-          isSelected: false,
-          isTyping: false,
-          isBlocked: false,
-          isKickedFromGroup: false,
-          left: false,
-        },
-        id4: {
-          id: 'id4',
-          activeAt: 20,
-          name: 'Á',
-          phoneNumber: 'notused',
-          type: ConversationTypeEnum.PRIVATE,
-          isMe: false,
-          unreadCount: 1,
-          mentionedUs: false,
-          isSelected: false,
-          isTyping: false,
-          isBlocked: false,
-          isKickedFromGroup: false,
-          left: false,
-        },
-        id5: {
-          id: 'id5',
-          activeAt: 30,
-          name: 'First!',
-          phoneNumber: 'notused',
-          type: ConversationTypeEnum.PRIVATE,
-          isMe: false,
-          unreadCount: 1,
-          mentionedUs: false,
-          isSelected: false,
-          isTyping: false,
-          isBlocked: false,
-          isKickedFromGroup: false,
-          left: false,
-        },
-      };
-      const comparator = _getConversationComparator(i18n);
-      const { conversations } = _getLeftPaneLists(data, comparator);
-
-      assert.strictEqual(conversations[0].name, 'First!');
-      assert.strictEqual(conversations[1].name, 'Á');
-      assert.strictEqual(conversations[2].name, 'B');
-      assert.strictEqual(conversations[3].name, 'C');
-    });
-  });
-});

ts/test/test-utils/utils/message.ts

@@ -88,6 +88,7 @@ export class MockConversation {
       zombies: [],
       triggerNotificationsFor: 'all',
       isTrustedForAttachmentDownload: false,
+      isPinned: false,
     };
   }