schema amends: change boolean selects to checkboxes #86
Conversation
Also set the _randomisation._blockCount to -1 by default, meaning all blocks are randomised if the setting is enabled.
The .slice() call was returning an array, when what we really want is the first assessment if no 'id' is specified.
- Returns the first item by default in _getStateByAssessmentId() if a blank 'id' is passed - Adds a warning message if an assessment has been registerd with an empty 'id' or if the 'id' already exists
js/assessment.js
Outdated
| @@ -242,7 +242,7 @@ define([ | |||
|
|
|||
| get: function(id) { | |||
| if (id === undefined) { | |||
| return this._assessments.slice(0); | |||
| return this._assessments[0]; | |||
There was a problem hiding this comment.
this isn't a minor change:
get() == getAll() ! getFirst();
get("id") == getById(id)
if you need getFirst may I suggest you add a new function?
getFirst: function() {
return this._assessments[0];
},
getAll: function() {
return this._assessments.slice(0);
}
There was a problem hiding this comment.
Good catch @oliverfoster. I didn't actually need a getFirst(). The change in 4ec4121 seems to do enough.
There was a problem hiding this comment.
I still don't think the approach you've taken solves the problem you've described.
issue: users aren't filling in the assessment id on the article and results component because it's not obvious or clear and it's breaking Adapt
solution: make assessment results and assessment work with no ids where appropriate
var assessmentModel;
if (!state.id) {
assessmentModel = Adapt.assessment.get()[0];
} else {
assessmentModel = Adapt.assessment.get(state.id);
}^ if no id defined in assessmentResults, pick the first one.
var assessmentConfig = this.get("_assessment");
if (!assessmentConfig._id) {
assessmentConfig._id = this.model.get("_id");
}^ if no id defined in assessment, take article id
My misunderstanding of this function introduced a breaking change, which @oliverfoster caught. A better solution is to standardise the handling of undefined or assessment 'id' values which are empty strings. Warning messages are now output when appropriate, but the code allows an empty assessment 'id', e.g. if there is only one assessment on the course.
|
Sorry but I do have to say this has now got well beyond the bounds of what should be 'tacked on' to a a PR that is specifically to do with schema changes! @brian-learningpool would you mind setting up an issue that documents the problem you are trying to solve and creating a new PR to go with it? |
|
Issue raised here: |
|
Closing PR at @moloko's request. This has been split into two separate PRs here: |
see adaptlearning/adapt_authoring#1380
please leave unmerged until all core plugins have been updated