diff --git a/ts/components/conversation/conversation-details/ConversationDetails.stories.tsx b/ts/components/conversation/conversation-details/ConversationDetails.stories.tsx index ced797c7703d..12b996241bbc 100644 --- a/ts/components/conversation/conversation-details/ConversationDetails.stories.tsx +++ b/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', () => { ({ - ...membership, - isAdmin: Boolean(membership.member.isMe), - })), - }} + memberships={times(32, i => ({ + isAdmin: i === 2, + member: getDefaultConversation({ + isMe: i === 2, + }), + }))} /> ); }); @@ -109,15 +95,14 @@ story.add('as only admin', () => { membership.member.isMe) - .map(membership => ({ - ...membership, - isAdmin: true, - })), - }} + memberships={[ + { + isAdmin: true, + member: getDefaultConversation({ + isMe: true, + }), + }, + ]} /> ); }); diff --git a/ts/components/conversation/conversation-details/ConversationDetails.tsx b/ts/components/conversation/conversation-details/ConversationDetails.tsx index f06c600612c5..618c0879fb5c 100644 --- a/ts/components/conversation/conversation-details/ConversationDetails.tsx +++ b/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; setDisappearingMessages: (seconds: number) => void; showAllMedia: () => void; showContactModal: (conversationId: string) => void; @@ -70,6 +74,7 @@ export const ConversationDetails: React.ComponentType = ({ i18n, isAdmin, loadRecentMediaItems, + memberships, setDisappearingMessages, showAllMedia, showContactModal, @@ -101,7 +106,6 @@ export const ConversationDetails: React.ComponentType = ({ throw new Error('ConversationDetails rendered without a conversation'); } - const memberships = conversation.memberships || []; const pendingMemberships = conversation.pendingMemberships || []; const pendingApprovalMemberships = conversation.pendingApprovalMemberships || []; diff --git a/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.stories.tsx b/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.stories.tsx index 88033730033b..1bd3a5b94a75 100644 --- a/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.stories.tsx +++ b/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, }), diff --git a/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.tsx b/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.tsx index d36ac2fc9f5f..656c149bd7f2 100644 --- a/ts/components/conversation/conversation-details/ConversationDetailsMembershipList.tsx +++ b/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; }; diff --git a/ts/components/conversation/conversation-details/PendingInvites.stories.tsx b/ts/components/conversation/conversation-details/PendingInvites.stories.tsx index 266cec04275b..cc2b7795d613 100644 --- a/ts/components/conversation/conversation-details/PendingInvites.stories.tsx +++ b/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'), }); diff --git a/ts/components/conversation/conversation-details/PendingInvites.tsx b/ts/components/conversation/conversation-details/PendingInvites.tsx index b14fc1c3667c..1eca1c72732f 100644 --- a/ts/components/conversation/conversation-details/PendingInvites.tsx +++ b/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; + readonly pendingMemberships: ReadonlyArray; readonly approvePendingMembership: (conversationId: string) => void; readonly revokePendingMemberships: (conversationIds: Array) => 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 = ({ conversation, i18n, ourConversationId, + pendingMemberships, + pendingApprovalMemberships, revokePendingMemberships, }) => { if (!conversation || !ourConversationId) { @@ -70,10 +71,6 @@ export const PendingInvites: React.ComponentType = ({ setStagedMemberships, ] = React.useState | null>(null); - const allPendingMemberships = conversation.pendingMemberships || []; - const allRequestingMemberships = - conversation.pendingApprovalMemberships || []; - return (
@@ -95,7 +92,7 @@ export const PendingInvites: React.ComponentType = ({ tabIndex={0} > {i18n('PendingInvites--tab-requests', { - count: String(allRequestingMemberships.length), + count: String(pendingApprovalMemberships.length), })}
@@ -117,7 +114,7 @@ export const PendingInvites: React.ComponentType = ({ tabIndex={0} > {i18n('PendingInvites--tab-invites', { - count: String(allPendingMemberships.length), + count: String(pendingMemberships.length), })}
@@ -126,7 +123,7 @@ export const PendingInvites: React.ComponentType = ({ ) : null} @@ -135,7 +132,7 @@ export const PendingInvites: React.ComponentType = ({ 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; + members: ReadonlyArray; ourConversationId: string; - stagedMemberships: Array; -}): string { + stagedMemberships: ReadonlyArray; +}>): string { if (!stagedMemberships || !stagedMemberships.length) { return ''; } @@ -300,12 +297,12 @@ function MembersPendingAdminApproval({ i18n, memberships, setStagedMemberships, -}: { +}: Readonly<{ conversation: ConversationType; i18n: LocalizerType; - memberships: Array; + memberships: ReadonlyArray; setStagedMemberships: (stagedMembership: Array) => void; -}) { +}>) { return ( {memberships.map(membership => ( @@ -371,14 +368,14 @@ function MembersPendingProfileKey({ memberships, ourConversationId, setStagedMemberships, -}: { +}: Readonly<{ conversation: ConversationType; i18n: LocalizerType; members: Array; - memberships: Array; + memberships: ReadonlyArray; ourConversationId: string; setStagedMemberships: (stagedMembership: Array) => void; -}) { +}>) { const groupedPendingMemberships = _.groupBy( memberships, membership => membership.metadata.addedByUserId diff --git a/ts/models/conversations.ts b/ts/models/conversations.ts index f91fa8f44d36..5bdd87adf8eb 100644 --- a/ts/models/conversations.ts +++ b/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 { + private getMemberships(): Array<{ + conversationId: string; + isAdmin: boolean; + }> { if (!this.isGroupV2()) { return []; } const members = this.get('membersV2') || []; - return members - .map(member => { - const conversationModel = window.ConversationController.get( - 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 - ); + return members.map(member => ({ + isAdmin: + member.role === window.textsecure.protobuf.Member.Role.ADMINISTRATOR, + conversationId: member.conversationId, + })); } getGroupLink(): string | undefined { @@ -2771,56 +2754,30 @@ export class ConversationModel extends window.Backbone return window.Signal.Groups.buildGroupLink(this); } - getPendingMemberships(): Array { + private getPendingMemberships(): Array<{ + addedByUserId?: string; + conversationId: string; + }> { if (!this.isGroupV2()) { return []; } const members = this.get('pendingMembersV2') || []; - return members - .map(member => { - const conversationModel = window.ConversationController.get( - member.conversationId - ); - if (!conversationModel || conversationModel.isUnregistered()) { - return null; - } - - return { - metadata: member, - member: conversationModel.format(), - }; - }) - .filter( - (membership): membership is GroupV2PendingMembership => - membership !== null - ); + return members.map(member => ({ + addedByUserId: member.addedByUserId, + conversationId: member.conversationId, + })); } - getPendingApprovalMemberships(): Array { + private getPendingApprovalMemberships(): Array<{ conversationId: string }> { if (!this.isGroupV2()) { return []; } const members = this.get('pendingAdminApprovalV2') || []; - return members - .map(member => { - 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 - ); + return members.map(member => ({ + conversationId: member.conversationId, + })); } getMembers( diff --git a/ts/state/ducks/conversations.ts b/ts/state/ducks/conversations.ts index c44b57ca1940..7ac56a291373 100644 --- a/ts/state/ducks/conversations.ts +++ b/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 - // membersV2 data and also has some extra metadata attached to the object - memberships?: Array; - pendingMemberships?: Array; - pendingApprovalMemberships?: Array; + memberships?: Array<{ + conversationId: string; + isAdmin: boolean; + }>; + pendingMemberships?: Array<{ + conversationId: string; + addedByUserId?: string; + }>; + pendingApprovalMemberships?: Array<{ + conversationId: string; + }>; muteExpiresAt?: number; type: ConversationTypeType; isMe?: boolean; diff --git a/ts/state/selectors/conversations.ts b/ts/state/selectors/conversations.ts index f004256f68e4..9856c0d9fdc6 100644 --- a/ts/state/selectors/conversations.ts +++ b/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 diff --git a/ts/state/smart/ContactModal.tsx b/ts/state/smart/ContactModal.tsx index 3af9d44c2090..42dfab114f78 100644 --- a/ts/state/smart/ContactModal.tsx +++ b/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; } diff --git a/ts/state/smart/ConversationDetails.tsx b/ts/state/smart/ConversationDetails.tsx index 2b2125077a81..0ee98edb3ff6 100644 --- a/ts/state/smart/ConversationDetails.tsx +++ b/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) => Promise; @@ -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, + ' expected a conversation to be found' + ); + const canEditGroupInfo = conversation && conversation.canEditGroupInfo ? conversation.canEditGroupInfo : false; + + const memberships = (conversation.memberships || []).reduce( + (result: Array, 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, }; }; diff --git a/ts/state/smart/PendingInvites.tsx b/ts/state/smart/PendingInvites.tsx index d02413fcedb9..366d6d95f9ad 100644 --- a/ts/state/smart/PendingInvites.tsx +++ b/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, + ' expected a conversation to be found' + ); + + const pendingApprovalMemberships = ( + conversation.pendingApprovalMemberships || [] + ).reduce((result: Array, membership) => { + const member = conversationSelector(membership.conversationId); + if (!member || isConversationUnregistered(member)) { + return result; + } + return [...result, { member }]; + }, []); + + const pendingMemberships = (conversation.pendingMemberships || []).reduce( + (result: Array, 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, }; }; diff --git a/ts/test-both/state/selectors/conversations_test.ts b/ts/test-both/state/selectors/conversations_test.ts index f1884f691b98..b03f4c9d2a3a 100644 --- a/ts/test-both/state/selectors/conversations_test.ts +++ b/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();