Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Extract metadata for ims scorm cp #4258

Open
wants to merge 4 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
@input="trackSelect"
@removed="handleRemoved"
/>
<div v-if="getChildren !== undefined">
<div v-if="getChildren !== undefined && getChildren.length">
<EditListItems
v-for="childId in getChildren"
:key="childId"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -515,12 +515,12 @@
});
} else if (file.metadata.folders) {
this.createNode('topic', file.metadata).then(newNodeId => {
file.metadata.folders.forEach(org => {
this.createNode('topic', org, newNodeId).then(topicNodeId => {
org.files.forEach(orgFile => {
file.metadata.folders.forEach(folder => {
this.createNode('topic', folder, newNodeId).then(topicNodeId => {
folder.files.forEach(folderFile => {
const extra_fields = {};
extra_fields['options'] = { entry: orgFile.resourceHref };
extra_fields['title'] = orgFile.title;
extra_fields['options'] = { entry: folderFile.resourceHref };
extra_fields['title'] = folderFile.title;
let file_kind = null;
FormatPresetsList.forEach(p => {
if (p.id === file.metadata.preset) {
Expand All @@ -532,7 +532,6 @@
return File.uploadUrl({
checksum: file.checksum,
size: file.file_size,
type: 'application/zip',
name: file.original_filename,
file_format: file.file_format,
preset: file.metadata.preset,
Expand All @@ -542,7 +541,7 @@
loaded: 0,
total: file.size,
};
if (index === 0) {
if (!this.selected.length) {
this.selected = [resourceNodeId];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

}
this.updateFile({
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -247,7 +247,6 @@ describe('file store', () => {
});
});
it('extractIMSMetadata should extract metadata from imsmanifest.xml', async () => {
// const manifestFile = get_imsmanifest_file({ title: 'Test file' });
const manifestContent = `
<manifest xmlns:imsmd="http://www.imsglobal.org/xsd/imsmd_v1p2">
<metadata>
Expand Down Expand Up @@ -317,7 +316,6 @@ describe('file store', () => {

const zip = new JSZip();
zip.file('imsmanifest.xml', manifestContent);
// zip.folder('ims_package.zip');
await zip.generateAsync({ type: 'blob' }).then(async function(imsBlob) {
await expect(extractIMSMetadata(imsBlob)).resolves.toEqual({
title: 'Test File',
Expand Down Expand Up @@ -506,7 +504,7 @@ describe('file store', () => {
<imsmd:lom>
<imsmd:general>
<imsmd:title>
<imsmd:langstring xml:lang="en">Test File</imsmd:langstring>
<imsmd:langstring xml:lang="und">\t\t\t\n\n\n\nTest File\n</imsmd:langstring>
</imsmd:title>
<imsmd:language>und</imsmd:language>
</imsmd:general>
Expand Down
32 changes: 13 additions & 19 deletions contentcuration/contentcuration/frontend/shared/vuex/file/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ async function getFolderMetadata(data, xmlDoc, zip, procssedFiles) {
};
if (orgNode.nodeType === 1) {
const title = orgNode.getElementsByTagName('title');
org.title = title[0].textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
org.title = title[0].textContent.trim();
const files = orgNode.getElementsByTagName('item');
const immediateChildNodes = [];
const childNodes = Object.values(orgNode.children);
Expand All @@ -74,7 +74,7 @@ async function getFolderMetadata(data, xmlDoc, zip, procssedFiles) {
await Promise.all(
immediateChildNodes.map(async (fileNode, k) => {
const file = {};
file.title = title[1 + k].textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
file.title = title[1 + k].textContent.trim();
file.identifierref = fileNode.getAttribute('identifierref');
file.resourceHref = xmlDoc
.querySelectorAll(`[identifier=${file.identifierref}]`)[0]
Expand Down Expand Up @@ -149,41 +149,37 @@ async function getManifestMetadata(manifestFile, zip, procssedFiles) {
if (xmlDoc.getElementsByTagName('lomes:title').length) {
metadata.title = xmlDoc
.getElementsByTagName('lomes:title')[0]
.children[0].textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
.children[0].textContent.trim();
}
if (
xmlDoc.getElementsByTagName('lomes:idiom').length &&
xmlDoc
.getElementsByTagName('lomes:idiom')[0]
.textContent.replace(/ {2}|\r\n|\n|\r/gm, '') !== 'und'
LanguagesMap.has(xmlDoc.getElementsByTagName('lomes:idiom')[0].textContent.trim()) &&
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

xmlDoc.getElementsByTagName('lomes:idiom')[0].textContent.trim() !== 'und'
) {
metadata.language = xmlDoc
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For language extraction let's replicate the validation we are doing in H5P to check this is a supported language code.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can also add some more tests for unhappy paths!

.getElementsByTagName('lomes:idiom')[0]
.children[0].textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
.children[0].textContent.trim();
}
if (xmlDoc.getElementsByTagName('lomes:description').length) {
metadata.description = xmlDoc
.getElementsByTagName('lomes:description')[0]
.children[0].textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
.children[0].textContent.trim();
}
} else {
if (xmlDoc.getElementsByTagName('imsmd:title').length) {
metadata.title = xmlDoc
.getElementsByTagName('imsmd:title')[0]
.textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
metadata.title = xmlDoc.getElementsByTagName('imsmd:title')[0].textContent.trim();
}
if (
xmlDoc.getElementsByTagName('imsmd:language').length &&
xmlDoc.getElementsByTagName('imsmd:language')[0].textContent !== 'und'
LanguagesMap.has(xmlDoc.getElementsByTagName('imsmd:language')[0].textContent.trim()) &&
xmlDoc.getElementsByTagName('imsmd:language')[0].textContent.trim() !== 'und'
) {
metadata.language = xmlDoc
.getElementsByTagName('imsmd:language')[0]
.textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
metadata.language = xmlDoc.getElementsByTagName('imsmd:language')[0].textContent.trim();
}
if (xmlDoc.getElementsByTagName('imsmd:description').length) {
metadata.description = xmlDoc
.getElementsByTagName('imsmd:description')[0]
.textContent.replace(/ {2}|\r\n|\n|\r/gm, '');
.textContent.trim();
}
}
return metadata;
Expand All @@ -204,9 +200,7 @@ export async function extractIMSMetadata(fileInput) {
}
})
.then(async manifestFile => {
return await getManifestMetadata(manifestFile, zip, procssedFiles).then(metadata => {
return metadata;
});
return await getManifestMetadata(manifestFile, zip, procssedFiles);
});
}

Expand Down