feat: add SCORM support to uploadcontent and uploadlargecontent plugins#783
feat: add SCORM support to uploadcontent and uploadlargecontent plugins#783romitshah02 wants to merge 6 commits into
Conversation
sntiwari1
left a comment
There was a problem hiding this comment.
Code Review: SCORM Support for Upload Plugins
Thank you for adding SCORM support to both upload plugins. The feature works conceptually but there are several bugs and quality issues that should be addressed before merging.
Critical Issues
1. Stale $scope.isScorm state in uploadapp.js
In the onSubmit callback, when a zip is uploaded but does NOT contain imsmanifest.xml, $scope.isScorm is never explicitly set to false. This means: if a user first uploads a SCORM zip (sets isScorm=true), then resets and uploads a regular HTML zip, the HTML zip will be incorrectly assigned the application/vnd.ekstep.scorm-archive mime type.
Fix: add an else branch to reset the flag:
JSZip.loadAsync(e.target.result).then(function(zip) {
$scope.isScorm = zip.file("imsmanifest.xml") !== null; // always set, don't leave stale
$scope.upload();
});2. Missing .catch() handler on JSZip promise (both plugins)
If the zip file is corrupted or unreadable, JSZip.loadAsync will reject silently. In uploadapp.js, this leaves the UI frozen (actions hidden, no loader shown). In uploadlargecontentapp.js, the same issue applies.
Fix: always add a rejection handler:
JSZip.loadAsync(e.target.result).then(function(zip) {
// ...
}).catch(function(err) {
$scope.toasterMsgHandler('error', 'Unable to read zip file. Please try again.');
$scope.uploader.reset();
$scope.showLoader(false);
// re-show upload actions
$('#qq-upload-actions').show();
});Medium Issues
3. Angular digest cycle not triggered for $scope.isScorm in uploadapp.js
The JSZip.loadAsync callback runs outside Angular's digest cycle. Setting $scope.isScorm = true inside the .then() won't trigger a digest in uploadapp.js (unlike uploadlargecontentapp.js which eventually calls $safeApply() via showLoader). The $scope.upload() call relies on $scope.isScorm being set correctly before Angular evaluates any bindings.
Recommend wrapping the callback body in $scope.$apply() or using $scope.$safeApply() to ensure consistency.
4. Outdated and duplicated JSZip library
- The bundled
jszip.min.jsis version 3.1.5 from 2016 — the current release is 3.10.x. Older versions may have unpatched vulnerabilities. - The exact same file is duplicated in both plugins (
org.ekstep.uploadcontent-1.5/editor/libs/andorg.ekstep.uploadlargecontent-1.0/editor/libs/).
Recommend using a CDN reference or a shared libs/ folder at the root level, and upgrade to the latest stable version.
Low / Nit
5. Error message in uploadlargecontentapp.js is misleading
The error shown when a non-SCORM zip is uploaded says:
"Invalid content type (supported type: mp4, webm and scorm)"
The word "scorm" will be confusing to users since they upload a .zip file — they don't know it's a SCORM package until after it's validated. Suggest:
"Invalid content type. Supported types: mp4, webm, or a SCORM-compliant zip (containing imsmanifest.xml)"
6. detectMimeType still returns html-archive for zip in both plugins
The zip case in detectMimeType returns application/vnd.ekstep.html-archive, and SCORM is then applied as a post-hoc override via the $scope.isScorm flag. This tight coupling is fragile — a future refactor could drop the override and break SCORM uploads silently. Consider either passing SCORM context into detectMimeType or renaming the override logic to be more explicit.
7. Dead variable in uploadlargecontentapp.js generatePreSignedUrl
const file = $scope.selectedFile // assigned but never used
const url = $scope.submitUri // assigned but never used These were likely left over from debugging.
Summary
This pull request adds support for uploading SCORM-compliant content packages to both the
org.ekstep.uploadcontentandorg.ekstep.uploadlargecontentplugins.Changes included:
imsmanifest.xmlusing JSZip to identify SCORM packages.application/vnd.ekstep.scorm-archivemime type during upload.Files Modified:
org.ekstep.uploadcontent-1.5/editor/uploadapp.jsorg.ekstep.uploadcontent-1.5/editor/upload.htmlorg.ekstep.uploadlargecontent-1.0/editor/uploadlargecontentapp.jsorg.ekstep.uploadlargecontent-1.0/editor/uploadlargecontent.html