From 83c979fb84fbc4750f7a70411bc872668f58f53d Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 27 Mar 2018 18:58:00 -0400 Subject: [PATCH 1/4] Rename `createTemporary` to `dangerouslyCreateAndAdd` Class: `ConversationController`. This function should not be used in application code as it creates potentially invalid `Conversation` instances in our global conversation collection. We keep making it available for testing purposes. --- js/conversation_controller.js | 2 +- js/views/conversation_search_view.js | 2 +- test/keychange_listener_test.js | 15 +++++++++++---- test/views/message_view_test.js | 2 +- 4 files changed, 14 insertions(+), 7 deletions(-) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index 54719049b3..a8bae85115 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -95,7 +95,7 @@ getUnsafe: function(id) { return conversations.get(id); }, - createTemporary: function(attributes) { + dangerouslyCreateAndAdd: function(attributes) { return conversations.add(attributes); }, getOrCreate: function(id, type) { diff --git a/js/views/conversation_search_view.js b/js/views/conversation_search_view.js index cb34e94a68..15649e9330 100644 --- a/js/views/conversation_search_view.js +++ b/js/views/conversation_search_view.js @@ -92,7 +92,7 @@ // Creates a view to display a new contact this.new_contact_view = new Whisper.NewContactView({ el: this.$new_contact, - model: ConversationController.createTemporary({ + model: ConversationController.dangerouslyCreateAndAdd({ type: 'private', }), }).render(); diff --git a/test/keychange_listener_test.js b/test/keychange_listener_test.js index 3e8182fd04..055a8bb7fa 100644 --- a/test/keychange_listener_test.js +++ b/test/keychange_listener_test.js @@ -16,9 +16,12 @@ describe('KeyChangeListener', function() { }); describe('When we have a conversation with this contact', function() { - var convo = new Whisper.Conversation({ id: phoneNumberWithKeyChange, type: 'private'}); + let convo; before(function() { - ConversationController.createTemporary(convo); + convo = ConversationController.dangerouslyCreateAndAdd({ + id: phoneNumberWithKeyChange, + type: 'private', + }); return convo.save(); }); @@ -41,9 +44,13 @@ describe('KeyChangeListener', function() { describe('When we have a group with this contact', function() { - var convo = new Whisper.Conversation({ id: 'groupId', type: 'group', members: [phoneNumberWithKeyChange] }); + let convo; before(function() { - ConversationController.createTemporary(convo); + convo = ConversationController.dangerouslyCreateAndAdd({ + id: 'groupId', + type: 'group', + members: [phoneNumberWithKeyChange], + }); return convo.save(); }); after(function() { diff --git a/test/views/message_view_test.js b/test/views/message_view_test.js index 8c40ba33fa..1383c73e51 100644 --- a/test/views/message_view_test.js +++ b/test/views/message_view_test.js @@ -2,7 +2,7 @@ describe('MessageView', function() { var convo, message; before(function() { - convo = ConversationController.createTemporary({id: 'foo'}); + convo = ConversationController.dangerouslyCreateAndAdd({id: 'foo'}); message = convo.messageCollection.add({ conversationId: convo.id, body: 'hello world', From 08f6886f3e2e45d16fa2bea266ee3c5d987d6f91 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 27 Mar 2018 19:00:25 -0400 Subject: [PATCH 2/4] Strengthen precondition of `ConversationController.getOrCreate` --- js/conversation_controller.js | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/js/conversation_controller.js b/js/conversation_controller.js index a8bae85115..05c6dc6584 100644 --- a/js/conversation_controller.js +++ b/js/conversation_controller.js @@ -99,6 +99,14 @@ return conversations.add(attributes); }, getOrCreate: function(id, type) { + if (typeof id !== 'string') { + throw new TypeError('"id" must be a string'); + } + + if (type !== 'private' && type !== 'group') { + throw new TypeError('"type" must be "private" or "group"; got: ' + type); + } + if (!this._initialFetchComplete) { throw new Error('ConversationController.get() needs complete initial fetch'); } From d6ea158e46f0a98edb5d664111bf593ec86b00d0 Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 27 Mar 2018 19:02:00 -0400 Subject: [PATCH 3/4] Avoid `dangerouslyCreateAndAdd` in `MessageView` test --- test/views/message_view_test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/test/views/message_view_test.js b/test/views/message_view_test.js index 1383c73e51..25e74e18ee 100644 --- a/test/views/message_view_test.js +++ b/test/views/message_view_test.js @@ -2,7 +2,7 @@ describe('MessageView', function() { var convo, message; before(function() { - convo = ConversationController.dangerouslyCreateAndAdd({id: 'foo'}); + convo = new Whisper.Conversation({id: 'foo'}); message = convo.messageCollection.add({ conversationId: convo.id, body: 'hello world', From b24dad23ea4eb414eed90a3ffbff86b33bb029ca Mon Sep 17 00:00:00 2001 From: Daniel Gasienica Date: Tue, 27 Mar 2018 19:04:55 -0400 Subject: [PATCH 4/4] Fix search view conversation reset bug MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When searching for an existing conversation using a phone number, it’s possible to click on ‘Start conversation…’ and have that new dummy entry overwrite the existing conversation. This change ensures we are always showing a dummy conversation model that is not part of the conversation collection. Adding it is always idempotent as it goes through `getOrCreateAndWait`. --- Gruntfile.js | 1 + js/views/conversation_search_view.js | 31 ++++++++++++---------------- 2 files changed, 14 insertions(+), 18 deletions(-) diff --git a/Gruntfile.js b/Gruntfile.js index efae449abe..78cba0c41a 100644 --- a/Gruntfile.js +++ b/Gruntfile.js @@ -106,6 +106,7 @@ module.exports = function(grunt) { '!js/logging.js', '!js/backup.js', '!js/modules/**/*.js', + '!js/views/conversation_search_view.js', '!js/views/debug_log_view.js', '!js/signal_protocol_store.js', '!js/database.js', diff --git a/js/views/conversation_search_view.js b/js/views/conversation_search_view.js index 15649e9330..7a0cbb1143 100644 --- a/js/views/conversation_search_view.js +++ b/js/views/conversation_search_view.js @@ -89,32 +89,27 @@ this.new_contact_view.undelegateEvents(); this.new_contact_view.$el.hide(); } - // Creates a view to display a new contact + const model = new Whisper.Conversation({ type: 'private' }); this.new_contact_view = new Whisper.NewContactView({ el: this.$new_contact, - model: ConversationController.dangerouslyCreateAndAdd({ - type: 'private', - }), + model, }).render(); }, - createConversation() { - if (this.new_contact_view.model.isValid()) { - // NOTE: Temporarily allow `then` until we convert the entire file - // to `async` / `await`: - // eslint-disable-next-line more/no-then - ConversationController.getOrCreateAndWait( - this.new_contact_view.model.id, - 'private' - ).then((conversation) => { - this.trigger('open', conversation); - this.initNewContact(); - this.resetTypeahead(); - }); - } else { + async createConversation() { + const isValidNumber = this.new_contact_view.model.isValid(); + if (!isValidNumber) { this.new_contact_view.$('.number').text(i18n('invalidNumberError')); this.$input.focus(); + return; } + + const newConversationId = this.new_contact_view.model.id; + const conversation = + await ConversationController.getOrCreateAndWait(newConversationId, 'private'); + this.trigger('open', conversation); + this.initNewContact(); + this.resetTypeahead(); }, reset() {