From ad1e564600ad1eaa9489fd779c22913b8f4e205f Mon Sep 17 00:00:00 2001 From: Dani Palou Date: Thu, 5 Apr 2018 16:17:03 +0200 Subject: [PATCH] MOBILE-2348 quiz: Fix problems in prefetch and offline --- src/addon/mod/quiz/components/index/index.ts | 14 +++++++++++--- src/addon/mod/quiz/pages/player/player.ts | 8 ++++++-- src/addon/mod/quiz/pages/review/review.ts | 4 +++- src/addon/mod/quiz/providers/helper.ts | 2 +- src/addon/mod/quiz/providers/prefetch-handler.ts | 7 +++++-- src/addon/mod/quiz/providers/quiz-offline.ts | 6 +++++- src/classes/sqlitedb.ts | 8 ++++++-- src/core/course/classes/main-activity-component.ts | 1 + src/core/course/components/module/module.ts | 13 +++++++------ src/core/question/providers/question.ts | 5 +++-- src/providers/filepool.ts | 6 +++--- 11 files changed, 51 insertions(+), 23 deletions(-) diff --git a/src/addon/mod/quiz/components/index/index.ts b/src/addon/mod/quiz/components/index/index.ts index f21bcc1e0..59561a7c9 100644 --- a/src/addon/mod/quiz/components/index/index.ts +++ b/src/addon/mod/quiz/components/index/index.ts @@ -16,6 +16,7 @@ import { Component, Optional, Injector } from '@angular/core'; import { Content, NavController } from 'ionic-angular'; import { CoreCourseModuleMainActivityComponent } from '@core/course/classes/main-activity-component'; import { CoreQuestionBehaviourDelegate } from '@core/question/providers/behaviour-delegate'; +import { CoreCourseModulePrefetchDelegate } from '@core/course/providers/module-prefetch-delegate'; import { AddonModQuizProvider } from '../../providers/quiz'; import { AddonModQuizHelperProvider } from '../../providers/helper'; import { AddonModQuizOfflineProvider } from '../../providers/quiz-offline'; @@ -70,7 +71,8 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp constructor(injector: Injector, protected quizProvider: AddonModQuizProvider, @Optional() protected content: Content, protected quizHelper: AddonModQuizHelperProvider, protected quizOffline: AddonModQuizOfflineProvider, protected quizSync: AddonModQuizSyncProvider, protected behaviourDelegate: CoreQuestionBehaviourDelegate, - protected prefetchHandler: AddonModQuizPrefetchHandler, protected navCtrl: NavController) { + protected prefetchHandler: AddonModQuizPrefetchHandler, protected navCtrl: NavController, + protected prefetchDelegate: CoreCourseModulePrefetchDelegate) { super(injector); } @@ -87,6 +89,8 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp this.quizProvider.logViewQuiz(this.quizData.id).then(() => { this.courseProvider.checkModuleCompletion(this.courseId, this.module.completionstatus); + }).catch((error) => { + // Ignore errors. }); }); @@ -110,7 +114,10 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp if (this.quizProvider.isQuizOffline(this.quizData)) { // Quiz supports offline, check if it needs to be downloaded. - if (this.currentStatus != CoreConstants.DOWNLOADED) { + // If the site doesn't support check updates, always prefetch it because we cannot tell if there's something new. + const isDownloaded = this.currentStatus == CoreConstants.DOWNLOADED; + + if (!isDownloaded || !this.prefetchDelegate.canCheckUpdates()) { // Prefetch the quiz. this.showStatusSpinner = true; @@ -118,8 +125,9 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp // Success downloading, open quiz. this.openQuiz(); }).catch((error) => { - if (this.hasOffline) { + if (this.hasOffline || (isDownloaded && !this.prefetchDelegate.canCheckUpdates())) { // Error downloading but there is something offline, allow continuing it. + // If the site doesn't support check updates, continue too because we cannot tell if there's something new. this.openQuiz(); } else { this.domUtils.showErrorModalDefault(error, 'core.errordownloading', true); diff --git a/src/addon/mod/quiz/pages/player/player.ts b/src/addon/mod/quiz/pages/player/player.ts index 9224c089f..341d1ec8f 100644 --- a/src/addon/mod/quiz/pages/player/player.ts +++ b/src/addon/mod/quiz/pages/player/player.ts @@ -420,7 +420,9 @@ export class AddonModQuizPlayerPage implements OnInit, OnDestroy { }); // Mark the page as viewed. We'll ignore errors in this call. - this.quizProvider.logViewAttempt(this.attempt.id, page, this.preflightData, this.offline); + this.quizProvider.logViewAttempt(this.attempt.id, page, this.preflightData, this.offline).catch((error) => { + // Ignore errors. + }); // Start looking for changes. this.autoSave.startCheckChangesProcess(this.quiz, this.attempt, this.preflightData, this.offline); @@ -445,7 +447,9 @@ export class AddonModQuizPlayerPage implements OnInit, OnDestroy { this.attempt.dueDateWarning = this.quizProvider.getAttemptDueDateWarning(this.quiz, this.attempt); // Log summary as viewed. - this.quizProvider.logViewAttemptSummary(this.attempt.id, this.preflightData); + this.quizProvider.logViewAttemptSummary(this.attempt.id, this.preflightData).catch((error) => { + // Ignore errors. + }); }); } diff --git a/src/addon/mod/quiz/pages/review/review.ts b/src/addon/mod/quiz/pages/review/review.ts index 2ce6411cb..05d9c9c16 100644 --- a/src/addon/mod/quiz/pages/review/review.ts +++ b/src/addon/mod/quiz/pages/review/review.ts @@ -77,7 +77,9 @@ export class AddonModQuizReviewPage implements OnInit { */ ngOnInit(): void { this.fetchData().then(() => { - this.quizProvider.logViewAttemptReview(this.attemptId); + this.quizProvider.logViewAttemptReview(this.attemptId).catch((error) => { + // Ignore errors. + }); }).finally(() => { this.loaded = true; }); diff --git a/src/addon/mod/quiz/providers/helper.ts b/src/addon/mod/quiz/providers/helper.ts index 4404a6703..3eb2b003a 100644 --- a/src/addon/mod/quiz/providers/helper.ts +++ b/src/addon/mod/quiz/providers/helper.ts @@ -78,7 +78,7 @@ export class AddonModQuizHelperProvider { return Promise.reject(error); } else if (retrying && !isPreflightCheckRequired) { // We're retrying after a failure, but the preflight check wasn't required. - // If this happens it means there's something wrong with some access rule. + // This means there's something wrong with some access rule or user is offline and data isn't cached. // Don't retry again because it would lead to an infinite loop. return Promise.reject(error); } else { diff --git a/src/addon/mod/quiz/providers/prefetch-handler.ts b/src/addon/mod/quiz/providers/prefetch-handler.ts index 7c9739d45..08b69cf66 100644 --- a/src/addon/mod/quiz/providers/prefetch-handler.ts +++ b/src/addon/mod/quiz/providers/prefetch-handler.ts @@ -233,7 +233,7 @@ export class AddonModQuizPrefetchHandler extends CoreCourseModulePrefetchHandler return Promise.all(promises); }).then(() => { // Check if we need to start a new attempt. - const attempt = attempts[attempts.length - 1]; + let attempt = attempts[attempts.length - 1]; if (!attempt || this.quizProvider.isAttemptFinished(attempt.state)) { // Check if the user can attempt the quiz. if (attemptAccessInfo.preventnewattemptreasons.length) { @@ -241,6 +241,7 @@ export class AddonModQuizPrefetchHandler extends CoreCourseModulePrefetchHandler } startAttempt = true; + attempt = undefined; } // Get the preflight data. This function will also start a new attempt if needed. @@ -407,9 +408,11 @@ export class AddonModQuizPrefetchHandler extends CoreCourseModulePrefetchHandler })); promises.push(this.quizProvider.getGradeFromGradebook(quiz.course, quiz.coursemodule, true, siteId) .then((gradebookData) => { - if (typeof gradebookData.grade != 'undefined') { + if (typeof gradebookData.graderaw != 'undefined') { return this.quizProvider.getFeedbackForGrade(quiz.id, gradebookData.graderaw, true, siteId); } + }).catch(() => { + // Ignore errors. })); promises.push(this.quizProvider.getAttemptAccessInformation(quiz.id, 0, false, true, siteId)); // Last attempt. diff --git a/src/addon/mod/quiz/providers/quiz-offline.ts b/src/addon/mod/quiz/providers/quiz-offline.ts index 7d575ac5e..34507bd55 100644 --- a/src/addon/mod/quiz/providers/quiz-offline.ts +++ b/src/addon/mod/quiz/providers/quiz-offline.ts @@ -66,6 +66,10 @@ export class AddonModQuizOfflineProvider { name: 'timecreated', type: 'INTEGER' }, + { + name: 'timemodified', + type: 'INTEGER' + }, { name: 'finished', type: 'INTEGER' @@ -245,7 +249,7 @@ export class AddonModQuizOfflineProvider { }).then((entry) => { // Save attempt in DB. entry.timemodified = now; - entry.finished = !!finish; + entry.finished = finish ? 1 : 0; return db.insertRecord(this.ATTEMPTS_TABLE, entry); }).then(() => { diff --git a/src/classes/sqlitedb.ts b/src/classes/sqlitedb.ts index fdf17f59b..327c1249b 100644 --- a/src/classes/sqlitedb.ts +++ b/src/classes/sqlitedb.ts @@ -343,6 +343,10 @@ export class SQLiteDB { * @param {object} data Data to insert. */ protected formatDataToInsert(data: object): void { + if (!data) { + return; + } + // Remove undefined entries and convert null to "NULL". for (const name in data) { const value = data[name]; @@ -782,6 +786,8 @@ export class SQLiteDB { */ updateRecords(table: string, data: any, conditions?: any): Promise { + this.formatDataToInsert(data); + if (!data || !Object.keys(data).length) { // No fields to update, consider it's done. return Promise.resolve(); @@ -792,8 +798,6 @@ export class SQLiteDB { let sql, params; - this.formatDataToInsert(data); - for (const key in data) { sets.push(`${key} = ?`); } diff --git a/src/core/course/classes/main-activity-component.ts b/src/core/course/classes/main-activity-component.ts index 9f78057c2..ca2d0abd2 100644 --- a/src/core/course/classes/main-activity-component.ts +++ b/src/core/course/classes/main-activity-component.ts @@ -205,6 +205,7 @@ export class CoreCourseModuleMainActivityComponent extends CoreCourseModuleMainR // Also, get the current status. this.modulePrefetchProvider.getModuleStatus(this.module, this.courseId).then((status) => { + this.currentStatus = status; this.showStatus(status); }); } diff --git a/src/core/course/components/module/module.ts b/src/core/course/components/module/module.ts index aaee9cb5c..cc8e7c0e9 100644 --- a/src/core/course/components/module/module.ts +++ b/src/core/course/components/module/module.ts @@ -66,6 +66,7 @@ export class CoreCourseModuleComponent implements OnInit, OnDestroy { protected prefetchHandler: CoreCourseModulePrefetchHandler; protected statusObserver; + protected isDestroyed = false; constructor(@Optional() protected navCtrl: NavController, protected prefetchDelegate: CoreCourseModulePrefetchDelegate, protected domUtils: CoreDomUtilsProvider, protected courseHelper: CoreCourseHelperProvider, @@ -128,14 +129,13 @@ export class CoreCourseModuleComponent implements OnInit, OnDestroy { // Get download size to ask for confirm if it's high. this.prefetchHandler.getDownloadSize(module, this.courseId).then((size) => { - this.courseHelper.prefetchModule(this.prefetchHandler, this.module, size, this.courseId, refresh).catch((error) => { - // Error or cancelled. - this.spinner = false; - }); + return this.courseHelper.prefetchModule(this.prefetchHandler, this.module, size, this.courseId, refresh); }).catch((error) => { - // Error getting download size, hide spinner. + // Error, hide spinner. this.spinner = false; - this.domUtils.showErrorModalDefault(error, 'core.errordownloading', true); + if (!this.isDestroyed && error) { + this.domUtils.showErrorModalDefault(error, 'core.errordownloading', true); + } }); } @@ -158,5 +158,6 @@ export class CoreCourseModuleComponent implements OnInit, OnDestroy { */ ngOnDestroy(): void { this.statusObserver && this.statusObserver.off(); + this.isDestroyed = true; } } diff --git a/src/core/question/providers/question.ts b/src/core/question/providers/question.ts index 9ca9c749a..e7d81d630 100644 --- a/src/core/question/providers/question.ts +++ b/src/core/question/providers/question.ts @@ -405,7 +405,8 @@ export class CoreQuestionProvider { */ getQuestionAnswers(component: string, attemptId: number, slot: number, filter?: boolean, siteId?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - return site.getDb().getRecords(this.QUESTION_ANSWERS_TABLE, {component, attemptId, slot}).then((answers) => { + return site.getDb().getRecords(this.QUESTION_ANSWERS_TABLE, {component, attemptId, questionSlot: slot}) + .then((answers) => { if (filter) { // Get only answers that isn't "extra" data like sequencecheck or certainty. return this.getBasicAnswersFromArray(answers); @@ -525,7 +526,7 @@ export class CoreQuestionProvider { */ removeQuestionAnswers(component: string, attemptId: number, slot: number, siteId?: string): Promise { return this.sitesProvider.getSite(siteId).then((site) => { - return site.getDb().deleteRecords(this.QUESTION_ANSWERS_TABLE, {component, attemptId, slot}); + return site.getDb().deleteRecords(this.QUESTION_ANSWERS_TABLE, {component, attemptId, questionSlot: slot}); }); } diff --git a/src/providers/filepool.ts b/src/providers/filepool.ts index 8663b050f..eb398387c 100644 --- a/src/providers/filepool.ts +++ b/src/providers/filepool.ts @@ -653,10 +653,10 @@ export class CoreFilepoolProvider { if (filePath && entry.path !== filePath) { newData.path = filePath; } - if (entry.isexternalfile !== options.isexternalfile) { + if (entry.isexternalfile !== options.isexternalfile && (entry.isexternalfile || options.isexternalfile)) { newData.isexternalfile = options.isexternalfile; } - if (entry.repositorytype !== options.repositorytype) { + if (entry.repositorytype !== options.repositorytype && (entry.repositorytype || options.repositorytype)) { newData.repositorytype = options.repositorytype; } @@ -2659,7 +2659,7 @@ export class CoreFilepoolProvider { // Going back from downloading to previous status, restore previous download time. newData.downloadTime = entry.previousDownloadTime; } - newData.status = entry.previous || CoreConstants.DOWNLOADED; + newData.status = entry.previous || CoreConstants.NOT_DOWNLOADED; newData.updated = Date.now(); this.logger.debug(`Set previous status '${entry.status}' for package ${component} ${componentId}`);