From 5337c77b7e042a8386474009df88156134d75071 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Tue, 10 Dec 2019 13:39:55 +0100 Subject: [PATCH 1/3] MOBILE-3245 grades: Fix course data population in grades --- src/core/course/providers/helper.ts | 4 +- src/core/courses/providers/courses.ts | 11 +++-- src/core/courses/providers/helper.ts | 2 +- src/core/grades/providers/helper.ts | 58 ++++++++++++++++++++------- src/providers/sites.ts | 32 ++++++++++++++- 5 files changed, 84 insertions(+), 23 deletions(-) diff --git a/src/core/course/providers/helper.ts b/src/core/course/providers/helper.ts index 82b9d03d6..580971705 100644 --- a/src/core/course/providers/helper.ts +++ b/src/core/course/providers/helper.ts @@ -919,7 +919,7 @@ export class CoreCourseHelperProvider { const totalOffline = offlineCompletions.length; let loaded = 0; - offlineCompletions = this.utils.arrayToObject(offlineCompletions, 'cmid'); + const offlineCompletionsMap = this.utils.arrayToObject(offlineCompletions, 'cmid'); // Load the offline data in the modules. for (let i = 0; i < sections.length; i++) { @@ -931,7 +931,7 @@ export class CoreCourseHelperProvider { for (let j = 0; j < section.modules.length; j++) { const module = section.modules[j], - offlineCompletion = offlineCompletions[module.id]; + offlineCompletion = offlineCompletionsMap[module.id]; if (offlineCompletion && typeof module.completiondata != 'undefined' && offlineCompletion.timecompleted >= module.completiondata.timecompleted * 1000) { diff --git a/src/core/courses/providers/courses.ts b/src/core/courses/providers/courses.ts index 226dca101..f869e467a 100644 --- a/src/core/courses/providers/courses.ts +++ b/src/core/courses/providers/courses.ts @@ -15,7 +15,7 @@ import { Injectable } from '@angular/core'; import { CoreEventsProvider } from '@providers/events'; import { CoreLoggerProvider } from '@providers/logger'; -import { CoreSitesProvider } from '@providers/sites'; +import { CoreSitesProvider, ReadingStrategy } from '@providers/sites'; import { CoreSite } from '@classes/site'; /** @@ -774,18 +774,21 @@ export class CoreCoursesProvider { * @param siteId Site to get the courses from. If not defined, use current site. * @return Promise resolved with the courses. */ - getUserCourses(preferCache?: boolean, siteId?: string): Promise { + getUserCourses(preferCache?: boolean, siteId?: string, strategy?: ReadingStrategy): Promise { return this.sitesProvider.getSite(siteId).then((site) => { const userId = site.getUserId(), data: any = { userid: userId }, + strategyPreSets = strategy + ? this.sitesProvider.getReadingStrategyPreSets(strategy) + : { omitExpires: !!preferCache }, preSets = { cacheKey: this.getUserCoursesCacheKey(), getCacheUsingCacheKey: true, - omitExpires: !!preferCache, - updateFrequency: CoreSite.FREQUENCY_RARELY + updateFrequency: CoreSite.FREQUENCY_RARELY, + ...strategyPreSets, }; if (site.isVersionGreaterEqualThan('3.7')) { diff --git a/src/core/courses/providers/helper.ts b/src/core/courses/providers/helper.ts index 0d98fe61e..30e3ab092 100644 --- a/src/core/courses/providers/helper.ts +++ b/src/core/courses/providers/helper.ts @@ -107,7 +107,7 @@ export class CoreCoursesHelperProvider { return Promise.resolve(); } - let coursesInfo = [], + let coursesInfo = {}, courseInfoAvailable = false; const site = this.sitesProvider.getCurrentSite(), diff --git a/src/core/grades/providers/helper.ts b/src/core/grades/providers/helper.ts index 236f05cc8..889523c56 100644 --- a/src/core/grades/providers/helper.ts +++ b/src/core/grades/providers/helper.ts @@ -15,7 +15,7 @@ import { Injectable } from '@angular/core'; import { NavController } from 'ionic-angular'; import { CoreLoggerProvider } from '@providers/logger'; -import { CoreSitesProvider } from '@providers/sites'; +import { CoreSitesProvider, ReadingStrategy } from '@providers/sites'; import { CoreCoursesProvider } from '@core/courses/providers/courses'; import { CoreCourseProvider } from '@core/course/providers/course'; import { CoreGradesProvider } from './grades'; @@ -198,22 +198,50 @@ export class CoreGradesHelperProvider { * @param grades Grades to get the data for. * @return Promise always resolved. Resolve param is the formatted grades. */ - getGradesCourseData(grades: any): Promise { - // Using cache for performance reasons. - return this.coursesProvider.getUserCourses(true).then((courses) => { - const indexedCourses = {}; - courses.forEach((course) => { - indexedCourses[course.id] = course; - }); + async getGradesCourseData(grades: any[]): Promise { + // Obtain courses from cache to prevent network requests. + const courses = await this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.OnlyCache); - grades.forEach((grade) => { - if (typeof indexedCourses[grade.courseid] != 'undefined') { - grade.courseFullName = indexedCourses[grade.courseid].fullname; - } - }); + const coursesMap = this.utils.arrayToObject(courses, 'id'); + const coursesWereMissing = this.addCourseData(grades, coursesMap); - return grades; - }); + // If any course wasn't found, make a network request. + if (coursesWereMissing) { + const coursesPromise = this.coursesProvider.isGetCoursesByFieldAvailable() + ? this.coursesProvider.getCoursesByField('ids', grades.map((grade) => grade.courseid)) + : this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.PreferNetwork); + + const courses = await coursesPromise; + + const coursesMap = this.utils.arrayToObject(courses, 'id'); + + this.addCourseData(grades, coursesMap); + } + + return grades.filter((grade) => grade.courseFullName !== undefined); + } + + /** + * Adds course data to grades. + * + * @param grades Array of grades to populate. + * @param courses HashMap of courses to read data from. + * @return Boolean indicating if some courses were not found. + */ + protected addCourseData(grades: any[], courses: any): boolean { + let someCoursesAreMissing = false; + + for (const grade of grades) { + if (!(grade.courseid in courses)) { + someCoursesAreMissing = true; + + continue; + } + + grade.courseFullName = courses[grade.courseid].fullname; + } + + return someCoursesAreMissing; } /** diff --git a/src/providers/sites.ts b/src/providers/sites.ts index 857a52058..0094d7096 100644 --- a/src/providers/sites.ts +++ b/src/providers/sites.ts @@ -26,7 +26,7 @@ import { CoreUtilsProvider } from './utils/utils'; import { CoreWSProvider } from './ws'; import { CoreConstants } from '@core/constants'; import { CoreConfigConstants } from '../configconstants'; -import { CoreSite } from '@classes/site'; +import { CoreSite, CoreSiteWSPreSets } from '@classes/site'; import { SQLiteDB, SQLiteDBTableSchema } from '@classes/sqlitedb'; import { Md5 } from 'ts-md5/dist/md5'; import { WP_PROVIDER } from '@app/app.module'; @@ -158,6 +158,12 @@ export interface CoreSiteSchema { migrate?(db: SQLiteDB, oldVersion: number, siteId: string): Promise | void; } +export const enum ReadingStrategy { + OnlyCache, + PreferCache, + PreferNetwork, +} + /* * Service to manage and interact with sites. * It allows creating tables in the databases of all sites. Each service or component should be responsible of creating @@ -1730,4 +1736,28 @@ export class CoreSitesProvider { return reset; } + + /** + * Returns presets for a given reading strategy. + * + * @param strategy Reading strategy. + * @return PreSets options object. + */ + getReadingStrategyPreSets(strategy: ReadingStrategy): CoreSiteWSPreSets { + switch (strategy) { + case ReadingStrategy.PreferCache: + return { + omitExpires: true, + }; + case ReadingStrategy.OnlyCache: + return { + omitExpires: true, + forceOffline: true, + }; + case ReadingStrategy.PreferNetwork: + default: + return {}; + } + } + } From accb83b3394b52d6897a391f36b7aa9c65efebe2 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Wed, 11 Dec 2019 13:32:00 +0100 Subject: [PATCH 2/3] MOBILE-3245 grades: Fix network methods calls when populating grades --- src/core/grades/providers/helper.ts | 15 +++++++++++---- 1 file changed, 11 insertions(+), 4 deletions(-) diff --git a/src/core/grades/providers/helper.ts b/src/core/grades/providers/helper.ts index 889523c56..ae8cc9071 100644 --- a/src/core/grades/providers/helper.ts +++ b/src/core/grades/providers/helper.ts @@ -200,15 +200,22 @@ export class CoreGradesHelperProvider { */ async getGradesCourseData(grades: any[]): Promise { // Obtain courses from cache to prevent network requests. - const courses = await this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.OnlyCache); + let coursesWereMissing; - const coursesMap = this.utils.arrayToObject(courses, 'id'); - const coursesWereMissing = this.addCourseData(grades, coursesMap); + try { + const courses = await this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.OnlyCache); + + const coursesMap = this.utils.arrayToObject(courses, 'id'); + + coursesWereMissing = this.addCourseData(grades, coursesMap); + } catch (error) { + coursesWereMissing = true; + } // If any course wasn't found, make a network request. if (coursesWereMissing) { const coursesPromise = this.coursesProvider.isGetCoursesByFieldAvailable() - ? this.coursesProvider.getCoursesByField('ids', grades.map((grade) => grade.courseid)) + ? this.coursesProvider.getCoursesByField('ids', grades.map((grade) => grade.courseid).join(',')) : this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.PreferNetwork); const courses = await coursesPromise; From 5ba3c03d91f021fb660201d1b864c2dd550ef523 Mon Sep 17 00:00:00 2001 From: Noel De Martin Date: Wed, 11 Dec 2019 13:33:31 +0100 Subject: [PATCH 3/3] MOBILE-3245 naming: Prefix ReadingStrategy name with CoreSites --- src/core/courses/providers/courses.ts | 4 ++-- src/core/grades/providers/helper.ts | 6 +++--- src/providers/sites.ts | 10 +++++----- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/core/courses/providers/courses.ts b/src/core/courses/providers/courses.ts index f869e467a..b0529d926 100644 --- a/src/core/courses/providers/courses.ts +++ b/src/core/courses/providers/courses.ts @@ -15,7 +15,7 @@ import { Injectable } from '@angular/core'; import { CoreEventsProvider } from '@providers/events'; import { CoreLoggerProvider } from '@providers/logger'; -import { CoreSitesProvider, ReadingStrategy } from '@providers/sites'; +import { CoreSitesProvider, CoreSitesReadingStrategy } from '@providers/sites'; import { CoreSite } from '@classes/site'; /** @@ -774,7 +774,7 @@ export class CoreCoursesProvider { * @param siteId Site to get the courses from. If not defined, use current site. * @return Promise resolved with the courses. */ - getUserCourses(preferCache?: boolean, siteId?: string, strategy?: ReadingStrategy): Promise { + getUserCourses(preferCache?: boolean, siteId?: string, strategy?: CoreSitesReadingStrategy): Promise { return this.sitesProvider.getSite(siteId).then((site) => { const userId = site.getUserId(), diff --git a/src/core/grades/providers/helper.ts b/src/core/grades/providers/helper.ts index ae8cc9071..7bdfc7b67 100644 --- a/src/core/grades/providers/helper.ts +++ b/src/core/grades/providers/helper.ts @@ -15,7 +15,7 @@ import { Injectable } from '@angular/core'; import { NavController } from 'ionic-angular'; import { CoreLoggerProvider } from '@providers/logger'; -import { CoreSitesProvider, ReadingStrategy } from '@providers/sites'; +import { CoreSitesProvider, CoreSitesReadingStrategy } from '@providers/sites'; import { CoreCoursesProvider } from '@core/courses/providers/courses'; import { CoreCourseProvider } from '@core/course/providers/course'; import { CoreGradesProvider } from './grades'; @@ -203,7 +203,7 @@ export class CoreGradesHelperProvider { let coursesWereMissing; try { - const courses = await this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.OnlyCache); + const courses = await this.coursesProvider.getUserCourses(undefined, undefined, CoreSitesReadingStrategy.OnlyCache); const coursesMap = this.utils.arrayToObject(courses, 'id'); @@ -216,7 +216,7 @@ export class CoreGradesHelperProvider { if (coursesWereMissing) { const coursesPromise = this.coursesProvider.isGetCoursesByFieldAvailable() ? this.coursesProvider.getCoursesByField('ids', grades.map((grade) => grade.courseid).join(',')) - : this.coursesProvider.getUserCourses(undefined, undefined, ReadingStrategy.PreferNetwork); + : this.coursesProvider.getUserCourses(undefined, undefined, CoreSitesReadingStrategy.PreferNetwork); const courses = await coursesPromise; diff --git a/src/providers/sites.ts b/src/providers/sites.ts index 0094d7096..3458aaefd 100644 --- a/src/providers/sites.ts +++ b/src/providers/sites.ts @@ -158,7 +158,7 @@ export interface CoreSiteSchema { migrate?(db: SQLiteDB, oldVersion: number, siteId: string): Promise | void; } -export const enum ReadingStrategy { +export const enum CoreSitesReadingStrategy { OnlyCache, PreferCache, PreferNetwork, @@ -1743,18 +1743,18 @@ export class CoreSitesProvider { * @param strategy Reading strategy. * @return PreSets options object. */ - getReadingStrategyPreSets(strategy: ReadingStrategy): CoreSiteWSPreSets { + getReadingStrategyPreSets(strategy: CoreSitesReadingStrategy): CoreSiteWSPreSets { switch (strategy) { - case ReadingStrategy.PreferCache: + case CoreSitesReadingStrategy.PreferCache: return { omitExpires: true, }; - case ReadingStrategy.OnlyCache: + case CoreSitesReadingStrategy.OnlyCache: return { omitExpires: true, forceOffline: true, }; - case ReadingStrategy.PreferNetwork: + case CoreSitesReadingStrategy.PreferNetwork: default: return {}; }