Extract metadata for ims scorm cp#4258
Extract metadata for ims scorm cp#4258manavagr1108 wants to merge 4 commits intolearningequality:unstablefrom
Conversation
- Create topic and resouce node for the metadata
cb939ec to
33b1680
Compare
33b1680 to
90102de
Compare
- adding test cases for extractIMSMetadata
rtibbles
left a comment
There was a problem hiding this comment.
Some minor tweaks needed, but this is looking very good!
Note: we will hold off merge until unstable has been released, so that we can release the H5P metadata extraction sooner, then this will be merged and released in a later release!
| @input="trackSelect" | ||
| @removed="handleRemoved" | ||
| /> | ||
| <div v-if="getChildren !== undefined"> |
There was a problem hiding this comment.
Should change this to a length check on the array!
| }); | ||
| } else if (file.metadata.folders) { | ||
| this.createNode('topic', file.metadata).then(newNodeId => { | ||
| file.metadata.folders.forEach(org => { |
| this.createNode('topic', file.metadata).then(newNodeId => { | ||
| file.metadata.folders.forEach(org => { | ||
| this.createNode('topic', org, newNodeId).then(topicNodeId => { | ||
| org.files.forEach(orgFile => { |
| return File.uploadUrl({ | ||
| checksum: file.checksum, | ||
| size: file.file_size, | ||
| type: 'application/zip', |
There was a problem hiding this comment.
Let's double check if we need to specify this explicitly, or if it should be inferred by existing functionality.
| total: file.size, | ||
| }; | ||
| if (index === 0) { | ||
| this.selected = [resourceNodeId]; |
There was a problem hiding this comment.
Let's change this to set this.selected if we haven't already set this.selected to something - so only the first finalized node gets selected.
| }); | ||
| }); | ||
| it('extractIMSMetadata should extract metadata from imsmanifest.xml', async () => { | ||
| // const manifestFile = get_imsmanifest_file({ title: 'Test file' }); |
| <imsmd:lom> | ||
| <imsmd:general> | ||
| <imsmd:title> | ||
| <imsmd:langstring xml:lang="en">Test File</imsmd:langstring> |
| ) { | ||
| metadata.language = xmlDoc | ||
| .getElementsByTagName('lomes:idiom')[0] | ||
| .children[0].textContent.replace(/ {2}|\r\n|\n|\r/gm, ''); |
There was a problem hiding this comment.
Just double check whether trim will do the same job here!
There was a problem hiding this comment.
The assertions in the tests should validate that this is working properly!
| .getElementsByTagName('lomes:idiom')[0] | ||
| .textContent.replace(/ {2}|\r\n|\n|\r/gm, '') !== 'und' | ||
| ) { | ||
| metadata.language = xmlDoc |
There was a problem hiding this comment.
For language extraction let's replicate the validation we are doing in H5P to check this is a supported language code.
There was a problem hiding this comment.
Can also add some more tests for unhappy paths!
| const IMS_PRESETS = [ | ||
| FormatPresetsNames.QTI, | ||
| FormatPresetsNames.HTML5_DEPENDENCY, | ||
| FormatPresetsNames.HTML5_ZIP, |
There was a problem hiding this comment.
Note for me - I may need to consider an IMSCP preset format.
2db6a02 to
6c94668
Compare
| xmlDoc | ||
| .getElementsByTagName('lomes:idiom')[0] | ||
| .textContent.replace(/ {2}|\r\n|\n|\r/gm, '') !== 'und' | ||
| LanguagesMap.has(xmlDoc.getElementsByTagName('lomes:idiom')[0].textContent.trim()) && |
There was a problem hiding this comment.
Could avoid a bit of repetition here and define outside of the if statement:
const language = xmlDoc.getElementsByTagName('lomes:idiom').length ? xmlDoc.getElementsByTagName('lomes:idiom')[0].textContent.trim() : 'und';
(defaulting to the disallowed und)
then you can do the checks and assignment against this value instead.
This code aims to extract metadata from IMS content package.
This Pr is a part of GSoC Project linked with the issue #4081.
Summary
Description of the change(s) you made
imsmanifest.xmlis present in file or notimsmetadata.xmlwe need to check this file as wellextractIMSMetadataManual verification steps performed
.zipformat:imsmanifest.xmlandimsmetadata.xmlfile and read the file as text.Comments
Checking for sub manifest files present in content packe is yet to be implemented.
Contributor's Checklist
PR process:
CHANGELOGlabel been added to this PR. Note: items with this label will be added to the CHANGELOG at a later timedocslabel has been added if this introduces a change that needs to be updated in the user docs?requirements.txtfiles also included in this PRStudio-specifc:
notranslateclass been added to elements that shouldn't be translated by Google Chrome's automatic translation feature (e.g. icons, user-generated text)pages,components, andlayoutsdirectories as described in the docsTesting:
Reviewer's Checklist
This section is for reviewers to fill out.
yarnandpip)