Release 0.80.2 - #3932
Merged
Merged
Release 0.80.2#3932
Conversation
…3924) The Institute has strict guidelines around the use of the MIT name
* Build internal resource links from learn_url Eleven components built links to related resources from (id, parent, title), slugifying each title locally: the podcast and video listings, the "more from this playlist" and "up next" rails, the series navigation, and the drawer's call-to-action. They now read the resource's learn_url, so the backend is the only thing that decides how a URL is spelled. Where a link is scoped by context rather than by the resource — a video in the playlist you are browsing, an episode under the podcast you arrived from — only the slug comes from learn_url and the parent segment stays as given. Podcasts and playlists have no such parent and use learn_url whole. Share URLs use it whole too, so sharing hands out the URL that owns the content rather than the context it was found in. The call-to-action keeps its membership guards: a video outside any playlist, or an episode with no podcast, still falls through to the source URL rather than to a Learn page. `PodcastEpisodeParentSerializer` gains learn_url. The episode page names its series in a breadcrumb and in its PodcastEpisode JSON-LD, and the embedded parent carried only an id, so `partOfSeries.url` would otherwise have to name a URL that redirects. `parent` is already select_related by the `_podcasts` prefetch, so this costs no extra query. With those callers converted, pathSlug and the four title-based path builders are unused and removed. slugify remains only for the drawer's cosmetic `resource_title`, which the backend cannot supply per-resource: a resource with a dedicated page has no resource_title in its learn_url. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Document the slug contract these path helpers actually have The block described a `title` argument that no helper takes, and attributed the "resource" fallback to the frontend when the backend owns it. State what the helpers do: take an already-derived slug, emit the bare redirecting path when it is undefined, and in generateVideoPlaylistPath's case return that bare form unconditionally. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Assert the episode row hrefs on the podcast page The fixture set each episode's parent ids but left its own learn_url drawer-shaped, so learnUrlSlug yielded "search" and every row rendered /podcast/<id>/podcast_episode/<id>/search. Nothing asserted the href, so the malformed link was invisible. Give episodes a dedicated-page learn_url, and pin the hrefs in the list test so a drawer-shaped fixture cannot quietly return. Add a case where the episode's canonical learn_url names a different parent — an episode in several podcasts is viewable under any of them — proving the row keeps the podcast being viewed as context and borrows only the slug. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Assert the "More from" row hrefs on the episode page The section's tests only checked that titles rendered, and the factory left each episode's learn_url drawer-shaped, so learnUrlSlug yielded "search" and every row rendered /podcast/<context>/podcast_episode/<id>/search undetected. Give factory episodes a dedicated-page learn_url scoped to a deliberately different canonical parent, so every row exercises the case an episode in several podcasts presents. Assert the hrefs by role: the context segment is the podcast being viewed, the slug comes from learn_url. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Assert the remaining context-plus-slug hrefs Two more rails read their slug from learn_url while keeping their own context, and neither had an href pinned: the latest-episodes list on /podcasts, and the "more from this playlist" rows on the video page. Both fixtures kept the factory's drawer-shaped learn_url, so each row rendered a "/search" slug with no test failing. Scope both fixtures' learn_url to a deliberately different canonical parent, so every row exercises the multi-parent case, and assert the hrefs by role. In VideoDetailPage the fixture default is applied only when a caller has not overridden learn_url, so the share-URL test still names its own value. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Stop justifying learn_url by naming the UI that reads it A fixture that explains a field by pointing at a breadcrumb and a JSON-LD block goes stale the moment either changes, and nobody returns to a fixture comment to notice. Drop it from both factories. The serializer docstring loses the same sentence but keeps what a reader cannot infer from the code: why the parent list is empty, and that `parent` is already select_related so the field costs no query. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> * Return a resource's URL slug as a top-level API field Consumers that build a resource page path need the slug segment, and the only way to get it today is to parse it back out of learn_url. Serve it directly as url_slug. * Consume url_slug; keep context for UI and share, canonical for crawlers Replaces the fifteen `learnUrlSlug(learn_url)` call sites with the resource's `url_slug` and drops both `learnUrlSlug` and `learnUrlPath`: Next treats an absolute same-origin href the same as a path, and the redirect pages now build their canonical from the slug directly. Splits the three uses of a resource URL the way the review asks: - HTML UI (breadcrumbs, carousels, "view all") keeps parent context - Share keeps parent context too, as `main` did — a video shared from the xTalk playlist should not land on a different series - JSON-LD is canonical throughout. `buildPodcastEpisodeStructuredData` now takes the canonical parent from the episode itself, so `url` and `partOfSeries` can no longer name two different podcasts. * Pass a playlist id as the string the API declares `VideoResource.playlists` is Array<string>, unlike an episode's `podcasts`, which is Array<number>. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * Build the episode share URL with absoluteUrl Matches the two video pages and drops the file's only use of env(). * Let the podcast fixture keep the factory's learn_url The href assertion compares against the fixture's own value, so a hand-written URL asserted nothing. The factory's drawer-shaped default makes it fail if the component builds the path instead of using learn_url. * Compare both sides of a canonical redirect with the same builder The incoming path was written out by hand at all four [slug] pages, so it repeated what the builder already knows and could drift from it. It also escaped the id segment differently: the builder runs ids through generatePath, the hand-written copy did not. videoDetailPath now takes a string id and playlist so the video page can pass its route params straight in. * Drop a String() on a value already typed string podcastEpisodePath takes both ids as strings, unlike the builders that accept number | string and need the conversion. --------- Co-authored-by: Ahtesham Quraish <ahtesham.quraish@arbisoft.com> Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com> Co-authored-by: Ahtesham Quraish <ahtesham.quraish@192.168.1.215> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
* feat(ovs): sanitize and render rich-text descriptions from OVS OVS is gaining a rich-text editor for video and collection descriptions (mitodl/hq#13064, mitodl/odl-video-service#1585). Two things follow for Learn. First, and independent of that work: the OVS ETL copied `description` straight through, and six components render it with dangerouslySetInnerHTML. Descriptions are plain text today so nothing is exploitable, but the moment OVS allows formatting this is live HTML from a source we do not control. `clean_description` runs both the video and the collection description through the same allowlist the podcast ETL uses for show notes - links kept, because links are the point of the feature. Second, the display side assumed plain text in places: - the series header interpolated `playlist.description`, so an author's formatting would have reached the learner as visible tags. - VideoCard, SeriesVideoList and MoreFromPlaylist inject the description inside a Link, where an author's <a> is a nested anchor - invalid HTML that browsers resolve by splitting the row's own link. They now use the existing stripAnchorTags helper, keeping the words. - descriptions had no styling for lists, links or emphasis. A shared richTextDescription rule set covers the full-width surfaces; the clamped previews flatten lists inline so a list cannot blow the box out. - the schema.org VideoObject description carried raw markup. It is stripped locally rather than with common/htmlToPlainText, which is documented server-only (it pulls in isomorphic-dompurify) and this module is imported by a "use client" component. * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * chore: prettier formatting * fix(ovs): strip tags to a fixed point in the JSON-LD description CodeQL, correctly: a single-pass replace(/<[^>]*>/g, "") is not idempotent - removing one match can join its neighbours into a new tag, so <scr<div>ipt> survives as <script>. This value lands inside a <script type="application/ld+json"> block. Not exploitable as it stood, because the injection point escapes </ - but a helper that claims to return plain text should not lean on its caller for that, and residual markup in schema.org data is wrong regardless. Now loops to a fixed point, decodes entities before dropping any surviving angle bracket so an entity cannot smuggle one back in. * fix(ovs): render rich descriptions in valid containers Review feedback from Copilot on #3879, all of it correct. Every description surface was a Typography, and this theme's variantMapping sends body1/body2 to <p> (smoot-design typography.ts) - so injecting <p>/<ul> put block elements inside a <p>. That is invalid markup, browsers reparent it, and it showed up as a real SSR hydration tree mismatch in the browser: React reported <p> containing <p> and <ul>. The CSS flattening only changed presentation, not validity. All five now take component="div", typed with Pick<TypographyProps, "component"> following the pattern PodcastPage/EpisodeContentTabs.tsx already established for its own sanitized descriptions: DescriptionText, MoreFromItemMeta, PageDescription, EpisodeDescription, CardMetaValue. Verified in a browser - the tree mismatch is gone; the only hydration warning left is Grammarly writing data-gr-ext-installed onto <body>. Two more findings from the same review: - FeaturedVideo was a fourth description surface I missed entirely. It rendered into a literal styled.p with a two-line clamp. Now a div with the same inline flattening as the other previews. Anchors are kept here, unlike the card and episode rows: this description is a sibling of the title link rather than inside it, so a link is valid and clickable. - the JSON-LD stripper only treated p/li/ul/ol/blockquote as boundaries, but the backend allowlist also keeps div, pre, caption, center, q and hr. So <div>First</div><div>Second</div> came out as "FirstSecond". All of them are boundaries now, with a regression case per tag. Adds videoStructuredData.test.ts (16 cases): the per-tag boundaries above plus the tag-reconstitution inputs behind the earlier CodeQL fix. * fix(ovs): complete the container fix CI caught Three typecheck errors on the previous commit, all mine. Two files used Pick<TypographyProps, "component"> without importing the type. Folded into the existing ol-components import rather than a second one, which import/no-duplicates rejects. The third was more than a missing import: VideoSeriesDetailPage renders Styled.DescriptionText from its own VideoSeriesDetailPage.styled, not from VideoDetailPage.styled. So the series page had a second, untyped DescriptionText that also never received the rich-text rules - lists and links on that page were unstyled, which the earlier commit missed entirely. It now takes the component generic and the shared rules. richTextDescription accordingly moves to shared.styled, which both page styled modules and VideoPageHeader already import, instead of one page's styled module importing another's. Local typecheck had hidden all of this: it aborts in the api workspace on a stale generated mitxonline client, so it never reached frontends/main. Checked with `yarn workspace main exec tsc --noEmit` this time - no errors in these files. lint-check and fmt-check clean. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
…TAIL rows (#3915) * fix(sentry): cap request bodies at 1KB and scrub Postgres DETAIL rows Two ways learner data reaches Sentry, neither gated by send_default_pii. Request bodies. The SDK sets request.data unconditionally at sentry_sdk/integrations/_wsgi_common.py:123; max_request_body_size, checked at :61, is the only control, and left unset it defaults to "medium" -- 10,000-byte bodies. Nobody chose that. Measured over the last 30 days, the write endpoints that actually raise are the sensitive ones: SCIM user PATCH, /api/v1/enrollments/, /api/checkout/result/, /api/checkout/redeem_discount/, /api/profile/details/, and CMS page edits. Set to "small" explicitly, so the choice is findable at the call site instead of in a dependency's defaults. Postgres DETAIL lines. A constraint violation carries a DETAIL line that echoes the whole offending row, and psycopg puts it in str(exc) -- so it ships inside the exception value, which no SDK privacy option covers. Measured on mitxonline MITXONLINE-6PK: a SCIM PATCH IntegrityError reproducing a learner email address three times per event, 46,764 occurrences since 2026-05-27. before_send now truncates at the DETAIL marker across exception values, logentry, and the legacy top-level message, keeping the primary error that names the failure. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RVJKTGk9KHUTN2xfU59ujX * fix(sentry): scrub DETAIL rows from the whole event, not three named fields Copilot review, verified against sentry-sdk 2.55.0 source. The first pass enumerated three paths -- exception values, logentry.message/.formatted, and the legacy top-level message -- and missed every other field that can carry the same string: breadcrumbs[].message LoggingIntegration records each log record as a breadcrumb (integrations/logging.py:311). This is exactly MITXONLINE-6PK's shape: mechanism=logging, logger=django_scim.views. logentry.params record.args verbatim (:274), so logger.error("...: %s", exc) carries it. frames[].vars include_local_variables defaults to True (consts.py:1028, utils.py:616), so a catch block holding the exception in a local carries it. Confirmed the old implementation leaked on all three shapes before changing it; the new tests fail against it and pass against the walk. Replaced with a recursive walk of the event instead of a longer path list -- it covers these without enumerating them and does not go stale when the SDK grows another such field. The walk only rewrites str leaves and preserves everything else, with a test pinning that. Copilot also suggested normalizing exception-valued params. Not needed: client._prepare_event serializes the event before calling before_send (client.py:650 vs :658), so every leaf is already a JSON primitive by then and there are no live exception objects left to coerce. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01RVJKTGk9KHUTN2xfU59ujX * fix(sentry): scrub repr'd DETAIL lines and narrow the body-size comment The SDK repr()s frame locals and logging params before before_send, so the DETAIL line there carries a literal backslash-n and the old find() missed it. Match both forms, and test through the real SDK. The body-size comment was copied from mitxonline; it now describes what this app receives. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MYc2F3cFvSCfVzqeRMshGy --------- Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
mitol-drf-lint 2026.8.28 added cross-file analysis and rules ORM003-ORM009; the hook pins no version, so pre-commit.ci picked the new release up when its cached env rebuilt. The checked-in baseline only covered ORM002, leaving 41 pre-existing N+1 risks unsuppressed and every build in the repo red. Regenerate the baseline over all tracked serializers.py so existing debt is grandfathered and recorded, while new violations still fail the hook. Existing ORM002 entries are preserved; nothing is dropped. Also ignore .drf_lint_cache.json, the cross-file index cache the new version writes at the repo root.
* pass start date into enrollment handlers and make sure a course has actually started before automatically redirecting to it * titles should also not be clickable by non-admins if the course hasn't started yet * allow Toaster to also display success messages and show one when you successfully enroll in a course but are not redirected * revert the enroll success toast Adding a success Snackbar belongs in smoot-design with a design that every surface can reuse, not bolted onto an unrelated bug fix. Reverted here and tracked separately. The start-date gate is unchanged: enrolling in a run that has not started still enrolls without redirecting, and the card re-renders as enrolled showing when the run starts. Toaster, toastStore and the test harness are back to their state on main. The comment rework and the SiblingRunsAccordion wording that rode along in the same commit are kept, which is why this is a manual revert rather than a git revert.
OpenAPI Changes26 changes: 0 error, 0 warning, 26 info 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
all checkboxes checked
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
7 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
Ahtisham Shahid
Tobias Macey
Ahtesham Quraish
renovate[bot]
Matt Bertrand
Peter Pinch