[oliverfoster]

Summary

Adds adapt-migrations content-migration scripts covering every schema change to adapt-contrib-vanilla from v3.1.0 up to the current v9.40.0, so course content authored against older theme versions can be automatically upgraded.

Fixes #544

migrations/v5.js

• v3.1.0 → v5.0.0: adds _vanilla._backgroundImage, _backgroundStyles and block _minimumHeights/_isDividerBlock to articles, blocks and contentObjects; adds _pageHeader._backgroundImage/_backgroundStyles/_minimumHeights to contentObjects.
• v5.0.0 → v5.4.0: adds _vanilla._responsiveClasses to articles, blocks and contentObjects.

migrations/v7.js

• v5.4.0 → v7.1.0: adds _vanilla._componentVerticalAlignment (default 'top') to blocks.

migrations/v8.js

• v7.1.0 → v8.0.0: adds _vanilla._paddingTop/_paddingBottom (default 'standard') to blocks.
• v8.0.0 → v8.1.0: adds _vanilla._textAlignment (_title/_body/_instruction) to articles, blocks and components, and _pageHeader._textAlignment to contentObjects.

migrations/v9.js

• v8.1.0 → v9.5.0: adds _vanilla._componentHorizontalAlignment (default 'left') to blocks.
• v9.5.0 → v9.6.0: adds _vanilla._favIcon._src to course.
• v9.6.0 → v9.6.13: adds menu-header colour theme variables (background/title/body/instruction).
• v9.6.13 → v9.6.15: adds _xlarge breakpoint entries to existing _backgroundImage, _responsiveClasses and _minimumHeights objects (article/block/contentObject/pageHeader).
• v9.6.15 → v9.8.2: converts empty-string _backgroundStyles values to their new non-empty defaults ('no-repeat'/'cover'/'center top') and narrows the valid enum accordingly.
• v9.8.2 → v9.16.1: adds loading/loading-inverted theme variables.
• v9.16.1 → v9.17.2: adds menu-header-subtitle-color theme variable.
• v9.17.2 → v9.25.0: adds the large batch of interaction-state theme variables (button, global UI, item, item-UI, menu, nav, notify and drawer state colours).
• v9.25.0 → v9.27.0: adds _pageHeader._textAlignment._subtitle to contentObjects.
• v9.27.0 → v9.29.0: adds _vanilla._articleHeader (background image/styles/minimum heights) to articles and _vanilla._blockHeader to blocks, plus notify-title-color theme variable.
• v9.29.0 → v9.32.0: adds _pageHeader._graphic (_src/alt) to contentObjects.
• v9.32.0 → v9.35.0: adds page-header colour theme variables.
• v9.35.0 → v9.36.0: adds drawer-item-selected-underline, body-background-color, shadow-opacity, pull-quote and tooltip theme variables.
• v9.36.0 → v9.39.0: adds _vanilla._favIcon._svg and _appleTouch to course.

Each version step includes whereFromPlugin/whereContent gating, idempotent mutateContent steps (skips fields already present), checkContent validation, and testSuccessWhere/testStopWhere unit tests.

🤖 Generated with Claude Code

[swashbuck]

Really thorough piece of work — I verified this fairly hard and the data accuracy is excellent. All 19 updatePlugin framework versions match the real tags, all 70 theme variable names/defaults match the schema at their target version, every _.set() path exists, and the v9.8.2 '' → no-repeat/cover/center top conversion is exactly right (and necessary — '' leaves the enum there).

I ran the suite (76/76 pass) and also pushed a synthetic v3.1.0 course through all 19 steps end-to-end; the output validates cleanly against the real v9.39.0 schemas.

One issue worth fixing, in the v9.6.13 > v9.6.15 step — inline suggestions below.

_xlarge is written empty rather than carrying _large over

To be clear up front: this is a pre-existing pattern in the ecosystem, not something introduced here. adapt-contrib-boxMenu has had exactly the same bug in a released migration since v6.3.9 — I've since raised adaptlearning/adapt-contrib-boxMenu#244 and adaptlearning/adapt-contrib-boxMenu#245 for it. So this is copying established precedent; the precedent just turns out to be wrong.

The step adds the new _xlarge breakpoint as '' / 0. That's not just a missed nicety — it's actively worse than leaving the key absent, because themeView.js resolves breakpoints with nullish coalescing:

// js/themeView.js:61 and :82
const backgroundImage = backgroundImages[`_${device.screenSize}`] ?? backgroundImages._small;
const minimumHeight   = minimumHeights[`_${device.screenSize}`]   ?? minimumHeights._small;

'' and 0 aren't nullish, so writing them suppresses the _small fallback that currently works. xlarge is live at 1280px in the default config, so this fires on any ordinary desktop monitor.

End-to-end on a v9.6.13 course with real assets, migrated to v9.39.0:

	as submitted	carrying _large over
page _backgroundImage	_xlarge="" → renders ""	_xlarge="ph.jpg" → renders "ph.jpg"
page _minimumHeights	_xlarge=0 → renders 0px	_xlarge=500 → renders 500px
block _backgroundImage	_xlarge="" → renders ""	_xlarge="bg.jpg" → renders "bg.jpg"
block _minimumHeights	_xlarge=0 → renders 0px	_xlarge=400 → renders 400px

So an AAT course with a background image or minimum height loses it on screens ≥1280px after migrating. Silent, no error raised.

Worth noting the current tests can't catch this — they assert _xlarge is a string/number, and every fixture uses empty strings. Strengthening the four checkContent blocks to also assert a populated _large was carried over, plus a testSuccessWhere with real values, does catch it — I checked that such a test fails against the code as it stands, so it isn't a vacuous assertion. Happy to send that across separately if useful; it's a bit long to put in a suggestion block.

One caveat on that, learned from boxMenu: asserting a value isn't automatically enough. boxMenu's check did assert a value — _xlarge === '' — and that simply locked the bug in. The assertion needs to encode the intended rendering outcome, not whatever the mutate step happened to write.

Not suggesting a change for _responsiveClasses._xlarge (line 170)

That one is not affected — it has no ?? _small fallback, it indexes directly (themeView.js:94), so '' and undefined are both a no-op addClass. Copying _large there would be a behaviour change rather than a fix (xl screens currently get no responsive class at all), and could plausibly break a course whose large-screen class hides or reflows something meant to appear at xl. Left it alone deliberately.

Two smaller points, neither blocking

1. v9.10.0 isn't mentioned anywhere. It changed ~20 theme variable defaults from hex to "" (4f4acbf, fixes #464). The v9.8.2 > v9.16.1 step spans it but only adds loading/loading-inverted. I think omitting it is right — you can't distinguish "author accepted #4D4D4D" from "author chose #4D4D4D", so clearing would be destructive — but it'd be worth a line either way so it's clear it was considered.

2. Version claims are slightly stale. The description says "up to the current v9.40.0", but the last step targets v9.39.0 and master is now v9.41.0. Neither needs a migration (v9.40.0 is LESS-only; v9.41.0 widens the backgroundSize enum), so it's just a rebase and a wording tweak — but worth confirming the final updatePlugin is meant to sit at v9.39.0.

Nothing else from me — the rest stood up to everything I threw at it.

Posted via collaboration with Claude Code

[swashbuck]

Carry _large over so existing backgrounds survive at the new breakpoint. Writing '' here suppresses the ?? _small fallback in themeView.js:61, so the background disappears entirely at ≥1280px.

Suggested change

	item._vanilla._backgroundImage._xlarge = '';
	// carry _large over so existing backgrounds survive at the new xlarge breakpoint
	item._vanilla._backgroundImage._xlarge = item._vanilla._backgroundImage._large ?? '';

[swashbuck]

Same here — 0 isn't nullish, so it suppresses the ?? _small fallback in themeView.js:82 and the minimum height is lost at ≥1280px.

Suggested change

	block._vanilla._minimumHeights._xlarge = 0;
	// carry _large over so existing minimum heights survive at the new xlarge breakpoint
	block._vanilla._minimumHeights._xlarge = block._vanilla._minimumHeights._large ?? 0;

[swashbuck]

Same issue for the page header background.

Suggested change

	contentObject._vanilla._pageHeader._backgroundImage._xlarge = '';
	// carry _large over so existing backgrounds survive at the new xlarge breakpoint
	contentObject._vanilla._pageHeader._backgroundImage._xlarge = contentObject._vanilla._pageHeader._backgroundImage._large ?? '';

[swashbuck]

And the page header minimum height.

Suggested change

	contentObject._vanilla._pageHeader._minimumHeights._xlarge = 0;
	// carry _large over so existing minimum heights survive at the new xlarge breakpoint
	contentObject._vanilla._pageHeader._minimumHeights._xlarge = contentObject._vanilla._pageHeader._minimumHeights._large ?? 0;

[joe-replin]

Confirmed on the mechanism, but I've fixed it differently and taken only half the suggestions.

You're right that ?? is the problem. themeView.js line 61 reads:

const backgroundImage = backgroundImages[`_${device.screenSize}`] ?? backgroundImages._small;

So the '' the migration wrote is a present-but-empty value that suppresses the fallback. Same at line 82, and at 109 and 128 for the header variants.

Two things pushed me away from _xlarge = _large. The fallback target is _small, not _large, so for the common case where an author sets only _small and lets larger breakpoints inherit, copying an empty _large still loses the image. And _backgroundImage._xlarge has no schema default: the parent object is "default": {} and no breakpoint key defines its own, so a course authored fresh on 9.6.15+ has the key absent. That absence is exactly what makes ?? work.

So the migration now leaves _xlarge unset on _backgroundImage, matching a freshly authored course. Both cases survive.

I've kept _minimumHeights._xlarge and _responsiveClasses._xlarge as they were, and I'd push back on those two suggestions: both keys carry schema defaults (0 and ""), so writing them matches a fresh course.

The two checkContent assertions are inverted rather than dropped, so they now assert the key stays absent. Verified they fail if the '' write comes back: 76 tests pass on the fix, dropping to 75 with one failure when either regression is reintroduced.

I also ran the full v3.1.0 to v9.39.0 chain against a real 59-item course. No _xlarge lands on any background image, and both other properties keep theirs.

[swashbuck]

@joe-replin I would push back on f93f816 "_vanilla._backgroundImage._xlarge should be left unset so the _small fallback applies".

In my experience, when a _large image is set and _xlarge doesn't yet exist, this is almost always what _xlarge should use. We don't want xlarge backgroundImage to fall back to a small mobile image, for instance. Furthermore, the breakpoints shift upwards with the introduction of _xlarge. This is the change that I was proposing in my code suggestions above.

[joe-replin]

You're right, and I've changed it to your suggestion. I missed the breakpoint shift.

Confirmed in config.schema.json: the screenSize defaults are small: 0, medium: 720, large: 960, xlarge: 1280, and device.checkScreenSize() picks the highest breakpoint at or below the viewport width. Adding xlarge doesn't move the lower bounds, but it caps large at 1279px, so a 1920px monitor that used to resolve to large now resolves to xlarge. For a course with _large set, leaving _xlarge absent means it falls through to _small and serves a mobile image on a desktop. That's worse than the blank I was trying to avoid, and it's the case you flagged.

One refinement I've kept from my side: the carry-over is conditional on _large being set.

if (!item._vanilla._backgroundImage._large) return;
item._vanilla._backgroundImage._xlarge = item._vanilla._backgroundImage._large;

Where _large is set this matches your suggestion exactly. Where it's empty, an unconditional _large ?? '' writes _xlarge: '', which suppresses the ?? _small fallback and blanks the background for courses authored small-first. Skipping those leaves the key absent so the fallback still applies. Same handling for _pageHeader._backgroundImage.

_minimumHeights._xlarge and _responsiveClasses._xlarge stay as they were, since those keys carry schema defaults (0 and "").

Tests are at 77 passing. I added a fixture with real image values because the existing ones all had an empty _large and never exercised the carry-over. Also ran the full v3.1.0 to v9.39.0 chain against a course with authored backgrounds: _large carried through on every item that had one, and stayed absent on the 18 blocks that didn't.

[swashbuck]

Agreed on the background images — the conditional carry-over is the right call, and you're right that my _large ?? '' would have written '' and blanked small-first courses. That part's settled.

One thing left: _minimumHeights._xlarge needs the same treatment, for the same reason you just accepted for images.

setMinimumHeight() in themeView.js reads minimumHeights[`_${device.screenSize}`] ?? minimumHeights._small, identical to setBackgroundImage(). So on the 1920px monitor you described — the one that used to resolve to large and now resolves to xlarge — a block with _minimumHeights._large: 400 and _small: 0 gets _xlarge: 0 from the migration and loses its 400px minimum height. Writing the schema default keeps the key shape matching a fresh course, but it costs the authored value at the new breakpoint, which is the trade you just decided against for _backgroundImage.

Your new fixture already contains that exact case:

{ _id: 'b-100', _type: 'block', _vanilla: { _backgroundImage: { _large: 'desktop.jpg', _medium: '', _small: '' }, _minimumHeights: { _large: 400, _medium: 0, _small: 0 } } }

It passes because the check only asserts typeof _xlarge === 'number', so 0 satisfies it. The fixture documents the regression rather than catching it.

Same rule as the images would do it:

if (!block._vanilla._minimumHeights._large) return;
block._vanilla._minimumHeights._xlarge = block._vanilla._minimumHeights._large;

Plus _pageHeader._minimumHeights, and the checks updated to the _large ? _xlarge === _large : !_.has(_xlarge) shape you're already using for backgrounds. Leaving the key absent where _large is 0 is harmless — an absent property with a schema default still validates, and the authoring tool applies defaults on write anyway.

No argument on _responsiveClasses._xlarge — setResponsiveClasses() has no ?? fallback, it reads the breakpoint key directly, so '' changes nothing there. Keep it as is.

Minor, separate: the mutations skip items that already have _xlarge (the idempotency guard), but the new checks assert _xlarge === _large unconditionally. Content carrying a pre-existing _xlarge that differs from _large would now fail the migration outright. Unlikely at 9.6.13 since the key wasn't in the schema yet, but the guard and the assertion disagree.

---

Posted via collaboration with Claude Code

[joe-replin]

Both fixed in 47731d5.

_minimumHeights._xlarge now uses the same conditional carry-over as the images, on blocks and _pageHeader alike. The checks assert presence rather than equality (_large set means _xlarge present, _large empty means absent), which is what the mutation actually guarantees and resolves the disagreement with the idempotency guard. _responsiveClasses left alone.

78 tests passing. Reverting either _minimumHeights mutation to _xlarge = 0 now drops it to 77, so the regression you spotted is covered rather than documented.

[swashbuck]

The carry-over in 47731d5 is right on every item type and every ?? _small reader, and 78/78 pass. One gap left in the checks, inline.

Posted via collaboration with Claude Code

[joe-replin]

The carry-over in 47731d5 is right on every item type and every ?? _small reader, and 78/78 pass. One gap left in the checks, inline.

Posted via collaboration with Claude Code

Both points hold, and I've applied your fix in [6214226].

• authoredXlarge is now recorded in whereContent, and isXlargeResolved(item, key) compares _xlarge with _large where _large is set. Writing '', 0 or _small there should now fail the test.
• Items with an authored _xlarge are let through, so _large: 0, _xlarge: 0 no longer aborts the migration. A new fixture covers it on an article, a block and a page header.

[swashbuck]

👍

[kirsty-hames]

Looks good to me and Claude thanks 👍

[github-actions]

🎉 This PR is included in version 9.42.0 🎉

The release is available on GitHub release

Your semantic-release bot 📦🚀