From 06f7a427c6537ccd6cefcd668845b9249f28cfaa Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Tue, 20 Nov 2018 11:31:52 +0100 Subject: [PATCH] MOBILE-2632 message: Send message online and offline --- src/addon/messages/lang/en.json | 1 + .../messages/pages/discussion/discussion.ts | 128 ++++++++------ .../messages/providers/messages-offline.ts | 150 +++++++++++++--- src/addon/messages/providers/messages.ts | 127 ++++++++++++- src/addon/messages/providers/sync.ts | 167 ++++++++++++++---- src/assets/lang/en.json | 1 + 6 files changed, 464 insertions(+), 110 deletions(-) diff --git a/src/addon/messages/lang/en.json b/src/addon/messages/lang/en.json index 6de944be2..1311eb52a 100644 --- a/src/addon/messages/lang/en.json +++ b/src/addon/messages/lang/en.json @@ -40,6 +40,7 @@ "type_strangers": "Others", "unblockuser": "Unblock user", "unblockuserconfirm": "Are you sure you want to unblock {{$a}}?", + "warningconversationmessagenotsent": "Couldn't send message(s) to conversation {{conversation}}. {{error}}", "warningmessagenotsent": "Couldn't send message(s) to user {{user}}. {{error}}", "you": "You:" } \ No newline at end of file diff --git a/src/addon/messages/pages/discussion/discussion.ts b/src/addon/messages/pages/discussion/discussion.ts index 606e6081c..fe0ee05cd 100644 --- a/src/addon/messages/pages/discussion/discussion.ts +++ b/src/addon/messages/pages/discussion/discussion.ts @@ -18,6 +18,7 @@ import { TranslateService } from '@ngx-translate/core'; import { CoreEventsProvider } from '@providers/events'; import { CoreSitesProvider } from '@providers/sites'; import { AddonMessagesProvider } from '../../providers/messages'; +import { AddonMessagesOfflineProvider } from '../../providers/messages-offline'; import { AddonMessagesSyncProvider } from '../../providers/sync'; import { CoreUserProvider } from '@core/user/providers/user'; import { CoreDomUtilsProvider } from '@providers/utils/dom'; @@ -79,7 +80,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { private userProvider: CoreUserProvider, private navCtrl: NavController, private messagesSync: AddonMessagesSyncProvider, private domUtils: CoreDomUtilsProvider, private messagesProvider: AddonMessagesProvider, logger: CoreLoggerProvider, private utils: CoreUtilsProvider, private appProvider: CoreAppProvider, private translate: TranslateService, - @Optional() private svComponent: CoreSplitViewComponent) { + @Optional() private svComponent: CoreSplitViewComponent, private messagesOffline: AddonMessagesOfflineProvider) { this.siteId = sitesProvider.getCurrentSiteId(); this.currentUserId = sitesProvider.getCurrentSiteUserId(); this.groupMessagingEnabled = this.messagesProvider.isGroupMessagingEnabled(); @@ -166,7 +167,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { } // Synchronize messages if needed. - this.messagesSync.syncDiscussion(this.userId).catch(() => { + this.messagesSync.syncDiscussion(this.conversationId, this.userId).catch(() => { // Ignore errors. }).then((warnings) => { if (warnings && warnings[0]) { @@ -253,7 +254,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { this.fetching = true; // Wait for synchronization process to finish. - return this.messagesSync.waitForSync(this.userId).then(() => { + return this.messagesSync.waitForSyncConversation(this.conversationId, this.userId).then(() => { // Fetch messages. Invalidate the cache before fetching. if (this.groupMessagingEnabled) { return this.messagesProvider.invalidateConversationMessages(this.conversationId).catch(() => { @@ -269,49 +270,57 @@ export class AddonMessagesDiscussionPage implements OnDestroy { }); } }).then((messages) => { - if (this.viewDestroyed) { - return Promise.resolve(); - } - - // Check if we are at the bottom to scroll it after render. - this.scrollBottom = this.domUtils.getScrollHeight(this.content) - this.domUtils.getScrollTop(this.content) === - this.domUtils.getContentHeight(this.content); - - if (this.messagesBeingSent > 0) { - // Ignore polling due to a race condition. - return Promise.reject(null); - } - - // Add new messages to the list and mark the messages that should still be displayed. - messages.forEach((message) => { - this.addMessage(message); - }); - - // Remove messages that shouldn't be in the list anymore. - for (const hash in this.keepMessageMap) { - this.removeMessage(hash); - } - - // Sort the messages. - this.messagesProvider.sortMessages(this.messages); - - // Calculate which messages need to display the date or user data. - this.messages.forEach((message, index): any => { - message.showDate = this.showDate(message, this.messages[index - 1]); - message.showUserData = this.showUserData(message, this.messages[index - 1]); - }); - - // Notify that there can be a new message. - this.notifyNewMessage(); - - // Mark retrieved messages as read if they are not. - this.markMessagesAsRead(); - + this.loadMessages(messages); }).finally(() => { this.fetching = false; }); } + /** + * Format and load a list of messages into the view. + * + * @param {any[]} messages Messages to load. + */ + protected loadMessages(messages: any[]): void { + if (this.viewDestroyed) { + return; + } + + // Check if we are at the bottom to scroll it after render. + this.scrollBottom = this.domUtils.getScrollHeight(this.content) - this.domUtils.getScrollTop(this.content) === + this.domUtils.getContentHeight(this.content); + + if (this.messagesBeingSent > 0) { + // Ignore polling due to a race condition. + return; + } + + // Add new messages to the list and mark the messages that should still be displayed. + messages.forEach((message) => { + this.addMessage(message); + }); + + // Remove messages that shouldn't be in the list anymore. + for (const hash in this.keepMessageMap) { + this.removeMessage(hash); + } + + // Sort the messages. + this.messagesProvider.sortMessages(this.messages); + + // Calculate which messages need to display the date or user data. + this.messages.forEach((message, index): any => { + message.showDate = this.showDate(message, this.messages[index - 1]); + message.showUserData = this.showUserData(message, this.messages[index - 1]); + }); + + // Notify that there can be a new message. + this.notifyNewMessage(); + + // Mark retrieved messages as read if they are not. + this.markMessagesAsRead(); + } + /** * Get the conversation. * @@ -328,18 +337,29 @@ export class AddonMessagesDiscussionPage implements OnDestroy { } else { // We don't have the conversation ID, check if it exists. promise = this.messagesProvider.getConversationBetweenUsers(this.userId).catch((error) => { - if (error.errorcode == 'conversationdoesntexist') { - // Conversation does not exist, return undefined. - return; - } - return Promise.reject(error); + // Probably conversation does not exist or user is offline. Try to load offline messages. + return this.messagesOffline.getMessages(this.userId).then((messages) => { + if (messages && messages.length) { + // We have offline messages, this probably means that the conversation didn't exist. Don't display error. + messages.forEach((message) => { + message.pending = true; + message.text = message.smallmessage; + }); + + this.loadMessages(messages); + } else if (error.errorcode != 'conversationdoesntexist') { + // Display the error. + return Promise.reject(error); + } + }); }); } return promise.then((conversation) => { this.conversation = conversation; if (conversation) { + this.conversationId = conversation.id; this.title = conversation.name; this.conversationImage = conversation.imageurl; this.isGroup = conversation.type == AddonMessagesProvider.MESSAGE_CONVERSATION_TYPE_GROUP; @@ -677,7 +697,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { * Function to delete a message. * * @param {any} message Message object to delete. - * @param {number} index Index where the mesasge is to delete it from the view. + * @param {number} index Index where the message is to delete it from the view. */ deleteMessage(message: any, index: number): void { const langKey = message.pending ? 'core.areyousure' : 'addon.messages.deletemessageconfirmation'; @@ -779,6 +799,7 @@ export class AddonMessagesDiscussionPage implements OnDestroy { text: text, timecreated: new Date().getTime() }; + message.showDate = this.showDate(message, this.messages[this.messages.length - 1]); this.addMessage(message, false); this.messagesBeingSent++; @@ -786,7 +807,15 @@ export class AddonMessagesDiscussionPage implements OnDestroy { // If there is an ongoing fetch, wait for it to finish. // Otherwise, if a message is sent while fetching it could disappear until the next fetch. this.waitForFetch().finally(() => { - this.messagesProvider.sendMessage(this.userId, text).then((data) => { + let promise; + + if (this.conversationId) { + promise = this.messagesProvider.sendMessageToConversation(this.conversationId, text); + } else { + promise = this.messagesProvider.sendMessage(this.userId, text); + } + + promise.then((data) => { let promise; this.messagesBeingSent--; @@ -836,9 +865,6 @@ export class AddonMessagesDiscussionPage implements OnDestroy { if (!prevMessage) { // First message, show it. return true; - } else if (message.pending) { - // If pending, it has no date, not show. - return false; } // Check if day has changed. diff --git a/src/addon/messages/providers/messages-offline.ts b/src/addon/messages/providers/messages-offline.ts index 5bf12837c..022515c74 100644 --- a/src/addon/messages/providers/messages-offline.ts +++ b/src/addon/messages/providers/messages-offline.ts @@ -26,7 +26,8 @@ export class AddonMessagesOfflineProvider { protected logger; // Variables for database. - static MESSAGES_TABLE = 'addon_messages_offline_messages'; + static MESSAGES_TABLE = 'addon_messages_offline_messages'; // When group messaging isn't available or a new conversation starts. + static CONVERSATION_MESSAGES_TABLE = 'addon_messages_offline_conversation_messages'; // Conversation messages. protected tablesSchema = [ { name: AddonMessagesOfflineProvider.MESSAGES_TABLE, @@ -53,6 +54,28 @@ export class AddonMessagesOfflineProvider { } ], primaryKeys: ['touserid', 'smallmessage', 'timecreated'] + }, + { + name: AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, + columns: [ + { + name: 'conversationid', + type: 'INTEGER' + }, + { + name: 'text', + type: 'TEXT' + }, + { + name: 'timecreated', + type: 'INTEGER' + }, + { + name: 'deviceoffline', // If message was stored because device was offline. + type: 'INTEGER' + } + ], + primaryKeys: ['conversationid', 'text', 'timecreated'] } ]; @@ -61,6 +84,25 @@ export class AddonMessagesOfflineProvider { this.sitesProvider.createTablesFromSchema(this.tablesSchema); } + /** + * Delete a message. + * + * @param {number} conversationId Conversation ID. + * @param {string} message The message. + * @param {number} timeCreated The time the message was created. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved if stored, rejected if failure. + */ + deleteConversationMessage(conversationId: number, message: string, timeCreated: number, siteId?: string): Promise { + return this.sitesProvider.getSite(siteId).then((site) => { + return site.getDb().deleteRecords(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, { + conversationid: conversationId, + text: message, + timecreated: timeCreated + }); + }); + } + /** * Delete a message. * @@ -84,24 +126,18 @@ export class AddonMessagesOfflineProvider { * Get all messages where deviceoffline is set to 1. * * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved with messages. + * @return {Promise} Promise resolved with messages. */ - getAllDeviceOfflineMessages(siteId?: string): Promise { + getAllDeviceOfflineMessages(siteId?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - return site.getDb().getRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE, {deviceoffline: 1}); - }); - } + const promises = []; - /** - * Get offline messages to send to a certain user. - * - * @param {number} toUserId User ID to get messages to. - * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved with messages. - */ - getMessages(toUserId: number, siteId?: string): Promise { - return this.sitesProvider.getSite(siteId).then((site) => { - return site.getDb().getRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE, {touserid: toUserId}); + promises.push(site.getDb().getRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE, {deviceoffline: 1})); + promises.push(site.getDb().getRecords(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, {deviceoffline: 1})); + + return Promise.all(promises).then((results) => { + return results[0].concat(results[1]); + }); }); } @@ -113,7 +149,54 @@ export class AddonMessagesOfflineProvider { */ getAllMessages(siteId?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - return site.getDb().getAllRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE); + const promises = []; + + promises.push(site.getDb().getAllRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE)); + promises.push(site.getDb().getAllRecords(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE)); + + return Promise.all(promises).then((results) => { + return results[0].concat(results[1]); + }); + }); + } + + /** + * Get offline messages to send to a certain user. + * + * @param {number} conversationId Conversation ID. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved with messages. + */ + getConversationMessages(conversationId: number, siteId?: string): Promise { + return this.sitesProvider.getSite(siteId).then((site) => { + return site.getDb().getRecords(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, + {conversationid: conversationId}); + }); + } + + /** + * Get offline messages to send to a certain user. + * + * @param {number} toUserId User ID to get messages to. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved with messages. + */ + getMessages(toUserId: number, siteId?: string): Promise { + return this.sitesProvider.getSite(siteId).then((site) => { + return site.getDb().getRecords(AddonMessagesOfflineProvider.MESSAGES_TABLE, {touserid: toUserId}); + }); + } + + /** + * Check if there are offline messages to send to a conversation. + * + * @param {number} conversationId Conversation ID. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved with boolean: true if has offline messages, false otherwise. + */ + hasConversationMessages(conversationId: number, siteId?: string): Promise { + return this.getConversationMessages(conversationId, siteId).then((messages) => { + return !!messages.length; }); } @@ -122,14 +205,37 @@ export class AddonMessagesOfflineProvider { * * @param {number} toUserId User ID to check. * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved with boolean: true if has offline messages, false otherwise. + * @return {Promise} Promise resolved with boolean: true if has offline messages, false otherwise. */ - hasMessages(toUserId: number, siteId?: string): Promise { + hasMessages(toUserId: number, siteId?: string): Promise { return this.getMessages(toUserId, siteId).then((messages) => { return !!messages.length; }); } + /** + * Save a conversation message to be sent later. + * + * @param {number} conversationId Conversation ID. + * @param {string} message The message to send. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved if stored, rejected if failure. + */ + saveConversationMessage(conversationId: number, message: string, siteId?: string): Promise { + return this.sitesProvider.getSite(siteId).then((site) => { + const entry = { + conversationid: conversationId, + text: message, + timecreated: Date.now(), + deviceoffline: this.appProvider.isOnline() ? 0 : 1 + }; + + return site.getDb().insertRecord(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, entry).then(() => { + return entry; + }); + }); + } + /** * Save a message to be sent later. * @@ -169,7 +275,11 @@ export class AddonMessagesOfflineProvider { data = { deviceoffline: value ? 1 : 0 }; messages.forEach((message) => { - promises.push(db.insertRecord(AddonMessagesOfflineProvider.MESSAGES_TABLE, data)); + if (message.conversationid) { + promises.push(db.insertRecord(AddonMessagesOfflineProvider.CONVERSATION_MESSAGES_TABLE, data)); + } else { + promises.push(db.insertRecord(AddonMessagesOfflineProvider.MESSAGES_TABLE, data)); + } }); return Promise.all(promises); diff --git a/src/addon/messages/providers/messages.ts b/src/addon/messages/providers/messages.ts index aaf5bba86..d96bba2e9 100644 --- a/src/addon/messages/providers/messages.ts +++ b/src/addon/messages/providers/messages.ts @@ -115,7 +115,11 @@ export class AddonMessagesProvider { } // It's an offline message. - return this.messagesOffline.deleteMessage(message.touserid, message.smallmessage, message.timecreated); + if (message.conversationid) { + return this.messagesOffline.deleteConversationMessage(message.conversationid, message.text, message.timecreated); + } else { + return this.messagesOffline.deleteMessage(message.touserid, message.smallmessage, message.timecreated); + } } /** @@ -127,13 +131,15 @@ export class AddonMessagesProvider { * @return {Promise} Promise resolved when the message has been deleted. */ deleteMessageOnline(id: number, read: number, userId?: number): Promise { - userId = userId || this.sitesProvider.getCurrentSiteUserId(); - const params = { + const params: any = { messageid: id, - userid: userId, - read: read + userid: userId || this.sitesProvider.getCurrentSiteUserId() }; + if (typeof read != 'undefined') { + params.read = read; + } + return this.sitesProvider.getCurrentSite().write('core_message_delete_message', params).then(() => { return this.invalidateDiscussionCache(userId); }); @@ -526,11 +532,11 @@ export class AddonMessagesProvider { } // Get offline messages. - return this.messagesOffline.getMessages(userId).then((offlineMessages) => { + return this.messagesOffline.getConversationMessages(conversationId).then((offlineMessages) => { // Mark offline messages as pending. offlineMessages.forEach((message) => { message.pending = true; - message.text = message.smallmessage; + message.useridfrom = userId; }); result.messages = result.messages.concat(offlineMessages); @@ -1497,6 +1503,113 @@ export class AddonMessagesProvider { }); } + /** + * Send a message to a conversation. + * + * @param {number} conversationId Conversation ID. + * @param {string} message The message to send. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved with: + * - sent (boolean) True if message was sent to server, false if stored in device. + * - message (any) If sent=false, contains the stored message. + * @since 3.6 + */ + sendMessageToConversation(conversationId: number, message: string, siteId?: string): Promise { + // Convenience function to store a message to be synchronized later. + const storeOffline = (): Promise => { + return this.messagesOffline.saveConversationMessage(conversationId, message, siteId).then((entry) => { + return { + sent: false, + message: entry + }; + }); + }; + + siteId = siteId || this.sitesProvider.getCurrentSiteId(); + + if (!this.appProvider.isOnline()) { + // App is offline, store the message. + return storeOffline(); + } + + // Check if this conversation already has offline messages. + // If so, store this message since they need to be sent in order. + return this.messagesOffline.hasConversationMessages(conversationId, siteId).catch(() => { + // Error, it's safer to assume it has messages. + return true; + }).then((hasStoredMessages) => { + if (hasStoredMessages) { + return storeOffline(); + } + + // Online and no messages stored. Send it to server. + return this.sendMessageToConversationOnline(conversationId, message).then(() => { + return { sent: true }; + }).catch((error) => { + if (this.utils.isWebServiceError(error)) { + // It's a WebService error, the user cannot send the message so don't store it. + return Promise.reject(error); + } + + // Error sending message, store it to retry later. + return storeOffline(); + }); + }); + } + + /** + * Send a message to a conversation. It will fail if offline or cannot connect. + * + * @param {number} conversationId Conversation ID. + * @param {string} message The message to send + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved if success, rejected if failure. + * @since 3.6 + */ + sendMessageToConversationOnline(conversationId: number, message: string, siteId?: string): Promise { + siteId = siteId || this.sitesProvider.getCurrentSiteId(); + + const messages = [ + { + text: message, + textformat: 1 + } + ]; + + return this.sendMessagesToConversationOnline(conversationId, messages, siteId).then((response) => { + return this.invalidateConversationMessages(conversationId, siteId).catch(() => { + // Ignore errors. + }).then(() => { + return response[0]; + }); + }); + } + + /** + * Send some messages to a conversation. It will fail if offline or cannot connect. + * + * @param {number} conversationId Conversation ID. + * @param {any} messages Messages to send. Each message must contain text and, optionally, textformat. + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved if success, rejected if failure. + * @since 3.6 + */ + sendMessagesToConversationOnline(conversationId: number, messages: any, siteId?: string): Promise { + return this.sitesProvider.getSite(siteId).then((site) => { + const params = { + conversationid: conversationId, + messages: messages.map((message) => { + return { + text: message.text, + textformat: typeof message.textformat != 'undefined' ? message.textformat : 1 + }; + }) + }; + + return site.write('core_message_send_messages_to_conversation', params); + }); + } + /** * Helper method to sort messages by time. * diff --git a/src/addon/messages/providers/sync.ts b/src/addon/messages/providers/sync.ts index 0a9b77ac7..0b2330bf0 100644 --- a/src/addon/messages/providers/sync.ts +++ b/src/addon/messages/providers/sync.ts @@ -42,6 +42,21 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { super('AddonMessagesSync', loggerProvider, sitesProvider, appProvider, syncProvider, textUtils, translate); } + /** + * Get the ID of a discussion sync. + * + * @param {number} conversationId Conversation ID. + * @param {number} userId User ID talking to (if no conversation ID). + * @return {string} Sync ID. + */ + protected getSyncId(conversationId: number, userId: number): string { + if (conversationId) { + return 'conversationid:' + conversationId; + } else { + return 'userid:' + userId; + } + } + /** * Try to synchronize all the discussions in a certain site or in all sites. * @@ -70,22 +85,39 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { return promise.then((messages) => { const userIds = [], + conversationIds = [], promises = []; - // Get all the discussions to be synced. + // Get all the conversations to be synced. messages.forEach((message) => { - if (userIds.indexOf(message.touserid) == -1) { + if (message.conversationid) { + if (conversationIds.indexOf(message.conversationid) == -1) { + conversationIds.push(message.conversationid); + } + } else if (userIds.indexOf(message.touserid) == -1) { userIds.push(message.touserid); } }); - // Sync all discussions. - userIds.forEach((userId) => { - promises.push(this.syncDiscussion(userId, siteId).then((warnings) => { + // Sync all conversations. + conversationIds.forEach((conversationId) => { + promises.push(this.syncDiscussion(conversationId, undefined, siteId).then((warnings) => { if (typeof warnings != 'undefined') { // Sync successful, send event. this.eventsProvider.trigger(AddonMessagesSyncProvider.AUTO_SYNCED, { - userid: userId, + conversationId: conversationId, + warnings: warnings + }, siteId); + } + })); + }); + + userIds.forEach((userId) => { + promises.push(this.syncDiscussion(undefined, userId, siteId).then((warnings) => { + if (typeof warnings != 'undefined') { + // Sync successful, send event. + this.eventsProvider.trigger(AddonMessagesSyncProvider.AUTO_SYNCED, { + userId: userId, warnings: warnings }, siteId); } @@ -99,24 +131,39 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { /** * Synchronize a discussion. * - * @param {number} userId User ID of the discussion. - * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved if sync is successful, rejected otherwise. + * @param {number} conversationId Conversation ID. + * @param {number} userId User ID talking to (if no conversation ID). + * @return {Promise} Promise resolved if sync is successful, rejected otherwise. */ - syncDiscussion(userId: number, siteId?: string): Promise { + syncDiscussion(conversationId: number, userId: number, siteId?: string): Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); - if (this.isSyncing(userId, siteId)) { - // There's already a sync ongoing for this SCORM, return the promise. - return this.getOngoingSync(userId, siteId); + const syncId = this.getSyncId(conversationId, userId), + groupMessagingEnabled = this.messagesProvider.isGroupMessagingEnabled(); + + if (this.isSyncing(syncId, siteId)) { + // There's already a sync ongoing for this conversation, return the promise. + return this.getOngoingSync(syncId, siteId); } const warnings = []; - this.logger.debug(`Try to sync discussion with user '${userId}'`); + if (conversationId) { + this.logger.debug(`Try to sync conversation '${conversationId}'`); + } else { + this.logger.debug(`Try to sync discussion with user '${userId}'`); + } // Get offline messages to be sent. - const syncPromise = this.messagesOffline.getMessages(userId, siteId).then((messages) => { + let syncPromise; + + if (conversationId) { + syncPromise = this.messagesOffline.getConversationMessages(conversationId, siteId); + } else { + syncPromise = this.messagesOffline.getMessages(userId, siteId); + } + + syncPromise = syncPromise.then((messages) => { if (!messages.length) { // Nothing to sync. return []; @@ -134,12 +181,19 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { messages = this.messagesProvider.sortMessages(messages); // Send the messages. - // We don't use AddonMessagesProvider#sendMessagesOnline because there's a problem with display order. - // @todo Use AddonMessagesProvider#sendMessagesOnline once the display order is fixed. + // Send them 1 by 1 to simulate web's behaviour and to make sure we know which message has failed. messages.forEach((message, index) => { // Chain message sending. If 1 message fails to be sent we'll stop sending. promise = promise.then(() => { - return this.messagesProvider.sendMessageOnline(userId, message.smallmessage, siteId).catch((error) => { + let subPromise; + + if (conversationId) { + subPromise = this.messagesProvider.sendMessageToConversationOnline(conversationId, message.text, siteId); + } else { + subPromise = this.messagesProvider.sendMessageOnline(userId, message.smallmessage, siteId); + } + + return subPromise.catch((error) => { if (this.utils.isWebServiceError(error)) { // Error returned by WS. Store the error to show a warning but keep sending messages. if (errors.indexOf(error) == -1) { @@ -158,22 +212,61 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { return Promise.reject(error); }).then(() => { // Message was sent, delete it from local DB. - return this.messagesOffline.deleteMessage(userId, message.smallmessage, message.timecreated, siteId); + if (conversationId) { + return this.messagesOffline.deleteConversationMessage(conversationId, message.text, + message.timecreated, siteId); + } else { + return this.messagesOffline.deleteMessage(userId, message.smallmessage, message.timecreated, siteId); + } }).then(() => { - // All done. Wait 1 second to ensure timecreated of messages is different. - if (index < messages.length - 1) { + // In some Moodle versions, wait 1 second to make sure timecreated is different. + // This is because there was a bug where messages with the same timecreated had a wrong order. + if (!groupMessagingEnabled && index < messages.length - 1) { return setTimeout(() => {return; }, 1000); } }); }); }); - return promise.then(() => { - return errors; - }); + return promise; }).then((errors) => { - if (errors && errors.length) { - // At least an error occurred, get user full name and add errors to warnings array. + return this.handleSyncErrors(conversationId, userId, errors, warnings); + }).then(() => { + // All done, return the warnings. + return warnings; + }); + + return this.addOngoingSync(syncId, syncPromise, siteId); + } + + /** + * Handle sync errors. + * + * @param {number} conversationId Conversation ID. + * @param {number} userId User ID talking to (if no conversation ID). + * @param {any[]} errors List of errors. + * @param {any[]} warnings Array where to place the warnings. + * @return {Promise} Promise resolved when done. + */ + protected handleSyncErrors(conversationId: number, userId: number, errors: any[], warnings: any[]): Promise { + if (errors && errors.length) { + if (conversationId) { + + // Get conversation name and add errors to warnings array. + return this.messagesProvider.getConversation(conversationId, false, false).catch(() => { + // Ignore errors. + return {}; + }).then((conversation) => { + errors.forEach((error) => { + warnings.push(this.translate.instant('addon.messages.warningconversationmessagenotsent', { + conversation: conversation.name ? conversation.name : conversationId, + error: this.textUtils.getErrorMessageFromError(error) + })); + }); + }); + } else { + + // Get user full name and add errors to warnings array. return this.userProvider.getProfile(userId, undefined, true).catch(() => { // Ignore errors. return {}; @@ -181,16 +274,26 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { errors.forEach((error) => { warnings.push(this.translate.instant('addon.messages.warningmessagenotsent', { user: user.fullname ? user.fullname : userId, - error: error + error: this.textUtils.getErrorMessageFromError(error) })); }); }); } - }).then(() => { - // All done, return the warnings. - return warnings; - }); + } + } - return this.addOngoingSync(userId, syncPromise, siteId); + /** + * If there's an ongoing sync for a certain conversation, wait for it to end. + * If there's no sync ongoing the promise will be resolved right away. + * + * @param {number} conversationId Conversation ID. + * @param {number} userId User ID talking to (if no conversation ID). + * @param {string} [siteId] Site ID. If not defined, current site. + * @return {Promise} Promise resolved when there's no sync going on for the identifier. + */ + waitForSyncConversation(conversationId: number, userId: number, siteId?: string): Promise { + const syncId = this.getSyncId(conversationId, userId); + + return this.waitForSync(syncId, siteId); } } diff --git a/src/assets/lang/en.json b/src/assets/lang/en.json index 29c852b9c..e6386fb88 100644 --- a/src/assets/lang/en.json +++ b/src/assets/lang/en.json @@ -188,6 +188,7 @@ "addon.messages.type_strangers": "Others", "addon.messages.unblockuser": "Unblock user", "addon.messages.unblockuserconfirm": "Are you sure you want to unblock {{$a}}?", + "addon.messages.warningconversationmessagenotsent": "Couldn't send message(s) to conversation {{conversation}}. {{error}}", "addon.messages.warningmessagenotsent": "Couldn't send message(s) to user {{user}}. {{error}}", "addon.messages.you": "You:", "addon.mod_assign.acceptsubmissionstatement": "Please accept the submission statement.",