refactor: introduce an LMS provider seam - #294
Open
DA-RAE wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this PR do?
Introduces a narrow LMS provider seam so that adding another LMS no longer requires copying core translation behavior or scattering LMS-specific conditions across content modules.
Motivation
Skilljar-specific selectors and page assumptions were consumed directly by the translation pipeline and by several learning tools. Adding a second real LMS would therefore mean either duplicating core behavior or spreading LMS checks across modules. This change introduces a seam based only on variation points already demonstrated by currently supported surfaces, without speculating about future provider capabilities.
Implementation notes
Providers implement exactly three required methods:
matches,probeRestricted, andgetPageContext.The runtime performs a two-phase
probe -> describeflow. Restricted certification routes stop after the probe — before metadata discovery, translator/storage initialization, or UI injection.CERT_DISABLE_PATTERNSremains in core as a non-bypassable safety floor.Core continues to own
popstate,hashchange, and History API wrapping. A route change only re-resolves provider context; providers never install navigation listeners.The Skilljar provider owns its LMS selectors and exposes semantic page context to the translation pipeline, quiz filtering, header controls, reading aid, and PDF export. Lesson keys omit query strings and fragments. No existing URL-keyed user data is migrated by this PR.
tests/lms-provider.test.jsregisters a fixture LMS to assert that a new provider can be added without touching core translation code, and that a restricted fixture halts at the probe phase.Type of Change
Testing
npm testpassesAutomated verification run on the rebased branch:
npm testnpx playwright test(exam-mode, spa-navigation, youtube-lifecycle, pdf-export, golden-translation)npx eslint src/ tests/ scripts/npm run typechecknpm run format:checknpm run check:selectorsnpm run validate/npm run glossarycheck-i18n-keys,check-locale-contamination,check-dict-coverage,check-dictscheck-bg-sync,check-permission-docs,build-plugin --checknpm run build:bundle/npm run build:firefoxgit diff --checkI have not re-run manual in-browser QA against a live lesson since rebasing onto
main, so those boxes are left unchecked deliberately. Suggested manual QA for review:QA Checklist (check items affected by your changes)
Expand checklist
Core Translation
golden-translation.spec.jsgolden-translation.spec.jsnpm run glossary/npm run validatepassExam Mode
exam-mode.spec.js(4 tests)Cross-Browser (touching manifest)
npm run build:firefoxsucceedsRisks
lessonIdentitylater only with backward-compatible reads and an explicit storage-version strategy.Related Issues
None yet. This is a standalone refactor with no behavior change intended.