GroupV2: Show summary of change details on re-join of group

This commit is contained in:
Scott Nonnenberg 2022-12-06 13:12:57 -08:00 committed by GitHub
parent 105162dc66
commit e9b7a74b32
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 181 additions and 42 deletions

View file

@ -4099,6 +4099,10 @@
"message": "The group was changed to allow all members to send messages.", "message": "The group was changed to allow all members to send messages.",
"description": "Shown in timeline or conversation preview when v2 group changes" "description": "Shown in timeline or conversation preview when v2 group changes"
}, },
"icu:GroupV2--summary": {
"messageformat": "This group's members or settings have changed.",
"description": "When rejoining a group, any detected changes are collapsed down into this summary"
},
"GroupV1--Migration--disabled": { "GroupV1--Migration--disabled": {
"message": "Upgrade this group to activate new features like @mentions and admins. Members who have not shared their name or photo in this group will be invited to join. $learnMore$", "message": "Upgrade this group to activate new features like @mentions and admins. Members who have not shared their name or photo in this group will be invited to join. $learnMore$",
"description": "Shown instead of composition area when user is forced to migrate a legacy group (GV1)." "description": "Shown instead of composition area when user is forced to migrate a legacy group (GV1)."

View file

@ -156,6 +156,13 @@
); );
} }
&--icon-group-summary::before {
@include system-message-icon(
'../images/icons/v2/info-16.svg',
'../images/icons/v2/info-16.svg'
);
}
&--icon-info::before { &--icon-info::before {
@include system-message-icon( @include system-message-icon(
'../images/icons/v2/info-16.svg', '../images/icons/v2/info-16.svg',

View file

@ -1278,14 +1278,45 @@ export function AdminApprovalRemove(): JSX.Element {
}, },
], ],
})} })}
</>
);
}
AdminApprovalRemove.story = {
name: 'Admin Approval (Remove)',
};
export function AdminApprovalBounce(): JSX.Element {
return (
<>
Should show button: Should show button:
{renderChange( {renderChange(
{ {
// From Joiner
from: CONTACT_A, from: CONTACT_A,
details: [ details: [
{ {
type: 'admin-approval-remove-one', type: 'admin-approval-bounce',
uuid: CONTACT_A, uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
},
],
},
{
groupMemberships: [{ uuid: CONTACT_C, isAdmin: false }],
groupBannedMemberships: [CONTACT_B],
}
)}
{renderChange(
{
// From nobody
details: [
{
type: 'admin-approval-bounce',
uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
}, },
], ],
}, },
@ -1295,37 +1326,26 @@ export function AdminApprovalRemove(): JSX.Element {
} }
)} )}
{renderChange({ {renderChange({
from: ADMIN_A,
details: [ details: [
{ {
type: 'admin-approval-remove-one', type: 'admin-approval-bounce',
uuid: CONTACT_A, uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
}, },
// No group membership info
], ],
})} })}
Should show button:
{renderChange(
{
details: [
{
type: 'admin-approval-remove-one',
uuid: CONTACT_A,
},
],
},
{
groupMemberships: [{ uuid: CONTACT_C, isAdmin: false }],
groupBannedMemberships: [CONTACT_B],
}
)}
Would show button, but we&apos;re not admin: Would show button, but we&apos;re not admin:
{renderChange( {renderChange(
{ {
from: CONTACT_A, from: CONTACT_A,
details: [ details: [
{ {
type: 'admin-approval-remove-one', type: 'admin-approval-bounce',
uuid: CONTACT_A, uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
}, },
], ],
}, },
@ -1338,8 +1358,10 @@ export function AdminApprovalRemove(): JSX.Element {
from: CONTACT_A, from: CONTACT_A,
details: [ details: [
{ {
type: 'admin-approval-remove-one', type: 'admin-approval-bounce',
uuid: CONTACT_A, uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
}, },
], ],
}, },
@ -1351,8 +1373,10 @@ export function AdminApprovalRemove(): JSX.Element {
from: CONTACT_A, from: CONTACT_A,
details: [ details: [
{ {
type: 'admin-approval-remove-one', type: 'admin-approval-bounce',
uuid: CONTACT_A, uuid: CONTACT_A,
times: 1,
isApprovalPending: false,
}, },
], ],
}, },
@ -1363,8 +1387,8 @@ export function AdminApprovalRemove(): JSX.Element {
); );
} }
AdminApprovalRemove.story = { AdminApprovalBounce.story = {
name: 'Admin Approval (Remove)', name: 'Admin Approval (Bounce)',
}; };
export function GroupLinkAdd(): JSX.Element { export function GroupLinkAdd(): JSX.Element {
@ -1646,3 +1670,18 @@ export function AnnouncementGroupChange(): JSX.Element {
AnnouncementGroupChange.story = { AnnouncementGroupChange.story = {
name: 'Announcement Group (Change)', name: 'Announcement Group (Change)',
}; };
export function Summary(): JSX.Element {
return (
<>
{renderChange({
from: OUR_ACI,
details: [
{
type: 'summary',
},
],
})}
</>
);
}

View file

@ -70,6 +70,7 @@ type GroupIconType =
| 'group-avatar' | 'group-avatar'
| 'group-decline' | 'group-decline'
| 'group-edit' | 'group-edit'
| 'group-summary'
| 'group-leave' | 'group-leave'
| 'group-remove'; | 'group-remove';
@ -120,6 +121,9 @@ function getIcon(
if (changeType === 'admin-approval-bounce' && isLastText) { if (changeType === 'admin-approval-bounce' && isLastText) {
possibleIcon = undefined; possibleIcon = undefined;
} }
if (changeType === 'summary') {
possibleIcon = 'group-summary';
}
return possibleIcon || 'group'; return possibleIcon || 'group';
} }

View file

@ -925,6 +925,9 @@ export function renderChangeDetail<T>(
} }
return renderString('GroupV2--announcements--member--unknown', i18n); return renderString('GroupV2--announcements--member--unknown', i18n);
} }
if (detail.type === 'summary') {
return renderString('icu:GroupV2--summary', i18n);
}
throw missingCaseError(detail); throw missingCaseError(detail);
} }

View file

@ -202,6 +202,9 @@ export type GroupV2DescriptionChangeType = {
// Adding this field; cannot remove previous field for backwards compatibility // Adding this field; cannot remove previous field for backwards compatibility
description?: string; description?: string;
}; };
export type GroupV2SummaryType = {
type: 'summary';
};
export type GroupV2ChangeDetailType = export type GroupV2ChangeDetailType =
| GroupV2AccessAttributesChangeType | GroupV2AccessAttributesChangeType
@ -227,6 +230,7 @@ export type GroupV2ChangeDetailType =
| GroupV2PendingAddOneChangeType | GroupV2PendingAddOneChangeType
| GroupV2PendingRemoveManyChangeType | GroupV2PendingRemoveManyChangeType
| GroupV2PendingRemoveOneChangeType | GroupV2PendingRemoveOneChangeType
| GroupV2SummaryType
| GroupV2TitleChangeType; | GroupV2TitleChangeType;
export type GroupV2ChangeType = { export type GroupV2ChangeType = {
@ -3658,32 +3662,62 @@ async function updateGroupViaSingleChange({
groupChange: Proto.IGroupChange; groupChange: Proto.IGroupChange;
newRevision: number; newRevision: number;
}): Promise<UpdatesResultType> { }): Promise<UpdatesResultType> {
const previouslyKnewAboutThisGroup =
isNumber(group.revision) && group.membersV2?.length;
const wasInGroup = !group.left; const wasInGroup = !group.left;
const result: UpdatesResultType = await integrateGroupChange({ const singleChangeResult: UpdatesResultType = await integrateGroupChange({
group, group,
groupChange, groupChange,
newRevision, newRevision,
}); });
const nowInGroup = !result.newAttributes.left; const nowInGroup = !singleChangeResult.newAttributes.left;
// If we were just added to the group (for example, via a join link), we go fetch the // If we were just added to the group (for example, via a join link), we go fetch the
// entire group state to make sure we're up to date. // entire group state to make sure we're up to date.
if (!wasInGroup && nowInGroup) { if (!wasInGroup && nowInGroup) {
const { newAttributes, members } = await updateGroupViaState({ const {
group: result.newAttributes, newAttributes,
members,
groupChangeMessages: catchupMessages,
} = await updateGroupViaState({
group: singleChangeResult.newAttributes,
}); });
const groupChangeMessages = [...singleChangeResult.groupChangeMessages];
// If we've just been added to a group we were previously in, we do want to show
// a summary instead of nothing.
if (
groupChangeMessages.length > 0 &&
previouslyKnewAboutThisGroup &&
catchupMessages.length > 0
) {
groupChangeMessages.push({
...generateBasicMessage(),
type: 'group-v2-change',
groupV2Change: {
details: [
{
type: 'summary',
},
],
},
readStatus: ReadStatus.Read,
// For simplicity, since we don't know who this change is from here, always Seen
seenStatus: SeenStatus.Seen,
});
}
// We discard any change events that come out of this full group fetch, but we do // We discard any change events that come out of this full group fetch, but we do
// keep the final group attributes generated, as well as any new members. // keep the final group attributes generated, as well as any new members.
return { return {
...result, groupChangeMessages,
members: [...result.members, ...members], members: [...singleChangeResult.members, ...members],
newAttributes, newAttributes,
}; };
} }
return result; return singleChangeResult;
} }
async function updateGroupViaLogs({ async function updateGroupViaLogs({
@ -4170,6 +4204,21 @@ function extractDiffs({
let areWePendingApproval = false; let areWePendingApproval = false;
let whoInvitedUsUserId = null; let whoInvitedUsUserId = null;
function isUs(uuid: UUIDStringType): boolean {
return uuid === ourACI.toString() || uuid === ourPNI?.toString();
}
function keepOnlyOurAdds(
list: Array<GroupV2ChangeDetailType>
): Array<GroupV2ChangeDetailType> {
return list.filter(
item =>
(item.type === 'member-add-from-invite' && isUs(item.uuid)) ||
(item.type === 'member-add-from-link' && isUs(item.uuid)) ||
(item.type === 'member-add-from-admin-approval' && isUs(item.uuid)) ||
(item.type === 'member-add' && isUs(item.uuid))
);
}
// access control // access control
if ( if (
@ -4274,6 +4323,10 @@ function extractDiffs({
const oldMemberLookup = new Map<UUIDStringType, GroupV2MemberType>( const oldMemberLookup = new Map<UUIDStringType, GroupV2MemberType>(
(old.membersV2 || []).map(member => [member.uuid, member]) (old.membersV2 || []).map(member => [member.uuid, member])
); );
const didWeStartInGroup =
(ourACI && oldMemberLookup.get(ourACI.toString())) ||
(ourPNI && oldMemberLookup.get(ourPNI.toString()));
const oldPendingMemberLookup = new Map< const oldPendingMemberLookup = new Map<
UUIDStringType, UUIDStringType,
GroupV2PendingMemberType GroupV2PendingMemberType
@ -4288,16 +4341,16 @@ function extractDiffs({
(current.membersV2 || []).forEach(currentMember => { (current.membersV2 || []).forEach(currentMember => {
const { uuid } = currentMember; const { uuid } = currentMember;
const isUs = uuid === ourACI.toString(); const uuidIsUs = isUs(uuid);
if (isUs) { if (uuidIsUs) {
areWeInGroup = true; areWeInGroup = true;
} }
const oldMember = oldMemberLookup.get(uuid); const oldMember = oldMemberLookup.get(uuid);
if (!oldMember) { if (!oldMember) {
let pendingMember = oldPendingMemberLookup.get(uuid); let pendingMember = oldPendingMemberLookup.get(uuid);
if (isUs && ourPNI && !pendingMember) { if (uuidIsUs && ourPNI && !pendingMember) {
pendingMember = oldPendingMemberLookup.get(ourPNI.toString()); pendingMember = oldPendingMemberLookup.get(ourPNI.toString());
} }
if (pendingMember) { if (pendingMember) {
@ -4347,7 +4400,7 @@ function extractDiffs({
// pretend that the PNI wasn't pending so that we won't generate a // pretend that the PNI wasn't pending so that we won't generate a
// pending-add-one notification below. // pending-add-one notification below.
if ( if (
isUs && uuidIsUs &&
ourPNI && ourPNI &&
!oldMember && !oldMember &&
oldPendingMemberLookup.has(ourPNI.toString()) && oldPendingMemberLookup.has(ourPNI.toString()) &&
@ -4373,7 +4426,7 @@ function extractDiffs({
const { uuid } = currentPendingMember; const { uuid } = currentPendingMember;
const oldPendingMember = oldPendingMemberLookup.get(uuid); const oldPendingMember = oldPendingMemberLookup.get(uuid);
if (uuid === ourACI.toString() || uuid === ourPNI?.toString()) { if (isUs(uuid)) {
if (uuid === ourACI.toString()) { if (uuid === ourACI.toString()) {
uuidKindInvitedToGroup = UUIDKind.ACI; uuidKindInvitedToGroup = UUIDKind.ACI;
} else if (uuidKindInvitedToGroup === undefined) { } else if (uuidKindInvitedToGroup === undefined) {
@ -4494,8 +4547,9 @@ function extractDiffs({
const firstUpdate = !isNumber(old.revision); const firstUpdate = !isNumber(old.revision);
const isFromUs = ourACI.toString() === sourceUuid; const isFromUs = ourACI.toString() === sourceUuid;
const justJoinedGroup = !firstUpdate && !didWeStartInGroup && areWeInGroup;
// Here we hardcode initial messages if this is our first time processing data this // 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', // group. Ideally we can collapse it down to just one of: 'you were added',
// 'you were invited', or 'you created.' // 'you were invited', or 'you created.'
if (firstUpdate && uuidKindInvitedToGroup !== undefined) { if (firstUpdate && uuidKindInvitedToGroup !== undefined) {
@ -4554,17 +4608,19 @@ function extractDiffs({
seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
}; };
} else if (firstUpdate && areWeInGroup) { } else if (firstUpdate && areWeInGroup) {
const filteredDetails = keepOnlyOurAdds(details);
strictAssert(
filteredDetails.length === 1,
'extractDiffs/firstUpdate: Should be only one self-add!'
);
message = { message = {
...generateBasicMessage(), ...generateBasicMessage(),
type: 'group-v2-change', type: 'group-v2-change',
groupV2Change: { groupV2Change: {
from: sourceUuid, from: sourceUuid,
details: [ details: filteredDetails,
{
type: 'member-add',
uuid: ourACI.toString(),
},
],
}, },
readStatus: ReadStatus.Read, readStatus: ReadStatus.Read,
seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
@ -4584,6 +4640,32 @@ function extractDiffs({
readStatus: ReadStatus.Read, readStatus: ReadStatus.Read,
seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen, seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
}; };
} else if (justJoinedGroup) {
const filteredDetails = keepOnlyOurAdds(details);
strictAssert(
filteredDetails.length === 1,
'extractDiffs/justJoinedGroup: Should be only one self-add!'
);
// If we've dropped other changes, we collapse them into a single summary
if (details.length > 1) {
filteredDetails.push({
type: 'summary',
});
}
message = {
...generateBasicMessage(),
type: 'group-v2-change',
sourceUuid,
groupV2Change: {
from: sourceUuid,
details: filteredDetails,
},
readStatus: ReadStatus.Read,
seenStatus: isFromUs ? SeenStatus.Seen : SeenStatus.Unseen,
};
} else if (details.length > 0) { } else if (details.length > 0) {
message = { message = {
...generateBasicMessage(), ...generateBasicMessage(),