Skip to content

Breaking: Version 2 improvements (fixes #29)#30

Open
oliverfoster wants to merge 36 commits intomasterfrom
issue/29
Open

Breaking: Version 2 improvements (fixes #29)#30
oliverfoster wants to merge 36 commits intomasterfrom
issue/29

Conversation

@oliverfoster
Copy link
Copy Markdown
Member

@oliverfoster oliverfoster commented Jul 24, 2025

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

Comment thread js/utils/models.js Outdated
Comment thread js/TotalSets.js Outdated
@cahirodoherty-learningpool
Copy link
Copy Markdown
Contributor

Conflicts need resolved on this

@joe-replin
Copy link
Copy Markdown

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

Comment thread js/helpers.js Outdated
Comment thread js/Lifecycle.js Outdated
Comment thread js/ScoringSet.js Outdated
Comment thread 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`.
Comment thread js/AdaptModelSet.js
Comment thread js/adapt-contrib-scoring.js
Comment thread js/TotalSets.js Outdated
Comment thread js/Lifecycle.js Outdated
danielghost and others added 8 commits April 15, 2026 12:26
…ause the other sets hadn't yet been restored. Removed unnecessary overrides, with a decision to use inherited `ScoringSet` events and move from `'scoring:*'` to `'scoring:total:*'` - core framework listeners will be updated accoprdingly. Namespaced lifecycle events for clarity.
…`TotalSets` as the score logic for the modifiers is different, with the previous modifier scores not representing the score associated with that set.
…rather than the models, so it is easier to understand how the scores were updated. Fixed issue with logging modifier score changes when models become unavailable.
…dy but would need a direct import of the file, so exposed in same way as other methods and classes).
…te` and `isPassed`. Prevented potential for last registered set to be removed if attempting to deregister a set that hadn't been registered (no path in the current code where deregister receives an unregistered set, but added defensive coding in case a set authour breaks the inheritance chain).
Comment thread js/Lifecycle.js Outdated
*/
onScoringSetReset(set) {
// TODO: determine if we should restart the sets on this model id only or all descendant sets as well
if (!set.model) return;
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not all sets have a single model, so resetting one of these will not reset all intersected sets. Should any set without a parent model actually allow a reset anyway?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What happens when you give a parent model to your sets that currently have no parent model?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a parent model to something like a BucketSet, which is configured on the course model, will cause all sets with this parent to reset, even if those sets models don't intersect any other models (excluding course) associated with the set being reset. This prevents the ability to only reset one set configured at the course level.

Suggest amending the logic to match the TODO comment and use:

const sets = filterSetsByIntersectingModels(getAllSets(), set.models);

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should talk about this.

Comment thread js/helpers.js
@oliverfoster
Copy link
Copy Markdown
Member Author

26cd9d3

This is quite a big change and not what was in the to-do comment. It looks like you've done intersecting sets, which also include ancestors.

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

Projects

Development

Successfully merging this pull request may close these issues.

Version 2

4 participants