From 831ae0957681e94f8555a58e2993c43c78654dcc Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 30 Aug 2019 09:42:13 +1000 Subject: [PATCH] Cherry pick "profile simplification" from Mik --- background.html | 1 - js/background.js | 20 +++--- js/conversation_controller.js | 2 +- js/models/conversations.js | 14 ++-- js/models/messages.js | 6 ++ js/models/profile.js | 56 ---------------- js/views/inbox_view.js | 2 +- libtextsecure/account_manager.js | 10 +-- libtextsecure/message_receiver.js | 2 +- libtextsecure/sendmessage.js | 22 +++++- test/index.html | 1 - test/models/profile_test.js | 108 ------------------------------ 12 files changed, 45 insertions(+), 199 deletions(-) delete mode 100644 js/models/profile.js delete mode 100644 test/models/profile_test.js diff --git a/background.html b/background.html index bc0ed8687..a7a201643 100644 --- a/background.html +++ b/background.html @@ -705,7 +705,6 @@ - diff --git a/js/background.js b/js/background.js index dd63f1151..88c2cd755 100644 --- a/js/background.js +++ b/js/background.js @@ -652,25 +652,21 @@ } }); - Whisper.events.on('onEditProfile', () => { + Whisper.events.on('onEditProfile', async () => { const ourNumber = textsecure.storage.user.getNumber(); - const profile = storage.getLocalProfile(); + const conversation = await ConversationController.getOrCreateAndWait( + ourNumber, + 'private' + ); + const profile = conversation.getLokiProfile(); const displayName = profile && profile.displayName; if (appView) { appView.showNicknameDialog({ title: window.i18n('editProfileTitle'), message: window.i18n('editProfileDisplayNameWarning'), nickname: displayName, - onOk: async newName => { - await storage.setProfileName(newName); - - // Update the conversation if we have it - const conversation = ConversationController.get(ourNumber); - if (conversation) { - const newProfile = storage.getLocalProfile(); - conversation.setProfile(newProfile); - } - }, + onOk: newName => + conversation.setLokiProfile({ displayName: newName }), }); } }); diff --git a/js/conversation_controller.js b/js/conversation_controller.js index fe2a45074..04894042c 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -238,7 +238,7 @@ } promises.concat([ - conversation.updateProfile(), + conversation.updateProfileName(), conversation.updateProfileAvatar(), conversation.resetPendingSend(), conversation.setFriendRequestExpiryTimeout(), diff --git a/js/models/conversations.js b/js/models/conversations.js index 15d265021..076effe27 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -1988,9 +1988,9 @@ Conversation: Whisper.Conversation, }); - await this.updateProfile(); + await this.updateProfileName(); }, - async setProfile(profile) { + async setLokiProfile(profile) { if (!_.isEqual(this.get('profile'), profile)) { this.set({ profile }); await window.Signal.Data.updateConversation(this.id, this.attributes, { @@ -1998,18 +1998,18 @@ }); } - await this.updateProfile(); + await this.updateProfileName(); }, - async updateProfile() { + async updateProfileName() { // Prioritise nickname over the profile display name const nickname = this.getNickname(); - const profile = this.getLocalProfile(); + const profile = this.getLokiProfile(); const displayName = profile && profile.displayName; const profileName = nickname || displayName || null; await this.setProfileName(profileName); }, - getLocalProfile() { + getLokiProfile() { return this.get('profile'); }, getNickname() { @@ -2041,7 +2041,7 @@ const c = await ConversationController.getOrCreateAndWait(id, 'private'); // We only need to update the profile as they are all stored inside the conversation - await c.updateProfile(); + await c.updateProfileName(); }, async setProfileName(name) { const profileName = this.get('profileName'); diff --git a/js/models/messages.js b/js/models/messages.js index 4b4a9cd55..ed5ae4197 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1977,6 +1977,12 @@ } ); } + } else if (dataMessage.profile) { + ConversationController.getOrCreateAndWait(source, 'private').then( + sender => { + sender.setLokiProfile(dataMessage.profile); + } + ); } let autoAccept = false; diff --git a/js/models/profile.js b/js/models/profile.js deleted file mode 100644 index 16ab2f1da..000000000 --- a/js/models/profile.js +++ /dev/null @@ -1,56 +0,0 @@ -/* global storage, _ */ -/* global storage: false */ - -/* eslint-disable more/no-then */ - -// eslint-disable-next-line func-names -(function() { - 'use strict'; - - window.Whisper = window.Whisper || {}; - - const PROFILE_ID = 'local-profile'; - - storage.getLocalProfile = () => { - const profile = storage.get(PROFILE_ID, null); - return profile; - }; - - storage.setProfileName = async newName => { - if (typeof newName !== 'string' && newName !== null) { - throw Error('Name must be a string!'); - } - - // Update our profiles accordingly' - const trimmed = newName && newName.trim(); - - // If we get an empty name then unset the name property - // Otherwise update it - const profile = storage.getLocalProfile(); - const newProfile = { ...profile }; - if (_.isEmpty(trimmed)) { - delete newProfile.displayName; - } else { - newProfile.displayName = trimmed; - } - - await storage.saveLocalProfile(newProfile); - }; - - storage.saveLocalProfile = async profile => { - const storedProfile = storage.get(PROFILE_ID, null); - - // Only store the profile if we have a different object - if (storedProfile && _.isEqual(storedProfile, profile)) { - return; - } - - window.log.info('saving local profile ', profile); - await storage.put(PROFILE_ID, profile); - }; - - storage.removeLocalProfile = async () => { - window.log.info('removing local profile'); - await storage.remove(PROFILE_ID); - }; -})(); diff --git a/js/views/inbox_view.js b/js/views/inbox_view.js index c4f1dc7ff..4ac23f855 100644 --- a/js/views/inbox_view.js +++ b/js/views/inbox_view.js @@ -303,7 +303,7 @@ } if (conversation) { - conversation.updateProfile(); + conversation.updateProfileName(); } this.conversation_stack.open(conversation); diff --git a/libtextsecure/account_manager.js b/libtextsecure/account_manager.js index 7684605d6..3fc9060f8 100644 --- a/libtextsecure/account_manager.js +++ b/libtextsecure/account_manager.js @@ -11,7 +11,6 @@ dcodeIO, StringView, log, - storage, Event, ConversationController, Whisper @@ -538,7 +537,7 @@ saveMnemonic(mnemonic) { return textsecure.storage.put('mnemonic', mnemonic); }, - async registrationDone(number, profileName) { + async registrationDone(number, displayName) { window.log.info('registration done'); // Ensure that we always have a conversation for ourself @@ -546,12 +545,7 @@ number, 'private' ); - - await storage.setProfileName(profileName); - - // Update the conversation if we have it - const newProfile = storage.getLocalProfile(); - await conversation.setProfile(newProfile); + await conversation.setLokiProfile({ displayName }); this.dispatchEvent(new Event('registration')); }, diff --git a/libtextsecure/message_receiver.js b/libtextsecure/message_receiver.js index e10354443..b1dffd7c8 100644 --- a/libtextsecure/message_receiver.js +++ b/libtextsecure/message_receiver.js @@ -1206,7 +1206,7 @@ MessageReceiver.prototype.extend({ } // Update the conversation - await conversation.setProfile(profile); + await conversation.setLokiProfile(profile); } if (friendRequest && isMe) { diff --git a/libtextsecure/sendmessage.js b/libtextsecure/sendmessage.js index c2be038d8..dcf2b6500 100644 --- a/libtextsecure/sendmessage.js +++ b/libtextsecure/sendmessage.js @@ -819,7 +819,18 @@ MessageSender.prototype = { return message.toArrayBuffer(); }, - sendMessageToNumber( + getOurProfile() { + try { + const ourNumber = textsecure.storage.user.getNumber(); + const conversation = window.ConversationController.get(ourNumber); + return conversation.getLokiProfile(); + } catch (e) { + window.log.error(`Failed to get our profile: ${e}`); + return null; + } + }, + + async sendMessageToNumber( number, messageText, attachments, @@ -830,7 +841,7 @@ MessageSender.prototype = { profileKey, options ) { - const profile = textsecure.storage.impl.getLocalProfile(); + const profile = this.getOurProfile(); return this.sendMessage( { recipients: [number], @@ -942,7 +953,11 @@ MessageSender.prototype = { options ) { const me = textsecure.storage.user.getNumber(); - const numbers = groupNumbers.filter(number => number !== me); + let numbers = groupNumbers.filter(number => number !== me); + if (options.isPublic) { + numbers = [groupId]; + } + const profile = this.getOurProfile(); const attrs = { recipients: numbers, body: messageText, @@ -953,6 +968,7 @@ MessageSender.prototype = { needsSync: true, expireTimer, profileKey, + profile, group: { id: groupId, type: textsecure.protobuf.GroupContext.Type.DELIVER, diff --git a/test/index.html b/test/index.html index 61087bb86..7a3454115 100644 --- a/test/index.html +++ b/test/index.html @@ -530,7 +530,6 @@ - diff --git a/test/models/profile_test.js b/test/models/profile_test.js deleted file mode 100644 index 9f9db696a..000000000 --- a/test/models/profile_test.js +++ /dev/null @@ -1,108 +0,0 @@ -/* global storage */ - -/* eslint no-await-in-loop: 0 */ - -'use strict'; - -const PROFILE_ID = 'local-profile'; - -describe('Profile', () => { - beforeEach(async () => { - await clearDatabase(); - await storage.remove(PROFILE_ID); - }); - - describe('getLocalProfile', () => { - it('returns the local profile', async () => { - const values = [null, 'hello', { a: 'b' }]; - for (let i = 0; i < values.length; i += 1) { - await storage.put(PROFILE_ID, values[i]); - assert.strictEqual(values[i], storage.getLocalProfile()); - } - }); - }); - - describe('saveLocalProfile', () => { - it('saves a profile', async () => { - const values = [null, 'hello', { a: 'b' }]; - for (let i = 0; i < values.length; i += 1) { - await storage.saveLocalProfile(values[i]); - assert.strictEqual(values[i], storage.get(PROFILE_ID)); - } - }); - }); - - describe('removeLocalProfile', () => { - it('removes a profile', async () => { - await storage.saveLocalProfile('a'); - assert.strictEqual('a', storage.getLocalProfile()); - - await storage.removeLocalProfile(); - assert.strictEqual(null, storage.getLocalProfile()); - }); - }); - - describe('setProfileName', () => { - it('throws if a name is not a string', async () => { - const values = [0, { a: 'b' }, [1, 2]]; - for (let i = 0; i < values.length; i += 1) { - try { - await storage.setProfileName(values[i]); - assert.fail( - `setProfileName did not throw an error for ${typeof values[i]}` - ); - } catch (e) { - assert.throws(() => { - throw e; - }, 'Name must be a string!'); - } - } - }); - - it('does not throw if we pass a string or null', async () => { - const values = [null, '1']; - for (let i = 0; i < values.length; i += 1) { - try { - await storage.setProfileName(values[i]); - } catch (e) { - assert.fail('setProfileName threw an error'); - } - } - }); - - it('saves the display name', async () => { - await storage.setProfileName('hi there!'); - const profile = storage.getLocalProfile(); - assert.deepEqual(profile.displayName, 'hi there!'); - }); - - it('saves the display name without overwriting the other profile properties', async () => { - const profile = { title: 'hello' }; - await storage.put(PROFILE_ID, profile); - await storage.setProfileName('hi there!'); - - const expected = { - ...profile, - displayName: 'hi there!', - }; - assert.deepEqual(expected, storage.getLocalProfile()); - }); - - it('trims the display name', async () => { - await storage.setProfileName(' in middle '); - const profile = storage.getLocalProfile(); - assert.deepEqual('in middle', profile.displayName); - }); - - it('unsets the display name property if it is empty', async () => { - const profile = { - displayName: 'HI THERE!', - }; - await storage.put(PROFILE_ID, profile); - assert.exists(storage.getLocalProfile().displayName); - - await storage.setProfileName(''); - assert.notExists(storage.getLocalProfile().displayName); - }); - }); -});