Variables and experimental features.

[FrenjaminBanklin]

THIS PULL REQUEST HAS MIGRATIONS.

Closes #1147. Closes #1814. Significant progress towards #554.

Adds variable creation and substitution and experimental feature flags.

Adds a 'state' column to the 'visits' table to store the variables generated for a visit rather than regenerating them every time.

FEATURE FLAGS

This PR adds the ability to enable experimental features that are not ready for general user consumption but could potentially be used by power users. Currently this is handled at the browser level using localStorage, which can be set manually or simplified using a globally available API. For example, to enable the variable creation UI in the editor, run the following in the console:

window.obojobo.flags.set('experimental.variables', true)

---

VARIABLES

Variables can be defined on any node or page in a module or at the top level of the module itself. Variables should be automatically available to any node in the hierarchy under the location at which the variable is defined, but can also be referred to by variables not in the vertical hierarchy by using specific notation. For example, suppose the module has defined variables $a and $b. A Text node on any page should be able to use those variables automatically:

A text node's ordinary contents can have variables included using notation such as {{$a}} and {{$b}}.

Or, for example, a text node with an ID of 'some-text-node' could define a variable $c. Another text node - a sibling, or a text node on another page - can also use that variable:

A node on another page has a variable {{$some-text-node:c}}.

Things that still need doing:

• [x] Make sure any node can have variables.
  • The particular method of calling/dismissing the variable management modal was throwing an error before the variables were actually saved, but for some reason this error would not occur when editing variables for the module or for pages. Additionally, every node type's converter.js required an additional line to make sure variables remain attached to the node's content, and every node's corresponding JSON parser function had to be altered in order to make sure XML translation contains variables properly.
• [x] Validate variable names and values where applicable.
  • The VariableListModal element will validate all variables when it is rendered, and any changes to variables will be validated individually when they are made. Coupled with the viewer side adequately handling invalid variables, this should be good enough.
• [x] Stress test variable creation UI.
  • There may be some value in adding more error messages when the values provided for variables are invalid. Currently we're only doing this for the seriesType attribute for random sequence variables, since it can be given an invalid value manually in the code editors which don't translate well in the visual editor.
• [x] Handle bad variables (missing/invalid attributes etc., probably only likely to happen if someone tries getting cute with the XML/JSON editors).
  • Wrapping the code that generates variables in a try/except block prevents the viewer from crashing when it encounters a bad variable. Currently the result is the same as though the variable was not defined at all, but we may want to substitute some kind of error message to make it easier to find problems.
• [x] Handle undefined variables
  • Currently the viewer just doesn't show anything when attempting to use an undefined variable. This is probably good enough, but we may instead want to show some kind of error message in this case instead.
• [x] Make sure variables are generated per assessment attempt and associated with that attempt so that resuming an attempt will use the same generated variables.
  • It looks like the Question nodes generated in assessment reviews lack the 'parent' linkage that eventually leads all the way back up to the module - see the 'Problems' section below. Fixed.
• [x] Make sure variable specificity works - variables can be defined on any node and when used in a node variables should be used according to whichever one is closest to the context (e.g. the module has a '$a' and a text node has a '$a', the text node's '$a' should be used over the module's).
  • This works in most cases, but gets weird in assessments with nested question banks. If nested question banks are used, the question nodes are rendered separate from the question bank node that 'contains' them - see the 'Problems' section below. Fixed.
• [x] Make sure nodes can use other nodes' variables.
  • Fixed old code that had swapped the variable name and owning node ID in one of the generation functions.
• [x] Make sure variables are also substituted in the module navigation area if they are used in headers.
  • It turns out the text that appears in the nav area is somehow separated completely from the nodes used to determine what links appear in the nav area, so the usual variable substitution process doesn't work at all. Absent some clever solution that does the substitution on the server side, we may have to let this ride for a bit. Passing the parent model along when generating nav items makes them available when trying to perform variable substitutions on the nav text, as those models are required when trying to locate the value generated for any given variable. That seems to be adequate.

---

PROBLEMS

• [x] Variable substitutions using the simple syntax in assessment attempts and assessment reviews may not function properly in certain conditions. For example, take the following structure:

  Assessment
  |---Question Bank (pick 1 random) $qb (value: qb0)
  |------Question Bank (pick all) $qb(value: qb1), $other (value: other)
  |---------Question
  |------Question Bank (pick all) $qb(value: qb2), $other (value: other)
  |---------Question

  Given the settings on the primary question bank, a single one of the nested question banks will be selected, and all questions in the selected bank will be used in the assessment. If both questions are using a variable in the simplified format (e.g. {{$qb}}), then the value substituted in should be the value of $qb defined on the nested question bank that is chosen, meaning if the second question bank is chosen, the value displayed would be 'qb2'. This is not what happens. Since the question node is isolated from the rest of the draft to be rendered during the assessment, its containing question bank node is not rendered and not attached to the question node as a parent - so instead of the displayed value being 'qb2', it is 'qb0'. Likewise the substitution for $other does not work at all, because the node it would be found attached to does not exist anywhere it can be reached by the variable utility.

• [x] Variable substitutions using the simple syntax in assessment reviews do not function properly in most conditions. Much like the previously mentioned issue, questions are isolated from the rest of the draft in assessment reviews. In this case, it's even more egregious than during assessments - the rendered question nodes have no parent properties and therefore the variable substitution utility can not traverse the question node's ancestors to determine the owner of the variable being used. For example, take the following structure:

  Module $m (value: m)
  |---Assessment $a (value: a)
  |------Question Bank $qb (value: qb)
  |---------Question

  During an assessment attempt $m, $a and $qb will all substitute properly because the node's ancestral hierarchy is fully available via the parent property of the question node and each subsequent parent. During review, however, the question node is rendered without any part of this hierarchy. As a result, none of the variables are substituted at all.

[FrenjaminBanklin]

This is just about done and working, but there are a couple of potentially serious problems that we probably need to address before we can ship this.

These problems only really surface when using the {{$varname}} shorthand. Substitution appears to be working properly in all cases when using the {{$ownerid:varname}} format, but I don't think we can reliably expect content authors to be mindful of this and it kind of goes against the whole purpose of being able to define variables in multiple places using the same name.

Paging @zachberry for any ideas for approaching the things listed in 'Problems' in the PR description above. Currently it doesn't look like there is any way of circumventing the way Obojobo handles questions in assessment attempts and during assessment review in order to make the questions' full node ancestor hierarchies available to the variable util. I'm considering trying to find some way of attaching each question's full ancestor tree on the server side and then somehow using that information on the viewer side, but even in the best case scenario the solution is going to be extremely hacky.

[zachberry]

I probably need to re-familiarize myself a bit more, but some initial top-of-my-head thoughts:

• Could variable substitution happen server-side for assessments? When the server is choosing which questions to give to the client I think you'd have the whole tree in memory at that point. The server tree objects are a bit different than OboModel but maybe they could still be used. Then when the client gets back the question it doesn't get back "What is {{a}}?", it gets back "What is 0.123?".
• Or, determine the value of all variables on the initial viewer page load and return them along with the viewer JSON. They could be connected to the ID of the node they're used in. So for example, if you have a document like this (this example deliberately uses both shorthand and longhand):

...
   Page (id=p1, variables: studentName=chooseOne('Dick', 'Jane'))
      TextChunk (id=tc1)
         Text (id=t1)
            textGroup: "Imagine a student named {{ studentName }}. When {{ p1:studentName }} drives 10 miles..."

The server could return the following when requesting a document (I'm not tied to this exact structure, just trying to convey the idea):

{
  root: { //...the whole obo document...// },
  variables: [
    {
      name: "studentName",
      value: "Jane",
      owner: "p1",
      refs: ["t1"]
    }
  ]
}

So the server comes back with a variable value it chose (Jane), where the variable is defined (the "owner", which is "p1", the Page) and finally anywhere the variable is used (the "refs", which is "t1", the TextChunk that uses it).

(You would ideally want to further process the variables data client-side to make it more useful to quickly look up - maybe make a data structure like this:)

const variableValuesByOboNodeId = {
  p1: {
    studentName: "Jane"
  },
  t1: {
    studentName: "Jane"
  }
}

That way when the parser/viewer comes to {{ studentName }} or {{ p1:studentName}} you would have enough information to find what its value should be.

If this was done for everything in the document then you would be fine if you're missing part of the node structure, such as in Assessment, because that variables response gives you everything you need to connect the dots.

Thoughts? I might be missing something obvious since it's been a minute since I looked at this stuff.

[FrenjaminBanklin]

My knee-jerk reaction to performing variable substitutions server-side but just for assessments is that we may as well just perform substitutions server-side for the whole document. There was a lot of code that seemed like it wasn't being used that I just got rid of, I'm not sure if any of it would have been supportive of this approach or if it was just left over from some unrelated experimentation. But then that seems like it would require some pretty significant rearchitecting of not just the way variables work but the way the whole viewer backend works. I'm also not sure if moving substitutions out of the client side completely would make it harder to use variables in nav links - or since the substitution would be done already if the nav links would just have the correct text to begin with.

I can look into limiting the server-side pre-baked substitutions to just everything downstream of the Assessment. I'd taken a really tentative step in this direction before but aborted because I couldn't see any obvious convenience tools for traversing a draft tree in reverse (there's a getChildNodeById method, but not a getParentNode method, for example - so that's something we'll probably want to add to make our lives easier) or for easily checking a node's text for the variable string once it's objectified (though admittedly I didn't experiment much with this). It'll be a bit of a hack to have substitution happening two different ways in two different places, but it could just be a 'this works for now, standardize on one later' kind of deal.

[FrenjaminBanklin]

The solution I ended up coming up with was a bit of a hybrid. When an attempt is started, each chosen question will have all of its descendent nodes checked for variables. After all of the variables used anywhere in the question have been aggregated, for each variable all of the question's ancestor nodes will be checked until the closest one that owns a variable of the same name. Once a match is found, a reference is stored as part of the attempt.state.chosen payload in a new varRef key so that it can be referenced when resuming or reviewing an attempt.

Luckily this only has to happen the one time, and then whenever attempt.state.chosen is used to pull all of the questions out of a draft, a separate step can recurse through all of the child nodes for each chosen question if a varRef exists for that question, and whenever a string matching the given variable is found it will just be replaced with the long form syntax.

So if a question contains any text nodes with a {{$qb}}, and that question's parent QuestionBank with an ID of 'qb-id' is the nearest node with a 'qb' variable defined, then before that question reaches the client for rendering all occurrences of {{$qb}} within the question will be changed to {{$qb-id:qb}}. At that point the existing tools on the client side should handle it.

It turns out that using Maps to manage the nodes in the draft tree makes it pretty difficult to determine the parent of any given node. At least, it seems that way to me. I'm willing to believe there's some convenience or cleverness that I could have used instead of my 'turn it into an array and just traverse it multiple times to find everything' approach, but what I came up with seems to be working. I've also adjusted the constructor code for the draft tree in currentDocument so that whenever a node detects that it has children, it adds a parentId property to those children so that it's easier to traverse the tree in both directions.