Fix inaccurate numbers on group details screen

ts/components/conversation/conversation-details/ConversationDetails.stories.tsx

@@ -45,6 +45,13 @@ const createProps = (hasGroupLink = false): Props => ({
       isMe: i === 2,
     }),
   })),
+  pendingApprovalMemberships: times(8, () => ({
+    member: getDefaultConversation(),
+  })),
+  pendingMemberships: times(5, () => ({
+    metadata: {},
+    member: getDefaultConversation(),
+  })),
   setDisappearingMessages: action('setDisappearingMessages'),
   showAllMedia: action('showAllMedia'),
   showContactModal: action('showContactModal'),

ts/components/conversation/conversation-details/ConversationDetails.tsx

@@ -22,6 +22,10 @@ import {
   ConversationDetailsMembershipList,
   GroupV2Membership,
 } from './ConversationDetailsMembershipList';
+import {
+  GroupV2PendingMembership,
+  GroupV2RequestingMembership,
+} from './PendingInvites';
 import { EditConversationAttributesModal } from './EditConversationAttributesModal';
 import { RequestState } from './util';
 
@@ -41,6 +45,8 @@ export type StateProps = {
   isAdmin: boolean;
   loadRecentMediaItems: (limit: number) => void;
   memberships: Array<GroupV2Membership>;
+  pendingApprovalMemberships: ReadonlyArray<GroupV2RequestingMembership>;
+  pendingMemberships: ReadonlyArray<GroupV2PendingMembership>;
   setDisappearingMessages: (seconds: number) => void;
   showAllMedia: () => void;
   showContactModal: (conversationId: string) => void;
@@ -77,6 +83,8 @@ export const ConversationDetails: React.ComponentType<Props> = ({
   isAdmin,
   loadRecentMediaItems,
   memberships,
+  pendingApprovalMemberships,
+  pendingMemberships,
   setDisappearingMessages,
   showAllMedia,
   showContactModal,
@@ -108,9 +116,6 @@ export const ConversationDetails: React.ComponentType<Props> = ({
     throw new Error('ConversationDetails rendered without a conversation');
   }
 
-  const pendingMemberships = conversation.pendingMemberships || [];
-  const pendingApprovalMemberships =
-    conversation.pendingApprovalMemberships || [];
   const invitesCount =
     pendingMemberships.length + pendingApprovalMemberships.length;
 
@@ -213,6 +218,7 @@ export const ConversationDetails: React.ComponentType<Props> = ({
         canEdit={canEditGroupInfo}
         conversation={conversation}
         i18n={i18n}
+        memberships={memberships}
         startEditing={() => {
           setModalState(ModalState.EditingGroupAttributes);
         }}

ts/components/conversation/conversation-details/ConversationDetailsHeader.stories.tsx

@@ -27,7 +27,6 @@ const createConversation = (): ConversationType =>
     type: 'group',
     lastUpdated: 0,
     title: text('conversation title', 'Some Conversation'),
-    memberships: new Array(number('conversation members length', 0)),
   });
 
 const createProps = (overrideProps: Partial<Props> = {}): Props => ({
@@ -35,6 +34,7 @@ const createProps = (overrideProps: Partial<Props> = {}): Props => ({
   i18n,
   canEdit: false,
   startEditing: action('startEditing'),
+  memberships: new Array(number('conversation members length', 0)),
   ...overrideProps,
 });
 

ts/components/conversation/conversation-details/ConversationDetailsHeader.tsx

@@ -7,12 +7,14 @@ import { Avatar } from '../../Avatar';
 import { Emojify } from '../Emojify';
 import { LocalizerType } from '../../../types/Util';
 import { ConversationType } from '../../../state/ducks/conversations';
+import { GroupV2Membership } from './ConversationDetailsMembershipList';
 import { bemGenerator } from './util';
 
 export type Props = {
   canEdit: boolean;
   conversation: ConversationType;
   i18n: LocalizerType;
+  memberships: Array<GroupV2Membership>;
   startEditing: () => void;
 };
 
@@ -22,10 +24,9 @@ export const ConversationDetailsHeader: React.ComponentType<Props> = ({
   canEdit,
   conversation,
   i18n,
+  memberships,
   startEditing,
 }) => {
-  const memberships = conversation.memberships || [];
-
   const contents = (
     <>
       <Avatar

ts/state/smart/ConversationDetails.tsx

@@ -12,10 +12,9 @@ import {
   getCandidateContactsForNewGroup,
   getConversationByIdSelector,
 } from '../selectors/conversations';
-import { GroupV2Membership } from '../../components/conversation/conversation-details/ConversationDetailsMembershipList';
+import { getGroupMemberships } from '../../util/getGroupMemberships';
 import { getIntl } from '../selectors/user';
 import { MediaItemType } from '../../components/LightboxGallery';
-import { isConversationUnregistered } from '../../util/isConversationUnregistered';
 import { assert } from '../../util/assert';
 
 export type SmartConversationDetailsProps = {
@@ -59,17 +58,6 @@ const mapStateToProps = (
       ? conversation.canEditGroupInfo
       : false;
 
-  const memberships = (conversation.memberships || []).reduce(
-    (result: Array<GroupV2Membership>, membership) => {
-      const member = conversationSelector(membership.conversationId);
-      if (!member || isConversationUnregistered(member)) {
-        return result;
-      }
-      return [...result, { isAdmin: membership.isAdmin, member }];
-    },
-    []
-  );
-
   const isAdmin = Boolean(conversation?.areWeAdmin);
   const candidateContactsToAdd = getCandidateContactsForNewGroup(state);
 
@@ -80,7 +68,7 @@ const mapStateToProps = (
     conversation,
     i18n: getIntl(state),
     isAdmin,
-    memberships,
+    ...getGroupMemberships(conversation, conversationSelector),
   };
 };
 

ts/state/smart/PendingInvites.tsx

@@ -4,8 +4,6 @@
 import { connect } from 'react-redux';
 import { mapDispatchToProps } from '../actions';
 import {
-  GroupV2PendingMembership,
-  GroupV2RequestingMembership,
   PendingInvites,
   PropsType,
 } from '../../components/conversation/conversation-details/PendingInvites';
@@ -13,7 +11,7 @@ import { StateType } from '../reducer';
 
 import { getIntl } from '../selectors/user';
 import { getConversationByIdSelector } from '../selectors/conversations';
-import { isConversationUnregistered } from '../../util/isConversationUnregistered';
+import { getGroupMemberships } from '../../util/getGroupMemberships';
 import { assert } from '../../util/assert';
 
 export type SmartPendingInvitesProps = {
@@ -35,39 +33,11 @@ const mapStateToProps = (
     '<SmartPendingInvites> expected a conversation to be found'
   );
 
-  const pendingApprovalMemberships = (
-    conversation.pendingApprovalMemberships || []
-  ).reduce((result: Array<GroupV2RequestingMembership>, membership) => {
-    const member = conversationSelector(membership.conversationId);
-    if (!member || isConversationUnregistered(member)) {
-      return result;
-    }
-    return [...result, { member }];
-  }, []);
-
-  const pendingMemberships = (conversation.pendingMemberships || []).reduce(
-    (result: Array<GroupV2PendingMembership>, membership) => {
-      const member = conversationSelector(membership.conversationId);
-      if (!member || isConversationUnregistered(member)) {
-        return result;
-      }
-      return [
-        ...result,
-        {
-          member,
-          metadata: { addedByUserId: membership.addedByUserId },
-        },
-      ];
-    },
-    []
-  );
-
   return {
     ...props,
+    ...getGroupMemberships(conversation, conversationSelector),
     conversation,
     i18n: getIntl(state),
-    pendingApprovalMemberships,
-    pendingMemberships,
   };
 };
 

ts/test-both/util/getGroupMemberships_test.ts

@@ -0,0 +1,235 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { assert } from 'chai';
+import { ConversationType } from '../../state/ducks/conversations';
+import { getDefaultConversation } from '../helpers/getDefaultConversation';
+
+import { getGroupMemberships } from '../../util/getGroupMemberships';
+
+describe('getGroupMemberships', () => {
+  const normalConversation1 = getDefaultConversation();
+  const normalConversation2 = getDefaultConversation();
+  const unregisteredConversation = getDefaultConversation({
+    discoveredUnregisteredAt: Date.now(),
+  });
+
+  function getConversationById(id: string): undefined | ConversationType {
+    return [
+      normalConversation1,
+      normalConversation2,
+      unregisteredConversation,
+    ].find(conversation => conversation.id === id);
+  }
+
+  describe('memberships', () => {
+    it('returns an empty array if passed undefined', () => {
+      const conversation = {};
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .memberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('returns an empty array if passed an empty array', () => {
+      const conversation = { memberships: [] };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .memberships;
+
+      assert.isEmpty(result);
+    });
+
+    it("filters out conversation IDs that don't exist", () => {
+      const conversation = {
+        memberships: [
+          {
+            conversationId: 'garbage',
+            isAdmin: true,
+          },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .memberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('filters out unregistered conversations', () => {
+      const conversation = {
+        memberships: [
+          {
+            conversationId: unregisteredConversation.id,
+            isAdmin: true,
+          },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .memberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('hydrates memberships', () => {
+      const conversation = {
+        memberships: [
+          {
+            conversationId: normalConversation2.id,
+            isAdmin: false,
+          },
+          {
+            conversationId: normalConversation1.id,
+            isAdmin: true,
+          },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .memberships;
+
+      assert.lengthOf(result, 2);
+      assert.deepEqual(result[0], {
+        isAdmin: false,
+        member: normalConversation2,
+      });
+      assert.deepEqual(result[1], {
+        isAdmin: true,
+        member: normalConversation1,
+      });
+    });
+  });
+
+  describe('pendingApprovalMemberships', () => {
+    it('returns an empty array if passed undefined', () => {
+      const conversation = {};
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingApprovalMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('returns an empty array if passed an empty array', () => {
+      const conversation = { pendingApprovalMemberships: [] };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingApprovalMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it("filters out conversation IDs that don't exist", () => {
+      const conversation = {
+        pendingApprovalMemberships: [{ conversationId: 'garbage' }],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingApprovalMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('filters out unregistered conversations', () => {
+      const conversation = {
+        pendingApprovalMemberships: [
+          { conversationId: unregisteredConversation.id },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingApprovalMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('hydrates pending-approval memberships', () => {
+      const conversation = {
+        pendingApprovalMemberships: [
+          { conversationId: normalConversation2.id },
+          { conversationId: normalConversation1.id },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingApprovalMemberships;
+
+      assert.lengthOf(result, 2);
+      assert.deepEqual(result[0], { member: normalConversation2 });
+      assert.deepEqual(result[1], { member: normalConversation1 });
+    });
+  });
+
+  describe('pendingMemberships', () => {
+    it('returns an empty array if passed undefined', () => {
+      const conversation = {};
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('returns an empty array if passed an empty array', () => {
+      const conversation = { pendingMemberships: [] };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it("filters out conversation IDs that don't exist", () => {
+      const conversation = {
+        pendingMemberships: [
+          { conversationId: 'garbage', addedByUserId: normalConversation1.id },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('filters out unregistered conversations', () => {
+      const conversation = {
+        pendingMemberships: [
+          {
+            conversationId: unregisteredConversation.id,
+            addedByUserId: normalConversation1.id,
+          },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingMemberships;
+
+      assert.isEmpty(result);
+    });
+
+    it('hydrates pending memberships', () => {
+      const conversation = {
+        pendingMemberships: [
+          { conversationId: normalConversation2.id, addedByUserId: 'abc' },
+          { conversationId: normalConversation1.id, addedByUserId: 'xyz' },
+        ],
+      };
+
+      const result = getGroupMemberships(conversation, getConversationById)
+        .pendingMemberships;
+
+      assert.lengthOf(result, 2);
+      assert.deepEqual(result[0], {
+        member: normalConversation2,
+        metadata: { addedByUserId: 'abc' },
+      });
+      assert.deepEqual(result[1], {
+        member: normalConversation1,
+        metadata: { addedByUserId: 'xyz' },
+      });
+    });
+  });
+});

ts/util/getGroupMemberships.ts

@@ -0,0 +1,65 @@
+// Copyright 2021 Signal Messenger, LLC
+// SPDX-License-Identifier: AGPL-3.0-only
+
+import { GroupV2Membership } from '../components/conversation/conversation-details/ConversationDetailsMembershipList';
+import {
+  GroupV2PendingMembership,
+  GroupV2RequestingMembership,
+} from '../components/conversation/conversation-details/PendingInvites';
+import { ConversationType } from '../state/ducks/conversations';
+import { isConversationUnregistered } from './isConversationUnregistered';
+
+export const getGroupMemberships = (
+  {
+    memberships = [],
+    pendingApprovalMemberships = [],
+    pendingMemberships = [],
+  }: Readonly<
+    Pick<
+      ConversationType,
+      'memberships' | 'pendingApprovalMemberships' | 'pendingMemberships'
+    >
+  >,
+  getConversationById: (conversationId: string) => undefined | ConversationType
+): {
+  memberships: Array<GroupV2Membership>;
+  pendingApprovalMemberships: Array<GroupV2RequestingMembership>;
+  pendingMemberships: Array<GroupV2PendingMembership>;
+} => ({
+  memberships: memberships.reduce(
+    (result: Array<GroupV2Membership>, membership) => {
+      const member = getConversationById(membership.conversationId);
+      if (!member || isConversationUnregistered(member)) {
+        return result;
+      }
+      return [...result, { isAdmin: membership.isAdmin, member }];
+    },
+    []
+  ),
+  pendingApprovalMemberships: pendingApprovalMemberships.reduce(
+    (result: Array<GroupV2RequestingMembership>, membership) => {
+      const member = getConversationById(membership.conversationId);
+      if (!member || isConversationUnregistered(member)) {
+        return result;
+      }
+      return [...result, { member }];
+    },
+    []
+  ),
+  pendingMemberships: pendingMemberships.reduce(
+    (result: Array<GroupV2PendingMembership>, membership) => {
+      const member = getConversationById(membership.conversationId);
+      if (!member || isConversationUnregistered(member)) {
+        return result;
+      }
+      return [
+        ...result,
+        {
+          member,
+          metadata: { addedByUserId: membership.addedByUserId },
+        },
+      ];
+    },
+    []
+  ),
+});