Ensure group details screen has the latest data

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

@@ -24,25 +24,6 @@ const conversation: ConversationType = {
   id: '',
   lastUpdated: 0,
   markedUnread: false,
-  memberships: Array.from(Array(32)).map((_, i) => ({
-    isAdmin: i === 1,
-    member: getDefaultConversation({
-      isMe: i === 2,
-    }),
-    metadata: {
-      conversationId: '',
-      joinedAtVersion: 0,
-      role: 2,
-    },
-  })),
-  pendingMemberships: Array.from(Array(16)).map(() => ({
-    member: getDefaultConversation({}),
-    metadata: {
-      conversationId: '',
-      role: 2,
-      timestamp: Date.now(),
-    },
-  })),
   title: 'Some Conversation',
   type: 'group',
 };
@@ -58,6 +39,12 @@ const createProps = (hasGroupLink = false): Props => ({
   i18n,
   isAdmin: false,
   loadRecentMediaItems: action('loadRecentMediaItems'),
+  memberships: times(32, i => ({
+    isAdmin: i === 1,
+    member: getDefaultConversation({
+      isMe: i === 2,
+    }),
+  })),
   setDisappearingMessages: action('setDisappearingMessages'),
   showAllMedia: action('showAllMedia'),
   showContactModal: action('showContactModal'),
@@ -91,13 +78,12 @@ story.add('as last admin', () => {
     <ConversationDetails
       {...props}
       isAdmin
-      conversation={{
+      memberships={times(32, i => ({
-        ...conversation,
+        isAdmin: i === 2,
-        memberships: conversation.memberships?.map(membership => ({
+        member: getDefaultConversation({
-          ...membership,
+          isMe: i === 2,
-          isAdmin: Boolean(membership.member.isMe),
+        }),
-        })),
+      }))}
-      }}
     />
   );
 });
@@ -109,15 +95,14 @@ story.add('as only admin', () => {
     <ConversationDetails
       {...props}
       isAdmin
-      conversation={{
+      memberships={[
-        ...conversation,
+        {
-        memberships: conversation.memberships
+          isAdmin: true,
-          ?.filter(membership => membership.member.isMe)
+          member: getDefaultConversation({
-          .map(membership => ({
+            isMe: true,
-            ...membership,
+          }),
-            isAdmin: true,
+        },
-          })),
+      ]}
-      }}
     />
   );
 });

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

@@ -20,7 +20,10 @@ import { ConversationDetailsActions } from './ConversationDetailsActions';
 import { ConversationDetailsHeader } from './ConversationDetailsHeader';
 import { ConversationDetailsIcon } from './ConversationDetailsIcon';
 import { ConversationDetailsMediaList } from './ConversationDetailsMediaList';
-import { ConversationDetailsMembershipList } from './ConversationDetailsMembershipList';
+import {
+  ConversationDetailsMembershipList,
+  GroupV2Membership,
+} from './ConversationDetailsMembershipList';
 import { EditConversationAttributesModal } from './EditConversationAttributesModal';
 import { RequestState } from './util';
 
@@ -39,6 +42,7 @@ export type StateProps = {
   i18n: LocalizerType;
   isAdmin: boolean;
   loadRecentMediaItems: (limit: number) => void;
+  memberships: Array<GroupV2Membership>;
   setDisappearingMessages: (seconds: number) => void;
   showAllMedia: () => void;
   showContactModal: (conversationId: string) => void;
@@ -70,6 +74,7 @@ export const ConversationDetails: React.ComponentType<Props> = ({
   i18n,
   isAdmin,
   loadRecentMediaItems,
+  memberships,
   setDisappearingMessages,
   showAllMedia,
   showContactModal,
@@ -101,7 +106,6 @@ export const ConversationDetails: React.ComponentType<Props> = ({
     throw new Error('ConversationDetails rendered without a conversation');
   }
 
-  const memberships = conversation.memberships || [];
   const pendingMemberships = conversation.pendingMemberships || [];
   const pendingApprovalMemberships =
     conversation.pendingApprovalMemberships || [];

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

@@ -33,8 +33,6 @@ const createMemberships = (
   ).map(
     (_, i): GroupV2Membership => ({
       isAdmin: i % 3 === 0,
-      // eslint-disable-next-line @typescript-eslint/no-explicit-any
-      metadata: {} as any,
       member: getDefaultConversation({
         isMe: i === 2,
       }),

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

@@ -8,13 +8,11 @@ import { Avatar } from '../../Avatar';
 
 import { ConversationDetailsIcon } from './ConversationDetailsIcon';
 import { ConversationType } from '../../../state/ducks/conversations';
-import { GroupV2MemberType } from '../../../model-types.d';
 import { PanelRow } from './PanelRow';
 import { PanelSection } from './PanelSection';
 
 export type GroupV2Membership = {
   isAdmin: boolean;
-  metadata: GroupV2MemberType;
   member: ConversationType;
 };
 

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

@@ -1,7 +1,8 @@
-// Copyright 2020 Signal Messenger, LLC
+// Copyright 2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import * as React from 'react';
+import { times } from 'lodash';
 
 import { storiesOf } from '@storybook/react';
 import { action } from '@storybook/addon-actions';
@@ -30,43 +31,6 @@ const conversation: ConversationType = {
   id: '',
   lastUpdated: 0,
   markedUnread: false,
-  memberships: sortedGroupMembers.map(member => ({
-    isAdmin: false,
-    member,
-    metadata: {
-      conversationId: 'abc123',
-      joinedAtVersion: 1,
-      role: 1,
-    },
-  })),
-  pendingMemberships: Array.from(Array(4))
-    .map(() => ({
-      member: getDefaultConversation({}),
-      metadata: {
-        addedByUserId: 'abc123',
-        conversationId: 'xyz789',
-        role: 1,
-        timestamp: Date.now(),
-      },
-    }))
-    .concat(
-      Array.from(Array(8)).map(() => ({
-        member: getDefaultConversation({}),
-        metadata: {
-          addedByUserId: 'def456',
-          conversationId: 'xyz789',
-          role: 1,
-          timestamp: Date.now(),
-        },
-      }))
-    ),
-  pendingApprovalMemberships: Array.from(Array(5)).map(() => ({
-    member: getDefaultConversation({}),
-    metadata: {
-      conversationId: 'xyz789',
-      timestamp: Date.now(),
-    },
-  })),
   sortedGroupMembers,
   title: 'Some Conversation',
   type: 'group',
@@ -77,6 +41,23 @@ const createProps = (): PropsType => ({
   conversation,
   i18n,
   ourConversationId: 'abc123',
+  pendingApprovalMemberships: times(5, () => ({
+    member: getDefaultConversation(),
+  })),
+  pendingMemberships: [
+    ...times(4, () => ({
+      member: getDefaultConversation(),
+      metadata: {
+        addedByUserId: 'abc123',
+      },
+    })),
+    ...times(8, () => ({
+      member: getDefaultConversation(),
+      metadata: {
+        addedByUserId: 'def456',
+      },
+    })),
+  ],
   revokePendingMemberships: action('revokePendingMemberships'),
 });
 

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

@@ -1,4 +1,4 @@
-// Copyright 2020 Signal Messenger, LLC
+// Copyright 2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import React from 'react';
@@ -12,26 +12,25 @@ import { ConfirmationDialog } from '../../ConfirmationDialog';
 import { PanelSection } from './PanelSection';
 import { PanelRow } from './PanelRow';
 import { ConversationDetailsIcon } from './ConversationDetailsIcon';
-import {
-  GroupV2PendingAdminApprovalType,
-  GroupV2PendingMemberType,
-} from '../../../model-types.d';
 
 export type PropsType = {
-  conversation?: ConversationType;
+  readonly conversation?: ConversationType;
   readonly i18n: LocalizerType;
-  ourConversationId?: string;
+  readonly ourConversationId?: string;
+  readonly pendingApprovalMemberships: ReadonlyArray<GroupV2RequestingMembership>;
+  readonly pendingMemberships: ReadonlyArray<GroupV2PendingMembership>;
   readonly approvePendingMembership: (conversationId: string) => void;
   readonly revokePendingMemberships: (conversationIds: Array<string>) => void;
 };
 
 export type GroupV2PendingMembership = {
-  metadata: GroupV2PendingMemberType;
+  metadata: {
+    addedByUserId?: string;
+  };
   member: ConversationType;
 };
 
 export type GroupV2RequestingMembership = {
-  metadata: GroupV2PendingAdminApprovalType;
   member: ConversationType;
 };
 
@@ -56,6 +55,8 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
   conversation,
   i18n,
   ourConversationId,
+  pendingMemberships,
+  pendingApprovalMemberships,
   revokePendingMemberships,
 }) => {
   if (!conversation || !ourConversationId) {
@@ -70,10 +71,6 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
     setStagedMemberships,
   ] = React.useState<Array<StagedMembershipType> | null>(null);
 
-  const allPendingMemberships = conversation.pendingMemberships || [];
-  const allRequestingMemberships =
-    conversation.pendingApprovalMemberships || [];
-
   return (
     <div className="conversation-details-panel">
       <div className="module-conversation-details__tabs">
@@ -95,7 +92,7 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
           tabIndex={0}
         >
           {i18n('PendingInvites--tab-requests', {
-            count: String(allRequestingMemberships.length),
+            count: String(pendingApprovalMemberships.length),
           })}
         </div>
 
@@ -117,7 +114,7 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
           tabIndex={0}
         >
           {i18n('PendingInvites--tab-invites', {
-            count: String(allPendingMemberships.length),
+            count: String(pendingMemberships.length),
           })}
         </div>
       </div>
@@ -126,7 +123,7 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
         <MembersPendingAdminApproval
           conversation={conversation}
           i18n={i18n}
-          memberships={allRequestingMemberships}
+          memberships={pendingApprovalMemberships}
           setStagedMemberships={setStagedMemberships}
         />
       ) : null}
@@ -135,7 +132,7 @@ export const PendingInvites: React.ComponentType<PropsType> = ({
           conversation={conversation}
           i18n={i18n}
           members={conversation.sortedGroupMembers || []}
-          memberships={allPendingMemberships}
+          memberships={pendingMemberships}
           ourConversationId={ourConversationId}
           setStagedMemberships={setStagedMemberships}
         />
@@ -233,12 +230,12 @@ function getConfirmationMessage({
   members,
   ourConversationId,
   stagedMemberships,
-}: {
+}: Readonly<{
   i18n: LocalizerType;
-  members: Array<ConversationType>;
+  members: ReadonlyArray<ConversationType>;
   ourConversationId: string;
-  stagedMemberships: Array<StagedMembershipType>;
+  stagedMemberships: ReadonlyArray<StagedMembershipType>;
-}): string {
+}>): string {
   if (!stagedMemberships || !stagedMemberships.length) {
     return '';
   }
@@ -300,12 +297,12 @@ function MembersPendingAdminApproval({
   i18n,
   memberships,
   setStagedMemberships,
-}: {
+}: Readonly<{
   conversation: ConversationType;
   i18n: LocalizerType;
-  memberships: Array<GroupV2RequestingMembership>;
+  memberships: ReadonlyArray<GroupV2RequestingMembership>;
   setStagedMemberships: (stagedMembership: Array<StagedMembershipType>) => void;
-}) {
+}>) {
   return (
     <PanelSection>
       {memberships.map(membership => (
@@ -371,14 +368,14 @@ function MembersPendingProfileKey({
   memberships,
   ourConversationId,
   setStagedMemberships,
-}: {
+}: Readonly<{
   conversation: ConversationType;
   i18n: LocalizerType;
   members: Array<ConversationType>;
-  memberships: Array<GroupV2PendingMembership>;
+  memberships: ReadonlyArray<GroupV2PendingMembership>;
   ourConversationId: string;
   setStagedMemberships: (stagedMembership: Array<StagedMembershipType>) => void;
-}) {
+}>) {
   const groupedPendingMemberships = _.groupBy(
     memberships,
     membership => membership.metadata.addedByUserId

ts/models/conversations.ts

@@ -11,11 +11,6 @@ import {
   ConversationAttributesType,
   VerificationOptions,
 } from '../model-types.d';
-import {
-  GroupV2PendingMembership,
-  GroupV2RequestingMembership,
-} from '../components/conversation/conversation-details/PendingInvites';
-import { GroupV2Membership } from '../components/conversation/conversation-details/ConversationDetailsMembershipList';
 import { CallMode, CallHistoryDetailsType } from '../types/Calling';
 import {
   CallbackResultType,
@@ -2731,32 +2726,20 @@ export class ConversationModel extends window.Backbone
     return member.role === MEMBER_ROLES.ADMINISTRATOR;
   }
 
-  getMemberships(): Array<GroupV2Membership> {
+  private getMemberships(): Array<{
+    conversationId: string;
+    isAdmin: boolean;
+  }> {
     if (!this.isGroupV2()) {
       return [];
     }
 
     const members = this.get('membersV2') || [];
-    return members
+    return members.map(member => ({
-      .map(member => {
+      isAdmin:
-        const conversationModel = window.ConversationController.get(
+        member.role === window.textsecure.protobuf.Member.Role.ADMINISTRATOR,
-          member.conversationId
+      conversationId: member.conversationId,
-        );
+    }));
-        if (!conversationModel || conversationModel.isUnregistered()) {
-          return null;
-        }
-
-        return {
-          isAdmin:
-            member.role ===
-            window.textsecure.protobuf.Member.Role.ADMINISTRATOR,
-          metadata: member,
-          member: conversationModel.format(),
-        };
-      })
-      .filter(
-        (membership): membership is GroupV2Membership => membership !== null
-      );
   }
 
   getGroupLink(): string | undefined {
@@ -2771,56 +2754,30 @@ export class ConversationModel extends window.Backbone
     return window.Signal.Groups.buildGroupLink(this);
   }
 
-  getPendingMemberships(): Array<GroupV2PendingMembership> {
+  private getPendingMemberships(): Array<{
+    addedByUserId?: string;
+    conversationId: string;
+  }> {
     if (!this.isGroupV2()) {
       return [];
     }
 
     const members = this.get('pendingMembersV2') || [];
-    return members
+    return members.map(member => ({
-      .map(member => {
+      addedByUserId: member.addedByUserId,
-        const conversationModel = window.ConversationController.get(
+      conversationId: member.conversationId,
-          member.conversationId
+    }));
-        );
-        if (!conversationModel || conversationModel.isUnregistered()) {
-          return null;
-        }
-
-        return {
-          metadata: member,
-          member: conversationModel.format(),
-        };
-      })
-      .filter(
-        (membership): membership is GroupV2PendingMembership =>
-          membership !== null
-      );
   }
 
-  getPendingApprovalMemberships(): Array<GroupV2RequestingMembership> {
+  private getPendingApprovalMemberships(): Array<{ conversationId: string }> {
     if (!this.isGroupV2()) {
       return [];
     }
 
     const members = this.get('pendingAdminApprovalV2') || [];
-    return members
+    return members.map(member => ({
-      .map(member => {
+      conversationId: member.conversationId,
-        const conversationModel = window.ConversationController.get(
+    }));
-          member.conversationId
-        );
-        if (!conversationModel || conversationModel.isUnregistered()) {
-          return null;
-        }
-
-        return {
-          metadata: member,
-          member: conversationModel.format(),
-        };
-      })
-      .filter(
-        (membership): membership is GroupV2RequestingMembership =>
-          membership !== null
-      );
   }
 
   getMembers(

ts/state/ducks/conversations.ts

@@ -25,11 +25,6 @@ import { AttachmentType } from '../../types/Attachment';
 import { ColorType } from '../../types/Colors';
 import { BodyRangeType } from '../../types/Util';
 import { CallMode, CallHistoryDetailsFromDiskType } from '../../types/Calling';
-import {
-  GroupV2PendingMembership,
-  GroupV2RequestingMembership,
-} from '../../components/conversation/conversation-details/PendingInvites';
-import { GroupV2Membership } from '../../components/conversation/conversation-details/ConversationDetailsMembershipList';
 import { MediaItemType } from '../../components/LightboxGallery';
 import {
   getGroupSizeRecommendedLimit,
@@ -96,11 +91,17 @@ export type ConversationType = {
   accessControlAttributes?: number;
   accessControlMembers?: number;
   expireTimer?: number;
-  // This is used by the ConversationDetails set of components, it includes the
+  memberships?: Array<{
-  // membersV2 data and also has some extra metadata attached to the object
+    conversationId: string;
-  memberships?: Array<GroupV2Membership>;
+    isAdmin: boolean;
-  pendingMemberships?: Array<GroupV2PendingMembership>;
+  }>;
-  pendingApprovalMemberships?: Array<GroupV2RequestingMembership>;
+  pendingMemberships?: Array<{
+    conversationId: string;
+    addedByUserId?: string;
+  }>;
+  pendingApprovalMemberships?: Array<{
+    conversationId: string;
+  }>;
   muteExpiresAt?: number;
   type: ConversationTypeType;
   isMe?: boolean;

ts/state/selectors/conversations.ts

@@ -595,6 +595,12 @@ export const getConversationSelector = createSelector(
   }
 );
 
+export const getConversationByIdSelector = createSelector(
+  getConversationLookup,
+  conversationLookup => (id: string): undefined | ConversationType =>
+    getOwn(conversationLookup, id)
+);
+
 // For now we use a shim, as selector logic is still happening in the Backbone Model.
 // What needs to happen to pull that selector logic here?
 //   1) translate ~500 lines of selector logic into TypeScript

ts/state/smart/ContactModal.tsx

@@ -42,7 +42,7 @@ const mapStateToProps = (
   let isAdmin = false;
   if (contact && currentConversation && currentConversation.memberships) {
     currentConversation.memberships.forEach(membership => {
-      if (membership.member.id === contact.id) {
+      if (membership.conversationId === contact.id) {
         isMember = true;
         isAdmin = membership.isAdmin;
       }

ts/state/smart/ConversationDetails.tsx

@@ -10,10 +10,13 @@ import {
 } from '../../components/conversation/conversation-details/ConversationDetails';
 import {
   getComposableContacts,
-  getConversationSelector,
+  getConversationByIdSelector,
 } from '../selectors/conversations';
+import { GroupV2Membership } from '../../components/conversation/conversation-details/ConversationDetailsMembershipList';
 import { getIntl } from '../selectors/user';
 import { MediaItemType } from '../../components/LightboxGallery';
+import { isConversationUnregistered } from '../../util/isConversationUnregistered';
+import { assert } from '../../util/assert';
 
 export type SmartConversationDetailsProps = {
   addMembers: (conversationIds: ReadonlyArray<string>) => Promise<void>;
@@ -44,11 +47,29 @@ const mapStateToProps = (
   state: StateType,
   props: SmartConversationDetailsProps
 ): StateProps => {
-  const conversation = getConversationSelector(state)(props.conversationId);
+  const conversationSelector = getConversationByIdSelector(state);
+  const conversation = conversationSelector(props.conversationId);
+  assert(
+    conversation,
+    '<SmartConversationDetails> expected a conversation to be found'
+  );
+
   const canEditGroupInfo =
     conversation && conversation.canEditGroupInfo
       ? 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 = getComposableContacts(state);
 
@@ -59,6 +80,7 @@ const mapStateToProps = (
     conversation,
     i18n: getIntl(state),
     isAdmin,
+    memberships,
   };
 };
 

ts/state/smart/PendingInvites.tsx

@@ -1,16 +1,20 @@
-// Copyright 2020 Signal Messenger, LLC
+// Copyright 2021 Signal Messenger, LLC
 // SPDX-License-Identifier: AGPL-3.0-only
 
 import { connect } from 'react-redux';
 import { mapDispatchToProps } from '../actions';
 import {
+  GroupV2PendingMembership,
+  GroupV2RequestingMembership,
   PendingInvites,
   PropsType,
 } from '../../components/conversation/conversation-details/PendingInvites';
 import { StateType } from '../reducer';
 
 import { getIntl } from '../selectors/user';
-import { getConversationSelector } from '../selectors/conversations';
+import { getConversationByIdSelector } from '../selectors/conversations';
+import { isConversationUnregistered } from '../../util/isConversationUnregistered';
+import { assert } from '../../util/assert';
 
 export type SmartPendingInvitesProps = {
   conversationId: string;
@@ -23,14 +27,47 @@ const mapStateToProps = (
   state: StateType,
   props: SmartPendingInvitesProps
 ): PropsType => {
-  const { conversationId } = props;
+  const conversationSelector = getConversationByIdSelector(state);
 
-  const conversation = getConversationSelector(state)(conversationId);
+  const conversation = conversationSelector(props.conversationId);
+  assert(
+    conversation,
+    '<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,
     conversation,
     i18n: getIntl(state),
+    pendingApprovalMemberships,
+    pendingMemberships,
   };
 };
 

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

@@ -23,12 +23,13 @@ import {
   getComposeSelectedContacts,
   getComposerConversationSearchTerm,
   getComposerStep,
+  getConversationByIdSelector,
   getConversationSelector,
+  getConversationsByTitleSelector,
   getInvitedContactsForNewlyCreatedGroup,
   getMaximumGroupSizeModalState,
   getPlaceholderContact,
   getRecommendedGroupSizeModalState,
-  getConversationsByTitleSelector,
   getSelectedConversation,
   getSelectedConversationId,
   hasGroupCreationError,
@@ -55,6 +56,28 @@ describe('both/state/selectors/conversations', () => {
 
   const i18n = setupI18n('en', enMessages);
 
+  describe('#getConversationByIdSelector', () => {
+    const state = {
+      ...getEmptyRootState(),
+      conversations: {
+        ...getEmptyState(),
+        conversationLookup: { abc123: getDefaultConversation('abc123') },
+      },
+    };
+
+    it('returns undefined if the conversation is not in the lookup', () => {
+      const selector = getConversationByIdSelector(state);
+      const actual = selector('xyz');
+      assert.isUndefined(actual);
+    });
+
+    it('returns the conversation in the lookup if it exists', () => {
+      const selector = getConversationByIdSelector(state);
+      const actual = selector('abc123');
+      assert.strictEqual(actual?.title, 'abc123 title');
+    });
+  });
+
   describe('#getConversationSelector', () => {
     it('returns empty placeholder if falsey id provided', () => {
       const state = getEmptyRootState();