Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Variables and experimental features. #2116

Open
wants to merge 25 commits into
base: dev/33-serpierite
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
737e7ab
Add Variable Modal
vutoan1245 Feb 7, 2020
8c83389
Add feature to add/delete/duplicate variable
vutoan1245 Feb 11, 2020
cdd30a8
Add example text for variable modal
vutoan1245 Feb 13, 2020
5b2919e
Add unit test for variable-property and variable-value
vutoan1245 Feb 18, 2020
6a00df9
[WIP] more unit test for variable modal
vutoan1245 Feb 20, 2020
f1b49ca
[WIP] add more unit tests for variables modal
vutoan1245 Feb 24, 2020
d5e9b41
[WIP] Add accessibility features and unit tests for variables modal
vutoan1245 Feb 25, 2020
0548bd3
Merge remote-tracking branch 'toan/issue/1147-variables-modal' into i…
zachberry Apr 30, 2021
83893fb
WIP: $variables now work when put in JSON, changed it so variable val…
zachberry May 3, 2021
f000147
Initial commit of feature-flags.js
zachberry May 11, 2021
323b78b
Adds the feature-flags.js class and exposes it on window.obojobo
zachberry May 11, 2021
08d5e31
Merge branch 'issue/1814-feature-flags' into issue/554c-variables
zachberry May 17, 2021
f04e31b
Merge branch 'dev/21-beryl' into issue/554c-variables
zachberry May 17, 2021
c04a5d5
WIP: Gets JSON/XML parsing of variables working
zachberry May 18, 2021
63b4280
WIP: Highlights variables, now saves them as expected
zachberry Jun 8, 2021
61a7c7d
New branch from whatever Zach and Toan had done, merged in most recen…
FrenjaminBanklin Sep 6, 2022
30405f4
#554 Fixed variable highlighting to highlight more than just the firs…
FrenjaminBanklin Sep 8, 2022
7837a1c
#554 Modernized from dev/33, corrected merge conflicts.
FrenjaminBanklin Oct 10, 2023
b2413a6
#554 Fixed variables only being applicable on the module and pages. F…
FrenjaminBanklin Oct 11, 2023
1206b75
#554 Fixed the viewer crashing when trying to parse erroneous/invalid…
FrenjaminBanklin Oct 13, 2023
1a7b13a
#554 Added source model to information available to nav items to enab…
FrenjaminBanklin Oct 17, 2023
1f35d07
#554 Adjusted all relevant obonode converters and JSON to XML parsers…
FrenjaminBanklin Nov 3, 2023
a914b38
#554 Addressed unit test errors and missing coverage. Cleaned up unus…
FrenjaminBanklin Nov 21, 2023
7fa7c89
#554 Necessary Prettier fixes.
FrenjaminBanklin Nov 21, 2023
a422f1f
#554 Draft nodes will now assign their node ID to all of their childr…
FrenjaminBanklin Dec 5, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
"test:ci": "TZ='America/New_York' CI=true jest --ci --useStderr --coverage --coverageReporters text-summary cobertura",
"test:ci:each": "lerna run test:ci",
"test:dev": "TZ='America/New_York' jest --verbose --watchAll --coverage --coverageReporters lcov",
"test:view-lcov": "open ./coverage/lcov-report/index.html",
"postinstall": "husky install"
},
"devDependencies": {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ describe('Nav', () => {

beforeEach(() => {
jest.clearAllMocks()
mockDispatcherTrigger.mockReset()
})

test('renders opened', () => {
Expand Down Expand Up @@ -195,6 +196,148 @@ describe('Nav', () => {
expect(tree).toMatchSnapshot()
})

test('does not try to substitute labels if no brackets are found', () => {
// this shouldn't run in this test, but if it does the label will be changed
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: 'label5',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('label5')
expect(mockDispatcherTrigger).not.toHaveBeenCalled()
})

test('does not try to substitute labels if the variable text is not correctly formatted', () => {
// this shouldn't run in this test, but if it does the label will be changed
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{label5',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('{{label5')
expect(mockDispatcherTrigger).not.toHaveBeenCalled()
})

test('renders substituted variables in labels - substitute exists', () => {
// this should run and as a result modify the label text
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = 'not-label5'
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{$var}}',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
expect(liElements[1].children[0].children[0]).toBe('not-label5')
expect(mockDispatcherTrigger).toHaveBeenCalledTimes(1)
// so this second argument is weird - it isn't the thing that was originally sent to Dispatch.trigger,
// which was { text: '', isNav: true }, but because the callback triggered changed a property of the
// object it was passed, then it was technically also changing the object it was passed, which means
// we have to consider the state that object would have been in after the callback ran
expect(mockDispatcherTrigger).toHaveBeenCalledWith(
'getTextForVariable',
{ text: 'not-label5', isNav: true },
'$var',
{}
)
})

test('renders substituted variables in labels - no substitute exists', () => {
// this should run but as a result not modify the label text
mockDispatcherTrigger.mockImplementationOnce((trigger, event) => {
event.text = null
return event
})
NavUtil.getOrderedList.mockReturnValueOnce([
{ id: 4, type: 'heading', label: 'label4' },
{
id: 5,
type: 'link',
label: '{{$var}}',
flags: { visited: false, complete: false, correct: false },
// exists to be passed along to variable utilities, doesn't matter for testing
sourceModel: {}
}
])
const props = {
navState: {
open: false,
locked: true,
navTargetId: 56 // select this item
}
}
const component = renderer.create(<Nav {...props} />)

const liElements = component.root.findAllByType('li')
// event text was unchanged so original label text should be used
expect(liElements[1].children[0].children[0]).toBe('{{$var}}')
expect(mockDispatcherTrigger).toHaveBeenCalledTimes(1)
// see previous test note re: the second arg
expect(mockDispatcherTrigger).toHaveBeenCalledWith(
'getTextForVariable',
{ text: null, isNav: true },
'$var',
{}
)
})

test('renders blank title', () => {
NavUtil.getOrderedList.mockReturnValueOnce([{ id: 4, type: 'heading', label: '' }])
const props = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ import { mount } from 'enzyme'
import testObject from 'obojobo-document-engine/test-object.json'
import mockConsole from 'jest-mock-console'
import injectKatexIfNeeded from 'obojobo-document-engine/src/scripts/common/util/inject-katex-if-needed'
import VariableStore from 'obojobo-document-engine/src/scripts/viewer/stores/variable-store'
import VariableUtil from 'obojobo-document-engine/src/scripts/viewer/util/variable-util'

jest.mock('obojobo-document-engine/src/scripts/viewer/util/viewer-api')
jest.mock('obojobo-document-engine/src/scripts/common/util/inject-katex-if-needed')
Expand All @@ -40,6 +42,8 @@ jest.mock('obojobo-document-engine/src/scripts/viewer/components/nav')
jest.mock('obojobo-document-engine/src/scripts/common/page/dom-util')
jest.mock('obojobo-document-engine/src/scripts/common/util/insert-dom-tag')
jest.mock('obojobo-document-engine/src/scripts/common/components/modal-container')
jest.mock('obojobo-document-engine/src/scripts/viewer/stores/variable-store')
jest.mock('obojobo-document-engine/src/scripts/viewer/util/variable-util')

describe('ViewerApp', () => {
let restoreConsole
Expand Down Expand Up @@ -1025,7 +1029,7 @@ describe('ViewerApp', () => {
})
})

test('sendCloseEvent calls navigator.sendBeacon', done => {
test('sendClose`Event calls navigator.sendBeacon', done => {
global.navigator.sendBeacon = jest.fn()

expect.assertions(1)
Expand Down Expand Up @@ -1841,4 +1845,155 @@ describe('ViewerApp', () => {
spy1.mockRestore()
spy2.mockRestore()
})

test('component passes variables to VariableStore correctly', done => {
mocksForMount()

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})
const component = mount(<ViewerApp />)

setTimeout(() => {
expect(VariableStore.init).toHaveBeenCalledWith(mockVariables)
component.update()
done()
})
})

test('component calls expected VariableUtil functions, standard variable name', done => {
mocksForMount()

NavUtil.getContext.mockReturnValueOnce('test-context')

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})

const mockVariableState = { mockVariableStateKey: 'mockVariableStateVal' }

VariableStore.getState.mockReturnValueOnce(mockVariableState)

const component = mount(<ViewerApp />)

setTimeout(() => {
component.update()

VariableUtil.findValueWithModel.mockReturnValueOnce('mock-var-value')

// ordinarily this is an oboModel instance, but it doesn't matter for tests
const mockTextModel = { key: 'val' }
const mockEvent = { text: '' }

component.instance().getTextForVariable(mockEvent, '$variablename1', mockTextModel)
expect(VariableUtil.findValueWithModel).toHaveBeenCalledWith(
'test-context',
mockVariableState,
mockTextModel,
'variablename1'
)
expect(VariableUtil.getValue).not.toHaveBeenCalled()

expect(mockEvent.text).toEqual('mock-var-value')

done()
})
})

test('component calls expected VariableUtil functions, owner:variable name', done => {
mocksForMount()

NavUtil.getContext.mockReturnValueOnce('test-context')

const mockVariables = {
'nodeid1:variablename1': 'var1',
'nodeid1:variablename2': 'var2'
}

// reset the mocked function to test variables
ViewerAPI.requestStart = jest.fn().mockResolvedValueOnce({
status: 'ok',
value: {
visitId: 123,
lti: {
lisOutcomeServiceUrl: 'http://lis-outcome-service-url.test/example.php'
},
isPreviewing: true,
extensions: {
':ObojoboDraft.Sections.Assessment:attemptHistory': []
},
variables: mockVariables
}
})

const mockVariableState = { mockVariableStateKey: 'mockVariableStateVal' }

VariableStore.getState.mockReturnValueOnce(mockVariableState)

const component = mount(<ViewerApp />)

setTimeout(() => {
component.update()

VariableUtil.getValue.mockReturnValueOnce('mock-var-value')

// ordinarily this is an oboModel instance, but it doesn't matter for tests
const mockTextModel = { key: 'val' }
const mockEvent = { text: '' }

component.instance().getTextForVariable(mockEvent, '$nodeid1:variablename1', mockTextModel)
expect(VariableUtil.getValue).toHaveBeenCalledWith(
'test-context',
mockVariableState,
'nodeid1',
'variablename1'
)
expect(VariableUtil.findValueWithModel).not.toHaveBeenCalled()

expect(mockEvent.text).toEqual('mock-var-value')

// bonus test to make sure event text is not overwritten if no value is found
VariableUtil.getValue.mockReturnValueOnce(null)
mockEvent.text = ''
expect(mockEvent.text).toEqual('')
component.instance().getTextForVariable(mockEvent, '$nodeid1:variablename1', mockTextModel)
expect(mockEvent.text).toEqual('')

done()
})
})
})
Loading
Loading