Breaking: Version 2 improvements (fixes #29) by oliverfoster · Pull Request #30 · adaptlearning/adapt-contrib-scoring

oliverfoster · 2025-07-24T21:20:02Z

fixes #29

Fix

Reverted changes to broken intersection queries

Update

isAvailableInHierarchy to account for detached models
sets sorted by .order
removed the word raw throughout and changed for more accurate names, availableComponents etc
removed the word sub throughout, from subset, and changed for more accurate names, set, getSet.., etc
generally refactor code so that logic is more distinct and easier to read
split scoringset into interactionset, lifecycleset and scoringset
adaptmodelset extends interactionset,
lifecycleset extends interactionset
isolate the compatibility layer
automatically assign set ids
formalise State and Objective apis
formalise lifecycle phases and triggers for init, restore, start, reset, restart, visit, leave and update, fully integrating adapt-contrib-modifiers
add order for lifecycle elements so that adapt-contrib-banking, adapt-contrib-randomise and adapt-contrib-scoringAssessment can execute in order, ordering sets appropriately
separate utility functions into appropriate files
add jsdocs throughout

New

added multiple functions to account for detached models
added TotalSets to replace the set behaviour on the main Adapt.scoring API
document INTERSECTION_QUERY.md
document LIFECYCLE.md

Breaking

Almost every function has been renamed or moved

js/utils/models.js

js/TotalSets.js

…dels

cahirodoherty-learningpool · 2025-10-21T14:02:48Z

Conflicts need resolved on this

Logging improvements migrated from #26

…ib-scoring into issue/29

joe-replin · 2026-01-02T16:45:37Z

Moved to resolve conflicts and brought in the logging improvements from #26

js/helpers.js

js/ScoringSet.js

Corrected wording for clarity in IntersectionSet section.

oliverfoster · 2026-03-17T14:14:04Z

~~Joe's commits need reverting. The modifiers stuff has been included in the code already and doesn't need a secondary interface. That commit is a misunderstanding of the intent of version 2.~~
Update: terminology mixup. Joe has reused the word "modifiers" which is an older concept from Dan for a different design part. That name probably needs changing, the functions simplified and some more function comments adding.

a1fb354

js/ScoringSet.js

…oved event trigger for 'scoring:reset' to `Lifecycle` for consistency. Questionable whether a public method to update scoring is required. No longer used directly in scoring plugins as was the case with v1.

… `IntersectionSet` rather than `ScoringSet` as this is only required for intersection queries.

oliverfoster · 2026-03-19T17:40:08Z

js/ScoringSet.js

+   * @param {Backbone.Model} model
+   * @returns {number}
+   */
+  getMinScoreByModel(model) {


If these functions, and those added by Joe, aren't used anywhere other than the logging code, can we move them out of the main ScoringSet API and into a helpers file? Or fold them into the places that they're called from? I don't see a need for multiple small functions when a single larger one would work.

This is almost 10 new main API functions, just for printing logs.

Will look into offloading the logic into another class, similar to how objectives are handled, but the different methods were setup to allow subclass extensibility, so we can easily override just the bits that differ between sets.

I don't see any use case for these beyond the logging stuff. Are you intending that the logging be overridden per set?

danielghost · 2026-03-20T16:25:27Z

js/ScoringSet.js

-    return this.models.some(model => model.get('_isVisited'));
+  get isPartlyCorrect() {
+    if (!this.isSubmitted) return null;
+    return (this.correctness < this.maxCorrectness);


Returns true if correctness is 0.

danielghost · 2026-03-20T16:29:37Z

js/ScoringSet.js

-    return this.isStarted && !this.isComplete;
+  get isIncorrect() {
+    if (!this.isSubmitted) return null;
+    return (!this.correctness && this.maxCorrectness);


Returns the maxCorrectness value if correctness equals 0.

danielghost · 2026-04-01T19:21:58Z

js/Lifecycle.js

+    },
+    // restart calls set.onStart when any intersecting model or set is reset
+    async restart(set) {
+      await set.onStart?.();


Could this call set.onRestart?.() instead to distinguish between a first attempt and a restart attempt? An alternative would be to have a flag akin to how wasRestored is used, but two different methods seems like a cleaner option. Is there a specific reason why this wouldn't work or cause problems?

See https://github.com/adaptlearning/adapt-contrib-scoring/pull/30/changes#r3024196814 for some context.

In cases where a set may require start and restart to be the same, onRestart called also call onStart within the set. Even in the current AssessmentSet.onStart`, some of these methods probably only need to happen after a reset.

I didn't see any difference when I was rewriting everything. Just that reset/restart is usually exactly the same behaviour as start. Start is always the beginning of the lifecycle and reset, although seemingly at the end, is always the beginning of the lifecycle. I removed the on restart phase to clarify that it is effectively starting again. If you need to keep state between attempts then that state should be kept and managed on the set and not in the lifecycle stuff, where it's just confusing. Reset is not a finished or complete.

js/ScoringSet.js

…ds in `ScoringSet` to query its models rather rather than the single parent model (which isn't always populated in a set). Fixed issues when evaluating correctness.

…rt attempt. Marked lifecycle callback hooks as protected methods, as calling these externally defeats the benefits of the LifecycleRenderer.

…ntrib-scoringAssessment#17. Resolved reset issues as the objective needs to wait until the set has been reset before updating the score and status - adjusted methods and logic accordingly so score and status can be handled separately by the sets lifecycle methods.

…class to keep this logic separate from the `ScoringSet`.

danielghost · 2026-04-14T08:48:10Z

js/AdaptModelSet.js

-   * Intentionally empty to prevent super Class event triggers
-   * @override
-   */
-  onCompleted() {}


Do we need AdaptModelSet to be firing the scoring event triggers? Anything listening to events such as 'scoring:set:complete' will be triggered every time one of these models is completed. Are we ever likely to use something like https://github.com/adaptlearning/adapt-contrib-scoringResults for these type of sets?

Leaving them in means we have a chatty scoring event system, which will probably not be utilised for these models. As these sets are only really needed to allow the queries and intersections to work, I would probably prevent them from firing the events to match v1, but happy to leave them in if it offers us more flexibility without compromising performance.

I think leave to them in; it's a valid event to fire as a set is being completed.

danielghost · 2026-04-14T08:56:26Z

js/adapt-contrib-scoring.js

-  _updateQueue(model) {
-    this._queuedChanges.push(model);
-    this._debouncedUpdate();
+  getSetById(id) {


This is no longer exported. It can obviously be used via Adapt.scoring.getSetById, but should we make all of these types of methods utility functions that can be imported by plugins as required?

Currently we have a mixture of those that can only be referenced via Adapt.scoring and others which can be both imported individually or used via the scoring object. Do we need both, or just use imports?

They should be exported, yes.

Adapt.scoring references are easy to grab from the console. This can be handy for debugging.

danielghost · 2026-04-14T11:33:49Z

js/TotalSets.js

+  /** @override */
+  onCompleted() {
+    if (this.isIntersectedSet) return;
+    Adapt.trigger('scoring:complete', Adapt.scoring);


Should use this as an argument rather than Adapt.scoring, to provide the correct context to listeners such as https://github.com/adaptlearning/adapt-contrib-core/blob/2c6ef07f8bfe5fe4dd317e4d0dad899cbd1f86e1/js/tracking.js#L93.

danielghost · 2026-04-14T13:18:07Z

js/Lifecycle.js

+  async restore() {
+    const sets = getAllSets();
+    await this.renderer.render.restore(sets);
+    Adapt.trigger('scoring:restored', this.scoring);


As per https://github.com/adaptlearning/adapt-contrib-scoring/pull/30/changes#r3079115424, the event triggers on this file should include the TotalSets context unless we change the way the listeners currently work.

oliverfoster added 3 commits July 24, 2025 22:11

Breaking: Version 2 improvements (fixes #29)

47a69e3

Markdown caps

b6cdba5

Merdown edits

f35cfaf

oliverfoster self-assigned this Jul 24, 2025

oliverfoster added the enhancement label Jul 24, 2025

oliverfoster added this to adapt_framework: The TODO Board Jul 24, 2025

github-project-automation bot moved this to New in adapt_framework: The TODO Board Jul 24, 2025

oliverfoster moved this from New to Needs Reviewing in adapt_framework: The TODO Board Jul 24, 2025

jsdoc updates

bb2e6fb

oliverfoster commented Jul 25, 2025

View reviewed changes

js/utils/models.js Outdated Show resolved Hide resolved

danielghost reviewed Jul 25, 2025

View reviewed changes

js/TotalSets.js Outdated Show resolved Hide resolved

oliverfoster added 3 commits July 28, 2025 12:14

jsdoc fixes

fc471eb

jsdoc fixes

a0213c4

jsdoc updates

7b59a5b

oliverfoster added 2 commits July 30, 2025 06:05

Formalise canReset

9c6bb79

jsdoc typo

4a06e65

oliverfoster mentioned this pull request Jul 30, 2025

Breaking: Upgrade to version 2 of scoring (fixes #21) adaptlearning/adapt-contrib-scoringAssessment#22

Open

oliverfoster added 2 commits August 12, 2025 08:37

Extend adaptmodelset to include more questionmodel interfacing

59be83f

Fixed findDescendantModels and getAllDescendantModels for detached mo…

6b89283

…dels

joe-replin added 2 commits January 2, 2026 10:36

Resolving conflicts & Implementing logging improvements.

a1fb354

Logging improvements migrated from #26

Merge branch 'master' of https://github.com/adaptlearning/adapt-contr…

ebdd9e4

…ib-scoring into issue/29

joe-replin reviewed Jan 2, 2026

View reviewed changes

js/helpers.js Show resolved Hide resolved

Schema nesting issue.

8e5cd11

Reverting FW version

8650fb5

danielghost reviewed Mar 17, 2026

View reviewed changes

js/ScoringSet.js Outdated Show resolved Hide resolved

Fix wording in IntersectionSet description

b93a076

Corrected wording for clarity in IntersectionSet section.

danielghost reviewed Mar 17, 2026

View reviewed changes

js/ScoringSet.js Outdated Show resolved Hide resolved

danielghost added 2 commits March 18, 2026 16:13

Update to remove the duplicated event trigger for 'scoring:update'. M…

8001592

…oved event trigger for 'scoring:reset' to `Lifecycle` for consistency. Questionable whether a public method to update scoring is required. No longer used directly in scoring plugins as was the case with v1.

Incorprated the log logic from #26. Amended AdaptModelSet to extend…

c530c6d

… `IntersectionSet` rather than `ScoringSet` as this is only required for intersection queries.

oliverfoster commented Mar 19, 2026

View reviewed changes

danielghost reviewed Mar 20, 2026

View reviewed changes

danielghost moved this from Needs Reviewing to Assigned in adapt_framework: The TODO Board Mar 20, 2026

danielghost reviewed Apr 1, 2026

View reviewed changes

js/ScoringSet.js Outdated Show resolved Hide resolved

danielghost reviewed Apr 2, 2026

View reviewed changes

js/ScoringSet.js Show resolved Hide resolved

danielghost added 5 commits April 2, 2026 19:01

Fixed @typedef paths and typos.

53b5cb6

Reverted AdaptModelSet to extend ScoringSet. Amended filter metho…

67e9b5a

…ds in `ScoringSet` to query its models rather rather than the single parent model (which isn't always populated in a set). Fixed issues when evaluating correctness.

Adjusted lifecycle to distinguish between a first attempt and a resta…

b54b50a

…rt attempt. Marked lifecycle callback hooks as protected methods, as calling these externally defeats the benefits of the LifecycleRenderer.

Moved the log for the scoring updates functionality into a Journal …

383b0e8

…class to keep this logic separate from the `ScoringSet`.

danielghost reviewed Apr 14, 2026

View reviewed changes

Conversation

oliverfoster commented Jul 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Fix

Update

New

Breaking

Uh oh!

Uh oh!

Uh oh!

cahirodoherty-learningpool commented Oct 21, 2025

Uh oh!

joe-replin commented Jan 2, 2026

Uh oh!

Uh oh!

Uh oh!

oliverfoster commented Mar 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

oliverfoster Mar 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielghost Apr 1, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

danielghost Apr 14, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

oliverfoster commented Jul 24, 2025 •

edited

Loading

oliverfoster commented Mar 17, 2026 •

edited

Loading

oliverfoster Mar 19, 2026 •

edited

Loading

danielghost Apr 1, 2026 •

edited

Loading

danielghost Apr 14, 2026 •

edited

Loading