Release 0.78.0 - #3864
Closed
odlbot wants to merge 14 commits into
Closed
Release 0.78.0#3864odlbot wants to merge 14 commits into
odlbot wants to merge 14 commits into
Conversation
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* adding staleness penalty for resource vector search * update tests
The Stay Updated button was gated on three things: the NEXT_PUBLIC_STAY_UPDATED_HUBSPOT_FORM_ID env var, the CMS page's show_stay_updated flag, and an enrollment-mode check requiring every enrollment mode to be "verified". The mode check was doing work the CMS flag already covers, and on courses it iterated course.courseruns unfiltered — including runs with live: false / is_enrollable: false that appear nowhere else on the page (the session selector filters to is_enrollable). One retired audit run therefore suppressed the button for the whole course. Example: course-v1:UAI_SOURCE+UAI.MLTL.1 has show_stay_updated: true and one enrollable verified-only run, but a second run (id 2465, live: false, is_enrollable: false) carries an "audit" mode, so the button never rendered. Visibility is now just: can it be shown (env var, checked in ProductPageTemplate) and should it be shown (page.show_stay_updated). Tests for each page collapse to those branches; the shared PROGRAM_HIDE_STAY_UPDATED_CASES only served the removed mode permutations. The env-var tests now set show_stay_updated explicitly — the page factory randomizes it, so without the mode clause they would have been coin-flips. Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
* Add overridable mutation error-toast infrastructure (Option 2)
Global MutationCache.onError shows a top-center error toast for every
browser mutation failure, so a failed mutation is never silent. Call
sites override via typed `meta` (declared in api/mutation-meta):
- `showErrorToast: false` to opt out (e.g. renders its own inline error)
- `errorMessage` / `getErrorMessage(error, variables)` for custom copy
`api` stays UI-free (declares typed `meta` only); the toast lives in
`main`. Copy resolution is defensive: a throwing getErrorMessage or
empty copy falls back to the generic message inside onError, so a bad
override can never make the failure silent again. The store holds one
toast; a newer error replaces it (never stacks), and it persists until
dismissed via smoot Alert's own close control.
The convention is documented in .github/instructions/frontend.instructions.md.
Not yet shippable: sites that already render inline alerts will
double-fire until opted out (follow-up migration).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Opt out surfaced inline-error sites from the global error toast
The global MutationCache.onError toast (previous commit) fires on every
browser mutation failure. Sites that already render their own co-located
inline error would therefore double-fire (inline alert + toast).
Give each shared api/ mutation hook an optional `meta` param
(MutationHookOptions, exported from api/mutation-meta) forwarded to
useMutation, and have every consumer that renders an inline error pass
`meta: SILENCE_ERROR_TOAST`. Opt-out lives at the consumer, never baked
into the shared hook, because several hooks (enrollment, baskets) are
used in both a surfaced and a silent context. UpgradeBanner opts out
only when its onUpgradeFailure callback is wired — without it the
banner has no error surface of its own, so the toast must stay.
Also mount <Toaster/> in renderWithProviders and reset the toast store
per test, so a missing opt-out at an inline-error site surfaces as a
double-role="alert" test failure rather than only showing up in manual
QA, and add end-to-end coverage (mutationErrorToast.test.tsx) that the
default toast fires and SILENCE_ERROR_TOAST suppresses it — so removing
the MutationCache wiring can no longer pass green.
Adopting the genuinely-silent sites (Start button, destroy dialogs,
listItemMove rollback) with tailored copy is a separate follow-up; those
already toast via the global default.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Strengthen consumer error-path tests around the toast opt-outs
Convert text-based error assertions to singular role queries
(findByRole("alert")) in the CourseEnrollmentDialog and JustInTimeDialog
tests, so a stray global toast — a missing opt-out, or a shared hook
that stops forwarding `meta` — fails these tests as a second alert.
Add failing-mutation coverage for all three DashboardDialogs (email
settings, unenroll, unenroll program), whose opt-outs and inline error
rendering were previously untested. Writing those tests surfaced that
each dialog awaited mutateAsync inside formik's onSubmit: the rejection
escaped onSubmit (formik logs an unhandled-error warning), and the
post-await isError guards were unreachable on the error path. Switch to
mutate(..., { onSuccess }) — same success/error behavior (inline alert
via isError, busy state via isPending), no escaping rejection.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Fail any test that leaves the global error toast unacknowledged
The double-alert test net only tripped where an error-path test happened
to assert via a singular role="alert" query. Make it structural: an
afterEach in setupJest fails any test that ends with the global error
toast still showing, and its message teaches the fix — opt the component
out with `meta: SILENCE_ERROR_TOAST` if it renders its own inline error,
or acknowledge the toast with the new `expectErrorToast(message)` helper
if the toast is the intended error surface. Every mutation failure a
test drives now requires a deliberate decision about its error surface.
The full main suite passes with zero acknowledgment churn — no existing
test drives a toasting failure it doesn't already account for.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
* Add tailored enroll error copy as errorMessage PoC
The dashboard enroll flow (useEnrollmentHandler) becomes the first adopter
of meta.errorMessage: a failed enroll toasts course- or program-specific
copy instead of the generic message, demonstrating the per-site override.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* Fall back to errorMessage when getErrorMessage throws or returns blank
resolveErrorMessage collapsed the dynamic and static tiers into one
nullish-coalescing expression, so a getErrorMessage that threw or
returned blank copy skipped a defined errorMessage and jumped straight
to the generic fallback. Evaluate the tiers separately so each falls
through to the next when it yields no message.
Flagged in review: #3837 (comment)
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
---------
Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…#3825) * ci: add a ci-gate job so one required check covers the whole suite GitHub's required status checks take an exact list of context names. There are no wildcards and no "all checks must pass" option, so requiring these six jobs from the ruleset in mitodl/ol-infrastructure means restating all six names there, and that list then goes stale in two ways. A renamed job keeps being required under its old name, which GitHub will never report again, so every PR waits forever on it. mitxonline hit exactly this when its python-tests job became a 4-way matrix and the checks turned into `python-tests (1)`..`(4)`. A newly added job is not required until somebody remembers to go and add it, so new CI silently cannot block a merge. `ci-gate` fails if any job it needs ends in anything but success or skipped. Requiring it instead keeps the list of what must pass in the same file as the jobs it names, so adding or renaming a job is one edit in one repo. openapi-diff is deliberately not covered: it lives in its own workflow, and it runs `oasdiff breaking --fail-on ERR`, so it goes red on intentional breaking API changes. It was red at merge on 8 of the last 40 merged PRs here, release PRs included, so gating merges on it would mean bypassing the ruleset routinely. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01QygRcd6WF4BmUi1C5cez4d * ci: wire up err-ignore allowlist for openapi-diff breaking check needs can't reach openapi-diff (separate workflow, pull_request trigger), so it can't join ci-gate directly. Give it the allowlist mechanism oasdiff already supports so intentional breaking changes stop needing a routine ruleset bypass, then require it alongside ci-gate in ol-infrastructure. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013wnKLHVJ2vP4G6mPN9AgFr * ci: move oasdiff-err-ignore.txt out of openapi/specs/ openapi_spec_check.sh diffs openapi/specs/ against freshly generated specs and fails on any extra file -- confirmed in the python-tests CI run for dcd62cf. Move the ignore list to openapi/ instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_013wnKLHVJ2vP4G6mPN9AgFr --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
…ync (#3808) * feat(cohort-1): MicroMasters/MITx Online certificate warehouse-pull sync Second PR in the replacement stack for closed mit-learn#3566, stacked on the StarRocks warehouse-pull machinery (#3807). Replaces Hightouch as the writer of external.programcertificate — currently broken since ~2025-06-01 — with a warehouse-pull Celery task reading integrations__learn__program_certificates (mitodl/ol-data-platform#2591). Resolves mitodl/hq#12954. - profiles/etl.py: transform_program_certificate + upsert_program_certificate. Deliberately never prunes, even on full_refresh — a certificate is a durable achievement record, not a catalog resource that should self-heal by deleting rows a pull didn't see (a transient upstream join gap must never read as "certificate revoked"). - profiles/tasks.py: SyncProgramCertificatesTask(BaseWarehouseETLTask), registered via app.register_task per the base class's documented requirement (the @app.task(base=...) decorator doesn't work for it). - main/settings_celery.py: one beat entry, gated on STARROCKS_HOST (daily). Doesn't participate in the WAREHOUSE_ETL_CUTOVER_SOURCES mechanism — Hightouch ran externally, not as an MIT Learn Celery task, so there's no legacy beat entry to retire. - main/settings.py: removed "programcertificate" from EXTERNAL_MODELS. That setting made main.routers.ExternalSchemaRouter reject every Django-side write to this model, correct back when only Hightouch wrote to it — this task is now the intended writer, so the guard no longer applies. Flagging explicitly: this removes a safety check (main/routers_test.py's test_external_tables_are_readonly, which asserted the exact behavior just removed, is deleted alongside it). Field set was checked directly against profiles.ProgramCertificate (not assumed from memory) — the model stores 11 more columns than first planned (user_edxorg_username, user_mitxonline_username, name/ demographic/address fields), all now covered by both the dbt model and this transform. * fix: require STARROCKS_USER too before scheduling the certificate sync The beat entry gate checked only STARROCKS_HOST, but _connect_starrocks() (learning_resources.lib.warehouse) also requires STARROCKS_USER — set HOST without USER and the task gets scheduled but fails every run with ImproperlyConfigured. Addresses sentry[bot] feedback on #3808. * Address review feedback: manage the model, isolate bad rows shanbady: with the warehouse-pull cutover Django owns writes to external.programcertificate, so it should own the schema too. Flipping managed generates a state-only migration — migration 0015 already created the table, so there is no DDL and no data risk. shanbady: one malformed certificate row no longer aborts the batch. Skipped rows self-heal because the daily beat schedule runs full_refresh over the whole view. An all-rows-failed batch still raises: that is a broken database rather than a bad row, and returning normally would advance an incremental run's watermark past a window nothing was written for. Copilot: the mapping test claimed to cover every field but left the three address fields null and unasserted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01JFhTucXavYwkjv6o4qXZ8r --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* Submit and link each resource under one URL, from learn_url Four places derived a resource's URL on Learn independently — the resources, video, podcast and products sitemaps — and the card href and drawer canonical derived it a fifth and sixth time. They did not agree, and the resources sitemap submitted a drawer URL for every resource on top of whatever dedicated page the per-type sitemaps submitted. A video was therefore advertised twice, and because the drawer canonicalized to itself, both URLs claimed to be the original and crawlers had no way to consolidate ranking signal onto the dedicated page. Read `learn_url` instead, which the backend computes as the resource's own page where it has one and its search drawer where it does not: - The card href points at it. `pushUrl` is untouched, so a click still opens the drawer in place and nothing changes for users; only what a crawler follows changes. - The drawer canonicalizes to it. For a video or podcast episode that is now the dedicated page rather than the drawer itself, which is what actually consolidates the signal — excluding a URL from the sitemap only stops us advertising it, and says nothing about one that is linked or shared. - The resources sitemap emits it for every published resource, so the video, podcast and products sitemaps are gone: they existed to emit dedicated-page URLs the frontend had to build itself. Collapsing the sitemaps also settles which parent scopes an episode's URL. The podcast sitemap mapped over every parent podcast, so an episode with two parents would have been submitted twice; one URL per resource cannot. The count query now asks for `limit: 1`. It reads only `count`, so it was fetching a thousand rows and discarding them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Fall back to the drawer URL when learn_url is missing learn_url is non-nullable and never blank, but a frontend deployed ahead of the backend sees the field absent, and both call sites degrade badly on a falsy value rather than merely pointing somewhere less ideal: - the drawer emitted no canonical tag at all, which is worse than the self-canonical it replaces - the card dropped its href, and BaseLearningResourceCard's `linkTarget` drops `pushUrl` along with a falsy href — so the title would be neither a link nor clickable, breaking the drawer for that card Fall back to the drawer URL in both, and assert it: reverting either fix fails the new cases. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Describe what the sitemap and canonical do, not what changed Both comments narrated the edit, which only parses while holding the diff. State the durable facts instead: the sitemap needs no per-type branching and a resource with several parents appears once under its canonical parent; the drawer hands its ranking signal to the resource's dedicated page. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Trim the drawer canonical comment to the invariant Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Narrow the falsified-learn_url cast to the one field `as never` on the whole object drops type checking for every other prop, so a later change to the resource shape would not surface here. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Share the resource's page rather than a drawer over search A shared link now lands on the page that owns the content where one exists, so sharing stops seeding drawer URLs for crawlers to find. Slightly user-visible: the recipient sees the dedicated page instead of the search page with a drawer over it. The drawer reads the detail endpoint, which always carries learn_url, so the fallback here is only for a frontend deployed ahead of the backend. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Read learn_url directly, without a fallback learn_url is non-nullable, so the drawer canonical, the card href and the share URL can read it as given. Drops the `||` branches, the comments explaining them, and the absent/blank cases that covered them. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
* Update dependency drf-spectacular to >=0.30,<0.31 * update spec --------- Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> Co-authored-by: Anastasia Beglova <abeglova@mit.edu>
OpenAPI Changes418 changes: 366 error, 0 warning, 52 info Unexpected changes? Ensure your branch is up-to-date with |
odlbot
added
waiting for checkboxes
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
6 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.
Matt Bertrand
renovate[bot]
Ahtesham Quraish
Tobias Macey
Chris Chudzicki
Shankar Ambady