Skip to content

Fix: Report completion to LMS when course completes during restore (fixes #319)#368

Draft
swashbuck wants to merge 1 commit into
masterfrom
issue/319
Draft

Fix: Report completion to LMS when course completes during restore (fixes #319)#368
swashbuck wants to merge 1 commit into
masterfrom
issue/319

Conversation

@swashbuck

@swashbuck swashbuck commented May 27, 2026

Copy link
Copy Markdown
Contributor

🚧 WORK IN PROGRESS / DRAFT — do not merge. Opened for review only; will be finalised after review.

Fixes #319

Fix

A trickle-locked course using _completionCriteria._requireContentCompleted: true could fail to report a completed status to the LMS when the course completed for the first time during the restore on reopen.

Root cause: core tracking.js subscribes to Adapt.course change:_isComplete on app:dataReady and fires tracking:complete. Spoor only subscribed to tracking:complete inside setupEventListeners, which runs on adapt:start (after app:dataReady). Trickle buttons are created on adapt:start and are not tracked, so they do not exist during the restore. On reopen, the restored content completes the blocks in the app:dataReady window, the course completes, and core fires tracking:complete before spoor is listening. The event is missed and the LMS status is never written.

This change subscribes tracking:complete in beginSession as well, so the listener exists before the restore completion cascade. It mirrors how app:dataReady and adapt:start are already registered in both beginSession and setupEventListeners; setupEventListeners still re-binds it for the live session after its stopListening reset, so there is exactly one active listener at any time.

Testing

  1. Author a trickle step-locked, single-page course with _completionCriteria._requireContentCompleted: true and spoor _onTrackingCriteriaMet: "completed".
  2. Session 1: complete all content, click every trickle button except the final one, leaving the course incomplete. Confirm the LMS status is incomplete and suspend data is committed.
  3. Reopen the course (preserving suspend data). On restore the course completes.
  4. Before this fix: LMS status stays incomplete. After this fix: LMS status becomes completed.

Also verified, with the early listener in place:

  • Normal live completion writes status once (no double write).
  • A partial session (final button unclicked) writes nothing prematurely.
  • Assessment courses: pass/fail each write the correct status once; a trickle + assessment course whose completion lands in the restore window writes passed and the score correctly.
  • _isLockedOnRevisit re-completion fires once per genuine completion, with no spurious write on the intervening reset.

Alternatives considered / anticipated questions

Is this trickle behaving as intended? Should the course stay incomplete until the final button is clicked?
The course completing on restore is correct: all content components are genuinely complete, and trickle buttons are deliberately non-tracked UI for pacing, not learning criteria. Requiring a learner to re-click "Finish" on resume to re-confirm already-complete content would be poor UX and is not how resume is expected to work. The defect is not that the course completes; it is that the tracking:complete event is dropped, so the LMS is never told. That event loss is unambiguously a bug.

Why fix in spoor rather than core?
The race is generic (core fires tracking:complete during restore before consumers subscribe), so a core-side change is a legitimate option, e.g. deferring the completion check until after adapt:start. That was rejected for now because it has a much larger blast radius across every tracking:complete consumer, whereas the spoor-side listener is an 8-line, self-contained change that restores the intended contract (spoor listens for completion) without altering core timing for anyone else. Happy to move it to core if maintainers prefer the systemic fix.

Why not just disable trickle on the final block?
It does avoid the bug (step locking means the only "all content complete but a gate pending" state is the final button), but it is not an acceptable resolution. The vulnerable configuration is trickle's own documented default: example.json ships _showEndOfPage: true with a final "Finish" button. So the workaround means "do not use trickle the way the docs show," it is silent (no warning links the missing LMS status to a Finish button), and it removes a deliberate end-of-page affordance to dodge a tracking-layer timing issue. It masks one trigger, not the underlying event-ordering gap.

Does this affect xAPI / adapt-contrib-xapi?
Possibly. xapi subscribes to tracking:complete and may share the same restore-window timing gap (candidate cause of adapt-contrib-xapi#108). Out of scope for this PR, but flagging it: if maintainers prefer the core-side fix, it would likely cover xapi too.

Posted via collaboration with Claude Code

@swashbuck

Copy link
Copy Markdown
Contributor Author

@oliverfoster Will you review this PR? What are your thoughts on implementing this fix in Spoor vs. Core (assuming you agree that it's a bug that should be fixed)?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

Development

Successfully merging this pull request may close these issues.

Course won't complete if Trickle unlocks

2 participants