Merge pull request #3606 from moodlehq/MOBILE-4311

MOBILE-4311 quiz: Avoid sending NaN to webservice

src/addons/mod/quiz/components/index/index.ts

@@ -13,6 +13,7 @@
 // limitations under the License.
 
 import { CoreConstants } from '@/core/constants';
+import { safeNumber, SafeNumber } from '@/core/utils/types';
 import { Component, OnDestroy, OnInit, Optional } from '@angular/core';
 
 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 moreAttempts = false; // Whether user can create/continue attempts.
     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 finishedObserver?: CoreEventObserver; // It will observe attempt finished events.
     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);
 
             if (data) {
+                const grade = data.graderaw ?? (data.grade !== undefined && data.grade !== null ? Number(data.grade) : undefined);
+
                 this.gradebookData = {
-                    grade: data.graderaw ?? (data.grade !== undefined && data.grade !== null ? Number(data.grade) : undefined),
+                    grade: safeNumber(grade),
                     feedback: data.feedback,
                 };
             }

src/addons/mod/quiz/pages/attempt/attempt.ts

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import { isSafeNumber } from '@/core/utils/types';
 import { Component, OnInit } from '@angular/core';
 import { CoreError } from '@classes/errors/error';
 import { IonRefresher } from '@ionic/angular';
@@ -108,7 +109,7 @@ export class AddonModQuizAttemptPage implements OnInit {
             const grade = Number(this.attempt.rescaledGrade);
 
             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.
                 const response = await AddonModQuiz.getFeedbackForGrade(this.quiz.id, grade, {

src/addons/mod/quiz/services/handlers/prefetch.ts

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

src/addons/mod/quiz/services/quiz.ts

@@ -12,6 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
+import { SafeNumber } from '@/core/utils/types';
 import { Injectable } from '@angular/core';
 
 import { CoreError } from '@classes/errors/error';
@@ -592,7 +593,7 @@ export class AddonModQuizProvider {
      */
     async getFeedbackForGrade(
         quizId: number,
-        grade: number,
+        grade: SafeNumber,
         options: CoreCourseCommonModWSOptions = {},
     ): Promise<AddonModQuizGetQuizFeedbackForGradeWSResponse> {
         const site = await CoreSites.getSite(options.siteId);
@@ -2053,7 +2054,7 @@ export type AddonModQuizAttemptWSData = {
     timemodified?: number; // Last modified time.
     timemodifiedoffline?: number; // Last modified time via webservices.
     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 = {
     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).
     warnings?: CoreWSExternalWarning[];
 };

src/core/features/grades/services/grades.ts

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

src/core/utils/types.ts

@@ -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
  */
 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;
+}