L-electron-Rare/moodle
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

MDL-57139 messages/notifications: ensure promise best practices

Browse Source 
Previously there were a few issues with the code

* We were capturing a promise without then then .then() that came after
  it, so our promise wouldn't always be completely resolved by the time
  then next piece of code was operating on it

* We weren't catching all errors with .catch()
This commit is contained in:
Dan Poltawski
2017-06-01 10:27:58 +01:00
parent 66a91ddada
commit 877d997fe1
5 changed files with 95 additions and 133 deletions
Download Patch File Download Diff File Expand all files Collapse all files
message/amd/src/message_area_messages.js
+41 -44
View File
@@ -455,9 +455,8 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/notification', 'core/cust
}
});
});
if (requests.length > 0) {
Ajax.call(requests)[requests.length - 1].then(function() {
$.when(Ajax.call(requests)).then(function() {
// Store the last message on the page, and the last message being deleted.
var updatemessage = null;
var messages = this.messageArea.find(SELECTORS.MESSAGE);
@@ -491,7 +490,7 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/notification', 'core/cust
// Trigger event letting other modules know messages were deleted.
this.messageArea.trigger(Events.MESSAGESDELETED, [this._getUserId(), updatemessage]);
}.bind(this), Notification.exception);
}.bind(this)).catch(Notification.exception);
} else {
// Trigger event letting other modules know messages were deleted.
this.messageArea.trigger(Events.MESSAGESDELETED, this._getUserId());
@@ -526,49 +525,47 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/notification', 'core/cust
* @private
*/
Messages.prototype._deleteAllMessages = function() {
// Create the confirmation modal if we haven't already.
if (!this._confirmationModal) {
Str.get_strings([
{key: 'confirm'},
{key: 'deleteallconfirm', component: 'message'}
]).done(function(s) {
ModalFactory.create({
title: s[0],
type: ModalFactory.types.CONFIRM,
body: s[1]
}, this.messageArea.find(SELECTORS.DELETEALLMESSAGES))
.done(function(modal) {
this._confirmationModal = modal;
// Only delete the conversation if the user agreed in the confirmation modal.
modal.getRoot().on(ModalEvents.yes, function() {
var otherUserId = this._getUserId();
var request = {
methodname: 'core_message_delete_conversation',
args: {
userid: this.messageArea.getCurrentUserId(),
otheruserid: otherUserId
}
};
// Delete the conversation.
Ajax.call([request])[0].then(function() {
// Clear the message area.
this.messageArea.find(SELECTORS.MESSAGESAREA).empty();
// Let the app know a conversation was deleted.
this.messageArea.trigger(Events.CONVERSATIONDELETED, otherUserId);
this._hideDeleteAction();
}.bind(this), Notification.exception);
}.bind(this));
// Display the confirmation.
modal.show();
}.bind(this));
}.bind(this));
} else {
// Otherwise just show the existing modal.
if (this._confirmationModal) {
// Just show the existing modal.
this._confirmationModal.show();
return;
}
Str.get_strings([
{key: 'confirm'},
{key: 'deleteallconfirm', component: 'message'}
]).then(function(s) {
return ModalFactory.create({
title: s[0],
type: ModalFactory.types.CONFIRM,
body: s[1]
}, this.messageArea.find(SELECTORS.DELETEALLMESSAGES));
}.bind(this)).then(function(modal) {
this._confirmationModal = modal;
// Only delete the conversation if the user agreed in the confirmation modal.
modal.getRoot().on(ModalEvents.yes, function() {
var otherUserId = this._getUserId();
var request = {
methodname: 'core_message_delete_conversation',
args: {
userid: this.messageArea.getCurrentUserId(),
otheruserid: otherUserId
}
};
// Delete the conversation.
Ajax.call([request])[0].then(function() {
// Clear the message area.
this.messageArea.find(SELECTORS.MESSAGESAREA).empty();
// Let the app know a conversation was deleted.
this.messageArea.trigger(Events.CONVERSATIONDELETED, otherUserId);
this._hideDeleteAction();
}.bind(this)).catch(Notification.exception);
}.bind(this));
// Display the confirmation.
modal.show();
}.bind(this)).catch(Notification.exception);
};
/**
message/amd/src/message_area_search.js
+1 -1
View File
@@ -385,7 +385,7 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/notification', 'core/str'
this.messageArea.find(SELECTORS.SEARCHFILTER).html(text);
Str.get_string('removecoursefilter', 'message', text).then(function(languagestring) {
this.messageArea.find(SELECTORS.SEARCHFILTERAREA).attr('aria-label', languagestring);
}.bind(this));
}.bind(this)).catch(Notification.exception);
};
/**
message/output/popup/amd/src/message_popover_controller.js
+18 -29
View File
@@ -151,7 +151,7 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/str',
this.unreadCount = count;
this.renderUnreadCount();
this.updateButtonAriaLabel();
}.bind(this));
}.bind(this)).catch(Notification.exception);
};
/**
@@ -165,38 +165,27 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/str',
*/
MessagePopoverController.prototype.renderMessages = function(messages, container) {
var promises = [];
var allhtml = [];
var alljs = [];
if (messages.length) {
$.each(messages, function(index, message) {
message.contexturl = URL.relativeUrl('/message/index.php', {
user: this.userId,
id: message.userid,
});
$.each(messages, function(index, message) {
message.contexturl = URL.relativeUrl('/message/index.php', {
user: this.userId,
id: message.userid,
});
message.profileurl = URL.relativeUrl('/user/profile.php', {
id: message.userid,
});
message.profileurl = URL.relativeUrl('/user/profile.php', {
id: message.userid,
});
var promise = Templates.render('message_popup/message_content_item', message);
promises.push(promise);
var promise = Templates.render('message_popup/message_content_item', message)
.then(function(html, js) {
container.append(html);
Templates.runTemplateJS(js);
return;
});
promises.push(promise);
}.bind(this));
promise.then(function(html, js) {
allhtml[index] = html;
alljs[index] = js;
});
}.bind(this));
}
return $.when.apply($.when, promises).then(function() {
if (messages.length) {
$.each(messages, function(index) {
container.append(allhtml[index]);
Templates.runTemplateJS(alljs[index]);
});
}
});
return $.when.apply($, promises);
};
/**
message/output/popup/amd/src/notification_area_control_area.js
+16 -28
View File
@@ -307,39 +307,27 @@ define(['jquery', 'core/templates', 'core/notification', 'core/custom_interactio
*/
ControlArea.prototype.renderNotifications = function(notifications) {
var promises = [];
var allhtml = [];
var alljs = [];
var container = this.getContent();
if (notifications.length) {
$.each(notifications, function(index, notification) {
// Need to remove the contexturl so the item isn't rendered
// as a link.
var contextUrl = notification.contexturl;
delete notification.contexturl;
$.each(notifications, function(index, notification) {
// Need to remove the contexturl so the item isn't rendered
// as a link.
var contextUrl = notification.contexturl;
delete notification.contexturl;
var promise = Templates.render(TEMPLATES.NOTIFICATION, notification);
promises.push(promise);
promise.then(function(html, js) {
allhtml[index] = html;
alljs[index] = js;
// Restore it for the cache.
notification.contexturl = contextUrl;
this.setCacheNotification(notification);
}.bind(this))
.fail(DebugNotification.exception);
var promise = Templates.render(TEMPLATES.NOTIFICATION, notification)
.then(function(html, js) {
container.append(html);
Templates.runTemplateJS(js);
// Restore it for the cache.
notification.contexturl = contextUrl;
this.setCacheNotification(notification);
return;
}.bind(this));
}
promises.push(promise);
}.bind(this));
return $.when.apply($.when, promises).then(function() {
if (notifications.length) {
$.each(notifications, function(index) {
container.append(allhtml[index]);
Templates.runTemplateJS(alljs[index]);
});
}
});
return $.when.apply($, promises);
};
/**
message/output/popup/amd/src/notification_popover_controller.js
+19 -31
View File
@@ -200,7 +200,7 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/str', 'core/url',
this.unreadCount = count;
this.renderUnreadCount();
this.updateButtonAriaLabel();
}.bind(this));
}.bind(this)).catch(DebugNotification.exception);
};
/**
@@ -226,39 +226,27 @@ define(['jquery', 'core/ajax', 'core/templates', 'core/str', 'core/url',
*/
NotificationPopoverController.prototype.renderNotifications = function(notifications, container) {
var promises = [];
var allhtml = [];
var alljs = [];
if (notifications.length) {
$.each(notifications, function(index, notification) {
// Determine what the offset was when loading this notification.
var offset = this.getOffset() - this.limit;
// Update the view more url to contain the offset to allow the notifications
// page to load to the correct position in the list of notifications.
notification.viewmoreurl = URL.relativeUrl('/message/output/popup/notifications.php', {
notificationid: notification.id,
offset: offset,
});
$.each(notifications, function(index, notification) {
// Determine what the offset was when loading this notification.
var offset = this.getOffset() - this.limit;
// Update the view more url to contain the offset to allow the notifications
// page to load to the correct position in the list of notifications.
notification.viewmoreurl = URL.relativeUrl('/message/output/popup/notifications.php', {
notificationid: notification.id,
offset: offset,
});
var promise = Templates.render('message_popup/notification_content_item', notification);
promises.push(promise);
var promise = Templates.render('message_popup/notification_content_item', notification)
.then(function(html, js) {
container.append(html);
Templates.runTemplateJS(js);
return;
});
promises.push(promise);
}.bind(this));
promise.then(function(html, js) {
allhtml[index] = html;
alljs[index] = js;
})
.fail(DebugNotification.exception);
}.bind(this));
}
return $.when.apply($.when, promises).then(function() {
if (notifications.length) {
$.each(notifications, function(index) {
container.append(allhtml[index]);
Templates.runTemplateJS(alljs[index]);
});
}
});
return $.when.apply($, promises);
};
/**