Fix rendering of group joins and remove checkServiceIdEquivalence

ts/components/conversation/GroupV2Change.stories.tsx

@@ -50,13 +50,6 @@ const renderContact: SmartContactRendererType<JSX.Element> = (
   </React.Fragment>
 );
 
-function checkServiceIdEquivalence(
-  left: ServiceIdString | undefined,
-  right: ServiceIdString | undefined
-): boolean {
-  return Boolean(left && right && contactMap[left] === contactMap[right]);
-}
-
 const renderChange = (
   change: GroupV2ChangeType,
   {
@@ -79,7 +72,6 @@ const renderChange = (
     blockGroupLinkRequests={action('blockGroupLinkRequests')}
     conversationId="some-conversation-id"
     change={change}
-    checkServiceIdEquivalence={checkServiceIdEquivalence}
     groupBannedMemberships={groupBannedMemberships}
     groupMemberships={groupMemberships}
     groupName={groupName}
@@ -615,7 +607,39 @@ export function MemberAddFromInvited(): JSX.Element {
           },
         ],
       })}
-      ACI accepts PNI invite:
+      ACI accepts PNI invite (X joined the group)
+      {renderChange({
+        from: OUR_PNI,
+        details: [
+          {
+            type: 'member-add-from-invite',
+            aci: OUR_ACI,
+            pni: OUR_PNI,
+            inviter: CONTACT_B,
+          },
+        ],
+      })}
+      {renderChange({
+        from: OUR_PNI,
+        details: [
+          {
+            type: 'member-add-from-invite',
+            aci: OUR_ACI,
+            pni: OUR_PNI,
+          },
+        ],
+      })}
+      {renderChange({
+        from: CONTACT_A_PNI,
+        details: [
+          {
+            type: 'member-add-from-invite',
+            aci: CONTACT_A,
+            pni: CONTACT_A_PNI,
+          },
+        ],
+      })}
+      ACI accepts PNI invite, the old way (X added X to group)
       {renderChange({
         from: OUR_PNI,
         details: [

ts/components/conversation/GroupV2Change.tsx

@@ -49,10 +49,6 @@ export type PropsActionsType = {
 };
 
 export type PropsHousekeepingType = {
-  checkServiceIdEquivalence(
-    left: ServiceIdString | undefined,
-    right: ServiceIdString | undefined
-  ): boolean;
   i18n: LocalizerType;
   renderContact: SmartContactRendererType<JSX.Element>;
 };
@@ -297,7 +293,6 @@ export function GroupV2Change(props: PropsType): ReactElement {
     areWeAdmin,
     blockGroupLinkRequests,
     change,
-    checkServiceIdEquivalence,
     conversationId,
     groupBannedMemberships,
     groupMemberships,
@@ -311,7 +306,6 @@ export function GroupV2Change(props: PropsType): ReactElement {
   return (
     <>
       {renderChange<JSX.Element>(change, {
-        checkServiceIdEquivalence,
         i18n,
         ourAci,
         ourPni,

ts/components/conversation/Timeline.stories.tsx

@@ -361,7 +361,6 @@ const renderItem = ({
     isNextItemCallingNotification={false}
     theme={ThemeType.light}
     platform="darwin"
-    checkServiceIdEquivalence={() => false}
     containerElementRef={containerElementRef}
     containerWidthBreakpoint={containerWidthBreakpoint}
     conversationId=""

ts/components/conversation/TimelineItem.stories.tsx

@@ -69,7 +69,6 @@ const getDefaultProps = () => ({
   toggleSelectMessage: action('toggleSelectMessage'),
   reactToMessage: action('reactToMessage'),
   checkForAccount: action('checkForAccount'),
-  checkServiceIdEquivalence: () => false,
   clearTargetedMessage: action('clearTargetedMessage'),
   setMessageToEdit: action('setMessageToEdit'),
   setQuoteByMessageId: action('setQuoteByMessageId'),

ts/components/conversation/TimelineItem.tsx

@@ -61,7 +61,6 @@ import {
   type MessageRequestResponseNotificationData,
 } from './MessageRequestResponseNotification';
 import type { MessageRequestState } from './MessageRequestActionsConfirmation';
-import type { ServiceIdString } from '../../types/ServiceId';
 
 type CallHistoryType = {
   type: 'callHistory';
@@ -173,10 +172,6 @@ export type TimelineItemType = (
 ) & { timestamp: number };
 
 type PropsLocalType = {
-  checkServiceIdEquivalence(
-    left: ServiceIdString | undefined,
-    right: ServiceIdString | undefined
-  ): boolean;
   containerElementRef: RefObject<HTMLElement>;
   conversationId: string;
   item?: TimelineItemType;

ts/groupChange.ts

@@ -37,10 +37,6 @@ export type RenderOptionsType<T extends string | JSX.Element> = {
   ourAci: AciString | undefined;
   ourPni: PniString | undefined;
   renderContact: SmartContactRendererType<T>;
-  checkServiceIdEquivalence(
-    left: ServiceIdString | undefined,
-    right: ServiceIdString | undefined
-  ): boolean;
   renderIntl: StringRendererType<T>;
 };
 
@@ -87,7 +83,6 @@ function renderChangeDetail<T extends string | JSX.Element>(
   options: RenderOptionsType<T>
 ): string | T | ReadonlyArray<string | T> {
   const {
-    checkServiceIdEquivalence,
     from,
     i18n: localizer,
     ourAci,
@@ -301,11 +296,14 @@ function renderChangeDetail<T extends string | JSX.Element>(
     });
   }
   if (detail.type === 'member-add-from-invite') {
-    const { aci, inviter } = detail;
+    const { aci, inviter, pni } = detail;
     const weAreJoiner = isOurServiceId(aci);
     const weAreInviter = isOurServiceId(inviter);
 
-    if (!from || !checkServiceIdEquivalence(from, aci)) {
+    const fromPni = pni && from === pni;
+    const fromAci = from === aci;
+
+    if (!from || (!fromPni && !fromAci)) {
       if (weAreJoiner) {
         // They can't be the same, no fromYou check here
         if (from) {
@@ -433,7 +431,7 @@ function renderChangeDetail<T extends string | JSX.Element>(
         memberName: renderContact(aci),
       });
     }
-    if (from && fromYou) {
+    if (from && from === aci) {
       return i18n('icu:GroupV2--member-remove--other--self', {
         memberName: renderContact(from),
       });

ts/groups.ts

@@ -149,6 +149,7 @@ type GroupV2MemberAddChangeType = {
 type GroupV2MemberAddFromInviteChangeType = {
   type: 'member-add-from-invite';
   aci: AciString;
+  pni?: PniString;
   inviter?: AciString;
 };
 type GroupV2MemberAddFromLinkChangeType = {
@@ -4430,6 +4431,7 @@ function extractDiffs({
         details.push({
           type: 'member-add-from-invite',
           aci,
+          pni,
           inviter: pendingMember.addedByUserId,
         });
       } else if (currentMember.joinedFromLink) {

ts/services/backups/export.ts

@@ -1253,7 +1253,13 @@ export class BackupExportStream extends Readable {
         update.groupMemberAddedUpdate = innerUpdate;
         updates.push(update);
       } else if (type === 'member-add-from-invite') {
-        if (from && checkServiceIdEquivalence(from, detail.aci)) {
+        const { aci, pni } = detail;
+        if (
+          from &&
+          ((pni && from === pni) ||
+            (aci && from === aci) ||
+            checkServiceIdEquivalence(from, aci))
+        ) {
           const innerUpdate = new Backups.GroupInvitationAcceptedUpdate();
           innerUpdate.newMemberAci = this.aciToBytes(detail.aci);
           if (detail.inviter) {

ts/state/selectors/conversations.ts

@@ -931,24 +931,6 @@ export const getConversationSelector = createSelector(
   }
 );
 
-export type CheckServiceIdEquivalenceType = (
-  left: ServiceIdString | undefined,
-  right: ServiceIdString | undefined
-) => boolean;
-export const getCheckServiceIdEquivalence = createSelector(
-  getConversationByAnyIdSelector,
-  (
-    getById: GetConversationByAnyIdSelectorType
-  ): CheckServiceIdEquivalenceType => {
-    return (
-      left: ServiceIdString | undefined,
-      right: ServiceIdString | undefined
-    ): boolean => {
-      return Boolean(left && right && getById(left) === getById(right));
-    };
-  }
-);
-
 export const getConversationByIdSelector = createSelector(
   getConversationLookup,
   conversationLookup =>

ts/state/smart/TimelineItem.tsx

@@ -21,10 +21,7 @@ import {
   getTheme,
   getPlatform,
 } from '../selectors/user';
-import {
+import { getTargetedMessage } from '../selectors/conversations';
-  getTargetedMessage,
-  getCheckServiceIdEquivalence,
-} from '../selectors/conversations';
 import { useTimelineItem } from '../selectors/timeline';
 import {
   areMessagesInSameGroup,
@@ -89,8 +86,6 @@ export const SmartTimelineItem = memo(function SmartTimelineItem(
   const isTargeted = Boolean(
     targetedMessage && messageId === targetedMessage.id
   );
-  const checkServiceIdEquivalence = useSelector(getCheckServiceIdEquivalence);
-
   const isNextItemCallingNotification = nextItem?.type === 'callHistory';
 
   const shouldCollapseAbove = areMessagesInSameGroup(
@@ -180,7 +175,6 @@ export const SmartTimelineItem = memo(function SmartTimelineItem(
     <TimelineItem
       item={item}
       id={messageId}
-      checkServiceIdEquivalence={checkServiceIdEquivalence}
       containerElementRef={containerElementRef}
       containerWidthBreakpoint={containerWidthBreakpoint}
       conversationId={conversationId}

ts/test-electron/backup/backup_groupv2_notifications_test.ts

@@ -709,7 +709,40 @@ describe('backup/groupv2/notifications', () => {
             },
           ],
         }),
-        // ACI accepts PNI invite:
+        // ACI accepts PNI invite (X joined the group)
+        // These don't roundtrip; the PNI from is replaced with ACI. See next test below.
+        // createMessage({
+        //   from: OUR_PNI,
+        //   details: [
+        //     {
+        //       type: 'member-add-from-invite',
+        //       aci: OUR_ACI,
+        //       pni: OUR_PNI,
+        //       inviter: CONTACT_B,
+        //     },
+        //   ],
+        // }),
+        // createMessage({
+        //   from: OUR_PNI,
+        //   details: [
+        //     {
+        //       type: 'member-add-from-invite',
+        //       aci: OUR_ACI,
+        //       pni: OUR_PNI,
+        //     },
+        //   ],
+        // }),
+        // createMessage({
+        //   from: CONTACT_A_PNI,
+        //   details: [
+        //     {
+        //       type: 'member-add-from-invite',
+        //       aci: CONTACT_A,
+        //       pni: CONTACT_A_PNI,
+        //     },
+        //   ],
+        // }),
+        // ACI accepts PNI invite, the old way (X added X to group)
         // These don't roundtrip; the PNI is replaced with ACI. See next test below.
         // createMessage({
         //   from: OUR_PNI,
@@ -813,8 +846,31 @@ describe('backup/groupv2/notifications', () => {
         { disableIncrement: true }
       );
 
-      const before = [firstBefore, secondBefore, thirdBefore];
+      const fourthBefore = createMessage({
-      const after = [firstAfter, secondAfter, thirdAfter];
+        from: CONTACT_A_PNI,
+        details: [
+          {
+            type: 'member-add-from-invite',
+            aci: CONTACT_A,
+            pni: CONTACT_A_PNI,
+          },
+        ],
+      });
+      const fourthAfter = createMessage(
+        {
+          from: CONTACT_A,
+          details: [
+            {
+              type: 'member-add-from-invite',
+              aci: CONTACT_A,
+            },
+          ],
+        },
+        { disableIncrement: true }
+      );
+
+      const before = [firstBefore, secondBefore, thirdBefore, fourthBefore];
+      const after = [firstAfter, secondAfter, thirdAfter, fourthAfter];
 
       await asymmetricRoundtripHarness(before, after);
     });

ts/util/getNotificationDataForMessage.ts

@@ -143,12 +143,6 @@ export function getNotificationDataForMessage(
     );
 
     const changes = GroupChange.renderChange<string>(change, {
-      checkServiceIdEquivalence: (left, right) => {
-        return (
-          window.ConversationController.get(left) ===
-          window.ConversationController.get(right)
-        );
-      },
       i18n: window.i18n,
       ourAci: window.textsecure.storage.user.getCheckedAci(),
       ourPni: window.textsecure.storage.user.getCheckedPni(),