diff --git a/package.json b/package.json index 6052bed7d..aff8b6e9a 100644 --- a/package.json +++ b/package.json @@ -190,7 +190,7 @@ "@babel/preset-typescript": "7.17.12", "@electron/fuses": "1.5.0", "@mixer/parallel-prettier": "2.0.1", - "@signalapp/mock-server": "2.11.0", + "@signalapp/mock-server": "2.12.0", "@storybook/addon-a11y": "6.5.6", "@storybook/addon-actions": "6.5.6", "@storybook/addon-controls": "6.5.6", diff --git a/ts/groupChange.ts b/ts/groupChange.ts index bdba7fd22..f3a0b6fc6 100644 --- a/ts/groupChange.ts +++ b/ts/groupChange.ts @@ -269,9 +269,8 @@ export function renderChangeDetail( const { uuid, inviter } = detail; const weAreJoiner = isOurUuid(uuid); const weAreInviter = isOurUuid(inviter); - const pniPromotedToACI = weAreJoiner && from === ourPNI; - if (!from || (from !== uuid && !pniPromotedToACI)) { + if (!from || from !== uuid) { if (weAreJoiner) { // They can't be the same, no fromYou check here if (from) { diff --git a/ts/groups.ts b/ts/groups.ts index 93803d473..3a18996a5 100644 --- a/ts/groups.ts +++ b/ts/groups.ts @@ -4102,16 +4102,18 @@ async function integrateGroupChange({ `from version ${group.revision} to ${groupChangeActions.version}` ); - const { newAttributes, newProfileKeys } = await applyGroupChange({ - group, - actions: decryptedChangeActions, - sourceUuid, - }); + const { newAttributes, newProfileKeys, promotedAciToPniMap } = + await applyGroupChange({ + group, + actions: decryptedChangeActions, + sourceUuid, + }); const groupChangeMessages = extractDiffs({ old: attributes, current: newAttributes, sourceUuid, + promotedAciToPniMap, }); attributes = newAttributes; @@ -4193,11 +4195,13 @@ function extractDiffs({ dropInitialJoinMessage, old, sourceUuid, + promotedAciToPniMap, }: { current: ConversationAttributesType; dropInitialJoinMessage?: boolean; old: ConversationAttributesType; sourceUuid?: UUIDStringType; + promotedAciToPniMap?: ReadonlyMap; }): Array { const logId = idForLogging(old.groupId); const details: Array = []; @@ -4345,6 +4349,16 @@ function extractDiffs({ (current.pendingMembersV2 || []).map(member => member.uuid) ); + const aciToPniMap = new Map(promotedAciToPniMap?.entries()); + if (ourACI && ourPNI) { + aciToPniMap.set(ourACI.toString(), ourPNI.toString()); + } + + const pniToAciMap = new Map(); + for (const [aci, pni] of aciToPniMap) { + pniToAciMap.set(pni, aci); + } + (current.membersV2 || []).forEach(currentMember => { const { uuid } = currentMember; const uuidIsUs = isUs(uuid); @@ -4356,9 +4370,19 @@ function extractDiffs({ const oldMember = oldMemberLookup.get(uuid); if (!oldMember) { let pendingMember = oldPendingMemberLookup.get(uuid); - if (uuidIsUs && ourPNI && !pendingMember) { - pendingMember = oldPendingMemberLookup.get(ourPNI.toString()); + const pni = aciToPniMap.get(uuid); + if (!pendingMember && pni) { + pendingMember = oldPendingMemberLookup.get(pni); + + // Someone's ACI just joined (wasn't a member before) and their PNI + // disappeared from the invite list. Treat this as a promotion from PNI + // to ACI and pretend that the PNI wasn't pending so that we won't + // generate a pending-add-one notification below. + if (pendingMember && !currentPendingMemberSet.has(pni)) { + oldPendingMemberLookup.delete(pni); + } } + if (pendingMember) { details.push({ type: 'member-add-from-invite', @@ -4400,20 +4424,6 @@ function extractDiffs({ // This deletion makes it easier to capture removals oldMemberLookup.delete(uuid); - - // Our ACI just joined (wasn't a member before) and our PNI disappeared - // from the invite list. Treat this as a promotion from PNI to ACI and - // pretend that the PNI wasn't pending so that we won't generate a - // pending-add-one notification below. - if ( - uuidIsUs && - ourPNI && - !oldMember && - oldPendingMemberLookup.has(ourPNI.toString()) && - !currentPendingMemberSet.has(ourPNI.toString()) - ) { - oldPendingMemberLookup.delete(ourPNI.toString()); - } }); const removedMemberIds = Array.from(oldMemberLookup.keys()); @@ -4555,6 +4565,8 @@ function extractDiffs({ const isFromUs = ourACI.toString() === sourceUuid; const justJoinedGroup = !firstUpdate && !didWeStartInGroup && areWeInGroup; + const from = (sourceUuid && pniToAciMap.get(sourceUuid)) ?? sourceUuid; + // Here we hardcode initial messages if this is our first time processing data for this // group. Ideally we can collapse it down to just one of: 'you were added', // 'you were invited', or 'you created.' @@ -4564,7 +4576,7 @@ function extractDiffs({ ...generateBasicMessage(), type: 'group-v2-change', groupV2Change: { - from: whoInvitedUsUserId || sourceUuid, + from: whoInvitedUsUserId || from, details: [ { type: 'pending-add-one', @@ -4603,7 +4615,7 @@ function extractDiffs({ ...generateBasicMessage(), type: 'group-v2-change', groupV2Change: { - from: sourceUuid, + from, details: [ { type: 'create', @@ -4625,7 +4637,7 @@ function extractDiffs({ ...generateBasicMessage(), type: 'group-v2-change', groupV2Change: { - from: sourceUuid, + from, details: filteredDetails, }, readStatus: ReadStatus.Read, @@ -4636,7 +4648,7 @@ function extractDiffs({ ...generateBasicMessage(), type: 'group-v2-change', groupV2Change: { - from: sourceUuid, + from, details: [ { type: 'create', @@ -4666,7 +4678,7 @@ function extractDiffs({ type: 'group-v2-change', sourceUuid, groupV2Change: { - from: sourceUuid, + from, details: filteredDetails, }, readStatus: ReadStatus.Read, @@ -4678,7 +4690,7 @@ function extractDiffs({ type: 'group-v2-change', sourceUuid, groupV2Change: { - from: sourceUuid, + from, details, }, readStatus: ReadStatus.Read, @@ -4737,6 +4749,10 @@ type GroupApplyResultType = { newProfileKeys: Array; }; +type GroupApplyChangeResultType = GroupApplyResultType & { + promotedAciToPniMap: Map; +}; + async function applyGroupChange({ actions, group, @@ -4745,7 +4761,7 @@ async function applyGroupChange({ actions: DecryptedGroupChangeActions; group: ConversationAttributesType; sourceUuid: UUIDStringType; -}): Promise { +}): Promise { const logId = idForLogging(group.groupId); const ourACI = window.storage.user.getCheckedUuid(UUIDKind.ACI).toString(); @@ -4755,6 +4771,7 @@ async function applyGroupChange({ const version = actions.version || 0; const result = { ...group }; const newProfileKeys: Array = []; + const promotedAciToPniMap = new Map(); const members: Record = fromPairs( (result.membersV2 || []).map(member => [member.uuid, member]) @@ -4992,6 +5009,8 @@ async function applyGroupChange({ const previousRecord = pendingMembers[pni]; + promotedAciToPniMap.set(aci, pni); + if (pendingMembers[pni]) { delete pendingMembers[pni]; } else { @@ -5278,6 +5297,7 @@ async function applyGroupChange({ return { newAttributes: result, newProfileKeys, + promotedAciToPniMap, }; } diff --git a/ts/test-mock/pnp/accept_gv2_invite_test.ts b/ts/test-mock/pnp/accept_gv2_invite_test.ts index 36dfa63e8..307bb8377 100644 --- a/ts/test-mock/pnp/accept_gv2_invite_test.ts +++ b/ts/test-mock/pnp/accept_gv2_invite_test.ts @@ -262,4 +262,58 @@ describe('pnp/accept gv2 invite', function needsName() { assert(!group.getPendingMemberByUUID(desktop.uuid)); assert(group.getPendingMemberByUUID(desktop.pni)); }); + + it('should display a single notification for remote PNI accept', async () => { + const { phone, contacts, desktop } = bootstrap; + + const [first, second] = contacts; + + debug('Creating new group with Desktop'); + group = await phone.createGroup({ + title: 'Remote Invite', + members: [phone, first], + }); + + debug('Inviting remote PNI to group'); + const secondKey = await second.device.popSingleUseKey(UUIDKind.PNI); + await first.addSingleUseKey(second.device, secondKey, UUIDKind.PNI); + + group = await first.inviteToGroup(group, second.device, { + uuidKind: UUIDKind.PNI, + timestamp: bootstrap.getTimestamp(), + + // There is no one to receive it so don't bother. + sendInvite: false, + }); + + debug('Sending message to group'); + await first.sendText(desktop, 'howdy', { + group, + timestamp: bootstrap.getTimestamp(), + }); + + const window = await app.getWindow(); + const leftPane = window.locator('.left-pane-wrapper'); + + debug('Opening new group'); + await leftPane + .locator( + '_react=ConversationListItem' + + `[title = ${JSON.stringify(group.title)}]` + ) + .click(); + + debug('Accepting remote invite'); + await second.acceptPniInvite(group, desktop, { + timestamp: bootstrap.getTimestamp(), + }); + + await window + .locator( + '.SystemMessage >> ' + + `text=${second.profileName} accepted an invitation to the group ` + + `from ${first.profileName}.` + ) + .waitFor(); + }); }); diff --git a/yarn.lock b/yarn.lock index 187e3a3ac..2d0a382c7 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2057,10 +2057,10 @@ node-gyp-build "^4.2.3" uuid "^8.3.0" -"@signalapp/mock-server@2.11.0": - version "2.11.0" - resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-2.11.0.tgz#fe5f6229c4a5c28b3591e986a1622218452c5112" - integrity sha512-m23XZ8lrBn0u+zakxkKG5SezyUg6fnWwZewFF28sHNL7fQDVPHJkFCJZgE9XJwHBDM7TYz9ca/ucReW4GIPHoQ== +"@signalapp/mock-server@2.12.0": + version "2.12.0" + resolved "https://registry.yarnpkg.com/@signalapp/mock-server/-/mock-server-2.12.0.tgz#ca7ed46406603746d2cb49349d3fb843fe4b45eb" + integrity sha512-d6OFulnOWG0U3Xj0ChBGHThyBlno54Va+Cb/E0ggEx7PhD1Y9GX8gLc0V8HHP203b+1JThX4SdK03dpsOILxfQ== dependencies: "@signalapp/libsignal-client" "^0.20.0" debug "^4.3.2"