Skip to content

UCF: Walkthrough of the Training Issue

Jump to bottom
Zachary Berry edited this page Apr 29, 2021 · 1 revision

This text will roughly follow the Obojobo Training Issue, which touches briefly on a multitude of Obojobo's mechanics.

Start the Dev Environment

Follow the instructions on the Obojobo GitHub Repository's Main Page to get the Obojobo dev environment running and log in - there's a lot of stuff that goes on in those few steps, but for the purposes of doing this quick and simple update it's not super important. Do the branch checkout etc. as well (following the guidelines in the Obojobo Contributing Wiki page) then start following the instructions. Follow also the Obojobo Style Guidelines.

It might not hurt to also run yarn:test prior to doing anything just to establish that if anything breaks it's absolutely your fault.

So You Want a Button in the Module Viewer's Left-Hand Navigation Area

Like the issue supposes, you want to add a button to the left-hand nav area of the module viewer that turns the nav red to indicate some kind of 'red alert' status, and we also want to log when it's turned on and when it's turned off - so we need to be able to write a log to the database and persist the 'red alert' status across page loads. We also need to make sure unit test coverage doesn't fall below 100%.

There are instructions specifying what to put where, but let's pretend they don't exist. Assume only baseline knowledge: React and Flux for the frontend, NodeJS + Express for the backend, PostgreSQL for the database, Jest for unit testing (both ends). Every action generates an event and every event is logged forever in the database for analytical purposes.

The first problem to solve is finding the code that generates the nav area. Obojobo source code (not counting necessary configuration or descriptive code contained in the root directory) is all contained in the packages directory. It is further divided up according to the purpose of the code - app for application code, obonode for OboNode definitions, where the code lives that is used to render each available content type in both the document editor and the document viewer or, sometimes, to handle server-side interactions (read more at the Obojobo docs), and util for utility functions.

We want to add a button to the nav area, so we need to find the code for the component - it's not one of the nodes available in the editor so it probably won't be with the rest of the OboNode code in obonode, and nothing in util is being used in the source code, so it's going to be somewhere in app. That doesn't narrow it down a whole lot, though.

app has six further divisions in it, as follows:

  • obojobo-document-engine: Contains code pertaining to the viewer and editor applications.
  • obojobo-document-json-parser: Contains code pertaining to translating JSON objects into the Obojobo document engine.
  • obojobo-document-xml-parser: Contains code pertaining to translating XML objects into the Obojobo document engine.
  • obojobo-express: Contains code pertaining to the vast majority of the Obojobo backend - API endpoints, database migrations and abstraction models, views, etc.
  • obojobo-module-selector: Contains code pertaining to LTI learning module lookup and selection interface.
  • obojobo-repository: Contains code pertaining to the Obojobo learning module repository, including front end components, backend routes and API endpoints, and database migrations and abstraction models.

Given the basic understanding of what's in all of those, probably the navigation area is somewhere in obojobo-document-engine. There's an index.js file here that exports two main things: the 'viewer' and the 'editor'. We're looking for the nav area in the viewer, so look further in the src/scripts/viewer folder to find the component that renders the nav area.

Now we're in packages/app/obojobo-document-engine/src/scripts/viewer, where there are numerous files and subdirectories. The index.js here will give a rough idea of what's going on - but since we're looking for the component rendering the navigation area, look at the 'components' part of the export of this file. It's got four things in it: ViewerApp, Logo, Header, and OboComponent. It's probably in one of those. Looking at each of the files being required to generate those, eventually it's pretty plain that the only one that makes sense is ViewerApp.

ViewerApp is exported from components/viewer-app.js. Looking at the imports in that, we can see a Nav, NavStore, and NavUtil. All three seem like they'll be important, but Nav is probably where the button itself will go.

components/nav.js is where the component code is - this is where the actual clickable button needs to be implemented. Throw a <Button>RED ALERT</Button> anywhere in there, zip boom pow new button on the page.

STEP ONE COMPLETE.

We want the nav area to turn red to indicate whether it's currently in 'red alert' mode. Per the code guidelines, every class indicating some kind of state must also have a class indicating the opposite of that state - so our is-red-alert class will need a counterpart is-not-red-alert class. Luckily the isOrNot utility method will do this for us, so don't worry too much about it. All of the classes being applied to a component need to be in a single string that's passed to the rendered element's className property.

Classes for components are stored right next to the components, so adjacent to nav.js there should be a nav.scss. Per the style guidelines, a style to indicate the 'red alert' state would be is-red-alert. Per the stylelint rules, colors have to be stored in variables. Ultimately, we'll probably end up with something like:

...

.viewer--components--nav {
	$color-nav-bg-red-alert: #ff0000;
...
	&.is-red-alert {
		background-color: $color-nav-bg-red-alert;
	}
...

We don't need any rules for is-not-red-alert because there won't be any visual change to indicate it.

In the component's render method, all of the classes that should be applied to the component are stored in a constant named className - so we'll need to add the is-red-alert class to that list somehow. It looks like all of the other variable classes are coming out of the navState prop being passed into the component, so this should probably come from there as well. Inside the render method for the ViewerApp component (the parent component, which we found earlier), we can see that Nav is passed the navState prop, which itself is ViewerApp's state.navState, which is being initialized in ViewerApp's constructor method (we can also see that it's being reset any time onNavStoreChange is called, and onNavStoreChange is being passed as a callback to NavStore.onChange, but we'll have to get to that later). It's being given a value in componentDidMount after a few asynchronous calls.

First thing that's happening is a call to ViewerAPI.requestStart. ViewerAPI is defined in obojobo-document-engine/src/scripts/viewer/util/viewer-api.js, and handles communication between the document viewer and the backend. The specifics of its requestStart method aren't really important right now, since all we care about is adding a button to the nav bar that toggles 'Red Alert' mode. But for posterity, it makes a call to 'api/visits/start' - the code that runs at that point can be found in obojobo-express/routes/api/visits.js. What's important is that it provides an object corresponding to this visit to this draft of this document.

Anyway, some stuff happens after the visit object is received. The next method in the promise chain that runs is ViewerAPI.getDraft - again, not really super important what happens there yet, but it provides an object corresponding to the document draft with the provided ID. After that succeeds, NavStore.init is run - a bit further down in the code, ViewerApp's state.navState is set to the result of NavStore.getState, so it's probably time to look at NavStore.

NavStore can be found at obojobo-document-engine/src/scripts/viewer/stores/nav-store.js. NavStore is a Flux store. There's a lot going on with Flux, but for now it's easiest to think of it as a way of managing the state of a thing according to events that occur that are relevant to that thing, and potentially making the state of that thing available to components without cluttering those components too much with logic for handling that thing's state.

To catch up a bit, recall that we saw NavStore.onChange being fed a callback that alters ViewerApp.state.navState. NavStore doesn't have its own onChange method, but the class it extends (Common/Flux/Store) does. We can see that in obojobo-document-engine/src/scripts/common/flux/store.js. onChange basically just assigns the given callback to run whenever Dispatcher receives a change event, which is likely going to be fired by the adjacent triggerChange method.

Likewise, NavStore.getState isn't really a method on NavStore, but on Common/Flux/Store - it just returns the store's state.

All of that exploration was to figure out where Nav's navState prop was coming from and to figure out how to add another state property to track the nav's 'red alert' status. In NavStore.init, we'll need to add a property to keep track of whether the red alert status is enabled or not. Presumably we'll be initializing this new property with the rest of the draft/visit data, too. But set it to false by default just in case whatever's initializing the store doesn't have the red alert status.

class NavStore extends Store {
...
	init(draftId, model, startingId, startingPath, visitId, viewState = {}, isRedAlertEnabled = false) {
		this.state = {
			items: {},
			redAlert: isRedAlertEnabled
...

Now that we have a state property that we can read in the Nav component, we can go back to the component and assign a class based on it. But first, it's a good idea to add a method to NavUtil that can tell us what the red alert status is. At the moment it's pretty simple - it's a boolean on the NavStore state, not super complicated - but the idea is to extract any potential future logic for determining 'is red alert on; yes or no' out of the controller, so this method would save the trouble in the future.

const NavUtil = {
...
	isRedAlertEnabled(state) {
		return state.redAlert
	}
...

Then we just have to pass navState into that method and assign its return to a value for use.

export default class Nav extends React.Component {
...
	render() {
...
		const isRedAlertEnabled = NavUtil.isRedAlertEnabled(navState)
		const className =
			'viewer--components--nav' +
...
			isOrNot(isRedAlertEnabled, 'red-alert')
...

Just keep in mind that this is a string, so don't leave any hanging plus signs.

So now we have a button on the page, NavStore has a state property to keep track of red alert status, and (presumably) classes are being applied correctly based on the value of it - now we need to rig up the button to tell the store to change the red alert status. We can see elsewhere in Nav that there are numerous methods in NavUtil that interpret the state from NavStore and make changes to it, so NavUtil is probably the most prudent place for a method that changes the value of NavStore.state.redAlert.

Call it setRedAlert per code guidelines, and have it take in a boolean value we want the red alert status to be. To change the state we'll need to trigger an event in the dispatcher and pass a value to it, like:

const NavUtil = {
...
	setRedAlert(redAlert) {
		return Dispatcher.trigger('nav:setRedAlert', {
			value: {
				redAlert
			}
		})
	}
...

We also need to add an onClick listener to the button we made in the Nav component to actually run this code. Since we have to pass an argument to the new method, we'll need a method that calls the new NavUtil method which is itself triggered by clicking the button.

In the interest of not defining a whole new function every time the nav element is re-rendered, attach a method to the class that we can use instead. Per code guidelines, it should probably be called onRedAlertClick. In order to make it usable, you'll have to bind it to the class in the constructor, like so:

	constructor(props) {
		super(props)
...
		this.onClickRedAlert = this.onClickRedAlert.bind(this)
	}

And then define it elsewhere like:

	onClickRedAlert() {
		const isRedAlertEnabled = NavUtil.isRedAlertEnabled(this.props.navState)
		NavUtil.setRedAlert(!isRedAlertEnabled)
	}

Then add the onClick handler to the button:

<Button onClick={this.onClickRedAlert}>

This won't do anything unless there's a corresponding event for the dispatcher to listen for, though. In NavStore, look for the part of the code where all the dispatcher's events are defined, and add a new one for the nav:setRedAlert event we're planning on using.

class NavStore extends Store {
...
		Dispatcher.on(
			{
				'nav:setRedAlert': payload => {
					this.state.redAlert = payload.value.redAlert
					return this.triggerChange()
				},
...

Now clicking the button we added will toggle the background between red and white. Woo.

STEP TWO COMPLETE.

So we've added a button to the navigation area and clicking it toggles red alert mode on and off, but we're not logging these toggle events and we're not persisting red alert mode between page loads. In order to do both, we need to send a message to the backend whenever the button is clicked.

NavStore is where we're updating the state locally, and if we look around the class we can see it using ViewerAPI to communicate with the backend as well. Just modify the dispatch event handler that we wrote earlier. Every event needs a draft ID, a visit ID, an action version, and an action for the backend to hitch onto. Payloads for events are optional, but since this event is meant to change something it'll need a payload. Indicate that we're switching the red alert status from true to false or vice versa:

'nav:setRedAlert': payload => {
	ViewerAPI.postEvent({
		draftId: this.state.draftId,
		action: 'nav:setRedAlert',
		eventVersion: '1.0.0',
		visitId: this.state.visitId,
		payload: {
			from: this.state.redAlert,
			to: payload.value.redAlert
		}
	})
	this.state.redAlert = payload.value.redAlert
	this.triggerChange()
}

Looking at ViewerAPI.postEvent we can see that it's posting to '/api/events'. Long story short, that's writing rows to the 'events' table in the DB - use a PostgreSQL client to look at your local database and watch the 'events' table to make sure those events are being recorded. The actual code running that API endpoint can be seen at packages/app/obojobo-express/routes/api/events.js.

STEP THREE COMPLETE.

Before we can do anything else, we need a place in the database to store the red alert status of a draft for a given person. While typically this would be attached to something that exists already (probably view_state), for demonstration purposes we'll just make a whole new table. We'd need a migration regardless. Luckily, obojobo-express has a command-line helper tool that assists with the creation of migrations. In an unoccupied terminal tab, cd into projects/app/obojobo-express and run yarn db:createmigration create-red-alert-status. The output of this will indicate where the newly created migration file was placed - open it up. Previous migrations can be examined for ideas of how to write this one, but it'll look like:

'use strict'

let dbm
let type
let seed

/**
  * We receive the dbmigrate dependency from dbmigrate initially.
  * This enables us to not have to rely on NODE_PATH.
  */
exports.setup = function(options, seedLink) {
  dbm = options.dbmigrate
  type = dbm.dataType
  seed = seedLink
}

exports.up = function(db) {
	return db
		.createTable('red_alert_status', {
			id: {
				type: 'UUID',
				primaryKey: true,
				defaultValue: new String('uuid_generate_v4()')
      },
      created_at: {
				type: 'timestamp WITH TIME ZONE',
				notNull: true,
				defaultValue: new String('now()')
			},
			user_id: { type: 'bigint', notNull: true },
      draft_id: { type: 'UUID', notNull: true },
      is_enabled: { type: 'boolean', notNull: true }
    })

		.then(result => {
			return db.addIndex('red_alert_status', 'ras_user_id_index', ['user_id'])
		})
		.then(result => {
			return db.addIndex('red_alert_status', 'ras_draft_id_index', ['draft_id'])
		})
		.then(() =>
			db.addIndex(
				'red_alert_status',
				'user_draft_unique',
				['user_id', 'draft_id'],
				true
			)
		)
}

exports.down = function(db) {
	return db.dropTable('red_alert_status')
}

exports._meta = {
  'version': 1
}

This'll create a table named 'red_alert_status' with columns 'id', 'created_at', 'user_id', 'draft_id', and 'is_enabled' with indices on the 'user_id' and 'draft_id' columns and a requirement that no two rows have the same 'user_id' and 'draft_id'. Make sure the database docker machine is running and then run yarn db:migrateup from the obojobo-express folder. Assuming nothing went wrong, now we'll have a table to persist our red alert states in.

STEP FOUR COMPLETE.

Now events are being logged and we have a database to persist the red alert status between page loads - all we have to do is rig up the back and front ends to save and load the red alert status correctly. If we look at packages/app/obojobo-express/routes/api/events.js we can see what code runs when saving an event to the database - we can also see that after an event is successfully recorded, oboEvents.emit runs. There's a bit of middleware running on the Obojobo Express backend that's a bit wacky between this file and oboEvents, but for the sake of simplicity treat it kind of the same way as the dispatcher in the frontend code - any time an event is stored in the database, a corresponding client: event is emitted - if a listener for the client: event is defined elsewhere (also using oboEvents), it'll pick up that event and do something else with it.

It so happens that there's already some code that handles events from the viewer - packages/app/obojobo-express/viewer_events.js.

We'll have to add a new listener for the client:nav:setRedAlert event. It'll look like:

oboEvents.on('client:nav:setRedAlert', event => {
	const userId = event.userId
	const draftId = event.draftId
	const isRedAlertEnabled = event.payload.to
	return db
		.none(
			`
				INSERT INTO red_alert_status
				(user_id, draft_id, is_enabled)
				VALUES($[userId], $[draftId], $[isRedAlertEnabled])
				ON CONFLICT (user_id, draft_id) DO UPDATE
				SET is_enabled = $[isRedAlertEnabled]
				WHERE red_alert_status.user_id = $[userId] AND
				red_alert_status.draft_id = $[draftId]
			`,
			{
				userId,
				draftId,
				isRedAlertEnabled
			}
		)
		.catch(error => {
			logger.error('DB UNEXPECTED on red_alert_status.set', error, error.toString())
		})
})

This will be using a couple of things that are not currently available in this file, but we just have to require them at the top:

const db = oboRequire('server/db')
const logger = oboRequire('server/logger')

Now when we click the button in the frontend, we should either be seeing a row added to the red_alert_status table, or modified to match whatever the current status of the red alert mode is in the frontend. The last thing to do is make sure that when the page reloads, it gets the current red alert status from the backend.

A while back, we saw that NavStore is having its state initialized inside the componentDidMount method of the ViewerApp component in obojobo-document-engine. Go back to packages/app/obojobo-document-engine/src/scripts/viewer/components/viewer-app.js and look in componentDidMount to see the chain of API calls that grab all of the data necessary for the document viewer. Two steps occur before NavStore is fed data - one call to requestStart and one call to getDraft. ViewerAPI.requestStart is making a call to the /api/visits/start endpoint and providing the draft ID - that's as good a place as any to look up the red alert status of the provided draft for the current user and pass it back to the front end.

The API code running at that endpoint is in packages/app/obojobo-express/routes/api/visits.js. There are a few ways we could approach adding the red alert status to what's passed back to the front end, but the easiest thing would probably be to track it in a new variable and just add a new promise to the beginning of the chain that queries the database. We can't be sure that there will be a record in the database for this draft/user combo, so default the new variable to false and make sure we only try to set it if we actually get a result from the database. Don't forget to also set a new constant userId to store the ID of the current user for easy use in the query.

router
	.route('/start')
	.post([requireCurrentUser, requireCurrentDocument, requireVisitId, checkValidationRules])
	.post((req, res) => {
		let isRedAlertEnabled = false
...
		const userId = req.currentUser.id
...
		return req
			.getCurrentVisitFromRequest()
			.catch(() => {
				throw 'Unable to start visit, visitId is no longer valid'
			})
			.then(() => db
				.oneOrNone(
					`
						SELECT is_enabled FROM red_alert_status
						WHERE
							user_id = $[userId]
							AND draft_id = $[draftId]
					`,
					{
						userId,
						draftId,
					}
				)
			)
			.then(result => {
				if (result) isRedAlertEnabled = result.is_enabled
			})

This file wasn't previously performing any queries so we'll also have to require the db class at the top of the file.

const db = oboRequire('server/db')

Then we just have to remember to add it to the payload being sent back.

				res.success({
					isRedAlertEnabled,
...

That should take care of the backend. Now we need to go back to the frontend and make sure we track the new data we're getting from ViewerAPI.requestStart.

	componentDidMount() {
...
		let isRedAlertEnabled
...
		ViewerAPI.requestStart(this.props.visitId, this.props.draftId)
			.then(visit => {
...
				isRedAlertEnabled = visit.value.isRedAlertEnabled
...

And then pass it into NavStore.init

				NavStore.init(
					OboModel.getRoot().get('draftId'),
					model,
					model.modelState.start,
					window.location.pathname,
					visitIdFromApi,
					viewState,
					isRedAlertEnabled
				)

And now we have a button on the left-hand document viewer that toggles red alert status for that draft for the current user, logs an event every time it's toggled, and persists the status of the toggle between page loads. Woo.

STEP FIVE COMPLETE.

We've added a button to the left-hand navigation area of the document viewer that, when clicked, will toggle a 'red alert' mode on and off, the status of which will persist between page loads and log an event each time it's toggled. The last thing to do is visit the unit tests, test the new code, and make sure coverage is still at 100% everywhere.

Running yarn test --watchAll in the root directory for the project will run all of the unit tests and watch files for changes so we can work on the broken tests until they're no longer broken. While it's still in watch mode, we can press the Enter/Return key to manually re-run the tests - but it should automatically run the relevant tests again any time we update one of the test files. Also Obojobo is a pretty big project with a whole lot of tests, so expect it to take a while to run them all.

Tests for any of the five app packages are usually located in the __tests__ directory for that package, and subdirectory structures will typically be mirrored within __tests__. Based on our changes, it looks like 4 test suites and 29 individual tests are now failing, along with 5 snapshot checks. From the top, you might see some complaints coming from the React test renderer - ignore those, focus on the first test failure:

 FAIL  packages/app/obojobo-express/__tests__/routes/api/visits.test.js

A little further down, and we can see the specific test that's failing in that file:

`/start in preview works and doesnt care about matching draft_content_ids`

And under that, it'll show us where specifically in the file that test is and why it's failing: Line 270, because a snapshot doesn't match what we're getting out of the code. We may as well make any necessary snapshot updates first, just to get them out of the way before we start adjusting tests.

We can press the 'i' key to run the tests in interactive snapshot update mode - rather than updating every snapshot automatically, it'll run the tests with snapshot failures and give us the option to address each snapshot mismatch one at a time.

NOTE: I've encountered strange inconsistencies in which snapshot mismatches come up first while in interactive mode. I'm not sure what causes this or how to fix it, but in spite of the order being unreliable it does still appear to hit all of the mismatched described below.

Do that, and we'll come to the first snapshot failure. It's failing because the snapshot is missing the new isRedAlertEnabled property that we added - doesn't seem like it would be unsafe to update this snapshot with that one change. Update this snapshot to match the new object by pressing the 'u' key. Do that, and it'll move on to the next test with a snapshot failure.

The next snapshot failure is in packages/app/obojobo-document-engine/__tests__/Viewer/stores/nav-store.test.js: Regisers events w/ dispatcher, line 47. This one is failing because the snapshot is missing the new event listener we added to the NavStore dispatcher. Update that and move on.

The next snapshot failure is in that same file: init builds state with basic options. This one's failing because the object coming out of the NavStore.getState snapshot doesn't have a value corresponding to the redAlert property that we added to NavStore.init. Update that one to move onto the next.

Still in the same file, next failure: init builds state locked state. Same problem as the last state mismatch, update it and move on.

The last snapshot failure is still in that file: init builds state open state. Looks like it's the same as the last two, as well. Update it and we'll be informed that we've reviewed and updated all 5 snapshot errors. Press the 'Enter' or 'Return' key here to go back to regular watch mode, with no more snapshot failures.

Keep in mind where all of those failing tests were - the tests were set up to make sure the results of NavStore.init being run with various inputs matched expectations, using snapshots to simplify the comparisons. When all the test failures have been addressed we'll have to write new tests for our changes, and one of the things

For now, we'll be seeing some more test failures. The next one from the top:

 FAIL  packages/app/obojobo-document-engine/__tests__/Viewer/components/nav.test.js

There are numerous tests failing here, but a quick glance suggests that they're all failing for the same reason. The test runner will show us line 203 in the Nav component, where we're checking NavUtil.isRedAlertEnabled. The runner is complaining that there's no such function, even though we definitely added it. The actual problem is that NavUtil is being mocked on line 50 of the test file, and the mock doesn't include a isRedAlertEnabled function. To start, add one to the mock:

// NavUtil
jest.mock('../../../src/scripts/viewer/util/nav-util', () => ({
	canNavigate: jest.fn(),
	gotoPath: jest.fn(),
	toggle: jest.fn(),
	getOrderedList: jest.fn(),
	getNavTarget: jest.fn(),
	close: jest.fn(),
	open: jest.fn(),
	isRedAlertEnabled: jest.fn(),
	setRedAlert: jest.fn()
}))

We also added setRedAlert to save us having to go back and add it separately later, when we're writing tests to check the new code.

Save the file and the tests will run automatically, this time with fewer failures. Except it looks like there are new snapshot failures that need to be worked out. Follow the same procedure as before - press the 'i' key to start addressing them one at a time.

Luckily all of the mismatches we're about to see are pretty straightforward. Two new things have been added to the Nav component - the nav element itself has one of two new classes, is-red-alert or is-not-red-alert, and there's a whole new Button component attached to it. The description of the snapshot differences match our expectations, so proceed through each mismatch and update them individually then return to watch mode afterwards.

NOTE: I encountered some weird issues where it complained about snapshots not matching, but when going through the interactive update process it wouldn't identify any mismatches to correct. I had to correct the snapshots by hand before it would let me move on.

It looks like there are no more failures - but that's a lie. Press the 'a' key to run all tests again, after which we'll have one more test failure to deal with.

 FAIL  packages/app/obojobo-express/__tests__/viewer_events.test.js

In this case, it's failing on line 37 - where it expects oboEvents.on to have been called twice, it's now being called three times. If we look at packages/app/obojobo-express/viewer_events.js we can see why - we added the client:nav:setRedAlert event. So our first addition to the tests here is to make sure oboEvents.on was called to set up the new event, and to change how many times we expect it to be called.

	test('registers expected events', () => {
...
		expect(oboEvents.on).toBeCalledWith('client:nav:setRedAlert', expect.any(Function))
		expect(oboEvents.on).toHaveBeenCalledTimes(3)

After that last fix, there won't be any more unit test failures.

STEP SIX COMPLETE.

The last thing to do is adjust the tests to cover everything we added.

We can generate coverage documentation by running yarn test --coverage --coverageReporters=lcov --watchAll. Once this finishes the first time, a coverage document will be available at obojoboRootDirectory/coverage/lcov-report/index.html. This'll make it easy to see which files are being covered and by how much.

NOTE: Sometimes the 'coverage' and 'watch' functionalities don't play nicely together - if anything starts acting strangely, restarting the tests seems to reliably get everything back to normal.

May as well go from top to bottom. The first place where coverage isn't at 100% is app/obojobo-document-engine/src/scripts/viewer/components. Click that link and it'll give us more details on the files contained in that folder. Here we can see that only nav.js isn't 100% covered. Click that link and it'll show us that the onClickRedAlert method is not being run at all. Keep in mind also that we need to test other things that are technically already covered - we need to make sure the is-red-alert class is applied correctly, for instance.

Since that one's more forgettable, let's handle it before it gets away from us forever. The test covering the Nav component can be found at packages/app/obojobo-document-engine/__test__/Viewer/components/nav.test.js. Recall from earlier that we saw a few places where the component was rendered and compared based on certain inputs. That's an appropriate location to make sure the red alert classes are being applied, like this:

	test('renders red alert classes appropriately (redAlert=true)', () => {
		const redAlert = true
		NavUtil.getOrderedList.mockReturnValueOnce(navItems)
		NavUtil.isRedAlertEnabled.mockReturnValueOnce(redAlert)
		const props = {
			navState: {
				redAlert
			}
		}
		const component = renderer.create(<Nav {...props} />)
		const tree = component.toJSON()
		expect(tree.props.className).toEqual(expect.stringContaining('is-red-alert'))
		expect(tree).toMatchSnapshot()
	})

	test('renders red alert classes appropriately (redAlert=false)', () => {
		const redAlert = false
		NavUtil.getOrderedList.mockReturnValueOnce(navItems)
		NavUtil.isRedAlertEnabled.mockReturnValueOnce(redAlert)
		const props = {
			navState: {
				redAlert
			}
		}
		const component = renderer.create(<Nav {...props} />)
		const tree = component.toJSON()
		expect(tree.props.className).toEqual(expect.stringContaining('is-not-red-alert'))
		expect(tree).toMatchSnapshot()
	})

With that out of the way, now we need to add a test to make sure the new onClickRedAlert function we wrote is behaving properly. Luckily it's as easy as making sure the correct NavUtil methods are being called with the correct arguments:

	test('onClickRedAlert calls NavUtil.isRedAlertEnabled and NavUtil.setRedAlert', () => {
		const redAlert = true
		NavUtil.getOrderedList.mockReturnValueOnce(navItems)
		NavUtil.isRedAlertEnabled.mockReturnValueOnce(redAlert)
		const props = {
			navState: {
				redAlert
			}
		}
		const component = mount(<Nav {...props} />)
		component.instance().selfRef = {
			current: {
				contains: () => false
			}
		}
		component.instance().onClickRedAlert()
		expect(NavUtil.isRedAlertEnabled).toHaveBeenCalled()
		expect(NavUtil.setRedAlert).toHaveBeenCalledWith(redAlert)
	})

With that done, coverage for the Nav element is back up to 100% and our tests are making sure our new classes are applied

The next thing down the list that's not fully covered any more is app/obojobo-document-engine/src/scripts/viewer/stores, specifically nav-store.js. Predictably, the new dispatcher event nav:setRedAlert is not being tested yet. The NavStore tests can be found in packages/app/obojobo-document-engine/__tests__/Viewer/stores/nav-store.test.js. We need to make sure that the API payload matches what we expect the dispatcher's callback to send, and that the store's state is set accordingly. This test can technically go anywhere in the file, but it makes the most sense to put it with the rest of the event tests - probably after the question:scoreSet sets flag with a score of 100 test.

	test('nav:setRedAlert event fires and updates state', () => {
		const redAlert = false
		NavStore.setState({
			redAlert
		})
		// simulate trigger
		Dispatcher.trigger.mockReturnValueOnce()

		// go
		eventCallbacks['nav:setRedAlert']({ value: { redAlert: !redAlert } })
		expect(Dispatcher.trigger).toHaveBeenCalledTimes(1)
		expect(Dispatcher.trigger.mock.calls[0]).toMatchSnapshot()
		expect(ViewerAPI.postEvent).toHaveBeenCalledTimes(1)
		expect(ViewerAPI.postEvent.mock.calls[0]).toMatchSnapshot()
		const payloadToAPI = ViewerAPI.postEvent.mock.calls[0][0].payload
		expect(payloadToAPI).toEqual({ from: redAlert, to: !redAlert })
		expect(NavStore.getState()).toMatchSnapshot()
	})

The next item in the list below 100% is app/obojobo-document-engine/src/scripts/viewer/util: nav-util.js. The two new methods we added, isRedAlertEnabled and setRedAlert both need testing. Like usual, the tests are in packages/app/obojobo-document-engine/__tests__/Viewer/util/nav-util.test.js.

NavUtil.isRedAlertEnabled is pretty simple - it returns the redAlert property of the state object passed into it:

	test('isRedAlertEnabled returns the red alert status from a state object', () => {
		const redAlert = true
		const navState = {
			redAlert
		}
		const isRedAlertEnabled = NavUtil.isRedAlertEnabled(navState)
		expect(isRedAlertEnabled).toBe(redAlert)
	})

NavUtil.setRedAlert is only a little more complicated - we need to make sure the dispatcher was triggered with the right payload based on its input:

	test('setRedAlert calls Dispatcher.trigger', () => {
		const redAlert = true
		NavUtil.setRedAlert(redAlert)

		expect(Common.flux.Dispatcher.trigger).toHaveBeenCalledWith('nav:setRedAlert', {
			value: { redAlert }
		})
	})

The next thing is app/obojobo-express:viewer_events.js, where we now need to test the new client:nav:setRedAlert event listener. Look up packages/app/obojobo-express/__tests__/viewer_events.test.js

Before we actually write the new test, we're going to have to both require and mock db and logger since our new event is going to be talking to one or the other, and we need to test both possibilities. We'll also want to make sure db.none (the db function we call when inserting our new red alert record) and logger.error (the logger function we call if something goes wrong inserting our new red alert record) mocks are reset before each test runs.

jest.mock('../db')
jest.mock('../logger')
...
let db
let logger
...
describe('viewer events', () => {
...

	beforeEach(() => {
		jest.resetAllMocks()
		jest.resetModules()
		db = oboRequire('server/db')
		logger = oboRequire('server/logger')
...

Now to write two tests - one to make sure the DB insert is being fed the correct inputs:

	test('client:nav:setRedAlert (insert success)', () => {
		expect.hasAssertions()

		db.none.mockResolvedValue(null)

		ve = oboRequire('viewer_events')
		const [eventName, callback] = oboEvents.on.mock.calls[2]
		expect(eventName).toBe('client:nav:setRedAlert')
		expect(callback).toHaveLength(1)

		return callback(mockRedAlertEvent).then(() => {
			expect(db.none).toHaveBeenCalledTimes(1)
			expect(logger.error).toHaveBeenCalledTimes(0)
			expect(db.none).toBeCalledWith(
				expect.stringContaining('INSERT INTO red_alert_status'),
				expect.objectContaining({
					userId: mockRedAlertEvent.userId,
					draftId: mockRedAlertEvent.draftId,
					isRedAlertEnabled: true
				})
			)
		})
	})

...and one to make sure the logger is being given the right values for an error if the DB insert fails:

	test('client:nav:setRedAlert (insert error)', () => {
		expect.hasAssertions()

		const message = 'error message'

		db.none.mockRejectedValue(message)

		ve = oboRequire('viewer_events')
		const [eventName, callback] = oboEvents.on.mock.calls[2]
		expect(eventName).toBe('client:nav:setRedAlert')
		expect(callback).toHaveLength(1)

		return callback(mockRedAlertEvent).then(() => {
			expect(db.none).toHaveBeenCalledTimes(1)
			expect(logger.error).toHaveBeenCalledTimes(1)
			expect(db.none).toBeCalledWith(
				expect.stringContaining('INSERT INTO red_alert_status'),
				expect.objectContaining({
					userId: mockRedAlertEvent.userId,
					draftId: mockRedAlertEvent.draftId,
					isRedAlertEnabled: true
				})
			)
			expect(logger.error).toBeCalledWith('DB UNEXPECTED on red_alert_status.set', message, message)
		})
	})

All that's left is to test the new code that gets the current red alert status of a draft for a user when a visit starts. That'll be in packages/app/obojobo-express/__tests__/api/visits.test.js For completeness we'll need two tests here, too. One to make sure the default red alert status is sent back to the front end when the database query doesn't return any results:

	test('/start uses default red alert status of current user/draft when DB does not return one', () => {
		expect.hasAssertions()
		const originalVisitSession = {}
		mockSession.visitSessions = originalVisitSession

		mockCurrentUser = { id: 99 }
		// resolve ltiLaunch lookup
		const launch = {
			reqVars: {
				lis_outcome_service_url: 'obojobo.com'
			}
		}
		ltiUtil.retrieveLtiLaunch.mockResolvedValueOnce(launch)

		//this solves coverage but is not satisfying
		db.oneOrNone.mockResolvedValue(null)

		// resolve viewerState.get
		viewerState.get.mockResolvedValueOnce('view state')
		mockCurrentDocument = {
			draftId: validUUID(),
			yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
			contentId: validUUID()
		}
		return request(app)
			.post('/api/start')
			.send({ visitId: validUUID() })
			.then(response => {
				expect(response.statusCode).toBe(200)
				expect(response.body.value).toHaveProperty('isRedAlertEnabled', false)
			})
	})

...and one to make sure that if the database returns a value, that one is sent to the front end:

	test('/start gets red alert status of current user/draft from DB', () => {
		const isRedAlertEnabled = true
		expect.hasAssertions()
		const originalVisitSession = {}
		mockSession.visitSessions = originalVisitSession

		mockCurrentUser = { id: 99 }
		// resolve ltiLaunch lookup
		const launch = {
			reqVars: {
				lis_outcome_service_url: 'obojobo.com'
			}
		}
		ltiUtil.retrieveLtiLaunch.mockResolvedValueOnce(launch)

		//this solves coverage but is not satisfying
		db.oneOrNone.mockResolvedValue({is_enabled: isRedAlertEnabled})

		// resolve viewerState.get
		viewerState.get.mockResolvedValueOnce('view state')
		mockCurrentDocument = {
			draftId: validUUID(),
			yell: jest.fn().mockResolvedValueOnce({ document: 'mock-document' }),
			contentId: validUUID()
		}
		return request(app)
			.post('/api/start')
			.send({ visitId: validUUID() })
			.then(response => {
				expect(db.oneOrNone).toBeCalledTimes(1)
				expect(db.oneOrNone).toBeCalledWith(
					expect.stringContaining('SELECT is_enabled FROM red_alert_status'),
					expect.objectContaining({
						userId: mockCurrentUser.id,
						draftId: mockCurrentDocument.draftId
					})
				)
				expect(response.statusCode).toBe(200)
				expect(response.body.value).toHaveProperty('isRedAlertEnabled', isRedAlertEnabled)
			})
	})

You'll also need to make sure the mocks for db.oneOrNone are reset before each test is run, so we don't have any interference between tests:

...
	beforeEach(() => {
		db.oneOrNone.mockReset()
...

If you haven't already, run all the unit tests with coverage again - we should now be back up to 100%, with all of our new code being tested and covered.

With that all done, go through the usual git add/commit process to run Obojobo's pre-commit hooks - which will check all of the modified files to make sure they match the style guidelines and also run all the tests to make sure they pass and meet coverage requirements.

If all of those work/pass, congratulations. You're done.

UCF Open Logo

Obojobo is a UCF open source project.

Documentation || Obojobo on Github || About UCF Open

Clone this wiki locally