From c13ee1dd615265caf1aa23e69480c0c0df3d1c37 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Tue, 6 Sep 2016 22:47:15 +0200 Subject: [PATCH 1/8] remove client-side cache because we can simply keep the data in memory --- js/app.js | 3 - js/background.js | 12 +- js/cache.js | 250 ----------------------------- js/controller/foldercontroller.js | 14 +- js/controller/messagecontroller.js | 15 +- js/models/folder.js | 3 +- js/models/message.js | 3 +- js/service/accountservice.js | 1 - js/service/messageservice.js | 86 ++++------ js/state.js | 18 --- js/views/account.js | 3 - js/views/foldercontent.js | 10 +- js/views/messagesitem.js | 5 - js/views/messagesview.js | 4 +- 14 files changed, 39 insertions(+), 388 deletions(-) delete mode 100644 js/cache.js diff --git a/js/app.js b/js/app.js index 1fdd24b696..2f958b50bc 100644 --- a/js/app.js +++ b/js/app.js @@ -31,7 +31,6 @@ define(function(require) { var Marionette = require('marionette'); var OC = require('OC'); var AppView = require('views/appview'); - var Cache = require('cache'); var Radio = require('radio'); var Router = require('router'); var AccountController = require('controller/accountcontroller'); @@ -84,8 +83,6 @@ define(function(require) { Mail.on('start', function() { this.view = new AppView(); - Cache.init(); - Radio.ui.trigger('content:loading'); this.registerProtocolHandler(); diff --git a/js/background.js b/js/background.js index 71deac0533..ec88a42cdd 100644 --- a/js/background.js +++ b/js/background.js @@ -16,7 +16,6 @@ define(function(require) { var _ = require('underscore'); var $ = require('jquery'); var OC = require('OC'); - var Cache = require('cache'); var Radio = require('radio'); var State = require('state'); var MessageCollection = require('models/messagecollection'); @@ -131,15 +130,8 @@ define(function(require) { if (State.currentAccount === changedAccount && State.currentFolder.get('id') === changes.id) { Radio.ui.request('messagesview:collection').add(changes.messages); - var messages = new MessageCollection(changes.messages).slice(0); - Radio.message.trigger('fetch:bodies', changedAccount, changedFolder, messages); - } - - // Save new messages to the cached message list - var cachedList = Cache.getMessageList(changedAccount, localFolder); - if (cachedList) { - cachedList = cachedList.concat(changes.messages); - Cache.addMessageList(changedAccount, localFolder, cachedList); + // var messages = new MessageCollection(changes.messages).slice(0); + // Radio.message.trigger('fetch:bodies', changedAccount, changedFolder, messages); } Radio.ui.trigger('title:update'); diff --git a/js/cache.js b/js/cache.js deleted file mode 100644 index 3acddc820d..0000000000 --- a/js/cache.js +++ /dev/null @@ -1,250 +0,0 @@ -/** - * Mail - * - * This file is licensed under the Affero General Public License version 3 or - * later. See the COPYING file. - * - * @author Christoph Wurst - * @copyright Christoph Wurst 2015, 2016 - */ - -define(function(require) { - 'use strict'; - - var _ = require('underscore'); - var $ = require('jquery'); - var storage = $.sessionStorage; - var Account = require('models/account'); - - var MessageCache = { - /** - * @param {Account} account - * @returns {string} - */ - getAccountPath: function(account) { - return ['messages', account.get('accountId').toString()].join('.'); - }, - /** - * @param {Account} account - * @param {Folder} folder - * @returns {string} - */ - getFolderPath: function(account, folder) { - return [this.getAccountPath(account), folder.get('id').toString()].join('.'); - }, - /** - * @param {Account} account - * @param {Folder} folder - * @param {number} messageId - * @returns {string} - */ - getMessagePath: function(account, folder, messageId) { - return [this.getFolderPath(account, folder), messageId.toString()].join('.'); - } - }; - - var FolderCache = { - /** - * @param {Account} account - * @returns {string} - */ - getAccountPath: function(account) { - return ['folders', account.get('accountId').toString()].join('.'); - }, - /** - * @param {Account} account - * @param {Folder} folder - * @returns {string} - */ - getFolderPath: function(account, folder) { - return [this.getAccountPath(account), folder.get('id').toString()].join('.'); - } - }; - - function init() { - console.log('initializing cache…'); - var installedVersion = $('#config-installed-version').val(); - if (storage.isSet('mail-app-version')) { - var cachedVersion = storage.get('mail-app-version'); - if (cachedVersion !== installedVersion) { - console.log('clearing cache because app version has changed'); - storage.removeAll(); - } - } else { - // Could be an old version -> clear data - storage.removeAll(); - } - storage.set('mail-app-version', installedVersion); - } - - /** - * @param {AccountsCollection} accounts - * @returns {undefined} - */ - function cleanUp(accounts) { - var activeAccounts = accounts.map(function(account) { - return account.get('accountId'); - }); - _.each(storage.get('messages'), function(account, accountId) { - var isActive = _.any(activeAccounts, function(a) { - return a === parseInt(accountId); - }); - if (!isActive) { - // Account does not exist anymore -> remove it - storage.remove('messages.' + accountId); - } - }); - } - - /** - * @param {Account} account - * @param {Folder} folder - * @returns {unresolved} - */ - function getFolderMessages(account, folder) { - var path = MessageCache.getFolderPath(account, folder); - return storage.isSet(path) ? storage.get(path) : null; - } - - /** - * @param {Account} account - * @param {Folder} folder - * @param {Message} messageId - * @returns {unresolved} - */ - function getMessage(account, folder, messageId) { - var path = MessageCache.getMessagePath(account, folder, messageId); - if (storage.isSet(path)) { - var message = storage.get(path); - // Update the timestamp - addMessage(account, folder, message); - return message; - } else { - return null; - } - } - - /** - * @param {Account} account - * @param {Folder} folder - * @param {Message} message - * @returns {undefined} - */ - function addMessage(account, folder, message) { - var path = MessageCache.getMessagePath(account, folder, message.id); - // Add timestamp for later cleanup - message.timestamp = Date.now(); - - // Save the message to local storage - storage.set(path, message); - - // Remove old messages (keep 20 most recently loaded) - var messages = $.map(getFolderMessages(account, folder), function(value) { - return [value]; - }); - messages.sort(function(m1, m2) { - return m2.timestamp - m1.timestamp; - }); - var oldMessages = messages.slice(20, messages.length); - _.each(oldMessages, function(message) { - storage.remove(MessageCache.getMessagePath(account, folder, message.id)); - }); - } - - /** - * @param {Account} account - * @param {Folder} folder - * @param {Message} messages - * @returns {undefined} - */ - function addMessages(account, folder, messages) { - _.each(messages, function(message) { - addMessage(account, folder, message); - }); - } - - /** - * @param {Account} account - * @param {Folder} folder - * @param {number} messageId - * @returns {undefined} - */ - function removeMessage(account, folder, messageId) { - var message = getMessage(account, folder, messageId); - if (message) { - // message exists in cache -> remove it - storage.remove(MessageCache.getMessagePath(account, folder, messageId)); - } - var messageList = getMessageList(account, folder); - if (messageList) { - // message list is cached -> remove message from it - var newList = _.filter(messageList, function(message) { - return message.id !== messageId; - }); - addMessageList(account, folder, newList); - } - } - - /** - * @param {Account} account - * @param {Folder} folder - * @returns {unresolved} - */ - function getMessageList(account, folder) { - var path = FolderCache.getFolderPath(account, folder); - if (storage.isSet(path)) { - return storage.get(path); - } else { - return null; - } - } - - /** - * @param {Account} account - * @param {Folder} folder - * @param {type} messages - * @returns {undefined} - */ - function addMessageList(account, folder, messages) { - var path = FolderCache.getFolderPath(account, folder); - storage.set(path, messages); - } - - /** - * @param {Account} account - * @returns {undefined} - */ - function removeAccount(account) { - // Remove cached message lists - var path = FolderCache.getAccountPath(account); - if (storage.isSet(path)) { - storage.remove(path); - } - - // Remove cached messages - path = MessageCache.getAccountPath(account); - if (storage.isSet(path)) { - storage.remove(path); - } - - // Unified inbox hack - if (account.get('accountId') !== -1) { - // Make sure unified inbox cache is cleared to prevent - // old message showing up on the next load - removeAccount(new Account({accountId: -1})); - } - // End unified inbox hack - } - - return { - init: init, - cleanUp: cleanUp, - getMessage: getMessage, - addMessage: addMessage, - addMessages: addMessages, - removeMessage: removeMessage, - getMessageList: getMessageList, - addMessageList: addMessageList, - removeAccount: removeAccount - }; -}); diff --git a/js/controller/foldercontroller.js b/js/controller/foldercontroller.js index 900d9c762d..5aa45f9f8a 100644 --- a/js/controller/foldercontroller.js +++ b/js/controller/foldercontroller.js @@ -50,13 +50,6 @@ define(function(require) { function loadFolderMessages(account, folder, noSelect, searchQuery) { Radio.ui.trigger('composer:leave'); - if (require('state').messagesLoading !== null) { - require('state').messagesLoading.abort(); - } - if (require('state').messageLoading !== null) { - require('state').messageLoading.abort(); - } - // Set folder active Radio.folder.trigger('setactive', account, folder); @@ -71,8 +64,7 @@ define(function(require) { } else { var loadingMessages = Radio.message.request('entities', account, folder, { cache: true, - filter: searchQuery, - replace: true + filter: searchQuery }); $.when(loadingMessages).done(function(messages, cached) { @@ -91,7 +83,7 @@ define(function(require) { if (messages.length > 0) { // Fetch first 10 messages in background - Radio.message.trigger('fetch:bodies', account, folder, messages.slice(0, 10)); + // Radio.message.trigger('fetch:bodies', account, folder, messages.slice(0, 10)); Radio.message.trigger('load', account, folder, messages.first()); @@ -168,7 +160,7 @@ define(function(require) { }); var fetchingMessageBodies = Radio.message.request('bodies', account, folder, ids); $.when(fetchingMessageBodies).done(function(messages) { - require('cache').addMessages(account, folder, messages); + // TODO: merge attributes }); } } diff --git a/js/controller/messagecontroller.js b/js/controller/messagecontroller.js index 1bc574c872..929be4e709 100644 --- a/js/controller/messagecontroller.js +++ b/js/controller/messagecontroller.js @@ -52,15 +52,6 @@ define(function(require) { Radio.ui.trigger('composer:leave'); - // TODO: expression is useless? - if (!options.force && false) { - return; - } - // Abort previous loading requests - if (require('state').messageLoading !== null) { - require('state').messageLoading.abort(); - } - // check if message is a draft var draftsFolder = account.get('specialFolders').drafts; var draft = draftsFolder === require('state').currentFolder.get('id'); @@ -87,16 +78,12 @@ define(function(require) { var fetchingMessage = Radio.message.request('entity', require('state').currentAccount, require('state').currentFolder, - message.get('id')); + message); $.when(fetchingMessage).done(function(message) { if (draft) { Radio.ui.trigger('composer:show', message); } else { - // TODO: ideally this should be handled in messageservice.js - require('cache').addMessage(require('state').currentAccount, - require('state').currentFolder, - message); Radio.ui.trigger('message:show', message); } }); diff --git a/js/models/folder.js b/js/models/folder.js index bf44dcfcf9..d96f05fffa 100644 --- a/js/models/folder.js +++ b/js/models/folder.js @@ -20,7 +20,8 @@ define(function(require) { defaults: { open: false, folders: [], - messages: [] + messages: [], + messagesLoaded: false }, initialize: function() { var FolderCollection = require('models/foldercollection'); diff --git a/js/models/message.js b/js/models/message.js index 4fb4768fcf..556be2a107 100644 --- a/js/models/message.js +++ b/js/models/message.js @@ -20,7 +20,8 @@ define(function(require) { var Message = Backbone.Model.extend({ defaults: { flags: [], - active: false + active: false, + hasDetails: false, }, initialize: function() { this.set('flags', new MessageFlags(this.get('flags'))); diff --git a/js/service/accountservice.js b/js/service/accountservice.js index 043e142940..d8a261de6d 100644 --- a/js/service/accountservice.js +++ b/js/service/accountservice.js @@ -62,7 +62,6 @@ define(function(require) { } else { accounts.fetch({ success: function(accounts) { - require('cache').cleanUp(accounts); defer.resolve(accounts); }, error: function() { diff --git a/js/service/messageservice.js b/js/service/messageservice.js index 068a862f3d..772a39f7af 100644 --- a/js/service/messageservice.js +++ b/js/service/messageservice.js @@ -24,7 +24,6 @@ define(function(require) { var _ = require('underscore'); var OC = require('OC'); var Radio = require('radio'); - var messageListXhr = null; Radio.message.reply('entities', getMessageEntities); Radio.message.reply('entity', getMessageEntity); @@ -43,37 +42,15 @@ define(function(require) { options = options || {}; var defaults = { cache: false, - replace: false, // Replace cached folder list force: false, filter: '' }; _.defaults(options, defaults); - // Do not cache search queries - if (options.filter !== '') { - options.cache = false; - } - - // Abort previous requests - if (messageListXhr !== null) { - messageListXhr.abort(); - } - var defer = $.Deferred(); - - if (options.cache) { - // Load cached version if available - var messageList = require('cache').getMessageList(account, folder); - if (!options.force && messageList) { - var collection = folder.get('messages'); - _.each(messageList, function(msg) { - collection.add(msg); - }); - defer.resolve(collection, true); - return defer.promise(); - } + if (folder.get('messagesLoaded')) { + return defer.resolve(folder.get('messages'), true); } - var url = OC.generateUrl('apps/mail/accounts/{accountId}/folders/{folderId}/messages', { accountId: account.get('accountId'), @@ -81,7 +58,7 @@ define(function(require) { }); // TODO: folder.get('messages').fetch() - messageListXhr = $.ajax(url, + $.ajax(url, { data: { from: options.from, @@ -89,16 +66,13 @@ define(function(require) { filter: options.filter }, success: function(messages) { - if (options.replace || options.cache) { - require('cache').addMessageList(account, folder, messages); - } var collection = folder.get('messages'); - if (options.replace) { - collection.reset(); - } _.each(messages, function(msg) { - collection.add(msg); + collection.add(msg, { + merge: true + }); }); + folder.set('messagesLoaded', true); defer.resolve(collection, false); }, error: function(error, status) { @@ -114,36 +88,32 @@ define(function(require) { /** * @param {Account} account * @param {Folder} folder - * @param {number} messageId + * @param {Message} messageId * @param {object} options * @returns {Promise} */ - function getMessageEntity(account, folder, messageId, options) { + function getMessageEntity(account, folder, message, options) { options = options || {}; - var defaults = { - backgroundMode: false - }; - _.defaults(options, defaults); var defer = $.Deferred(); - - // Load cached version if available - var message = require('cache').getMessage(account, - folder, - messageId); - if (message) { - defer.resolve(message); - return defer.promise(); + + if (message.get('hasDetails')) { + return message; } var url = OC.generateUrl('apps/mail/accounts/{accountId}/folders/{folderId}/messages/{messageId}', { accountId: account.get('accountId'), folderId: folder.get('id'), - messageId: messageId + messageId: message.get('id') }); - var xhr = $.ajax(url, { + $.ajax(url, { type: 'GET', - success: function(message) { + success: function(messageDetails) { + // do not override nested Backbone model 'flags' + delete messageDetails.flags; + + message.set(messageDetails); + message.set('hasDetails', true); defer.resolve(message); }, error: function(jqXHR, textStatus) { @@ -152,10 +122,6 @@ define(function(require) { } } }); - if (!options.backgroundMode) { - // Save xhr to allow aborting unneeded requests - require('state').messageLoading = xhr; - } return defer.promise(); } @@ -169,13 +135,14 @@ define(function(require) { function fetchMessageBodies(account, folder, messageIds) { var defer = $.Deferred(); - var cachedMessages = []; var uncachedIds = []; _.each(messageIds, function(messageId) { - var message = require('cache').getMessage(account, folder, messageId); - if (message) { - cachedMessages.push(message); - } else { + var message = folder.get('messages').get(messageId); + if (!message) { + // Weird, shouldn't have happened, but let's ignore this one + return; + } + if (!message.get('hasDetails')) { uncachedIds.push(messageId); } }); @@ -190,6 +157,7 @@ define(function(require) { $.ajax(url, { type: 'GET', success: function(data) { + // TODO: merge attributes defer.resolve(data); }, error: function() { diff --git a/js/state.js b/js/state.js index c345880cff..94a16e9fb7 100644 --- a/js/state.js +++ b/js/state.js @@ -22,8 +22,6 @@ define(function(require) { var currentMessage = null; var currentMessageSubject = null; var currentMessageBody = ''; - var messagesLoading = null; - var messageLoading = null; Object.defineProperties(state, { accounts: { @@ -77,22 +75,6 @@ define(function(require) { set: function(body) { currentMessageBody = body; } - }, - messagesLoading: { - get: function() { - return messagesLoading; - }, - set: function(loading) { - messagesLoading = loading; - } - }, - messageLoading: { - get: function() { - return messageLoading; - }, - set: function(loading) { - messageLoading = loading; - } } }); diff --git a/js/views/account.js b/js/views/account.js index 29ab7b3987..c74ae01b42 100644 --- a/js/views/account.js +++ b/js/views/account.js @@ -90,9 +90,6 @@ define(function(require) { data: {accountId: account.get('accountId')}, type: 'DELETE', success: function() { - // Delete cached message lists - require('cache').removeAccount(account); - // reload the complete page // TODO should only reload the app nav/content window.location.reload(); diff --git a/js/views/foldercontent.js b/js/views/foldercontent.js index ab593354b6..9a55d6c6b3 100644 --- a/js/views/foldercontent.js +++ b/js/views/foldercontent.js @@ -79,11 +79,10 @@ define(function(require) { // Temporarily disable new-message composer events Radio.ui.trigger('composer:events:undelegate'); - var messageModel = new Backbone.Model(message); this.message.show(new MessageView({ account: this.account, folder: this.folder, - model: messageModel + model: message })); this.detailView = DetailView.MESSAGE; @@ -94,13 +93,6 @@ define(function(require) { $('#mail_new_message').prop('disabled', true); $('#mail-message').removeClass('hidden-mobile'); - // Abort message loads - if (require('state').messageLoading !== null) { - require('state').messageLoading.abort(); - $('iframe').parent().removeClass('icon-loading'); - $('#mail_message').removeClass('icon-loading'); - } - // setup composer view this.message.show(new ComposerView({ accounts: require('state').accounts, diff --git a/js/views/messagesitem.js b/js/views/messagesitem.js index 9e2b192967..31cd961220 100644 --- a/js/views/messagesitem.js +++ b/js/views/messagesitem.js @@ -127,11 +127,6 @@ define(function(require) { }), { data: {}, type: 'DELETE', - success: function() { - var cache = require('cache'); - var state = require('state'); - cache.removeMessage(state.currentAccount, state.currentFolder, thisModel.id); - }, error: function() { Radio.ui.trigger('error:show', t('mail', 'Error while deleting message.')); } diff --git a/js/views/messagesview.js b/js/views/messagesview.js index e8501c5905..069758dfd2 100644 --- a/js/views/messagesview.js +++ b/js/views/messagesview.js @@ -222,9 +222,7 @@ define(function(require) { from: from, to: from + 20, force: true, - filter: this.searchQuery || '', - // Replace cached message list on reload - replace: reload + filter: this.searchQuery || '' }); $.when(loadingMessages).done(function() { From 8c222b764143e1524baf0992d40c964c0f973bd8 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 18:53:54 +0200 Subject: [PATCH 2/8] fix infinite scroll + message list refreshing --- js/background.js | 1 - js/controller/foldercontroller.js | 4 ++-- js/service/messageservice.js | 7 +++---- js/views/messagesview.js | 2 +- 4 files changed, 6 insertions(+), 8 deletions(-) diff --git a/js/background.js b/js/background.js index ec88a42cdd..61c021be73 100644 --- a/js/background.js +++ b/js/background.js @@ -18,7 +18,6 @@ define(function(require) { var OC = require('OC'); var Radio = require('radio'); var State = require('state'); - var MessageCollection = require('models/messagecollection'); /*jshint maxparams: 6 */ function showNotification(title, body, tag, icon, account, folder) { diff --git a/js/controller/foldercontroller.js b/js/controller/foldercontroller.js index 5aa45f9f8a..57bde6ac5b 100644 --- a/js/controller/foldercontroller.js +++ b/js/controller/foldercontroller.js @@ -63,8 +63,8 @@ define(function(require) { require('state').currentlyLoading = null; } else { var loadingMessages = Radio.message.request('entities', account, folder, { - cache: true, - filter: searchQuery + filter: searchQuery, + cache: true }); $.when(loadingMessages).done(function(messages, cached) { diff --git a/js/service/messageservice.js b/js/service/messageservice.js index 772a39f7af..6c92125842 100644 --- a/js/service/messageservice.js +++ b/js/service/messageservice.js @@ -41,14 +41,13 @@ define(function(require) { function getMessageEntities(account, folder, options) { options = options || {}; var defaults = { - cache: false, - force: false, - filter: '' + filter: '', + cache: false // Do *not* returned a cached version immediately }; _.defaults(options, defaults); var defer = $.Deferred(); - if (folder.get('messagesLoaded')) { + if (options.cache && folder.get('messagesLoaded')) { return defer.resolve(folder.get('messages'), true); } var url = OC.generateUrl('apps/mail/accounts/{accountId}/folders/{folderId}/messages', diff --git a/js/views/messagesview.js b/js/views/messagesview.js index 069758dfd2..4bc0d306f6 100644 --- a/js/views/messagesview.js +++ b/js/views/messagesview.js @@ -221,7 +221,7 @@ define(function(require) { { from: from, to: from + 20, - force: true, + cache: false, filter: this.searchQuery || '' }); From d6530d848cf61b14e6ad9889988948d1fe995363 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 19:17:45 +0200 Subject: [PATCH 3/8] when reloading the message list, we have to remove any messages that were not in the response --- js/service/messageservice.js | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/js/service/messageservice.js b/js/service/messageservice.js index 6c92125842..dc5ecefd34 100644 --- a/js/service/messageservice.js +++ b/js/service/messageservice.js @@ -66,11 +66,17 @@ define(function(require) { }, success: function(messages) { var collection = folder.get('messages'); + var messageIds = []; _.each(messages, function(msg) { + messageIds.push(msg.id); collection.add(msg, { merge: true }); }); + if (options.from === 0) { + // Reloading + cleanUpCollection(collection, messageIds); + } folder.set('messagesLoaded', true); defer.resolve(collection, false); }, @@ -84,6 +90,22 @@ define(function(require) { return defer.promise(); } + function cleanUpCollection(collection, ids) { + var toRemove = []; + collection.forEach(function(message) { + if (ids.indexOf(message.get('id')) === -1) { + // Message was removed, so le't remove it + // from the client collection too + // TODO: use Backbone+horde sync and don't + // discard the data + toRemove.push(message.get('id')); + } + }); + _.each(toRemove, function(id) { + collection.remove(id); + }); + } + /** * @param {Account} account * @param {Folder} folder @@ -95,7 +117,7 @@ define(function(require) { options = options || {}; var defer = $.Deferred(); - + if (message.get('hasDetails')) { return message; } From 35081b05f20c37f251d71c1af624f85ec58cd39f Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 19:53:51 +0200 Subject: [PATCH 4/8] fix new-message-button and fetch incoming messages in the background --- js/background.js | 10 +++++++-- js/controller/foldercontroller.js | 8 +------- js/service/messageservice.js | 34 ++++++++++++++++--------------- js/views/newmessage.js | 8 ++++---- 4 files changed, 31 insertions(+), 29 deletions(-) diff --git a/js/background.js b/js/background.js index 61c021be73..552f8e3751 100644 --- a/js/background.js +++ b/js/background.js @@ -18,6 +18,8 @@ define(function(require) { var OC = require('OC'); var Radio = require('radio'); var State = require('state'); + var Message = require('models/message'); + var MessageCollection = require('models/messagecollection'); /*jshint maxparams: 6 */ function showNotification(title, body, tag, icon, account, folder) { @@ -125,12 +127,16 @@ define(function(require) { localFolder.set('unseen', changes.unseen); localFolder.set('total', changes.total); + var messages = _.map(changes.messages, function(messageData) { + return new Message(messageData); + }); + changedFolder.get('messages').add(messages); + // reload if current selected folder has changed if (State.currentAccount === changedAccount && State.currentFolder.get('id') === changes.id) { Radio.ui.request('messagesview:collection').add(changes.messages); - // var messages = new MessageCollection(changes.messages).slice(0); - // Radio.message.trigger('fetch:bodies', changedAccount, changedFolder, messages); + Radio.message.trigger('fetch:bodies', changedAccount, changedFolder, new MessageCollection(messages)); } Radio.ui.trigger('title:update'); diff --git a/js/controller/foldercontroller.js b/js/controller/foldercontroller.js index 57bde6ac5b..285401f77b 100644 --- a/js/controller/foldercontroller.js +++ b/js/controller/foldercontroller.js @@ -155,13 +155,7 @@ define(function(require) { */ function fetchBodies(account, folder, messages) { if (messages.length > 0) { - var ids = _.map(messages, function(message) { - return message.get('id'); - }); - var fetchingMessageBodies = Radio.message.request('bodies', account, folder, ids); - $.when(fetchingMessageBodies).done(function(messages) { - // TODO: merge attributes - }); + Radio.message.request('bodies', account, folder, messages); } } diff --git a/js/service/messageservice.js b/js/service/messageservice.js index dc5ecefd34..9a249d93a1 100644 --- a/js/service/messageservice.js +++ b/js/service/messageservice.js @@ -130,11 +130,7 @@ define(function(require) { $.ajax(url, { type: 'GET', success: function(messageDetails) { - // do not override nested Backbone model 'flags' - delete messageDetails.flags; - - message.set(messageDetails); - message.set('hasDetails', true); + addDetailsToMessage(message, messageDetails); defer.resolve(message); }, error: function(jqXHR, textStatus) { @@ -147,24 +143,27 @@ define(function(require) { return defer.promise(); } + function addDetailsToMessage(message, data) { + // do not override nested Backbone model 'flags' + delete data.flags; + + message.set(data); + message.set('hasDetails', true); + } + /** * @param {Account} account * @param {Folder} folder - * @param {array} messageIds - * @returns {undefined} + * @param {MessageCollection} messages + * @returns {Promise} */ - function fetchMessageBodies(account, folder, messageIds) { + function fetchMessageBodies(account, folder, messages) { var defer = $.Deferred(); var uncachedIds = []; - _.each(messageIds, function(messageId) { - var message = folder.get('messages').get(messageId); - if (!message) { - // Weird, shouldn't have happened, but let's ignore this one - return; - } + messages.forEach(function(message) { if (!message.get('hasDetails')) { - uncachedIds.push(messageId); + uncachedIds.push(message.get('id')); } }); @@ -178,7 +177,10 @@ define(function(require) { $.ajax(url, { type: 'GET', success: function(data) { - // TODO: merge attributes + _.each(data, function(msg) { + var message = messages.get(msg.id); + addDetailsToMessage(message, msg); + }); defer.resolve(data); }, error: function() { diff --git a/js/views/newmessage.js b/js/views/newmessage.js index 56e73c235c..922dcc4fcf 100644 --- a/js/views/newmessage.js +++ b/js/views/newmessage.js @@ -29,10 +29,10 @@ define(function(require) { template: Handlebars.compile(NewMessageTemplate), accounts: null, ui: { - input: 'input' + button: 'button' }, events: { - 'click input': 'onClick' + 'click button': 'onClick' }, initialize: function(options) { this.accounts = options.accounts; @@ -44,9 +44,9 @@ define(function(require) { }, onAccountsChanged: function() { if (this.accounts.size === 0) { - this.ui.input.hide(); + this.ui.button.hide(); } else { - this.ui.input.show(); + this.ui.button.show(); } }, onClick: function(e) { From b3aeeed91e4b06d802d63bf290e696e3da393243 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 20:43:04 +0200 Subject: [PATCH 5/8] merge nested flags model --- js/models/message.js | 6 ++++++ js/service/messageservice.js | 14 ++++---------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/js/models/message.js b/js/models/message.js index 556be2a107..98671e3a83 100644 --- a/js/models/message.js +++ b/js/models/message.js @@ -25,12 +25,18 @@ define(function(require) { }, initialize: function() { this.set('flags', new MessageFlags(this.get('flags'))); + this.on('change:flags', this._mergeFlags); this.listenTo(this.get('flags'), 'change', this._transformEvent); }, _transformEvent: function() { this.trigger('change'); this.trigger('change:flags', this); }, + _mergeFlags: function(model, value) { + var oldFlags = this.previousAttributes()['flags']; + oldFlags.set(value); // Merge changes + this.set('flags', oldFlags); + }, toJSON: function() { var data = Backbone.Model.prototype.toJSON.call(this); if (data.flags && data.flags.toJSON) { diff --git a/js/service/messageservice.js b/js/service/messageservice.js index 9a249d93a1..e4d8fac18f 100644 --- a/js/service/messageservice.js +++ b/js/service/messageservice.js @@ -130,7 +130,8 @@ define(function(require) { $.ajax(url, { type: 'GET', success: function(messageDetails) { - addDetailsToMessage(message, messageDetails); + message.set(messageDetails); + message.set('hasDetails', true); defer.resolve(message); }, error: function(jqXHR, textStatus) { @@ -143,14 +144,6 @@ define(function(require) { return defer.promise(); } - function addDetailsToMessage(message, data) { - // do not override nested Backbone model 'flags' - delete data.flags; - - message.set(data); - message.set('hasDetails', true); - } - /** * @param {Account} account * @param {Folder} folder @@ -179,7 +172,8 @@ define(function(require) { success: function(data) { _.each(data, function(msg) { var message = messages.get(msg.id); - addDetailsToMessage(message, msg); + message.set(msg); + message.set('hasDetails', true); }); defer.resolve(data); }, From 817725a24b6403f810b972853fb59bb4e0203089 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 20:52:46 +0200 Subject: [PATCH 6/8] make sure we don't get endless recursion --- js/models/message.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/js/models/message.js b/js/models/message.js index 98671e3a83..9cd3bb0d24 100644 --- a/js/models/message.js +++ b/js/models/message.js @@ -34,7 +34,7 @@ define(function(require) { }, _mergeFlags: function(model, value) { var oldFlags = this.previousAttributes()['flags']; - oldFlags.set(value); // Merge changes + oldFlags.set(value, {silent: true}); // Merge changes this.set('flags', oldFlags); }, toJSON: function() { From 746b38df2ab4f2a2889f94061c33f233772ea034 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 21:29:54 +0200 Subject: [PATCH 7/8] only render on change for view that really need it --- js/app.js | 2 -- js/views/account.js | 3 +++ js/views/aliases-list.js | 3 +++ js/views/attachments.js | 3 +++ js/views/folder.js | 3 +++ js/views/messagesitem.js | 3 +++ 6 files changed, 15 insertions(+), 2 deletions(-) diff --git a/js/app.js b/js/app.js index 2f958b50bc..17c577e461 100644 --- a/js/app.js +++ b/js/app.js @@ -52,8 +52,6 @@ define(function(require) { Marionette.TemplateCache.prototype.compileTemplate = function(rawTemplate) { return Handlebars.compile(rawTemplate); }; - Marionette.ItemView.prototype.modelEvents = {change: 'render'}; - Marionette.CompositeView.prototype.modelEvents = {change: 'render'}; var Mail = Marionette.Application.extend({ registerProtocolHandler: function() { diff --git a/js/views/account.js b/js/views/account.js index c74ae01b42..b888d38881 100644 --- a/js/views/account.js +++ b/js/views/account.js @@ -45,6 +45,9 @@ define(function(require) { 'click @ui.settingsButton': 'showAccountSettings', 'click @ui.email': 'onClick' }, + modelEvents: { + 'change': 'render' + }, ui: { 'email': '.mail-account-email', 'menu': 'div.app-navigation-entry-menu', diff --git a/js/views/aliases-list.js b/js/views/aliases-list.js index b308254ba3..a7e012fc00 100644 --- a/js/views/aliases-list.js +++ b/js/views/aliases-list.js @@ -34,6 +34,9 @@ define(function(require) { events: { 'click @ui.deleteButton': 'deleteAlias' }, + modelEvents: { + 'change': 'render' + }, initialize: function(options) { this.model = options.model; }, diff --git a/js/views/attachments.js b/js/views/attachments.js index 49bde443bb..00908cbcb0 100644 --- a/js/views/attachments.js +++ b/js/views/attachments.js @@ -25,6 +25,9 @@ define(function(require) { events: { 'click #mail_new_attachment': 'addAttachment' }, + modelEvents: { + 'change': 'render' + }, initialize: function(options) { this.collection = options.collection; }, diff --git a/js/views/folder.js b/js/views/folder.js index 3d16d75c4b..c08ea32de7 100644 --- a/js/views/folder.js +++ b/js/views/folder.js @@ -43,6 +43,9 @@ define(function(require) { 'click .collapse': 'collapseFolder', 'click .folder': 'loadFolder' }, + modelEvents: { + 'change': 'render' + }, collapseFolder: function(e) { e.preventDefault(); this.model.toggleOpen(); diff --git a/js/views/messagesitem.js b/js/views/messagesitem.js index 31cd961220..9a3f0d00cf 100644 --- a/js/views/messagesitem.js +++ b/js/views/messagesitem.js @@ -30,6 +30,9 @@ define(function(require) { 'click .mail-message-header': 'openMessage', 'click .star': 'toggleMessageStar' }, + modelEvents: { + 'change': 'render' + }, onRender: function() { // Get rid of that pesky wrapping-div. // Assumes 1 child element present in template. From 8a2cc2bbe30dc59d0b38753b72f9cf2ee2adfc20 Mon Sep 17 00:00:00 2001 From: Christoph Wurst Date: Wed, 7 Sep 2016 22:32:45 +0200 Subject: [PATCH 8/8] break infinte recursion when serializing models --- js/models/account.js | 13 ------------- js/models/alias.js | 19 ++----------------- js/models/folder.js | 10 +++------- js/models/message.js | 10 ---------- 4 files changed, 5 insertions(+), 47 deletions(-) diff --git a/js/models/account.js b/js/models/account.js index 3ebaf0eb31..f78ed4666f 100644 --- a/js/models/account.js +++ b/js/models/account.js @@ -64,19 +64,6 @@ define(function(require) { } } return undefined; - }, - toJSON: function() { - var data = Backbone.Model.prototype.toJSON.call(this); - if (data.folders && data.folders.toJSON) { - data.folders = data.folders.toJSON(); - } - if (data.aliases && data.aliases.toJSON) { - data.aliases = data.aliases.toJSON(); - } - if (!data.id) { - data.id = this.cid; - } - return data; } }); diff --git a/js/models/alias.js b/js/models/alias.js index b79c54f4a3..f2d981499a 100644 --- a/js/models/alias.js +++ b/js/models/alias.js @@ -5,6 +5,7 @@ * later. See the COPYING file. * * @author Tahaa Karim + * @author Christoph WUrst * @copyright Tahaa Karim 2016 */ @@ -16,23 +17,7 @@ define(function(require) { /** * @class Alias */ - var Alias = Backbone.Model.extend({ - defaults: { - }, - initialize: function() { - - }, - toJSON: function() { - var data = Backbone.Model.prototype.toJSON.call(this); - if (data.alias && data.alias.toJSON) { - data.alias = data.alias.toJSON(); - } - if (!data.id) { - data.id = this.cid; - } - return data; - } - }); + var Alias = Backbone.Model.extend({}); return Alias; }); diff --git a/js/models/folder.js b/js/models/folder.js index d96f05fffa..8fcb1d8961 100644 --- a/js/models/folder.js +++ b/js/models/folder.js @@ -33,13 +33,9 @@ define(function(require) { this.set({open: !this.get('open')}); }, toJSON: function() { - var data = Backbone.Model.prototype.toJSON.call(this); - if (data.folders && data.folders.toJSON) { - data.folders = data.folders.toJSON(); - } - if (!data.id) { - data.id = this.cid; - } + var data = _.clone(this.attributes); + delete data.folders; + delete data.messages; return data; } }); diff --git a/js/models/message.js b/js/models/message.js index 9cd3bb0d24..f1733c3b85 100644 --- a/js/models/message.js +++ b/js/models/message.js @@ -36,16 +36,6 @@ define(function(require) { var oldFlags = this.previousAttributes()['flags']; oldFlags.set(value, {silent: true}); // Merge changes this.set('flags', oldFlags); - }, - toJSON: function() { - var data = Backbone.Model.prototype.toJSON.call(this); - if (data.flags && data.flags.toJSON) { - data.flags = data.flags.toJSON(); - } - if (!data.id) { - data.id = this.cid; - } - return data; } });