New: added migration scripts

[danielghost]

Fixes #80.

New

• Added migration scripts.

[danielghost]

Look to add helper methods to https://github.com/adaptlearning/adapt-migrations/blob/master/api/helpers.js.

[joe-allen-89]

A few changes from best practise from other repos and a couple of linting issues. @danielghost the majority of changes are the same across the other repos you've picked up, do you want me to leave the others until you've had chance to make the changes?

[joe-allen-89]

Whilst these work perfectly, after discussing helper functions we've gone with using lodash as the standard with these functions already built in, we can therefore use _.has and _.set to save ourselves adding helper functions. I'll not raise this for each file.

Suggested change

	function hasKey(object, key) {
	if (!object) return false;
	return Object.hasOwn(object, key);
	}
	function setObjectPathValue(object, path, value, force = false) {
	if (!object) return;
	const paths = path.split('.');
	const key = paths.pop();
	const target = paths.reduce((o, p) => {
	if (!hasKey(o, p)) o[p] = {};
	return o?.[p];
	}, object);
	if (!force && hasKey(target, key)) return;
	target[key] = value;
	}
	import _ from 'lodash'

[danielghost]

See #81 (comment) to confirm we prefer to go this route.

[joe-allen-89]

Suggested change

	updatePlugin('adapt-contrib-text - update to v2.0.1', {name: 'adapt-contrib-text', version: '2.0.1', framework: '>=2.0.0'})
	updatePlugin('adapt-contrib-text - update to v2.0.1', { name: 'adapt-contrib-text', version: '2.0.1', framework: '>=2.0.0' })

[joe-allen-89]

As mentioned we'll be changing to _.has (I'll not raise a comment for every instance),

Suggested change

	return !hasKey(course?._globals?._components?._text, 'ariaRegion');
	return !_.has(course, '_globals._components._text.ariaRegion');

[joe-allen-89]

Again I'll not raise one for every instance but update setObjectPathValue to _.set

Suggested change

	setObjectPathValue(course, '_globals._components._text.ariaRegion', '');
	_.set(course, '_globals._components._text.ariaRegion', '');

[danielghost]

If an author has changed the value from the default, we shouldn't apply the new default value. This would mean adding extra conditions before the set call, whereas setObjectPathValue takes care of this automatically:

adapt-contrib-text/migrations/v6.js

Line 16 in 7d25310

if (!force && hasKey(target, key)) return;

Would we rather add these extra conditions in and use lodash instead? There could be many instances of needing this check throughout all the plugin migrations:

if (!_.has(course, '_globals._components._text.ariaRegion')) _.set(course, '_globals._components._text.ariaRegion', '');

Switching to use the lodash method works in the v2 migration as the whereContent checks to see if the attribute is present first and returns early if so, but the v6 migration doesn't have this because it potentially needs to do this additional step regardless of whether the new attribute is present:

adapt-contrib-text/migrations/v6.js

Line 29 in 7d25310

delete course?._globals?._text;

[oliverfoster]

Original discussion: adaptlearning/adapt-migrations#28 (comment)

The idea is to only add helper functions that have behaviour which is easily described by its name and which isn't already available as a known api.

adapt-contrib-text/migrations/v6.js

Lines 8 to 18 in 7d25310

	function setObjectPathValue(object, path, value, force = false) {
	if (!object) return;
	const paths = path.split('.');
	const key = paths.pop();
	const target = paths.reduce((o, p) => {
	if (!hasKey(o, p)) o[p] = {};
	return o?.[p];
	}, object);
	if (!force && hasKey(target, key)) return;
	target[key] = value;
	}

setObjectPathValue uses value as a default value unless force=true. You would probably need to remove the force argument and have two functions, setObjectPathDefaultValue and setObjectPathValue in order to have self descriptive helper functions.

lodash is already installed and has got setObjectPathValue as _.set and hasKey as _.has, it just doesn't have setObjectPathDefaultValue.

lodash also has many more useful helper functions and documentation, making it much easier to reference and more useful and more widely beneficial for everyone to understand.

It is a little longer, but you can achieve a similar and simpler effect with lodash, without writing helpers:

const ariaRegionPath = '_globals._components._text.ariaRegion'
const ariaRegionDefault = ''
if (!_.has(course, ariaRegionPath)) {
  _.set(course, ariaRegionPath, ariaRegionDefault);
}

Or alternatively:

const ariaRegionPath = '_globals._components._text.ariaRegion'
const ariaRegionDefault = ''
_.set(course, ariaRegionPath, _.get(course, ariaRegionPath, ariaRegionDefault));

Or with lodash as a helper:

function setDefault(object, path, defaultValue) {
  if (_.has(object, path)) return
  _.set(object, path, defaultValue);
}

I know @joe-allen-89 has had discussions with others about the other question If an author has changed the value from the default, we shouldn't apply the new default value. I'm sure he has some references or advice in that regard.

Examples of simple helpers which don't exist externally, getConfig, getCourse, getComponents: https://github.com/adaptlearning/adapt-migrations/blob/a4b7824e0b4a9c588bb18bbd107ed79fa8a8f1d3/api/helpers.js#L7-L17

[joe-allen-89]

Picky I know but the same in both v2 and v6

Suggested change

	import { describe , whereContent, whereFromPlugin, whereToPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations';
	import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations';

[danielghost]

I think I copied this from an example file, but will correct and only use methods that are used in the migration script.

[oliverfoster]

You should be able to use eslint inside migration scripts. It'll flag up red for stuff that isn't needed, or whitespace and indentation that's off.

[joe-allen-89]

Was this used for testing? Don't think we need to delete the globals here.

[danielghost]

This is the location of the ariaRegion in the v1 schema before it was updated, so it should be deleted to keep things correct.

[joe-allen-89]

Suggested change

	updatePlugin('adapt-contrib-text - update to v6.1.3', {name: 'adapt-contrib-text', version: '6.1.3', framework: '>=5.19.1'})
	updatePlugin('adapt-contrib-text - update to v6.1.3', { name: 'adapt-contrib-text', version: '6.1.3', framework: '>=5.19.1' });

[joe-allen-89]

Suggested change

	whereFromPlugin('adapt-contrib-text - from v4.2.0 to v6.1.3', { name: 'adapt-contrib-text', version: '<6.1.3'});
	whereFromPlugin('adapt-contrib-text - from v4.2.0 to v6.1.3', { name: 'adapt-contrib-text', version: '<6.1.3' });