From 57308d3104f5d8e3c306660d2839e38cd92bab64 Mon Sep 17 00:00:00 2001 From: Scott Nonnenberg Date: Tue, 4 Aug 2020 18:13:19 -0700 Subject: [PATCH] Fixes several bugs --- _locales/en/messages.json | 22 ++++ js/background.js | 7 +- js/models/conversations.js | 116 ++++++++++-------- js/models/messages.js | 17 ++- stylesheets/_emoji.scss | 2 +- ts/ConversationController.ts | 2 + .../conversation/ConversationHero.stories.tsx | 14 +-- .../conversation/ConversationHero.tsx | 38 ++++-- ts/model-types.d.ts | 2 + ts/util/lint/exceptions.json | 2 +- ts/util/storageService.ts | 26 ++-- 11 files changed, 164 insertions(+), 84 deletions(-) diff --git a/_locales/en/messages.json b/_locales/en/messages.json index 427c293e42..d4306e609b 100644 --- a/_locales/en/messages.json +++ b/_locales/en/messages.json @@ -2596,6 +2596,28 @@ } } }, + "ConversationHero--membership-extra": { + "message": "Member of $group1$, $group2$, $group3$ and $remainingCount$ more.", + "description": "Shown in the conversation hero to indicate this user is a member of at least three mutual groups", + "placeholders": { + "group1": { + "content": "$1", + "example": "NYC Rock Climbers" + }, + "group2": { + "content": "$2", + "example": "Dinner Party" + }, + "group3": { + "content": "$3", + "example": "Friends 🌿" + }, + "remainingCount": { + "content": "$4", + "example": "3" + } + } + }, "ConversationHero--membership-added": { "message": "$name$ added you to the group.", "description": "Shown Indicates that you were added to a group by a given individual.", diff --git a/js/background.js b/js/background.js index 85c8d77818..05f7240004 100644 --- a/js/background.js +++ b/js/background.js @@ -595,6 +595,11 @@ }); window.Signal.conversationControllerStart(); + + // We start this up before ConversationController.load() to ensure that our feature + // flags are represented in the cached props we generate on load of each convo. + window.Signal.RemoteConfig.initRemoteConfig(); + try { await Promise.all([ ConversationController.load(), @@ -1515,8 +1520,6 @@ } }); - window.Signal.RemoteConfig.initRemoteConfig(); - // Maybe refresh remote configuration when we become active window.registerForActive(async () => { await window.Signal.RemoteConfig.maybeRefreshRemoteConfig(); diff --git a/js/models/conversations.js b/js/models/conversations.js index a89d69cf21..79c27e5f8b 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -134,6 +134,13 @@ this.updateLastMessage.bind(this), 200 ); + this.throttledUpdateSharedGroups = + this.throttledUpdateSharedGroups || + _.throttle( + this.updateSharedGroups.bind(this), + 1000 * 60 * 5 // five minutes + ); + this.listenTo( this.messageCollection, 'add remove destroy content-changed', @@ -168,11 +175,10 @@ this.typingPauseTimer = null; // Keep props ready - const generateProps = () => { + this.generateProps = () => { this.cachedProps = this.getProps(); }; - this.on('change', generateProps); - generateProps(); + this.on('change', this.generateProps); }, isMe() { @@ -445,6 +451,8 @@ getProps() { const color = this.getColor(); + this.throttledUpdateSharedGroups(); + const typingValues = _.values(this.contactTypingTimers || {}); const typingMostRecent = _.first(_.sortBy(typingValues, 'timestamp')); const typingContact = typingMostRecent @@ -467,41 +475,39 @@ uuid: this.get('uuid'), e164: this.get('e164'), - isAccepted: this.getAccepted(), - isArchived: this.get('isArchived'), - isBlocked: this.isBlocked(), - isVerified: this.isVerified(), + acceptedMessageRequest: this.getAccepted(), activeAt: this.get('active_at'), avatarPath: this.getAvatarPath(), color, - type: this.isPrivate() ? 'direct' : 'group', - isMe: this.isMe(), - typingContact: typingContact ? typingContact.format() : null, - lastUpdated: this.get('timestamp'), - name: this.get('name'), - firstName: this.get('profileName'), - profileName: this.getProfileName(), - timestamp, - inboxPosition, - title: this.getTitle(), - unreadCount: this.get('unreadCount') || 0, - - shouldShowDraft, draftPreview, draftText, - - phoneNumber: this.getNumber(), - membersCount: this.isPrivate() - ? undefined - : (this.get('members') || []).length, + firstName: this.get('profileName'), + inboxPosition, + isAccepted: this.getAccepted(), + isArchived: this.get('isArchived'), + isBlocked: this.isBlocked(), + isMe: this.isMe(), + isVerified: this.isVerified(), lastMessage: { status: this.get('lastMessageStatus'), text: this.get('lastMessage'), deletedForEveryone: this.get('lastMessageDeletedForEveryone'), }, - - acceptedMessageRequest: this.getAccepted(), + lastUpdated: this.get('timestamp'), + membersCount: this.isPrivate() + ? undefined + : (this.get('members') || []).length, messageRequestsEnabled, + name: this.get('name'), + phoneNumber: this.getNumber(), + profileName: this.getProfileName(), + sharedGroupNames: this.get('sharedGroupNames'), + shouldShowDraft, + timestamp, + title: this.getTitle(), + type: this.isPrivate() ? 'direct' : 'group', + typingContact: typingContact ? typingContact.format() : null, + unreadCount: this.get('unreadCount') || 0, }; return result; @@ -1892,16 +1898,8 @@ : null, }); - // Because we're no longer using Backbone-integrated saves, we need to manually - // clear the changed fields here so our hasChanged() check below is useful. - this.changed = {}; this.set(lastMessageUpdate); - - if (this.hasChanged()) { - window.Signal.Data.updateConversation(this.attributes, { - Conversation: Whisper.Conversation, - }); - } + window.Signal.Data.updateConversation(this.attributes); }, async setArchived(isArchived) { @@ -2281,6 +2279,31 @@ } }, + // This is an expensive operation we use to populate the message request hero row. It + // shows groups the current user has in common with this potential new contact. + async updateSharedGroups() { + if (!this.isPrivate()) { + return; + } + if (this.isMe()) { + return; + } + + const ourGroups = await ConversationController.getAllGroupsInvolvingId( + ConversationController.getOurConversationId() + ); + const theirGroups = await ConversationController.getAllGroupsInvolvingId( + this.id + ); + + const sharedGroups = _.intersection(ourGroups, theirGroups); + const sharedGroupNames = sharedGroups.map(conversation => + conversation.getTitle() + ); + + this.set({ sharedGroupNames }); + }, + onChangeProfileKey() { if (this.isPrivate()) { this.getProfiles(); @@ -2325,9 +2348,6 @@ const clientZkProfileCipher = getClientZkProfileOperations( window.getServerPublicParams() ); - // Because we're no longer using Backbone-integrated saves, we need to manually - // clear the changed fields here so our hasChanged() check is useful. - c.changed = {}; let profile; @@ -2500,9 +2520,7 @@ } } - if (c.hasChanged()) { - window.Signal.Data.updateConversation(c.attributes); - } + window.Signal.Data.updateConversation(c.attributes); }, async setProfileName(encryptedName) { if (!encryptedName) { @@ -2527,10 +2545,12 @@ const profileName = given ? stringFromBytes(given) : null; const profileFamilyName = family ? stringFromBytes(family) : null; - // check for changes + // set then check for changes const oldName = this.getProfileName(); - const newName = Util.combineNames(profileName, profileFamilyName); const hadPreviousName = Boolean(oldName); + this.set({ profileName, profileFamilyName }); + + const newName = this.getProfileName(); // Note that we compare the combined names to ensure that we don't present the exact // same before/after string, even if someone is moving from just first name to @@ -2544,11 +2564,8 @@ newName, }; - this.addProfileChange(change); + await this.addProfileChange(change); } - - // set - this.set({ profileName, profileFamilyName }); }, async setProfileAvatar(avatarPath) { if (!avatarPath) { @@ -2593,9 +2610,6 @@ profileKeyVersion: null, profileKeyCredential: null, accessKey: null, - profileName: null, - profileFamilyName: null, - profileAvatar: null, sealedSender: SEALED_SENDER.UNKNOWN, }); diff --git a/js/models/messages.js b/js/models/messages.js index d938e24ab8..d2ba8d96c1 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -459,6 +459,10 @@ }); } + const placeholderContact = { + title: i18n('unknownContact'), + }; + if (groupUpdate.joined) { changes.push({ type: 'add', @@ -466,7 +470,8 @@ Array.isArray(groupUpdate.joined) ? groupUpdate.joined : [groupUpdate.joined], - identifier => this.findAndFormatContact(identifier) + identifier => + this.findAndFormatContact(identifier) || placeholderContact ), }); } @@ -483,7 +488,8 @@ Array.isArray(groupUpdate.left) ? groupUpdate.left : [groupUpdate.left], - identifier => this.findAndFormatContact(identifier) + identifier => + this.findAndFormatContact(identifier) || placeholderContact ), }); } @@ -2451,7 +2457,12 @@ conversation.get('members') ); if (difference.length > 0) { - pendingGroupUpdate.push(['joined', difference]); + // Because GroupV1 groups are based on e164 only + const e164s = difference.map(id => { + const c = ConversationController.get(id); + return c ? c.get('e164') : null; + }); + pendingGroupUpdate.push(['joined', e164s]); } if (conversation.get('left')) { window.log.warn('re-added to a left group'); diff --git a/stylesheets/_emoji.scss b/stylesheets/_emoji.scss index 53c6ec4582..f79545d795 100644 --- a/stylesheets/_emoji.scss +++ b/stylesheets/_emoji.scss @@ -43,7 +43,7 @@ span.emoji-inner { img.emoji { width: 1.4em; height: 1.4em; - margin-bottom: -4px; + margin-bottom: -5px; vertical-align: baseline; } diff --git a/ts/ConversationController.ts b/ts/ConversationController.ts index f2e6e166a4..7a1c17665e 100644 --- a/ts/ConversationController.ts +++ b/ts/ConversationController.ts @@ -636,6 +636,8 @@ export class ConversationController { await Promise.all( this._conversations.map(async conversation => { + conversation.generateProps(); + if (!conversation.get('lastMessage')) { await conversation.updateLastMessage(); } diff --git a/ts/components/conversation/ConversationHero.stories.tsx b/ts/components/conversation/ConversationHero.stories.tsx index 29579e93b5..580bd630a3 100644 --- a/ts/components/conversation/ConversationHero.stories.tsx +++ b/ts/components/conversation/ConversationHero.stories.tsx @@ -29,7 +29,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={getProfileName()} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={['NYC Rock Climbers', 'Dinner Party', 'Friends 🌿']} + sharedGroupNames={['NYC Rock Climbers', 'Dinner Party', 'Friends 🌿']} /> ); @@ -45,7 +45,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={getProfileName()} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={['NYC Rock Climbers', 'Dinner Party']} + sharedGroupNames={['NYC Rock Climbers', 'Dinner Party']} /> ); @@ -61,7 +61,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={getProfileName()} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={['NYC Rock Climbers']} + sharedGroupNames={['NYC Rock Climbers']} /> ); @@ -77,7 +77,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={text('profileName', '')} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={[]} + sharedGroupNames={[]} /> ); @@ -93,7 +93,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={getProfileName()} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={[]} + sharedGroupNames={[]} /> ); @@ -109,7 +109,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={text('profileName', '')} phoneNumber={getPhoneNumber()} conversationType="direct" - groups={[]} + sharedGroupNames={[]} /> ); @@ -125,7 +125,7 @@ storiesOf('Components/Conversation/ConversationHero', module) profileName={text('profileName', '')} phoneNumber={text('phoneNumber', '')} conversationType="direct" - groups={[]} + sharedGroupNames={[]} /> ); diff --git a/ts/components/conversation/ConversationHero.tsx b/ts/components/conversation/ConversationHero.tsx index ad2cc81556..e797a8d779 100644 --- a/ts/components/conversation/ConversationHero.tsx +++ b/ts/components/conversation/ConversationHero.tsx @@ -9,7 +9,7 @@ import { LocalizerType } from '../../types/Util'; export type Props = { i18n: LocalizerType; isMe?: boolean; - groups?: Array; + sharedGroupNames?: Array; membersCount?: number; phoneNumber?: string; onHeightChange?: () => unknown; @@ -17,10 +17,10 @@ export type Props = { const renderMembershipRow = ({ i18n, - groups, + sharedGroupNames, conversationType, isMe, -}: Pick) => { +}: Pick) => { const className = 'module-conversation-hero__membership'; const nameClassName = `${className}__name`; @@ -28,14 +28,34 @@ const renderMembershipRow = ({ return
{i18n('noteToSelfHero')}
; } - if (conversationType === 'direct' && groups && groups.length > 0) { - const firstThreeGroups = take(groups, 3).map((group, i) => ( + if ( + conversationType === 'direct' && + sharedGroupNames && + sharedGroupNames.length > 0 + ) { + const firstThreeGroups = take(sharedGroupNames, 3).map((group, i) => ( )); - if (firstThreeGroups.length >= 3) { + if (sharedGroupNames.length > 3) { + const remainingCount = sharedGroupNames.length - 3; + return ( +
+ +
+ ); + } else if (firstThreeGroups.length === 3) { return (
`g-${g}`), + ...sharedGroupNames.map(g => `g-${g}`), ]); const phoneNumberOnly = Boolean( @@ -160,7 +180,7 @@ export const ConversationHero = ({ : phoneNumber}
) : null} - {renderMembershipRow({ isMe, groups, conversationType, i18n })} + {renderMembershipRow({ isMe, sharedGroupNames, conversationType, i18n })} ); }; diff --git a/ts/model-types.d.ts b/ts/model-types.d.ts index 4161c6f2d0..a4127ad64b 100644 --- a/ts/model-types.d.ts +++ b/ts/model-types.d.ts @@ -84,6 +84,8 @@ declare class ConversationModelType extends Backbone.Model< ): void; cleanup(): Promise; disableProfileSharing(): void; + dropProfileKey(): Promise; + generateProps(): void; getAccepted(): boolean; getAvatarPath(): string | undefined; getColor(): ColorType | undefined; diff --git a/ts/util/lint/exceptions.json b/ts/util/lint/exceptions.json index 88fb195f9b..1083632d26 100644 --- a/ts/util/lint/exceptions.json +++ b/ts/util/lint/exceptions.json @@ -207,7 +207,7 @@ "rule": "jQuery-wrap(", "path": "js/models/conversations.js", "line": " await wrap(", - "lineNumber": 665, + "lineNumber": 671, "reasonCategory": "falseMatch", "updated": "2020-06-09T20:26:46.515Z" }, diff --git a/ts/util/storageService.ts b/ts/util/storageService.ts index e1fc784f66..ac9ba06925 100644 --- a/ts/util/storageService.ts +++ b/ts/util/storageService.ts @@ -189,15 +189,18 @@ async function mergeContactRecord( conversation.set({ isArchived: Boolean(contactRecord.archived), - profileFamilyName: contactRecord.familyName, - profileKey: contactRecord.profileKey - ? arrayBufferToBase64(contactRecord.profileKey.toArrayBuffer()) - : null, - profileName: contactRecord.givenName, storageID, verified, }); + if (contactRecord.profileKey) { + await conversation.setProfileKey( + arrayBufferToBase64(contactRecord.profileKey.toArrayBuffer()) + ); + } else { + await conversation.dropProfileKey(); + } + applyMessageRequestState(contactRecord, conversation); const identityKey = await window.textsecure.storage.protocol.loadIdentityKey( @@ -278,14 +281,17 @@ async function mergeAccountRecord( ); conversation.set({ - profileFamilyName: accountRecord.familyName, - profileKey: accountRecord.profileKey - ? arrayBufferToBase64(accountRecord.profileKey.toArrayBuffer()) - : null, - profileName: accountRecord.givenName, storageID, }); + if (accountRecord.profileKey) { + await conversation.setProfileKey( + arrayBufferToBase64(accountRecord.profileKey.toArrayBuffer()) + ); + } else { + await conversation.dropProfileKey(); + } + updateConversation(conversation.attributes); window.log.info(