New: migration scripts added to repo (fixes #208) #209
Conversation
migrations/v6.js
Outdated
| import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations'; | ||
| import _ from 'lodash'; | ||
|
|
||
| const getCourse = content => { | ||
| const course = content.find(({ _type }) => _type === 'course'); | ||
| return course; | ||
| }; |
There was a problem hiding this comment.
| import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin } from 'adapt-migrations'; | |
| import _ from 'lodash'; | |
| const getCourse = content => { | |
| const course = content.find(({ _type }) => _type === 'course'); | |
| return course; | |
| }; | |
| import { describe, whereContent, whereFromPlugin, mutateContent, checkContent, updatePlugin, getCourse } from 'adapt-migrations'; | |
| import _ from 'lodash'; |
migrations/v6.js
Outdated
| const getGlobals = content => { | ||
| return getCourse(content)?._globals?._menu?._boxMenu; | ||
| }; |
There was a problem hiding this comment.
As you've added a mutate for checking globals exists I don't think the getGlobals function is needed here, could be kept inline.
| mutateContent('Box menu - add _graphic attribute', async (content) => { | ||
| course._boxMenu._graphic = defaultGraphic; | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
@chris-steele Thoughts on splitting these into two mutates as there's two attributes in the object? Or as it's setting a single attribute leave it as one?
There was a problem hiding this comment.
You could argue that you are adding a single attribute; it just happens to be of type object. I'd say let's leave as is otherwise we might fall on the sword of consistency elsewhere where there are tons of new properties which will make for a very verbose script.
migrations/v6.js
Outdated
| whereFromPlugin('Box menu - from v6.3.8', { name: 'adapt-contrib-boxMenu', version: '<6.3.9' }); | ||
|
|
||
| whereContent('Box menu - where course has _backgroundImage', async (content) => { | ||
| course = getCourse(content); |
There was a problem hiding this comment.
| course = getCourse(content); | |
| course = getCourse(); |
migrations/v4.js
Outdated
| mutateContent('Box menu - remove globals', async (content) => { | ||
| delete courseBoxMenuGlobals.ariaRegion; | ||
| delete courseBoxMenuGlobals.menuItem; | ||
| delete courseBoxMenuGlobals.menuEnd; | ||
| return true; | ||
| }); | ||
|
|
||
| checkContent('Box menu - check globals', async (content) => { | ||
| const globals = getGlobals(content); | ||
| const isValid = ( | ||
| Object.hasOwn(globals, 'ariaRegion') === false && | ||
| Object.hasOwn(globals, 'menuItem') === false && | ||
| Object.hasOwn(globals, 'menuEnd') === false | ||
| ); | ||
| if (!isValid) throw new Error('Box menu - global attributes ariaRegion menuItem menuEnd'); | ||
| return true; | ||
| }); |
There was a problem hiding this comment.
Similar to another comment, best practice on seperating these rather than in one mutate/check?
migrations/v2.js
Outdated
| let course, courseBoxMenuGlobals; | ||
| const durationLabel = 'Duration:'; | ||
|
|
||
| whereFromPlugin('Box menu - from v2.0.2', { name: 'adapt-contrib-boxMenu', version: '<2.0.3' }); |
There was a problem hiding this comment.
Just so we're not migrating anything earlier than v2
| whereFromPlugin('Box menu - from v2.0.2', { name: 'adapt-contrib-boxMenu', version: '<2.0.3' }); | |
| whereFromPlugin('Box menu - from v2.0.2', { name: 'adapt-contrib-boxMenu', version: '>=2.0.0 <2.0.3' }); |
migrations/v6.js
Outdated
| let course, courseBoxMenuGlobals; | ||
| const itemCount = 'Item {{_nthChild}} of {{_totalChild}}'; | ||
|
|
||
| whereFromPlugin('Box menu - from v6.3.9', { name: 'adapt-contrib-boxMenu', version: '<v6.3.10' }); |
There was a problem hiding this comment.
| whereFromPlugin('Box menu - from v6.3.9', { name: 'adapt-contrib-boxMenu', version: '<v6.3.10' }); | |
| whereFromPlugin('Box menu - from v6.3.9', { name: 'adapt-contrib-boxMenu', version: '<6.3.10' }); |
There was a problem hiding this comment.
What's the reason for this suggestion @joe-allen-89 ? v6.3.10 is a valid release/tag
There was a problem hiding this comment.
@chris-steele it throws an error as invalid version number as it fails VERSION_CHECK
There was a problem hiding this comment.
We're not comparing release tags, we're comparing valid semver version numbers with semver ranges. They don't have a v on the front. Think package.json:version not GitHub tags
migrations/v6.js
Outdated
| return true; | ||
| }); | ||
|
|
||
| updatePlugin('Box menu - update to v6.3.10', { name: 'adapt-contrib-boxMenu', version: 'v6.3.10', framework: '">=5.24.2' }); |
There was a problem hiding this comment.
| updatePlugin('Box menu - update to v6.3.10', { name: 'adapt-contrib-boxMenu', version: 'v6.3.10', framework: '">=5.24.2' }); | |
| updatePlugin('Box menu - update to v6.3.10', { name: 'adapt-contrib-boxMenu', version: '6.3.10', framework: '">=5.24.2' }); |
migrations/v4.js
Outdated
| }); | ||
|
|
||
| checkContent('Box menu - check global attribute menuEnd', async (content) => { | ||
| const isValid = Object.hasOwn(courseBoxMenuGlobals, 'menuEnd') === false; |
There was a problem hiding this comment.
Because boolean === false is the same as !booleanwhat you've got here is:
if ((Object.hasOwn(courseBoxMenuGlobals, 'menuEnd') === false) === false) throw...Object.has('') returns a Boolean that you're flipping twice. Which is a bit weird. You could instead change the variable name to isInvalid to suit the throw early rather than having if (!isValid) throw... or instead just return true early with if (isValid) return true;.
const isInvalid = Object.hasOwn(courseBoxMenuGlobals, 'menuEnd');
if (isInvalid) throw....
return true; const isValid = !Object.hasOwn(courseBoxMenuGlobals, 'menuEnd');
if (isValid) return true;
throw...You can use ! to flip Booleans rather than === false or !== true.
#208
New