Fetch all conversations on startup of app, not on inbox load (#1437)

* Fetch all conversations on startup of app, not on inbox load

A recent change to fetch conversations less didn't take into account all
that can happen in the app without the inbox loaded. That only happens
when the window is shown, and messages can come in with the app in the
background. In that case, the conversation wouldn't have been loaded
from the database, but would be saved to the database anyway, losing
data.

This change fetches all conversations as soon as the the data store is
ready for a fetch. It also introduces failsafe throws to ensure that
synchronous ConversationController accesses don't happen until the
initial fetch is complete. A new getUnsafe() method was required to
account for some of the model setup that happens during that initial
conversation fetch.

Fixes #1428

FREEBIE

* Fix tests: ConversationController.load() required before get()

FREEBIE
This commit is contained in:
Scott Nonnenberg 2017-09-06 18:18:46 -07:00 committed by GitHub
parent 8caecd50cd
commit 4cba16cb61
6 changed files with 82 additions and 24 deletions

View file

@ -72,6 +72,8 @@
storage.fetch();
storage.onready(function() {
ConversationController.load();
window.dispatchEvent(new Event('storage_ready'));
setUnreadCount(storage.get("unreadCount", 0));
@ -518,7 +520,7 @@
getAppView: function(destWindow) {
var self = this;
return ConversationController.updateInbox().then(function() {
return ConversationController.loadPromise().then(function() {
try {
if (self.inboxView) { self.inboxView.remove(); }
self.inboxView = new Whisper.InboxView({

View file

@ -83,12 +83,24 @@
window.ConversationController = {
get: function(id) {
if (!this._initialFetchComplete) {
throw new Error('ConversationController.get() needs complete initial fetch');
}
return conversations.get(id);
},
// Needed for some model setup which happens during the initial fetch() call below
getUnsafe: function(id) {
return conversations.get(id);
},
createTemporary: function(attributes) {
return conversations.add(attributes);
},
getOrCreate: function(id, type) {
if (!this._initialFetchComplete) {
throw new Error('ConversationController.get() needs complete initial fetch');
}
var conversation = conversations.get(id);
if (conversation) {
return conversation;
@ -114,17 +126,19 @@
return conversation;
},
getOrCreateAndWait: function(id, type) {
var conversation = this.getOrCreate(id, type);
return this._initialPromise.then(function() {
var conversation = this.getOrCreate(id, type);
if (conversation) {
return conversation.initialPromise.then(function() {
return conversation;
});
}
if (conversation) {
return conversation.initialPromise.then(function() {
return conversation;
});
}
return Promise.reject(
new Error('getOrCreateAndWait: did not get conversation')
);
return Promise.reject(
new Error('getOrCreateAndWait: did not get conversation')
);
}.bind(this));
},
getAllGroupsInvolvingId: function(id) {
var groups = new Whisper.GroupCollection();
@ -134,8 +148,30 @@
});
});
},
updateInbox: function() {
return conversations.fetch();
loadPromise: function() {
return this._initialPromise;
},
load: function() {
console.log('ConversationController: starting initial fetch');
if (this._initialPromise) {
throw new Error('ConversationController.load() has already been called!');
}
this._initialPromise = new Promise(function(resolve, reject) {
conversations.fetch().then(function() {
console.log('ConversationController: done with initial fetch');
this._initialFetchComplete = true;
resolve();
}.bind(this), function(error) {
console.log(
'ConversationController: initial fetch failed',
error && error.stack ? error.stack : error
);
reject(error);
});
}.bind(this));
return this._initialPromise;
}
};
})();

View file

@ -11,7 +11,10 @@
var expired = new Whisper.MessageCollection();
expired.on('add', function(message) {
console.log('message', message.get('sent_at'), 'expired');
message.getConversation().trigger('expired', message);
var conversation = message.getConversation();
if (conversation) {
conversation.trigger('expired', message);
}
// We delete after the trigger to allow the conversation time to process
// the expiration before the message is removed from the database.

View file

@ -147,7 +147,10 @@
return this.imageUrl;
},
getConversation: function() {
return ConversationController.get(this.get('conversationId'));
// This needs to be an unsafe call, because this method is called during
// initial module setup. We may be in the middle of the initial fetch to
// the database.
return ConversationController.getUnsafe(this.get('conversationId'));
},
getExpirationTimerUpdateSource: function() {
if (this.isExpirationTimerUpdate()) {
@ -235,7 +238,7 @@
someRecipientsFailed: function() {
var c = this.getConversation();
if (c.isPrivate()) {
if (!c || c.isPrivate()) {
return false;
}

View file

@ -11,7 +11,7 @@ describe("Fixtures", function() {
});
it('renders', function(done) {
ConversationController.updateInbox().then(function() {
ConversationController.loadPromise().then(function() {
var view = new Whisper.InboxView({window: window});
view.onEmpty();
view.$el.prependTo($('#render-android'));

View file

@ -4,11 +4,13 @@
(function () {
'use strict';
function clear(done) {
var messages = new Whisper.MessageCollection();
return messages.fetch().then(function() {
messages.destroyAll();
done();
function deleteAllMessages() {
return new Promise(function(resolve, reject) {
var messages = new Whisper.MessageCollection();
return messages.fetch().then(function() {
messages.destroyAll();
resolve();
}, reject);
});
}
@ -21,9 +23,18 @@
var attachment = { data: 'datasaurus',
contentType: 'plain/text' };
var source = '+14155555555';
describe('MessageCollection', function() {
before(clear);
after(clear);
before(function() {
return Promise.all([
deleteAllMessages(),
ConversationController.load()
]);
});
after(function() {
return deleteAllMessages();
});
it('has no image url', function() {
var messages = new Whisper.MessageCollection();
@ -49,7 +60,10 @@
it('gets incoming contact', function() {
var messages = new Whisper.MessageCollection();
var message = messages.add({ type: 'incoming' });
var message = messages.add({
type: 'incoming',
source: source
});
message.getContact();
});