From 96c8a150b8d8bf96806ace9513c260e07b0e15c9 Mon Sep 17 00:00:00 2001 From: erseco Date: Mon, 28 Sep 2026 18:24:28 +0100 Subject: [PATCH 1/3] Grade only the iDevices the learner touched The package runtime seeds every gradable iDevice on a page with a score of 0. Once an attempt started, the tracker sent that seed for untouched iDevices too, so under "Last attempt" an iDevice the learner never answered dropped to 0. The tracker now remembers which .idevice_node each trusted interaction landed in and posts itemscores for those iDevices only. It keeps watching later pages after the attempt starts. Related to exelearning/exelearning#2481. --- docs/TRACKING.md | 6 +++ js/scorm_tracker.js | 26 ++++++++-- tests/js/scorm_tracker.test.js | 87 ++++++++++++++++++++++++++++++++++ tests/track_test.php | 31 ++++++++++++ version.php | 2 +- 5 files changed, 148 insertions(+), 4 deletions(-) diff --git a/docs/TRACKING.md b/docs/TRACKING.md index 88c7f0a..76ea15d 100644 --- a/docs/TRACKING.md +++ b/docs/TRACKING.md @@ -79,6 +79,12 @@ persists**: `ingest()` returns before any gradebook write same page (`js/scorm_tracker.js`, exelearning issue 2458). Opening or reviewing the activity creates no attempt, consumes no allowed attempt and changes no grade; a submitted 0 is still recorded like any other score. +- **Only the iDevices the learner touched are sent.** Once the attempt has started, + the seed of every other gradable iDevice on the page is still in `cmi.suspend_data`. + The tracker records which `.idevice_node` each interaction landed in and posts + `itemscores` for those iDevices only (exelearning issue 2481). An untouched iDevice + gets no row in that attempt, so under "Last attempt" it keeps its previous grade, + or stays empty if it was never answered. - **Flat table.** `exelearning_attempt` holds one row per `(exelearningid, userid, attempt, itemnumber)`; `itemnumber=0` is the overall, `>0` is an iDevice (`db/install.xml:71-82`). `record_item()` upserts so repeated diff --git a/js/scorm_tracker.js b/js/scorm_tracker.js index e507065..f860eb9 100644 --- a/js/scorm_tracker.js +++ b/js/scorm_tracker.js @@ -438,19 +438,39 @@ // score is being written can start the attempt. var interactedDoc = null; var watchedDocs = []; + // The iDevices (by objectid) the learner interacted with during this visit. + // The runtime seeds every gradable iDevice on the page with 0, and that seed + // stays in suspend_data, so only touched iDevices are sent (exelearning + // issue 2481): an untouched one keeps its previous grade, or stays empty. + var touched = {}; // Record a learner interaction when it happened inside an iDevice. Navigation // and clicks elsewhere in the package do not answer anything. function noteInteraction(target) { - if (target && typeof target.closest === 'function' && target.closest('.idevice_node')) { + var node = target && typeof target.closest === 'function' && target.closest('.idevice_node'); + if (node) { interactedDoc = target.ownerDocument; + if (node.id) { touched[node.id] = true; } } } + // The item scores to send: every captured score when interaction gating is + // off, otherwise only those of the iDevices the learner touched. + function touchedItemScores() { + if (!awaitInteraction) { return itemScores; } + var out = {}; + for (var oid in itemScores) { + if (itemScores.hasOwnProperty(oid) && touched[oid]) { out[oid] = itemScores[oid]; } + } + return out; + } + // Listen for the learner's own input on a package page. The iframe loads a new // document per package page, so this runs whenever the SCO talks to the API. function watchDocument(doc) { - if (started || !doc || typeof doc.addEventListener !== 'function' + // Keep watching after the attempt starts: later pages still need to know + // which of their iDevices the learner touched. + if (!awaitInteraction || !doc || typeof doc.addEventListener !== 'function' || watchedDocs.indexOf(doc) !== -1) { return; } @@ -477,7 +497,7 @@ // keep the values buffered for the first real commit. if (!dirty || !started) { return true; } var snapshot = JSON.stringify(cmi); - var payload = buildPayload(cmid, session, cmi, itemScores, sesskey); + var payload = buildPayload(cmid, session, cmi, touchedItemScores(), sesskey); try { var xhr = xhrFactory(); // Synchronous in LMSFinish (student closes the tab); async otherwise. diff --git a/tests/js/scorm_tracker.test.js b/tests/js/scorm_tracker.test.js index 3279b94..f0b8935 100644 --- a/tests/js/scorm_tracker.test.js +++ b/tests/js/scorm_tracker.test.js @@ -771,3 +771,90 @@ describe('parseSuspend (versioned exe12 payload, core PR #2209)', () => { expect(parseSuspend('1. "Quiz"; score: 60%; weighted: 30%.')[1].title).toBe('Quiz'); }); }); + +describe('createScormApi: only the iDevices the learner touched are graded', () => { + // Two gradable iDevices on one page, as in the demo activity (True/False + Guess). + // On load the runtime seeds both with 0 in a single suspend_data write. + const SEED = '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 0%; Peso: 50%'; + let scheduled; + function config(xhr) { + scheduled = null; + return { + cmid: 42, + trackurl: 'https://example.test/track.php', + session: 'tok', + bindUnload: false, + getScoringDocument: () => document, + xhrFactory: () => xhr, + setTimeout: (fn) => { scheduled = fn; return 1; }, + clearTimeout: () => { scheduled = null; }, + }; + } + function seedOnLoad(api) { + api.LMSInitialize(''); + api.LMSSetValue('cmi.suspend_data', SEED); + api.LMSSetValue('cmi.core.score.raw', '0'); + } + beforeEach(() => { + document.body.innerHTML = '
' + + '
'; + }); + + it('does not send the seeded 0 of an iDevice the learner never touched (issue 2481)', () => { + const xhr = makeXhr(200); + const tracker = createScormApi(config(xhr)); + seedOnLoad(tracker.api); + // The learner plays only the Guess iDevice and gets it right. + tracker.noteInteraction(document.getElementById('guess')); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + tracker.api.LMSSetValue('cmi.core.score.raw', '50'); + scheduled(); + expect(xhr.calls).toHaveLength(1); + expect(JSON.parse(xhr.lastPayload).itemscores).toEqual({ + 'ide-guess': { scorepct: 100, weighted: 50, title: 'Adivina' }, + }); + }); + + it('sends a legitimate 0 for the second iDevice once the learner answers it too', () => { + const xhr = makeXhr(200); + const tracker = createScormApi(config(xhr)); + seedOnLoad(tracker.api); + tracker.noteInteraction(document.getElementById('guess')); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + scheduled(); + // Then answers the True/False wrongly: its value stays 0, identical to the seed. + tracker.noteInteraction(document.getElementById('tf')); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + scheduled(); + expect(JSON.parse(xhr.lastPayload).itemscores).toEqual({ + 'ide-tf': { scorepct: 0, weighted: 50, title: 'Verdadero o falso' }, + 'ide-guess': { scorepct: 100, weighted: 50, title: 'Adivina' }, + }); + }); + + it('keeps tracking touched iDevices on later pages after the attempt has started', () => { + const xhr = makeXhr(200); + let current = document; + const tracker = createScormApi({ ...config(xhr), getScoringDocument: () => current }); + seedOnLoad(tracker.api); + tracker.noteInteraction(document.getElementById('guess')); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + scheduled(); + // Page 2 carries one gradable iDevice; the learner answers it. + current = document.implementation.createHTMLDocument('page 2'); + current.body.innerHTML = '
'; + const listeners = {}; + current.addEventListener = (type, fn) => { listeners[type] = fn; }; + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Quiz"; Puntuación: 0%; Peso: 100%'); + // Real input on the new page must still be watched after the attempt started. + listeners.pointerdown({ isTrusted: true, target: current.getElementById('p2') }); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Quiz"; Puntuación: 80%; Peso: 100%'); + scheduled(); + expect(Object.keys(JSON.parse(xhr.lastPayload).itemscores).sort()).toEqual(['ide-guess', 'ide-p2']); + }); +}); diff --git a/tests/track_test.php b/tests/track_test.php index e2c1ab4..b0a4ab2 100644 --- a/tests/track_test.php +++ b/tests/track_test.php @@ -614,6 +614,37 @@ public function test_ingest_records_a_submitted_zero_under_last_attempt(): void $this->assertEqualsWithDelta(0.0, $this->published_grade($instance, $student->id, 1), 0.0001); } + /** + * An iDevice missing from itemscores keeps its grade under "Last attempt". + * + * Contract relied on by the tracker (exelearning issue 2481): it only sends the + * iDevices the learner touched during a visit, so an attempt that scores one + * iDevice must leave every other iDevice's last grade alone. + */ + public function test_ingest_leaves_items_missing_from_itemscores_untouched(): void { + [$instance, $student] = $this->create_activity_with_student([ + 'grademodel' => EXELEARNING_GRADEMODEL_PERITEM, + 'grademethod' => \mod_exelearning\local\attempts::GRADE_LAST, + ]); + [$course, $cm] = $this->course_and_cm($instance); + $tf = $this->objectid_for($instance, 1); + $guess = $this->objectid_for($instance, 2); + $submit = function (string $session, array $itemscores) use ($instance, $course, $cm, $student): void { + $result = track::ingest($instance, $course, $cm, $student->id, [ + 'session' => $session, + 'cmi' => ['cmi.core.score.raw' => '50', 'cmi.core.score.max' => '100'], + 'itemscores' => $itemscores, + ], false); + $this->assertTrue($result['ok']); + }; + + $submit('visitTf', [$tf => ['scorepct' => 100.0, 'weighted' => 50.0, 'title' => 'TF']]); + $submit('visitGuess', [$guess => ['scorepct' => 100.0, 'weighted' => 50.0, 'title' => 'Guess']]); + + $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 1), 0.0001); + $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 2), 0.0001); + } + /** * With the master grading switch off (DEC-13-07), ingest() records NOTHING * (DEC-126-01). diff --git a/version.php b/version.php index 2794860..26c6202 100644 --- a/version.php +++ b/version.php @@ -34,7 +34,7 @@ // in $plugin->release ('dev'); a release-preparation PR commits the final // version + semver release BEFORE the tag is created (see DEVELOPMENT.md, // "Versioning and releases"). -$plugin->version = 2026092611; +$plugin->version = 2026092612; $plugin->release = 'dev'; $plugin->requires = 2024100700; // Moodle 4.5 LTS+. $plugin->supported = [405, 502]; // Moodle 4.5 LTS through Moodle 5.2. From 948c3a34c7d8732c855255b1366fc4e2fdc078d8 Mon Sep 17 00:00:00 2001 From: erseco Date: Mon, 28 Sep 2026 18:52:50 +0100 Subject: [PATCH 2/3] Keep the raw score consistent in the untouched-iDevice test ingest() reports a debugging() divergence when cmi.core.score.raw differs from the overall recomputed from itemscores; CI treats it as an error. --- tests/track_test.php | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/tests/track_test.php b/tests/track_test.php index b0a4ab2..ff9729b 100644 --- a/tests/track_test.php +++ b/tests/track_test.php @@ -632,7 +632,9 @@ public function test_ingest_leaves_items_missing_from_itemscores_untouched(): vo $submit = function (string $session, array $itemscores) use ($instance, $course, $cm, $student): void { $result = track::ingest($instance, $course, $cm, $student->id, [ 'session' => $session, - 'cmi' => ['cmi.core.score.raw' => '50', 'cmi.core.score.max' => '100'], + // Raw matches the overall recomputed from the one scored iDevice, so + // ingest() does not report a divergence (DEC-6-01). + 'cmi' => ['cmi.core.score.raw' => '100', 'cmi.core.score.max' => '100'], 'itemscores' => $itemscores, ], false); $this->assertTrue($result['ok']); From d82f05fdf72a6a09279255e85c2c048deed6bb5a Mon Sep 17 00:00:00 2001 From: erseco Date: Mon, 5 Oct 2026 17:57:07 +0100 Subject: [PATCH 3/3] Grade only answered iDevices, keep the overall on the full map Sending only the touched iDevices broke three things. Under the overall model ingest() recomputed the overall from that partial map, so answering one of two equally weighted iDevices scored 100 instead of 50, and the reported grade was inflated the same way. An empty filtered map made the server parse cmi.suspend_data itself and record every seeded 0 again. And any click inside an iDevice (its text, a hint) marked it touched, so its seed 0 replaced a real grade under "Last attempt". The tracker now sends the full itemscores map again plus `answered`, the objectids answered during this visit. A score write after a trusted interaction is attributed to the iDevice of the learner's last interaction only, and an iDevice whose score moved away from its load seed counts too. ingest() keeps recomputing the overall from the full map, which is the attempt score the package computes, and writes per-iDevice rows only for `answered`, whether the scores come from the client map, an exe12 suspend_data or the legacy page-local fallback (matched through the grade item each slot routes to). `answered` is validated like itemscores. Without the key, as from the mobile web service or an older cached tracker, ingestion behaves as before. --- classes/local/track.php | 73 ++++++++++- docs/TRACKING.md | 26 +++- docs/scorm-shim-current-flow.md | 6 +- js/scorm_tracker.js | 51 +++++--- tests/js/scorm_tracker.test.js | 90 +++++++++---- tests/track_test.php | 224 +++++++++++++++++++++++++++++--- 6 files changed, 398 insertions(+), 72 deletions(-) diff --git a/classes/local/track.php b/classes/local/track.php index bf006c8..0698789 100644 --- a/classes/local/track.php +++ b/classes/local/track.php @@ -50,6 +50,9 @@ class track { /** Bit 1 of a versioned record's flag field: the activity counts towards the score. */ private const EXE12_FLAG_EVALUABLE = 1; + /** Most entries a client-supplied itemscores map or answered list may carry. */ + private const MAX_CLIENT_ITEMS = 1000; + /** * Ingests a SCORM tracking payload: records the attempt, routes per-iDevice * scores and updates the gradebook + completion. Shared by the web `track.php` @@ -68,7 +71,8 @@ class track { * @param \stdClass $course The course record (for completion). * @param \stdClass $cm The course_module record (for completion). * @param int $userid The grading user. - * @param array $payload Decoded payload: {cmi:{...}, session?:string, itemscores?:array}. + * @param array $payload Decoded payload: {cmi:{...}, session?:string, itemscores?:array, + * answered?:array}. * @param bool $ispreview When true, acknowledge the score without grading (DEC-0-06). * @return array Result map: always has 'ok'. May add noop|mode|error|attempt|rawscore|status|peritem. */ @@ -127,7 +131,7 @@ public static function ingest( // to the page-local index from cmi.suspend_data only when none is supplied. $itemscores = (isset($payload['itemscores']) && is_array($payload['itemscores'])) ? $payload['itemscores'] : []; - if (count($itemscores) > 1000) { + if (count($itemscores) > self::MAX_CLIENT_ITEMS) { // A well-formed package emits one entry per gradable iDevice; a map far // larger than any real package is malformed/abusive — drop it. debugging( @@ -164,6 +168,9 @@ public static function ingest( // legacy fallback must never see them. $peritem = []; } + // The iDevices the learner answered during this visit, or null when the client + // does not say (exelearning issue 2481, see answered_objectids()). + $answered = self::answered_objectids($exe, $payload); $grademethod = (int) ($exe->grademethod ?? attempts::GRADE_HIGHEST); $grademodel = (int) ($exe->grademodel ?? EXELEARNING_GRADEMODEL_PERITEM); @@ -229,16 +236,23 @@ public static function ingest( } } - // 1) Attempts + aggregated grade per iDevice (itemnumber > 0). + // 1) Attempts + aggregated grade per iDevice (itemnumber > 0). When the + // client names the answered iDevices, only those get a row, whatever the + // source of the scores: the others carry the runtime's load seed of 0, + // which is no answer (exelearning issue 2481). $persaved = []; if ($itemscores !== []) { - $persaved = self::apply_item_scores($exe, $userid, $attempt, $itemscores, $sessiontoken); + $answeredscores = ($answered === null) ? $itemscores : array_intersect_key($itemscores, $answered); + $persaved = self::apply_item_scores($exe, $userid, $attempt, $answeredscores, $sessiontoken); } else if ($peritem) { - $persaved = self::apply_legacy_peritem($exe, $userid, $attempt, $peritem, $sessiontoken); + $persaved = self::apply_legacy_peritem($exe, $userid, $attempt, $peritem, $sessiontoken, $answered); } // 2) Overall (itemnumber=0): recompute from the per-iDevice scores when an // objectid map was supplied (DEC-6-01), never trusting the client overall. + // It uses the FULL map, answered or not: that is the attempt score the + // package itself computes, where an unanswered iDevice counts 0 for this + // attempt (exelearning issue 2481). if ($itemscores !== []) { $overallpct = self::recompute_overall_pct($itemscores); if ($overallpct !== null) { @@ -666,6 +680,44 @@ private static function filter_registered_scores(\stdClass $exe, array $itemscor ); } + /** + * Reads the objectids the learner answered during this visit from the payload. + * + * The tracker sends the full itemscores map, so the overall stays the score the + * package computes, and lists in `answered` the iDevices that received an answer + * during the visit: every other one still carries the runtime's load seed of 0 + * (exelearning issue 2481). The list is client input, validated like itemscores: + * a non-array or oversized list names nothing, and only strings naming a + * registered, non-deleted gradable iDevice are kept. + * + * @param \stdClass $exe The exelearning instance record. + * @param array $payload Decoded payload. + * @return array|null Set of objectid => true, or null when the payload carries no + * `answered` key (the mobile web service, an older cached tracker): every + * scored iDevice is then recorded, as before. + */ + private static function answered_objectids(\stdClass $exe, array $payload): ?array { + if (!array_key_exists('answered', $payload)) { + return null; + } + $answered = $payload['answered']; + if (!is_array($answered) || count($answered) > self::MAX_CLIENT_ITEMS) { + debugging( + 'mod_exelearning: malformed answered list ignored; no per-iDevice result is recorded.', + DEBUG_DEVELOPER + ); + return []; + } + $registered = array_flip(array_map('strval', self::registered_objectids($exe))); + $set = []; + foreach ($answered as $objectid) { + if (is_string($objectid) && isset($registered[$objectid])) { + $set[$objectid] = true; + } + } + return $set; + } + /** * Casts a parsed numeric string to float, accepting a comma decimal separator. * @@ -802,6 +854,11 @@ public static function apply_item_scores( * @param int $attempt Attempt number from attempts::resolve_attempt_number(). * @param array $peritem Map N => ['scorepct' => float, ...] from parse_suspend_data(). * @param string $sessiontoken Page-load session token. + * @param array|null $answered Set objectid => true of the iDevices answered in this + * visit, or null to record every entry. A slot is + * matched through the objectid of the grade item it + * is routed to (N as itemnumber), the same mapping this + * fallback already relies on (exelearning issue 2481). * @return array Map of itemnumber => final published grade. */ public static function apply_legacy_peritem( @@ -809,7 +866,8 @@ public static function apply_legacy_peritem( int $userid, int $attempt, array $peritem, - string $sessiontoken + string $sessiontoken, + ?array $answered = null ): array { global $DB; @@ -836,6 +894,9 @@ public static function apply_legacy_peritem( if (!isset($rows[$itemnumber]) || !is_array($info)) { continue; } + if ($answered !== null && !isset($answered[(string) $rows[$itemnumber]->objectid])) { + continue; + } $scorepct = max(0.0, min(100.0, (float) ($info['scorepct'] ?? 0))); $persaved[$itemnumber] = self::apply_one( $exe, diff --git a/docs/TRACKING.md b/docs/TRACKING.md index 51c85e1..8171623 100644 --- a/docs/TRACKING.md +++ b/docs/TRACKING.md @@ -26,7 +26,7 @@ funnels every channel into **one** server-side scoring method, `track::ingest()` window.API shim view.php:380-537 (inline JS in the parent window) │ buffers CMI pairs; on cmi.suspend_data, resolves each scored iDevice │ to its stable objectid by reading the iframe DOM (DEC-5-01) - │ POST { id:, sesskey, session, cmi, itemscores } + │ POST { id:, sesskey, session, cmi, itemscores, answered } ▼ track.php (sesskey + capability; web/AJAX entry) │ required_param id (track.php:40) · decode body, then @@ -83,12 +83,24 @@ persists**: `ingest()` returns before any gradebook write never reaches the page); switching tabs with an iDevice field focused does not. Opening or reviewing the activity creates no attempt, consumes no allowed attempt and changes no grade; a submitted 0 is still recorded like any other score. -- **Only the iDevices the learner touched are sent.** Once the attempt has started, - the seed of every other gradable iDevice on the page is still in `cmi.suspend_data`. - The tracker records which `.idevice_node` each interaction landed in and posts - `itemscores` for those iDevices only (exelearning issue 2481). An untouched iDevice - gets no row in that attempt, so under "Last attempt" it keeps its previous grade, - or stays empty if it was never answered. +- **Only answered iDevices get a per-iDevice row.** Once the attempt has started, + the seed of every gradable iDevice the learner did not answer is still 0 in + `cmi.suspend_data` (exelearning issue 2481). The tracker posts the full + `itemscores` map plus `answered`, the objectids answered during this visit. A score + write (`cmi.suspend_data`, `cmi.core.score.raw`, `cmi.core.lesson_status`) made + after a trusted interaction on that page is attributed to the iDevice of the + learner's last such interaction only, so clicking a question's text or opening a + hint and then answering another iDevice names just the one answered; a genuine 0 + is still attributed. An iDevice whose captured score moved away from its seed + counts as answered too. `ingest()` recomputes the overall from the full map (the + attempt score the package computes: an unanswered iDevice counts 0 for this + attempt) and writes rows with `itemnumber > 0` only for `answered`, whether the + scores come from the map, an `exe12/` `cmi.suspend_data` or the legacy page-local + fallback (a slot is matched through the objectid of the grade item it routes to). + An unanswered iDevice keeps its previous grade, or stays empty. `answered` is + filtered like `itemscores` (strings naming registered objectids, `> 1000` entries + or a non-array name nothing). Without the key (the `save_track` web service, an + older cached tracker) every scored iDevice is recorded as before. - **Flat table.** `exelearning_attempt` holds one row per `(exelearningid, userid, attempt, itemnumber)`; `itemnumber=0` is the overall, `>0` is an iDevice (`db/install.xml:71-82`). `record_item()` upserts so repeated diff --git a/docs/scorm-shim-current-flow.md b/docs/scorm-shim-current-flow.md index 57d70e0..5fb3e0f 100644 --- a/docs/scorm-shim-current-flow.md +++ b/docs/scorm-shim-current-flow.md @@ -37,8 +37,10 @@ The iframe keeps the permissions documented in [TRACKING](TRACKING.md). 6. The service acknowledges preview and ungraded activity requests without writes (DEC-0-06, DEC-126-01). Scored work is serialized per activity/user, constrained by the attempt limit, clamped and filtered to registered objectids. -7. Per-item attempts are recorded; the overall is recomputed from reported item - scores and their weights. PERITEM publishes only itemnumber 1..N; OVERALL +7. Per-item attempts are recorded for the iDevices listed in `answered` (every + reported one when the key is absent); the overall is recomputed from all reported + item scores and their weights, where a load seed counts 0 (see + [TRACKING](TRACKING.md)). PERITEM publishes only itemnumber 1..N; OVERALL publishes only itemnumber 0. Completion and lifecycle events follow the shared ingestion result. diff --git a/js/scorm_tracker.js b/js/scorm_tracker.js index d7eeaac..529e60c 100644 --- a/js/scorm_tracker.js +++ b/js/scorm_tracker.js @@ -377,14 +377,17 @@ * @param {Object} cmi Buffered CMI key/value pairs. * @param {Object} itemscores objectid -> {scorepct, weighted, title}. * @param {string} sesskey Moodle session key, validated server-side. + * @param {string[]} [answered] Objectids the learner answered during this visit; + * when undefined the key is omitted and every scored iDevice is recorded. * @returns {string} JSON payload. */ - function buildPayload(cmid, session, cmi, itemscores, sesskey) { + function buildPayload(cmid, session, cmi, itemscores, sesskey, answered) { return JSON.stringify({ id: cmid, session: session, cmi: cmi, itemscores: itemscores, + answered: answered, sesskey: sesskey, }); } @@ -442,11 +445,15 @@ // Package pages already listened to. Weak, so a visited page's document can be // garbage-collected once the iframe moves on. var watchedDocs = new WeakSet(); - // The iDevices (by objectid) the learner interacted with during this visit. - // The runtime seeds every gradable iDevice on the page with 0, and that seed - // stays in suspend_data, so only touched iDevices are sent (exelearning - // issue 2481): an untouched one keeps its previous grade, or stays empty. - var touched = {}; + // The objectid of the iDevice the learner last interacted with on interactedDoc. + var lastIdevice = null; + // The seeds of the iDevices the learner never answered stay in suspend_data + // with 0, so the payload names the iDevices answered during this visit + // (exelearning issue 2481) and only those get a per-iDevice result. answered + // holds the iDevices a score write was attributed to; seeds holds each + // iDevice's score when first captured, so a changed score counts too. + var answered = {}; + var seeds = {}; // Record a learner interaction when it happened inside an iDevice. Navigation // and clicks elsewhere in the package do not answer anything. @@ -454,17 +461,19 @@ var node = target && typeof target.closest === 'function' && target.closest('.idevice_node'); if (node) { interactedDoc = target.ownerDocument; - if (node.id) { touched[node.id] = true; } + lastIdevice = node.id || null; } } - // The item scores to send: every captured score when interaction gating is - // off, otherwise only those of the iDevices the learner touched. - function touchedItemScores() { - if (!awaitInteraction) { return itemScores; } - var out = {}; + // The objectids answered during this visit: those a score write was attributed + // to, plus any whose captured score moved away from its seed. + function answeredIds() { + var out = Object.keys(answered); for (var oid in itemScores) { - if (itemScores.hasOwnProperty(oid) && touched[oid]) { out[oid] = itemScores[oid]; } + if (itemScores.hasOwnProperty(oid) && !answered[oid] + && itemScores[oid].scorepct !== seeds[oid]) { + out.push(oid); + } } return out; } @@ -473,7 +482,7 @@ // document per package page, so this runs whenever the SCO talks to the API. function watchDocument(doc) { // Keep watching after the attempt starts: later pages still need to know - // which of their iDevices the learner touched. + // which of their iDevices the learner answers. if (!awaitInteraction || !doc || typeof doc.addEventListener !== 'function' || watchedDocs.has(doc)) { return; @@ -511,7 +520,10 @@ // keep the values buffered for the first real commit. if (!dirty || !started) { return true; } var snapshot = JSON.stringify(cmi); - var payload = buildPayload(cmid, session, cmi, touchedItemScores(), sesskey); + // Without interaction gating there is no attribution to report: omitting + // answered keeps the server recording every scored iDevice. + var payload = buildPayload(cmid, session, cmi, itemScores, sesskey, + awaitInteraction ? answeredIds() : undefined); try { var xhr = xhrFactory(); // Synchronous in LMSFinish (student closes the tab); async otherwise. @@ -553,7 +565,10 @@ var domMap = resolveObjectMap(getScoringDocument()); var result = captureItemScores(newParsed, prevSuspend, domMap); for (var oid in result.delta) { - if (result.delta.hasOwnProperty(oid)) { itemScores[oid] = result.delta[oid]; } + if (result.delta.hasOwnProperty(oid)) { + if (!seeds.hasOwnProperty(oid)) { seeds[oid] = result.delta[oid].scorepct; } + itemScores[oid] = result.delta[oid]; + } } prevSuspend = result.prev; } @@ -569,6 +584,10 @@ cmi[k] = String(v); dirty = true; if (interactedDoc && interactedDoc === doc && SCORE_KEYS.indexOf(k) !== -1) { started = true; + // The write answers the iDevice the learner last interacted with, + // not every iDevice touched before it: clicking a question's text + // or opening a hint answers nothing. + if (lastIdevice) { answered[lastIdevice] = true; } } // Resolve per-iDevice scores to stable objectids while the scoring // page is still loaded in the iframe (DEC-5-01). diff --git a/tests/js/scorm_tracker.test.js b/tests/js/scorm_tracker.test.js index d2cafcb..ab8934f 100644 --- a/tests/js/scorm_tracker.test.js +++ b/tests/js/scorm_tracker.test.js @@ -848,11 +848,16 @@ describe('parseSuspend (versioned exe12 payload, core PR #2209)', () => { }); }); -describe('createScormApi: only the iDevices the learner touched are graded', () => { +describe('createScormApi: the payload names the iDevices the learner answered', () => { // Two gradable iDevices on one page, as in the demo activity (True/False + Guess). - // On load the runtime seeds both with 0 in a single suspend_data write. + // On load the runtime seeds both with 0 in a single suspend_data write. Their seeds + // stay in every later write, so the tracker sends the full map (the attempt score + // the package computes) and lists in `answered` the iDevices answered in this visit + // (exelearning issue 2481): only those get a per-iDevice result. const SEED = '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + '2. "Adivina"; Puntuación: 0%; Peso: 50%'; + const GUESS_RIGHT = '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' + + '2. "Adivina"; Puntuación: 100%; Peso: 50%'; let scheduled; function config(xhr) { scheduled = null; @@ -872,65 +877,104 @@ describe('createScormApi: only the iDevices the learner touched are graded', () api.LMSSetValue('cmi.suspend_data', SEED); api.LMSSetValue('cmi.core.score.raw', '0'); } + function lastBody(xhr) { + return JSON.parse(xhr.lastPayload); + } beforeEach(() => { - document.body.innerHTML = '
' + document.body.innerHTML = '
' + + '

The sky is green.

' + '
'; }); - it('does not send the seeded 0 of an iDevice the learner never touched (issue 2481)', () => { + it('sends the full map and lists only the answered iDevice (issue 2481)', () => { const xhr = makeXhr(200); const tracker = createScormApi(config(xhr)); seedOnLoad(tracker.api); // The learner plays only the Guess iDevice and gets it right. tracker.noteInteraction(document.getElementById('guess')); - tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' - + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + tracker.api.LMSSetValue('cmi.suspend_data', GUESS_RIGHT); tracker.api.LMSSetValue('cmi.core.score.raw', '50'); scheduled(); expect(xhr.calls).toHaveLength(1); - expect(JSON.parse(xhr.lastPayload).itemscores).toEqual({ + expect(lastBody(xhr).itemscores).toEqual({ + 'ide-tf': { scorepct: 0, weighted: 50, title: 'Verdadero o falso' }, 'ide-guess': { scorepct: 100, weighted: 50, title: 'Adivina' }, }); + expect(lastBody(xhr).answered).toEqual(['ide-guess']); + }); + + it('does not count a click on another iDevice\'s text as an answer', () => { + const xhr = makeXhr(200); + const tracker = createScormApi(config(xhr)); + seedOnLoad(tracker.api); + // The learner reads the True/False statement, then answers only Guess, wrongly: + // the Guess score stays at its seed, so only the attribution can name it. + tracker.noteInteraction(document.getElementById('tf-text')); + tracker.noteInteraction(document.getElementById('guess')); + tracker.api.LMSSetValue('cmi.suspend_data', SEED); + tracker.api.LMSSetValue('cmi.core.score.raw', '0'); + scheduled(); + expect(lastBody(xhr).answered).toEqual(['ide-guess']); }); - it('sends a legitimate 0 for the second iDevice once the learner answers it too', () => { + it('lists a genuine 0 answer, identical to its seed', () => { const xhr = makeXhr(200); const tracker = createScormApi(config(xhr)); seedOnLoad(tracker.api); tracker.noteInteraction(document.getElementById('guess')); - tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' - + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + tracker.api.LMSSetValue('cmi.suspend_data', GUESS_RIGHT); scheduled(); // Then answers the True/False wrongly: its value stays 0, identical to the seed. tracker.noteInteraction(document.getElementById('tf')); - tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' - + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + tracker.api.LMSSetValue('cmi.suspend_data', GUESS_RIGHT); scheduled(); - expect(JSON.parse(xhr.lastPayload).itemscores).toEqual({ - 'ide-tf': { scorepct: 0, weighted: 50, title: 'Verdadero o falso' }, - 'ide-guess': { scorepct: 100, weighted: 50, title: 'Adivina' }, - }); + expect(lastBody(xhr).answered.sort()).toEqual(['ide-guess', 'ide-tf']); }); - it('keeps tracking touched iDevices on later pages after the attempt has started', () => { + it('lists an iDevice whose score moved away from its seed', () => { + const xhr = makeXhr(200); + const tracker = createScormApi(config(xhr)); + seedOnLoad(tracker.api); + // The write that scores Guess lands after the learner's last interaction moved + // to True/False (an asynchronous check): attribution names True/False, and the + // changed score still names Guess. + tracker.noteInteraction(document.getElementById('tf')); + tracker.api.LMSSetValue('cmi.suspend_data', GUESS_RIGHT); + scheduled(); + expect(lastBody(xhr).answered.sort()).toEqual(['ide-guess', 'ide-tf']); + }); + + it('omits answered when interaction gating is off', () => { + const xhr = makeXhr(200); + const tracker = createScormApi({ ...config(xhr), awaitInteraction: false }); + seedOnLoad(tracker.api); + scheduled(); + expect(lastBody(xhr)).not.toHaveProperty('answered'); + expect(Object.keys(lastBody(xhr).itemscores).sort()).toEqual(['ide-guess', 'ide-tf']); + }); + + it('keeps attributing answers on later pages after the attempt has started', () => { const xhr = makeXhr(200); let current = document; const tracker = createScormApi({ ...config(xhr), getScoringDocument: () => current }); seedOnLoad(tracker.api); tracker.noteInteraction(document.getElementById('guess')); - tracker.api.LMSSetValue('cmi.suspend_data', '1. "Verdadero o falso"; Puntuación: 0%; Peso: 50%.\t' - + '2. "Adivina"; Puntuación: 100%; Peso: 50%'); + tracker.api.LMSSetValue('cmi.suspend_data', GUESS_RIGHT); scheduled(); - // Page 2 carries one gradable iDevice; the learner answers it. + // Page 2 carries one gradable iDevice; its seed is not attributed to Guess. current = document.implementation.createHTMLDocument('page 2'); current.body.innerHTML = '
'; const listeners = {}; current.addEventListener = (type, fn) => { listeners[type] = fn; }; tracker.api.LMSSetValue('cmi.suspend_data', '1. "Quiz"; Puntuación: 0%; Peso: 100%'); - // Real input on the new page must still be watched after the attempt started. + scheduled(); + expect(lastBody(xhr).answered).toEqual(['ide-guess']); + // Real input on the new page must still be watched after the attempt started; + // a wrong answer leaves the score at its seed. listeners.pointerdown({ isTrusted: true, target: current.getElementById('p2') }); - tracker.api.LMSSetValue('cmi.suspend_data', '1. "Quiz"; Puntuación: 80%; Peso: 100%'); + tracker.api.LMSSetValue('cmi.suspend_data', '1. "Quiz"; Puntuación: 0%; Peso: 100%'); scheduled(); - expect(Object.keys(JSON.parse(xhr.lastPayload).itemscores).sort()).toEqual(['ide-guess', 'ide-p2']); + expect(lastBody(xhr).answered.sort()).toEqual(['ide-guess', 'ide-p2']); + expect(Object.keys(lastBody(xhr).itemscores).sort()).toEqual(['ide-guess', 'ide-p2', 'ide-tf']); }); }); diff --git a/tests/track_test.php b/tests/track_test.php index ff9729b..ae039b8 100644 --- a/tests/track_test.php +++ b/tests/track_test.php @@ -615,36 +615,224 @@ public function test_ingest_records_a_submitted_zero_under_last_attempt(): void } /** - * An iDevice missing from itemscores keeps its grade under "Last attempt". + * Returns the rows recorded for one attempt as itemnumber => rawscore. * - * Contract relied on by the tracker (exelearning issue 2481): it only sends the - * iDevices the learner touched during a visit, so an attempt that scores one - * iDevice must leave every other iDevice's last grade alone. + * @param \stdClass $instance + * @param int $userid + * @param int $attempt + * @return array + */ + protected function attempt_rows(\stdClass $instance, int $userid, int $attempt): array { + global $DB; + return array_map('floatval', $DB->get_records_menu('exelearning_attempt', [ + 'exelearningid' => $instance->id, + 'userid' => $userid, + 'attempt' => $attempt, + ], 'itemnumber ASC', 'itemnumber, rawscore')); + } + + /** + * Submits one visit with the full map of two equally weighted iDevices. + * + * @param \stdClass $instance + * @param \stdClass $student + * @param string $session Page-load session token. + * @param float $tfpct Score of the iDevice at itemnumber 1. + * @param float $guesspct Score of the iDevice at itemnumber 2. + * @param array|null $answered Objectids to send as `answered`; null omits the key. + * @return array The ingest() result. + */ + protected function submit_two_idevice_visit( + \stdClass $instance, + \stdClass $student, + string $session, + float $tfpct, + float $guesspct, + ?array $answered + ): array { + [$course, $cm] = $this->course_and_cm($instance); + $payload = [ + 'session' => $session, + // Raw matches the overall the package computes from the full map, so + // ingest() reports no divergence (DEC-6-01). + 'cmi' => [ + 'cmi.core.score.raw' => (string) (($tfpct + $guesspct) / 2), + 'cmi.core.score.max' => '100', + ], + 'itemscores' => [ + $this->objectid_for($instance, 1) => ['scorepct' => $tfpct, 'weighted' => 50.0, 'title' => 'TF'], + $this->objectid_for($instance, 2) => ['scorepct' => $guesspct, 'weighted' => 50.0, 'title' => 'Guess'], + ], + ]; + if ($answered !== null) { + $payload['answered'] = $answered; + } + $result = track::ingest($instance, $course, $cm, $student->id, $payload, false); + $this->assertTrue($result['ok']); + return $result; + } + + /** + * Under the OVERALL model the overall is recomputed from the FULL itemscores map, + * not from the answered iDevices alone (exelearning issue 2481): an unanswered + * iDevice counts 0 for that attempt, as in the package's own score. Answering each + * of two equally weighted iDevices in a separate visit is worth 50 per attempt, so + * "Highest attempt" publishes 50, never 100. + */ + public function test_ingest_overall_is_recomputed_from_the_full_map(): void { + [$instance, $student] = $this->create_activity_with_student([ + 'grademodel' => EXELEARNING_GRADEMODEL_OVERALL, + 'grademethod' => \mod_exelearning\local\attempts::GRADE_HIGHEST, + ]); + $tf = $this->objectid_for($instance, 1); + $guess = $this->objectid_for($instance, 2); + + $first = $this->submit_two_idevice_visit($instance, $student, 'visitTf', 100.0, 0.0, [$tf]); + $this->assertEqualsWithDelta(50.0, $first['rawscore'], 0.0001); + $second = $this->submit_two_idevice_visit($instance, $student, 'visitGuess', 0.0, 100.0, [$guess]); + $this->assertEqualsWithDelta(50.0, $second['rawscore'], 0.0001); + + $this->assertEqualsWithDelta(50.0, $this->published_grade($instance, $student->id, 0), 0.0001); + // Each attempt still records a per-iDevice row only for the iDevice answered in it. + $this->assertEqualsWithDelta([0 => 50.0, 1 => 100.0], $this->attempt_rows($instance, $student->id, 1), 0.0001); + $this->assertEqualsWithDelta([0 => 50.0, 2 => 100.0], $this->attempt_rows($instance, $student->id, 2), 0.0001); + } + + /** + * Only the iDevices named in `answered` get a per-iDevice row (exelearning issue + * 2481). The second visit's full map still carries the load seed of 0 for the + * iDevice the learner did not answer; under "Last attempt" that iDevice keeps the + * 100 earned in the first visit, while the overall row follows the full map. */ - public function test_ingest_leaves_items_missing_from_itemscores_untouched(): void { + public function test_ingest_records_item_rows_only_for_answered_idevices(): void { [$instance, $student] = $this->create_activity_with_student([ 'grademodel' => EXELEARNING_GRADEMODEL_PERITEM, 'grademethod' => \mod_exelearning\local\attempts::GRADE_LAST, ]); - [$course, $cm] = $this->course_and_cm($instance); $tf = $this->objectid_for($instance, 1); $guess = $this->objectid_for($instance, 2); - $submit = function (string $session, array $itemscores) use ($instance, $course, $cm, $student): void { - $result = track::ingest($instance, $course, $cm, $student->id, [ - 'session' => $session, - // Raw matches the overall recomputed from the one scored iDevice, so - // ingest() does not report a divergence (DEC-6-01). - 'cmi' => ['cmi.core.score.raw' => '100', 'cmi.core.score.max' => '100'], - 'itemscores' => $itemscores, - ], false); - $this->assertTrue($result['ok']); - }; - $submit('visitTf', [$tf => ['scorepct' => 100.0, 'weighted' => 50.0, 'title' => 'TF']]); - $submit('visitGuess', [$guess => ['scorepct' => 100.0, 'weighted' => 50.0, 'title' => 'Guess']]); + $this->submit_two_idevice_visit($instance, $student, 'visitTf', 100.0, 0.0, [$tf]); + $second = $this->submit_two_idevice_visit($instance, $student, 'visitGuess', 0.0, 100.0, [$guess]); + $this->assertEquals([2], array_keys($second['peritem'])); $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 1), 0.0001); $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 2), 0.0001); + $this->assertEqualsWithDelta([0 => 50.0, 2 => 100.0], $this->attempt_rows($instance, $student->id, 2), 0.0001); + } + + /** + * An empty `answered` list records the attempt's overall row but no per-iDevice + * row: every score in the map is a load seed. + */ + public function test_ingest_with_empty_answered_records_only_the_overall(): void { + [$instance, $student] = $this->create_activity_with_student(); + + $result = $this->submit_two_idevice_visit($instance, $student, 'visitNone', 0.0, 0.0, []); + + $this->assertSame([], $result['peritem']); + $this->assertEqualsWithDelta([0 => 0.0], $this->attempt_rows($instance, $student->id, 1), 0.0001); + $this->assertNull($this->published_grade($instance, $student->id, 1)); + $this->assertNull($this->published_grade($instance, $student->id, 2)); + } + + /** + * Without an `answered` key (the mobile web service, an older cached tracker) + * every scored iDevice is recorded, exactly as before. + */ + public function test_ingest_without_answered_records_every_item(): void { + [$instance, $student] = $this->create_activity_with_student(); + + $result = $this->submit_two_idevice_visit($instance, $student, 'visitLegacy', 0.0, 100.0, null); + + $this->assertEquals([1, 2], array_keys($result['peritem'])); + $this->assertEqualsWithDelta(0.0, $this->published_grade($instance, $student->id, 1), 0.0001); + $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 2), 0.0001); + } + + /** + * `answered` is client input: unknown objectids and non-string entries are + * dropped, and a list that is not an array names nothing. + */ + public function test_ingest_validates_the_answered_list(): void { + [$instance, $student] = $this->create_activity_with_student(); + $guess = $this->objectid_for($instance, 2); + + $result = $this->submit_two_idevice_visit( + $instance, + $student, + 'visitMixed', + 0.0, + 100.0, + ['fake-unknown', 7, ['nested'], $guess] + ); + $this->assertEquals([2], array_keys($result['peritem'])); + + [$course, $cm] = $this->course_and_cm($instance); + $malformed = track::ingest($instance, $course, $cm, $student->id, [ + 'session' => 'visitMalformed', + 'cmi' => ['cmi.core.score.raw' => '100', 'cmi.core.score.max' => '100'], + 'itemscores' => [$guess => ['scorepct' => 100.0, 'weighted' => 100.0, 'title' => 'Guess']], + 'answered' => $guess, + ], false); + $this->assertDebuggingCalled(); + $this->assertTrue($malformed['ok']); + $this->assertSame([], $malformed['peritem']); + } + + /** + * When the client map is empty the server reads the scores from an `exe12/` + * cmi.suspend_data itself; `answered` still decides which iDevices get a row, so + * the seeds the tracker left out are not recorded there either (issue 2481). + */ + public function test_ingest_exe12_suspend_data_fallback_honours_answered(): void { + [$instance, $student] = $this->create_activity_with_student(); + [$course, $cm] = $this->course_and_cm($instance); + $tf = $this->objectid_for($instance, 1); + $guess = $this->objectid_for($instance, 2); + + $result = track::ingest($instance, $course, $cm, $student->id, [ + 'session' => 'sessExe12Answered', + 'cmi' => [ + 'cmi.core.score.raw' => '50', + 'cmi.core.score.max' => '100', + // The True/False record is the load seed; Guess was answered. + 'cmi.suspend_data' => 'exe12/1|' . rawurlencode($tf) . ';7;0;1;0;50;0;100' + . '|' . rawurlencode($guess) . ';7;1;1;100;50;0;100', + ], + 'itemscores' => [], + 'answered' => [$guess], + ], false); + + $this->assertTrue($result['ok']); + $this->assertEqualsWithDelta(50.0, $result['rawscore'], 0.0001); + $this->assertEqualsWithDelta([0 => 50.0, 2 => 100.0], $this->attempt_rows($instance, $student->id, 1), 0.0001); + $this->assertNull($this->published_grade($instance, $student->id, 1)); + } + + /** + * The legacy page-local fallback honours `answered` too: each slot is matched + * through the objectid of the grade item it is routed to. + */ + public function test_ingest_legacy_suspend_data_fallback_honours_answered(): void { + [$instance, $student] = $this->create_activity_with_student(); + [$course, $cm] = $this->course_and_cm($instance); + $guess = $this->objectid_for($instance, 2); + + $result = track::ingest($instance, $course, $cm, $student->id, [ + 'session' => 'sessLegacyAnswered', + 'cmi' => [ + 'cmi.core.score.raw' => '50', + 'cmi.core.score.max' => '100', + 'cmi.suspend_data' => "1. \"TF\"; Score: 0%; Weight: 50%.\t2. \"Guess\"; Score: 100%; Weight: 50%.", + ], + 'answered' => [$guess], + ], false); + + $this->assertTrue($result['ok']); + $this->assertEqualsWithDelta([0 => 50.0, 2 => 100.0], $this->attempt_rows($instance, $student->id, 1), 0.0001); + $this->assertNull($this->published_grade($instance, $student->id, 1)); + $this->assertEqualsWithDelta(100.0, $this->published_grade($instance, $student->id, 2), 0.0001); } /**