New: added migration scripts (fixes #79) #80
Conversation
| whereFromPlugin('Glossary - from v2.0.3', { name: 'adapt-contrib-glossary', version: '<2.1.0' }); | ||
|
|
||
| whereContent('Glossary - where configured', async (content) => { | ||
| course = getCourse(content); |
There was a problem hiding this comment.
Is there an advantage to moving this to a function? The common practice so far has been to keep the find function within the whereContent.
Best to use find rather than filter as we'll only have a single instance of course.
| course = getCourse(content); | |
| const course = content.find(({ _type }) => _type === 'course'); |
There was a problem hiding this comment.
@joe-allen-89 I just moved it to a function as I use it several times.
migrations/v2.js
Outdated
|
|
||
| whereContent('Glossary - where configured', async (content) => { | ||
| course = getCourse(content); | ||
| if (course._glossary) return true; |
There was a problem hiding this comment.
Simplify to the below?
| if (course._glossary) return true; | |
| return course._glossary; |
migrations/v2.js
Outdated
| courseGlossaryGlobals = getGlobals(content); | ||
| if (courseGlossaryGlobals) return true; | ||
| course._globals._extensions = course._globals._extensions || {}; | ||
| courseGlossaryGlobals = course._globals._extensions._glossary = {}; | ||
| return true; |
There was a problem hiding this comment.
course = content.find(({ _type }) => _type === 'course');
if (!_.has(course, '_globals._components._glossary')) _.set(course, '_globals._components._glossary', {});
courseGlossaryGlobals = course._globals._components._glossary;
return true;
There was a problem hiding this comment.
Forgot to update this to use _.set and _.has
migrations/v2.js
Outdated
| }); | ||
|
|
||
| checkContent('Glossary - check attribute clearSearch', async (content) => { | ||
| return course._glossary.clearSearch === ''; |
There was a problem hiding this comment.
@chris-steele currently none of the checkContent functions have the throw Error included, think they'll need updating to do this by using the isValid/isInvalid that we've used in the other repos.
| return course._glossary.clearSearch === ''; | |
| isValid = course._glossary.clearSearch === ''; | |
| if (!isValid) throw new Error('Glossary - clearSearch global attribute not found') | |
| return |
migrations/v3.js
Outdated
| return true; | ||
| }); | ||
|
|
||
| mutateContent('Glossary - add new globals', async (content) => { |
There was a problem hiding this comment.
Whilst this does work, from a best practise perspective it would be better to split these up, so having a mutate and check for each attribute seperately. It does add to the code but it does make the logs much easier to read in case of an issue.
| if (course._glossary.description !== '') return true; | ||
| course._glossary.description = 'Select here to view the glossary for this course'; | ||
| return true; |
There was a problem hiding this comment.
Think we can condense this slightly, thoughts?
| if (course._glossary.description !== '') return true; | |
| course._glossary.description = 'Select here to view the glossary for this course'; | |
| return true; | |
| if (course._glossary.description === '') course._glossary.description = 'Select here to view the glossary for this course'; | |
| return true; |
migrations/v4.js
Outdated
|
|
||
| whereContent('Glossary has items', async content => { | ||
| course = getCourse(content); | ||
| return course._glossary?._items?.length > 0; |
There was a problem hiding this comment.
| return course._glossary?._items?.length > 0; | |
| return course._glossary?._items?.length; |
#79
New