Release 0.70.1#3395
Merged
Merged
Conversation
Add support for learn embed urls --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.1.7>
* add initial support for variants * restructure the remaining things using "language support" to use variants instead * pass the actual variants instead of just a value and label * fix default variant selection, refactor key and label functions * make casting a bit more sane * reorganize hooks and view model code based on our organization strategy * bump api package * add variantpicker and use it on contractcontent, remove language picker from programenrollmentdisplay * bump api package * use variant_options on contractpage * fix nextJS build issue * fix more nextjs build issues * remove accidentally committed local dev arg * remove stray comment * remove stray default value * accessibility fixes for variant picker * add variant picker tests * remove unnecessary casts * remove unnecessary comment * replace learning language in tests * strip out the rest of the language picker stuff from the b2c program dashboard * add a null guard for no selected variant * update to released API package * update package version again * refine variant run matching * simplify courseentry stuff * fix run selection bug and make it aware of the user's enrollments * fix sort order * fix typescript error * fix test failures * remove remaining language picker code * fix yarn lock after rebase * strip out unnecessary selectedVariantKey / availableVariants arguments * move language code normalization to the proper place * simplify filtering logic * remove branch of dashboardcard code that was handling synthetic runs from the language picker * update docstring * clean up unnecessary non-null assertion
* docs: add API runtime client config design
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: refine API runtime client config design
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: address API runtime spec review
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* docs: incorporate second spec review round
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* revert: remove committed API runtime spec
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat: add runtime API client configuration core
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor(task2): derive test URLs from runtime config
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: normalize paginated API follow-up URLs
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* feat: bootstrap API clients from main runtime
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: move MITx basket redirect into main
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* test: mark API runtime test files as modules
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: own env nullability at bootstrap boundary
Replace non-null assertions in bootstrapApiClients with a requireEnv helper
backed by tiny-invariant. Errors now name the missing env var instead of
the downstream config field. configureApiClients keeps its strict signature
so callers retain accurate type hints and compile-time checks.
Also document why bootstrapApiClients() must run at module scope in
providers.tsx — child components fire React Query during render, before
any effect would run.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: drop configsMatch; first-wins via isApiClientsConfigured
configureApiClients now throws on any second call, eliminating the
hand-maintained (or isEqual-based) deep equality check and its
maintenance footgun. First-wins idempotency moves to a single explicit
gate in bootstrapApiClients(), using a new isApiClientsConfigured()
predicate. resetApiClientsForTests remains the test escape hatch.
This separates concerns: configureApiClients writes config once,
isApiClientsConfigured answers "have we?", and bootstrapApiClients
owns the "exactly once across triggers" policy in one obvious place.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix: address review feedback on runtime config
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* refactor: drop runtime shape validation; trust the TS
configureApiClients's signature already guarantees ApiClientsConfig.
The isRecord/typeof guards re-checked what the type promises and the
tests that exercised them had to cast through 'as unknown as' to even
reach the runtime branch — defending JS-level inputs that the typed
surface won't produce.
What stays: normalizeBaseUrl still throws when the post-normalize value
is empty (the slash-only case), which is a real invariant (an empty
baseURL would silently break the test mock's URL matching).
Also: useReplaceBasketItem.test.tsx now restores process.env in
beforeEach so the 'throws at module load' test is robust to reordering
instead of relying on Jest source order.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: remove dead queries.test.ts
Filename suggested coverage of queries.ts but the file only re-tested
toRelativeApiUrl, which runtime/urls.test.ts already covers. The
jest.resetModules + dynamic import + process.env mutation scaffolding
was solving a problem that doesn't exist: toRelativeApiUrl is a pure
function and never reads env.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: trim redundant assertions
runtime/index.test.ts: drop Object.isFrozen and Reflect.set return-value
assertions from the frozen-config test. They were verifying JS engine
semantics around Object.freeze. The post-mutation read-back is the
behavioral guard that actually catches freezeConfig regressions: if
freeze were removed, the Reflect.set calls would succeed and the
read-back would observe the mutated values.
bootstrap/api.test.ts: drop the mitxonline propagation block from the
server-URL test. The browser-URL test already asserts the full config
shape; the server test now narrows to the Learn server-fallback URL,
which is the only thing that varies between the two cases.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* chore: address lint errors
- Delete frontends/api/src/clients.test.ts — tautological test that
asserted a hardcoded constant; flagged by jest/no-export rule.
- Drop the orphaned BASE_PATH export from clients.ts; it was widened
only to feed the deleted test.
- Convert stray require("../runtime") in api setupJest.ts to a static
import; flagged by @typescript-eslint/no-require-imports.
- Add an eslint-disable for the legitimate require() inside
jest.isolateModules in useReplaceBasketItem.test.tsx — the callback
must be synchronous, so require is the only way to load the module.
The remaining warning in useContractDashboardData.ts is pre-existing
and not in scope for this branch.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test: migrate assertions to adapter makeRequest signature
The cc/mock-axios-adapter mock changed makeRequest from positional
(method, url, body) to a single object arg ({ method, url, body }).
Three new tests added on this branch (the pagination tests for
learningResources and userLists, and the resolved-URL example in
mock-requests-example) still used the positional form; migrate them
to expect.objectContaining({ method, url }) to match the adapter.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* refactor: move client API bootstrap to instrumentation-client
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
* fix: avoid ReDoS in baseUrl normalization; fix lint/format
- normalizeBaseUrl: replace the trailing-slash regex /\/+$/ with a linear
index scan. CodeQL flagged the regex as polynomial ReDoS — it backtracks
O(n^2) on inputs with many trailing slashes, and baseUrl is env-derived
(uncontrolled per static analysis). Behavior is unchanged: strip trailing
slashes, reject an empty result.
- Prettier: wrap two long template-literal arrows in the test URL builders
(the getApiBaseUrl() prefix pushed them past print width).
- Add an eslint-disable for the jest.isolateModules require() in
instrumentation-client.test.ts (sync callback; require is required).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: strengthen runtime config and basket coverage
- runtime config: assert configureApiClients propagates xsrfCookieName and
withCredentials (not just baseURL) to both axios singletons — the
CSRF/credentials wiring this branch establishes was previously unverified.
- runtime config: drop the freeze test's axios-default assertions; they could
not fail for the freeze invariant (axios holds an independent copy) and the
propagation is now covered above.
- useReplaceBasketItem: add failure-path tests proving no add + no redirect
when the sync clear never succeeds or the async clear rejects. The sync case
also pins the clear->add->redirect nesting.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: capture bootstrap failures in Sentry; fix stale comment
- instrumentation-client: run bootstrapApiClients() after Sentry.init so a
missing-env failure during bootstrap is reported to Sentry instead of
crashing before error monitoring is armed. Still runs at module load,
before hydration, so React Query hooks remain safe on first paint.
- bootstrap/api.ts: correct the first-wins comment — providers.tsx no longer
calls bootstrap, so the server configures once at startup (instrumentation.ts)
and the browser once (instrumentation-client.ts, re-entrant under Fast Refresh).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: extract createConfigurableAxios; share instance via globalThis
Collapse the near-identical Learn and MITx axios scaffolds into a single
createConfigurableAxios(registryKey) factory: axios.create + the
fail-loud 'not configured' request interceptor + apply/reset helpers.
Park the instance on globalThis under Symbol.for(registryKey) so it is a
true per-process singleton. Next.js evaluates the instrumentation runtime
and the server render runtime as separate module graphs; without sharing,
configureApiClients() from instrumentation would mutate only
instrumentation's copy and leave the render graph unconfigured. The global
symbol registry gives every module graph the same slot.
The configured-state guard now derives from instance.defaults.baseURL
presence rather than a separate boolean, removing one of the duplicated
sources of truth.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: unify API client config source of truth; fix client-side env
Make configurableAxios the sole owner of the globalThis singleton mechanism
and have the runtime config derive all state through it, so the configured
instances and the "configured?" signal can no longer disagree across Next.js's
separate module graphs.
- Extract the globalThis/Symbol.for helper into configurableAxios and drop the
standalone globalSingleton module; runtime no longer keeps its own
module-local config state (which was reset per graph / per Fast Refresh while
the instances persisted).
- Derive isApiClientsConfigured() from the shared instances; configureApiClients
returns void.
- Remove getApiClientsConfig / client getConfig: their only consumers were the
url test-utils, which only need the base URL. They now read it straight off
the configured axios instance (the single source of truth).
- Fix bootstrap/api.ts requireEnv: read each NEXT_PUBLIC_* var as a static
literal so Next inlines it client-side. Dynamic process.env[name] lookups
yield undefined in the browser, throwing even when the var is set.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix: handle blank server API URL; tidy runtime config types
- bootstrap: coalesce empty NEXT_SERVER_MITOL_API_BASE_URL with `||` so a
blank value (env/codespaces.env) falls back to the public URL instead of
throwing during normalization; add regression test
- runtime: collapse identical LearnApiConfig/MitxOnlineApiConfig into the
canonical ConfigurableAxiosConfig
- runtime/urls: restore hq#10999 rationale as a docstring on toRelativeApiUrl
- remove tautological instrumentation-client.test.ts
- move lodash back to devDependencies (test-utils only)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* test: dedupe pagination-normalization tests into shared helper
The three infinite-hook tests (learningPaths, learningResources, userLists)
were near-identical copy-paste asserting the same toRelativeApiUrl wiring.
Extract the body into assertNormalizesPaginationNext in hooks/test-utils, so
each hook keeps a thin co-located test (drift protection) without the
boilerplate. Also allow devDependencies in **/test-utils.tsx (the glob only
listed the .ts variant).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
* refactor: track axios-configured state via explicit instance flag
isConfigured previously inferred "configured" from defaults.baseURL, which
conflates "configure ran" with "a default happens to be set". Track it
explicitly with a Symbol.for-keyed flag on the (already-singleton) instance —
cross-graph visible without a separate globalThis entry — and give
createConfigurableAxios an explicit ConfigurableAxiosClient return type so the
internal marker doesn't leak.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
---------
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
OpenAPI ChangesNo changes detected Unexpected changes? Ensure your branch is up-to-date with |
odlbot
added
waiting for checkboxes
all checkboxes checked
and removed
deploying to rc
waiting for checkboxes
labels
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
5 participants
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.
Carey P Gumaer
Chris Chudzicki
Danielle Frappier
Ahtesham Quraish