Migration: Use pendingMember roles, better 'you were invited'

_locales/en/messages.json

@@ -4010,6 +4010,10 @@
     "message": "All message history and media will be kept from before the upgrade.",
     "description": "Shown on Migration popup before GV1 migration"
   },
+  "GroupV1--Migration--info--invited--you": {
+    "message": "You will need to accept an invite to join this group again, and will not receive group messages until you accept.",
+    "description": "Shown on Learn More popup after GV1 migration"
+  },
   "GroupV1--Migration--info--invited--many": {
     "message": "These members will need to accept an invite to join this group again, and will not receive group messages until they accept:",
     "description": "Shown on Learn More popup after or Migration popup before GV1 migration"
@@ -4034,6 +4038,10 @@
     "message": "This member was not capable of joining New Groups, and was removed from the group:",
     "description": "Shown on Learn More popup after or Migration popup before GV1 migration"
   },
+  "GroupV1--Migration--invited--you": {
+    "message": "You couldn't be added to the New Group and have been invited to join.",
+    "description": "Shown in timeline when a group is upgraded and you were invited instead of added"
+  },
   "GroupV1--Migration--invited--one": {
     "message": "$contact$ couldn’t be added to the New Group and has been invited to join.",
     "description": "Shown in timeline when a group is upgraded and one person was invited, instead of added",

ts/components/GroupV1MigrationDialog.stories.tsx

@@ -36,6 +36,10 @@ function booleanOr(value: boolean | undefined, defaultValue: boolean): boolean {
 }
 
 const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
+  areWeInvited: boolean(
+    'areWeInvited',
+    booleanOr(overrideProps.areWeInvited, false)
+  ),
   droppedMembers: overrideProps.droppedMembers || [contact1],
   hasMigrated: boolean(
     'hasMigrated',
@@ -63,6 +67,17 @@ stories.add('Migrated, basic', () => {
   );
 });
 
+stories.add('Migrated, you are invited', () => {
+  return (
+    <GroupV1MigrationDialog
+      {...createProps({
+        hasMigrated: true,
+        areWeInvited: true,
+      })}
+    />
+  );
+});
+
 stories.add('Not yet migrated, multiple dropped and invited members', () => {
   return (
     <GroupV1MigrationDialog

ts/components/GroupV1MigrationDialog.tsx

@@ -16,6 +16,7 @@ export type ActionSpec = {
 type CallbackType = () => unknown;
 
 export type DataPropsType = {
+  readonly areWeInvited: boolean;
   readonly droppedMembers: Array<ConversationType>;
   readonly hasMigrated: boolean;
   readonly invitedMembers: Array<ConversationType>;
@@ -37,6 +38,7 @@ function focusRef(el: HTMLElement | null) {
 
 export const GroupV1MigrationDialog = React.memo((props: PropsType) => {
   const {
+    areWeInvited,
     droppedMembers,
     hasMigrated,
     i18n,
@@ -76,12 +78,23 @@ export const GroupV1MigrationDialog = React.memo((props: PropsType) => {
             {keepHistory}
           </div>
         </div>
-        {renderMembers(
+        {areWeInvited ? (
-          invitedMembers,
+          <div className="module-group-v2-migration-dialog__item">
-          'GroupV1--Migration--info--invited',
+            <div className="module-group-v2-migration-dialog__item__bullet" />
-          i18n
+            <div className="module-group-v2-migration-dialog__item__content">
+              {i18n('GroupV1--Migration--info--invited--you')}
+            </div>
+          </div>
+        ) : (
+          <>
+            {renderMembers(
+              invitedMembers,
+              'GroupV1--Migration--info--invited',
+              i18n
+            )}
+            {renderMembers(droppedMembers, droppedMembersKey, i18n)}
+          </>
         )}
-        {renderMembers(droppedMembers, droppedMembersKey, i18n)}
       </div>
       {renderButtons(hasMigrated, onClose, migrate, i18n)}
     </div>

ts/components/conversation/GroupV1Migration.stories.tsx

@@ -4,6 +4,8 @@
 /* eslint-disable-next-line max-classes-per-file */
 import * as React from 'react';
 import { storiesOf } from '@storybook/react';
+import { isBoolean } from 'lodash';
+import { boolean } from '@storybook/addon-knobs';
 
 import { setup as setupI18n } from '../../../js/modules/i18n';
 import enMessages from '../../../_locales/en/messages.json';
@@ -30,6 +32,10 @@ const contact2 = {
 };
 
 const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
+  areWeInvited: boolean(
+    'areWeInvited',
+    isBoolean(overrideProps.areWeInvited) ? overrideProps.areWeInvited : false
+  ),
   droppedMembers: overrideProps.droppedMembers || [contact1],
   i18n,
   invitedMembers: overrideProps.invitedMembers || [contact2],
@@ -37,6 +43,14 @@ const createProps = (overrideProps: Partial<PropsType> = {}): PropsType => ({
 
 const stories = storiesOf('Components/Conversation/GroupV1Migration', module);
 
+stories.add('You were invited', () => (
+  <GroupV1Migration
+    {...createProps({
+      areWeInvited: true,
+    })}
+  />
+));
+
 stories.add('Single dropped and single invited member', () => (
   <GroupV1Migration {...createProps()} />
 ));

ts/components/conversation/GroupV1Migration.tsx

@@ -11,6 +11,7 @@ import { ModalHost } from '../ModalHost';
 import { GroupV1MigrationDialog } from '../GroupV1MigrationDialog';
 
 export type PropsDataType = {
+  areWeInvited: boolean;
   droppedMembers: Array<ConversationType>;
   invitedMembers: Array<ConversationType>;
 };
@@ -22,7 +23,7 @@ export type PropsHousekeepingType = {
 export type PropsType = PropsDataType & PropsHousekeepingType;
 
 export function GroupV1Migration(props: PropsType): React.ReactElement {
-  const { droppedMembers, i18n, invitedMembers } = props;
+  const { areWeInvited, droppedMembers, i18n, invitedMembers } = props;
   const [showingDialog, setShowingDialog] = React.useState(false);
 
   const showDialog = React.useCallback(() => {
@@ -39,8 +40,16 @@ export function GroupV1Migration(props: PropsType): React.ReactElement {
       <div className="module-group-v1-migration--text">
         {i18n('GroupV1--Migration--was-upgraded')}
       </div>
-      {renderUsers(invitedMembers, i18n, 'GroupV1--Migration--invited')}
+      {areWeInvited ? (
-      {renderUsers(droppedMembers, i18n, 'GroupV1--Migration--removed')}
+        <div className="module-group-v1-migration--text">
+          {i18n('GroupV1--Migration--invited--you')}
+        </div>
+      ) : (
+        <>
+          {renderUsers(invitedMembers, i18n, 'GroupV1--Migration--invited')}
+          {renderUsers(droppedMembers, i18n, 'GroupV1--Migration--removed')}
+        </>
+      )}
       <button
         type="button"
         className="module-group-v1-migration--button"
@@ -51,6 +60,7 @@ export function GroupV1Migration(props: PropsType): React.ReactElement {
       {showingDialog ? (
         <ModalHost onClose={dismissDialog}>
           <GroupV1MigrationDialog
+            areWeInvited
             droppedMembers={droppedMembers}
             hasMigrated
             i18n={i18n}

ts/groups.ts

@@ -365,7 +365,7 @@ async function buildGroupProto({
 
     const uuidCipherTextBuffer = encryptUuid(clientZkGroupCipher, uuid);
     member.userId = uuidCipherTextBuffer;
-    member.role = MEMBER_ROLE_ENUM.DEFAULT;
+    member.role = item.role || MEMBER_ROLE_ENUM.DEFAULT;
 
     pendingMember.member = member;
     pendingMember.timestamp = item.timestamp;
@@ -726,8 +726,6 @@ export async function isGroupEligibleToMigrate(
 export async function getGroupMigrationMembers(
   conversation: ConversationModel
 ): Promise<{
-  areWeInvited: boolean;
-  areWeMember: boolean;
   droppedGV2MemberIds: Array<string>;
   membersV2: Array<GroupV2MemberType>;
   pendingMembersV2: Array<GroupV2PendingMemberType>;
@@ -871,13 +869,19 @@ export async function getGroupMigrationMembers(
         conversationId,
         timestamp: now,
         addedByUserId: ourConversationId,
+        role: MEMBER_ROLE_ENUM.ADMINISTRATOR,
       };
     })
   );
 
+  if (!areWeMember) {
+    throw new Error(`getGroupMigrationMembers/${logId}: We are not a member!`);
+  }
+  if (areWeInvited) {
+    throw new Error(`getGroupMigrationMembers/${logId}: We are invited!`);
+  }
+
   return {
-    areWeInvited,
-    areWeMember,
     droppedGV2MemberIds,
     membersV2,
     pendingMembersV2,
@@ -929,25 +933,12 @@ export async function initiateMigrationToGroupV2(
       }
 
       const {
-        areWeMember,
-        areWeInvited,
         membersV2,
         pendingMembersV2,
         droppedGV2MemberIds,
         previousGroupV1Members,
       } = await getGroupMigrationMembers(conversation);
 
-      if (!areWeMember) {
-        throw new Error(
-          `initiateMigrationToGroupV2/${logId}: After members migration, we are not a member!`
-        );
-      }
-      if (areWeInvited) {
-        throw new Error(
-          `initiateMigrationToGroupV2/${logId}: After members migration, we are invited!`
-        );
-      }
-
       const rawSizeLimit = window.Signal.RemoteConfig.getValue(
         'global.groupsv2.groupSizeHardLimit'
       );
@@ -1308,31 +1299,35 @@ export async function respondToGroupV2Migration({
     ...(newAttributes.membersV2 || []).map(item => item.conversationId),
     ...(newAttributes.pendingMembersV2 || []).map(item => item.conversationId),
   ];
-  const droppedGV2MemberIds: Array<string> = difference(
+  const droppedMemberIds: Array<string> = difference(
     previousGroupV1MembersIds,
     combinedConversationIds
   ).filter(id => id && id !== ourConversationId);
-  const invitedGV2Members = (newAttributes.pendingMembersV2 || []).filter(
+  const invitedMembers = (newAttributes.pendingMembersV2 || []).filter(
     item => item.conversationId !== ourConversationId
   );
 
-  // Generate notifications into the timeline
-  const groupChangeMessages: Array<MessageAttributesType> = [];
-  groupChangeMessages.push({
-    ...generateBasicMessage(),
-    type: 'group-v1-migration',
-    invitedGV2Members,
-    droppedGV2MemberIds,
-  });
-
   const areWeInvited = (newAttributes.pendingMembersV2 || []).some(
     item => item.conversationId === ourConversationId
   );
   const areWeMember = (newAttributes.membersV2 || []).some(
     item => item.conversationId === ourConversationId
   );
+
+  // Generate notifications into the timeline
+  const groupChangeMessages: Array<MessageAttributesType> = [];
+  groupChangeMessages.push({
+    ...generateBasicMessage(),
+    type: 'group-v1-migration',
+    groupMigration: {
+      areWeInvited,
+      invitedMembers,
+      droppedMemberIds,
+    },
+  });
+
   if (!areWeInvited && !areWeMember) {
-    // Add a message to the timeline saying the user was removed
+    // Add a message to the timeline saying the user was removed. This shouldn't happen.
     groupChangeMessages.push({
       ...generateBasicMessage(),
       type: 'group-v2-change',
@@ -1345,20 +1340,6 @@ export async function respondToGroupV2Migration({
         ],
       },
     });
-  } else if (areWeInvited && !areWeMember && ourConversationId) {
-    // Add a message to the timeline saying we were invited to the group
-    groupChangeMessages.push({
-      ...generateBasicMessage(),
-      type: 'group-v2-change',
-      groupV2Change: {
-        details: [
-          {
-            type: 'pending-add-one' as const,
-            conversationId: ourConversationId,
-          },
-        ],
-      },
-    });
   }
 
   // This buffer ensures that all migration-related messages are sorted above
@@ -2609,6 +2590,7 @@ async function applyGroupChange({
       conversationId: conversation.id,
       addedByUserId: added.addedByUserId,
       timestamp: added.timestamp,
+      role: added.member.role || MEMBER_ROLE_ENUM.DEFAULT,
     };
 
     if (added.member && added.member.profileKey) {
@@ -2659,6 +2641,8 @@ async function applyGroupChange({
       }
     );
 
+    const previousRecord = pendingMembers[conversation.id];
+
     if (pendingMembers[conversation.id]) {
       delete pendingMembers[conversation.id];
     } else {
@@ -2677,7 +2661,7 @@ async function applyGroupChange({
     members[conversation.id] = {
       conversationId: conversation.id,
       joinedAtVersion: version,
-      role: MEMBER_ROLE_ENUM.DEFAULT,
+      role: previousRecord.role || MEMBER_ROLE_ENUM.DEFAULT,
     };
 
     newProfileKeys.push({
@@ -2843,6 +2827,7 @@ async function applyGroupState({
 }): Promise<ConversationAttributesType> {
   const logId = idForLogging(group);
   const ACCESS_ENUM = window.textsecure.protobuf.AccessControl.AccessRequired;
+  const MEMBER_ROLE_ENUM = window.textsecure.protobuf.Member.Role;
   const version = groupState.version || 0;
   const result = { ...group };
 
@@ -2912,17 +2897,12 @@ async function applyGroupState({
         }
       }
 
-      if (
+      if (!isValidRole(member.role)) {
-        !member.role ||
+        throw new Error('applyGroupState: Member had invalid role');
-        member.role === window.textsecure.protobuf.Member.Role.UNKNOWN
-      ) {
-        throw new Error(
-          'applyGroupState: Received false or UNKNOWN member.role'
-        );
       }
 
       return {
-        role: member.role,
+        role: member.role || MEMBER_ROLE_ENUM.DEFAULT,
         joinedAtVersion: member.joinedAtVersion || version,
         conversationId: conversation.id,
       };
@@ -2947,7 +2927,9 @@ async function applyGroupState({
             }
           );
         } else {
-          throw new Error('Pending member did not have an associated userId');
+          throw new Error(
+            'applyGroupState: Pending member did not have an associated userId'
+          );
         }
 
         if (member.addedByUserId) {
@@ -2956,13 +2938,20 @@ async function applyGroupState({
             'private'
           );
         } else {
-          throw new Error('Pending member did not have an addedByUserID');
+          throw new Error(
+            'applyGroupState: Pending member did not have an addedByUserID'
+          );
+        }
+
+        if (!isValidRole(member.member.role)) {
+          throw new Error('applyGroupState: Pending member had invalid role');
         }
 
         return {
           addedByUserId: invitedBy.id,
           conversationId: pending.id,
           timestamp: member.timestamp,
+          role: member.member.role || MEMBER_ROLE_ENUM.DEFAULT,
         };
       }
     );
@@ -2971,7 +2960,7 @@ async function applyGroupState({
   return result;
 }
 
-function isValidRole(role?: number): boolean {
+function isValidRole(role?: number): role is number {
   const MEMBER_ROLE_ENUM = window.textsecure.protobuf.Member.Role;
 
   return (

ts/model-types.d.ts

@@ -13,7 +13,11 @@ import {
   LastMessageStatus,
 } from './state/ducks/conversations';
 import { SendOptionsType } from './textsecure/SendMessage';
-import { SyncMessageClass } from './textsecure.d';
+import {
+  AccessRequiredEnum,
+  MemberRoleEnum,
+  SyncMessageClass,
+} from './textsecure.d';
 import { UserMessage } from './types/Message';
 import { MessageModel } from './models/messages';
 import { ConversationModel } from './models/conversations';
@@ -46,6 +50,12 @@ export interface CustomError extends Error {
   number?: string;
 }
 
+export type GroupMigrationType = {
+  areWeInvited: boolean;
+  droppedMemberIds: Array<string>;
+  invitedMembers: Array<GroupV2PendingMemberType>;
+};
+
 export type MessageAttributesType = {
   bodyPending: boolean;
   bodyRanges: BodyRangesType;
@@ -57,11 +67,11 @@ export type MessageAttributesType = {
   deletedForEveryoneTimestamp?: number;
   delivered: number;
   delivered_to: Array<string | null>;
-  droppedGV2MemberIds?: Array<string>;
   errors: Array<CustomError> | null;
   expirationStartTimestamp: number | null;
   expireTimer: number;
   expires_at: number;
+  groupMigration?: GroupMigrationType;
   group_update: {
     avatarUpdated: boolean;
     joined: Array<string>;
@@ -74,7 +84,6 @@ export type MessageAttributesType = {
   isErased: boolean;
   isTapToViewInvalid: boolean;
   isViewOnce: boolean;
-  invitedGV2Members?: Array<GroupV2PendingMemberType>;
   key_changed: string;
   local: boolean;
   logger: unknown;
@@ -139,6 +148,10 @@ export type MessageAttributesType = {
 
   unread: number;
   timestamp: number;
+
+  // Backwards-compatibility with prerelease data schema
+  invitedGV2Members?: Array<GroupV2PendingMemberType>;
+  droppedGV2MemberIds?: Array<string>;
 };
 
 export type ConversationAttributesTypeType = 'private' | 'group';
@@ -215,8 +228,8 @@ export type ConversationAttributesType = {
 
   // GroupV2 other fields
   accessControl?: {
-    attributes: number;
+    attributes: AccessRequiredEnum;
-    members: number;
+    members: AccessRequiredEnum;
   };
   avatar?: {
     url: string;
@@ -232,13 +245,14 @@ export type ConversationAttributesType = {
 
 export type GroupV2MemberType = {
   conversationId: string;
-  role: number;
+  role: MemberRoleEnum;
   joinedAtVersion: number;
 };
 export type GroupV2PendingMemberType = {
   addedByUserId?: string;
   conversationId: string;
   timestamp: number;
+  role: MemberRoleEnum;
 };
 
 export type VerificationOptions = {

ts/models/messages.ts

@@ -515,17 +515,40 @@ export class MessageModel extends window.Backbone.Model<MessageAttributesType> {
   }
 
   getPropsForGroupV1Migration(): GroupV1MigrationPropsType {
-    const invitedGV2Members = this.get('invitedGV2Members') || [];
+    const migration = this.get('groupMigration');
-    const droppedGV2MemberIds = this.get('droppedGV2MemberIds') || [];
+    if (!migration) {
+      // Backwards-compatibility with data schema in early betas
+      const invitedGV2Members = this.get('invitedGV2Members') || [];
+      const droppedGV2MemberIds = this.get('droppedGV2MemberIds') || [];
 
-    const invitedMembers = invitedGV2Members.map(item =>
+      const invitedMembers = invitedGV2Members.map(item =>
+        this.findAndFormatContact(item.conversationId)
+      );
+      const droppedMembers = droppedGV2MemberIds.map(conversationId =>
+        this.findAndFormatContact(conversationId)
+      );
+
+      return {
+        areWeInvited: false,
+        droppedMembers,
+        invitedMembers,
+      };
+    }
+
+    const {
+      areWeInvited,
+      droppedMemberIds,
+      invitedMembers: rawInvitedMembers,
+    } = migration;
+    const invitedMembers = rawInvitedMembers.map(item =>
       this.findAndFormatContact(item.conversationId)
     );
-    const droppedMembers = droppedGV2MemberIds.map(conversationId =>
+    const droppedMembers = droppedMemberIds.map(conversationId =>
       this.findAndFormatContact(conversationId)
     );
 
     return {
+      areWeInvited,
       droppedMembers,
       invitedMembers,
     };

ts/textsecure.d.ts

@@ -261,7 +261,7 @@ export declare class MemberClass {
   // Note: only role and presentation are required when creating a group
 }
 
-type MemberRoleEnum = number;
+export type MemberRoleEnum = number;
 
 // Note: we need to use namespaces to express nested classes in Typescript
 export declare namespace MemberClass {
@@ -283,8 +283,6 @@ export declare class PendingMemberClass {
   timestamp?: ProtoBigNumberType;
 }
 
-type AccessRequiredEnum = number;
-
 export declare class AccessControlClass {
   static decode: (
     data: ArrayBuffer | ByteBufferClass,
@@ -295,6 +293,8 @@ export declare class AccessControlClass {
   members?: AccessRequiredEnum;
 }
 
+export type AccessRequiredEnum = number;
+
 // Note: we need to use namespaces to express nested classes in Typescript
 export declare namespace AccessControlClass {
   class AccessRequired {

ts/views/conversation_view.ts

@@ -1201,7 +1201,8 @@ Whisper.ConversationView = Whisper.View.extend({
       });
     };
 
-    // Grab the dropped/invited user set
+    // Note: this call will throw if, after generating member lists, we are no longer a
+    //   member or are in the pending member list.
     const {
       droppedGV2MemberIds,
       pendingMembersV2,
@@ -1219,6 +1220,7 @@ Whisper.ConversationView = Whisper.View.extend({
       JSX: window.Signal.State.Roots.createGroupV1MigrationModal(
         window.reduxStore,
         {
+          areWeInvited: false,
           droppedMemberIds: droppedGV2MemberIds,
           hasMigrated: false,
           invitedMemberIds,