Dev/17 peridot

.github/workflows/tests.yml

@@ -14,9 +14,19 @@ jobs:
       uses: actions/setup-node@v1
       with:
         node-version: ${{ matrix.node-version }}
+
+    # caches yarn dependencies in the yarn cache (not npm_modules)
+    # should help to speed up yarn install and be safe
+    # across changes to the repo and platforms
+    - name: Cache Yarn Deps
+      uses: c-hive/gha-yarn-cache@v1      
+
     - run: echo "github.ref = ${{ github.ref }}"
-    - run: npm install -g yarn
-    - run: yarn install
+
+    # frozen lockfile should make the cache more effective
+    # and our tests more predictable
+    - run: yarn install --frozen-lockfile
+
     - run: yarn test:ci
 
   build_docker_images:

app.json

@@ -14,6 +14,10 @@
 			"description": "PG-Promise SSL Configuration. https://github.com/vitaly-t/pg-promise/wiki/Connection-Syntax",
 			"value": "{\"rejectUnauthorized\":false}"
 		},
+		"DB_TIMEOUT": {
+			"description": "DB Query timeout in milliseconds https://github.com/brianc/node-postgres/pull/1760",
+			"value": "29000"
+		},
 		"OBO_LTI_KEYS_JSON": {
 			"description": "Edit the secret value. JSON string for LTI key:secret values.",
 			"value": "{\"obo-production-lti-key\":\"CHANGE_THIS_VALUE_NOW\"}"

docker/obojobo-pm2-server-src/package.json

@@ -1,6 +1,6 @@
 {
   "name": "obojobo-pm2-server-app",
-  "version": "11.2.0",
+  "version": "11.3.0",
   "description": "Reference project for deploying and customizing an Obojobo Next server",
   "main": "./index.js",
   "private": true,

lerna.json

@@ -2,7 +2,7 @@
   "packages": [
     "packages/**/*"
   ],
-  "version": "11.2.0",
+  "version": "11.3.0",
   "command": {
     "command": {
       "run": {

package.json

@@ -102,5 +102,5 @@
 		"yarn": "^1.15.2",
 		"node": "^12.11.1"
 	},
-	"version": "11.2.0"
+	"version": "11.3.0"
 }

packages/app/obojobo-document-engine/__tests__/Viewer/stores/assessment-store.test.js

@@ -318,6 +318,24 @@ describe('AssessmentStore', () => {
 		)
 	})
 
+	test('startAttemptWithAPICall shows an error if the assessment ID is invalid', () => {
+		OboModel.create(getExampleAssessment())
+
+		AssessmentAPI.startAttempt.mockResolvedValueOnce({
+			status: 'error',
+			value: {
+				message: 'ID not found'
+			}
+		})
+
+		return AssessmentStore.startAttemptWithAPICall('draftId', 'visitId', 'assessmentId').then(
+			() => {
+				expect(ErrorUtil.show).toHaveBeenCalledTimes(1)
+				expect(AssessmentStore.triggerChange).toHaveBeenCalledTimes(1)
+			}
+		)
+	})
+
 	test('startAttemptWithAPICall shows a generic error if an unrecognized error is thrown and triggers a change', () => {
 		OboModel.create(getExampleAssessment())
 

packages/app/obojobo-document-engine/__tests__/oboeditor/components/navigation/header.test.js

@@ -4,7 +4,9 @@ import Common from 'src/scripts/common'
 import { mount } from 'enzyme'
 
 import EditorUtil from 'src/scripts/oboeditor/util/editor-util'
+import isValidId from 'src/scripts/oboeditor/util/is-valid-id'
 jest.mock('src/scripts/oboeditor/util/editor-util')
+jest.mock('src/scripts/oboeditor/util/is-valid-id')
 
 describe('Header', () => {
 	beforeEach(() => {
@@ -199,6 +201,7 @@ describe('Header', () => {
 		}
 		const component = mount(<Header {...props} />)
 		Common.models.OboModel.models['5'].setId.mockReturnValueOnce(true)
+		isValidId.mockReturnValueOnce(true)
 		component.instance().saveId('5', 'mock-id')
 
 		expect(EditorUtil.rebuildMenu).toHaveBeenCalled()
@@ -242,9 +245,54 @@ describe('Header', () => {
 			]
 		}
 		const component = mount(<Header {...props} />)
+		isValidId.mockReturnValueOnce(true)
 		Common.models.OboModel.models['5'].setId.mockReturnValueOnce(false)
 		component.instance().saveId('5', 'mock-id')
 
 		expect(EditorUtil.rebuildMenu).not.toHaveBeenCalled()
 	})
+
+	test('saveId does nothing if the old and new ids are the same', () => {
+		const props = {
+			index: 0,
+			list: [
+				{
+					id: '5',
+					type: 'header',
+					label: 'label5',
+					contentType: 'Page',
+					flags: {
+						assessment: false
+					}
+				}
+			]
+		}
+		const component = mount(<Header {...props} />)
+		component.instance().saveId('5', '5')
+
+		expect(EditorUtil.rebuildMenu).not.toHaveBeenCalled()
+	})
+
+	test('saveId checks if the id is valid', () => {
+		const props = {
+			index: 0,
+			list: [
+				{
+					id: '5',
+					type: 'header',
+					label: 'label5',
+					contentType: 'Page',
+					flags: {
+						assessment: false
+					}
+				}
+			]
+		}
+		const component = mount(<Header {...props} />)
+		isValidId.mockReturnValueOnce(false)
+		component.instance().saveId('5', '5!')
+
+		expect(isValidId).toHaveBeenCalled()
+		expect(Common.models.OboModel.models['5'].setId).not.toHaveBeenCalled()
+	})
 })

packages/app/obojobo-document-engine/__tests__/oboeditor/components/navigation/sub-menu.test.js

@@ -4,6 +4,7 @@ import renderer from 'react-test-renderer'
 
 import Common from '../../../../src/scripts/common'
 import EditorUtil from '../../../../src/scripts/oboeditor/util/editor-util'
+import isValidId from '../../../../src/scripts/oboeditor/util/is-valid-id'
 import {
 	getTriggersWithActionsAdded,
 	getTriggersWithActionsRemoved,
@@ -56,6 +57,7 @@ jest.mock('../../../../src/scripts/common', () => ({
 }))
 
 jest.mock('../../../../src/scripts/oboeditor/util/editor-util')
+jest.mock('../../../../src/scripts/oboeditor/util/is-valid-id')
 jest.mock('../../../../src/scripts/common/util/trigger-util')
 jest.mock('../../../../src/scripts/oboeditor/stores/editor-store', () => ({
 	state: { startingId: null }
@@ -323,6 +325,7 @@ describe('SubMenu', () => {
 		]
 
 		const component = mount(<SubMenu index={0} list={itemList} updateNavTargetId={jest.fn()} />)
+		isValidId.mockReturnValue(true)
 
 		component.instance().saveId('7', 'mock-id')
 		expect(EditorUtil.rebuildMenu).not.toHaveBeenCalled()
@@ -343,13 +346,34 @@ describe('SubMenu', () => {
 			}
 		]
 		Common.models.OboModel.models['7'].setId.mockReturnValueOnce(true)
+		isValidId.mockReturnValueOnce(true)
 
 		const component = mount(<SubMenu index={0} list={itemList} updateNavTargetId={jest.fn()} />)
 
 		component.instance().saveId('7', 'mock-id')
 		expect(EditorUtil.rebuildMenu).toHaveBeenCalled()
 	})
 
+	test('saveId checks if the id is valid', () => {
+		const itemList = [
+			{
+				id: 7,
+				type: 'link',
+				label: 'label7',
+				flags: {
+					assessment: false
+				}
+			}
+		]
+		const component = mount(<SubMenu index={0} list={itemList} updateNavTargetId={jest.fn()} />)
+		Common.models.OboModel.models['7'].setId = jest.fn()
+		isValidId.mockReturnValueOnce(false)
+		component.instance().saveId('7', '7!')
+
+		expect(isValidId).toHaveBeenCalled()
+		expect(Common.models.OboModel.models['7'].setId).not.toHaveBeenCalled()
+	})
+
 	test('saveContent updates model', () => {
 		const itemList = [
 			{

packages/app/obojobo-document-engine/__tests__/oboeditor/components/node/editor-component.test.js

@@ -7,6 +7,7 @@ import Common from '../../../../src/scripts/common'
 import Node from 'src/scripts/oboeditor/components/node/editor-component'
 import InsertMenu from 'src/scripts/oboeditor/components/node/components/insert-menu'
 import MoreInfoBox from 'src/scripts/oboeditor/components/navigation/more-info-box'
+import isValidId from 'src/scripts/oboeditor/util/is-valid-id'
 
 const mockNode = {
 	isInsertable: true,
@@ -17,6 +18,7 @@ const mockNode = {
 	cloneBlankNode: () => ({ type: 'mockNode' })
 }
 
+jest.mock('src/scripts/oboeditor/util/is-valid-id')
 jest.mock('src/scripts/oboeditor/components/navigation/more-info-box', () =>
 	global.mockReactComponent(this, 'MockMoreInfoBox')
 )
@@ -229,6 +231,7 @@ describe('Component Editor Node', () => {
 
 	test('saveId does not allow duplicate nodes', () => {
 		const component = rtr.create(<Node {...mockProps} />)
+		isValidId.mockReturnValueOnce(true)
 		const result = component.getInstance().saveId('mock-duplicate-id', 'mock-id2')
 		expect(result).toBe('The id "mock-id2" already exists. Please choose a unique id.')
 		expect(Transforms.setNodes).not.toHaveBeenCalled()
@@ -244,6 +247,7 @@ describe('Component Editor Node', () => {
 	test('saveId updates the node if ids are not the same', () => {
 		ReactEditor.findPath.mockReturnValue([0]) // for getSlatePath
 		const component = rtr.create(<Node {...mockProps} />)
+		isValidId.mockReturnValueOnce(true)
 		const result = component.getInstance().saveId('mock-id', 'mock-id2')
 		expect(result).toBe()
 		expect(Transforms.setNodes).toHaveBeenCalledWith(
@@ -253,6 +257,14 @@ describe('Component Editor Node', () => {
 		)
 	})
 
+	test('saveId checks if the id is valid', () => {
+		const component = rtr.create(<Node {...mockProps} />)
+		isValidId.mockReturnValueOnce(false)
+		component.getInstance().saveId('5', '5!')
+
+		expect(isValidId).toHaveBeenCalled()
+	})
+
 	test('saveContent calls Transforms.setNodes', () => {
 		const component = rtr.create(<Node {...mockProps} />)
 		component.getInstance().saveContent({}, {})

packages/app/obojobo-document-engine/__tests__/oboeditor/components/visual-editor.test.js

@@ -10,8 +10,9 @@ import ModalStore from '../../../src/scripts/common/stores/modal-store'
 import Dispatcher from '../../../src/scripts/common/flux/dispatcher'
 import OboModel from 'src/scripts/common/models/obo-model'
 
-import { Editor } from 'slate'
+import { Editor, Transforms } from 'slate'
 import { ReactEditor } from 'slate-react'
+// jest.mock('slate')
 jest.mock('src/scripts/viewer/util/editor-api')
 jest.mock('src/scripts/common/util/modal-util')
 jest.mock('src/scripts/oboeditor/components/node/editor', () => ({
@@ -79,6 +80,43 @@ describe('VisualEditor', () => {
 		expect(component.toJSON()).toMatchSnapshot()
 	})
 
+	test('VisualEditor component handles triple click', () => {
+		const spy = jest.spyOn(Transforms, 'move').mockReturnValueOnce(true)
+		const props = {
+			insertableItems: 'mock-insertable-items',
+			page: {
+				attributes: { children: [{ type: 'mockNode' }] },
+				get: jest.fn(),
+				toJSON: () => ({ children: [{ type: 'mockNode' }] })
+			},
+			model: { title: 'Mock Title' }
+		}
+		const component = mount(<VisualEditor {...props} />)
+
+		// Double click
+		component
+			.find('.obojobo-draft--pages--page')
+			.at(0)
+			.simulate('click', { detail: 2 })
+		expect(spy).not.toHaveBeenCalled()
+
+		// Selection does not end at the beginning
+		component.instance().editor.selection.focus.offset = 1
+		component
+			.find('.obojobo-draft--pages--page')
+			.at(0)
+			.simulate('click', { detail: 3 })
+		expect(spy).not.toHaveBeenCalled()
+
+		// Selection ends at the beginning
+		component.instance().editor.selection.focus.offset = 0
+		component
+			.find('.obojobo-draft--pages--page')
+			.at(0)
+			.simulate('click', { detail: 3 })
+		expect(spy).toHaveBeenCalled()
+	})
+
 	test('VisualEditor component - editor is disable when modal is opened', () => {
 		ModalStore.init()
 

packages/app/obojobo-document-engine/__tests__/oboeditor/util/is-valid-id.test.js

@@ -0,0 +1,20 @@
+import isValidId from 'src/scripts/oboeditor/util/is-valid-id'
+
+describe('Validate ID', () => {
+	test.each`
+		input          | output
+		${'mock-id'}   | ${true}
+		${'mock_id'}   | ${true}
+		${'mock:id'}   | ${true}
+		${'mock.id'}   | ${true}
+		${'mock+id'}   | ${false}
+		${'mock=id'}   | ${false}
+		${'abc123'}    | ${true}
+		${'ABC123'}    | ${true}
+		${'"mock-id"'} | ${false}
+		${'/mock-id/'} | ${false}
+		${'[mock-id]'} | ${false}
+	`('isValidId("$input") = "$output"', ({ input, output }) => {
+		expect(isValidId(input)).toBe(output)
+	})
+})

packages/app/obojobo-document-engine/package.json

@@ -1,6 +1,6 @@
 {
 	"name": "obojobo-document-engine",
-	"version": "11.2.0",
+	"version": "11.3.0",
 	"license": "AGPL-3.0-only",
 	"description": "",
 	"engines": {
@@ -45,7 +45,7 @@
 		"codemirror": "^5.58.1",
 		"date-fns": "^2.16.1",
 		"downloadjs": "^1.4.7",
-		"katex": "^0.11.1",
+		"katex": "^0.12.0",
 		"prop-types": "15.7.2",
 		"react": "^16.13.1",
 		"react-codemirror2": "^7.2.1",

packages/app/obojobo-document-engine/src/scripts/common/util/inject-katex-if-needed.js

@@ -16,12 +16,12 @@ const injectKatexIfNeeded = async ({ value: draftModel }) => {
 
 	if (stringModel.includes('latex')) {
 		const jsProps = {
-			src: 'https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.11.1/katex.min.js'
+			src: 'https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.12.0/katex.min.js'
 		}
 		const cssProps = {
 			rel: 'stylesheet',
 			type: 'text/css',
-			href: 'https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.11.1/katex.min.css'
+			href: 'https://cdnjs.cloudflare.com/ajax/libs/KaTeX/0.12.0/katex.min.css'
 		}
 		insertDomTag(jsProps, 'script')
 		insertDomTag(cssProps, 'link')

packages/app/obojobo-document-engine/src/scripts/oboeditor/components/navigation/header.js

@@ -1,5 +1,6 @@
 import Common from 'obojobo-document-engine/src/scripts/common'
 import EditorUtil from '../../util/editor-util'
+import isValidId from 'obojobo-document-engine/src/scripts/oboeditor/util/is-valid-id'
 import React from 'react'
 import MoreInfoBox from './more-info-box'
 
@@ -33,15 +34,21 @@ class Header extends React.Component {
 	}
 
 	saveId(oldId, newId) {
+		if (oldId === newId) return
+
 		const model = OboModel.models[oldId]
 
 		if (!newId) {
-			return 'Please enter an id'
+			return 'Please enter an id.'
+		}
+
+		if (!isValidId(newId)) {
+			return 'Invalid characters in id. Only letters, numbers, and special characters (-, _, :, .) are permitted.'
 		}
 
 		// prettier-ignore
 		if (!model.setId(newId)) {
-			return 'The id "' + newId + '" already exists. Please choose a unique id'
+			return 'The id "' + newId + '" already exists. Please choose a unique id.'
 		}
 
 		EditorUtil.rebuildMenu(OboModel.getRoot())