MOBILE-4311 quiz: Avoid sending NaN to webservice

main
Noel De Martin 2023-04-13 13:08:29 +02:00
parent e14f5002d5
commit e48d82ddb2
6 changed files with 60 additions and 11 deletions

View File

@ -13,6 +13,7 @@
// limitations under the License. // limitations under the License.
import { CoreConstants } from '@/core/constants'; import { CoreConstants } from '@/core/constants';
import { safeNumber, SafeNumber } from '@/core/utils/types';
import { Component, OnDestroy, OnInit, Optional } from '@angular/core'; import { Component, OnDestroy, OnInit, Optional } from '@angular/core';
import { CoreCourseModuleMainActivityComponent } from '@features/course/classes/main-activity-component'; import { CoreCourseModuleMainActivityComponent } from '@features/course/classes/main-activity-component';
@ -88,7 +89,7 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp
protected attemptAccessInfo?: AddonModQuizGetAttemptAccessInformationWSResponse; // Last attempt access info. protected attemptAccessInfo?: AddonModQuizGetAttemptAccessInformationWSResponse; // Last attempt access info.
protected moreAttempts = false; // Whether user can create/continue attempts. protected moreAttempts = false; // Whether user can create/continue attempts.
protected options?: AddonModQuizCombinedReviewOptions; // Combined review options. protected options?: AddonModQuizCombinedReviewOptions; // Combined review options.
protected gradebookData?: { grade?: number; feedback?: string }; // The gradebook grade and feedback. protected gradebookData?: { grade?: SafeNumber; feedback?: string }; // The gradebook grade and feedback.
protected overallStats = false; // Equivalent to overallstats in mod_quiz_view_object in Moodle. protected overallStats = false; // Equivalent to overallstats in mod_quiz_view_object in Moodle.
protected finishedObserver?: CoreEventObserver; // It will observe attempt finished events. protected finishedObserver?: CoreEventObserver; // It will observe attempt finished events.
protected hasPlayed = false; // Whether the user has gone to the quiz player (attempted). protected hasPlayed = false; // Whether the user has gone to the quiz player (attempted).
@ -633,8 +634,10 @@ export class AddonModQuizIndexComponent extends CoreCourseModuleMainActivityComp
const data = await AddonModQuiz.getGradeFromGradebook(this.courseId, this.module.id); const data = await AddonModQuiz.getGradeFromGradebook(this.courseId, this.module.id);
if (data) { if (data) {
const grade = data.graderaw ?? (data.grade !== undefined && data.grade !== null ? Number(data.grade) : undefined);
this.gradebookData = { this.gradebookData = {
grade: data.graderaw ?? (data.grade !== undefined && data.grade !== null ? Number(data.grade) : undefined), grade: safeNumber(grade),
feedback: data.feedback, feedback: data.feedback,
}; };
} }

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
import { isSafeNumber } from '@/core/utils/types';
import { Component, OnInit } from '@angular/core'; import { Component, OnInit } from '@angular/core';
import { CoreError } from '@classes/errors/error'; import { CoreError } from '@classes/errors/error';
import { IonRefresher } from '@ionic/angular'; import { IonRefresher } from '@ionic/angular';
@ -108,7 +109,7 @@ export class AddonModQuizAttemptPage implements OnInit {
const grade = Number(this.attempt.rescaledGrade); const grade = Number(this.attempt.rescaledGrade);
if (this.quiz.showFeedbackColumn && AddonModQuiz.isAttemptFinished(this.attempt.state) && if (this.quiz.showFeedbackColumn && AddonModQuiz.isAttemptFinished(this.attempt.state) &&
options.someoptions.overallfeedback && !isNaN(grade)) { options.someoptions.overallfeedback && isSafeNumber(grade)) {
// Feedback should be displayed, get the feedback for the grade. // Feedback should be displayed, get the feedback for the grade.
const response = await AddonModQuiz.getFeedbackForGrade(this.quiz.id, grade, { const response = await AddonModQuiz.getFeedbackForGrade(this.quiz.id, grade, {

View File

@ -13,6 +13,7 @@
// limitations under the License. // limitations under the License.
import { CoreConstants } from '@/core/constants'; import { CoreConstants } from '@/core/constants';
import { isSafeNumber } from '@/core/utils/types';
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { CoreError } from '@classes/errors/error'; import { CoreError } from '@classes/errors/error';
@ -120,11 +121,12 @@ export class AddonModQuizPrefetchHandlerService extends CoreCourseActivityPrefet
} }
const attemptGrade = AddonModQuiz.rescaleGrade(attempt.sumgrades, quiz, false); const attemptGrade = AddonModQuiz.rescaleGrade(attempt.sumgrades, quiz, false);
if (attemptGrade === undefined) { const attemptGradeNumber = attemptGrade !== undefined && Number(attemptGrade);
if (!isSafeNumber(attemptGradeNumber)) {
return; return;
} }
const feedback = await AddonModQuiz.getFeedbackForGrade(quiz.id, Number(attemptGrade), { const feedback = await AddonModQuiz.getFeedbackForGrade(quiz.id, attemptGradeNumber, {
cmId: quiz.coursemodule, cmId: quiz.coursemodule,
readingStrategy: CoreSitesReadingStrategy.ONLY_NETWORK, readingStrategy: CoreSitesReadingStrategy.ONLY_NETWORK,
siteId, siteId,
@ -421,8 +423,9 @@ export class AddonModQuizPrefetchHandlerService extends CoreCourseActivityPrefet
if (AddonModQuiz.isAttemptFinished(attempt.state)) { if (AddonModQuiz.isAttemptFinished(attempt.state)) {
// Attempt is finished, get feedback and review data. // Attempt is finished, get feedback and review data.
const attemptGrade = AddonModQuiz.rescaleGrade(attempt.sumgrades, quiz, false); const attemptGrade = AddonModQuiz.rescaleGrade(attempt.sumgrades, quiz, false);
if (attemptGrade !== undefined) { const attemptGradeNumber = attemptGrade !== undefined && Number(attemptGrade);
promises.push(AddonModQuiz.getFeedbackForGrade(quiz.id, Number(attemptGrade), modOptions)); if (isSafeNumber(attemptGradeNumber)) {
promises.push(AddonModQuiz.getFeedbackForGrade(quiz.id, attemptGradeNumber, modOptions));
} }
// Get the review for each page. // Get the review for each page.

View File

@ -12,6 +12,7 @@
// See the License for the specific language governing permissions and // See the License for the specific language governing permissions and
// limitations under the License. // limitations under the License.
import { SafeNumber } from '@/core/utils/types';
import { Injectable } from '@angular/core'; import { Injectable } from '@angular/core';
import { CoreError } from '@classes/errors/error'; import { CoreError } from '@classes/errors/error';
@ -592,7 +593,7 @@ export class AddonModQuizProvider {
*/ */
async getFeedbackForGrade( async getFeedbackForGrade(
quizId: number, quizId: number,
grade: number, grade: SafeNumber,
options: CoreCourseCommonModWSOptions = {}, options: CoreCourseCommonModWSOptions = {},
): Promise<AddonModQuizGetQuizFeedbackForGradeWSResponse> { ): Promise<AddonModQuizGetQuizFeedbackForGradeWSResponse> {
const site = await CoreSites.getSite(options.siteId); const site = await CoreSites.getSite(options.siteId);
@ -2053,7 +2054,7 @@ export type AddonModQuizAttemptWSData = {
timemodified?: number; // Last modified time. timemodified?: number; // Last modified time.
timemodifiedoffline?: number; // Last modified time via webservices. timemodifiedoffline?: number; // Last modified time via webservices.
timecheckstate?: number; // Next time quiz cron should check attempt for state changes. NULL means never check. timecheckstate?: number; // Next time quiz cron should check attempt for state changes. NULL means never check.
sumgrades?: number | null; // Total marks for this attempt. sumgrades?: SafeNumber | null; // Total marks for this attempt.
}; };
/** /**
@ -2304,7 +2305,7 @@ export type AddonModQuizGetUserBestGradeWSParams = {
*/ */
export type AddonModQuizGetUserBestGradeWSResponse = { export type AddonModQuizGetUserBestGradeWSResponse = {
hasgrade: boolean; // Whether the user has a grade on the given quiz. hasgrade: boolean; // Whether the user has a grade on the given quiz.
grade?: number; // The grade (only if the user has a grade). grade?: SafeNumber; // The grade (only if the user has a grade).
gradetopass?: number; // @since 3.11. The grade to pass the quiz (only if set). gradetopass?: number; // @since 3.11. The grade to pass the quiz (only if set).
warnings?: CoreWSExternalWarning[]; warnings?: CoreWSExternalWarning[];
}; };

View File

@ -21,6 +21,7 @@ import { CoreLogger } from '@singletons/logger';
import { CoreWSExternalWarning } from '@services/ws'; import { CoreWSExternalWarning } from '@services/ws';
import { CoreSiteWSPreSets } from '@classes/site'; import { CoreSiteWSPreSets } from '@classes/site';
import { CoreError } from '@classes/errors/error'; import { CoreError } from '@classes/errors/error';
import { SafeNumber } from '@/core/utils/types';
/** /**
* Service to provide grade functionalities. * Service to provide grade functionalities.
@ -475,7 +476,7 @@ export type CoreGradesGradeItem = {
weightraw?: number; // Weight raw. weightraw?: number; // Weight raw.
weightformatted?: string; // Weight. weightformatted?: string; // Weight.
status?: string; // Status. status?: string; // Status.
graderaw?: number; // Grade raw. graderaw?: SafeNumber; // Grade raw.
gradedatesubmitted?: number; // Grade submit date. gradedatesubmitted?: number; // Grade submit date.
gradedategraded?: number; // Grade graded date. gradedategraded?: number; // Grade graded date.
gradehiddenbydate?: boolean; // Grade hidden by date?. gradehiddenbydate?: boolean; // Grade hidden by date?.

View File

@ -51,3 +51,43 @@ export type Pretty<T> = T extends infer U ? {[K in keyof U]: U[K]} : never;
* @see https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types * @see https://www.typescriptlang.org/docs/handbook/2/conditional-types.html#distributive-conditional-types
*/ */
export type OmitUnion<T, A extends keyof T> = T extends '' ? never : Omit<T, A>; export type OmitUnion<T, A extends keyof T> = T extends '' ? never : Omit<T, A>;
/**
* Helper to create branded types.
*
* A branded type can be used to mark other types as having passed some validations.
*
* @see https://twitter.com/mattpocockuk/status/1625173884885401600
*/
export type Brand<T, TBrand extends string> = T & { [brand]: TBrand };
declare const brand: unique symbol;
/**
* Number type excluding NaN values.
*/
export type SafeNumber = Brand<number, 'SafeNumber'>;
/**
* Check whether a given number is safe to use (does not equal undefined nor NaN).
*
* @param value Number value.
* @returns Whether the number is safe.
*/
export function isSafeNumber(value?: unknown): value is SafeNumber {
return typeof value === 'number' && !isNaN(value);
}
/**
* Make sure that a given number is safe to use, and convert it to undefined otherwise.
*
* @param value Number value.
* @returns Branded number value if safe, undefined otherwise.
*/
export function safeNumber(value?: unknown): SafeNumber | undefined {
if (!isSafeNumber(value)) {
return undefined;
}
return value;
}