From c4dd10a5cdaff64e3d62e6201ad41d37a1516d8d Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 30 Aug 2019 09:42:13 +1000 Subject: [PATCH 1/2] Simplified profile setting. Renamed functions to be less confusing. Removed storing profile in local storage. --- js/background.js | 20 +++--- js/conversation_controller.js | 2 +- js/models/conversations.js | 14 ++-- js/models/messages.js | 2 +- js/models/profile.js | 56 ---------------- js/views/inbox_view.js | 2 +- libtextsecure/account_manager.js | 10 +-- libtextsecure/message_receiver.js | 2 +- libtextsecure/sendmessage.js | 17 ++++- test/models/profile_test.js | 108 ------------------------------ 10 files changed, 35 insertions(+), 198 deletions(-) delete mode 100644 js/models/profile.js delete mode 100644 test/models/profile_test.js diff --git a/js/background.js b/js/background.js index a791a9ccc..26e203ef9 100644 --- a/js/background.js +++ b/js/background.js @@ -671,25 +671,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 50b3aae33..3f83d6b61 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -242,7 +242,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 4b674503e..6b398ccb0 100644 --- a/js/models/conversations.js +++ b/js/models/conversations.js @@ -2020,9 +2020,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, { @@ -2030,18 +2030,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() { @@ -2111,7 +2111,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 b79307473..70e474c0b 100644 --- a/js/models/messages.js +++ b/js/models/messages.js @@ -1990,7 +1990,7 @@ } else if (dataMessage.profile) { ConversationController.getOrCreateAndWait(source, 'private').then( sender => { - sender.setProfile(dataMessage.profile); + sender.setLokiProfile(dataMessage.profile); } ); } 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 e1bb7ef69..16c613c8a 100644 --- a/js/views/inbox_view.js +++ b/js/views/inbox_view.js @@ -302,7 +302,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 219d8bb7a..98eab0038 100644 --- a/libtextsecure/account_manager.js +++ b/libtextsecure/account_manager.js @@ -10,7 +10,6 @@ dcodeIO, StringView, log, - storage, Event, ConversationController */ @@ -527,7 +526,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 @@ -535,12 +534,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 a65955bed..1d1cebcf8 100644 --- a/libtextsecure/message_receiver.js +++ b/libtextsecure/message_receiver.js @@ -1074,7 +1074,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 b5b608f20..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], @@ -946,7 +957,7 @@ MessageSender.prototype = { if (options.isPublic) { numbers = [groupId]; } - const profile = textsecure.storage.impl.getLocalProfile(); + const profile = this.getOurProfile(); const attrs = { recipients: numbers, body: messageText, 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); - }); - }); -}); From 83d198108c294151425021290ae9d211732c5ceb Mon Sep 17 00:00:00 2001 From: Mikunj Date: Fri, 30 Aug 2019 10:02:35 +1000 Subject: [PATCH 2/2] Fix the ui bug where items would appear over the loki logo. --- stylesheets/_modules.scss | 1 + 1 file changed, 1 insertion(+) diff --git a/stylesheets/_modules.scss b/stylesheets/_modules.scss index ee97bdf7a..1f433c904 100644 --- a/stylesheets/_modules.scss +++ b/stylesheets/_modules.scss @@ -3264,6 +3264,7 @@ .module-left-pane__list { flex-grow: 1; flex-shrink: 1; + overflow-y: auto; } .module-left-pane__virtual-list {