Skip to content

Combined updates#186

Open
taylortom wants to merge 12 commits intomasterfrom
feature/combined-updates
Open

Combined updates#186
taylortom wants to merge 12 commits intomasterfrom
feature/combined-updates

Conversation

@taylortom
Copy link
Copy Markdown
Contributor

@taylortom taylortom commented Mar 23, 2026

Fix

  • Filter null courses from findOne in migrateExistingCourses.js, add _type: 'course' constraint, and report missing courses as failures instead of crashing
  • Fix catch block null safety using course?._id?.toString() to prevent errors when course is null/undefined
  • Normalize both sides of isDeepStrictEqual comparison via JSON.parse(JSON.stringify()) to avoid spurious DB writes caused by type coercion
  • Make invalidatePrebuiltCache async and await invalidateCache() with error handling before triggering rebuild to prevent unhandled promise rejections and race conditions
  • Deduplicate concurrent prebuildSharedCache calls by reusing the in-flight promise and clearing it via .finally()
  • Remove unused FW_UPDATE_MIGRATION_FAILED error definition from errors.json
  • Resolve merge conflict in updateFramework() combining getLatestVersion() fallback with migration tracking from master

Update

  • Add adapt-migrations-based content migration utilities and run them after framework updates and during imports
  • Add a prebuilt preview-build cache keyed by a deterministic plugin hash, with optional eager shared-cache prebuild and invalidation hooks
  • Replace courseassets dependency with content._assetIds queries
  • Update prebuildSharedCache docstring to accurately reflect reuse-of-in-flight-promise behaviour
  • Use upath.join in prebuiltCache test assertions for correct cross-platform (Windows) path comparison

Testing

  1. All 155 tests pass

⚡ Quickly spin up Copilot coding agent tasks from anywhere on your macOS or Windows machine with Raycast.

Cache shared grunt output (JS bundle, templates, CSS, plugin assets) per
theme+menu combo so subsequent preview builds skip compilation entirely,
reducing build time from ~60s to ~2-5s.
- Cache all build root entries except course/ (previously missed required
  files like connection.txt and SCORM HTML files)
- Use CSS_ENTRIES set for theme/menu-specific files (adapt.css, fonts)
- Apply schema defaults via jsonschema module on cached builds to
  replicate grunt's schema-defaults task
- Update tests to match actual grunt build output structure
Adds a `prebuildSharedCache` config option that, when enabled, triggers
a background grunt build after cache invalidation to eagerly rebuild
the shared prebuilt cache (JS, HTML, templates, libraries). CSS remains
lazily built per theme/menu combination on first preview.
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces content migration and build caching utilities to support framework updates/imports and faster preview builds, along with associated tests and configuration.

Changes:

  • Add adapt-migrations-based content migration utilities and run them after framework updates / during imports.
  • Add a prebuilt preview-build cache keyed by a deterministic plugin hash (+ optional eager shared-cache prebuild + invalidation hooks).
  • Remove courseassets usage and add/expand node:test coverage for the new utilities.

Reviewed changes

Copilot reviewed 27 out of 27 changed files in this pull request and generated 9 comments.

Show a summary per file
File Description
tests/utils-runContentMigration.spec.js Adds unit tests for runContentMigration behavior and journal shape.
tests/utils-readFrameworkPluginVersions.spec.js Adds tests for reading plugin versions from framework bower.json files.
tests/utils-prebuiltCache.spec.js Adds tests for cache pathing, populate/restore, and invalidation flows.
tests/utils-migrateExistingCourses.spec.js Adds tests for bulk course migration behavior and error isolation.
tests/utils-generateLanguageManifest.spec.js Adds tests for language manifest filtering.
tests/utils-computePluginHash.spec.js Adds tests for deterministic plugin-hash generation.
tests/utils-collectMigrationScripts.spec.js Adds tests for migration script discovery via globbing.
tests/utils-applyBuildReplacements.spec.js Adds tests for placeholder replacement in index.html.
tests/AdaptFrameworkImport.spec.js Updates rollback expectations after removing courseassets deletion.
package.json Drops adapt-authoring-courseassets; adds adapt-migrations; keeps node:test runner options.
lib/utils/runContentMigration.js New wrapper around adapt-migrations (load + migrate) using a Journal.
lib/utils/readFrameworkPluginVersions.js New helper to read installed framework plugin versions from bower.json files.
lib/utils/prebuiltCache.js New cache implementation for shared vs CSS/theme-specific build artifacts.
lib/utils/prebuildSharedCache.js New eager background build to prepopulate shared cache artifacts.
lib/utils/migrateExistingCourses.js New bulk migration runner that loads course content, migrates, and writes back only changes.
lib/utils/generateLanguageManifest.js New helper to generate language data manifest file list.
lib/utils/computePluginHash.js New deterministic hash generator for installed plugin sets via adapt-cli Project.
lib/utils/collectMigrationScripts.js New helper to discover migration scripts in framework src/**/migrations.
lib/utils/applyBuildReplacements.js New helper to apply @@... placeholder substitutions in cached builds’ index.html.
lib/utils.js Exports the newly added utilities.
lib/handlers.js Update endpoint now returns migration results from updateFramework().
lib/AdaptFrameworkModule.js Runs migrations after framework update; adds cache invalidation hooks and plugin-hash helpers.
lib/AdaptFrameworkImport.js Switches to in-memory adapt-migrations flow and removes courseassets dependency.
lib/AdaptFrameworkBuild.js Adds preview cache hit path + post-build cache population; replaces courseassets lookup with _assetIds scan.
index.js Re-exports readFrameworkPluginVersions from the public entrypoint.
errors/errors.json Adds FW_UPDATE_MIGRATION_FAILED error definition.
conf/config.schema.json Adds prebuildSharedCache boolean config option.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

- Resolve merge conflict in updateFramework() combining getLatestVersion() fallback with migration tracking
- Filter null courses in migrateExistingCourses to handle missing courseIds gracefully
- Normalize both sides of isDeepStrictEqual comparison to avoid spurious DB writes
- Make invalidatePrebuiltCache async and await cache invalidation before rebuild
- Deduplicate concurrent prebuildSharedCache calls via shared in-flight promise
- Remove unused FW_UPDATE_MIGRATION_FAILED error definition
- Use upath in prebuiltCache test assertions for Windows compatibility
@taylortom
Copy link
Copy Markdown
Contributor Author

Addressed review comments and resolved merge conflict with master in 99898c2.

Merge conflict: Combined master's getLatestVersion() fallback with HEAD's migration tracking in updateFramework().

Review comments actioned:

  • migrateExistingCourses.js: Filter null courses from findOne, add _type: 'course' constraint, report missing as failures. Fixed catch block null safety (course?._id?.toString()). Normalize both sides of isDeepStrictEqual comparison via JSON.parse(JSON.stringify()) to avoid spurious DB writes from type coercion.
  • AdaptFrameworkModule.js: invalidatePrebuiltCache is now async and awaits invalidateCache() with error handling before triggering rebuild. prebuildSharedCache reuses in-flight promise and clears via .finally(), with updated docstring.
  • errors/errors.json: Removed unused FW_UPDATE_MIGRATION_FAILED — migration results are already returned to the caller.
  • tests/utils-prebuiltCache.spec.js: getCachePaths assertions now use upath.join to match the implementation and avoid failures on Windows.

Review comments not actioned:

  • Path sanitization of theme/menu (prebuiltCache.js): These values come from contentplugin DB records (plugin names like adapt-contrib-vanilla), not from user input. Path traversal is not a realistic risk here.
  • Date.now() temp dir collision (prebuiltCache.js): safeRename already handles ENOTEMPTY/EEXIST gracefully, so a same-millisecond collision would be benign.

All 155 tests pass. @copilot

Copy link
Copy Markdown
Contributor

Copilot AI commented Mar 27, 2026

Addressed review comments and resolved merge conflict with master in 99898c2.

Merge conflict: Combined master's getLatestVersion() fallback with HEAD's migration tracking in updateFramework()...

All changes from commit 99898c2 look good — CI is passing. The review comments have been addressed as described.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants