diff --git a/RELEASE.rst b/RELEASE.rst index 1bd4c4cac9..91cbdc3d35 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,19 @@ Release Notes ============= +Version 0.72.2 +-------------- + +- update elipses css (#3533) +- fix(search): pass pool_maxsize instead of connections_per_node to opensearch-py (#3544) +- chore: remove video-shorts feature flag (#3537) +- chore: Remove podcast-detail-page feature flag (#3541) +- chore: remove video-playlist-page feature flag (#3538) +- make uai heading navigable (#3532) +- Hybrid Search: Fix "resource type" filter (#3528) +- feat: Implement timestamp-aware share/embed links for video player (#3513) +- Fix flaky test (#3536) + Version 0.72.1 (Released June 30, 2026) -------------- diff --git a/frontends/api/src/generated/v0/api.ts b/frontends/api/src/generated/v0/api.ts index 4bcd1484f4..119e0e671f 100644 --- a/frontends/api/src/generated/v0/api.ts +++ b/frontends/api/src/generated/v0/api.ts @@ -10822,6 +10822,7 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( * @param {boolean} [published] If the resource is published. We default to True unless passed in * @param {string} [q] The search text * @param {string} [readable_id] The readable id of the resource + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {number} [score_cutoff] The minimum score a result must have to be returned. Defaults to 0.0 when omitted, but the server clamps the effective cutoff to the minimum allowed for the selected search mode (dense or hybrid). @@ -10851,6 +10852,7 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( published?: boolean, q?: string, readable_id?: string, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, score_cutoff?: number, @@ -10948,6 +10950,10 @@ export const VectorLearningResourcesSearchApiAxiosParamCreator = function ( localVarQueryParameter["readable_id"] = readable_id } + if (resource_category) { + localVarQueryParameter["resource_category"] = resource_category + } + if (resource_type) { localVarQueryParameter["resource_type"] = resource_type } @@ -11024,6 +11030,7 @@ export const VectorLearningResourcesSearchApiFp = function ( * @param {boolean} [published] If the resource is published. We default to True unless passed in * @param {string} [q] The search text * @param {string} [readable_id] The readable id of the resource + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {number} [score_cutoff] The minimum score a result must have to be returned. Defaults to 0.0 when omitted, but the server clamps the effective cutoff to the minimum allowed for the selected search mode (dense or hybrid). @@ -11053,6 +11060,7 @@ export const VectorLearningResourcesSearchApiFp = function ( published?: boolean, q?: string, readable_id?: string, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, score_cutoff?: number, @@ -11087,6 +11095,7 @@ export const VectorLearningResourcesSearchApiFp = function ( published, q, readable_id, + resource_category, resource_type, resource_type_group, score_cutoff, @@ -11154,6 +11163,7 @@ export const VectorLearningResourcesSearchApiFactory = function ( requestParameters.published, requestParameters.q, requestParameters.readable_id, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.score_cutoff, @@ -11300,6 +11310,13 @@ export interface VectorLearningResourcesSearchApiVectorLearningResourcesSearchRe */ readonly readable_id?: string + /** + * The resource category for the resource + * @type {Array} + * @memberof VectorLearningResourcesSearchApiVectorLearningResourcesSearchRetrieve + */ + readonly resource_category?: Array + /** * The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @type {Array<'course' | 'program' | 'learning_path' | 'podcast' | 'podcast_episode' | 'video' | 'video_playlist' | 'document'>} @@ -11389,6 +11406,7 @@ export class VectorLearningResourcesSearchApi extends BaseAPI { requestParameters.published, requestParameters.q, requestParameters.readable_id, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.score_cutoff, diff --git a/frontends/api/src/generated/v1/api.ts b/frontends/api/src/generated/v1/api.ts index 34a26cc995..a71480a838 100644 --- a/frontends/api/src/generated/v1/api.ts +++ b/frontends/api/src/generated/v1/api.ts @@ -16586,6 +16586,7 @@ export const LearningResourcesApiAxiosParamCreator = function ( * @param {Array} [offered_by] The organization that offers the learning resource * `mitx` - MITx * `ocw` - MIT OpenCourseWare * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `climate` - MIT Climate * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ @@ -16606,6 +16607,7 @@ export const LearningResourcesApiAxiosParamCreator = function ( offered_by?: Array, platform?: Array, professional?: boolean | null, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, topic?: Array, @@ -16680,6 +16682,10 @@ export const LearningResourcesApiAxiosParamCreator = function ( localVarQueryParameter["professional"] = professional } + if (resource_category) { + localVarQueryParameter["resource_category"] = resource_category + } + if (resource_type) { localVarQueryParameter["resource_type"] = resource_type } @@ -16930,6 +16936,7 @@ export const LearningResourcesApiAxiosParamCreator = function ( * @param {Array} [offered_by] The organization that offers the learning resource * `mitx` - MITx * `ocw` - MIT OpenCourseWare * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `climate` - MIT Climate * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ @@ -16950,6 +16957,7 @@ export const LearningResourcesApiAxiosParamCreator = function ( offered_by?: Array, platform?: Array, professional?: boolean | null, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, topic?: Array, @@ -17025,6 +17033,10 @@ export const LearningResourcesApiAxiosParamCreator = function ( localVarQueryParameter["professional"] = professional } + if (resource_category) { + localVarQueryParameter["resource_category"] = resource_category + } + if (resource_type) { localVarQueryParameter["resource_type"] = resource_type } @@ -17406,6 +17418,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { * @param {Array} [offered_by] The organization that offers the learning resource * `mitx` - MITx * `ocw` - MIT OpenCourseWare * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `climate` - MIT Climate * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ @@ -17426,6 +17439,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { offered_by?: Array, platform?: Array, professional?: boolean | null, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, topic?: Array, @@ -17451,6 +17465,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { offered_by, platform, professional, + resource_category, resource_type, resource_type_group, topic, @@ -17610,6 +17625,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { * @param {Array} [offered_by] The organization that offers the learning resource * `mitx` - MITx * `ocw` - MIT OpenCourseWare * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `climate` - MIT Climate * @param {Array} [platform] The platform on which the learning resource is offered * `edx` - edX * `ocw` - MIT OpenCourseWare * `oll` - Open Learning Library * `mitxonline` - MITx Online * `bootcamps` - Bootcamps * `xpro` - MIT xPRO * `csail` - CSAIL * `mitpe` - MIT Professional Education * `see` - MIT Sloan Executive Education * `scc` - Schwarzman College of Computing * `ctl` - Center for Transportation & Logistics * `whu` - WHU * `susskind` - Susskind * `globalalumni` - Global Alumni * `simplilearn` - Simplilearn * `emeritus` - Emeritus * `podcast` - Podcast * `youtube` - YouTube * `canvas` - Canvas * `climate` - MIT Climate * `ovs` - ODL Video Service * @param {boolean | null} [professional] + * @param {Array} [resource_category] The resource category for the resource * @param {Array} [resource_type] The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @param {Array} [resource_type_group] The category of learning resource * `course` - Course * `program` - Program * `learning_material` - Learning Material * @param {Array} [topic] The topic name. To see a list of options go to api/v1/topics/ @@ -17630,6 +17646,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { offered_by?: Array, platform?: Array, professional?: boolean | null, + resource_category?: Array, resource_type?: Array, resource_type_group?: Array, topic?: Array, @@ -17655,6 +17672,7 @@ export const LearningResourcesApiFp = function (configuration?: Configuration) { offered_by, platform, professional, + resource_category, resource_type, resource_type_group, topic, @@ -17868,6 +17886,7 @@ export const LearningResourcesApiFactory = function ( requestParameters.offered_by, requestParameters.platform, requestParameters.professional, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.topic, @@ -17956,6 +17975,7 @@ export const LearningResourcesApiFactory = function ( requestParameters.offered_by, requestParameters.platform, requestParameters.professional, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.topic, @@ -18385,6 +18405,13 @@ export interface LearningResourcesApiLearningResourcesSimilarListRequest { */ readonly professional?: boolean | null + /** + * The resource category for the resource + * @type {Array} + * @memberof LearningResourcesApiLearningResourcesSimilarList + */ + readonly resource_category?: Array + /** * The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @type {Array<'course' | 'program' | 'learning_path' | 'podcast' | 'podcast_episode' | 'video' | 'video_playlist' | 'document'>} @@ -18665,6 +18692,13 @@ export interface LearningResourcesApiLearningResourcesVectorSimilarListRequest { */ readonly professional?: boolean | null + /** + * The resource category for the resource + * @type {Array} + * @memberof LearningResourcesApiLearningResourcesVectorSimilarList + */ + readonly resource_category?: Array + /** * The type of learning resource * `course` - course * `program` - program * `learning_path` - learning path * `podcast` - podcast * `podcast_episode` - podcast episode * `video` - video * `video_playlist` - video playlist * `document` - document * @type {Array<'course' | 'program' | 'learning_path' | 'podcast' | 'podcast_episode' | 'video' | 'video_playlist' | 'document'>} @@ -18890,6 +18924,7 @@ export class LearningResourcesApi extends BaseAPI { requestParameters.offered_by, requestParameters.platform, requestParameters.professional, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.topic, @@ -18984,6 +19019,7 @@ export class LearningResourcesApi extends BaseAPI { requestParameters.offered_by, requestParameters.platform, requestParameters.professional, + requestParameters.resource_category, requestParameters.resource_type, requestParameters.resource_type_group, requestParameters.topic, diff --git a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx index 793155fd1a..d5e8f9b374 100644 --- a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx +++ b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx @@ -22,9 +22,13 @@ import { useFeatureFlagEnabled, usePostHog } from "posthog-js/react" import { PostHogEvents } from "@/common/constants" jest.mock("posthog-js/react") +jest.mock("ol-components/CarouselV2", () => ({ + CarouselV2: ({ children }: { children: React.ReactNode }) => ( +
{children}
+ ), +})) const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) const mockedPostHogCapture = jest.fn() -jest.mock("posthog-js/react") jest.mocked(usePostHog).mockReturnValue( // @ts-expect-error Not mocking all of posthog { capture: mockedPostHogCapture }, @@ -91,6 +95,13 @@ const setupAPIs = () => { attestations, ) + setMockResponse.get(expect.stringContaining(urls.search.resources()), { + count: 0, + next: null, + previous: null, + results: [], + }) + mockedUseFeatureFlagEnabled.mockReturnValue(false) return { featured, media } } @@ -417,22 +428,27 @@ describe("Home Page Carousel", () => { renderWithProviders() - await screen.findAllByRole("tablist").then(([featured, media]) => { + await screen.findAllByRole("tablist").then(([featured]) => { within(featured).getByRole("tab", { name: "All" }) within(featured).getByRole("tab", { name: "Free" }) within(featured).getByRole("tab", { name: "With Certificate" }) within(featured).getByRole("tab", { name: "Professional & Executive Learning", }) - within(media).getByRole("tab", { name: "All" }) - within(media).getByRole("tab", { name: "Videos" }) - within(media).getByRole("tab", { name: "Podcasts" }) }) }) }) test("Headings", async () => { - const { featured, media } = setupAPIs() + const { featured } = setupAPIs() + + const videoShorts = [learningResources.video(), learningResources.video()] + setMockResponse.get(expect.stringContaining(urls.search.resources()), { + count: videoShorts.length, + next: null, + previous: null, + results: videoShorts, + }) renderWithProviders() await waitFor(() => { @@ -442,8 +458,7 @@ test("Headings", async () => { // Featured course order is randomized on frontend, so just check for presence ...featured.results.map(() => ({ level: 3, name: expect.any(String) })), { level: 2, name: "Continue Your Journey" }, - { level: 2, name: "Media" }, - ...media.results.map((result) => ({ level: 3, name: result.title })), + { level: 2, name: "MIT Learning Moments" }, { level: 2, name: "Browse by Topic" }, { level: 2, name: "From Our Community" }, { level: 2, name: "MIT News & Events" }, diff --git a/frontends/main/src/app-pages/HomePage/HomePage.tsx b/frontends/main/src/app-pages/HomePage/HomePage.tsx index 148120a12e..9d68003caa 100644 --- a/frontends/main/src/app-pages/HomePage/HomePage.tsx +++ b/frontends/main/src/app-pages/HomePage/HomePage.tsx @@ -41,7 +41,7 @@ const LearningResourceDrawer = dynamic( import("@/page-components/LearningResourceDrawer/LearningResourceDrawer"), ) -const MediaSection = dynamic(() => import("./MediaSection")) +const VideoShortsSection = dynamic(() => import("./VideoShortsSection")) const HomePage: React.FC<{ heroImageIndex: number }> = ({ heroImageIndex }) => { return ( @@ -61,7 +61,7 @@ const HomePage: React.FC<{ heroImageIndex: number }> = ({ heroImageIndex }) => { - + diff --git a/frontends/main/src/app-pages/HomePage/MediaSection.tsx b/frontends/main/src/app-pages/HomePage/MediaSection.tsx deleted file mode 100644 index 46980e72ab..0000000000 --- a/frontends/main/src/app-pages/HomePage/MediaSection.tsx +++ /dev/null @@ -1,38 +0,0 @@ -import React from "react" -import { Container, styled } from "ol-components" -import VideoShortsSection from "./VideoShortsSection" -import { FeatureFlags } from "@/common/feature_flags" -import { useFeatureFlagEnabled } from "posthog-js/react" -import ResourceCarousel from "@/page-components/ResourceCarousel/ResourceCarousel" -import * as carousels from "./carousels" - -const MediaCarousel = styled(ResourceCarousel)(({ theme }) => ({ - margin: "80px 0", - minHeight: "388px", - [theme.breakpoints.down("md")]: { - margin: "40px 0", - minHeight: "418px", - }, -})) - -const MediaSection = () => { - const videoShortsEnabled = useFeatureFlagEnabled(FeatureFlags.VideoShorts) - - if (videoShortsEnabled === undefined) { - return null - } - if (videoShortsEnabled) { - return - } - return ( - - - - ) -} - -export default MediaSection diff --git a/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx b/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx index e982f30fdd..5a3ae0cbbe 100644 --- a/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx +++ b/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.test.tsx @@ -3,16 +3,8 @@ import { factories, setMockResponse, urls } from "api/test-utils" import { ResourceTypeEnum } from "api/v1" import type { LearningResource, PodcastEpisodeResource } from "api/v1" import { renderWithProviders, screen, user } from "@/test-utils" -import { useFeatureFlagEnabled } from "posthog-js/react" -import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" import { PodcastDetailPage } from "./PodcastDetailPage" -jest.mock("posthog-js/react") -jest.mock("@/common/useFeatureFlagsLoaded") - -const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) -const mockedUseFeatureFlagsLoaded = jest.mocked(useFeatureFlagsLoaded) - jest.mock( "@/page-components/LearningResourceDrawer/LearningResourceDrawer", () => ({ @@ -94,15 +86,6 @@ const setupApis = ({ } describe("PodcastDetailPage", () => { - beforeEach(() => { - mockedUseFeatureFlagEnabled.mockReturnValue(true) - mockedUseFeatureFlagsLoaded.mockReturnValue(true) - }) - - afterEach(() => { - jest.clearAllMocks() - }) - test("renders initial episode list", async () => { const episodes = makePodcastEpisodes(3) const { podcast } = setupApis({ episodesPage1: episodes }) diff --git a/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.tsx b/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.tsx index 051eab5654..bf72821077 100644 --- a/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.tsx +++ b/frontends/main/src/app-pages/PodcastPage/PodcastDetailPage.tsx @@ -1,7 +1,6 @@ "use client" import React, { useState, useEffect, useRef } from "react" -import { notFound } from "next/navigation" import Link from "next/link" import { Breadcrumbs, Typography, styled, useMediaQuery } from "ol-components" import type { Theme } from "ol-components" @@ -19,9 +18,6 @@ import moment from "moment" import { formatDate } from "ol-utilities" import { HOME, podcastEpisodePageView } from "@/common/urls" import PodcastContainer from "./PodcastContainer" -import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" -import { useFeatureFlagEnabled } from "posthog-js/react" -import { FeatureFlags } from "@/common/feature_flags" const HeaderSection = styled.div(({ theme }) => ({ borderBottom: `1px solid ${theme.custom.colors.lightGray2}`, @@ -404,10 +400,6 @@ const EPISODES_PAGE_SIZE = 5 export const PodcastDetailPage: React.FC = ({ podcastId, }) => { - const showPodcastDetailPage = useFeatureFlagEnabled( - FeatureFlags.PodcastDetailPage, - ) - const flagsLoaded = useFeatureFlagsLoaded() const id = Number(podcastId) const isMobile = useMediaQuery((theme: Theme) => theme.breakpoints.down("sm")) const [playingEpisode, setPlayingEpisode] = useState( @@ -511,10 +503,6 @@ export const PodcastDetailPage: React.FC = ({ } }, [currentTrack, isMobile]) - if (!showPodcastDetailPage) { - return flagsLoaded ? notFound() : null - } - return ( <> diff --git a/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.test.tsx b/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.test.tsx index 0922f230f9..09057295fd 100644 --- a/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.test.tsx +++ b/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.test.tsx @@ -3,16 +3,8 @@ import { factories, setMockResponse, urls } from "api/test-utils" import { ResourceTypeEnum } from "api/v1" import type { LearningResource, PodcastEpisodeResource } from "api/v1" import { renderWithProviders, screen, user } from "@/test-utils" -import { useFeatureFlagEnabled } from "posthog-js/react" -import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" import { PodcastEpisodeDetailPage } from "./PodcastEpisodeDetailPage" -jest.mock("posthog-js/react") -jest.mock("@/common/useFeatureFlagsLoaded") - -const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) -const mockedUseFeatureFlagsLoaded = jest.mocked(useFeatureFlagsLoaded) - jest.mock("./PodcastPlayer", () => ({ __esModule: true, PLAYER_HEIGHT: { desktop: 104, mobile: 220 }, @@ -90,15 +82,6 @@ const setupApis = ({ } describe("PodcastEpisodeDetailPage", () => { - beforeEach(() => { - mockedUseFeatureFlagEnabled.mockReturnValue(true) - mockedUseFeatureFlagsLoaded.mockReturnValue(true) - }) - - afterEach(() => { - jest.clearAllMocks() - }) - test("renders episode title and podcast name on the page", async () => { const { episode, podcast } = setupApis({ moreEpisodes: [] }) @@ -275,34 +258,4 @@ describe("PodcastEpisodeDetailPage", () => { moreEpisode.title!, ) }) - - test("returns null (not found) when feature flag is not loaded yet", () => { - mockedUseFeatureFlagsLoaded.mockReturnValue(false) - mockedUseFeatureFlagEnabled.mockReturnValue(false) - - const episode = makePodcastEpisode() - const podcast = makePodcast() - - setMockResponse.get( - urls.learningResources.details({ id: episode.id }), - episode, - ) - setMockResponse.get( - urls.learningResources.details({ id: podcast.id }), - podcast, - ) - setMockResponse.get( - `${urls.learningResources.items({ id: podcast.id })}?limit=${EPISODES_PAGE_SIZE}`, - makeItemsResponse([]), - ) - - const { view } = renderWithProviders( - , - ) - - expect(view.container).toBeEmptyDOMElement() - }) }) diff --git a/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.tsx b/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.tsx index e259e1e91e..a22f73e54d 100644 --- a/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.tsx +++ b/frontends/main/src/app-pages/PodcastPage/PodcastEpisodeDetailPage.tsx @@ -11,9 +11,6 @@ import { import type { Theme } from "ol-components" import { Button } from "@mitodl/smoot-design" import { RiPlayFill, RiPauseFill } from "@remixicon/react" -import { useFeatureFlagsLoaded } from "@/common/useFeatureFlagsLoaded" -import { useFeatureFlagEnabled } from "posthog-js/react" -import { FeatureFlags } from "@/common/feature_flags" import PodcastPlayer, { PLAYER_HEIGHT } from "./PodcastPlayer" import type { PodcastTrack, PodcastPlayerHandle } from "./PodcastPlayer" import { @@ -29,7 +26,6 @@ import { HOME, podcastPageView, podcastEpisodePageView } from "@/common/urls" import DOMPurify from "isomorphic-dompurify" import { EpisodeItem } from "./PodcastDetailPage" import PodcastContainer from "./PodcastContainer" -import { notFound } from "next/navigation" import Link from "next/link" /* ── Layout ── */ @@ -202,10 +198,6 @@ export const PodcastEpisodeDetailPage: React.FC< const [isAudioPlaying, setIsAudioPlaying] = useState(false) const playerRef = useRef(null) - const showPodcastDetailPage = useFeatureFlagEnabled( - FeatureFlags.PodcastDetailPage, - ) - const flagsLoaded = useFeatureFlagsLoaded() const { data: episode } = useLearningResourcesDetail(Number(episodeId)) const { data: podcast } = useLearningResourcesDetail(Number(podcastId)) @@ -293,9 +285,6 @@ export const PodcastEpisodeDetailPage: React.FC< ? podcastPageView(podcastId, podcast?.title) : "/" - if (!showPodcastDetailPage) { - return flagsLoaded ? notFound() : null - } return ( <> diff --git a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx index c8724b4330..6c99505671 100644 --- a/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx +++ b/frontends/main/src/app-pages/SearchPage/SearchPage.test.tsx @@ -85,6 +85,16 @@ const getLastApiSearchParams = () => { return fullUrl.searchParams } +const getLastVectorApiSearchParams = () => { + const call = makeRequest.mock.calls.find(([args]) => { + if (args.method !== "get") return false + return args.url.startsWith(urls.search.vectorResources()) + }) + invariant(call) + const fullUrl = new URL(call[0].url, "http://mit.edu") + return fullUrl.searchParams +} + describe("SearchPage", () => { test("Renders search results", async () => { const resources = factories.learningResources.resources({ @@ -1080,6 +1090,39 @@ describe("UniversalAIBanner", () => { }) }) + test("Vector Hybrid Search passes resource category filters without a text query", async () => { + setMockApiResponses({ + search: { + count: 1, + metadata: { + aggregations: { + resource_type_group: [{ key: "learning_material", doc_count: 1 }], + resource_category: [{ key: "Video", doc_count: 1 }], + }, + suggestions: [], + }, + }, + }) + + renderWithProviders(, { + url: "?vector_search=true&resource_type_group=learning_material&resource_category=Video&topic=Systems%20Engineering", + }) + + await waitFor(() => { + expect( + makeRequest.mock.calls.some(([args]) => + args.url.startsWith(urls.search.vectorResources()), + ), + ).toBe(true) + }) + + const apiSearchParams = getLastVectorApiSearchParams() + expect(apiSearchParams.get("hybrid_search")).toBe("true") + expect(apiSearchParams.get("resource_type_group")).toBe("learning_material") + expect(apiSearchParams.get("resource_category")).toBe("Video") + expect(apiSearchParams.get("topic")).toBe("Systems Engineering") + }) + test("Vector Hybrid Search with a query hides pagination and filters results locally", async () => { const physicsTopic = factories.learningResources.topic({ name: "Physics" }) const chemistryTopic = factories.learningResources.topic({ diff --git a/frontends/main/src/app-pages/VideoEmbedPage/VideoEmbedPage.tsx b/frontends/main/src/app-pages/VideoEmbedPage/VideoEmbedPage.tsx index 94ed9009a2..ac3b8640fc 100644 --- a/frontends/main/src/app-pages/VideoEmbedPage/VideoEmbedPage.tsx +++ b/frontends/main/src/app-pages/VideoEmbedPage/VideoEmbedPage.tsx @@ -13,9 +13,13 @@ const EmbedPlayer = styled(VideoResourcePlayer)({ type VideoEmbedPageProps = { videoResource: VideoResource + startTime?: number } -const VideoEmbedPage: React.FC = ({ videoResource }) => { +const VideoEmbedPage: React.FC = ({ + videoResource, + startTime, +}) => { const videoTitleLabel = videoResource.title.trim() return ( @@ -25,6 +29,7 @@ const VideoEmbedPage: React.FC = ({ videoResource }) => { isLoading={false} videoTitleLabel={videoTitleLabel} videoThumbnailAlt={`Video thumbnail for ${videoTitleLabel}`} + startTime={startTime} /> ) } diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.test.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.test.tsx index 4d8640e241..8a97999752 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.test.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.test.tsx @@ -13,6 +13,31 @@ const makeVideo = (overrides: Partial = {}): VideoResource => ...overrides, }) as VideoResource +const makeYouTubeVideo = ( + overrides: Partial = {}, +): VideoResource => + makeVideo({ + url: "https://www.youtube.com/watch?v=abc123XYZ", + platform: { code: "youtube", name: "YouTube" } as VideoResource["platform"], + ...overrides, + }) + +const makeOvsVideo = (overrides: Partial = {}): VideoResource => + makeVideo({ + platform: { + code: "ovs", + name: "ODL Video Service", + } as VideoResource["platform"], + video: { + id: 1, + caption_urls: [], + streaming_url: "https://cdn.odl.mit.edu/video/stream.m3u8", + duration: "300", + cover_image_url: null, + }, + ...overrides, + }) + const renderDialog = (video: VideoResource = makeVideo()) => renderWithProviders( />, ) +const renderDialogWithTime = ( + getCurrentTime: () => number, + video: VideoResource = makeYouTubeVideo(), +) => + renderWithProviders( + , + ) + describe("VideoShareDialog", () => { describe("tab switching", () => { test("defaults to the Share tab", async () => { @@ -143,4 +183,105 @@ describe("VideoShareDialog", () => { expect(textarea.value).not.toContain("youtube.com") }) }) + + describe("timestamp-aware share links", () => { + describe("Share tab", () => { + test("does not append ?t= when video is at position 0", async () => { + renderDialogWithTime(() => 0) + expect(await screen.findByDisplayValue(PAGE_URL)).toBeInTheDocument() + }) + }) + + describe("Embed tab — timestamp checkbox", () => { + test("shows 'Start at' checkbox when video is mid-play", async () => { + renderDialogWithTime(() => 90) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + expect( + screen.getByRole("checkbox", { name: /start at/i }), + ).toBeInTheDocument() + }) + + test("does not show 'Start at' checkbox when video is at position 0", async () => { + renderDialogWithTime(() => 0) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + expect( + screen.queryByRole("checkbox", { name: /start at/i }), + ).not.toBeInTheDocument() + }) + + test("formats sub-hour timestamp as m:ss", async () => { + renderDialogWithTime(() => 90) // 1 min 30 sec + await user.click(await screen.findByRole("tab", { name: /embed/i })) + expect(screen.getByText("Start at 1:30")).toBeInTheDocument() + }) + + test("formats over-hour timestamp as h:mm:ss", async () => { + renderDialogWithTime(() => 3661) // 1 h 1 min 1 sec + await user.click(await screen.findByRole("tab", { name: /embed/i })) + expect(screen.getByText("Start at 1:01:01")).toBeInTheDocument() + }) + + test("unchecking 'Start at' removes timestamp from YouTube embed HTML", async () => { + renderDialogWithTime(() => 90) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + await user.click(screen.getByRole("checkbox", { name: /start at/i })) + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).not.toContain("start=") + }) + + test("re-checking 'Start at' restores timestamp in YouTube embed HTML", async () => { + renderDialogWithTime(() => 90) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + const checkbox = screen.getByRole("checkbox", { name: /start at/i }) + await user.click(checkbox) // uncheck + await user.click(checkbox) // re-check + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).toContain("start=90") + }) + }) + + describe("Embed tab — YouTube timestamp", () => { + test("YouTube embed HTML includes start= param", async () => { + renderDialogWithTime(() => 90) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).toContain("start=90") + }) + + test("YouTube embed HTML omits start= when at position 0", async () => { + renderDialogWithTime(() => 0) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).not.toContain("start=") + }) + }) + + describe("Embed tab — OVS timestamp", () => { + test("OVS embed HTML includes t= param", async () => { + renderDialogWithTime(() => 90, makeOvsVideo()) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).toContain("t=90") + }) + + test("OVS embed HTML omits t= when at position 0", async () => { + renderDialogWithTime(() => 0, makeOvsVideo()) + await user.click(await screen.findByRole("tab", { name: /embed/i })) + const textarea = screen.getByRole("textbox", { + name: "Embed HTML", + }) as HTMLTextAreaElement + expect(textarea.value).not.toMatch(/[?&]t=/) + }) + }) + }) }) diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.tsx index df1f64048c..60587e75d8 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/ShareDialog.tsx @@ -152,6 +152,24 @@ const EmbedFooter = styled.div({ alignItems: "center", }) +const StartAtRow = styled.div(({ theme }) => ({ + display: "flex", + alignItems: "center", + gap: "8px", + ...theme.typography.body2, + color: theme.custom.colors.darkGray2, + "input[type='checkbox']": { + width: "16px", + height: "16px", + cursor: "pointer", + accentColor: theme.custom.colors.red, + }, + label: { + cursor: "pointer", + userSelect: "none", + }, +})) + // ─── Helpers ─────────────────────────────────────────────────────────────── async function copyToClipboard(text: string): Promise { @@ -180,21 +198,46 @@ function escapeHtmlAttr(value: string): string { .replace(/ 0 ? `${h}:${mm}:${ss}` : `${m}:${ss}` +} + +function withTimeParam(url: string, seconds: number, param = "t"): string { + try { + const u = new URL(url) + u.searchParams.set(param, String(Math.floor(seconds))) + return u.toString() + } catch { + return url + } +} + function buildVideoEmbedHtml( video: VideoResource, title: string, embedPageUrl: string, + startTime = 0, ): string { const isOvs = video.platform?.code === "ovs" || video.platform?.code === "ocw" const escapedTitle = escapeHtmlAttr(title) + const t = Math.floor(startTime) if (isOvs) { if (!embedPageUrl) return "" - return `` + const src = t > 0 ? withTimeParam(embedPageUrl, t) : embedPageUrl + return `` } const youtubeId = getYouTubeVideoId(video.url) if (!youtubeId) return "" - return `` + const ytBase = `https://www.youtube.com/embed/${youtubeId}` + const src = t > 0 ? withTimeParam(ytBase, t, "start") : ytBase + return `` } function buildPodcastEmbedHtml( @@ -218,6 +261,7 @@ type ShareDialogProps = { resource?: PodcastEpisodeResource pageUrl: string title: string + getCurrentTime?: () => number } const ShareDialog = ({ @@ -227,10 +271,24 @@ const ShareDialog = ({ resource, pageUrl, title, + + getCurrentTime, }: ShareDialogProps) => { const [activeTab, setActiveTab] = useState("share") const [copyLinkText, setCopyLinkText] = useState("Copy Link") const [copyEmbedText, setCopyEmbedText] = useState("Copy") + const [startTime, setStartTime] = useState(0) + const [includeTime, setIncludeTime] = useState(false) + + const wasOpenRef = useRef(false) + useEffect(() => { + if (open && !wasOpenRef.current) { + const t = getCurrentTime?.() ?? 0 + setStartTime(t) + setIncludeTime(t > 0) + } + wasOpenRef.current = open + }, [open, getCurrentTime]) const copyLinkTimerRef = useRef | null>(null) const copyEmbedTimerRef = useRef | null>(null) @@ -276,14 +334,23 @@ const ShareDialog = ({ ? `${NEXT_PUBLIC_ORIGIN}/podcast/embed/${resource.id}` : null + const t = includeTime && startTime > 0 ? Math.floor(startTime) : 0 + + const activeShareUrl = pageUrl + const activeEmbedUrl = + embedUrl && t > 0 ? withTimeParam(embedUrl, t) : embedUrl + const embedHtml = video - ? buildVideoEmbedHtml(video, title, videoEmbedPageUrl) + ? buildVideoEmbedHtml(video, title, videoEmbedPageUrl, t) : resource ? buildPodcastEmbedHtml(resource, title, embedUrl || "") : "" const embedUrlLabel = video ? "Video URL" : "Audio URL" + const startAtLabel = + video && startTime > 0 ? `Start at ${formatTimestamp(startTime)}` : null + return ( Share on Social { const input = event.currentTarget.querySelector("input") @@ -383,9 +450,9 @@ const ShareDialog = ({ variant="bordered" startIcon={} onClick={async () => { - if (!pageUrl) return + if (!activeShareUrl) return try { - await copyToClipboard(pageUrl) + await copyToClipboard(activeShareUrl) setCopyLinkText("Copied!") } catch { setCopyLinkText("Failed to copy") @@ -409,7 +476,7 @@ const ShareDialog = ({ {embedUrlLabel} { @@ -429,6 +496,17 @@ const ShareDialog = ({ /> + {startAtLabel && ( + + setIncludeTime(e.target.checked)} + /> + + + )} string currentVideo: VideoResource shareUrl: string + playerRef?: React.RefObject } const UpNextSection: React.FC = ({ @@ -22,6 +24,7 @@ const UpNextSection: React.FC = ({ getVideoHref, currentVideo, shareUrl, + playerRef, }) => { return ( @@ -34,6 +37,7 @@ const UpNextSection: React.FC = ({ video={currentVideo} title={currentVideo.title ?? ""} pageUrl={shareUrl} + playerRef={playerRef} /> { }, ) + describe("share button", () => { + test("is not shown while data is still loading", async () => { + const video = makeVideo({ title: "Loading Test" }) + renderPage({ video }) + // Synchronously after render the API hasn't resolved yet + expect( + screen.queryByRole("button", { name: /share/i }), + ).not.toBeInTheDocument() + // Confirm it does appear once loaded (proving the above was the loading state) + await screen.findByRole("button", { name: /share loading test/i }) + }) + + test("aria-label includes the video title", async () => { + const video = makeVideo({ title: "Quantum Computing" }) + renderPage({ video }) + expect( + await screen.findByRole("button", { name: /share quantum computing/i }), + ).toBeInTheDocument() + }) + + test("clicking the button opens the share dialog", async () => { + const video = makeVideo({ title: "Open Dialog Test" }) + renderPage({ video }) + await user.click( + await screen.findByRole("button", { name: /share open dialog test/i }), + ) + expect(screen.getByRole("dialog")).toBeInTheDocument() + }) + + test("closing the dialog removes it from the page", async () => { + const video = makeVideo({ title: "Close Dialog Test" }) + renderPage({ video }) + await user.click( + await screen.findByRole("button", { name: /share close dialog test/i }), + ) + await user.click(screen.getByRole("button", { name: /^close$/i })) + expect(screen.queryByRole("dialog")).not.toBeInTheDocument() + }) + }) + test("renders VideoResourcePlayer for the video", async () => { const video = makeVideo({ video: { diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPage.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPage.tsx index 262b5eb7dd..a24e7ccd3a 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPage.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPage.tsx @@ -21,6 +21,7 @@ import { formatDurationClockTime } from "ol-utilities" import { videoDetailPageView, videoPlaylistPageView } from "@/common/urls" import { buildVideoStructuredData } from "./videoStructuredData" import VideoResourcePlayer from "./VideoResourcePlayer" +import type { VideoPlayerHandle } from "./VideoResourcePlayer" const NEXT_PUBLIC_ORIGIN = env("NEXT_PUBLIC_ORIGIN") @@ -291,6 +292,7 @@ type VideoDetailPageProps = { playlistId: number | null playlistData?: VideoPlaylistResource playlistLoading: boolean + startTime?: number } const VideoDetailPage: React.FC = ({ @@ -298,8 +300,10 @@ const VideoDetailPage: React.FC = ({ playlistId, playlistData, playlistLoading, + startTime, }) => { const titleRef = useRef(null) + const playerRef = useRef(null) const { data: resource, isLoading: videoLoading } = useLearningResourcesDetail(videoId) @@ -440,11 +444,13 @@ const VideoDetailPage: React.FC = ({ )} @@ -468,6 +474,7 @@ const VideoDetailPage: React.FC = ({ video={video} title={video.title ?? "video"} pageUrl={`${NEXT_PUBLIC_ORIGIN}${videoDetailPageView(video.id, playlistId ?? undefined, video.title)}`} + playerRef={playerRef} /> )} diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPageRouter.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPageRouter.tsx index fcb13f0f78..f9ab0a1fee 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPageRouter.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoDetailPageRouter.tsx @@ -11,11 +11,13 @@ import { LoadingSpinner } from "ol-components" type VideoDetailPageRouterProps = { videoId: number playlistId: number | null + startTime?: number } const VideoDetailPageRouter: React.FC = ({ videoId, playlistId, + startTime, }) => { const { data: playlist, isLoading: playlistLoading } = useQuery({ ...videoPlaylistQueries.detail(playlistId ?? 0), @@ -31,6 +33,7 @@ const VideoDetailPageRouter: React.FC = ({ if (isOcw) { return ( = ({ playlistId={playlistId} playlistData={playlist} playlistLoading={playlistLoading} + startTime={startTime} /> ) } diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoJsPlayer.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoJsPlayer.tsx index 6b4867c687..a472b92d29 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoJsPlayer.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoJsPlayer.tsx @@ -25,6 +25,7 @@ export type VideoJsPlayerProps = { playsinline?: boolean ariaLabel?: string ariaDescribedBy?: string + startTime?: number onReady?: (player: Player) => void } @@ -44,6 +45,7 @@ const VideoJsPlayer: React.FC = ({ playsinline = false, ariaLabel, ariaDescribedBy, + startTime, onReady, }) => { const videoRef = useRef(null) @@ -116,6 +118,15 @@ const VideoJsPlayer: React.FC = ({ // Add tracks inside the ready callback — this is the earliest safe // point; adding them before ready can silently fail on some browsers. addTracks(this, tracks) + if (startTime && startTime > 0) { + // Seek only after metadata (duration/seekable range) is available. + // Calling currentTime() in ready() runs before metadata loads, so the + // seek is silently ignored for non-YouTube (HTML5) sources. `one` + // fires a single time, so it seeks on initial load only. + this.one("loadedmetadata", () => { + this.currentTime(startTime) + }) + } onReady?.(this) // Set the flag here so the update effect only runs after the player // is truly ready and the initial setup is complete. @@ -136,6 +147,7 @@ const VideoJsPlayer: React.FC = ({ playsinline, poster, sources, + startTime, tracks, ]) diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoResourcePlayer.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoResourcePlayer.tsx index beb5835688..37dfa0d54d 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoResourcePlayer.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoResourcePlayer.tsx @@ -1,15 +1,18 @@ "use client" -import React, { useMemo } from "react" +import React, { useImperativeHandle, useMemo, useRef } from "react" import dynamic from "next/dynamic" import Image from "next/image" import { Skeleton, styled } from "ol-components" import { NoVideoMessage } from "./shared.styled" import { resolveVideoSources } from "./videoSources" import { convertToEmbedUrl } from "@/common/utils" -import YouTubeIframePlayer from "./YouTubeIframePlayer" +import YouTubeIframePlayer, { + type YouTubePlayerHandle, +} from "./YouTubeIframePlayer" import type { VideoResource } from "api/v1" import type { VideoJsPlayerProps } from "./VideoJsPlayer" +import type Player from "video.js/dist/types/player" const VideoJsPlayer = dynamic( () => import("./VideoJsPlayer"), @@ -79,6 +82,10 @@ const ScreenReaderOnly = styled.span({ border: 0, }) +export type VideoPlayerHandle = { + getCurrentTime: () => number +} + export type VideoResourcePlayerProps = { video: VideoResource | undefined videoId: number @@ -86,6 +93,7 @@ export type VideoResourcePlayerProps = { videoTitleLabel: string videoThumbnailAlt: string ariaDescribedBy?: string + startTime?: number className?: string } @@ -95,98 +103,136 @@ export type VideoResourcePlayerProps = { * * Accepts `className` so it can be extended with `styled(VideoResourcePlayer)` * for per-page layout overrides (e.g. margin, border). + * + * Exposes a `VideoPlayerHandle` ref so callers can read `getCurrentTime()`. + * OVS (VideoJS) returns the actual playback position; YouTube returns 0 + * until the YouTube IFrame API is wired up. */ -const VideoResourcePlayer: React.FC = ({ - video, - videoId, - isLoading, - videoTitleLabel, - videoThumbnailAlt, - ariaDescribedBy = "video-description", - className, -}) => { - const sources = useMemo( - () => - video - ? resolveVideoSources( - video.video?.streaming_url, - video.url, - video.content_files?.[0]?.youtube_id, - ) - : [], - [video], - ) - - const captionUrls = video?.video?.caption_urls ?? [] - - const embedUrl = - sources[0]?.type === "video/youtube" - ? convertToEmbedUrl(sources[0].src) - : null - - const thumbnailUrl = - video?.image?.url ?? video?.content_files?.[0]?.image_src ?? null - - const posterUrl = - video?.video?.cover_image_url ?? - video?.content_files?.[0]?.image_src ?? - video?.image?.url ?? - undefined - - return ( - - {isLoading ? ( -
- -
- ) : embedUrl ? ( - - ) : sources.length > 0 ? ( - - ) : thumbnailUrl ? ( - - {videoThumbnailAlt}( + ( + { + video, + videoId, + isLoading, + videoTitleLabel, + videoThumbnailAlt, + ariaDescribedBy = "video-description", + startTime, + className, + }, + ref, + ) => { + const vjsPlayerRef = useRef(null) + const ytPlayerRef = useRef(null) + + const sources = useMemo( + () => + video + ? resolveVideoSources( + video.video?.streaming_url, + video.url, + video.content_files?.[0]?.youtube_id, + ) + : [], + [video], + ) + + const captionUrls = video?.video?.caption_urls ?? [] + + const embedUrl = + sources[0]?.type === "video/youtube" + ? convertToEmbedUrl(sources[0].src) + : null + + useImperativeHandle( + ref, + () => ({ + getCurrentTime: () => + embedUrl + ? (ytPlayerRef.current?.getCurrentTime() ?? 0) + : (vjsPlayerRef.current?.currentTime() ?? 0), + }), + [embedUrl], + ) + + const thumbnailUrl = + video?.image?.url ?? video?.content_files?.[0]?.image_src ?? null + + const posterUrl = + video?.video?.cover_image_url ?? + video?.content_files?.[0]?.image_src ?? + video?.image?.url ?? + undefined + + return ( + + {isLoading ? ( +
+ +
+ ) : embedUrl ? ( + - - ) : ( - <> - - No playable source available for this video. - - - No playable source available for this video. - - - )} - - ) -} + ) : sources.length > 0 ? ( + { + vjsPlayerRef.current = player + }} + /> + ) : thumbnailUrl ? ( + + {videoThumbnailAlt} + + ) : ( + <> + + No playable source available for this video. + + + No playable source available for this video. + + + )} + + ) + }, +) +VideoResourcePlayer.displayName = "VideoResourcePlayer" export default VideoResourcePlayer diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.test.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.test.tsx index e97f6cf20d..9f466eac50 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.test.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.test.tsx @@ -1,6 +1,6 @@ import React from "react" import { setMockResponse, urls, factories } from "api/test-utils" -import { renderWithProviders, screen } from "@/test-utils" +import { renderWithProviders, screen, user } from "@/test-utils" import VideoSeriesDetailPage from "./VideoSeriesDetailPage" import { ResourceTypeEnum } from "api/v1" import type { VideoResource, VideoPlaylistResource } from "api/v1" @@ -265,6 +265,90 @@ describe("VideoSeriesDetailPage", () => { }) }) + describe("share button in Up Next section", () => { + test("is not shown while playlist items are still loading", async () => { + const playlist = makePlaylist() + const video = makeVideo({ title: "Loading Items Test" }) + + setupVideoApi(video) + setMockResponse.get( + urls.learningResources.items({ id: playlist.id }), + new Promise(() => {}), // never resolves → items stay loading + ) + + renderWithProviders( + , + ) + + await screen.findByRole("heading", { name: video.title }) + expect( + screen.queryByRole("button", { name: /share/i }), + ).not.toBeInTheDocument() + }) + + test("share URL uses the slugged canonical form with playlist param", async () => { + const playlist = makePlaylist({ id: 99 }) + const current = makeVideo({ id: 720, title: "Intro to Machine Learning" }) + const next = makeVideo({ title: "Next Lecture" }) + renderPage({ + video: current, + playlistId: playlist.id, + playlistData: playlist, + playlistItems: [current, next], + }) + + await screen.findByText("Up Next") + await user.click( + screen.getByRole("button", { + name: /share intro to machine learning/i, + }), + ) + expect(screen.getByRole("textbox")).toHaveValue( + "http://test.learn.odl.local:8062/video/720/intro-to-machine-learning?playlist=99", + ) + }) + + test("clicking the share button opens the dialog", async () => { + const playlist = makePlaylist() + const current = makeVideo({ title: "Current Video" }) + const next = makeVideo({ title: "Next Video" }) + renderPage({ + video: current, + playlistId: playlist.id, + playlistData: playlist, + playlistItems: [current, next], + }) + + await user.click( + await screen.findByRole("button", { name: /share current video/i }), + ) + expect(screen.getByRole("dialog")).toBeInTheDocument() + }) + + test("closing the share dialog removes it from the page", async () => { + const playlist = makePlaylist() + const current = makeVideo({ title: "Close Dialog Test" }) + const next = makeVideo({ title: "Next Video" }) + renderPage({ + video: current, + playlistId: playlist.id, + playlistData: playlist, + playlistItems: [current, next], + }) + + await user.click( + await screen.findByRole("button", { name: /share close dialog test/i }), + ) + await user.click(screen.getByRole("button", { name: /^close$/i })) + expect(screen.queryByRole("dialog")).not.toBeInTheDocument() + }) + }) + describe("Up Next section", () => { test("renders Up Next with a Continue button when there is a next video", async () => { const playlist = makePlaylist() diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.tsx index b748acb9df..5b642bb29c 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoSeriesDetailPage.tsx @@ -14,6 +14,7 @@ import * as Styled from "./VideoSeriesDetailPage.styled" import { env } from "@/env" import { buildVideoStructuredData } from "./videoStructuredData" import VideoResourcePlayer from "./VideoResourcePlayer" +import type { VideoPlayerHandle } from "./VideoResourcePlayer" const NEXT_PUBLIC_ORIGIN = env("NEXT_PUBLIC_ORIGIN") @@ -35,6 +36,7 @@ const VideoSeriesDetailPage: React.FC = ({ playlistLoading, }) => { const titleRef = useRef(null) + const playerRef = useRef(null) const { data: resource, isLoading: videoLoading } = useLearningResourcesDetail(videoId) @@ -171,6 +173,7 @@ const VideoSeriesDetailPage: React.FC = ({ )} {/* Video player */} = ({ nextVideo={nextVideo} getVideoHref={getVideoHref} currentVideo={video} + playerRef={playerRef} shareUrl={`${NEXT_PUBLIC_ORIGIN}${videoDetailPageView(video.id, playlistId ?? undefined, video.title)}`} /> )} diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.test.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.test.tsx new file mode 100644 index 0000000000..04b28d0de5 --- /dev/null +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.test.tsx @@ -0,0 +1,112 @@ +import React from "react" +import { renderWithProviders, screen, user } from "@/test-utils" +import { factories } from "api/test-utils" +import { ResourceTypeEnum } from "api/v1" +import type { VideoResource } from "api/v1" +import VideoShareButton from "./VideoShareButton" +import type { VideoPlayerHandle } from "./VideoResourcePlayer" + +const makeVideo = (overrides: Partial = {}): VideoResource => + factories.learningResources.video({ + resource_type: ResourceTypeEnum.Video, + ...overrides, + }) as VideoResource + +const PAGE_URL = "https://learn.mit.edu/video/42?playlist=1" + +type RenderOptions = { + video?: VideoResource + title?: string + pageUrl?: string + playerRef?: React.RefObject +} + +const renderButton = ({ + video = makeVideo(), + title = video.title ?? "Test video", + pageUrl = PAGE_URL, + playerRef, +}: RenderOptions = {}) => + renderWithProviders( + , + ) + +describe("VideoShareButton", () => { + describe("share button", () => { + test("renders with an aria-label containing the video title", async () => { + renderButton({ title: "My Cool Video" }) + expect( + await screen.findByRole("button", { name: /share my cool video/i }), + ).toBeInTheDocument() + }) + + test("dialog is not visible before the button is clicked", async () => { + renderButton() + await screen.findByRole("button", { name: /share/i }) + expect(screen.queryByRole("dialog")).not.toBeInTheDocument() + }) + + test("clicking the button opens the share dialog", async () => { + renderButton({ title: "Opening Test" }) + await user.click( + await screen.findByRole("button", { name: /share opening test/i }), + ) + expect(screen.getByRole("dialog")).toBeInTheDocument() + }) + + test("closing the dialog hides it", async () => { + renderButton({ title: "Close Test" }) + await user.click( + await screen.findByRole("button", { name: /share close test/i }), + ) + await user.click(screen.getByRole("button", { name: /^close$/i })) + expect(screen.queryByRole("dialog")).not.toBeInTheDocument() + }) + }) + + describe("getCurrentTime integration", () => { + test("passes 0 to ShareDialog when no playerRef is provided", async () => { + renderButton({ title: "No Ref" }) + await user.click( + await screen.findByRole("button", { name: /share no ref/i }), + ) + // With no getCurrentTime value > 0 the share URL is the bare page URL + expect(screen.getByDisplayValue(PAGE_URL)).toBeInTheDocument() + }) + + test("passes 0 to ShareDialog when playerRef.current is null", async () => { + const playerRef = React.createRef() + renderButton({ title: "Null Ref", playerRef }) + await user.click( + await screen.findByRole("button", { name: /share null ref/i }), + ) + expect(screen.getByDisplayValue(PAGE_URL)).toBeInTheDocument() + }) + + test("reads current time from playerRef and passes it to ShareDialog", async () => { + const handle: VideoPlayerHandle = { getCurrentTime: () => 90 } + const playerRef = { + current: handle, + } as React.RefObject + + renderButton({ title: "With Time", playerRef }) + await user.click( + await screen.findByRole("button", { name: /share with time/i }), + ) + // The share-tab link is always the bare page URL; the timestamp read + // from playerRef is surfaced in the Embed tab. + await user.click(await screen.findByRole("tab", { name: /embed/i })) + // 90s read from playerRef.getCurrentTime() → "Start at 1:30" + ?t=90 + expect(screen.getByText("Start at 1:30")).toBeInTheDocument() + const videoUrlInput = screen.getByRole("textbox", { + name: "Video URL", + }) as HTMLInputElement + expect(videoUrlInput.value).toContain("t=90") + }) + }) +}) diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.tsx index 3dcfab283c..f70dad6447 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/VideoShareButton.tsx @@ -4,12 +4,14 @@ import React, { useState } from "react" import { RiShareForwardFill } from "@remixicon/react" import type { VideoResource } from "api/v1" import ShareDialog from "./ShareDialog" +import type { VideoPlayerHandle } from "./VideoResourcePlayer" import * as Styled from "./VideoSeriesDetailPage.styled" type VideoShareButtonProps = { video: VideoResource title: string pageUrl: string + playerRef?: React.RefObject className?: string } @@ -17,6 +19,7 @@ const VideoShareButton: React.FC = ({ video, title, pageUrl, + playerRef, className, }) => { const [shareOpen, setShareOpen] = useState(false) @@ -37,6 +40,7 @@ const VideoShareButton: React.FC = ({ title={title} onClose={() => setShareOpen(false)} pageUrl={pageUrl} + getCurrentTime={() => playerRef?.current?.getCurrentTime() ?? 0} /> ) diff --git a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/YouTubeIframePlayer.tsx b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/YouTubeIframePlayer.tsx index f8e77ea146..2ab88b36f6 100644 --- a/frontends/main/src/app-pages/VideoPlaylistCollectionPage/YouTubeIframePlayer.tsx +++ b/frontends/main/src/app-pages/VideoPlaylistCollectionPage/YouTubeIframePlayer.tsx @@ -1,39 +1,151 @@ "use client" import { requiredEnv } from "@/env" -import React from "react" +import React, { useEffect, useImperativeHandle, useRef } from "react" + +export type YouTubePlayerHandle = { + getCurrentTime: () => number +} export type YouTubeIframePlayerProps = { embedUrl: string title?: string ariaLabel?: string ariaDescribedBy?: string + startTime?: number +} + +interface YTPlayerInstance { + getCurrentTime(): number + destroy(): void +} + +declare global { + interface Window { + YT?: { + Player: new ( + elementOrId: HTMLElement | string, + options?: { + events?: { + onReady?: () => void + } + }, + ) => YTPlayerInstance + } + onYouTubeIframeAPIReady?: () => void + _ytApiCallbacks?: Array<() => void> + } } /** - * Renders a plain YouTube embed iframe. - * - * Used instead of the VideoJS wrapper for YouTube videos so that: - * - YouTube's native keyboard/accessibility controls work without conflict - * - Thumbnail scrubbing and other native YouTube features are preserved - * - * Accepts a base `embedUrl` (e.g. from `convertToEmbedUrl`) and appends - * the required `rel=0` and `origin` query params automatically. + * Loads the YouTube IFrame API script once and calls `onReady` when it is + * initialised (or immediately if the API is already available). */ -const YouTubeIframePlayer: React.FC = ({ - embedUrl, - title, - ariaLabel, - ariaDescribedBy, -}) => { +function ensureYTApi(onReady: () => void): void { + if (window.YT?.Player) { + onReady() + return + } + + // Queue the callback regardless of whether the script is already loading. + if (!window._ytApiCallbacks) window._ytApiCallbacks = [] + window._ytApiCallbacks.push(onReady) + + // Chain into any previously registered onYouTubeIframeAPIReady callback so + // other components' players still initialise correctly. + if (!document.querySelector('script[src*="youtube.com/iframe_api"]')) { + const prev = window.onYouTubeIframeAPIReady + window.onYouTubeIframeAPIReady = () => { + prev?.() + window._ytApiCallbacks?.forEach((fn) => fn()) + window._ytApiCallbacks = [] + } + const s = document.createElement("script") + s.src = "https://www.youtube.com/iframe_api" + document.head.appendChild(s) + } +} + +const YouTubeIframePlayer = React.forwardRef< + YouTubePlayerHandle, + YouTubeIframePlayerProps +>(({ embedUrl, title, ariaLabel, ariaDescribedBy, startTime }, ref) => { + const iframeRef = useRef(null) + const playerRef = useRef(null) + + useImperativeHandle(ref, () => ({ + // Before onReady the postMessage bridge isn't established so + // getCurrentTime() can return undefined instead of a number. + // Guard with a typeof+isFinite check so we always return a valid number. + getCurrentTime: () => { + if (typeof playerRef.current?.getCurrentTime !== "function") return 0 + try { + const t = playerRef.current.getCurrentTime() + return typeof t === "number" && isFinite(t) && t >= 0 ? t : 0 + } catch { + return 0 + } + }, + })) + + useEffect(() => { + let destroyed = false + const iframe = iframeRef.current + if (!iframe) return + + const createPlayer = () => { + // Guard against double-creation (load event + fallback timer both firing). + if (destroyed || playerRef.current) return + ensureYTApi(() => { + if (destroyed || playerRef.current || !iframeRef.current) return + if (!window.YT?.Player) return + // Pass the element reference directly rather than an id string so + // YT.Player tracks this specific element; an id string would cause + // document.getElementById lookup at cleanup time and could match a + // newly-mounted iframe sharing the same position-based id. + playerRef.current = new window.YT.Player(iframeRef.current, {}) + }) + } + + // Wrap the iframe with YT.Player only AFTER its embedded player has loaded. + // Wrapping a not-yet-loaded iframe leaves the postMessage bridge — and so + // getCurrentTime() — permanently disconnected. On the first page load the + // YT API script fetch naturally delays us past the iframe load, but on + // client-side navigation window.YT already exists, so without this guard we + // would wrap the brand-new iframe synchronously, before its content loads. + iframe.addEventListener("load", createPlayer) + // Fallback for an iframe that already finished loading before this effect + // attached the listener (e.g. a server-rendered iframe after hydration), + // where the load event will not fire again. No-op once createPlayer's guard + // wins via the load event on the navigation path. + const fallbackTimer = setTimeout(createPlayer, 1500) + + return () => { + destroyed = true + clearTimeout(fallbackTimer) + iframe.removeEventListener("load", createPlayer) + // Do NOT call playerRef.current?.destroy() here. + // YT.Player.destroy() removes the iframe element from the DOM. In React 18 + // Strict Mode (and during key-based remounts), this fires before the next + // effect runs, leaving iframeRef.current pointing to a detached element so + // the new YT.Player call silently fails. React owns the iframe's DOM + // lifecycle — when it removes the element, the browser terminates the + // embedded player automatically. + playerRef.current = null + } + }, []) + const url = new URL(embedUrl) url.searchParams.set("rel", "0") url.searchParams.set("origin", requiredEnv("NEXT_PUBLIC_ORIGIN")) - + url.searchParams.set("enablejsapi", "1") + if (startTime && startTime > 0) { + url.searchParams.set("start", String(Math.floor(startTime))) + } const src = url.toString() - return (