From 3f25bc2f7c7513d91ed8397fcb3745da8a9fe07d Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Fri, 28 Feb 2020 10:33:39 +0100 Subject: [PATCH] MOBILE-3298 course: Don't fail module download if avatar fails --- .../mod/assign/providers/prefetch-handler.ts | 10 +--- .../mod/chat/providers/prefetch-handler.ts | 4 +- src/addon/mod/forum/providers/forum.ts | 5 +- .../mod/forum/providers/prefetch-handler.ts | 50 ++++++++----------- .../glossary/providers/prefetch-handler.ts | 24 ++++----- .../workshop/providers/prefetch-handler.ts | 4 +- src/core/user/providers/user.ts | 37 +++++++++++++- 7 files changed, 73 insertions(+), 61 deletions(-) diff --git a/src/addon/mod/assign/providers/prefetch-handler.ts b/src/addon/mod/assign/providers/prefetch-handler.ts index ca7865ea0..e67846720 100644 --- a/src/addon/mod/assign/providers/prefetch-handler.ts +++ b/src/addon/mod/assign/providers/prefetch-handler.ts @@ -354,15 +354,7 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan }).then(() => { // Participiants already fetched, we don't need to ignore cache now. return this.assignHelper.getParticipants(assign, group.id, false, siteId).then((participants) => { - const promises = []; - - participants.forEach((participant) => { - if (participant.profileimageurl) { - promises.push(this.filepoolProvider.addToQueueByUrl(siteId, participant.profileimageurl)); - } - }); - - return Promise.all(promises); + return this.userProvider.prefetchUserAvatars(participants, 'profileimageurl', siteId); }).catch(() => { // Fail silently (Moodle < 3.2). }); diff --git a/src/addon/mod/chat/providers/prefetch-handler.ts b/src/addon/mod/chat/providers/prefetch-handler.ts index ecf8cde63..f5c5900b1 100644 --- a/src/addon/mod/chat/providers/prefetch-handler.ts +++ b/src/addon/mod/chat/providers/prefetch-handler.ts @@ -182,9 +182,7 @@ export class AddonModChatPrefetchHandler extends CoreCourseActivityPrefetchHandl }); const userIds = Object.keys(users).map(Number); - return this.userProvider.prefetchProfiles(userIds, courseId, siteId).catch(() => { - // Ignore errors, some users might not exist. - }); + return this.userProvider.prefetchProfiles(userIds, courseId, siteId); }); } } diff --git a/src/addon/mod/forum/providers/forum.ts b/src/addon/mod/forum/providers/forum.ts index 96fd64ce1..40bfeb59f 100644 --- a/src/addon/mod/forum/providers/forum.ts +++ b/src/addon/mod/forum/providers/forum.ts @@ -244,13 +244,14 @@ export class AddonModForumProvider { * Check if a user can post to all groups. * * @param forumId Forum ID. + * @param siteId Site ID. If not defined, current site. * @return Promise resolved with an object with the following properties: * - status (boolean) * - canpindiscussions (boolean) * - cancreateattachment (boolean) */ - canAddDiscussionToAll(forumId: number): Promise { - return this.canAddDiscussion(forumId, AddonModForumProvider.ALL_PARTICIPANTS); + canAddDiscussionToAll(forumId: number, siteId?: string): Promise { + return this.canAddDiscussion(forumId, AddonModForumProvider.ALL_PARTICIPANTS, siteId); } /** diff --git a/src/addon/mod/forum/providers/prefetch-handler.ts b/src/addon/mod/forum/providers/prefetch-handler.ts index 743d99b97..bf741d780 100644 --- a/src/addon/mod/forum/providers/prefetch-handler.ts +++ b/src/addon/mod/forum/providers/prefetch-handler.ts @@ -107,12 +107,13 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand * Get the posts to be prefetched. * * @param forum Forum instance. + * @param siteId Site ID. If not defined, current site. * @return Promise resolved with array of posts. */ - protected getPostsForPrefetch(forum: any): Promise { + protected getPostsForPrefetch(forum: any, siteId?: string): Promise { const promises = this.forumProvider.getAvailableSortOrders().map((sortOrder) => { // Get discussions in first 2 pages. - return this.forumProvider.getDiscussionsInPages(forum.id, sortOrder.value, false, 2).then((response) => { + return this.forumProvider.getDiscussionsInPages(forum.id, sortOrder.value, false, 2, 0, siteId).then((response) => { if (response.error) { return Promise.reject(null); } @@ -120,7 +121,7 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand const promises = []; response.discussions.forEach((discussion) => { - promises.push(this.forumProvider.getDiscussionPosts(discussion.discussion)); + promises.push(this.forumProvider.getDiscussionPosts(discussion.discussion, siteId)); }); return Promise.all(promises); @@ -200,41 +201,31 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand */ protected prefetchForum(module: any, courseId: number, single: boolean, siteId: string): Promise { // Get the forum data. - return this.forumProvider.getForum(courseId, module.id).then((forum) => { + return this.forumProvider.getForum(courseId, module.id, siteId).then((forum) => { const promises = []; // Prefetch the posts. - promises.push(this.getPostsForPrefetch(forum).then((posts) => { + promises.push(this.getPostsForPrefetch(forum, siteId).then((posts) => { const promises = []; - // Gather user profile images. - const avatars = {}; // List of user avatars, preventing duplicates. - - posts.forEach((post) => { - if (post.userpictureurl) { - avatars[post.userpictureurl] = true; - } - }); - - // Prefetch intro files, attachments, embedded files and user avatars. - const avatarFiles = Object.keys(avatars).map((url) => { - return { fileurl: url }; - }); - const files = this.getIntroFilesFromInstance(module, forum).concat(this.getPostsFiles(posts)).concat(avatarFiles); + const files = this.getIntroFilesFromInstance(module, forum).concat(this.getPostsFiles(posts)); promises.push(this.filepoolProvider.addFilesToQueue(siteId, files, this.component, module.id)); // Prefetch groups data. - promises.push(this.prefetchGroupsInfo(forum, courseId, forum.cancreatediscussions)); + promises.push(this.prefetchGroupsInfo(forum, courseId, forum.cancreatediscussions, siteId)); + + // Prefetch avatars. + promises.push(this.userProvider.prefetchUserAvatars(posts, 'userpictureurl', siteId)); return Promise.all(promises); })); // Prefetch access information. - promises.push(this.forumProvider.getAccessInformation(forum.id)); + promises.push(this.forumProvider.getAccessInformation(forum.id, false, siteId)); // Prefetch sort order preference. if (this.forumProvider.isDiscussionListSortingAvailable()) { - promises.push(this.userProvider.getUserPreference(AddonModForumProvider.PREFERENCE_SORTORDER)); + promises.push(this.userProvider.getUserPreference(AddonModForumProvider.PREFERENCE_SORTORDER, siteId)); } return Promise.all(promises); @@ -247,30 +238,31 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand * @param module The module object returned by WS. * @param courseI Course ID the module belongs to. * @param canCreateDiscussions Whether the user can create discussions in the forum. + * @param siteId Site ID. If not defined, current site. * @return Promise resolved when group data has been prefetched. */ - protected prefetchGroupsInfo(forum: any, courseId: number, canCreateDiscussions: boolean): any { + protected prefetchGroupsInfo(forum: any, courseId: number, canCreateDiscussions: boolean, siteId?: string): any { // Check group mode. - return this.groupsProvider.getActivityGroupMode(forum.cmid).then((mode) => { + return this.groupsProvider.getActivityGroupMode(forum.cmid, siteId).then((mode) => { if (mode !== CoreGroupsProvider.SEPARATEGROUPS && mode !== CoreGroupsProvider.VISIBLEGROUPS) { // Activity doesn't use groups. Prefetch canAddDiscussionToAll to determine if user can pin/attach. - return this.forumProvider.canAddDiscussionToAll(forum.id).catch(() => { + return this.forumProvider.canAddDiscussionToAll(forum.id, siteId).catch(() => { // Ignore errors. }); } // Activity uses groups, prefetch allowed groups. - return this.groupsProvider.getActivityAllowedGroups(forum.cmid).then((result) => { + return this.groupsProvider.getActivityAllowedGroups(forum.cmid, undefined, siteId).then((result) => { if (mode === CoreGroupsProvider.SEPARATEGROUPS) { // Groups are already filtered by WS. Prefetch canAddDiscussionToAll to determine if user can pin/attach. - return this.forumProvider.canAddDiscussionToAll(forum.id).catch(() => { + return this.forumProvider.canAddDiscussionToAll(forum.id, siteId).catch(() => { // Ignore errors. }); } if (canCreateDiscussions) { // Prefetch data to check the visible groups when creating discussions. - return this.forumProvider.canAddDiscussionToAll(forum.id).catch(() => { + return this.forumProvider.canAddDiscussionToAll(forum.id, siteId).catch(() => { // The call failed, let's assume he can't. return { status: false @@ -284,7 +276,7 @@ export class AddonModForumPrefetchHandler extends CoreCourseActivityPrefetchHand // The user can't post to all groups, let's check which groups he can post to. const groupPromises = []; result.groups.forEach((group) => { - groupPromises.push(this.forumProvider.canAddDiscussion(forum.id, group.id).catch(() => { + groupPromises.push(this.forumProvider.canAddDiscussion(forum.id, group.id, siteId).catch(() => { // Ignore errors. })); }); diff --git a/src/addon/mod/glossary/providers/prefetch-handler.ts b/src/addon/mod/glossary/providers/prefetch-handler.ts index 590294ad7..30bdf4903 100644 --- a/src/addon/mod/glossary/providers/prefetch-handler.ts +++ b/src/addon/mod/glossary/providers/prefetch-handler.ts @@ -26,6 +26,7 @@ import { AddonModGlossaryProvider } from './glossary'; import { AddonModGlossarySyncProvider } from './sync'; import { CoreFilterHelperProvider } from '@core/filter/providers/helper'; import { CorePluginFileDelegate } from '@providers/plugin-file-delegate'; +import { CoreUserProvider } from '@core/user/providers/user'; /** * Handler to prefetch forums. @@ -48,7 +49,8 @@ export class AddonModGlossaryPrefetchHandler extends CoreCourseActivityPrefetchH pluginFileDelegate: CorePluginFileDelegate, protected glossaryProvider: AddonModGlossaryProvider, protected commentsProvider: CoreCommentsProvider, - protected syncProvider: AddonModGlossarySyncProvider) { + protected syncProvider: AddonModGlossarySyncProvider, + protected userProvider: CoreUserProvider) { super(translate, appProvider, utils, courseProvider, filepoolProvider, sitesProvider, domUtils, filterHelper, pluginFileDelegate); @@ -165,35 +167,29 @@ export class AddonModGlossaryPrefetchHandler extends CoreCourseActivityPrefetchH // Fetch all entries to get information from. promises.push(this.glossaryProvider.fetchAllEntries(this.glossaryProvider.getEntriesByLetter, [glossary.id, 'ALL'], false, false, siteId).then((entries) => { - const promises = [], - commentsEnabled = !this.commentsProvider.areCommentsDisabledInSite(), - avatars = {}; // List of user avatars, preventing duplicates. + const promises = []; + const commentsEnabled = !this.commentsProvider.areCommentsDisabledInSite(); entries.forEach((entry) => { // Don't fetch individual entries, it's too many WS calls. - if (entry.userpictureurl) { - avatars[entry.userpictureurl] = true; - } - if (glossary.allowcomments && commentsEnabled) { promises.push(this.commentsProvider.getComments('module', glossary.coursemodule, 'mod_glossary', entry.id, 'glossary_entry', 0, siteId)); } }); - // Prefetch intro files, entries files and user avatars. - const avatarFiles = Object.keys(avatars).map((url) => { - return { fileurl: url }; - }); - const files = this.getFilesFromGlossaryAndEntries(module, glossary, entries).concat(avatarFiles); + const files = this.getFilesFromGlossaryAndEntries(module, glossary, entries); promises.push(this.filepoolProvider.addFilesToQueue(siteId, files, this.component, module.id)); + // Prefetch user avatars. + promises.push(this.userProvider.prefetchUserAvatars(entries, 'userpictureurl', siteId)); + return Promise.all(promises); })); // Get all categories. - promises.push(this.glossaryProvider.getAllCategories(glossary.id)); + promises.push(this.glossaryProvider.getAllCategories(glossary.id, siteId)); // Prefetch data for link handlers. promises.push(this.courseProvider.getModuleBasicInfo(module.id, siteId)); diff --git a/src/addon/mod/workshop/providers/prefetch-handler.ts b/src/addon/mod/workshop/providers/prefetch-handler.ts index cf7b99d42..1213804c6 100644 --- a/src/addon/mod/workshop/providers/prefetch-handler.ts +++ b/src/addon/mod/workshop/providers/prefetch-handler.ts @@ -375,9 +375,7 @@ export class AddonModWorkshopPrefetchHandler extends CoreCourseActivityPrefetchH }); }).then(() => { // Prefetch user profiles. - return this.userProvider.prefetchProfiles(userIds, courseId, siteId).catch(() => { - // Ignore errors. - }); + return this.userProvider.prefetchProfiles(userIds, courseId, siteId); }); } diff --git a/src/core/user/providers/user.ts b/src/core/user/providers/user.ts index fc685dadb..1bbb23c80 100644 --- a/src/core/user/providers/user.ts +++ b/src/core/user/providers/user.ts @@ -522,8 +522,10 @@ export class CoreUserProvider { promises.push(this.getProfile(userId, courseId, false, siteId).then((profile) => { if (profile.profileimageurl) { - this.filepoolProvider.addToQueueByUrl(siteId, profile.profileimageurl); + return this.filepoolProvider.addToQueueByUrl(siteId, profile.profileimageurl); } + }).catch((error) => { + this.logger.warn(`Ignore error when prefetching user ${userId}`, error); })); } }); @@ -531,6 +533,39 @@ export class CoreUserProvider { return Promise.all(promises); } + /** + * Prefetch user avatars. It prevents duplicates. + * + * @param entries List of entries that have the images. + * @param propertyName The name of the property that contains the image. + * @param siteId Site ID. If not defined, current site. + * @return Promise resolved when prefetched. + */ + async prefetchUserAvatars(entries: any[], propertyName: string, siteId?: string): Promise { + siteId = siteId || this.sitesProvider.getCurrentSiteId(); + + const treated = {}; + + const promises = entries.map(async (entry) => { + const imageUrl = entry[propertyName]; + + if (!imageUrl || treated[imageUrl]) { + // It doesn't have an image or it has already been treated. + return; + } + + treated[imageUrl] = true; + + try { + await this.filepoolProvider.addToQueueByUrl(siteId, imageUrl); + } catch (ex) { + this.logger.warn(`Ignore error when prefetching user avatar ${imageUrl}`, entry, ex); + } + }); + + await Promise.all(promises); + } + /** * Search participants in a certain course. *