diff --git a/src/addon/messages/pages/discussion/discussion.ts b/src/addon/messages/pages/discussion/discussion.ts index 043376f1d..11a4046a1 100644 --- a/src/addon/messages/pages/discussion/discussion.ts +++ b/src/addon/messages/pages/discussion/discussion.ts @@ -581,35 +581,38 @@ export class AddonMessagesDiscussionPage implements OnDestroy { * @param offset Offset for message list. * @return Promise resolved with the list of messages. */ - protected getConversationMessages(pagesToLoad: number, offset: number = 0) + protected async getConversationMessages(pagesToLoad: number, offset: number = 0) : Promise { const excludePending = offset > 0; - return this.messagesProvider.getConversationMessages(this.conversationId, excludePending, offset).then((result) => { - pagesToLoad--; - - // Treat members. Don't use CoreUtilsProvider.arrayToObject because we don't want to override the existing object. - if (result.members) { - result.members.forEach((member) => { - this.members[member.id] = member; - }); - } - - if (pagesToLoad > 0 && result.canLoadMore) { - offset += AddonMessagesProvider.LIMIT_MESSAGES; - - // Get more messages. - return this.getConversationMessages(pagesToLoad, offset).then((nextMessages) => { - return result.messages.concat(nextMessages); - }); - } else { - // No more messages to load, return them. - this.canLoadMore = result.canLoadMore; - - return result.messages; - } + const result = await this.messagesProvider.getConversationMessages(this.conversationId, { + excludePending: excludePending, + limitFrom: offset, }); + + pagesToLoad--; + + // Treat members. Don't use CoreUtilsProvider.arrayToObject because we don't want to override the existing object. + if (result.members) { + result.members.forEach((member) => { + this.members[member.id] = member; + }); + } + + if (pagesToLoad > 0 && result.canLoadMore) { + offset += AddonMessagesProvider.LIMIT_MESSAGES; + + // Get more messages. + const nextMessages = await this.getConversationMessages(pagesToLoad, offset); + + return result.messages.concat(nextMessages); + } else { + // No more messages to load, return them. + this.canLoadMore = result.canLoadMore; + + return result.messages; + } } /** diff --git a/src/addon/messages/providers/messages.ts b/src/addon/messages/providers/messages.ts index 0288cb13d..ce0b4ae5d 100644 --- a/src/addon/messages/providers/messages.ts +++ b/src/addon/messages/providers/messages.ts @@ -22,7 +22,7 @@ import { CoreUtilsProvider } from '@providers/utils/utils'; import { CoreTimeUtilsProvider } from '@providers/utils/time'; import { CoreEmulatorHelperProvider } from '@core/emulator/providers/helper'; import { CoreEventsProvider } from '@providers/events'; -import { CoreSite } from '@classes/site'; +import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { CoreWSExternalWarning } from '@providers/ws'; /** @@ -398,7 +398,7 @@ export class AddonMessagesProvider { * @param userId The other person with whom the current user is having the discussion. * @return Cache key. */ - protected getCacheKeyForDiscussion(userId: number): string { + getCacheKeyForDiscussion(userId: number): string { return this.ROOT_CACHE_KEY + 'discussion:' + userId; } @@ -889,93 +889,92 @@ export class AddonMessagesProvider { * Get a conversation by the conversation ID. * * @param conversationId Conversation ID to fetch. - * @param excludePending True to exclude messages pending to be sent. - * @param limitFrom Offset for messages list. - * @param limitTo Limit of messages. - * @param newestFirst Whether to order messages by newest first. - * @param timeFrom The timestamp from which the messages were created. - * @param siteId Site ID. If not defined, use current site. - * @param userId User ID. If not defined, current user in the site. + * @param options Options. * @return Promise resolved with the response. * @since 3.6 */ - getConversationMessages(conversationId: number, excludePending: boolean, limitFrom: number = 0, limitTo?: number, - newestFirst: boolean = true, timeFrom: number = 0, siteId?: string, userId?: number) + async getConversationMessages(conversationId: number, options?: AddonMessagesGetConversationMessagesOptions) : Promise { - return this.sitesProvider.getSite(siteId).then((site) => { - userId = userId || site.getUserId(); + options = options || {}; - if (typeof limitTo == 'undefined' || limitTo === null) { - limitTo = this.LIMIT_MESSAGES; + const site = await this.sitesProvider.getSite(options.siteId); + + options.userId = options.userId || site.getUserId(); + options.limitFrom = options.limitFrom || 0; + options.limitTo = options.limitTo === undefined || options.limitTo === null ? this.LIMIT_MESSAGES : options.limitTo; + options.timeFrom = options.timeFrom || 0; + options.newestFirst = options.newestFirst === undefined || options.newestFirst === null ? true : options.newestFirst; + + const preSets: CoreSiteWSPreSets = { + cacheKey: this.getCacheKeyForConversationMessages(options.userId, conversationId), + }; + const params = { + currentuserid: options.userId, + convid: conversationId, + limitfrom: options.limitFrom, + limitnum: options.limitTo < 1 ? options.limitTo : options.limitTo + 1, // If there's a limit, get 1 more than requested. + newest: options.newestFirst ? 1 : 0, + timefrom: options.timeFrom, + }; + + if (options.limitFrom > 0) { + // Do not use cache when retrieving older messages. + // This is to prevent storing too much data and to prevent inconsistencies between "pages" loaded. + preSets.getFromCache = false; + preSets.saveToCache = false; + preSets.emergencyCache = false; + } else if (options.forceCache) { + preSets.omitExpires = true; + } else if (options.ignoreCache) { + preSets.getFromCache = false; + preSets.emergencyCache = false; + } + + const result: AddonMessagesGetConversationMessagesResult = + await site.read('core_message_get_conversation_messages', params, preSets); + + if (options.limitTo < 1) { + result.canLoadMore = false; + result.messages = result.messages; + } else { + result.canLoadMore = result.messages.length > options.limitTo; + result.messages = result.messages.slice(0, options.limitTo); + } + + let lastReceived; + + result.messages.forEach((message) => { + // Convert time to milliseconds. + message.timecreated = message.timecreated ? message.timecreated * 1000 : 0; + + if (!lastReceived && message.useridfrom != options.userId) { + lastReceived = message; } - - const preSets = { - cacheKey: this.getCacheKeyForConversationMessages(userId, conversationId) - }, - params: any = { - currentuserid: userId, - convid: conversationId, - limitfrom: limitFrom, - limitnum: limitTo < 1 ? limitTo : limitTo + 1, // If there is a limit, get 1 more than requested. - newest: newestFirst ? 1 : 0, - timefrom: timeFrom - }; - - if (limitFrom > 0) { - // Do not use cache when retrieving older messages. - // This is to prevent storing too much data and to prevent inconsistencies between "pages" loaded. - preSets['getFromCache'] = false; - preSets['saveToCache'] = false; - preSets['emergencyCache'] = false; - } - - return site.read('core_message_get_conversation_messages', params, preSets) - .then((result: AddonMessagesGetConversationMessagesResult) => { - - if (limitTo < 1) { - result.canLoadMore = false; - result.messages = result.messages; - } else { - result.canLoadMore = result.messages.length > limitTo; - result.messages = result.messages.slice(0, limitTo); - } - - let lastReceived; - - result.messages.forEach((message) => { - // Convert time to milliseconds. - message.timecreated = message.timecreated ? message.timecreated * 1000 : 0; - - if (!lastReceived && message.useridfrom != userId) { - lastReceived = message; - } - }); - - if (this.appProvider.isDesktop() && limitFrom === 0 && lastReceived) { - // Store the last received message (we cannot know if it's unread or not). Don't block the user for this. - this.storeLastReceivedMessageIfNeeded(conversationId, lastReceived, site.getId()); - } - - if (excludePending) { - // No need to get offline messages, return the ones we have. - return result; - } - - // Get offline messages. - return this.messagesOffline.getConversationMessages(conversationId).then((offlineMessages) => { - // Mark offline messages as pending. - offlineMessages.forEach((message) => { - message.pending = true; - message.useridfrom = userId; - }); - - result.messages = result.messages.concat(offlineMessages); - - return result; - }); - }); }); + + if (this.appProvider.isDesktop() && options.limitFrom === 0 && lastReceived) { + // Store the last received message (we cannot know if it's unread or not). Don't block the user for this. + this.storeLastReceivedMessageIfNeeded(conversationId, lastReceived, site.getId()); + } + + if (options.excludePending) { + // No need to get offline messages, return the ones we have. + return result; + } + + // Get offline messages. + const offlineMessages = await this.messagesOffline.getConversationMessages(conversationId); + + // Mark offline messages as pending. + offlineMessages.forEach((message) => { + message.pending = true; + message.useridfrom = options.userId; + }); + + result.messages = result.messages.concat(offlineMessages); + + return result; } /** @@ -1412,7 +1411,7 @@ export class AddonMessagesProvider { * @param siteId Site ID. If not defined, use current site. * @return Promise resolved with the data. */ - protected getRecentMessages(params: any, preSets: any, limitFromUnread: number = 0, limitFromRead: number = 0, + getRecentMessages(params: any, preSets: any, limitFromUnread: number = 0, limitFromRead: number = 0, toDisplay: boolean = true, siteId?: string): Promise { limitFromUnread = limitFromUnread || 0; limitFromRead = limitFromRead || 0; @@ -2787,6 +2786,21 @@ export class AddonMessagesProvider { } } +/** + * Options to pass to getConversationMessages. + */ +export type AddonMessagesGetConversationMessagesOptions = { + excludePending?: boolean; // True to exclude messages pending to be sent. + limitFrom?: number; // Offset for messages list. Defaults to 0. + limitTo?: number; // Limit of messages. + newestFirst?: boolean; // Whether to order messages by newest first. + timeFrom?: number; // The timestamp from which the messages were created (in seconds). Defaults to 0. + siteId?: string; // Site ID. If not defined, use current site. + userId?: number; // User ID. If not defined, current user in the site. + forceCache?: boolean; // True if it should return cached data. Has priority over ignoreCache. + ignoreCache?: boolean; // True if it should ignore cached data (it will always fail in offline or server down). +}; + /** * Conversation. */ diff --git a/src/addon/messages/providers/sync.ts b/src/addon/messages/providers/sync.ts index 09e024884..379bab7ff 100644 --- a/src/addon/messages/providers/sync.ts +++ b/src/addon/messages/providers/sync.ts @@ -134,112 +134,102 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { * * @param conversationId Conversation ID. * @param userId User ID talking to (if no conversation ID). - * @return Promise resolved if sync is successful, rejected otherwise. + * @param siteId Site ID. + * @return Promise resolved with the list of warnings if sync is successful, rejected otherwise. */ - syncDiscussion(conversationId: number, userId: number, siteId?: string): Promise { + syncDiscussion(conversationId: number, userId: number, siteId?: string): Promise { siteId = siteId || this.sitesProvider.getCurrentSiteId(); - const syncId = this.getSyncId(conversationId, userId), - groupMessagingEnabled = this.messagesProvider.isGroupMessagingEnabled(); + const syncId = this.getSyncId(conversationId, userId); if (this.isSyncing(syncId, siteId)) { // There's already a sync ongoing for this conversation, return the promise. return this.getOngoingSync(syncId, siteId); } - const warnings = []; + return this.addOngoingSync(syncId, this.performSyncDiscussion(conversationId, userId, siteId), siteId); + } + + /** + * Perform the synchronization of a discussion. + * + * @param conversationId Conversation ID. + * @param userId User ID talking to (if no conversation ID). + * @param siteId Site ID. + * @return Promise resolved with the list of warnings if sync is successful, rejected otherwise. + */ + protected async performSyncDiscussion(conversationId: number, userId: number, siteId?: string): Promise { + const groupMessagingEnabled = this.messagesProvider.isGroupMessagingEnabled(); + let messages: any[]; + const errors = []; + const warnings: string[] = []; if (conversationId) { this.logger.debug(`Try to sync conversation '${conversationId}'`); + messages = await this.messagesOffline.getConversationMessages(conversationId, siteId); } else { this.logger.debug(`Try to sync discussion with user '${userId}'`); + messages = await this.messagesOffline.getMessages(userId, siteId); } - // Get offline messages to be sent. - let syncPromise; + if (!messages.length) { + // Nothing to sync. + return []; + } else if (!this.appProvider.isOnline()) { + // Cannot sync in offline. Mark messages as device offline. + this.messagesOffline.setMessagesDeviceOffline(messages, true); - if (conversationId) { - syncPromise = this.messagesOffline.getConversationMessages(conversationId, siteId); - } else { - syncPromise = this.messagesOffline.getMessages(userId, siteId); + return Promise.reject(null); } - syncPromise = syncPromise.then((messages) => { - if (!messages.length) { - // Nothing to sync. - return []; - } else if (!this.appProvider.isOnline()) { - // Cannot sync in offline. Mark messages as device offline. - this.messagesOffline.setMessagesDeviceOffline(messages, true); + // Order message by timecreated. + messages = this.messagesProvider.sortMessages(messages); - return Promise.reject(null); - } + // Send the messages. Send them 1 by 1 to simulate web's behaviour and to make sure we know which message has failed. + for (let i = 0; i < messages.length; i++) { + const message = messages[i]; - let promise: Promise = Promise.resolve(); - const errors = []; - - // Order message by timecreated. - messages = this.messagesProvider.sortMessages(messages); - - // Send the messages. - // 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(() => { - let subPromise; - - if (conversationId) { - subPromise = this.messagesProvider.sendMessageToConversationOnline(conversationId, message.text, siteId); - } else { - subPromise = this.messagesProvider.sendMessageOnline(userId, message.smallmessage, siteId); + try { + if (conversationId) { + await this.messagesProvider.sendMessageToConversationOnline(conversationId, message.text, siteId); + } else { + await this.messagesProvider.sendMessageOnline(userId, message.smallmessage, siteId); + } + } catch (error) { + if (!this.utils.isWebServiceError(error)) { + // Error sending, stop execution. + if (this.appProvider.isOnline()) { + // App is online, unmark deviceoffline if marked. + this.messagesOffline.setMessagesDeviceOffline(messages, false); } - 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) { - errors.push(error); - } + throw error; + } - return; - } + // Error returned by WS. Store the error to show a warning but keep sending messages. + if (errors.indexOf(error) == -1) { + errors.push(error); + } + } - // Error sending, stop execution. - if (this.appProvider.isOnline()) { - // App is online, unmark deviceoffline if marked. - this.messagesOffline.setMessagesDeviceOffline(messages, false); - } + // Message was sent, delete it from local DB. + if (conversationId) { + await this.messagesOffline.deleteConversationMessage(conversationId, message.text, message.timecreated, siteId); + } else { + await this.messagesOffline.deleteMessage(userId, message.smallmessage, message.timecreated, siteId); + } - return Promise.reject(error); - }).then(() => { - // Message was sent, delete it from local DB. - if (conversationId) { - return this.messagesOffline.deleteConversationMessage(conversationId, message.text, - message.timecreated, siteId); - } else { - return this.messagesOffline.deleteMessage(userId, message.smallmessage, message.timecreated, siteId); - } - }).then(() => { - // 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 new Promise((resolve, reject): any => { - setTimeout(resolve, 1000); - }); - } - }); - }); - }); + // 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 && i < messages.length - 1) { + await this.utils.wait(1000); + } + } - return promise; - }).then((errors) => { - return this.handleSyncErrors(conversationId, userId, errors, warnings); - }).then(() => { - // All done, return the warnings. - return warnings; - }); + await this.handleSyncErrors(conversationId, userId, errors, warnings); - return this.addOngoingSync(syncId, syncPromise, siteId); + // All done, return the warnings. + return warnings; } /** @@ -251,7 +241,7 @@ export class AddonMessagesSyncProvider extends CoreSyncBaseProvider { * @param warnings Array where to place the warnings. * @return Promise resolved when done. */ - protected handleSyncErrors(conversationId: number, userId: number, errors: any[], warnings: any[]): Promise { + protected handleSyncErrors(conversationId: number, userId: number, errors: any[], warnings: string[]): Promise { if (errors && errors.length) { if (conversationId) { diff --git a/src/providers/utils/utils.ts b/src/providers/utils/utils.ts index 0efcbe1b6..2172f30f4 100644 --- a/src/providers/utils/utils.ts +++ b/src/providers/utils/utils.ts @@ -1592,6 +1592,18 @@ export class CoreUtilsProvider { // Ignore errors. } } + + /** + * Wait some time. + * + * @param milliseconds Number of milliseconds to wait. + * @return Promise resolved after the time has passed. + */ + wait(milliseconds: number): Promise { + return new Promise((resolve, reject): void => { + setTimeout(resolve, milliseconds); + }); + } } export class CoreUtils extends makeSingleton(CoreUtilsProvider) {}