From 88137554d858cb4939a509b60983422fb8f4e648 Mon Sep 17 00:00:00 2001 From: Albert Gasset Date: Tue, 2 Apr 2019 16:02:27 +0200 Subject: [PATCH] MOBILE-2948 assign: Fix prefetch for teachers --- .../pages/submission-list/submission-list.ts | 36 +++--- src/addon/mod/assign/providers/assign.ts | 118 ------------------ src/addon/mod/assign/providers/helper.ts | 97 +++++++++++++- .../mod/assign/providers/prefetch-handler.ts | 70 ++++++----- 4 files changed, 148 insertions(+), 173 deletions(-) diff --git a/src/addon/mod/assign/pages/submission-list/submission-list.ts b/src/addon/mod/assign/pages/submission-list/submission-list.ts index 8ce653613..abf2cd778 100644 --- a/src/addon/mod/assign/pages/submission-list/submission-list.ts +++ b/src/addon/mod/assign/pages/submission-list/submission-list.ts @@ -55,7 +55,7 @@ export class AddonModAssignSubmissionListPage implements OnInit, OnDestroy { protected gradedObserver; // Observer to refresh data when a grade changes. protected submissionsData: any; - constructor(navParams: NavParams, sitesProvider: CoreSitesProvider, eventsProvider: CoreEventsProvider, + constructor(navParams: NavParams, protected sitesProvider: CoreSitesProvider, eventsProvider: CoreEventsProvider, protected domUtils: CoreDomUtilsProvider, protected translate: TranslateService, protected assignProvider: AddonModAssignProvider, protected assignOfflineProvider: AddonModAssignOfflineProvider, protected assignHelper: AddonModAssignHelperProvider, protected groupsProvider: CoreGroupsProvider) { @@ -142,32 +142,26 @@ export class AddonModAssignSubmissionListPage implements OnInit, OnDestroy { * @return {Promise} Resolved when done. */ setGroup(groupId: number): Promise { - let participants, - grades; - this.groupId = groupId; this.haveAllParticipants = true; - // Get the participants. - return this.assignHelper.getParticipants(this.assign, this.groupId).then((parts) => { - this.haveAllParticipants = true; - participants = parts; - }).catch(() => { + if (!this.sitesProvider.getCurrentSite().wsAvailable('mod_assign_list_participants')) { + // Submissions are not displayed in Moodle 3.1 without the local plugin, see MOBILE-2968. this.haveAllParticipants = false; - }).then(() => { - if (!this.assign.markingworkflow) { - // Get assignment grades only if workflow is not enabled to check grading date. - return this.assignProvider.getAssignmentGrades(this.assign.id).then((assignmentGrades) => { - grades = assignmentGrades; - }); - } - }).then(() => { - // We want to show the user data on each submission. - return this.assignProvider.getSubmissionsUserData(this.submissionsData.submissions, this.courseId, this.assign.id, - this.assign.blindmarking && !this.assign.revealidentities, participants); - }).then((submissions) => { + this.submissions = []; + return Promise.resolve(); + } + + // Fetch submissions and grades. + const promises = [ + this.assignHelper.getSubmissionsUserData(this.assign, this.submissionsData.submissions, this.groupId), + // Get assignment grades only if workflow is not enabled to check grading date. + !this.assign.markingworkflow ? this.assignProvider.getAssignmentGrades(this.assign.id) : Promise.resolve(null), + ]; + + return Promise.all(promises).then(([submissions, grades]) => { // Filter the submissions to get only the ones with the right status and add some extra data. const getNeedGrading = this.selectedStatus == AddonModAssignProvider.NEED_GRADING, searchStatus = getNeedGrading ? AddonModAssignProvider.SUBMISSION_STATUS_SUBMITTED : this.selectedStatus, diff --git a/src/addon/mod/assign/providers/assign.ts b/src/addon/mod/assign/providers/assign.ts index 93e8204f0..461548f7e 100644 --- a/src/addon/mod/assign/providers/assign.ts +++ b/src/addon/mod/assign/providers/assign.ts @@ -315,27 +315,6 @@ export class AddonModAssignProvider { return this.ROOT_CACHE_KEY + 'assigngrades:' + assignId; } - /** - * Find participant on a list. - * - * @param {any[]} participants List of participants. - * @param {number} id ID of the participant to get. - * @return {any} Participant, undefined if not found. - */ - protected getParticipantFromUserId(participants: any[], id: number): any { - if (participants) { - for (const i in participants) { - if (participants[i].id == id) { - // Remove the participant from the list and return it. - const participant = participants[i]; - delete participants[i]; - - return participant; - } - } - } - } - /** * Returns the color name for a given grading status name. * @@ -616,103 +595,6 @@ export class AddonModAssignProvider { } } - /** - * Get user data for submissions since they only have userid. - * - * @param {any[]} submissions Submissions to get the data for. - * @param {number} courseId ID of the course the submissions belong to. - * @param {number} assignId ID of the assignment the submissions belong to. - * @param {boolean} [blind] Whether the user data need to be blinded. - * @param {any[]} [participants] List of participants in the assignment. - * @param {boolean} [ignoreCache] True if it should ignore cached data (it will always fail in offline or server down). - * @param {string} [siteId] Site id (empty for current site). - * @return {Promise} Promise always resolved. Resolve param is the formatted submissions. - */ - getSubmissionsUserData(submissions: any[], courseId: number, assignId: number, blind?: boolean, participants?: any[], - ignoreCache?: boolean, siteId?: string): Promise { - - const promises = [], - subs = [], - hasParticipants = participants && participants.length > 0; - - if (!hasParticipants) { - return Promise.resolve([]); - } - - submissions.forEach((submission) => { - submission.submitid = submission.userid > 0 ? submission.userid : submission.blindid; - if (submission.submitid <= 0) { - return; - } - - const participant = this.getParticipantFromUserId(participants, submission.submitid); - if (!participant) { - // Avoid permission denied error. Participant not found on list. - return; - } - - if (!blind) { - submission.userfullname = participant.fullname; - submission.userprofileimageurl = participant.profileimageurl; - } - - submission.manyGroups = !!participant.groups && participant.groups.length > 1; - if (participant.groupname) { - submission.groupid = participant.groupid; - submission.groupname = participant.groupname; - } - - let promise; - if (submission.userid > 0 && blind) { - // Blind but not blinded! (Moodle < 3.1.1, 3.2). - delete submission.userid; - - promise = this.getAssignmentUserMappings(assignId, submission.submitid, ignoreCache, siteId).then((blindId) => { - submission.blindid = blindId; - }); - } - - promise = promise || Promise.resolve(); - - promises.push(promise.then(() => { - // Add to the list. - if (submission.userfullname || submission.blindid) { - subs.push(submission); - } - })); - }); - - return Promise.all(promises).then(() => { - if (hasParticipants) { - // Create a submission for each participant left in the list (the participants already treated were removed). - participants.forEach((participant) => { - const submission: any = { - submitid: participant.id - }; - - if (!blind) { - submission.userid = participant.id; - submission.userfullname = participant.fullname; - submission.userprofileimageurl = participant.profileimageurl; - } else { - submission.blindid = participant.id; - } - - if (participant.groupname) { - submission.groupid = participant.groupid; - submission.groupname = participant.groupname; - } - submission.status = participant.submitted ? AddonModAssignProvider.SUBMISSION_STATUS_SUBMITTED : - AddonModAssignProvider.SUBMISSION_STATUS_NEW; - - subs.push(submission); - }); - } - - return subs; - }); - } - /** * Given a list of plugins, returns the plugin names that aren't supported for editing. * diff --git a/src/addon/mod/assign/providers/helper.ts b/src/addon/mod/assign/providers/helper.ts index 6b64d5c1b..7942f91bf 100644 --- a/src/addon/mod/assign/providers/helper.ts +++ b/src/addon/mod/assign/providers/helper.ts @@ -153,7 +153,7 @@ export class AddonModAssignHelperProvider { * @param {number} [groupId] Group Id. * @param {boolean} [ignoreCache] True if it should ignore cached data (it will always fail in offline or server down). * @param {string} [siteId] Site ID. If not defined, current site. - * @return {Promise} Promise resolved with the list of participants and summary of submissions. */ getParticipants(assign: any, groupId?: number, ignoreCache?: boolean, siteId?: string): Promise { groupId = groupId || 0; @@ -288,6 +288,101 @@ export class AddonModAssignHelperProvider { }); } + /** + * Get user data for submissions since they only have userid. + * + * @param {any} assign Assignment object. + * @param {any[]} submissions Submissions to get the data for. + * @param {number} [groupId] Group Id. + * @param {boolean} [ignoreCache] True if it should ignore cached data (it will always fail in offline or server down). + * @param {string} [siteId] Site id (empty for current site). + * @return {Promise} Promise always resolved. Resolve param is the formatted submissions. + */ + getSubmissionsUserData(assign: any, submissions: any[], groupId?: number, ignoreCache?: boolean, siteId?: string): + Promise { + return this.getParticipants(assign, groupId).then((participants) => { + const blind = assign.blindmarking && !assign.revealidentities; + const promises = []; + const result = []; + + participants = this.utils.arrayToObject(participants, 'id'); + + submissions.forEach((submission) => { + submission.submitid = submission.userid > 0 ? submission.userid : submission.blindid; + if (submission.submitid <= 0) { + return; + } + + const participant = participants[submission.submitid]; + if (participant) { + delete participants[submission.submitid]; + } else { + // Avoid permission denied error. Participant not found on list. + return; + } + + if (!blind) { + submission.userfullname = participant.fullname; + submission.userprofileimageurl = participant.profileimageurl; + } + + submission.manyGroups = !!participant.groups && participant.groups.length > 1; + if (participant.groupname) { + submission.groupid = participant.groupid; + submission.groupname = participant.groupname; + } + + let promise; + if (submission.userid > 0 && blind) { + // Blind but not blinded! (Moodle < 3.1.1, 3.2). + delete submission.userid; + + promise = this.assignProvider.getAssignmentUserMappings(assign.id, submission.submitid, ignoreCache, siteId). + then((blindId) => { + submission.blindid = blindId; + }); + } + + promise = promise || Promise.resolve(); + + promises.push(promise.then(() => { + // Add to the list. + if (submission.userfullname || submission.blindid) { + result.push(submission); + } + })); + }); + + return Promise.all(promises).then(() => { + // Create a submission for each participant left in the list (the participants already treated were removed). + this.utils.objectToArray(participants).forEach((participant) => { + const submission: any = { + submitid: participant.id + }; + + if (!blind) { + submission.userid = participant.id; + submission.userfullname = participant.fullname; + submission.userprofileimageurl = participant.profileimageurl; + } else { + submission.blindid = participant.id; + } + + if (participant.groupname) { + submission.groupid = participant.groupid; + submission.groupname = participant.groupname; + } + submission.status = participant.submitted ? AddonModAssignProvider.SUBMISSION_STATUS_SUBMITTED : + AddonModAssignProvider.SUBMISSION_STATUS_NEW; + + result.push(submission); + }); + + return result; + }); + }); + } + /** * Check if the feedback data has changed for a certain submission and assign. * diff --git a/src/addon/mod/assign/providers/prefetch-handler.ts b/src/addon/mod/assign/providers/prefetch-handler.ts index cae9b6fec..7837ee180 100644 --- a/src/addon/mod/assign/providers/prefetch-handler.ts +++ b/src/addon/mod/assign/providers/prefetch-handler.ts @@ -103,8 +103,8 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan if (data.canviewsubmissions) { // Teacher, get all submissions. - return this.assignProvider.getSubmissionsUserData(data.submissions, courseId, assign.id, blindMarking, - undefined, false, siteId).then((submissions) => { + return this.assignHelper.getSubmissionsUserData(assign, data.submissions, 0, false, siteId) + .then((submissions) => { const promises = []; @@ -289,11 +289,10 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan protected prefetchSubmissions(assign: any, courseId: number, moduleId: number, userId: number, siteId: string): Promise { // Get submissions. return this.assignProvider.getSubmissions(assign.id, true, siteId).then((data) => { - const promises = [], - blindMarking = assign.blindmarking && !assign.revealidentities; + const promises = []; if (data.canviewsubmissions) { - // Teacher. Do not send participants to getSubmissionsUserData to retrieve user profiles. + // Teacher, prefetch all submissions. promises.push(this.groupsProvider.getActivityGroupInfo(assign.cmid, false, undefined, siteId).then((groupInfo) => { const groupProms = []; if (!groupInfo.groups || groupInfo.groups.length == 0) { @@ -301,8 +300,8 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan } groupInfo.groups.forEach((group) => { - groupProms.push(this.assignProvider.getSubmissionsUserData(data.submissions, courseId, assign.id, - blindMarking, undefined, true, siteId).then((submissions) => { + groupProms.push(this.assignHelper.getSubmissionsUserData(assign, data.submissions, group.id, true, siteId) + .then((submissions) => { const subPromises = []; @@ -334,37 +333,41 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan } return Promise.all(subPromises); - })); + }).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 = []; - // Get list participants. - groupProms.push(this.assignHelper.getParticipants(assign, group.id, true, siteId).then((participants) => { - participants.forEach((participant) => { - if (participant.profileimageurl) { - this.filepoolProvider.addToQueueByUrl(siteId, participant.profileimageurl); - } + participants.forEach((participant) => { + if (participant.profileimageurl) { + promises.push(this.filepoolProvider.addToQueueByUrl(siteId, participant.profileimageurl)); + } + }); + + return Promise.all(promises); + }).catch(() => { + // Fail silently (Moodle < 3.2). }); - }).catch(() => { - // Fail silently (Moodle < 3.2). })); }); return Promise.all(groupProms); })); - } else { - // Student. - promises.push( - this.assignProvider.getSubmissionStatusWithRetry(assign, userId, undefined, false, true, true, siteId) - .then((subm) => { - return this.prefetchSubmission(assign, courseId, moduleId, subm, userId, siteId); - }).catch((error) => { - // Ignore if the user can't view their own submission. - if (error.errorcode != 'nopermission') { - return Promise.reject(error); - } - }) - ); } + // Prefetch own submission, we need to do this for teachers too so the response with error is cached. + promises.push( + this.assignProvider.getSubmissionStatusWithRetry(assign, userId, undefined, false, true, true, siteId) + .then((subm) => { + return this.prefetchSubmission(assign, courseId, moduleId, subm, userId, siteId); + }).catch((error) => { + // Ignore if the user can't view their own submission. + if (error.errorcode != 'nopermission') { + return Promise.reject(error); + } + }) + ); + promises.push(this.groupsProvider.activityHasGroups(assign.cmid, siteId, true)); promises.push(this.groupsProvider.getActivityAllowedGroups(assign.cmid, undefined, siteId, true)); @@ -422,6 +425,11 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan } } + // Prefetch grade items. + if (userId) { + promises.push(this.gradesHelper.getGradeModuleItems(courseId, moduleId, userId, undefined, siteId, true)); + } + // Prefetch feedback. if (submission.feedback) { // Get profile and image of the grader. @@ -429,10 +437,6 @@ export class AddonModAssignPrefetchHandler extends CoreCourseActivityPrefetchHan userIds.push(submission.feedback.grade.grader); } - if (userId) { - promises.push(this.gradesHelper.getGradeModuleItems(courseId, moduleId, userId, undefined, siteId, true)); - } - // Prefetch feedback plugins data. if (submission.feedback.plugins) { submission.feedback.plugins.forEach((plugin) => {