@@ -291,7 +153,6 @@ const VariableListModal = props => {
isSelected={index === currSelect}
creatingVariable={creatingVariable}
firstRef={index === 0 ? firstRef : null}
- onClickVariable={onClickVariable}
index={index}
onClick={() => onClickVariable(index)}
/>
diff --git a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/variables/variable-util.js b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/variables/variable-util.js
index 79405d2563..46b5be1ba3 100644
--- a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/variables/variable-util.js
+++ b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/variables/variable-util.js
@@ -1,3 +1,5 @@
+import { getParsedRange } from '../../../common/util/range-parsing'
+
import {
STATIC_VALUE,
STATIC_LIST,
@@ -27,55 +29,66 @@ const DEFAULT_VALUES = {
}
const changeVariableToType = (variable, type) => {
+ const alteredVariable = { ...variable }
const desiredAttrsList = ['name', 'type']
- switch(type) {
+ switch (type) {
case STATIC_VALUE:
case STATIC_LIST:
case PICK_ONE:
desiredAttrsList.push('value')
break
case RANDOM_NUMBER:
- desiredAttrsList.push(...['valueMin','valueMax','decimalPlacesMin','decimalPlacesMax'])
+ desiredAttrsList.push(...['valueMin', 'valueMax', 'decimalPlacesMin', 'decimalPlacesMax'])
break
case RANDOM_LIST:
- desiredAttrsList.push(...['sizeMin','sizeMax','unique','valueMin','valueMax','decimalPlacesMin','decimalPlacesMax'])
+ desiredAttrsList.push(
+ ...[
+ 'sizeMin',
+ 'sizeMax',
+ 'unique',
+ 'valueMin',
+ 'valueMax',
+ 'decimalPlacesMin',
+ 'decimalPlacesMax'
+ ]
+ )
break
case RANDOM_SEQUENCE:
- desiredAttrsList.push(...['sizeMin','sizeMax','start','seriesType', 'step'])
+ desiredAttrsList.push(...['sizeMin', 'sizeMax', 'start', 'seriesType', 'step'])
break
case PICK_LIST:
- desiredAttrsList.push(...['chooseMin','chooseMax','ordered'])
+ desiredAttrsList.push(...['chooseMin', 'chooseMax', 'ordered'])
break
}
// strip any unwanted attributes
- for (const attr in variable) {
- if ( ! desiredAttrsList.includes(attr)) delete variable[attr]
+ for (const attr in alteredVariable) {
+ if (!desiredAttrsList.includes(attr)) delete alteredVariable[attr]
}
// add any missing attributes
desiredAttrsList.forEach(attr => {
- if ( ! Object.keys(variable).includes(attr)) variable[attr] = DEFAULT_VALUES[attr] ?? ''
+ if (!Object.keys(alteredVariable).includes(attr)) alteredVariable[attr] = DEFAULT_VALUES[attr]
})
// check for new issues
const variableErrors = {}
- for (const attr in variable) {
- const error = validateVariableValue(attr, variable[attr])
+ for (const attr in alteredVariable) {
+ const error = validateVariableValue(attr, alteredVariable[attr])
if (error) variableErrors[attr] = true
}
if (Object.keys(variableErrors).length) {
- variable.errors = variableErrors
+ alteredVariable.errors = variableErrors
}
- return variable
+ return alteredVariable
}
const validateVariableValue = (name, value) => {
- switch(name) {
+ switch (name) {
// variable names should only contain alphanumeric characters and underscores
// variable names should only ever start with an underscore or a letter
case 'name':
- return ! (new RegExp(/^[_a-zA-Z]{1}([_a-zA-Z0-9])*$/).test(value))
+ return !new RegExp(/^[_a-zA-Z]{1}([_a-zA-Z0-9])*$/).test(value)
// min/max list sizes and decimal place values must be integers
case 'decimalPlacesMin':
case 'decimalPlacesMax':
@@ -83,17 +96,17 @@ const validateVariableValue = (name, value) => {
case 'sizeMax':
case 'chooseMin':
case 'chooseMax':
- return ( value === '' || ! Number.isInteger(Number(value)))
+ return value === '' || !Number.isInteger(Number(value))
// min/max and sequence start/step numeric values have to be numbers, but not integers
// this should be constrained by the input fields, but we can check it here just to be safe
case 'valueMin':
case 'valueMax':
case 'start':
case 'step':
- return (value === '' || isNaN(Number(value)))
+ return value === '' || isNaN(Number(value))
// this will be an empty string by default, but one of the options must be chosen to be valid
case 'seriesType':
- return ! (value === 'arithmetic' || value === 'geometric')
+ return !(value === 'arithmetic' || value === 'geometric')
// no validation should be necessary otherwise
default:
break
@@ -116,8 +129,152 @@ const validateMultipleVariables = variables => {
return variables
}
+const rangesToIndividualValues = vars => {
+ if (!vars) return []
+
+ return vars.map(v => {
+ switch (v.type) {
+ case RANDOM_LIST: {
+ const size = getParsedRange(v.size)
+ const decimalPlaces = getParsedRange(v.decimalPlaces)
+ const value = getParsedRange(v.value)
+
+ return {
+ name: v.name,
+ type: v.type,
+ sizeMin: size.min,
+ sizeMax: size.max,
+ decimalPlacesMin: decimalPlaces.min,
+ decimalPlacesMax: decimalPlaces.max,
+ valueMin: value.min,
+ valueMax: value.max,
+ unique: v.unique
+ }
+ }
+
+ case RANDOM_SEQUENCE: {
+ const size = getParsedRange(v.size)
+
+ return {
+ name: v.name,
+ type: v.type,
+ sizeMin: size.min,
+ sizeMax: size.max,
+ start: v.start,
+ step: v.step,
+ seriesType: v.seriesType
+ }
+ }
+
+ case RANDOM_NUMBER: {
+ const value = getParsedRange(v.value)
+ const decimalPlaces = getParsedRange(v.decimalPlaces)
+
+ return {
+ name: v.name,
+ type: v.type,
+ valueMin: value.min,
+ valueMax: value.max,
+ decimalPlacesMin: decimalPlaces.min,
+ decimalPlacesMax: decimalPlaces.max
+ }
+ }
+
+ case PICK_LIST: {
+ const choose = getParsedRange(v.choose)
+
+ return {
+ name: v.name,
+ type: v.type,
+ chooseMin: choose.min,
+ chooseMax: choose.max,
+ value: v.value,
+ ordered: v.ordered
+ }
+ }
+
+ case PICK_ONE:
+ case STATIC_VALUE:
+ case STATIC_LIST: {
+ return {
+ name: v.name,
+ type: v.type,
+ value: v.value
+ }
+ }
+
+ default:
+ throw 'Unexpected type!'
+ }
+ })
+}
+
+const individualValuesToRanges = vars => {
+ if (!vars) return []
+
+ return vars.map(v => {
+ switch (v.type) {
+ case RANDOM_LIST: {
+ return {
+ name: v.name,
+ type: v.type,
+ size: `[${v.sizeMin},${v.sizeMax}]`,
+ decimalPlaces: `[${v.decimalPlacesMin},${v.decimalPlacesMax}]`,
+ value: `[${v.valueMin},${v.valueMax}]`,
+ unique: v.unique
+ }
+ }
+
+ case RANDOM_SEQUENCE: {
+ return {
+ name: v.name,
+ type: v.type,
+ size: `[${v.sizeMin},${v.sizeMax}]`,
+ start: v.start,
+ step: v.step,
+ seriesType: v.seriesType
+ }
+ }
+
+ case RANDOM_NUMBER: {
+ return {
+ name: v.name,
+ type: v.type,
+ value: `[${v.valueMin},${v.valueMax}]`,
+ decimalPlaces: `[${v.decimalPlacesMin},${v.decimalPlacesMax}]`
+ }
+ }
+
+ case PICK_LIST: {
+ return {
+ name: v.name,
+ type: v.type,
+ choose: `[${v.chooseMin},${v.chooseMax}]`,
+ value: v.value,
+ ordered: v.ordered
+ }
+ }
+
+ case PICK_ONE:
+ case STATIC_VALUE:
+ case STATIC_LIST: {
+ return {
+ name: v.name,
+ type: v.type,
+ value: v.value
+ }
+ }
+
+ default:
+ throw 'Unexpected type!'
+ }
+ })
+}
+
export {
changeVariableToType,
validateVariableValue,
- validateMultipleVariables
+ validateMultipleVariables,
+ rangesToIndividualValues,
+ individualValuesToRanges
}
diff --git a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js
index b894109d78..45b3e20acf 100644
--- a/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js
+++ b/packages/app/obojobo-document-engine/src/scripts/oboeditor/components/visual-editor.js
@@ -625,8 +625,6 @@ class VisualEditor extends React.Component {
)
}
- // console.log('renderLeaf', props)
-
if (leaf.highlight) {
return (
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/components/nav.js b/packages/app/obojobo-document-engine/src/scripts/viewer/components/nav.js
index 08a615f77d..428430a3ef 100644
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/components/nav.js
+++ b/packages/app/obojobo-document-engine/src/scripts/viewer/components/nav.js
@@ -109,7 +109,10 @@ export default class Nav extends React.Component {
}
renderLabel(label, sourceModel) {
- return this.substituteLabelVariables((label instanceof StyleableText ? label.value : label), sourceModel)
+ return this.substituteLabelVariables(
+ label instanceof StyleableText ? label.value : label,
+ sourceModel
+ )
}
substituteLabelVariables(label, sourceModel) {
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/components/viewer-app.js b/packages/app/obojobo-document-engine/src/scripts/viewer/components/viewer-app.js
index 5f609de74d..5fdbf6a8d4 100644
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/components/viewer-app.js
+++ b/packages/app/obojobo-document-engine/src/scripts/viewer/components/viewer-app.js
@@ -397,24 +397,8 @@ export default class ViewerApp extends React.Component {
})
}
- markSub(model, variable, value) {
- if (!this.subs[model.get('id')]) {
- this.subs[model.get('id')] = {}
- }
- this.subs[model.get('id')][variable] = value
-
- if (model.parent) {
- this.markSub(model.parent, variable, value)
- }
- }
-
getTextForVariable(event, variable, textModel) {
- if (!this.subs) {
- this.subs = {}
- window.__subs = this.subs
- }
-
- if (variable.indexOf('$') === 0) {
+ if (variable && variable.indexOf('$') === 0) {
variable = variable.substr(1)
let value = null
@@ -437,7 +421,6 @@ export default class ViewerApp extends React.Component {
}
if (value !== null) {
- this.markSub(textModel, variable, value)
event.text = value
}
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/question-store.js b/packages/app/obojobo-document-engine/src/scripts/viewer/stores/question-store.js
index 6b895f04e7..0c01c6352a 100644
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/question-store.js
+++ b/packages/app/obojobo-document-engine/src/scripts/viewer/stores/question-store.js
@@ -7,8 +7,6 @@ import FocusUtil from '../util/focus-util'
import NavStore from '../stores/nav-store'
import QuestionResponseSendStates from './question-store/question-response-send-states'
-import VariableUtil from '../util/variable-util'
-import VariableStore from './variable-store'
const { Store } = Common.flux
const { Dispatcher } = Common.flux
@@ -274,7 +272,6 @@ class QuestionStore extends Store {
assessmentId,
attemptId,
sendResponseImmediately
- // variableState: VariableUtil.getVariableStateSummary(VariableStore.getState())
}
}
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variable-generator.js b/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variable-generator.js
deleted file mode 100644
index ce1e811539..0000000000
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variable-generator.js
+++ /dev/null
@@ -1,269 +0,0 @@
-const { getParsedRange } = require('../../../common/util/range-parsing')
-const shuffle = require('../../../common/util/shuffle').default
-
-const parse = s => s.split(',')
-
-const getRange = rangeString => {
- const range = getParsedRange(rangeString)
-
- if (!range.isMinInclusive || !range.isMaxInclusive) {
- throw 'Range ' + rangeString + ' must be inclusive!'
- }
-
- if (range.min > range.max) {
- throw 'Range ' + rangeString + ' is inverted'
- }
-
- const min = parseFloat(range.min)
- const max = parseFloat(range.max)
-
- if (!Number.isFinite(min) || !Number.isFinite(max)) {
- throw 'Range ' + rangeString + ' has non-numeric values'
- }
-
- return [min, max]
-}
-
-const getPosIntRange = rangeString => {
- const [min, max] = getRange(rangeString)
-
- if (min < 0 || max < 0) {
- throw 'Range ' + rangeString + ' must be positive!'
- }
-
- if (parseInt(min, 10) !== min || parseInt(max, 10) !== max) {
- throw 'Range ' + rangeString + ' must be int values only'
- }
-
- return [min, max]
-}
-
-const getPosNonZeroIntRange = rangeString => {
- const [min, max] = getPosIntRange(rangeString)
-
- if (min < 1 || max < 1) {
- throw 'Range ' + rangeString + ' values must be non-zero!'
- }
-
- return [min, max]
-}
-
-class VariableGenerator {
- // generate(defs) {
- // const generated = defs.reduce(
- // (acc, def) => acc.concat(this.generateOne(def)),
- // []
- // )
-
- // const table = {}
- // generated.forEach(g => {
- // if (table[g.name]) {
- // throw 'Duplicate variable definition!'
- // }
-
- // table[g.name] = g.value
- // })
-
- // return table
- // }
-
- generateOne(def) {
- // if (def.type === 'set') {
- // return this.getSet(def)
- // }
-
- // if (!def.name) {
- // throw 'Missing required name property!'
- // }
-
- let value = null
-
- switch (def.type) {
- case 'random-list':
- value = this.getRandomList(def)
- break
-
- case 'random-sequence':
- value = this.getRandomSequence(def)
- break
-
- case 'random-number':
- value = this.getRandomNumber(def)
- break
-
- case 'pick-one':
- value = this.getPickOne(def)
- break
-
- case 'pick-list':
- value = this.getPickList(def)
- break
-
- case 'static-value':
- case 'static-list':
- value = def.value
- break
-
- // case 'fn':
- // value = parse(def)
- // break
-
- default:
- throw 'Unexpected type!'
- }
-
- // return {
- // name: def.name,
- // value
- // }
- return value
- }
-
- getRandomList(def) {
- const [sizeMin, sizeMax] = getPosIntRange(def.size)
- const [valueMin, valueMax] = getRange(def.value)
- const [decimalPlacesMin, decimalPlacesMax] = getPosIntRange(def.decimalPlaces)
-
- return this.generateRandomArray(
- this.rand(sizeMin, sizeMax),
- valueMin,
- valueMax,
- decimalPlacesMin,
- decimalPlacesMax,
- Boolean(def.unique)
- )
- }
-
- getRandomSequence(def) {
- const list = []
-
- const start = parseFloat(def.start) || 1
- const step = parseFloat(def.step) || 1
- const [sizeMin, sizeMax] = getPosIntRange(def.size)
- const size = this.rand(sizeMin, sizeMax)
- const seriesType = ('' + def.seriesType).toLowerCase()
-
- let next
- switch (seriesType) {
- case 'arithmetic':
- next = (arr, index, step) => arr[index - 1] + step
- break
-
- case 'geometric':
- next = (arr, index, step) => arr[index - 1] * step
- break
-
- default:
- throw 'Invalid sequence seriesType!'
- }
-
- list.push(start)
- for (let i = 1, len = size; i < len; i++) {
- list.push(next(list, i, step))
- }
-
- return list
- }
-
- getRandomNumber(def) {
- const [valueMin, valueMax] = getRange(def.value)
- const [decimalPlacesMin, decimalPlacesMax] = getPosIntRange(def.decimalPlaces)
-
- return this.rand(valueMin, valueMax, this.rand(decimalPlacesMin, decimalPlacesMax))
- }
-
- getPickOne(def) {
- return this.pickOne(parse(def.value))
- }
-
- getPickList(def) {
- const [chooseMin, chooseMax] = getPosNonZeroIntRange(def.choose)
-
- return this.pickMany(parse(def.value), chooseMin, chooseMax, Boolean(def.ordered))
- }
-
- // getSet(def) {
- // const names = {}
- // def.values.forEach(arr => {
- // arr.forEach(childDef => {
- // if (childDef.type === 'set') {
- // throw 'Unable to nest sets!'
- // }
-
- // if (!names[childDef.name]) {
- // names[childDef.name] = 0
- // }
-
- // names[childDef.name]++
- // })
- // })
-
- // const nameCounts = Object.values(names)
- // const expectedCount = nameCounts[0]
- // nameCounts.forEach(c => {
- // if (c !== expectedCount) {
- // throw 'Variable mismatch inside set!'
- // }
- // })
-
- // const chosenDefs = this.pickOne(def.values)
-
- // const results = chosenDefs.map(this.generateOne)
-
- // return results
- // }
-
- rand(min, max, decimals = 0) {
- if (min > max) {
- throw 'Min cannot be above max!'
- }
-
- if (decimals < 0) {
- throw 'Decimals must be >= 0!'
- }
-
- // debugger
-
- return parseFloat((Math.random() * (max - min) + min).toFixed(decimals))
- }
-
- generateRandomArray(size, valueMin, valueMax, decimalsMin, decimalsMax, unique = false) {
- const list = []
-
- while (list.length < size) {
- const n = this.rand(valueMin, valueMax, this.rand(decimalsMin, decimalsMax))
-
- if (!unique || list.indexOf(n) === -1) {
- list.push(n)
- }
- }
-
- return list
- }
-
- pickOne(list) {
- return list[this.rand(0, list.length - 1)]
- }
-
- pickMany(list, sizeMin, sizeMax, ordered = false) {
- if (sizeMin > list.length || sizeMax > list.length) {
- throw 'min or max cannot be larger than the size of the list!'
- }
-
- if (sizeMin > sizeMax) {
- throw 'min cannot be larger than max!'
- }
-
- const size = this.rand(sizeMin, sizeMax)
-
- list = this.generateRandomArray(size, 0, list.length - 1, 0, 0, true)
- .sort()
- .map(i => list[i])
-
- return ordered ? list : shuffle(list)
- }
-}
-
-const variableGenerator = new VariableGenerator()
-
-module.exports = variableGenerator
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variables.js b/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variables.js
deleted file mode 100644
index 24adfa94e2..0000000000
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/stores/variable-store/variables.js
+++ /dev/null
@@ -1,173 +0,0 @@
-// import VariableGenerator from './variable-generator'
-
-// // import parse from './parser'
-
-// const getKey = (ownerId, varName) => {
-// return `${ownerId}:${varName}`
-// }
-
-// class Variables {
-// static fromObject(o) {
-// this.state.defs = { ...o.defs }
-// this.state.values = { ...o.values }
-// this.state.varNamesByOwnerId = { ...o.varNamesByOwnerId }
-// }
-
-// constructor(state) {
-// this.init(state)
-// }
-
-// init(state) {
-// this.state = state
-// }
-
-// getDefinition(ownerId, varName) {
-// const key = getKey(ownerId, varName)
-// return this.hasDefinition(ownerId, varName) ? this.state.defs[key] : null
-// }
-
-// has(ownerId, varName) {
-// const key = getKey(ownerId, varName)
-// return (
-// typeof this.state.defs[key] !== 'undefined' || typeof this.state.values[key] !== 'undefined'
-// )
-// }
-
-// getValue(ownerId, varName) {
-// if (!this.isValueComputed(ownerId, varName)) {
-// return null
-// }
-
-// return this.state.values[getKey(ownerId, varName)]
-// }
-
-// computeValue(ownerId, varName) {
-// this.setValue(
-// ownerId,
-// varName,
-// VariableGenerator.generateOne(this.getDefinition(ownerId, varName))
-// )
-// }
-
-// getOrSetValue(ownerId, varName) {
-// // console.log('we checking if has value', ownerId, varName, this.isValueComputed(ownerId, varName))
-// if (!this.isValueComputed(ownerId, varName)) {
-// this.computeValue(ownerId, varName)
-// }
-
-// return this.getValue(ownerId, varName)
-// }
-
-// // special case, if varDefintion is not an object we take that as the value too
-// // add(ownerId, varName, varDefintionOrValue) {
-// // // console.log('add', ownerId, varName, varDefintionOrValue)
-// // if (typeof varDefintionOrValue !== 'object') {
-// // const value = varDefintionOrValue
-// // // this acts as the default:
-// // console.log('parsey', value)
-// // const parsedValue = parse(value)
-// // this.setDefinition(ownerId, varName, parsedValue)
-// // this.setValue(ownerId, varName, parsedValue)
-// // } else {
-// // const def = varDefintionOrValue
-// // this.setDefinition(ownerId, varName, def)
-// // }
-
-// // if (!this.state.varNamesByOwnerId[ownerId]) {
-// // this.state.varNamesByOwnerId[ownerId] = {}
-// // }
-// // this.state.varNamesByOwnerId[ownerId][varName] = true
-
-// // this.computeValue(ownerId, varName)
-// // }
-
-// add(ownerId, varName, varDefintion, computeValue) {
-// // console.log('add', ownerId, varName, varDefintionOrValue)
-
-// this.setDefinition(ownerId, varName, varDefintion)
-
-// if (!this.state.varNamesByOwnerId[ownerId]) {
-// this.state.varNamesByOwnerId[ownerId] = {}
-// }
-// this.state.varNamesByOwnerId[ownerId][varName] = true
-
-// if (computeValue) {
-// this.computeValue(ownerId, varName)
-// }
-// }
-
-// addMultiple(ownerId, variableDefinitions = [], computeValue) {
-// variableDefinitions.forEach(v => {
-// this.add(ownerId, v.name, { ...v }, computeValue)
-// })
-// }
-
-// setDefinition(ownerId, varName, definition) {
-// const key = getKey(ownerId, varName)
-// this.state.defs[key] = definition
-// }
-
-// setValue(ownerId, varName, value) {
-// const key = getKey(ownerId, varName)
-// this.state.values[key] = value
-// }
-
-// hasDefinition(ownerId, varName) {
-// const key = getKey(ownerId, varName)
-// return typeof this.state.defs[key] !== 'undefined'
-// }
-
-// isValueComputed(ownerId, varName) {
-// const key = getKey(ownerId, varName)
-// return typeof this.state.values[key] !== 'undefined'
-// }
-
-// clearValue(ownerId, varName) {
-// const key = getKey(ownerId, varName)
-
-// if (this.isComputableVariable(ownerId, varName)) {
-// delete this.state.values[key]
-// } else {
-// // reset to default
-// this.setValue(ownerId, varName, this.getDefinition(ownerId, varName))
-// }
-// }
-
-// clearAllValues(ownerId) {
-// Object.keys(this.state.varNamesByOwnerId[ownerId]).forEach(varName => {
-// this.clearValue(ownerId, varName)
-// })
-// }
-
-// remove(ownerId, varName) {
-// this.clearValue(ownerId, varName)
-// const key = getKey(ownerId, varName)
-
-// delete this.state.defs[key]
-// delete this.state.varNamesByOwnerId[ownerId][varName]
-
-// if (Object.keys(this.state.varNamesByOwnerId[ownerId]).length === 0) {
-// delete this.state.varNamesByOwnerId[ownerId]
-// }
-// }
-
-// removeAll(ownerId) {
-// Object.keys(this.state.varNamesByOwnerId[ownerId]).forEach(varName => {
-// this.remove(ownerId, varName)
-// })
-// }
-
-// toObject() {
-// return {
-// defs: { ...this.state.defs },
-// values: { ...this.state.values },
-// varNamesByOwnerId: { ...this.state.varNamesByOwnerId }
-// }
-// }
-// }
-
-// const variables = new Variables()
-
-// window.__v = variables
-
-// export default variables
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/util/variable-util.js b/packages/app/obojobo-document-engine/src/scripts/viewer/util/variable-util.js
index ce9e65e9cf..019201fb9b 100644
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/util/variable-util.js
+++ b/packages/app/obojobo-document-engine/src/scripts/viewer/util/variable-util.js
@@ -7,6 +7,21 @@ const VariableUtil = {
return state.contexts[context] || null
},
+ hasValue(context, state, ownerId, varName) {
+ const contextState = VariableUtil.getStateForContext(state, context)
+ if (!contextState) return false
+
+ const key = VariableUtil.getKey(ownerId, varName)
+ return typeof contextState.values[key] !== 'undefined'
+ },
+
+ getValue(context, state, ownerId, varName) {
+ const contextState = VariableUtil.getStateForContext(state, context)
+ if (!contextState) return null
+
+ return contextState.values[VariableUtil.getKey(ownerId, varName)]
+ },
+
getOwnerOfVariable(context, state, model, varName) {
const contextState = VariableUtil.getStateForContext(state, context)
if (!contextState) return null
@@ -24,34 +39,16 @@ const VariableUtil = {
return null
},
- // Recursive!
findValueWithModel(context, state, model, varName) {
const contextState = VariableUtil.getStateForContext(state, context)
if (!contextState) return null
const owner = VariableUtil.getOwnerOfVariable(context, state, model, varName)
- if (!owner) {
- return null
- }
+ if (!owner) return null
return VariableUtil.getValue(context, state, owner.get('id'), varName)
},
- hasValue(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return false
-
- const key = VariableUtil.getKey(ownerId, varName)
- return typeof contextState.values[key] !== 'undefined'
- },
-
- getValue(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- return contextState.values[VariableUtil.getKey(ownerId, varName)]
- },
-
getVariableStateSummary(context, state) {
const contextState = VariableUtil.getStateForContext(state, context)
if (!contextState) return null
diff --git a/packages/app/obojobo-document-engine/src/scripts/viewer/util/varible-util.old.js b/packages/app/obojobo-document-engine/src/scripts/viewer/util/varible-util.old.js
deleted file mode 100644
index 566c9bcfaf..0000000000
--- a/packages/app/obojobo-document-engine/src/scripts/viewer/util/varible-util.old.js
+++ /dev/null
@@ -1,171 +0,0 @@
-import Common from 'Common'
-
-const { Dispatcher } = Common.flux
-
-const VariableUtil = {
- getKey(ownerId, varName) {
- return `${ownerId}:${varName}`
- },
-
- clear(id, name, context) {
- return Dispatcher.trigger('variable:clear', {
- value: {
- id,
- name,
- context
- }
- })
- },
-
- clearAll(id, context) {
- return Dispatcher.trigger('variable:clearAll', {
- value: {
- id,
- context
- }
- })
- },
-
- setValue(id, name, value, context) {
- return Dispatcher.trigger('variable:setValue', {
- value: {
- id,
- name,
- value,
- context
- }
- })
- },
-
- regenerateValue(id, name, context) {
- return Dispatcher.trigger('variable:regenerateValue', {
- value: {
- id,
- name,
- context
- }
- })
- },
-
- getStateForContext(state, context) {
- return state.contexts[context] || null
- },
-
- // Non-recursive!
- generateAndGetValue(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- if (!VariableUtil.hasDefinition(context, state, ownerId, varName)) {
- console.log('no def for', ownerId, varName)
- return null
- }
-
- if (!VariableUtil.isValueGenerated(context, state, ownerId, varName)) {
- VariableUtil.regenerateValue(context, ownerId, varName)
- }
-
- return VariableUtil.getValue(context, state, ownerId, varName)
- },
-
- getOwnerOfVariable(context, state, model, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- const ownerId = model.get('id')
-
- if (VariableUtil.hasDefinition(context, state, ownerId, varName)) {
- return model
- }
-
- if (model.parent) {
- return VariableUtil.getOwnerOfVariable(context, state, model.parent, varName)
- }
-
- return null
- },
-
- // Recursive!
- generateAndGetValueForModel(context, state, model, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- const owner = VariableUtil.getOwnerOfVariable(context, state, model, varName)
- console.log('gagvfm', context, state, model, varName, contextState, owner)
- if (!owner) {
- return null
- }
-
- return VariableUtil.generateAndGetValue(context, state, owner.get('id'), varName)
- // const ownerId = model.get('id')
-
- // console.log('gagvfm', ownerId, varName, VariableUtil.hasDefinition(state, ownerId, varName))
-
- // if (VariableUtil.hasDefinition(state, ownerId, varName)) {
- // return VariableUtil.generateAndGetValue(state, ownerId, varName)
- // }
-
- // if (model.parent) {
- // return VariableUtil.generateAndGetValueForModel(state, model.parent, varName)
- // }
-
- // return null
- },
-
- getDefinition(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- const key = VariableUtil.getKey(ownerId, varName)
- return VariableUtil.hasDefinition(context, state, ownerId, varName)
- ? contextState.defs[key]
- : null
- },
-
- has(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return false
-
- const key = VariableUtil.getKey(ownerId, varName)
- return (
- typeof contextState.defs[key] !== 'undefined' ||
- typeof contextState.values[key] !== 'undefined'
- )
- },
-
- getValue(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- if (!VariableUtil.isValueGenerated(context, state, ownerId, varName)) {
- return null
- }
-
- return contextState.values[VariableUtil.getKey(ownerId, varName)]
- },
-
- hasDefinition(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return false
-
- const key = VariableUtil.getKey(ownerId, varName)
- return typeof contextState.defs[key] !== 'undefined'
- },
-
- isValueGenerated(context, state, ownerId, varName) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- const key = VariableUtil.getKey(ownerId, varName)
- return typeof contextState.values[key] !== 'undefined'
- },
-
- getVariableStateSummary(context, state) {
- const contextState = VariableUtil.getStateForContext(state, context)
- if (!contextState) return null
-
- return { ...contextState.values }
- }
-}
-
-export default VariableUtil
diff --git a/packages/app/obojobo-document-json-parser/src/parsers/code-node-parser.js b/packages/app/obojobo-document-json-parser/src/parsers/code-node-parser.js
index e13d473d32..bbfd32e0a2 100644
--- a/packages/app/obojobo-document-json-parser/src/parsers/code-node-parser.js
+++ b/packages/app/obojobo-document-json-parser/src/parsers/code-node-parser.js
@@ -11,7 +11,7 @@ const codeNodeParser = node => {
const varsXML = processVars(node.content.variables)
const objectivesXML = processObjectives(node.content.objectives)
- return `` + textGroupXML + triggersXML + objectivesXML + varsXML ``
+ return `` + textGroupXML + triggersXML + objectivesXML + varsXML + ``
}
module.exports = codeNodeParser
diff --git a/packages/app/obojobo-document-xml-parser/__tests__/src/variables-parser.test.js b/packages/app/obojobo-document-xml-parser/__tests__/src/variables-parser.test.js
new file mode 100644
index 0000000000..ab4fd97c63
--- /dev/null
+++ b/packages/app/obojobo-document-xml-parser/__tests__/src/variables-parser.test.js
@@ -0,0 +1,26 @@
+const variableParser = require('../../src/variables-parser')
+
+describe('Variable parser', () => {
+ // there are two other internal methods that we can't test independently
+ // it is what it is
+ test('parses variables from an element', () => {
+ // we happen to know the typical structure so this is a little magical
+ const mockElement = {
+ elements: [
+ {
+ name: 'var',
+ attributes: {
+ attr1: 'wat',
+ attr2: 'destiny'
+ }
+ }
+ ]
+ }
+ expect(variableParser(mockElement)).toEqual([
+ {
+ attr1: 'wat',
+ attr2: 'destiny'
+ }
+ ])
+ })
+})
diff --git a/packages/app/obojobo-express/__tests__/models/draft_node.test.js b/packages/app/obojobo-express/__tests__/models/draft_node.test.js
index 8e952ff48c..d6504b63b1 100644
--- a/packages/app/obojobo-express/__tests__/models/draft_node.test.js
+++ b/packages/app/obojobo-express/__tests__/models/draft_node.test.js
@@ -1,8 +1,14 @@
jest.mock('../../server/db')
+jest.mock('../../server/models/variable-generator')
+
import Draft from '../../server/models/draft'
import DraftNode from '../../server/models/draft_node'
+import VariableGenerator from '../../server/models/variable-generator'
+
+import mockConsole from 'jest-mock-console'
+
const mockRawDraft = {
id: 'whatever',
version: 9,
@@ -36,10 +42,19 @@ const mockRawDraft = {
}
describe('models draft', () => {
+ let restoreConsole
+
beforeAll(() => {})
afterAll(() => {})
- beforeEach(() => {})
- afterEach(() => {})
+
+ beforeEach(() => {
+ jest.resetAllMocks()
+ restoreConsole = mockConsole('error')
+ })
+
+ afterEach(() => {
+ restoreConsole()
+ })
test('constructor initializes expected properties', () => {
const draftTree = {}
@@ -113,13 +128,17 @@ describe('models draft', () => {
const d = new DraftNode({}, mockRawDraft.content, jest.fn())
const eventFn = jest.fn()
+ // draft nodes register internal:generateVariables on their own
+ expect(d._listeners.size).toBe(1)
+ expect(d._listeners.has('internal:generateVariables')).toBe(true)
+
d.registerEvents({ test: eventFn, test2: eventFn })
- expect(d._listeners.size).toBe(2)
+ expect(d._listeners.size).toBe(3)
d.registerEvents({ test: eventFn, test3: eventFn })
- expect(d._listeners.size).toBe(3)
+ expect(d._listeners.size).toBe(4)
})
test('contains finds child nodes', () => {
@@ -194,6 +213,25 @@ describe('models draft', () => {
expect(eventFn).toHaveBeenCalled()
})
+ // since 'internal:generateVariables' is registered in the constructor this should never happen
+ test('yell does nothing if no listeners are registered', () => {
+ const draftTree = {}
+
+ const eventFn = jest.fn().mockReturnValueOnce(false)
+
+ const d = new DraftNode(draftTree, mockRawDraft.content)
+
+ expect(d._listeners.size).toBe(1)
+ d.registerEvents({ test: eventFn })
+ expect(d._listeners.size).toBe(2)
+
+ // the only way for the listeners list to be falsy is to get rid of it entirely
+ delete d._listeners
+
+ d.yell('test')
+ expect(eventFn).not.toHaveBeenCalled()
+ })
+
test('toObject converts itself to an object', () => {
const draftTree = {}
const initFn = jest.fn()
@@ -214,4 +252,78 @@ describe('models draft', () => {
})
)
})
+
+ test('does not call VariableGenerator functions when reacting to internal:generateVariables yell for no variables', () => {
+ const draftTree = {}
+ const d = new DraftNode(draftTree, mockRawDraft.content)
+
+ const mockVariableValues = []
+
+ d.yell('internal:generateVariables', ({}, {}, mockVariableValues))
+
+ expect(VariableGenerator.generateOne).not.toHaveBeenCalled()
+ })
+
+ test('calls VariableGenerator functions when reacting to internal:generateVariables yell, no errors', () => {
+ const draftTree = {}
+ const thisMockRawDraft = { ...mockRawDraft }
+
+ VariableGenerator.generateOne = jest
+ .fn()
+ .mockReturnValueOnce('value1')
+ .mockReturnValueOnce('value2')
+
+ // variables would have more to them than this, but this component doesn't really need to care about it
+ thisMockRawDraft.content.content.variables = [{ name: 'var1' }, { name: 'var2' }]
+ const d = new DraftNode(draftTree, thisMockRawDraft.content)
+
+ const mockVariableValues = []
+
+ d.yell('internal:generateVariables', {}, {}, mockVariableValues)
+
+ expect(VariableGenerator.generateOne).toHaveBeenCalledTimes(2)
+ expect(VariableGenerator.generateOne.mock.calls[0][0]).toEqual({ name: 'var1' })
+ expect(VariableGenerator.generateOne.mock.calls[1][0]).toEqual({ name: 'var2' })
+
+ expect(console.error).not.toHaveBeenCalled()
+
+ expect(mockVariableValues).toEqual([
+ { id: mockRawDraft.content.id + ':var1', value: 'value1' },
+ { id: mockRawDraft.content.id + ':var2', value: 'value2' }
+ ])
+ })
+
+ test('calls VariableGenerator functions when reacting to internal:generateVariables yell, with errors', () => {
+ const draftTree = {}
+ const thisMockRawDraft = { ...mockRawDraft }
+
+ VariableGenerator.generateOne = jest
+ .fn()
+ .mockImplementationOnce(() => {
+ throw 'mock error 1'
+ })
+ .mockImplementationOnce(() => {
+ throw 'mock error 2'
+ })
+
+ thisMockRawDraft.content.content.variables = [{ name: 'var1' }, { name: 'var2' }]
+ const d = new DraftNode(draftTree, thisMockRawDraft.content)
+
+ const mockVariableValues = []
+
+ d.yell('internal:generateVariables', {}, {}, mockVariableValues)
+
+ expect(VariableGenerator.generateOne).toHaveBeenCalledTimes(2)
+
+ expect(console.error).toHaveBeenCalledTimes(2)
+ expect(console.error.mock.calls[0][0]).toBe('Variable generation error:')
+ expect(console.error.mock.calls[0][1]).toBe('mock error 1')
+ expect(console.error.mock.calls[1][0]).toBe('Variable generation error:')
+ expect(console.error.mock.calls[1][1]).toBe('mock error 2')
+
+ expect(mockVariableValues).toEqual([
+ { id: mockRawDraft.content.id + ':var1', value: '' },
+ { id: mockRawDraft.content.id + ':var2', value: '' }
+ ])
+ })
})
diff --git a/packages/app/obojobo-express/__tests__/models/variable-generator.test.js b/packages/app/obojobo-express/__tests__/models/variable-generator.test.js
new file mode 100644
index 0000000000..9d50130ab0
--- /dev/null
+++ b/packages/app/obojobo-express/__tests__/models/variable-generator.test.js
@@ -0,0 +1,414 @@
+jest.mock('obojobo-document-engine/src/scripts/common/util/shuffle')
+
+const shuffle = require('obojobo-document-engine/src/scripts/common/util/shuffle')
+
+// ideally we could import the list of constants defined in the document engine
+// unfortunately they're exported in a way Node doesn't like because JavaScript is stupid
+// revisit this when JavaScript is less stupid
+const STATIC_VALUE = 'static-value'
+const RANDOM_NUMBER = 'random-number'
+const STATIC_LIST = 'static-list'
+const RANDOM_LIST = 'random-list'
+const RANDOM_SEQUENCE = 'random-sequence'
+const PICK_ONE = 'pick-one'
+const PICK_LIST = 'pick-list'
+
+const VariableGenerator = require('../../server/models/variable-generator')
+
+// there are other methods used by this class which we can't test directly
+// we'll have to make due by checking their outputs to make sure everything works
+describe('VariableGenerator', () => {
+ // we need Math.random to act predictably if we're to have any hope of testing some of these
+ // const realMath = global.Math
+ // beforeAll(() => {
+ // global.Math.random = jest.fn()
+ // })
+ // beforeEach(() => {
+ // global.Math.random.mockReset()
+ // })
+ // afterAll(() => {
+ // global.Math = realMath
+ // })
+
+ beforeEach(() => {
+ jest.restoreAllMocks()
+ })
+
+ // technically we should also do these same tests with decimalPlaces
+ // but it'll be the same as min/max, so this should be fine for now
+ // also technically, we should also do these same tests for other range-based methods
+ // but again, it'll be the same as getRandomList, so this should be fine for now
+ test('getRandomList throws when min/max size string is not defined', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({})
+ }).toThrow("Range 'undefined' is invalid!")
+ })
+
+ test('getRandomList throws when min/max size string is null', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: null
+ })
+ }).toThrow("Range 'null' is invalid!")
+ })
+
+ test('getRandomList throws when min/max size string an empty string', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: ''
+ })
+ }).toThrow("Range '' has non-numeric values!")
+ })
+
+ test('getRandomList throws when min/max size string is formatted to be non-inclusive', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '(1,1)'
+ })
+ }).toThrow("Range '(1,1)' must be inclusive!")
+ })
+
+ test('getRandomList throws when min/max size string has a higher value before a lower value', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '[2,1]'
+ })
+ }).toThrow("Range '[2,1]' is inverted!")
+ })
+
+ test('getRandomList throws when min/max size string has a non-numeric min or max values', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '["invalid",1]'
+ })
+ }).toThrow('Range \'["invalid",1]\' has non-numeric values!')
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '[1,"invalid"]'
+ })
+ }).toThrow('Range \'[1,"invalid"]\' has non-numeric values!')
+ })
+
+ test('getRandomList throws when decimalPlaces contains negative numbers', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '[1,1]',
+ value: '[1,2]',
+ decimalPlaces: '[-1,0]'
+ })
+ }).toThrow("Range '[-1,0]' must be positive!")
+ })
+
+ test('getRandomList throws when decimalPlaces contains non-integers', () => {
+ expect(() => {
+ VariableGenerator.getRandomList({
+ size: '[1,1]',
+ value: '[1,2]',
+ decimalPlaces: '[0,1.2]'
+ })
+ }).toThrow("Range '[0,1.2]' must be int values only!")
+ })
+
+ // this is kind of a lie - we can't really test that everything works with randomly selected values in ranges
+ // we'll mostly be testing that things are being called correctly
+ test('getRandomList correctly generates a random list - non-unique values allowed', () => {
+ // we'll be testing this function on its own later
+ // being able to circumvent it allows us to test everything else more easily
+ jest
+ .spyOn(VariableGenerator, 'rand')
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(2)
+
+ const varDef = {
+ size: '[1,2]',
+ value: '[1,10]',
+ decimalPlaces: '[1,2]',
+ unique: false
+ }
+ const randomList = VariableGenerator.getRandomList(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(5)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([1, 2]) // size
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([1, 2]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([1, 10, 1]) // first value - third arg is decimal places
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([1, 2]) // decimal places again
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([1, 10, 1]) // second value
+ expect(randomList).toEqual([2, 2])
+ })
+
+ test('getRandomList correctly generates a random list - only unique values allowed', () => {
+ // we'll be testing this function on its own later
+ // being able to circumvent it allows us to test everything else more easily
+ jest
+ .spyOn(VariableGenerator, 'rand')
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(3)
+
+ const varDef = {
+ size: '[1,2]',
+ value: '[1,10]',
+ decimalPlaces: '[1,2]',
+ unique: true
+ }
+ const randomList = VariableGenerator.getRandomList(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(7)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([1, 2]) // size
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([1, 2]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([1, 10, 1]) // first value - third arg is decimal places
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([1, 2]) // decimal places again
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([1, 10, 1]) // second value
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([1, 2]) // decimal places again... again
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([1, 10, 1]) // second value, second attempt
+ expect(randomList).toEqual([2, 3])
+ })
+
+ test('getRandomSequence throws when given an invalid series type', () => {
+ const varDef = {
+ start: 1,
+ step: 1,
+ size: '[1,1]',
+ seriesType: 'invalid-series-type'
+ }
+ expect(() => {
+ VariableGenerator.getRandomSequence(varDef)
+ }).toThrow('Invalid sequence seriesType!')
+ })
+
+ test('getRandomSequence correctly generates a sequence - arithmetic', () => {
+ // luckily the only thing randomly selected here is the list size
+ jest.spyOn(VariableGenerator, 'rand').mockReturnValueOnce(3)
+
+ const varDef = {
+ start: 10,
+ step: 2,
+ size: '[2,4]',
+ seriesType: 'arithmetic'
+ }
+ const randomSequence = VariableGenerator.getRandomSequence(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(1)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([2, 4]) // size
+ expect(randomSequence).toEqual([10, 12, 14])
+ })
+
+ test('getRandomSequence correctly generates a sequence - geometric', () => {
+ // luckily the only thing randomly selected here is the list size
+ jest.spyOn(VariableGenerator, 'rand').mockReturnValueOnce(3)
+
+ const varDef = {
+ start: 10,
+ step: 2,
+ size: '[2,4]',
+ seriesType: 'geometric'
+ }
+ const randomSequence = VariableGenerator.getRandomSequence(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(1)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([2, 4]) // size
+ expect(randomSequence).toEqual([10, 20, 40])
+ })
+
+ test('getRandomSequence correctly forces start and step to 1 if they are unset', () => {
+ // luckily the only thing randomly selected here is the list size
+ jest.spyOn(VariableGenerator, 'rand').mockReturnValueOnce(3)
+
+ const varDef = {
+ size: '[2,4]',
+ seriesType: 'arithmetic'
+ }
+ const randomSequence = VariableGenerator.getRandomSequence(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(1)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([2, 4]) // size
+ expect(randomSequence).toEqual([1, 2, 3])
+ })
+
+ // we're mocking the function that actually generates the random number here, so this is a lie
+ // but we're mostly just making sure the correct arguments are being sent to what we're mocking anyway
+ test('getRandomNumber gets a random number', () => {
+ jest
+ .spyOn(VariableGenerator, 'rand')
+ .mockReturnValueOnce(0)
+ .mockReturnValueOnce(3)
+
+ const varDef = {
+ value: '[2,4]',
+ decimalPlaces: '[0,1]'
+ }
+ const randomNumber = VariableGenerator.getRandomNumber(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(2)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([0, 1]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([2, 4, 0]) // value
+ expect(randomNumber).toBe(3)
+ })
+
+ // see above re: randomness being a lie
+ test('getPickOne returns a random item from a list', () => {
+ jest.spyOn(VariableGenerator, 'rand').mockReturnValueOnce(2)
+
+ const varDef = {
+ value: '2,4,6,8,10,12,420'
+ }
+ const randomNumber = VariableGenerator.getPickOne(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(1)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([0, 6])
+ expect(randomNumber).toBe('6')
+ })
+
+ test('getPickList throws when choose contains zero', () => {
+ jest.spyOn(VariableGenerator, 'rand')
+
+ const varDef = {
+ choose: '[0,0]',
+ value: '1,2'
+ }
+ expect(() => {
+ VariableGenerator.getPickList(varDef)
+ }).toThrow("Range '[0,0]' values must be non-zero!")
+ expect(VariableGenerator.rand).not.toHaveBeenCalled()
+ })
+
+ test('getPickList throws when chooseMin is higher than the list size', () => {
+ jest.spyOn(VariableGenerator, 'rand')
+
+ const varDef = {
+ choose: '[3,3]',
+ value: '1,2'
+ }
+ expect(() => {
+ VariableGenerator.getPickList(varDef)
+ }).toThrow('min or max cannot be larger than the size of the list!')
+ expect(VariableGenerator.rand).not.toHaveBeenCalled()
+ })
+
+ test('getPickList throws when chooseMax is higher than the list size', () => {
+ jest.spyOn(VariableGenerator, 'rand')
+
+ const varDef = {
+ choose: '[2,3]',
+ value: '1,2'
+ }
+ expect(() => {
+ VariableGenerator.getPickList(varDef)
+ }).toThrow('min or max cannot be larger than the size of the list!')
+ expect(VariableGenerator.rand).not.toHaveBeenCalled()
+ })
+
+ test('getPickList returns a random list in order', () => {
+ jest
+ .spyOn(VariableGenerator, 'rand')
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(0)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(0)
+ .mockReturnValueOnce(4)
+
+ const varDef = {
+ choose: '[2,4]',
+ value: '2,4,6,8,10,12,420',
+ ordered: true
+ }
+ const pickList = VariableGenerator.getPickList(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(5)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([2, 4]) // list size
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([0, 0]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([0, 6, 0]) // first list index
+ expect(VariableGenerator.rand.mock.calls[3]).toEqual([0, 0]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[4]).toEqual([0, 6, 0]) // second list index
+ expect(pickList).toEqual(['4', '10'])
+ })
+
+ test('getPickList returns a random list, unordered', () => {
+ // again - we're not testing that the list is actually shuffled, just that this is being called
+ shuffle.mockReturnValueOnce(['10', '4'])
+
+ jest
+ .spyOn(VariableGenerator, 'rand')
+ .mockReturnValueOnce(2)
+ .mockReturnValueOnce(0)
+ .mockReturnValueOnce(1)
+ .mockReturnValueOnce(0)
+ .mockReturnValueOnce(4)
+
+ const varDef = {
+ choose: '[2,4]',
+ value: '2,4,6,8,10,12,420'
+ }
+ const pickList = VariableGenerator.getPickList(varDef)
+ expect(VariableGenerator.rand).toHaveBeenCalledTimes(5)
+ expect(VariableGenerator.rand.mock.calls[0]).toEqual([2, 4]) // list size
+ expect(VariableGenerator.rand.mock.calls[1]).toEqual([0, 0]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[2]).toEqual([0, 6, 0]) // first list index
+ expect(VariableGenerator.rand.mock.calls[3]).toEqual([0, 0]) // decimal places
+ expect(VariableGenerator.rand.mock.calls[4]).toEqual([0, 6, 0]) // second list index
+
+ expect(shuffle).toHaveBeenCalledTimes(1)
+ expect(shuffle).toHaveBeenCalledWith(['4', '10'])
+
+ expect(pickList).toEqual(['10', '4'])
+ })
+
+ test('rand throws if the provided min is larger than the provided max', () => {
+ expect(() => {
+ VariableGenerator.rand(10, 1)
+ }).toThrow('Min cannot be above max!')
+ })
+
+ test('rand throws if the provided number of decimals is lower than 0', () => {
+ expect(() => {
+ VariableGenerator.rand(1, 1, -1)
+ }).toThrow('Decimals must be >= 0!')
+ })
+
+ test('generateOne throws if given an invalid variable type', () => {
+ expect(() => {
+ VariableGenerator.generateOne({ type: 'invalid-type ' })
+ }).toThrow('Unexpected type!')
+ })
+
+ test.each`
+ variableType | targetFunction
+ ${RANDOM_LIST} | ${'getRandomList'}
+ ${RANDOM_SEQUENCE} | ${'getRandomSequence'}
+ ${RANDOM_NUMBER} | ${'getRandomNumber'}
+ ${PICK_ONE} | ${'getPickOne'}
+ ${PICK_LIST} | ${'getPickList'}
+ ${STATIC_VALUE} | ${'var-val'}
+ ${STATIC_LIST} | ${'var-val'}
+ `(
+ "generateOne calls internal function $targetFunction when variable type is '$variableType'",
+ ({ variableType, targetFunction }) => {
+ const allFunctions = [
+ 'getRandomList',
+ 'getRandomSequence',
+ 'getRandomNumber',
+ 'getPickOne',
+ 'getPickList'
+ ]
+ // mock all of the functions downstream of generateOne
+ // have them return their own name just so we can make sure they're being called
+ // actual implementations of each don't matter here, we just want to check what's called
+ allFunctions.forEach(fName => {
+ jest.spyOn(VariableGenerator, fName).mockReturnValue(fName)
+ })
+
+ const result = VariableGenerator.generateOne({ type: variableType, value: 'var-val' })
+ allFunctions.forEach(fName => {
+ if (fName === targetFunction) {
+ expect(VariableGenerator[fName]).toHaveBeenCalledTimes(1)
+ } else {
+ expect(VariableGenerator[fName]).not.toHaveBeenCalled()
+ }
+ // reset any calls that might have been made to this function
+ VariableGenerator[fName].mockReset()
+ })
+
+ // we're kind of overloading targetFunction to equal the return value for the static variable types
+ expect(result).toBe(targetFunction)
+ }
+ )
+})
diff --git a/packages/app/obojobo-express/__tests__/models/visit.test.js b/packages/app/obojobo-express/__tests__/models/visit.test.js
index 03b049a371..47ed2f7ed6 100644
--- a/packages/app/obojobo-express/__tests__/models/visit.test.js
+++ b/packages/app/obojobo-express/__tests__/models/visit.test.js
@@ -221,4 +221,32 @@ describe('Visit Model', () => {
})
})
})
+
+ test('updateState updates with expected values', () => {
+ db.none.mockResolvedValueOnce()
+
+ // this one is an instant method rather than a static one, we'll need to create an instance
+ // obviously more props than this on a visit, but these are the only two we care about for this test
+ const thisVisit = new Visit({
+ id: 'mock-visit',
+ state: {}
+ })
+
+ const newState = {
+ key: 'val'
+ }
+
+ thisVisit.updateState(newState)
+
+ const queryString = `
+ UPDATE visits
+ SET state=$[state]
+ WHERE id=$[visitId]
+ `
+
+ expect(db.none).toHaveBeenCalledWith(queryString, {
+ visitId: 'mock-visit',
+ state: newState
+ })
+ })
})
diff --git a/packages/app/obojobo-express/__tests__/routes/api/visits.test.js b/packages/app/obojobo-express/__tests__/routes/api/visits.test.js
index 9e85d9565d..24c3f1ae0d 100644
--- a/packages/app/obojobo-express/__tests__/routes/api/visits.test.js
+++ b/packages/app/obojobo-express/__tests__/routes/api/visits.test.js
@@ -49,6 +49,13 @@ describe('api visits route', () => {
let mockSession
let app
+ const standardYellMock = () => {
+ return jest
+ .fn()
+ .mockResolvedValueOnce({ document: 'mock-document' })
+ .mockResolvedValueOnce()
+ }
+
beforeAll(() => {})
afterAll(() => {})
beforeEach(() => {
@@ -61,7 +68,9 @@ describe('api visits route', () => {
id: validUUID(),
is_preview: false,
draft_content_id: validUUID(),
- resource_link_id: '12345'
+ resource_link_id: '12345',
+ updateState: jest.fn(),
+ state: {}
}
VisitModel.fetchById.mockResolvedValue(mockCurrentVisit)
@@ -212,7 +221,10 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99 }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce(),
+ yell: jest
+ .fn()
+ .mockResolvedValueOnce()
+ .mockResolvedValueOnce(),
contentId: validUUID()
}
return request(app)
@@ -232,7 +244,10 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99 }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce(),
+ yell: jest
+ .fn()
+ .mockResolvedValueOnce()
+ .mockResolvedValueOnce(),
contentId: 'some-invalid-content-id'
}
@@ -261,7 +276,7 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99, hasPermission: perm => perm === 'canViewEditor' }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
return request(app)
@@ -278,6 +293,7 @@ describe('api visits route', () => {
"lti": Object {
"lisOutcomeServiceUrl": null,
},
+ "variables": Array [],
"visitId": "00000000-0000-0000-0000-000000000000",
}
`)
@@ -302,7 +318,7 @@ describe('api visits route', () => {
viewerState.get.mockResolvedValueOnce('view state')
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
return request(app)
@@ -333,7 +349,7 @@ describe('api visits route', () => {
viewerState.get.mockResolvedValueOnce('view state')
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
return request(app)
@@ -355,7 +371,7 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99 }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
return request(app)
@@ -373,8 +389,8 @@ describe('api visits route', () => {
})
})
- test('/start yells internal:startVisit and respond with success', () => {
- expect.assertions(4)
+ test('/start yells internal:startVisit and internal:generateVariables and respond with success', () => {
+ expect.assertions(5)
// resolve ltiLaunch lookup
const launch = {
reqVars: {
@@ -389,7 +405,7 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99 }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
return request(app)
@@ -398,12 +414,13 @@ describe('api visits route', () => {
.then(response => {
expect(response.header['content-type']).toContain('application/json')
expect(response.statusCode).toBe(200)
- expect(mockCurrentDocument.yell).toHaveBeenCalledTimes(1)
+ expect(mockCurrentDocument.yell).toHaveBeenCalledTimes(2)
expect(mockCurrentDocument.yell.mock.calls[0][0]).toBe('internal:startVisit')
+ expect(mockCurrentDocument.yell.mock.calls[1][0]).toBe('internal:generateVariables')
})
})
- test('visit:start event created', () => {
+ test('visit:start event created, no variables', () => {
expect.assertions(3)
// resolve ltiLaunch lookup
const launch = {
@@ -419,7 +436,7 @@ describe('api visits route', () => {
mockCurrentUser = { id: 99 }
mockCurrentDocument = {
draftId: validUUID(),
- yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
+ yell: standardYellMock(),
contentId: validUUID()
}
@@ -435,11 +452,128 @@ describe('api visits route', () => {
actorTime: '2016-09-22T16:57:14.500Z',
draftId: validUUID(),
contentId: validUUID(),
- eventVersion: '1.0.0',
+ eventVersion: '1.1.0',
+ ip: '::ffff:127.0.0.1',
+ isPreview: false,
+ metadata: {},
+ payload: {
+ variables: [],
+ visitId: validUUID()
+ },
+ userId: 99,
+ visitId: validUUID()
+ })
+ })
+ })
+
+ test('visit:start event created, new variables', () => {
+ expect.assertions(5)
+ // resolve ltiLaunch lookup
+ const launch = {
+ reqVars: {
+ lis_outcome_service_url: 'howtune.com'
+ }
+ }
+ ltiUtil.retrieveLtiLaunch.mockResolvedValueOnce(launch)
+
+ // resolve viewerState.get
+ viewerState.get.mockResolvedValueOnce('view state')
+
+ const mockVariables = [{ key: 'val1' }, { key: 'val2' }]
+
+ mockCurrentUser = { id: 99 }
+ mockCurrentDocument = {
+ draftId: validUUID(),
+ yell: jest
+ .fn()
+ .mockResolvedValueOnce({ document: 'mock-document' })
+ .mockImplementationOnce((call, req, res, variableListRef) => {
+ variableListRef.push(...mockVariables)
+ return Promise.resolve()
+ }),
+ contentId: validUUID()
+ }
+
+ return request(app)
+ .post('/api/start')
+ .send({ visitId: validUUID() })
+ .then(response => {
+ expect(response.header['content-type']).toContain('application/json')
+ expect(response.statusCode).toBe(200)
+
+ expect(mockCurrentVisit.updateState).toHaveBeenCalledTimes(1)
+ expect(mockCurrentVisit.updateState).toHaveBeenCalledWith({ variables: mockVariables })
+
+ expect(insertEvent).toBeCalledWith({
+ action: 'visit:start',
+ actorTime: '2016-09-22T16:57:14.500Z',
+ draftId: validUUID(),
+ contentId: validUUID(),
+ eventVersion: '1.1.0',
+ ip: '::ffff:127.0.0.1',
+ isPreview: false,
+ metadata: {},
+ // payload: { visitId: req.currentVisit.id, variables: variableValues },
+ payload: {
+ variables: mockVariables,
+ visitId: validUUID()
+ },
+ userId: 99,
+ visitId: validUUID()
+ })
+ })
+ })
+
+ test('visit:start event created, existing variables', () => {
+ expect.assertions(4)
+ // resolve ltiLaunch lookup
+ const launch = {
+ reqVars: {
+ lis_outcome_service_url: 'howtune.com'
+ }
+ }
+ ltiUtil.retrieveLtiLaunch.mockResolvedValueOnce(launch)
+
+ // resolve viewerState.get
+ viewerState.get.mockResolvedValueOnce('view state')
+
+ const mockVariables = [{ key: 'val1' }, { key: 'val2' }]
+
+ mockCurrentVisit.state.variables = mockVariables
+
+ mockCurrentUser = { id: 99 }
+ mockCurrentDocument = {
+ draftId: validUUID(),
+ yell: jest
+ .fn()
+ .mockResolvedValueOnce({ document: 'mock-document' })
+ .mockResolvedValueOnce(),
+ contentId: validUUID()
+ }
+
+ return request(app)
+ .post('/api/start')
+ .send({ visitId: validUUID() })
+ .then(response => {
+ expect(response.header['content-type']).toContain('application/json')
+ expect(response.statusCode).toBe(200)
+
+ expect(mockCurrentVisit.updateState).toHaveBeenCalledTimes(0)
+
+ expect(insertEvent).toBeCalledWith({
+ action: 'visit:start',
+ actorTime: '2016-09-22T16:57:14.500Z',
+ draftId: validUUID(),
+ contentId: validUUID(),
+ eventVersion: '1.1.0',
ip: '::ffff:127.0.0.1',
isPreview: false,
metadata: {},
- payload: { visitId: validUUID() },
+ // payload: { visitId: req.currentVisit.id, variables: variableValues },
+ payload: {
+ variables: mockVariables,
+ visitId: validUUID()
+ },
userId: 99,
visitId: validUUID()
})
diff --git a/packages/app/obojobo-express/server/models/draft_node.js b/packages/app/obojobo-express/server/models/draft_node.js
index feb72ba129..a48e408418 100644
--- a/packages/app/obojobo-express/server/models/draft_node.js
+++ b/packages/app/obojobo-express/server/models/draft_node.js
@@ -39,7 +39,7 @@ class DraftNode {
})
} catch (e) {
//eslint-disable-next-line no-console
- console.error('Variable generation error: ', e)
+ console.error('Variable generation error:', e)
variableValues.push({
id: this.node.id + ':' + v.name,
value: ''
@@ -71,8 +71,6 @@ class DraftNode {
yell(event) {
let promises = []
- // console.log(this.node, 'yell', this._listeners)
-
if (this._listeners) {
const eventListener = this._listeners.get(event)
if (eventListener) {
diff --git a/packages/app/obojobo-express/server/models/variable-generator.js b/packages/app/obojobo-express/server/models/variable-generator.js
index 4e8d3c1ba7..2443f5c1f3 100644
--- a/packages/app/obojobo-express/server/models/variable-generator.js
+++ b/packages/app/obojobo-express/server/models/variable-generator.js
@@ -1,3 +1,16 @@
+// ideally we could import the list of constants defined in the document engine
+// unfortunately they're exported in a way Node doesn't like because JavaScript is stupid
+// revisit this when JavaScript is less stupid
+const STATIC_VALUE = 'static-value'
+const RANDOM_NUMBER = 'random-number'
+const STATIC_LIST = 'static-list'
+const RANDOM_LIST = 'random-list'
+const RANDOM_SEQUENCE = 'random-sequence'
+const PICK_ONE = 'pick-one'
+const PICK_LIST = 'pick-list'
+
+const shuffle = require('obojobo-document-engine/src/scripts/common/util/shuffle')
+
const getParsedRange = range => {
if (typeof range === 'undefined' || range === null) return null
@@ -15,8 +28,6 @@ const getParsedRange = range => {
}
const getParsedRangeFromSingleValue = value => {
- if (typeof value === 'undefined' || value === null) return null
-
return {
min: value,
isMinInclusive: true,
@@ -25,105 +36,28 @@ const getParsedRangeFromSingleValue = value => {
}
}
-// replaceDict is an object of possibile replacements for `value`.
-// For example, if replaceDict = { '$highest_score':100 } and `value` is '$highest_score' then
-// `value` will be replaced with 100.
-// nonParsedValueOrValues is a value or an array of values that won't be parsed by parseFloat.
-// If `value` is one of these values then `value` is not parsed and simply returned.
-// For example, if nonParsedValueOrValues is `[null, undefined]` and `value` is null
-// then null is returned.
-const tryGetParsedFloat = (value, replaceDict = {}, nonParsedValueOrValues = []) => {
- let nonParsedValues
-
- if (!(nonParsedValueOrValues instanceof Array)) {
- nonParsedValues = [nonParsedValueOrValues]
- } else {
- nonParsedValues = nonParsedValueOrValues
- }
-
- for (const placeholder in replaceDict) {
- if (value === placeholder) {
- value = replaceDict[placeholder]
- break
- }
- }
-
- // If the value is an allowed non-numeric value then we don't parse it
- // and simply return it as is
- if (nonParsedValues.indexOf(value) > -1) return value
-
- const parsedValue = parseFloat(value)
-
- if (!Number.isFinite(parsedValue) && parsedValue !== Infinity && parsedValue !== -Infinity) {
- throw new Error(`Unable to parse "${value}": Got "${parsedValue}" - Unsure how to proceed`)
- }
-
- return parsedValue
-}
-
-const isValueInRange = (value, range, replaceDict) => {
- // By definition a value is not inside a null range
- if (range === null) return false
-
- let isMinRequirementMet, isMaxRequirementMet
-
- const min = tryGetParsedFloat(range.min, replaceDict)
- const max = tryGetParsedFloat(range.max, replaceDict)
-
- if (range.isMinInclusive) {
- isMinRequirementMet = value >= min
- } else {
- isMinRequirementMet = value > min
- }
-
- if (range.isMaxInclusive) {
- isMaxRequirementMet = value <= max
- } else {
- isMaxRequirementMet = value < max
- }
-
- return isMinRequirementMet && isMaxRequirementMet
-}
-
-const shuffle = array => {
- let m = array.length
- let t
- let i
-
- // While there remain elements to shuffle…
- while (m) {
- // Pick a remaining element…
- i = Math.floor(Math.random() * m--)
-
- // And swap it with the current element.
- t = array[m]
- array[m] = array[i]
- array[i] = t
- }
-
- return array
-}
-
-module.exports = shuffle
-
const parse = s => s.split(',')
const getRange = rangeString => {
const range = getParsedRange(rangeString)
- if (!range.isMinInclusive || !range.isMaxInclusive) {
- throw 'Range ' + rangeString + ' must be inclusive!'
+ if (!range) {
+ throw `Range '${rangeString}' is invalid!`
}
- if (range.min > range.max) {
- throw 'Range ' + rangeString + ' is inverted'
+ if (!range.isMinInclusive || !range.isMaxInclusive) {
+ throw `Range '${rangeString}' must be inclusive!`
}
const min = parseFloat(range.min)
const max = parseFloat(range.max)
if (!Number.isFinite(min) || !Number.isFinite(max)) {
- throw 'Range ' + rangeString + ' has non-numeric values'
+ throw `Range '${rangeString}' has non-numeric values!`
+ }
+
+ if (min > max) {
+ throw `Range '${rangeString}' is inverted!`
}
return [min, max]
@@ -132,12 +66,13 @@ const getRange = rangeString => {
const getPosIntRange = rangeString => {
const [min, max] = getRange(rangeString)
+ // max < 0 should not be reachable since getRange will throw before it gets here
if (min < 0 || max < 0) {
- throw 'Range ' + rangeString + ' must be positive!'
+ throw `Range '${rangeString}' must be positive!`
}
if (parseInt(min, 10) !== min || parseInt(max, 10) !== max) {
- throw 'Range ' + rangeString + ' must be int values only'
+ throw `Range '${rangeString}' must be int values only!`
}
return [min, max]
@@ -147,64 +82,42 @@ const getPosNonZeroIntRange = rangeString => {
const [min, max] = getPosIntRange(rangeString)
if (min < 1 || max < 1) {
- throw 'Range ' + rangeString + ' values must be non-zero!'
+ throw `Range '${rangeString}' values must be non-zero!`
}
return [min, max]
}
class VariableGenerator {
- // generate(defs) {
- // const generated = defs.reduce(
- // (acc, def) => acc.concat(this.generateOne(def)),
- // []
- // )
-
- // const table = {}
- // generated.forEach(g => {
- // if (table[g.name]) {
- // throw 'Duplicate variable definition!'
- // }
-
- // table[g.name] = g.value
- // })
-
- // return table
- // }
-
generateOne(def) {
let value = null
switch (def.type) {
- case 'random-list':
+ case RANDOM_LIST:
value = this.getRandomList(def)
break
- case 'random-sequence':
+ case RANDOM_SEQUENCE:
value = this.getRandomSequence(def)
break
- case 'random-number':
+ case RANDOM_NUMBER:
value = this.getRandomNumber(def)
break
- case 'pick-one':
+ case PICK_ONE:
value = this.getPickOne(def)
break
- case 'pick-list':
+ case PICK_LIST:
value = this.getPickList(def)
break
- case 'static-value':
- case 'static-list':
+ case STATIC_VALUE:
+ case STATIC_LIST:
value = def.value
break
- // case 'fn':
- // value = parse(def)
- // break
-
default:
throw 'Unexpected type!'
}
@@ -275,37 +188,6 @@ class VariableGenerator {
return this.pickMany(parse(def.value), chooseMin, chooseMax, Boolean(def.ordered))
}
- // getSet(def) {
- // const names = {}
- // def.values.forEach(arr => {
- // arr.forEach(childDef => {
- // if (childDef.type === 'set') {
- // throw 'Unable to nest sets!'
- // }
-
- // if (!names[childDef.name]) {
- // names[childDef.name] = 0
- // }
-
- // names[childDef.name]++
- // })
- // })
-
- // const nameCounts = Object.values(names)
- // const expectedCount = nameCounts[0]
- // nameCounts.forEach(c => {
- // if (c !== expectedCount) {
- // throw 'Variable mismatch inside set!'
- // }
- // })
-
- // const chosenDefs = this.pickOne(def.values)
-
- // const results = chosenDefs.map(this.generateOne)
-
- // return results
- // }
-
rand(min, max, decimals = 0) {
if (min > max) {
throw 'Min cannot be above max!'
@@ -315,12 +197,10 @@ class VariableGenerator {
throw 'Decimals must be >= 0!'
}
- // debugger
-
return parseFloat((Math.random() * (max - min) + min).toFixed(decimals))
}
- generateRandomArray(size, valueMin, valueMax, decimalsMin, decimalsMax, unique = false) {
+ generateRandomArray(size, valueMin, valueMax, decimalsMin, decimalsMax, unique) {
const list = []
while (list.length < size) {
@@ -338,15 +218,11 @@ class VariableGenerator {
return list[this.rand(0, list.length - 1)]
}
- pickMany(list, sizeMin, sizeMax, ordered = false) {
+ pickMany(list, sizeMin, sizeMax, ordered) {
if (sizeMin > list.length || sizeMax > list.length) {
throw 'min or max cannot be larger than the size of the list!'
}
- if (sizeMin > sizeMax) {
- throw 'min cannot be larger than max!'
- }
-
const size = this.rand(sizeMin, sizeMax)
list = this.generateRandomArray(size, 0, list.length - 1, 0, 0, true)
diff --git a/packages/obonode/obojobo-sections-assessment/components/full-review/index.js b/packages/obonode/obojobo-sections-assessment/components/full-review/index.js
index c8cc339a0d..3585a62f48 100644
--- a/packages/obonode/obojobo-sections-assessment/components/full-review/index.js
+++ b/packages/obonode/obojobo-sections-assessment/components/full-review/index.js
@@ -26,13 +26,16 @@ class AssessmentReviewView extends React.Component {
// exist for all attempts
// the variable store should prevent duplicates on its own
this.props.attempts.forEach(attempt => {
- const attemptContext = `assessmentReview:${attempt.id}`
- Dispatcher.trigger('variables:addContext', {
- value: {
- context: attemptContext,
- variables: attempt.state.variables
- }
- })
+ // attempt.state should always exist, but dodge errors just in case it doesn't
+ if (attempt.state && attempt.state.variables) {
+ const attemptContext = `assessmentReview:${attempt.id}`
+ Dispatcher.trigger('variables:addContext', {
+ value: {
+ context: attemptContext,
+ variables: attempt.state.variables
+ }
+ })
+ }
})
}
diff --git a/packages/obonode/obojobo-sections-assessment/components/full-review/index.test.js b/packages/obonode/obojobo-sections-assessment/components/full-review/index.test.js
index 15adb57c72..c0b229a011 100644
--- a/packages/obonode/obojobo-sections-assessment/components/full-review/index.test.js
+++ b/packages/obonode/obojobo-sections-assessment/components/full-review/index.test.js
@@ -2,6 +2,7 @@ import AssessmentUtil from 'obojobo-document-engine/src/scripts/viewer/util/asse
import FullReview from './index'
import NavUtil from 'obojobo-document-engine/src/scripts/viewer/util/nav-util'
import OboModel from 'obojobo-document-engine/src/scripts/common/models/obo-model'
+import Dispatcher from 'obojobo-document-engine/src/scripts/common/flux/dispatcher.js'
import React from 'react'
import { mount } from 'enzyme'
import renderer from 'react-test-renderer'
@@ -13,6 +14,7 @@ jest.mock('../assessment-score-report-view', () => {
return () => Mock assessment score report view
})
jest.mock('obojobo-document-engine/src/scripts/common/util/uuid', () => () => 'mock-uuid')
+jest.mock('obojobo-document-engine/src/scripts/common/flux/dispatcher.js')
// register the modules required for this test
require('../../viewer')
@@ -780,4 +782,90 @@ describe('FullReview', () => {
expect(NavUtil.setContext).toHaveBeenCalledTimes(2)
expect(NavUtil.setContext).toHaveBeenCalledWith('assessmentReview:mockSecondAttemptId')
})
+
+ test('dispatches variables:addContext trigger per attempt', () => {
+ const moduleData = {
+ assessmentState: 'mockAssessmentState',
+ navState: {
+ context: 'assessmentReview:mockAttemptId'
+ },
+ questionState: {
+ contexts: {
+ 'assessmentReview:mockAttemptId': {
+ scores: {},
+ responses: {},
+ revealedQuestions: {}
+ }
+ }
+ },
+ focusState: {}
+ }
+ const model = OboModel.create(assessmentJSON)
+
+ const mockFirstAttemptVariables = [{ name: 'var1' }, { name: 'var2' }]
+ const mockSecondAttemptVariables = [{ name: 'var3' }, { name: 'var4' }]
+
+ const attempts = [
+ {
+ id: 'mockAttemptId',
+ attemptNumber: 3,
+ assessmentScore: 80,
+ completedAt: '2018-06-05 20:28:11.228294+00',
+ result: {
+ questionScores: [
+ {
+ id: 'questionId'
+ }
+ ]
+ },
+ state: {
+ questionModels: {
+ questionId: questionJSON
+ },
+ variables: mockFirstAttemptVariables
+ }
+ },
+ {
+ id: 'mockSecondAttemptId',
+ attemptNumber: 2,
+ assessmentScore: 100,
+ completedAt: '2018-06-05 20:28:11.228294+00',
+ result: {
+ questionScores: [
+ {
+ id: 'questionId'
+ }
+ ]
+ },
+ state: {
+ questionModels: {
+ questionId: questionJSON
+ },
+ variables: mockSecondAttemptVariables
+ }
+ }
+ ]
+ AssessmentUtil.getNumPossibleCorrect.mockReturnValue(1)
+
+ AssessmentUtil.getLastAttemptForModel.mockReturnValueOnce({ id: 'mockAttemptId' })
+ AssessmentUtil.getHighestAttemptsForModelByAssessmentScore.mockReturnValueOnce([])
+
+ mount()
+
+ expect(Dispatcher.trigger).toHaveBeenCalledTimes(2)
+ expect(Dispatcher.trigger.mock.calls[0][0]).toEqual('variables:addContext')
+ expect(Dispatcher.trigger.mock.calls[0][1]).toEqual({
+ value: {
+ context: 'assessmentReview:' + attempts[0].id,
+ variables: mockFirstAttemptVariables
+ }
+ })
+ expect(Dispatcher.trigger.mock.calls[1][0]).toEqual('variables:addContext')
+ expect(Dispatcher.trigger.mock.calls[1][1]).toEqual({
+ value: {
+ context: 'assessmentReview:' + attempts[1].id,
+ variables: mockSecondAttemptVariables
+ }
+ })
+ })
})
diff --git a/packages/obonode/obojobo-sections-assessment/server/attempt-start.js b/packages/obonode/obojobo-sections-assessment/server/attempt-start.js
index 8ed1408972..7af5319096 100644
--- a/packages/obonode/obojobo-sections-assessment/server/attempt-start.js
+++ b/packages/obonode/obojobo-sections-assessment/server/attempt-start.js
@@ -30,7 +30,7 @@ const startAttempt = (req, res) => {
attemptHistory: null,
numAttemptsTaken: null,
questionUsesMap: null,
- variables: req.currentVisit.state.variables ?? null,
+ variables: req.currentVisit.state ? req.currentVisit.state.variables ?? null : null,
resourceLinkId: req.currentVisit.resource_link_id
}
diff --git a/packages/obonode/obojobo-sections-assessment/server/attempt-start.test.js b/packages/obonode/obojobo-sections-assessment/server/attempt-start.test.js
index 8225b39fdb..e27b6ae3ed 100644
--- a/packages/obonode/obojobo-sections-assessment/server/attempt-start.test.js
+++ b/packages/obonode/obojobo-sections-assessment/server/attempt-start.test.js
@@ -82,42 +82,60 @@ describe('start attempt route', () => {
mockRes = {}
})
+ // reusable mock object structures only really used in the first few tests
+ const standardMockAssessmentNode = {
+ getChildNodeById: jest.fn(() => ({
+ node: {
+ content: {
+ attempts: 3
+ }
+ },
+ children: [
+ {},
+ {
+ childrenSet: [],
+ buildAssessment: jest.fn().mockReturnValueOnce([
+ {
+ id: 'mockQuestion',
+ type: QUESTION_NODE_TYPE
+ }
+ ])
+ }
+ ],
+ draftTree: {
+ getChildNodeById: jest.fn().mockReturnValueOnce({
+ id: 'mockQuestion',
+ type: QUESTION_NODE_TYPE,
+ children: [],
+ yell: jest.fn(),
+ toObject: jest.fn().mockReturnValueOnce({})
+ })
+ }
+ }))
+ }
+ const standardMockAttempt = {
+ attemptId: 'mockAttemptId',
+ state: {
+ chosen: [
+ {
+ id: 'mockQuestion',
+ type: QUESTION_NODE_TYPE
+ },
+ {
+ id: 'mockQuestionBank',
+ type: QUESTION_BANK_NODE_TYPE
+ }
+ ]
+ }
+ }
+
test('startAttempt calls database, inserts events, adds assessment questions to response, and returns expected object', () => {
mockRes = {
success: jest.fn(),
reject: jest.fn()
}
- const mockAssessmentNode = {
- getChildNodeById: jest.fn(() => ({
- node: {
- content: {
- attempts: 3
- }
- },
- children: [
- {},
- {
- childrenSet: [],
- buildAssessment: jest.fn().mockReturnValueOnce([
- {
- id: 'mockQuestion',
- type: QUESTION_NODE_TYPE
- }
- ])
- }
- ],
- draftTree: {
- getChildNodeById: jest.fn().mockReturnValueOnce({
- id: 'mockQuestion',
- type: QUESTION_NODE_TYPE,
- children: [],
- yell: jest.fn(),
- toObject: jest.fn().mockReturnValueOnce({})
- })
- }
- }))
- }
+ const mockAssessmentNode = { ...standardMockAssessmentNode }
mockReq = {
body: {
@@ -133,22 +151,99 @@ describe('start attempt route', () => {
currentUser: { id: 4 }
}
- const mockAttempt = {
- attemptId: 'mockAttemptId',
- state: {
- chosen: [
- {
- id: 'mockQuestion',
- type: QUESTION_NODE_TYPE
- },
- {
- id: 'mockQuestionBank',
- type: QUESTION_BANK_NODE_TYPE
- }
- ]
- }
+ const mockAttempt = { ...standardMockAttempt }
+
+ AssessmentModel.getCompletedAssessmentAttemptHistory.mockResolvedValueOnce([])
+ AssessmentModel.createNewAttempt.mockResolvedValueOnce(mockAttempt)
+
+ insertEvent.mockReturnValueOnce('mockInsertResult')
+
+ expect.hasAssertions()
+ return startAttempt(mockReq, mockRes).then(() => {
+ expect(mockRes.success).toBeCalledTimes(1)
+ expect(AssessmentModel.getCompletedAssessmentAttemptHistory).toHaveBeenCalled()
+ expect(AssessmentModel.createNewAttempt).toHaveBeenCalled()
+ // bonus test - make sure the variables are 'null' by default
+ expect(AssessmentModel.createNewAttempt.mock.calls[0][4].variables).toBeNull()
+ expect(getFullQuestionsFromDraftTree).toHaveBeenCalledTimes(1)
+ })
+ })
+
+ test('startAttempt handles lack of defined variables when visit state is otherwise present', () => {
+ mockRes = {
+ success: jest.fn(),
+ reject: jest.fn()
+ }
+
+ const mockAssessmentNode = { ...standardMockAssessmentNode }
+
+ mockReq = {
+ body: {
+ draftId: 'mockDraftId',
+ assessmentId: 'mockAssessmentId'
+ },
+ hostname: 'mockHostname',
+ connection: {
+ remoteAddress: 'mockRemoteAddress'
+ },
+ currentVisit: {
+ is_preview: false,
+ state: {}
+ },
+ currentDocument: mockAssessmentNode,
+ currentUser: { id: 4 }
+ }
+
+ const mockAttempt = { ...standardMockAttempt }
+
+ AssessmentModel.getCompletedAssessmentAttemptHistory.mockResolvedValueOnce([])
+ AssessmentModel.createNewAttempt.mockResolvedValueOnce(mockAttempt)
+
+ insertEvent.mockReturnValueOnce('mockInsertResult')
+
+ expect.hasAssertions()
+ return startAttempt(mockReq, mockRes).then(() => {
+ expect(mockRes.success).toBeCalledTimes(1)
+ expect(AssessmentModel.getCompletedAssessmentAttemptHistory).toHaveBeenCalled()
+ expect(AssessmentModel.createNewAttempt).toHaveBeenCalled()
+ // make sure the variables are still 'null' if currentVisit.state exists but lacks variables
+ expect(AssessmentModel.createNewAttempt.mock.calls[0][4].variables).toBeNull()
+ expect(getFullQuestionsFromDraftTree).toHaveBeenCalledTimes(1)
+ })
+ })
+
+ test('startAttempt associates current visit variables with a new attempt properly', () => {
+ mockRes = {
+ success: jest.fn(),
+ reject: jest.fn()
+ }
+
+ const mockAssessmentNode = { ...standardMockAssessmentNode }
+
+ // actual variables would have much more detail than this
+ const mockVariables = [{ name: 'var1' }, { name: 'var2' }]
+
+ mockReq = {
+ body: {
+ draftId: 'mockDraftId',
+ assessmentId: 'mockAssessmentId'
+ },
+ hostname: 'mockHostname',
+ connection: {
+ remoteAddress: 'mockRemoteAddress'
+ },
+ currentVisit: {
+ is_preview: false,
+ state: {
+ variables: mockVariables
+ }
+ },
+ currentDocument: mockAssessmentNode,
+ currentUser: { id: 4 }
}
+ const mockAttempt = { ...standardMockAttempt }
+
AssessmentModel.getCompletedAssessmentAttemptHistory.mockResolvedValueOnce([])
AssessmentModel.createNewAttempt.mockResolvedValueOnce(mockAttempt)
@@ -159,6 +254,7 @@ describe('start attempt route', () => {
expect(mockRes.success).toBeCalledTimes(1)
expect(AssessmentModel.getCompletedAssessmentAttemptHistory).toHaveBeenCalled()
expect(AssessmentModel.createNewAttempt).toHaveBeenCalled()
+ expect(AssessmentModel.createNewAttempt.mock.calls[0][4].variables).toEqual(mockVariables)
expect(getFullQuestionsFromDraftTree).toHaveBeenCalledTimes(1)
})
})