From 9d9d096ff2ee7801ee4e102a9c72a508435d980a Mon Sep 17 00:00:00 2001 From: Shankar Ambady Date: Fri, 3 Oct 2025 10:43:15 -0400 Subject: [PATCH 01/16] Fix qdrant payload schema type and add mechanism to keep types in sync (#2553) * adding test for field type as well as name * fixing type for certification field * adding tests --- vector_search/constants.py | 2 +- vector_search/utils.py | 37 ++++++++++----------- vector_search/utils_test.py | 64 +++++++++++++++++++++++++++++++++++++ 3 files changed, 84 insertions(+), 19 deletions(-) diff --git a/vector_search/constants.py b/vector_search/constants.py index 2b322f049c..97445d892d 100644 --- a/vector_search/constants.py +++ b/vector_search/constants.py @@ -46,7 +46,7 @@ QDRANT_LEARNING_RESOURCE_INDEXES = { "readable_id": models.PayloadSchemaType.KEYWORD, "resource_type": models.PayloadSchemaType.KEYWORD, - "certification": models.PayloadSchemaType.KEYWORD, + "certification": models.PayloadSchemaType.BOOL, "certification_type.code": models.PayloadSchemaType.KEYWORD, "professional": models.PayloadSchemaType.BOOL, "published": models.PayloadSchemaType.BOOL, diff --git a/vector_search/utils.py b/vector_search/utils.py index 4a5e2aa37f..67e485943b 100644 --- a/vector_search/utils.py +++ b/vector_search/utils.py @@ -155,24 +155,25 @@ def update_qdrant_indexes(): Create or update Qdrant indexes based on mapping in constants """ client = qdrant_client() - for index_field in QDRANT_LEARNING_RESOURCE_INDEXES: - collection_name = RESOURCES_COLLECTION_NAME - collection = client.get_collection(collection_name=collection_name) - if index_field not in collection.payload_schema: - client.create_payload_index( - collection_name=collection_name, - field_name=index_field, - field_schema=QDRANT_LEARNING_RESOURCE_INDEXES[index_field], - ) - for index_field in QDRANT_CONTENT_FILE_INDEXES: - collection_name = CONTENT_FILES_COLLECTION_NAME - collection = client.get_collection(collection_name=collection_name) - if index_field not in collection.payload_schema: - client.create_payload_index( - collection_name=collection_name, - field_name=index_field, - field_schema=QDRANT_CONTENT_FILE_INDEXES[index_field], - ) + + for index in [ + (QDRANT_LEARNING_RESOURCE_INDEXES, RESOURCES_COLLECTION_NAME), + (QDRANT_CONTENT_FILE_INDEXES, CONTENT_FILES_COLLECTION_NAME), + ]: + indexes = index[0] + collection_name = index[1] + for index_field in indexes: + collection = client.get_collection(collection_name=collection_name) + if ( + index_field not in collection.payload_schema + or indexes[index_field] + != collection.payload_schema[index_field].dict()["data_type"] + ): + client.create_payload_index( + collection_name=collection_name, + field_name=index_field, + field_schema=indexes[index_field], + ) def vector_point_id(readable_id): diff --git a/vector_search/utils_test.py b/vector_search/utils_test.py index 35f3552feb..346ccda029 100644 --- a/vector_search/utils_test.py +++ b/vector_search/utils_test.py @@ -23,7 +23,9 @@ from main.utils import checksum_for_content from vector_search.constants import ( CONTENT_FILES_COLLECTION_NAME, + QDRANT_CONTENT_FILE_INDEXES, QDRANT_CONTENT_FILE_PARAM_MAP, + QDRANT_LEARNING_RESOURCE_INDEXES, QDRANT_RESOURCE_PARAM_MAP, RESOURCES_COLLECTION_NAME, ) @@ -39,6 +41,7 @@ should_generate_resource_embeddings, update_content_file_payload, update_learning_resource_payload, + update_qdrant_indexes, vector_point_id, vector_search, ) @@ -851,3 +854,64 @@ def test_embed_learning_resources_contentfile_summarization_filter(mocker): # Assert that the summarizer was called with the correct content file IDs assert sorted(mock_content_summarizer.mock_calls[0].args[0]) == sorted(cf_ids) + + +@pytest.mark.django_db +def test_update_qdrant_indexes_adds_missing_index(mocker): + """ + Test that update_qdrant_indexes adds an index if it doesn't already exist + """ + mock_client = mocker.patch("vector_search.utils.qdrant_client").return_value + mock_client.get_collection.return_value.payload_schema = {} + + update_qdrant_indexes() + + # Ensure create_payload_index is called for missing indexes + expected_calls = [ + mocker.call( + collection_name=RESOURCES_COLLECTION_NAME, + field_name=index_field, + field_schema=QDRANT_LEARNING_RESOURCE_INDEXES[index_field], + ) + for index_field in QDRANT_LEARNING_RESOURCE_INDEXES + ] + [ + mocker.call( + collection_name=CONTENT_FILES_COLLECTION_NAME, + field_name=index_field, + field_schema=QDRANT_CONTENT_FILE_INDEXES[index_field], + ) + for index_field in QDRANT_CONTENT_FILE_INDEXES + ] + mock_client.create_payload_index.assert_has_calls(expected_calls, any_order=True) + + +@pytest.mark.django_db +def test_update_qdrant_indexes_updates_mismatched_field_type(mocker): + """ + Test that update_qdrant_indexes updates the index if the field types mismatch + """ + mock_client = mocker.patch("vector_search.utils.qdrant_client").return_value + mock_client.get_collection.return_value.payload_schema = { + index_field: mocker.MagicMock(data_type="wrong_type") + for index_field in QDRANT_LEARNING_RESOURCE_INDEXES + } + + update_qdrant_indexes() + + # Ensure create_payload_index is called for mismatched field types + expected_calls = [ + mocker.call( + collection_name=RESOURCES_COLLECTION_NAME, + field_name=index_field, + field_schema=QDRANT_LEARNING_RESOURCE_INDEXES[index_field], + ) + for index_field in QDRANT_LEARNING_RESOURCE_INDEXES + ] + [ + mocker.call( + collection_name=CONTENT_FILES_COLLECTION_NAME, + field_name=index_field, + field_schema=QDRANT_CONTENT_FILE_INDEXES[index_field], + ) + for index_field in QDRANT_CONTENT_FILE_INDEXES + ] + mock_client.create_payload_index.assert_has_calls(expected_calls, any_order=True) From f824eec9cb11e2968990bcda10c8b08a20f7ca99 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 3 Oct 2025 11:36:42 -0400 Subject: [PATCH 02/16] Program Page basic layout (#2556) * extremely minimal program page * reorganize productpage dir * set default images * restore who-can-take copy * cover whole page * slightly larger gap * rename courseDetail and programsDetail queries/keys --- .../api/src/mitxonline/hooks/pages/queries.ts | 20 +- .../mitxonline/test-utils/factories/pages.ts | 187 +++++- .../api/src/mitxonline/test-utils/urls.ts | 6 +- .../ProductPages/AboutSection.test.tsx | 96 +++ .../app-pages/ProductPages/AboutSection.tsx | 64 ++ .../ProductPages/CoursePage.test.tsx | 144 +--- .../src/app-pages/ProductPages/CoursePage.tsx | 634 +++--------------- .../ProductPages/InstructorsSection.test.tsx | 68 ++ .../ProductPages/InstructorsSection.tsx | 183 +++++ .../ProductPages/ProductPageTemplate.tsx | 265 ++++++++ ...mmary.test.tsx => ProductSummary.test.tsx} | 8 +- .../{CourseSummary.tsx => ProductSummary.tsx} | 15 +- .../ProductPages/ProgramPage.test.tsx | 162 +++++ .../app-pages/ProductPages/ProgramPage.tsx | 148 ++++ .../src/app-pages/ProductPages/RawHTML.tsx | 57 ++ .../main/src/app-pages/ProductPages/util.ts | 9 + .../(products)/courses/[readable_id]/page.tsx | 8 +- .../programs/[readable_id]/page.tsx | 66 ++ .../ol-utilities/src/hooks/useThrottle.tsx | 2 + .../src/hooks/useWindowDimensions.tsx | 2 + frontends/ol-utilities/src/index.ts | 2 - frontends/ol-utilities/src/ssr/NoSSR.tsx | 2 + 22 files changed, 1464 insertions(+), 684 deletions(-) create mode 100644 frontends/main/src/app-pages/ProductPages/AboutSection.test.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/AboutSection.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/InstructorsSection.test.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/InstructorsSection.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx rename frontends/main/src/app-pages/ProductPages/{CourseSummary.test.tsx => ProductSummary.test.tsx} (98%) rename frontends/main/src/app-pages/ProductPages/{CourseSummary.tsx => ProductSummary.tsx} (96%) create mode 100644 frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/ProgramPage.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/RawHTML.tsx create mode 100644 frontends/main/src/app-pages/ProductPages/util.ts create mode 100644 frontends/main/src/app/(products)/programs/[readable_id]/page.tsx diff --git a/frontends/api/src/mitxonline/hooks/pages/queries.ts b/frontends/api/src/mitxonline/hooks/pages/queries.ts index 697397bdc7..9898dfcabd 100644 --- a/frontends/api/src/mitxonline/hooks/pages/queries.ts +++ b/frontends/api/src/mitxonline/hooks/pages/queries.ts @@ -3,23 +3,37 @@ import { pagesApi } from "../../clients" const pagesKeys = { root: ["mitxonline", "pages"], - coursePageDetail: (readableId: string) => [ + coursePages: (readableId: string) => [ ...pagesKeys.root, "course_detail", readableId, ], + programPages: (readableId: string) => [ + ...pagesKeys.root, + "program_detail", + readableId, + ], } const pagesQueries = { - courseDetail: (readableId: string) => + coursePages: (readableId: string) => queryOptions({ - queryKey: pagesKeys.coursePageDetail(readableId), + queryKey: pagesKeys.coursePages(readableId), queryFn: async () => { return pagesApi .pagesfieldstypecmsCoursePageRetrieve({ readable_id: readableId }) .then((res) => res.data) }, }), + programPages: (readableId: string) => + queryOptions({ + queryKey: pagesKeys.programPages(readableId), + queryFn: async () => { + return pagesApi + .pagesfieldstypecmsProgramPageRetrieve({ readable_id: readableId }) + .then((res) => res.data) + }, + }), } export { pagesQueries, pagesKeys } diff --git a/frontends/api/src/mitxonline/test-utils/factories/pages.ts b/frontends/api/src/mitxonline/test-utils/factories/pages.ts index 398d6e07b3..ff2322a1e2 100644 --- a/frontends/api/src/mitxonline/test-utils/factories/pages.ts +++ b/frontends/api/src/mitxonline/test-utils/factories/pages.ts @@ -8,6 +8,7 @@ import type { Faculty, CoursePageList, V2Course, + ProgramPageItem, } from "@mitodl/mitxonline-api-axios/v2" import { UniqueEnforcer } from "enforce-unique" @@ -172,4 +173,188 @@ const coursePageList: PartialFactory = (override) => { return mergeOverrides(defaults, override) } -export { coursePageItem, coursePageList } +const programPageItem: PartialFactory = (override) => { + const defaults: ProgramPageItem = { + id: uniquePageId.enforce(() => faker.number.int()), + meta: { + alias_of: null, + detail_url: faker.internet.url(), + first_published_at: faker.date.past().toISOString(), + html_url: faker.internet.url(), + last_published_at: faker.date.recent().toISOString(), + live: true, + locale: "en-us", + search_description: faker.lorem.sentence(), + seo_title: faker.lorem.sentence(), + show_in_menus: false, + slug: faker.lorem.slug(), + type: "cms.ProgramPage", + }, + title: faker.lorem.words(3), + description: makeHTMLParagraph(1), + length: `${faker.number.int({ min: 1, max: 4 })} terms (${faker.number.int({ min: 6, max: 24 })} months)`, + effort: `${faker.number.int({ min: 5, max: 20 })} hours per week`, + min_weekly_hours: `${faker.number.int({ min: 5, max: 10 })}`, + max_weekly_hours: `${faker.number.int({ min: 11, max: 20 })}`, + min_weeks: faker.number.int({ min: 24, max: 48 }), + max_weeks: faker.number.int({ min: 48, max: 96 }), + price: [ + priceItem({ + value: { + text: `$${faker.number.int({ min: 250, max: 1000 })} - $${faker.number.int({ min: 1000, max: 5000 })} per course`, + link: "", + }, + }), + ], + min_price: faker.number.int({ min: 1000, max: 2500 }), + max_price: faker.number.int({ min: 2500, max: 10000 }), + prerequisites: makeHTMLParagraph(1), + faq_url: faker.internet.url(), + about: makeHTMLParagraph(3), + what_you_learn: makeHTMLList(5), + feature_image: featureImage(), + video_url: faker.datatype.boolean() ? faker.internet.url() : null, + faculty_section_title: "Meet your instructors", + faculty: Array.from({ length: faker.number.int({ min: 2, max: 6 }) }, () => + faculty(), + ), + certificate_page: { + id: faker.number.int({ min: 100, max: 200 }), + meta: { + alias_of: null, + detail_url: faker.internet.url(), + first_published_at: faker.date.past().toISOString(), + html_url: faker.internet.url(), + last_published_at: faker.date.recent().toISOString(), + live: true, + locale: "en-us", + search_description: faker.lorem.sentence(), + seo_title: faker.lorem.sentence(), + show_in_menus: false, + slug: faker.lorem.slug(), + type: "cms.CertificatePage", + }, + title: `Certificate For ${faker.lorem.words(3)}`, + product_name: faker.lorem.words(3), + CEUs: "", + overrides: [], + signatory_items: Array.from( + { length: faker.number.int({ min: 3, max: 5 }) }, + () => ({ + name: faker.person.fullName(), + title_1: faker.person.jobTitle(), + title_2: faker.person.jobTitle(), + organization: "Massachusetts Institute of Technology", + signature_image: faker.image.urlLoremFlickr({ + width: 200, + height: 100, + }), + }), + ), + }, + program_details: { + title: faker.lorem.words(3), + readable_id: `program-v1:${faker.lorem.slug()}`, + id: faker.number.int({ min: 1, max: 100 }), + courses: Array.from( + { length: faker.number.int({ min: 3, max: 8 }) }, + () => faker.number.int({ min: 1, max: 50 }), + ), + collections: [], + requirements: { + courses: { + required: Array.from( + { length: faker.number.int({ min: 2, max: 4 }) }, + () => faker.number.int({ min: 1, max: 20 }), + ), + electives: Array.from( + { length: faker.number.int({ min: 2, max: 5 }) }, + () => faker.number.int({ min: 21, max: 50 }), + ), + }, + programs: { + required: [], + electives: [], + }, + }, + req_tree: [ + { + data: { + node_type: "operator", + operator: "all_of", + operator_value: null, + program: faker.lorem.slug(), + course: null, + title: "Required Courses", + elective_flag: false, + }, + id: 1, + }, + { + data: { + node_type: "operator", + operator: "min_number_of", + operator_value: "2", + program: faker.lorem.slug(), + course: null, + title: "Elective Courses", + elective_flag: true, + }, + id: 2, + }, + ], + page: { + feature_image_src: faker.image.urlLoremFlickr({ + width: 1134, + height: 675, + }), + page_url: `/programs/${faker.lorem.slug()}/`, + financial_assistance_form_url: faker.internet.url(), + description: makeHTMLParagraph(1), + live: true, + length: `${faker.number.int({ min: 1, max: 4 })} terms (${faker.number.int({ min: 6, max: 24 })} months)`, + effort: `${faker.number.int({ min: 5, max: 20 })} hours per week`, + price: `$${faker.number.int({ min: 250, max: 1000 })} - $${faker.number.int({ min: 1000, max: 5000 })} per course`, + }, + program_type: faker.helpers.arrayElement([ + "MicroMasters®", + "Professional Certificate", + "XSeries", + ]), + certificate_type: faker.helpers.arrayElement([ + "MicroMasters Credential", + "Professional Certificate", + "XSeries Certificate", + ]), + departments: [ + { + name: faker.company.name(), + }, + ], + live: true, + topics: Array.from( + { length: faker.number.int({ min: 3, max: 7 }) }, + () => ({ + name: faker.lorem.word(), + }), + ), + availability: faker.helpers.arrayElement(["anytime", "dated"]), + start_date: faker.date.future().toISOString(), + end_date: null, + enrollment_start: null, + enrollment_end: null, + required_prerequisites: faker.datatype.boolean(), + duration: `${faker.number.int({ min: 1, max: 4 })} terms (${faker.number.int({ min: 6, max: 24 })} months)`, + min_weeks: faker.number.int({ min: 24, max: 48 }), + max_weeks: faker.number.int({ min: 48, max: 96 }), + min_price: faker.number.int({ min: 1000, max: 2500 }), + max_price: faker.number.int({ min: 2500, max: 10000 }), + time_commitment: `${faker.number.int({ min: 5, max: 20 })} hours per week`, + min_weekly_hours: `${faker.number.int({ min: 5, max: 10 })}`, + max_weekly_hours: `${faker.number.int({ min: 11, max: 20 })}`, + }, + } + return mergeOverrides(defaults, override) +} + +export { coursePageItem, coursePageList, faculty, programPageItem } diff --git a/frontends/api/src/mitxonline/test-utils/urls.ts b/frontends/api/src/mitxonline/test-utils/urls.ts index 08960ab423..9a54a99ab7 100644 --- a/frontends/api/src/mitxonline/test-utils/urls.ts +++ b/frontends/api/src/mitxonline/test-utils/urls.ts @@ -48,10 +48,14 @@ const courses = { } const pages = { - courseDetail: (readableId: string) => + coursePages: (readableId: string) => `${API_BASE_URL}/api/v2/pages/?fields=*&type=cms.CoursePage&readable_id=${encodeURIComponent( readableId, )}`, + programPages: (readableId: string) => + `${API_BASE_URL}/api/v2/pages/?fields=*&type=cms.ProgramPage&readable_id=${encodeURIComponent( + readableId, + )}`, } const organization = { diff --git a/frontends/main/src/app-pages/ProductPages/AboutSection.test.tsx b/frontends/main/src/app-pages/ProductPages/AboutSection.test.tsx new file mode 100644 index 0000000000..692709fe6b --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/AboutSection.test.tsx @@ -0,0 +1,96 @@ +import React from "react" +import { factories } from "api/mitxonline-test-utils" +import { renderWithProviders, screen, within, user } from "@/test-utils" +import invariant from "tiny-invariant" +import AboutSection from "./AboutSection" +import { faker } from "@faker-js/faker/locale/en" + +const makePage = factories.pages.coursePageItem + +const expectRawContent = (el: HTMLElement, htmlString: string) => { + const raw = within(el).getByTestId("raw") + expect(htmlString.length).toBeGreaterThan(0) + expect(raw.innerHTML).toBe(htmlString) +} + +test("About section has expected content", async () => { + const about = `

${faker.lorem.paragraph()}

` + const noun = faker.word.noun() + renderWithProviders() + + const section = await screen.findByRole("region", { + name: `About this ${noun}`, + }) + expectRawContent(section, about) +}) + +test("About section expands and collapses", async () => { + const firstParagraph = "This paragraph should be initially shown." + const secondParagraph = "This should be hidden 1." + const thirdParagraph = "T his should be hidden 2." + const aboutContent = [firstParagraph, secondParagraph, thirdParagraph] + .map((p) => `

${p}

`) + .join("\n") + const noun = faker.word.noun() + + const page = makePage({ about: aboutContent }) + invariant(page.about) + renderWithProviders( + , + ) + + const about = await screen.findByRole("region", { + name: `About this ${noun}`, + }) + + const p1 = within(about).getByText(firstParagraph) + const p2 = within(about).queryByText(secondParagraph) + const p3 = within(about).queryByText(thirdParagraph) + + expect(p1).toBeVisible() + expect(p2).not.toBeVisible() + expect(p3).not.toBeVisible() + + const toggle = within(about).getByRole("button", { name: "Show more" }) + await user.click(toggle) + + expect(p1).toBeVisible() + expect(p2).toBeVisible() + expect(p3).toBeVisible() + + expect(toggle).toHaveTextContent("Show less") +}) + +test.each([ + { aboutParagraphs: 1, expectToggler: false }, + { aboutParagraphs: 2, expectToggler: true }, +])( + "Show more/less link is not shown if there is only one paragraph in the About section", + async ({ aboutParagraphs, expectToggler }) => { + const aboutContent = Array.from( + { length: aboutParagraphs }, + (_, i) => `

This is paragraph ${i + 1} in the about section.

`, + ).join("\n") + const noun = faker.word.noun() + + renderWithProviders( + , + ) + + const about = await screen.findByRole("region", { + name: `About this ${noun}`, + }) + + const toggler = within(about).getByRole("button", { + hidden: true, + }) + // Can't reliably use name matcher because accessible name isn't computed when hidden. + expect(toggler).toHaveTextContent(/show more|show less/i) + + if (expectToggler) { + expect(toggler).toBeVisible() + } else { + expect(toggler).not.toBeVisible() + } + }, +) diff --git a/frontends/main/src/app-pages/ProductPages/AboutSection.tsx b/frontends/main/src/app-pages/ProductPages/AboutSection.tsx new file mode 100644 index 0000000000..a6778ede6c --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/AboutSection.tsx @@ -0,0 +1,64 @@ +import React from "react" +import { UnderlinedLink } from "./ProductSummary" +import { HeadingIds } from "./util" +import RawHTML from "./RawHTML" +import { Typography } from "ol-components" +import { styled } from "@mitodl/smoot-design" + +const AboutSectionRoot = styled.section<{ expanded: boolean }>( + ({ expanded }) => { + return [ + { + display: "flex", + flexDirection: "column", + gap: "16px", + /** + * Note: browser suppport for ':has' is decent but not universal. + * At worst, users will see show more/less link when they shouldn't. + */ + "&:has(.raw-html > *:only-child)": { + "& .show-more-less": { + display: "none", + }, + }, + }, + !expanded && { + ".raw-html > *:not(:first-child)": { + display: "none", + }, + }, + ] + }, +) + +const AboutSection: React.FC<{ + aboutHtml: string + productNoun: string +}> = ({ aboutHtml, productNoun }) => { + const [aboutExpanded, setAboutExpanded] = React.useState(false) + return ( + + + About this {productNoun} + + + { + e.preventDefault() + setAboutExpanded((curr) => !curr) + }} + > + {aboutExpanded ? "Show less" : "Show more"} + + + ) +} + +export default AboutSection diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx index 3fb01780ee..2ef00c0535 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.test.tsx @@ -5,19 +5,13 @@ import type { CourseWithCourseRunsSerializerV2, } from "@mitodl/mitxonline-api-axios/v2" import { setMockResponse } from "api/test-utils" -import { - renderWithProviders, - waitFor, - screen, - within, - user, -} from "@/test-utils" -import CoursePage, { HeadingIds } from "./CoursePage" +import { renderWithProviders, waitFor, screen, within } from "@/test-utils" +import CoursePage from "./CoursePage" +import { HeadingIds } from "./util" import { assertHeadings } from "ol-test-utilities" import { notFound } from "next/navigation" import { useFeatureFlagEnabled } from "posthog-js/react" -import { faker } from "@faker-js/faker/locale/en" import invariant from "tiny-invariant" jest.mock("posthog-js/react") @@ -44,7 +38,7 @@ const setupApis = ({ { results: [course] }, ) - setMockResponse.get(urls.pages.courseDetail(course.readable_id), { + setMockResponse.get(urls.pages.coursePages(course.readable_id), { items: [page], }) } @@ -84,11 +78,11 @@ describe("CoursePage", () => { assertHeadings([ { level: 1, name: page.title }, { level: 2, name: "Course summary" }, - { level: 2, name: "About this course" }, + { level: 2, name: "About this Course" }, { level: 2, name: "What you'll learn" }, { level: 2, name: "Prerequisites" }, { level: 2, name: "Meet your instructors" }, - { level: 2, name: "Who can take this course?" }, + { level: 2, name: "Who can take this Course?" }, ]) }) }) @@ -109,7 +103,7 @@ describe("CoursePage", () => { expect(links[1]).toHaveTextContent("What you'll learn") expect(links[1]).toHaveAttribute("href", `#${HeadingIds.What}`) expect(links[2]).toHaveTextContent("Prerequisites") - expect(links[2]).toHaveAttribute("href", `#${HeadingIds.Prerequisites}`) + expect(links[2]).toHaveAttribute("href", `#${HeadingIds.Prereqs}`) expect(links[3]).toHaveTextContent("Instructors") expect(links[3]).toHaveAttribute("href", `#${HeadingIds.Instructors}`) @@ -119,6 +113,7 @@ describe("CoursePage", () => { }) }) + // Collasping sections tested in AboutSection.test.tsx test("About section has expected content", async () => { const course = makeCourse() const page = makePage({ course_details: course }) @@ -127,91 +122,37 @@ describe("CoursePage", () => { renderWithProviders() const section = await screen.findByRole("region", { - name: "About this course", + name: "About this Course", }) expectRawContent(section, page.about) }) - test("About section expands and collapses", async () => { + test("What You'll Learn section has expected content", async () => { const course = makeCourse() - const firstParagraph = "This paragraph should be initially shown." - const secondParagraph = "This should be hidden 1." - const thirdParagraph = "This should be hidden 2." - const aboutContent = [firstParagraph, secondParagraph, thirdParagraph] - .map((p) => `

${p}

`) - .join("\n") - const page = makePage({ course_details: course, about: aboutContent }) + const page = makePage({ course_details: course }) + invariant(page.what_you_learn) setupApis({ course, page }) renderWithProviders() - const about = await screen.findByRole("region", { - name: "About this course", + const section = await screen.findByRole("region", { + name: "What you'll learn", }) - - const p1 = within(about).getByText(firstParagraph) - const p2 = within(about).queryByText(secondParagraph) - const p3 = within(about).queryByText(thirdParagraph) - - expect(p1).toBeVisible() - expect(p2).not.toBeVisible() - expect(p3).not.toBeVisible() - - const toggle = within(about).getByRole("button", { name: "Show more" }) - await user.click(toggle) - - expect(p1).toBeVisible() - expect(p2).toBeVisible() - expect(p3).toBeVisible() - - expect(toggle).toHaveTextContent("Show less") + expectRawContent(section, page.what_you_learn) }) - test.each([ - { aboutParagraphs: 1, expectToggler: false }, - { aboutParagraphs: 2, expectToggler: true }, - ])( - "Show more/less link is not shown if there is only one paragraph in the About section", - async ({ aboutParagraphs, expectToggler }) => { - const course = makeCourse() - const aboutContent = Array.from( - { length: aboutParagraphs }, - (_, i) => `

This is paragraph ${i + 1} in the about section.

`, - ).join("\n") - const page = makePage({ course_details: course, about: aboutContent }) - setupApis({ course, page }) - renderWithProviders(, { - url: `/courses/${course.readable_id}/`, - }) - - const about = await screen.findByRole("region", { - name: "About this course", - }) - - const toggler = within(about).getByRole("button", { - hidden: true, - }) - // Can't reliably use name matcher because accessible name isn't computed when hidden. - expect(toggler).toHaveTextContent(/show more|show less/i) - - if (expectToggler) { - expect(toggler).toBeVisible() - } else { - expect(toggler).not.toBeVisible() - } - }, - ) - - test("What You'll Learn section has expected content", async () => { + // Dialog tested in InstructorsSection.test.tsx + test("Instructors section has expected content", async () => { const course = makeCourse() const page = makePage({ course_details: course }) - invariant(page.what_you_learn) + invariant(page.faculty.length > 0) setupApis({ course, page }) renderWithProviders() const section = await screen.findByRole("region", { - name: "What you'll learn", + name: "Meet your instructors", }) - expectRawContent(section, page.what_you_learn) + const items = within(section).getAllByRole("listitem") + expect(items.length).toBe(page.faculty.length) }) test("Prerequisites section has expected content", async () => { @@ -234,7 +175,7 @@ describe("CoursePage", () => { urls.courses.coursesList({ readable_id: "readable_id" }), { results: courses }, ) - setMockResponse.get(urls.pages.courseDetail("readable_id"), { + setMockResponse.get(urls.pages.coursePages("readable_id"), { items: pages, }) @@ -243,45 +184,4 @@ describe("CoursePage", () => { expect(notFound).toHaveBeenCalled() }) }) - - test("Instructors section has expected content", async () => { - const course = makeCourse() - const page = makePage({ course_details: course }) - const instructors = page.faculty - expect(instructors.length).toBeGreaterThan(0) - - setupApis({ course, page }) - renderWithProviders() - - const section = await screen.findByRole("region", { - name: "Meet your instructors", - }) - const instructorEls = within(section).getAllByRole("listitem") - instructorEls.forEach((el, i) => { - const instructor = instructors[i] - within(el).getByRole("button", { name: instructor.instructor_name }) - within(el).getByText(instructor.instructor_title) - }) - - const instructor = faker.helpers.arrayElement(instructors) - const button = within(section).getByRole("button", { - name: instructor.instructor_name, - }) - await user.click(button) - - const dialog = await screen.findByRole("dialog", { - name: `${instructor.instructor_name}`, - }) - within(dialog).getByRole("heading", { - level: 2, - name: instructor.instructor_name, - }) - expectRawContent(dialog, instructor.instructor_bio_long) - - const closeButton = within(dialog).getByRole("button", { name: "Close" }) - await user.click(closeButton) - await waitFor(() => { - expect(dialog).not.toBeInTheDocument() - }) - }) }) diff --git a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx index eba73e70a1..c91c40312f 100644 --- a/frontends/main/src/app-pages/ProductPages/CoursePage.tsx +++ b/frontends/main/src/app-pages/ProductPages/CoursePage.tsx @@ -1,232 +1,32 @@ "use client" -import React, { memo, useId } from "react" -import { - Container, - Stack, - Breadcrumbs, - BannerBackground, - Typography, - MuiDialog, -} from "ol-components" -import { backgroundSrcSetCSS } from "ol-utilities" -import { HOME } from "@/common/urls" -import backgroundSteps from "@/public/images/backgrounds/background_steps.jpg" +import React from "react" +import { Stack, Typography } from "ol-components" + import { pagesQueries } from "api/mitxonline-hooks/pages" import { useQuery } from "@tanstack/react-query" -import { ActionButton, ButtonLink, styled } from "@mitodl/smoot-design" -import Image from "next/image" -import DOMPurify from "isomorphic-dompurify" -import type { Faculty } from "@mitodl/mitxonline-api-axios/v2" +import { styled } from "@mitodl/smoot-design" import { coursesQueries } from "api/mitxonline-hooks/courses" -import { CourseSummary, UnderlinedLink } from "./CourseSummary" +import { CourseSummary } from "./ProductSummary" import { useFeatureFlagEnabled } from "posthog-js/react" import { FeatureFlags } from "@/common/feature_flags" import { notFound } from "next/navigation" -import { RiCloseLine } from "@remixicon/react" +import { HeadingIds } from "./util" +import InstructorsSection from "./InstructorsSection" +import RawHTML from "./RawHTML" +import AboutSection from "./AboutSection" +import ProductPageTemplate, { + HeadingData, + ProductNavbar, + WhoCanTake, +} from "./ProductPageTemplate" +import { CoursePageItem } from "@mitodl/mitxonline-api-axios/v2" +import { DEFAULT_RESOURCE_IMG } from "ol-utilities" type CoursePageProps = { readableId: string } -const StyledBreadcrumbs = styled(Breadcrumbs)(({ theme }) => ({ - paddingBottom: "24px", - [theme.breakpoints.down("md")]: { - paddingBottom: "16px", - }, -})) - -const TitleBox = styled(Stack)(({ theme }) => ({ - color: theme.custom.colors.white, -})) -const OfferedByTag = styled.div(({ theme }) => ({ - backgroundColor: theme.custom.colors.darkGray1, - display: "flex", - alignItems: "center", - justifyContent: "center", - height: "32px", - padding: "0 12px", - borderRadius: "4px", - marginBottom: "4px", - ...theme.typography.subtitle1, - [theme.breakpoints.down("md")]: { - display: "none", - }, -})) - -const Page = styled.div(({ theme }) => ({ - backgroundColor: theme.custom.colors.white, - paddingBottom: "80px", - [theme.breakpoints.down("md")]: { - paddingBottom: "24px", - }, - ".raw-include": { - "*:first-child": { - marginTop: 0, - }, - ...theme.typography.body1, - lineHeight: "1.5", - p: { - marginTop: "16px", - marginBottom: "0", - }, - "& > ul": { - listStyleType: "none", - marginTop: "16px", - marginBottom: 0, - padding: 0, - "> li": { - padding: "16px", - border: `1px solid ${theme.custom.colors.lightGray2}`, - borderBottom: "none", - ":first-of-type": { - borderRadius: "4px 4px 0 0", - }, - ":last-of-type": { - borderBottom: `1px solid ${theme.custom.colors.lightGray2}`, - borderRadius: "0 0 4px 4px", - }, - ":first-of-type:last-of-type": { - borderRadius: "4px", - }, - }, - }, - - [theme.breakpoints.down("md")]: { - ...theme.typography.body2, - }, - }, -})) - -const TopContainer = styled(Container)({ - display: "flex", - justifyContent: "space-between", - gap: "60px", -}) -const BottomContainer = styled(Container)(({ theme }) => ({ - display: "flex", - justifyContent: "space-between", - gap: "60px", - flexDirection: "row-reverse", - - [theme.breakpoints.down("md")]: { - flexDirection: "column", - alignItems: "center", - gap: "0px", - }, -})) - -const MainCol = styled.div({ - flex: 1, - display: "flex", - flexDirection: "column", -}) - -const SidebarCol = styled.div(({ theme }) => ({ - width: "100%", - maxWidth: "410px", - [theme.breakpoints.down("md")]: { - marginTop: "24px", - }, -})) -const SidebarSpacer = styled.div(({ theme }) => ({ - width: "410px", - [theme.breakpoints.down("md")]: { - display: "none", - }, -})) - -const StyledLink = styled(ButtonLink)(({ theme }) => ({ - backgroundColor: theme.custom.colors.white, - borderColor: theme.custom.colors.white, - [theme.breakpoints.down("md")]: { - backgroundColor: theme.custom.colors.lightGray1, - border: `1px solid ${theme.custom.colors.lightGray2}`, - }, -})) - -const LinksContainer = styled.nav(({ theme }) => ({ - display: "flex", - flexWrap: "wrap", - [theme.breakpoints.up("md")]: { - gap: "24px", - padding: "12px 16px", - backgroundColor: theme.custom.colors.white, - borderRadius: "4px", - border: `1px solid ${theme.custom.colors.lightGray2}`, - boxShadow: "0 8px 20px 0 rgba(120, 147, 172, 0.10)", - marginTop: "-24px", - width: "calc(100%)", - marginBottom: "40px", - }, - [theme.breakpoints.down("md")]: { - alignSelf: "center", - gap: "8px", - rowGap: "16px", - padding: 0, - margin: "24px 0", - }, -})) - -const SidebarImageWrapper = styled.div(({ theme }) => ({ - [theme.breakpoints.up("md")]: { - height: "0px", - }, -})) -const SidebarImage = styled(Image)(({ theme }) => ({ - borderRadius: "4px", - width: "100%", - maxWidth: "410px", - height: "230px", - display: "block", - [theme.breakpoints.up("md")]: { - transform: "translateY(-100%)", - }, - [theme.breakpoints.down("md")]: { - border: `1px solid ${theme.custom.colors.lightGray2}`, - borderRadius: "4px 4px 0 0", - }, -})) - -const RawHTML: React.FC<{ html: string }> = memo( - // Note: Using 'memo' here makes this behave more like a normal react component. - // The underlying HTML elements will remain complemtely unchanged unless the - // 'html' prop changes. - // Without memo, the elements are replaced one each render. - ({ html }) => { - return ( -
- ) - }, -) - -const AboutSection = styled.section<{ expanded: boolean }>(({ expanded }) => { - return [ - { - display: "flex", - flexDirection: "column", - gap: "16px", - /** - * Note: browser suppport for ':has' is decent but not universal. - * At worst, users will see show more/less link when they shouldn't. - */ - "&:has(.raw-include > *:only-child)": { - "& .show-more-less": { - display: "none", - }, - }, - }, - !expanded && { - ".raw-include > *:not(:first-child)": { - display: "none", - }, - }, - ] -}) const WhatSection = styled.section({ display: "flex", flexDirection: "column", @@ -238,214 +38,50 @@ const PrerequisitesSection = styled.section({ gap: "16px", }) -const InstructorsSection = styled.section({}) -const InstructorsList = styled.ul(({ theme }) => ({ - display: "flex", - flexWrap: "wrap", - padding: 0, - margin: 0, - marginTop: "24px", - gap: "24px", - [theme.breakpoints.down("sm")]: { - gap: "16px", - justifyContent: "center", - }, -})) -const InstructorCardRoot = styled.li(({ theme }) => ({ - display: "flex", - flexDirection: "column", - gap: "8px", - border: `1px solid ${theme.custom.colors.lightGray2}`, - borderRadius: "8px", - padding: "16px", - width: "252px", - minHeight: "272px", - [theme.breakpoints.down("sm")]: { - width: "calc(50% - 8px)", - minWidth: "162px", - }, - ":hover": { - boxShadow: "0 8px 20px 0 rgba(120, 147, 172, 0.10)", - cursor: "pointer", - }, -})) -const InstructorButton = styled.button(({ theme }) => ({ - backgroundColor: "unset", - border: "none", - textAlign: "left", - padding: 0, - ...theme.typography.h5, - marginTop: "8px", - cursor: "inherit", -})) -const InstructorImage = styled(Image)(({ theme }) => ({ - height: "140px", - width: "100%", - objectFit: "cover", - borderRadius: "8px", - [theme.breakpoints.down("sm")]: { - height: "155px", - }, -})) -const InstructorCard: React.FC<{ - instructor: Faculty -}> = ({ instructor }) => { - const [open, setOpen] = React.useState(false) - return ( - <> - setOpen(true)}> - - {instructor.instructor_name} - {instructor.instructor_title} - - setOpen(false)} - /> - - ) -} - -const CloseButton = styled(ActionButton)(({ theme }) => ({ - position: "absolute", - top: "24px", - right: "28px", - backgroundColor: theme.custom.colors.lightGray2, - "&&:hover": { - backgroundColor: theme.custom.colors.red, - color: theme.custom.colors.white, - }, - [theme.breakpoints.down("md")]: { - right: "16px", - }, -})) -const DialogImage = styled(Image)({ - width: "100%", - aspectRatio: "1.92", - objectFit: "cover", -}) -const DialogContent = styled.div(({ theme }) => ({ - padding: "32px", - ".raw-include": { - ...theme.typography.body2, - "*:first-child": { - marginTop: 0, +const getNavLinks = (page: CoursePageItem): HeadingData[] => { + const all = [ + { + id: HeadingIds.About, + label: "About", + variant: "primary", + content: page.about, }, - p: { - marginTop: "8px", - marginBottom: "0", + { + id: HeadingIds.What, + label: "What you'll learn", + variant: "secondary", + content: page.what_you_learn, }, - }, -})) -const InstructorDialog: React.FC<{ - instructor: Faculty - open: boolean - onClose: () => void -}> = ({ instructor, open, onClose }) => { - const titleId = useId() - return ( - - - - - - - - {instructor.instructor_name} - - - {instructor.instructor_title} - - - - - ) -} - -const WhoCanTakeSection = styled.section(({ theme }) => ({ - padding: "32px", - border: `1px solid ${theme.custom.colors.lightGray2}`, - borderRadius: "8px", - display: "flex", - flexDirection: "column", - gap: "24px", - ...theme.typography.body1, - lineHeight: "1.5", - [theme.breakpoints.down("md")]: { - padding: "16px", - gap: "16px", - ...theme.typography.body2, - }, -})) - -enum HeadingIds { - About = "about", - What = "what-you-will-learn", - Prerequisites = "prerequisites", - Instructors = "instructors", - WhoCanTake = "who-can-take-this-course", -} - -type HeadingData = { - id: HeadingIds - label: string - variant: "primary" | "secondary" + { + id: HeadingIds.Prereqs, + label: "Prerequisites", + variant: "secondary", + content: page.prerequisites, + }, + { + id: HeadingIds.Instructors, + label: "Instructors", + variant: "secondary", + content: page.faculty.length ? "x" : undefined, + }, + ] as const + return all.filter((item) => item.content) } -const HEADINGS: HeadingData[] = [ - { id: HeadingIds.About, label: "About", variant: "primary" }, - { id: HeadingIds.What, label: "What you'll learn", variant: "secondary" }, - { - id: HeadingIds.Prerequisites, - label: "Prerequisites", - variant: "secondary", - }, - { id: HeadingIds.Instructors, label: "Instructors", variant: "secondary" }, -] const CoursePage: React.FC = ({ readableId }) => { - const courseDetail = useQuery(pagesQueries.courseDetail(readableId)) + const pages = useQuery(pagesQueries.coursePages(readableId)) const courses = useQuery( coursesQueries.coursesList({ readable_id: readableId }), ) - const page = courseDetail.data?.items[0] + const page = pages.data?.items[0] const course = courses.data?.results?.[0] - const [aboutExpanded, setAboutExpanded] = React.useState(false) const enabled = useFeatureFlagEnabled(FeatureFlags.ProductPageCourse) if (enabled === false) { return notFound() } if (!enabled) return - const doneLoading = courseDetail.isSuccess && courses.isSuccess + const doneLoading = pages.isSuccess && courses.isSuccess if (!page || !course) { if (doneLoading) { @@ -455,148 +91,50 @@ const CoursePage: React.FC = ({ readableId }) => { } } - return ( - - - - - - - MITx - - - {page.title} - - - {page.course_details.page.description} - - - - - - - - - - - - - - - - - {HEADINGS.map((heading) => { - const LinkComponent = - heading.variant === "primary" ? ButtonLink : StyledLink - return ( - - {heading.label} - - ) - })} - - - {page.about ? ( - - - About this course - - - { - e.preventDefault() - setAboutExpanded((curr) => !curr) - }} - > - {aboutExpanded ? "Show less" : "Show more"} - - - ) : null} + const navLinks = getNavLinks(page) - {page.what_you_learn ? ( - - - What you'll learn - - - - ) : null} - {page.prerequisites ? ( - - - Prerequisites - - - - ) : null} - {page.faculty.length ? ( - - - Meet your instructors - - - {page.faculty.map((instructor) => { - return ( - - ) - })} - - - ) : null} + const imageSrc = page.feature_image + ? page.course_details.page.feature_image_src + : DEFAULT_RESOURCE_IMG - - - Who can take this course? - - Because of U.S. Office of Foreign Assets Control (OFAC) - restrictions and other U.S. federal regulations, learners residing - in one or more of the following countries or regions will not be - able to register for this course: Iran, Cuba, Syria, North Korea - and the Crimea, Donetsk People's Republic and Luhansk People's - Republic regions of Ukraine. - - - - - + return ( + } + navLinks={navLinks} + > + + + {page.about ? ( + + ) : null} + {page.what_you_learn ? ( + + + What you'll learn + + + + ) : null} + {page.prerequisites ? ( + + + Prerequisites + + + + ) : null} + {page.faculty.length ? ( + + ) : null} + + + ) } export default CoursePage -export { HeadingIds } diff --git a/frontends/main/src/app-pages/ProductPages/InstructorsSection.test.tsx b/frontends/main/src/app-pages/ProductPages/InstructorsSection.test.tsx new file mode 100644 index 0000000000..62570a7ec2 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/InstructorsSection.test.tsx @@ -0,0 +1,68 @@ +import React from "react" +import { factories } from "api/mitxonline-test-utils" + +import { + renderWithProviders, + waitFor, + screen, + within, + user, +} from "@/test-utils" + +import { getByImageSrc } from "ol-test-utilities" + +import { faker } from "@faker-js/faker/locale/en" +import InstructorsSection from "./InstructorsSection" + +const makeFaculty = factories.pages.faculty + +const expectRawContent = (el: HTMLElement, htmlString: string) => { + const raw = within(el).getByTestId("raw") + expect(htmlString.length).toBeGreaterThan(0) + expect(raw.innerHTML).toBe(htmlString) +} + +test("Renders each instructor", async () => { + const instructors = Array.from({ length: 3 }, () => makeFaculty()) + renderWithProviders() + + const section = await screen.findByRole("region", { + name: "Meet your instructors", + }) + + const items = within(section).getAllByRole("listitem") + + expect(instructors.length).toBe(3) + instructors.forEach((instructor, index) => { + const item = items[index] + within(item).getByRole("button", { name: instructor.instructor_name }) + within(item).getByText(instructor.instructor_title) + getByImageSrc(item, instructor.feature_image_src) + }) +}) + +test("Opens and closes instructor dialog", async () => { + const instructors = Array.from({ length: 3 }, () => makeFaculty()) + renderWithProviders() + + const instructor = faker.helpers.arrayElement(instructors) + const button = screen.getByRole("button", { + name: instructor.instructor_name, + }) + await user.click(button) + + const dialog = await screen.findByRole("dialog", { + name: `${instructor.instructor_name}`, + }) + within(dialog).getByRole("heading", { + level: 2, + name: instructor.instructor_name, + }) + expectRawContent(dialog, instructor.instructor_bio_long) + + const closeButton = within(dialog).getByRole("button", { name: "Close" }) + await user.click(closeButton) + await waitFor(() => { + expect(dialog).not.toBeInTheDocument() + }) +}) diff --git a/frontends/main/src/app-pages/ProductPages/InstructorsSection.tsx b/frontends/main/src/app-pages/ProductPages/InstructorsSection.tsx new file mode 100644 index 0000000000..d3d14971c3 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/InstructorsSection.tsx @@ -0,0 +1,183 @@ +"use client" + +import React, { useId } from "react" +import { Typography, MuiDialog } from "ol-components" +import Image from "next/image" +import { ActionButton, styled } from "@mitodl/smoot-design" + +import type { Faculty } from "@mitodl/mitxonline-api-axios/v2" +import { HeadingIds } from "./util" +import { RiCloseLine } from "@remixicon/react" +import RawHTML from "./RawHTML" + +const InstructorsSectionRoot = styled.section({}) +const InstructorsList = styled.ul(({ theme }) => ({ + display: "flex", + flexWrap: "wrap", + padding: 0, + margin: 0, + marginTop: "24px", + gap: "24px", + [theme.breakpoints.down("sm")]: { + gap: "16px", + justifyContent: "center", + }, +})) +const InstructorCardRoot = styled.li(({ theme }) => ({ + display: "flex", + flexDirection: "column", + gap: "8px", + border: `1px solid ${theme.custom.colors.lightGray2}`, + borderRadius: "8px", + padding: "16px", + width: "252px", + minHeight: "272px", + [theme.breakpoints.down("sm")]: { + width: "calc(50% - 8px)", + minWidth: "162px", + }, + ":hover": { + boxShadow: "0 8px 20px 0 rgba(120, 147, 172, 0.10)", + cursor: "pointer", + }, +})) +const InstructorButton = styled.button(({ theme }) => ({ + backgroundColor: "unset", + border: "none", + textAlign: "left", + padding: 0, + ...theme.typography.h5, + marginTop: "8px", + cursor: "inherit", +})) +const InstructorImage = styled(Image)(({ theme }) => ({ + height: "140px", + width: "100%", + objectFit: "cover", + borderRadius: "8px", + [theme.breakpoints.down("sm")]: { + height: "155px", + }, +})) +const InstructorCard: React.FC<{ + instructor: Faculty +}> = ({ instructor }) => { + const [open, setOpen] = React.useState(false) + return ( + <> + setOpen(true)}> + + {instructor.instructor_name} + {instructor.instructor_title} + + setOpen(false)} + /> + + ) +} + +const CloseButton = styled(ActionButton)(({ theme }) => ({ + position: "absolute", + top: "24px", + right: "28px", + backgroundColor: theme.custom.colors.lightGray2, + "&&:hover": { + backgroundColor: theme.custom.colors.red, + color: theme.custom.colors.white, + }, + [theme.breakpoints.down("md")]: { + right: "16px", + }, +})) +const DialogImage = styled(Image)({ + width: "100%", + aspectRatio: "1.92", + objectFit: "cover", +}) +const DialogContent = styled.div(({ theme }) => ({ + padding: "32px", + ".raw-include": { + ...theme.typography.body2, + "*:first-child": { + marginTop: 0, + }, + p: { + marginTop: "8px", + marginBottom: "0", + }, + }, +})) +const InstructorDialog: React.FC<{ + instructor: Faculty + open: boolean + onClose: () => void +}> = ({ instructor, open, onClose }) => { + const titleId = useId() + return ( + + + + + + + + {instructor.instructor_name} + + + {instructor.instructor_title} + + + + + ) +} + +const InstructorsSection: React.FC<{ instructors: Faculty[] }> = ({ + instructors, +}) => { + return ( + + + Meet your instructors + + + {instructors.map((instructor) => { + return + })} + + + ) +} + +export default InstructorsSection diff --git a/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx new file mode 100644 index 0000000000..398762c810 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/ProductPageTemplate.tsx @@ -0,0 +1,265 @@ +"use client" + +import React from "react" +import { + Container, + Stack, + Breadcrumbs, + BannerBackground, + Typography, +} from "ol-components" +import { backgroundSrcSetCSS } from "ol-utilities" +import { HOME } from "@/common/urls" +import backgroundSteps from "@/public/images/backgrounds/background_steps.jpg" +import { ButtonLink, styled } from "@mitodl/smoot-design" +import Image from "next/image" +import { HeadingIds } from "./util" + +const StyledBreadcrumbs = styled(Breadcrumbs)(({ theme }) => ({ + paddingBottom: "24px", + [theme.breakpoints.down("md")]: { + paddingBottom: "16px", + }, +})) + +const TitleBox = styled(Stack)(({ theme }) => ({ + color: theme.custom.colors.white, +})) +const OfferedByTag = styled.div(({ theme }) => ({ + backgroundColor: theme.custom.colors.darkGray1, + display: "flex", + alignItems: "center", + justifyContent: "center", + height: "32px", + padding: "0 12px", + borderRadius: "4px", + marginBottom: "4px", + ...theme.typography.subtitle1, + [theme.breakpoints.down("md")]: { + display: "none", + }, +})) + +const Page = styled.div(({ theme }) => ({ + backgroundColor: theme.custom.colors.white, + paddingBottom: "80px", + [theme.breakpoints.down("md")]: { + paddingBottom: "24px", + }, + height: "100%", +})) + +const TopContainer = styled(Container)({ + display: "flex", + justifyContent: "space-between", + gap: "56px", +}) +const BottomContainer = styled(Container)(({ theme }) => ({ + display: "flex", + justifyContent: "space-between", + gap: "56px", + flexDirection: "row-reverse", + + [theme.breakpoints.down("md")]: { + flexDirection: "column", + alignItems: "center", + gap: "0px", + }, +})) + +const MainCol = styled.div({ + flex: 1, + display: "flex", + flexDirection: "column", +}) + +const SidebarCol = styled.div(({ theme }) => ({ + width: "100%", + maxWidth: "410px", + [theme.breakpoints.down("md")]: { + marginTop: "24px", + }, +})) +const SidebarSpacer = styled.div(({ theme }) => ({ + width: "410px", + [theme.breakpoints.down("md")]: { + display: "none", + }, +})) + +const StyledLink = styled(ButtonLink)(({ theme }) => ({ + backgroundColor: theme.custom.colors.white, + borderColor: theme.custom.colors.white, + [theme.breakpoints.down("md")]: { + backgroundColor: theme.custom.colors.lightGray1, + border: `1px solid ${theme.custom.colors.lightGray2}`, + }, +})) + +const LinksContainer = styled.nav(({ theme }) => ({ + display: "flex", + flexWrap: "wrap", + [theme.breakpoints.up("md")]: { + gap: "24px", + padding: "12px 16px", + backgroundColor: theme.custom.colors.white, + borderRadius: "4px", + border: `1px solid ${theme.custom.colors.lightGray2}`, + boxShadow: "0 8px 20px 0 rgba(120, 147, 172, 0.10)", + marginTop: "-24px", + width: "calc(100%)", + marginBottom: "40px", + }, + [theme.breakpoints.down("md")]: { + alignSelf: "center", + gap: "8px", + rowGap: "16px", + padding: 0, + margin: "24px 0", + }, +})) + +const SidebarImageWrapper = styled.div(({ theme }) => ({ + [theme.breakpoints.up("md")]: { + height: "0px", + }, +})) +const SidebarImage = styled(Image)(({ theme }) => ({ + borderRadius: "4px", + width: "100%", + maxWidth: "410px", + height: "230px", + display: "block", + [theme.breakpoints.up("md")]: { + transform: "translateY(-100%)", + }, + [theme.breakpoints.down("md")]: { + border: `1px solid ${theme.custom.colors.lightGray2}`, + borderRadius: "4px 4px 0 0", + }, +})) + +const WhoCanTakeSection = styled.section(({ theme }) => ({ + padding: "32px", + border: `1px solid ${theme.custom.colors.lightGray2}`, + borderRadius: "8px", + display: "flex", + flexDirection: "column", + gap: "24px", + ...theme.typography.body1, + lineHeight: "1.5", + [theme.breakpoints.down("md")]: { + padding: "16px", + gap: "16px", + ...theme.typography.body2, + }, +})) + +type HeadingData = { + id: HeadingIds + label: string + variant: "primary" | "secondary" +} + +type ProductPageTemplateProps = { + offeredBy: string + currentBreadcrumbLabel: string + title: string + shortDescription: React.ReactNode + imageSrc: string + sidebarSummary: React.ReactNode + children: React.ReactNode + navLinks: HeadingData[] +} +const ProductPageTemplate: React.FC = ({ + offeredBy, + currentBreadcrumbLabel, + title, + shortDescription, + imageSrc, + sidebarSummary, + children, +}) => { + return ( + + + + + + + {offeredBy} + + + {title} + + + {shortDescription} + + + + + + + + + + + + + {sidebarSummary} + + {children} + + + ) +} + +const ProductNavbar: React.FC<{ + navLinks: HeadingData[] + productNoun: string +}> = ({ navLinks, productNoun }) => { + if (navLinks.length === 0) { + return null + } + return ( + + {navLinks.map((heading) => { + const LinkComponent = + heading.variant === "primary" ? ButtonLink : StyledLink + return ( + + {heading.label} + + ) + })} + + ) +} + +const WhoCanTake: React.FC<{ productNoun: string }> = ({ productNoun }) => { + return ( + + + Who can take this {productNoun}? + + Because of U.S. Office of Foreign Assets Control (OFAC) restrictions and + other U.S. federal regulations, learners residing in one or more of the + following countries or regions will not be able to register for this + course: Iran, Cuba, Syria, North Korea and the Crimea, Donetsk People's + Republic and Luhansk People's Republic regions of Ukraine. + + ) +} + +export default ProductPageTemplate +export { WhoCanTake, ProductNavbar } +export type { HeadingData, ProductPageTemplateProps } diff --git a/frontends/main/src/app-pages/ProductPages/CourseSummary.test.tsx b/frontends/main/src/app-pages/ProductPages/ProductSummary.test.tsx similarity index 98% rename from frontends/main/src/app-pages/ProductPages/CourseSummary.test.tsx rename to frontends/main/src/app-pages/ProductPages/ProductSummary.test.tsx index 2d20c87b59..0dc2056a05 100644 --- a/frontends/main/src/app-pages/ProductPages/CourseSummary.test.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductSummary.test.tsx @@ -2,7 +2,7 @@ import React from "react" import { factories } from "api/mitxonline-test-utils" import { renderWithProviders, screen, within, user } from "@/test-utils" -import { CourseSummary, TestIds } from "./CourseSummary" +import { CourseSummary, TestIds } from "./ProductSummary" import { formatDate } from "ol-utilities" import invariant from "tiny-invariant" import { faker } from "@faker-js/faker/locale/en" @@ -89,7 +89,7 @@ describe("CourseSummary", () => { }) }) -describe("Dates Row", () => { +describe("Course Dates Row", () => { test("Renders expected start/end dates", async () => { const run = makeRun() const course = makeCourse({ @@ -227,7 +227,7 @@ describe("Course Format Row", () => { }) }) -describe("Duration Row", () => { +describe("Course Duration Row", () => { test.each([ { length: "5 weeks", @@ -257,7 +257,7 @@ describe("Duration Row", () => { ) }) -describe("Price Row", () => { +describe("Course Price Row", () => { test.each([ { label: "has no products", diff --git a/frontends/main/src/app-pages/ProductPages/CourseSummary.tsx b/frontends/main/src/app-pages/ProductPages/ProductSummary.tsx similarity index 96% rename from frontends/main/src/app-pages/ProductPages/CourseSummary.tsx rename to frontends/main/src/app-pages/ProductPages/ProductSummary.tsx index f25d3d9c96..341d1d0e76 100644 --- a/frontends/main/src/app-pages/ProductPages/CourseSummary.tsx +++ b/frontends/main/src/app-pages/ProductPages/ProductSummary.tsx @@ -411,4 +411,17 @@ const CourseSummary: React.FC<{ ) } -export { CourseSummary, UnderlinedLink, TestIds } +const ProgramSummary: React.FC = () => { + return ( + + +

Program summary

+ + + Program Summary Placeholder + + + ) +} + +export { CourseSummary, ProgramSummary, UnderlinedLink, TestIds } diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx new file mode 100644 index 0000000000..24ac349381 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.test.tsx @@ -0,0 +1,162 @@ +import React from "react" +import { urls, factories } from "api/mitxonline-test-utils" +import type { + V2Program, + ProgramPageItem, +} from "@mitodl/mitxonline-api-axios/v2" +import { setMockResponse } from "api/test-utils" +import { renderWithProviders, waitFor, screen, within } from "@/test-utils" +import ProgramPage from "./ProgramPage" +import { HeadingIds } from "./util" +import { assertHeadings } from "ol-test-utilities" +import { notFound } from "next/navigation" + +import { useFeatureFlagEnabled } from "posthog-js/react" +import invariant from "tiny-invariant" + +jest.mock("posthog-js/react") +const mockedUseFeatureFlagEnabled = jest.mocked(useFeatureFlagEnabled) + +const makeProgram = factories.programs.program +const makePage = factories.pages.programPageItem + +const expectRawContent = (el: HTMLElement, htmlString: string) => { + const raw = within(el).getByTestId("raw") + expect(htmlString.length).toBeGreaterThan(0) + expect(raw.innerHTML).toBe(htmlString) +} + +const setupApis = ({ + program, + page, +}: { + program: V2Program + page: ProgramPageItem +}) => { + setMockResponse.get( + urls.programs.programsList({ readable_id: program.readable_id }), + { results: [program] }, + ) + + setMockResponse.get(urls.pages.programPages(program.readable_id), { + items: [page], + }) +} + +describe("ProgramPage", () => { + beforeEach(() => { + mockedUseFeatureFlagEnabled.mockReturnValue(true) + }) + + test.each([true, false])( + "Calls noFound if and only the feature flag is disabled", + async (isEnabled) => { + mockedUseFeatureFlagEnabled.mockReturnValue(isEnabled) + + const program = makeProgram() + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders(, { + url: `/programs/${program.readable_id}/`, + }) + + if (isEnabled) { + expect(notFound).not.toHaveBeenCalled() + } else { + expect(notFound).toHaveBeenCalled() + } + }, + ) + + test("Page has expected headings", async () => { + const program = makeProgram() + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + await waitFor(() => { + assertHeadings([ + { level: 1, name: page.title }, + { level: 2, name: "Program summary" }, + { level: 2, name: "About this Program" }, + { level: 2, name: "What you'll learn" }, + { level: 2, name: "Prerequisites" }, + { level: 2, name: "Meet your instructors" }, + { level: 2, name: "Who can take this Program?" }, + ]) + }) + }) + + test("Page Navigation", async () => { + const program = makeProgram() + const page = makePage({ program_details: program }) + setupApis({ program, page }) + renderWithProviders() + + const nav = await screen.findByRole("navigation", { + name: "Program Details", + }) + const links = within(nav).getAllByRole("link") + + expect(links[0]).toHaveTextContent("About") + expect(links[0]).toHaveAttribute("href", `#${HeadingIds.About}`) + expect(links[1]).toHaveTextContent("What you'll learn") + expect(links[1]).toHaveAttribute("href", `#${HeadingIds.What}`) + expect(links[2]).toHaveTextContent("Prerequisites") + expect(links[2]).toHaveAttribute("href", `#${HeadingIds.Prereqs}`) + expect(links[3]).toHaveTextContent("Instructors") + expect(links[3]).toHaveAttribute("href", `#${HeadingIds.Instructors}`) + + const headings = screen.getAllByRole("heading") + Object.values(HeadingIds).forEach((id) => { + expect(headings.find((h) => h.id === id)).toBeVisible() + }) + }) + + // Collasping sections tested in AboutSection.test.tsx + test("About section has expected content", async () => { + const program = makeProgram() + const page = makePage({ program_details: program }) + invariant(page.about) + setupApis({ program, page }) + renderWithProviders() + const section = await screen.findByRole("region", { + name: "About this Program", + }) + expectRawContent(section, page.about) + }) + + // Dialog tested in InstructorsSection.test.tsx + test("Instructors section has expected content", async () => { + const program = makeProgram() + const page = makePage({ program_details: program }) + invariant(page.faculty.length > 0) + setupApis({ program, page }) + renderWithProviders() + + const section = await screen.findByRole("region", { + name: "Meet your instructors", + }) + const items = within(section).getAllByRole("listitem") + expect(items.length).toBe(page.faculty.length) + }) + + test.each([ + { courses: [], pages: [makePage()] }, + { courses: [makeProgram()], pages: [] }, + { courses: [], pages: [] }, + ])("Returns 404 if no course found", async ({ courses, pages }) => { + setMockResponse.get( + urls.programs.programsList({ readable_id: "readable_id" }), + { results: courses }, + ) + setMockResponse.get(urls.pages.programPages("readable_id"), { + items: pages, + }) + + renderWithProviders() + await waitFor(() => { + expect(notFound).toHaveBeenCalled() + }) + }) +}) diff --git a/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx new file mode 100644 index 0000000000..c323ec6074 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/ProgramPage.tsx @@ -0,0 +1,148 @@ +"use client" + +import React from "react" +import { Stack, Typography } from "ol-components" + +import { pagesQueries } from "api/mitxonline-hooks/pages" +import { useQuery } from "@tanstack/react-query" +import { styled } from "@mitodl/smoot-design" +import { programsQueries } from "api/mitxonline-hooks/programs" +import { useFeatureFlagEnabled } from "posthog-js/react" +import { FeatureFlags } from "@/common/feature_flags" +import { notFound } from "next/navigation" +import { HeadingIds } from "./util" +import InstructorsSection from "./InstructorsSection" +import RawHTML, { UnstyledRawHTML } from "./RawHTML" +import AboutSection from "./AboutSection" +import ProductPageTemplate, { + HeadingData, + ProductNavbar, + WhoCanTake, +} from "./ProductPageTemplate" +import { ProgramPageItem } from "@mitodl/mitxonline-api-axios/v2" +import { ProgramSummary } from "./ProductSummary" +import { DEFAULT_RESOURCE_IMG } from "ol-utilities" + +type ProgramPageProps = { + readableId: string +} + +const WhatSection = styled.section({ + display: "flex", + flexDirection: "column", + gap: "16px", +}) +const PrerequisitesSection = styled.section({ + display: "flex", + flexDirection: "column", + gap: "16px", +}) + +const getNavLinks = (page: ProgramPageItem): HeadingData[] => { + const all = [ + { + id: HeadingIds.About, + label: "About", + variant: "primary", + content: page.about, + }, + { + id: HeadingIds.What, + label: "What you'll learn", + variant: "secondary", + content: page.what_you_learn, + }, + { + id: HeadingIds.Prereqs, + label: "Prerequisites", + variant: "secondary", + content: page.prerequisites, + }, + { + id: HeadingIds.Instructors, + label: "Instructors", + variant: "secondary", + content: page.faculty.length ? "x" : undefined, + }, + ] as const + return all.filter((item) => item.content) +} + +const DescriptionHTML = styled(UnstyledRawHTML)({ + p: { margin: 0 }, +}) + +const ProgramPage: React.FC = ({ readableId }) => { + const pages = useQuery(pagesQueries.programPages(readableId)) + const programs = useQuery( + programsQueries.programsList({ readable_id: readableId }), + ) + const page = pages.data?.items[0] + const course = programs.data?.results?.[0] + const enabled = useFeatureFlagEnabled(FeatureFlags.ProductPageCourse) + if (enabled === false) { + return notFound() + } + if (!enabled) return + + const doneLoading = pages.isSuccess && programs.isSuccess + + if (!page || !course) { + if (doneLoading) { + return notFound() + } else { + return null + } + } + + const navLinks = getNavLinks(page) + + const imageSrc = page.feature_image + ? page.program_details.page.feature_image_src + : DEFAULT_RESOURCE_IMG + return ( + + } + imageSrc={imageSrc} + sidebarSummary={} + navLinks={navLinks} + > + + + {page.about ? ( + + ) : null} + {page.what_you_learn ? ( + + + What you'll learn + + + + ) : null} + {page.prerequisites ? ( + + + Prerequisites + + + + ) : null} + {page.faculty.length ? ( + + ) : null} + + + + ) +} + +export default ProgramPage diff --git a/frontends/main/src/app-pages/ProductPages/RawHTML.tsx b/frontends/main/src/app-pages/ProductPages/RawHTML.tsx new file mode 100644 index 0000000000..21836ba7ac --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/RawHTML.tsx @@ -0,0 +1,57 @@ +import DOMPurify from "isomorphic-dompurify" +import { styled } from "@mitodl/smoot-design" +import React, { memo } from "react" +import classnames from "classnames" + +const UnstyledRawHTML: React.FC<{ + html: string + className?: string + Component?: React.ElementType +}> = memo(({ html, className, Component = "div" }) => { + return ( + + ) +}) + +const RawHTML = styled(UnstyledRawHTML)(({ theme }) => ({ + "*:first-child": { + marginTop: 0, + }, + ...theme.typography.body1, + lineHeight: "1.5", + p: { + marginTop: "16px", + marginBottom: "0", + }, + "& > ul": { + listStyleType: "none", + marginTop: "16px", + marginBottom: 0, + padding: 0, + "> li": { + padding: "16px", + border: `1px solid ${theme.custom.colors.lightGray2}`, + borderBottom: "none", + ":first-of-type": { + borderRadius: "4px 4px 0 0", + }, + ":last-of-type": { + borderBottom: `1px solid ${theme.custom.colors.lightGray2}`, + borderRadius: "0 0 4px 4px", + }, + ":first-of-type:last-of-type": { + borderRadius: "4px", + }, + }, + }, + [theme.breakpoints.down("md")]: { + ...theme.typography.body2, + }, +})) + +export default RawHTML +export { UnstyledRawHTML } diff --git a/frontends/main/src/app-pages/ProductPages/util.ts b/frontends/main/src/app-pages/ProductPages/util.ts new file mode 100644 index 0000000000..8953669e51 --- /dev/null +++ b/frontends/main/src/app-pages/ProductPages/util.ts @@ -0,0 +1,9 @@ +enum HeadingIds { + About = "about", + What = "what-you-will-learn", + Prereqs = "prerequisites", + Instructors = "instructors", + WhoCanTake = "who-can-take", +} + +export { HeadingIds } diff --git a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx index d5d83d0e7f..c55b08dba4 100644 --- a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx @@ -8,6 +8,7 @@ import * as Sentry from "@sentry/nextjs" import { notFound } from "next/navigation" import { pagesQueries } from "api/mitxonline-hooks/pages" import { coursesQueries } from "api/mitxonline-hooks/courses" +import { DEFAULT_RESOURCE_IMG } from "ol-utilities" export const generateMetadata = async ( props: PageProps<"/courses/[readable_id]">, @@ -23,9 +24,12 @@ export const generateMetadata = async ( notFound() } const [course] = resp.data.items + const image = course.feature_image + ? course.course_details.page.feature_image_src + : DEFAULT_RESOURCE_IMG return standardizeMetadata({ title: course.title, - image: course.course_details.page.feature_image_src, + image, robots: "noindex, nofollow", }) } catch (error) { @@ -47,7 +51,7 @@ const Page: React.FC> = async (props) => { * This approach blocked by wagtail api requiring auth. */ const { dehydratedState } = await prefetch([ - pagesQueries.courseDetail(readableId), + pagesQueries.coursePages(readableId), coursesQueries.coursesList({ readable_id: readableId }), ]) return ( diff --git a/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx new file mode 100644 index 0000000000..3841afc920 --- /dev/null +++ b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx @@ -0,0 +1,66 @@ +import React from "react" +import { HydrationBoundary } from "@tanstack/react-query" +import { standardizeMetadata } from "@/common/metadata" +import { prefetch } from "api/ssr/prefetch" +import ProgramPage from "@/app-pages/ProductPages/ProgramPage" +import { pagesApi } from "api/mitxonline" +import * as Sentry from "@sentry/nextjs" +import { notFound } from "next/navigation" +import { pagesQueries } from "api/mitxonline-hooks/pages" +import { programsQueries } from "api/mitxonline-hooks/programs" +import { DEFAULT_RESOURCE_IMG } from "ol-utilities" + +export const generateMetadata = async ( + props: PageProps<"/programs/[readable_id]">, +) => { + const params = await props.params + + try { + const resp = await pagesApi.pagesfieldstypecmsProgramPageRetrieve({ + readable_id: decodeURIComponent(params.readable_id), + }) + + if (resp.data.items.length === 0) { + notFound() + } + const [program] = resp.data.items + + // Note: feature_image.src is relative to mitxonline root. + const image = program.feature_image + ? program.program_details.page.feature_image_src + : DEFAULT_RESOURCE_IMG + return standardizeMetadata({ + title: program.title, + image, + robots: "noindex, nofollow", + }) + } catch (error) { + Sentry.captureException(error) + console.error( + "Error fetching course page metadata for", + params.readable_id, + error, + ) + } +} + +const Page: React.FC> = async (props) => { + const params = await props.params + const readableId = decodeURIComponent(params.readable_id) + /** + * TODO: Consider removing react-query from this page + * fetching via client, and calling notFound() if data missing. + * This approach blocked by wagtail api requiring auth. + */ + const { dehydratedState } = await prefetch([ + pagesQueries.programPages(readableId), + programsQueries.programsList({ readable_id: readableId }), + ]) + return ( + + + + ) +} + +export default Page diff --git a/frontends/ol-utilities/src/hooks/useThrottle.tsx b/frontends/ol-utilities/src/hooks/useThrottle.tsx index 9641bffbae..72f2de2437 100644 --- a/frontends/ol-utilities/src/hooks/useThrottle.tsx +++ b/frontends/ol-utilities/src/hooks/useThrottle.tsx @@ -1,3 +1,5 @@ +"use client" + import { useCallback, useRef } from "react" export const useThrottle = void>( diff --git a/frontends/ol-utilities/src/hooks/useWindowDimensions.tsx b/frontends/ol-utilities/src/hooks/useWindowDimensions.tsx index 230f573749..00a4e841dd 100644 --- a/frontends/ol-utilities/src/hooks/useWindowDimensions.tsx +++ b/frontends/ol-utilities/src/hooks/useWindowDimensions.tsx @@ -1,3 +1,5 @@ +"use client" + import { useEffect, useState } from "react" export const useWindowDimensions = () => { diff --git a/frontends/ol-utilities/src/index.ts b/frontends/ol-utilities/src/index.ts index 15af895d64..30dc9c29af 100644 --- a/frontends/ol-utilities/src/index.ts +++ b/frontends/ol-utilities/src/index.ts @@ -1,5 +1,3 @@ -"use client" - // eslint-disable-next-line @typescript-eslint/triple-slash-reference /// diff --git a/frontends/ol-utilities/src/ssr/NoSSR.tsx b/frontends/ol-utilities/src/ssr/NoSSR.tsx index ca93c8596f..a6aa80d660 100644 --- a/frontends/ol-utilities/src/ssr/NoSSR.tsx +++ b/frontends/ol-utilities/src/ssr/NoSSR.tsx @@ -1,3 +1,5 @@ +"use client" + import React, { useState, useEffect, ReactNode } from "react" type NoSSRProps = { From 74906390da63e710f8a48a46cfdc4fdbc391aa99 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Mon, 6 Oct 2025 10:57:54 -0400 Subject: [PATCH 03/16] add "just in time" dialog (#2530) * update currentuser to use userme, add country listing and user me mutation hooks, and add the first draft of the just in time modal * use our select components * fix the tests * use "dialog" instead of "modal" * add tests and fix some existing ones by mocking the countries api * fix CI testing issue * fix select placeholder color * update API package and remove first name and last name from updates * use yup schema, errortext * convert mitxonline user queries to queryOptions * fix ts error * fix country display * fix enrollment * remove unncessary check * add tests --------- Co-authored-by: Chris Chudzicki --- frontends/api/package.json | 2 +- frontends/api/src/mitxonline/clients.ts | 3 + .../api/src/mitxonline/hooks/user/index.ts | 64 +++- .../mitxonline/test-utils/factories/pages.ts | 1 + .../api/src/mitxonline/test-utils/urls.ts | 13 +- frontends/api/src/test-utils/mockAxios.ts | 6 +- frontends/main/package.json | 3 +- .../B2BAttachPage/B2BAttachPage.test.tsx | 6 +- .../app-pages/B2BAttachPage/B2BAttachPage.tsx | 4 +- .../CoursewareDisplay/DashboardCard.test.tsx | 46 ++- .../CoursewareDisplay/DashboardCard.tsx | 34 ++- .../DashboardDialogs.test.tsx | 287 +++++++++++++++++- .../CoursewareDisplay/DashboardDialogs.tsx | 149 ++++++++- .../EnrollmentDisplay.test.tsx | 2 + .../CoursewareDisplay/test-utils.ts | 4 +- .../DashboardPage/DashboardLayout.test.tsx | 2 +- .../DashboardPage/DashboardLayout.tsx | 7 +- .../DashboardPage/HomeContent.test.tsx | 2 + .../DashboardPage/OrganizationContent.tsx | 4 +- .../LoadingSpinner/LoadingSpinner.tsx | 4 +- .../components/SimpleSelect/SimpleSelect.tsx | 2 + yarn.lock | 29 +- 22 files changed, 615 insertions(+), 59 deletions(-) diff --git a/frontends/api/package.json b/frontends/api/package.json index ff8b519ffe..c67ed7bbc0 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -30,7 +30,7 @@ "ol-test-utilities": "0.0.0" }, "dependencies": { - "@mitodl/mitxonline-api-axios": "^2025.9.26", + "@mitodl/mitxonline-api-axios": "^2025.9.30", "@tanstack/react-query": "^5.66.0", "axios": "^1.12.2" } diff --git a/frontends/api/src/mitxonline/clients.ts b/frontends/api/src/mitxonline/clients.ts index 3495c437f8..427f7012d6 100644 --- a/frontends/api/src/mitxonline/clients.ts +++ b/frontends/api/src/mitxonline/clients.ts @@ -9,6 +9,7 @@ import { UsersApi, ProgramEnrollmentsApi, PagesApi, + CountriesApi, } from "@mitodl/mitxonline-api-axios/v2" import axios from "axios" @@ -25,6 +26,7 @@ const BASE_PATH = process.env.NEXT_PUBLIC_MITX_ONLINE_BASE_URL?.replace(/\/+$/, "") ?? "" const usersApi = new UsersApi(undefined, BASE_PATH, axiosInstance) +const countriesApi = new CountriesApi(undefined, BASE_PATH, axiosInstance) const b2bApi = new B2bApi(undefined, BASE_PATH, axiosInstance) const programsApi = new ProgramsApi(undefined, BASE_PATH, axiosInstance) const programCollectionsApi = new ProgramCollectionsApi( @@ -63,6 +65,7 @@ const pagesApi = new PagesApi(undefined, BASE_PATH, axiosInstance) export { usersApi, + countriesApi, b2bApi, courseRunEnrollmentsApi, programEnrollmentsApi, diff --git a/frontends/api/src/mitxonline/hooks/user/index.ts b/frontends/api/src/mitxonline/hooks/user/index.ts index 1d3c919859..eabdf349a7 100644 --- a/frontends/api/src/mitxonline/hooks/user/index.ts +++ b/frontends/api/src/mitxonline/hooks/user/index.ts @@ -1,18 +1,58 @@ -import { useQuery } from "@tanstack/react-query" -import { usersApi } from "../../clients" +import { + queryOptions, + useMutation, + useQuery, + useQueryClient, +} from "@tanstack/react-query" +import { countriesApi, usersApi } from "../../clients" import type { User } from "@mitodl/mitxonline-api-axios/v2" +import { UsersApiUsersMePartialUpdateRequest } from "@mitodl/mitxonline-api-axios/v2" -const useMitxOnlineCurrentUser = (opts: { enabled?: boolean } = {}) => - useQuery({ - queryKey: ["mitxonline", "currentUser"], - queryFn: async (): Promise => { - const response = await usersApi.usersCurrentUserRetrieve() - return { - ...response.data, - } +const userKeys = { + root: ["mitxonline", "users"] as const, + me: () => [...userKeys.root, "me"] as const, + countries: () => ["mitxonline", "countries"] as const, +} + +const queries = { + me: () => + queryOptions({ + queryKey: userKeys.me(), + queryFn: async () => { + const response = await usersApi.usersMeRetrieve() + return response.data + }, + }), + countries: () => + queryOptions({ + queryKey: userKeys.countries(), + queryFn: async () => { + const response = await countriesApi.countriesList() + return response.data + }, + }), +} + +/** + * @deprecated Prefer direct use of queries.me() + */ +const useMitxOnlineUserMe = (opts: { enabled?: boolean } = {}) => + useQuery({ ...queries.me(), ...opts }) + +const useUpdateUserMutation = () => { + const queryClient = useQueryClient() + return useMutation({ + mutationFn: (opts: UsersApiUsersMePartialUpdateRequest) => + usersApi.usersMePartialUpdate(opts), + onSuccess: () => { + queryClient.invalidateQueries({ queryKey: userKeys.me() }) }, - ...opts, }) +} -export { useMitxOnlineCurrentUser } +export { + queries as mitxUserQueries, + useMitxOnlineUserMe, + useUpdateUserMutation, +} export type { User as MitxOnlineUser } diff --git a/frontends/api/src/mitxonline/test-utils/factories/pages.ts b/frontends/api/src/mitxonline/test-utils/factories/pages.ts index ff2322a1e2..5177f9aba2 100644 --- a/frontends/api/src/mitxonline/test-utils/factories/pages.ts +++ b/frontends/api/src/mitxonline/test-utils/factories/pages.ts @@ -244,6 +244,7 @@ const programPageItem: PartialFactory = (override) => { name: faker.person.fullName(), title_1: faker.person.jobTitle(), title_2: faker.person.jobTitle(), + title_3: "", organization: "Massachusetts Institute of Technology", signature_image: faker.image.urlLoremFlickr({ width: 200, diff --git a/frontends/api/src/mitxonline/test-utils/urls.ts b/frontends/api/src/mitxonline/test-utils/urls.ts index 9a54a99ab7..edeba636da 100644 --- a/frontends/api/src/mitxonline/test-utils/urls.ts +++ b/frontends/api/src/mitxonline/test-utils/urls.ts @@ -5,14 +5,16 @@ import type { ProgramCollectionsApiProgramCollectionsListRequest, ProgramsApiProgramsListV2Request, } from "@mitodl/mitxonline-api-axios/v2" -import { RawAxiosRequestConfig } from "axios" import { queryify } from "ol-test-utilities" const API_BASE_URL = process.env.NEXT_PUBLIC_MITX_ONLINE_BASE_URL -const currentUser = { - get: (opts?: RawAxiosRequestConfig) => - `${API_BASE_URL}/api/v0/users/current_user/${queryify(opts)}`, +const userMe = { + get: () => `${API_BASE_URL}/api/v0/users/me`, +} + +const countries = { + list: () => `${API_BASE_URL}/api/v0/countries/`, } const enrollment = { @@ -83,7 +85,8 @@ const certificates = { export { b2b, b2bAttach, - currentUser, + userMe, + countries, enrollment, programs, programCollections, diff --git a/frontends/api/src/test-utils/mockAxios.ts b/frontends/api/src/test-utils/mockAxios.ts index 2a4959bde9..3c284a32c8 100644 --- a/frontends/api/src/test-utils/mockAxios.ts +++ b/frontends/api/src/test-utils/mockAxios.ts @@ -14,9 +14,13 @@ type RequestMaker = ( body?: unknown, ) => Promise -const alwaysError: RequestMaker = (method, url, _body) => { +const alwaysError: RequestMaker = (method, url, body) => { const msg = `No response specified for ${method} ${url}` console.error(msg) + if (body) { + console.error("and body:") + console.error(body) + } throw new Error(msg) } diff --git a/frontends/main/package.json b/frontends/main/package.json index 5dd7d023b6..42fd14c563 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -14,7 +14,7 @@ "@emotion/cache": "^11.13.1", "@emotion/styled": "^11.11.0", "@mitodl/course-search-utils": "3.3.2", - "@mitodl/mitxonline-api-axios": "^2025.9.26", + "@mitodl/mitxonline-api-axios": "^2025.9.30", "@mitodl/smoot-design": "^6.17.1", "@next/bundle-analyzer": "^14.2.15", "@react-pdf/renderer": "^4.3.0", @@ -62,6 +62,7 @@ "next-router-mock": "^1.0.2", "ol-test-utilities": "0.0.0", "ts-jest": "^29.2.4", + "type-fest": "^5.0.1", "typescript": "^5" } } diff --git a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx b/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx index 184c891207..6732bb8e20 100644 --- a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx +++ b/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx @@ -30,7 +30,7 @@ describe("B2BAttachPage", () => { [Permission.Authenticated]: false, }) - setMockResponse.get(mitxOnlineUrls.currentUser.get(), null) + setMockResponse.get(mitxOnlineUrls.userMe.get(), null) setMockResponse.post(b2bUrls.b2bAttach.b2bAttachView("test-code"), []) renderWithProviders(, { @@ -58,7 +58,7 @@ describe("B2BAttachPage", () => { }) setMockResponse.get( - mitxOnlineUrls.currentUser.get(), + mitxOnlineUrls.userMe.get(), mitxOnlineFactories.user.user(), ) @@ -88,7 +88,7 @@ describe("B2BAttachPage", () => { [Permission.Authenticated]: true, }) - setMockResponse.get(mitxOnlineUrls.currentUser.get(), mitxOnlineUser) + setMockResponse.get(mitxOnlineUrls.userMe.get(), mitxOnlineUser) setMockResponse.post(b2bUrls.b2bAttach.b2bAttachView("test-code"), []) diff --git a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx b/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx index 195bb5d7b7..54d7339e2c 100644 --- a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx +++ b/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx @@ -3,7 +3,7 @@ import React from "react" import { styled, Breadcrumbs, Container, Typography } from "ol-components" import * as urls from "@/common/urls" import { useB2BAttachMutation } from "api/mitxonline-hooks/organizations" -import { useMitxOnlineCurrentUser } from "api/mitxonline-hooks/user" +import { useMitxOnlineUserMe } from "api/mitxonline-hooks/user" import { userQueries } from "api/hooks/user" import { useQuery } from "@tanstack/react-query" import { useRouter } from "next-nprogress-bar" @@ -31,7 +31,7 @@ const B2BAttachPage: React.FC = ({ code }) => { ...userQueries.me(), staleTime: 0, }) - const { data: mitxOnlineUser } = useMitxOnlineCurrentUser() + const { data: mitxOnlineUser } = useMitxOnlineUserMe() React.useEffect(() => { attach?.() diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx index 96a1dfef7f..d01b729fd8 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx @@ -5,15 +5,18 @@ import { setMockResponse, user, within, - expectWindowNavigation, } from "@/test-utils" import * as mitxonline from "api/mitxonline-test-utils" +import { + urls as testUrls, + factories as testFactories, + mockAxiosInstance, +} from "api/test-utils" import { DashboardCard, getDefaultContextMenuItems } from "./DashboardCard" import { dashboardCourse } from "./test-utils" import { faker } from "@faker-js/faker/locale/en" import moment from "moment" import { EnrollmentMode, EnrollmentStatus } from "./types" -import { mockAxiosInstance } from "api/test-utils" const pastDashboardCourse: typeof dashboardCourse = (...overrides) => { return dashboardCourse( @@ -49,6 +52,14 @@ const futureDashboardCourse: typeof dashboardCourse = (...overrides) => { ) } +beforeEach(() => { + // Mock user API call + const user = testFactories.user.user() + const mitxUser = mitxonline.factories.user.user() + setMockResponse.get(testUrls.userMe.get(), user) + setMockResponse.get(mitxonline.urls.userMe.get(), mitxUser) +}) + describe.each([ { display: "desktop", testId: "enrollment-card-desktop" }, { display: "mobile", testId: "enrollment-card-mobile" }, @@ -476,19 +487,44 @@ describe.each([ status: EnrollmentStatus.NotEnrolled, }, }) + + // Mock user without country and year_of_birth to trigger JustInTimeDialog + const baseUser = mitxonline.factories.user.user() + const mitxUserWithoutRequiredFields = { + ...baseUser, + legal_address: { ...baseUser.legal_address, country: undefined }, + user_profile: { ...baseUser.user_profile, year_of_birth: undefined }, + } + setMockResponse.get( + mitxonline.urls.userMe.get(), + mitxUserWithoutRequiredFields, + ) + setMockResponse.post( mitxonline.urls.b2b.courseEnrollment(course.coursewareId ?? undefined), { result: "b2b-enroll-success", order: 1 }, ) + // Mock countries data needed by JustInTimeDialog + setMockResponse.get(mitxonline.urls.countries.list(), [ + { code: "US", name: "United States" }, + { code: "CA", name: "Canada" }, + ]) + renderWithProviders() const card = getCard() const coursewareButton = within(card).getByTestId("courseware-button") - await expectWindowNavigation(async () => { - await user.click(coursewareButton) + // Now the button should show the JustInTimeDialog instead of directly enrolling + await user.click(coursewareButton) + + // Verify the JustInTimeDialog appeared + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", }) + expect(dialog).toBeInTheDocument() - expect(mockAxiosInstance.request).toHaveBeenCalledWith( + // The enrollment API should NOT be called yet (until dialog is completed) + expect(mockAxiosInstance.request).not.toHaveBeenCalledWith( expect.objectContaining({ method: "POST", url: mitxonline.urls.b2b.courseEnrollment( diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx index 6d75e14298..32da8e1df9 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx @@ -25,9 +25,14 @@ import { import { calendarDaysUntil, isInPast, NoSSR } from "ol-utilities" import { EnrollmentStatusIndicator } from "./EnrollmentStatusIndicator" -import { EmailSettingsDialog, UnenrollDialog } from "./DashboardDialogs" +import { + EmailSettingsDialog, + JustInTimeDialog, + UnenrollDialog, +} from "./DashboardDialogs" import NiceModal from "@ebay/nice-modal-react" import { useCreateEnrollment } from "api/mitxonline-hooks/enrollment" +import { useMitxOnlineUserMe } from "api/mitxonline-hooks/user" const CardRoot = styled.div<{ screenSize: "desktop" | "mobile" @@ -156,10 +161,14 @@ const CoursewareButton = styled( courseNoun, enrollmentStatus, }) + const mitxOnlineUser = useMitxOnlineUserMe() const hasStarted = startDate && isInPast(startDate) const hasEnrolled = enrollmentStatus && enrollmentStatus !== EnrollmentStatus.NotEnrolled const createEnrollment = useCreateEnrollment() + const userCountry = mitxOnlineUser.data?.legal_address?.country + const userYearOfBirth = mitxOnlineUser.data?.user_profile?.year_of_birth + const showJustInTimeDialog = !userCountry || !userYearOfBirth return (hasStarted && href) || !hasEnrolled ? ( hasEnrolled && href ? ( { - await createEnrollment.mutateAsync( - { readable_id: coursewareId ?? "" }, - { - onSuccess: () => { - if (href) { + if (!href || !coursewareId) return + if (showJustInTimeDialog) { + NiceModal.show(JustInTimeDialog, { + href: href, + readableId: coursewareId, + }) + return + } else { + await createEnrollment.mutateAsync( + { readable_id: coursewareId }, + { + onSuccess: () => { window.location.href = href - } + }, }, - }, - ) + ) + } }} {...others} > diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx index 699a1bf0ea..feae79ed00 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.test.tsx @@ -7,12 +7,21 @@ import { within, } from "@/test-utils" import { EnrollmentDisplay } from "./EnrollmentDisplay" +import { DashboardCard } from "./DashboardCard" +import { dashboardCourse, setupEnrollments } from "./test-utils" import * as mitxonline from "api/mitxonline-test-utils" +import { + urls as testUrls, + factories as testFactories, + mockAxiosInstance, +} from "api/test-utils" import { useFeatureFlagEnabled } from "posthog-js/react" -import { setupEnrollments } from "./test-utils" import { faker } from "@faker-js/faker/locale/en" -import { mockAxiosInstance } from "api/test-utils" import invariant from "tiny-invariant" +import { EnrollmentStatus } from "./types" +import { getDescriptionFor } from "ol-test-utilities" +import type { User as MitxUser } from "@mitodl/mitxonline-api-axios/v2" +import { PartialDeep } from "type-fest" jest.mock("posthog-js/react") const mockedUseFeatureFlagEnabled = jest @@ -21,6 +30,8 @@ const mockedUseFeatureFlagEnabled = jest describe("DashboardDialogs", () => { const setupApis = (includeExpired: boolean = true) => { + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) const { enrollments, completed, expired, started, notStarted } = setupEnrollments(includeExpired) @@ -130,3 +141,275 @@ describe("DashboardDialogs", () => { ) }) }) + +describe("JustInTimeDialog", () => { + const getFields = (root: HTMLElement) => { + return { + country: within(root).getByRole("combobox", { name: "Country" }), + year_of_birth: within(root).getByRole("combobox", { + name: "Year of Birth", + }), + } + } + + const originalLocation = window.location + + beforeAll(() => { + Object.defineProperty(window, "location", { + configurable: true, + enumerable: true, + value: { ...originalLocation, assign: jest.fn() }, + }) + }) + + afterAll(() => { + Object.defineProperty(window, "location", { + configurable: true, + enumerable: true, + value: originalLocation, + }) + }) + + type SetupJitOptions = { + userOverrides?: PartialDeep + } + + const setupJustInTimeTest = ({ + userOverrides = {}, + }: SetupJitOptions = {}) => { + // Setup MIT Learn user + const mitLearnUser = testFactories.user.user() + setMockResponse.get(testUrls.userMe.get(), mitLearnUser) + + // Setup incomplete mitxonline user (missing country and year_of_birth) + const incompleteMitxUser = mitxonline.factories.user.user({ + legal_address: null, + user_profile: null, + ...userOverrides, + }) + setMockResponse.get(mitxonline.urls.userMe.get(), incompleteMitxUser) + + // Setup countries data + const countries = [ + { code: "US", name: "United States" }, + { code: "CA", name: "Canada" }, + { code: "GB", name: "United Kingdom" }, + ] + setMockResponse.get(mitxonline.urls.countries.list(), countries) + + // Setup course for enrollment + const course = dashboardCourse({ + enrollment: { status: EnrollmentStatus.NotEnrolled }, + marketingUrl: "https://example.com/course", + }) + + // Setup enrollment API + setMockResponse.post( + mitxonline.urls.b2b.courseEnrollment(course.coursewareId || ""), + null, + ) + + return { mitLearnUser, incompleteMitxUser, countries, course } + } + + test("Opens just-in-time dialog when enrolling with incomplete mitxonline user data", async () => { + const { course } = setupJustInTimeTest() + + renderWithProviders() + + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + expect(dialog).toBeInTheDocument() + + expect( + within(dialog).getByText( + "We need a bit more info before you can enroll.", + ), + ).toBeInTheDocument() + const fields = getFields(dialog) + expect(fields.country).toBeVisible() + expect(fields.year_of_birth).toBeInTheDocument() + }) + + test.each([ + { + userOverrides: { legal_address: { country: "CA" } }, + expectCountry: "Canada", + expectYob: "Please Select", + }, + { + userOverrides: { user_profile: { year_of_birth: 1988 } }, + expectCountry: "Please Select", + expectYob: "1988", + }, + ])( + "Dialog pre-populates with user data if available", + async ({ userOverrides, expectCountry, expectYob }) => { + const { course } = setupJustInTimeTest({ userOverrides }) + renderWithProviders() + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + const fields = getFields(dialog) + expect(fields.country).toHaveTextContent(expectCountry) + expect(fields.year_of_birth).toHaveTextContent(expectYob) + }, + ) + + test("Validates required fields in just-in-time dialog", async () => { + const { course } = setupJustInTimeTest() + + renderWithProviders() + + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + + const submitButton = within(dialog).getByRole("button", { + name: "Submit", + }) + + // Try submitting with empty fields - should show validation errors + await user.click(submitButton) + + // Should show validation errors + const fields = getFields(dialog) + expect(fields.country).toBeInvalid() + expect(fields.year_of_birth).toBeInvalid() + expect(getDescriptionFor(fields.country)).toHaveTextContent( + "Country is required", + ) + expect(getDescriptionFor(fields.year_of_birth)).toHaveTextContent( + "Year of birth is required", + ) + }) + + test("Generates correct year of birth options (minimum age 13)", async () => { + const { course } = setupJustInTimeTest() + + renderWithProviders() + + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + const fields = getFields(dialog) + await user.click(fields.year_of_birth) + + const currentYear = new Date().getFullYear() + const maxYear = currentYear - 13 + const options = screen.getAllByRole("option") + const optionValues = options.map((opt) => opt.textContent) + const expectedYears = Array.from({ length: maxYear - 1900 + 1 }, (_, i) => + (maxYear - i).toString(), + ) + expect(expectedYears.length).toBeGreaterThan(50) // sanity + expect(optionValues).toEqual(["Please Select", ...expectedYears]) + }) + + test("Shows expected countries in country dropdown", async () => { + const { course, countries } = setupJustInTimeTest() + + renderWithProviders() + + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + const fields = getFields(dialog) + await user.click(fields.country) + + const options = screen.getAllByRole("option") + expect(countries.length).toBeGreaterThan(1) // sanity + const optionValues = options.map((opt) => opt.textContent) + expect(optionValues).toEqual([ + "Please Select", + ...countries.map((c) => c.name), + ]) + }) + + test("Cancels just-in-time dialog without making API calls", async () => { + const { course } = setupJustInTimeTest() + + renderWithProviders() + + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + + const cancelButton = within(dialog).getByRole("button", { + name: "Cancel", + }) + await user.click(cancelButton) + + // No PATCH calls should have been made + expect(mockAxiosInstance.request).not.toHaveBeenCalledWith( + expect.objectContaining({ + method: "PATCH", + url: mitxonline.urls.userMe.get(), + }), + ) + }) + + test("Submitting just-in-time dialog makes proper API calls", async () => { + const { course } = setupJustInTimeTest({ + userOverrides: { user_profile: { year_of_birth: 1988 } }, + }) + + renderWithProviders() + const enrollButtons = await screen.findAllByTestId("courseware-button") + await user.click(enrollButtons[0]) // Use the first (desktop) button + + const dialog = await screen.findByRole("dialog", { + name: "Just a Few More Details", + }) + const fields = getFields(dialog) + await user.click(fields.country) + + const option = screen.getByRole("option", { name: "Canada" }) + await user.click(option) // Select third option (first is "Please Select") + + invariant(course.coursewareId) + const spies = { + createEnrollment: jest.fn(), + patchUser: jest.fn(), + } + setMockResponse.patch(mitxonline.urls.userMe.get(), spies.patchUser, { + requestBody: { + user_profile: { year_of_birth: 1988 }, + legal_address: { country: "CA" }, + }, + }) + setMockResponse.post( + mitxonline.urls.b2b.courseEnrollment(course.coursewareId), + spies.createEnrollment, + ) + + const submitButton = within(dialog).getByRole("button", { + name: "Submit", + }) + + await user.click(submitButton) + await expect(spies.patchUser).toHaveBeenCalled() + await expect(spies.createEnrollment).toHaveBeenCalled() + expect(window.location.assign).toHaveBeenCalledWith( + course.run.coursewareUrl, + ) + }) +}) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.tsx index 42a2aa8aea..b0510a6aa8 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardDialogs.tsx @@ -6,16 +6,25 @@ import { DialogActions, Stack, LoadingSpinner, + SimpleSelectField, + SimpleSelectOption, } from "ol-components" import { Button, Checkbox, Alert } from "@mitodl/smoot-design" +import { useQuery } from "@tanstack/react-query" import NiceModal, { muiDialogV5 } from "@ebay/nice-modal-react" import { useFormik } from "formik" import { + useCreateEnrollment, useDestroyEnrollment, useUpdateEnrollment, } from "api/mitxonline-hooks/enrollment" import { DashboardCourseEnrollment } from "./types" +import { + mitxUserQueries, + useUpdateUserMutation, +} from "api/mitxonline-hooks/user" +import * as Yup from "yup" const BoldText = styled.span(({ theme }) => ({ ...theme.typography.subtitle1, @@ -25,6 +34,10 @@ const SpinnerContainer = styled.div({ marginLeft: "8px", }) +const SelectPlaceholder = styled("span")(({ theme }) => ({ + color: theme.custom.colors.silverGrayDark, +})) + type DashboardDialogProps = { title: string enrollment: DashboardCourseEnrollment @@ -80,6 +93,7 @@ const EmailSettingsDialogInner: React.FC = ({ {updateEnrollment.isPending && ( @@ -161,6 +175,7 @@ const UnenrollDialogInner: React.FC = ({ @@ -182,7 +197,139 @@ const UnenrollDialogInner: React.FC = ({ ) } +const jitSchema = Yup.object().shape({ + country: Yup.string().required("Country is required"), + year_of_birth: Yup.string().required("Year of birth is required"), +}) + +const JustInTimeDialogInner: React.FC<{ href: string; readableId: string }> = ({ + href, + readableId, +}) => { + const { data: countries } = useQuery(mitxUserQueries.countries()) + const updateUser = useUpdateUserMutation() + const createEnrollment = useCreateEnrollment() + const user = useQuery(mitxUserQueries.me()) + const modal = NiceModal.useModal() + + // Generate year options (minimum age 13, so current year - 13 back to 1900) + const currentYear = new Date().getFullYear() + const maxYear = currentYear - 13 + const years = Array.from({ length: maxYear - 1900 + 1 }, (_, i) => + (maxYear - i).toString(), + ) + + const yob = user?.data?.user_profile?.year_of_birth + + const formik = useFormik({ + enableReinitialize: true, + validateOnChange: false, + validateOnBlur: false, + initialValues: { + country: user?.data?.legal_address?.country || "", + year_of_birth: yob ? yob.toString() : "", + }, + validationSchema: jitSchema, + onSubmit: async (values) => { + await updateUser.mutateAsync({ + PatchedUserRequest: { + user_profile: { + year_of_birth: parseInt(values.year_of_birth, 10), + }, + legal_address: { + country: values.country, + }, + }, + }) + await createEnrollment.mutateAsync({ + readable_id: readableId, + }) + window.location.assign(href) + modal.hide() + }, + }) + const countryOptions: SimpleSelectOption[] = [ + { value: "", label: "Please Select", disabled: true }, + ...(countries?.map(({ code, name }) => ({ value: code, label: name })) ?? + []), + ] + const yobOptions: SimpleSelectOption[] = [ + { value: "", label: "Please Select", disabled: true }, + ...years.map((year): SimpleSelectOption => ({ value: year, label: year })), + ] + + return ( + + + + + } + > + + + We need a bit more info before you can enroll. + + + Please Select + } + error={!!formik.errors.country} + errorText={formik.errors.country} + /> + Please Select + } + error={!!formik.errors.year_of_birth} + errorText={formik.errors.year_of_birth} + /> + + + ) +} + const EmailSettingsDialog = NiceModal.create(EmailSettingsDialogInner) const UnenrollDialog = NiceModal.create(UnenrollDialogInner) +const JustInTimeDialog = NiceModal.create(JustInTimeDialogInner) -export { EmailSettingsDialog, UnenrollDialog } +export { EmailSettingsDialog, UnenrollDialog, JustInTimeDialog } diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx index 8bc13138f2..09c002b42f 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.test.tsx @@ -18,6 +18,8 @@ const mockedUseFeatureFlagEnabled = jest describe("EnrollmentDisplay", () => { const setupApis = (includeExpired: boolean = true) => { + const mitxOnlineUser = mitxonline.factories.user.user() + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) const { enrollments, completed, expired, started, notStarted } = setupEnrollments(includeExpired) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts index e97fe24e60..15fcbb8b29 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts @@ -122,7 +122,7 @@ const setupProgramsAndCourses = () => { const orgX = factories.organizations.organization({ name: "Org X" }) const mitxOnlineUser = factories.user.user({ b2b_organizations: [orgX] }) setMockResponse.get(u.urls.userMe.get(), user) - setMockResponse.get(urls.currentUser.get(), mitxOnlineUser) + setMockResponse.get(urls.userMe.get(), mitxOnlineUser) setMockResponse.get(urls.organization.organizationList(""), orgX) setMockResponse.get(urls.organization.organizationList(orgX.slug), orgX) @@ -244,7 +244,7 @@ function setupOrgDashboardMocks( ) { // Basic user and org setup setMockResponse.get(u.urls.userMe.get(), user) - setMockResponse.get(mitxonline.urls.currentUser.get(), mitxOnlineUser) + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) setMockResponse.get( mitxonline.urls.organization.organizationList(org.slug), org, diff --git a/frontends/main/src/app-pages/DashboardPage/DashboardLayout.test.tsx b/frontends/main/src/app-pages/DashboardPage/DashboardLayout.test.tsx index 2abcec8135..b40e367589 100644 --- a/frontends/main/src/app-pages/DashboardPage/DashboardLayout.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/DashboardLayout.test.tsx @@ -46,7 +46,7 @@ describe("DashboardLayout", () => { }) setMockResponse.get(urls.userMe.get(), user) - setMockResponse.get(mitxOnlineUrls.currentUser.get(), user) + setMockResponse.get(mitxOnlineUrls.userMe.get(), user) renderWithProviders( diff --git a/frontends/main/src/app-pages/DashboardPage/DashboardLayout.tsx b/frontends/main/src/app-pages/DashboardPage/DashboardLayout.tsx index 4fd32866d0..ec83d97d83 100644 --- a/frontends/main/src/app-pages/DashboardPage/DashboardLayout.tsx +++ b/frontends/main/src/app-pages/DashboardPage/DashboardLayout.tsx @@ -33,10 +33,7 @@ import { SETTINGS, } from "@/common/urls" import dynamic from "next/dynamic" -import { - useMitxOnlineCurrentUser, - MitxOnlineUser, -} from "api/mitxonline-hooks/user" +import { useMitxOnlineUserMe, MitxOnlineUser } from "api/mitxonline-hooks/user" import { useUserMe } from "api/hooks/user" import { useFeatureFlagEnabled } from "posthog-js/react" import { FeatureFlags } from "@/common/feature_flags" @@ -293,7 +290,7 @@ const DashboardPage: React.FC<{ const { isLoading: isLoadingUser, data: user } = useUserMe() const orgsEnabled = useFeatureFlagEnabled(FeatureFlags.OrganizationDashboard) const { isLoading: isLoadingMitxOnlineUser, data: mitxOnlineUser } = - useMitxOnlineCurrentUser({ enabled: !!orgsEnabled }) + useMitxOnlineUserMe({ enabled: !!orgsEnabled }) const tabData = useMemo( () => diff --git a/frontends/main/src/app-pages/DashboardPage/HomeContent.test.tsx b/frontends/main/src/app-pages/DashboardPage/HomeContent.test.tsx index 67031b8fc6..8362593685 100644 --- a/frontends/main/src/app-pages/DashboardPage/HomeContent.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/HomeContent.test.tsx @@ -75,6 +75,7 @@ describe("HomeContent", () => { }, }) invariant(user.profile) + const mitxOnlineUser = mitxonline.factories.user.user() invariant(user.profile) const courses = factories.learningResources.courses @@ -93,6 +94,7 @@ describe("HomeContent", () => { } setMockResponse.get(urls.userMe.get(), user) + setMockResponse.get(mitxonline.urls.userMe.get(), mitxOnlineUser) setMockResponse.get(urls.profileMe.get(), user.profile) // Set Top Picks Response diff --git a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx index d8f7105e58..7e11ab8af2 100644 --- a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx +++ b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx @@ -27,7 +27,7 @@ import { OrganizationPage, UserProgramEnrollmentDetail, } from "@mitodl/mitxonline-api-axios/v2" -import { useMitxOnlineCurrentUser } from "api/mitxonline-hooks/user" +import { useMitxOnlineUserMe } from "api/mitxonline-hooks/user" import { ButtonLink } from "@mitodl/smoot-design" import { RiAwardFill } from "@remixicon/react" @@ -433,7 +433,7 @@ const OrganizationContent: React.FC = ({ orgSlug, }) => { const { isLoading: isLoadingMitxOnlineUser, data: mitxOnlineUser } = - useMitxOnlineCurrentUser() + useMitxOnlineUserMe() const b2bOrganization = mitxOnlineUser?.b2b_organizations.find( (org) => org.slug.replace("org-", "") === orgSlug, ) diff --git a/frontends/ol-components/src/components/LoadingSpinner/LoadingSpinner.tsx b/frontends/ol-components/src/components/LoadingSpinner/LoadingSpinner.tsx index 4ddc547ef5..a6db555c23 100644 --- a/frontends/ol-components/src/components/LoadingSpinner/LoadingSpinner.tsx +++ b/frontends/ol-components/src/components/LoadingSpinner/LoadingSpinner.tsx @@ -17,6 +17,7 @@ type LoadingSpinnerProps = { loading: boolean size?: number | string "aria-label"?: string + color?: "primary" | "inherit" } const noDelay = { transitionDelay: "0ms" } @@ -25,11 +26,12 @@ const LoadingSpinner: React.FC = ({ loading, size, "aria-label": label = "Loading", + color, }) => { return ( - + ) diff --git a/frontends/ol-components/src/components/SimpleSelect/SimpleSelect.tsx b/frontends/ol-components/src/components/SimpleSelect/SimpleSelect.tsx index a3d9410ce7..117858120e 100644 --- a/frontends/ol-components/src/components/SimpleSelect/SimpleSelect.tsx +++ b/frontends/ol-components/src/components/SimpleSelect/SimpleSelect.tsx @@ -51,6 +51,7 @@ type SimpleSelectFieldProps = Pick< | "fullWidth" | "label" | "helpText" + | "error" | "errorText" | "required" | "size" @@ -58,6 +59,7 @@ type SimpleSelectFieldProps = Pick< | "onChange" | "name" | "className" + | "renderValue" > & { /** * The options for the dropdown diff --git a/yarn.lock b/yarn.lock index e6e69b9fba..694dcdb075 100644 --- a/yarn.lock +++ b/yarn.lock @@ -2978,13 +2978,13 @@ __metadata: languageName: node linkType: hard -"@mitodl/mitxonline-api-axios@npm:^2025.9.26": - version: 2025.9.26 - resolution: "@mitodl/mitxonline-api-axios@npm:2025.9.26" +"@mitodl/mitxonline-api-axios@npm:^2025.9.30": + version: 2025.9.30 + resolution: "@mitodl/mitxonline-api-axios@npm:2025.9.30" dependencies: "@types/node": "npm:^20.11.19" axios: "npm:^1.6.5" - checksum: 10/99b57605623b8a68d53b4273c7bfec3fca45adfb664329f4b34f4ce67bce7bf216d0875c8bdf7f284d27ee55625bc6a29ed3bc09294994b83413098394b0e15e + checksum: 10/6ad742353a4913721d4695e22addf99495884cb0c8e85211066fd88f3f6e6bdc858ca891f4f6bd93b6621dfc86847edbf9e85c0291f88d5bcf3a2169c1b3deb5 languageName: node linkType: hard @@ -7166,7 +7166,7 @@ __metadata: resolution: "api@workspace:frontends/api" dependencies: "@faker-js/faker": "npm:^9.9.0" - "@mitodl/mitxonline-api-axios": "npm:^2025.9.26" + "@mitodl/mitxonline-api-axios": "npm:^2025.9.30" "@tanstack/react-query": "npm:^5.66.0" "@testing-library/react": "npm:^16.3.0" axios: "npm:^1.12.2" @@ -13834,7 +13834,7 @@ __metadata: "@emotion/styled": "npm:^11.11.0" "@faker-js/faker": "npm:^9.9.0" "@mitodl/course-search-utils": "npm:3.3.2" - "@mitodl/mitxonline-api-axios": "npm:^2025.9.26" + "@mitodl/mitxonline-api-axios": "npm:^2025.9.30" "@mitodl/smoot-design": "npm:^6.17.1" "@next/bundle-analyzer": "npm:^14.2.15" "@react-pdf/renderer": "npm:^4.3.0" @@ -13878,6 +13878,7 @@ __metadata: slick-carousel: "npm:^1.8.1" tiny-invariant: "npm:^1.3.3" ts-jest: "npm:^29.2.4" + type-fest: "npm:^5.0.1" typescript: "npm:^5" yup: "npm:^1.4.0" languageName: unknown @@ -18858,6 +18859,13 @@ __metadata: languageName: node linkType: hard +"tagged-tag@npm:^1.0.0": + version: 1.0.0 + resolution: "tagged-tag@npm:1.0.0" + checksum: 10/e37653df3e495daa7ea7790cb161b810b00075bba2e4d6c93fb06a709e747e3ae9da11a120d0489833203926511b39e038a2affbd9d279cfb7a2f3fcccd30b5d + languageName: node + linkType: hard + "tapable@npm:^2.0.0, tapable@npm:^2.1.1, tapable@npm:^2.2.0, tapable@npm:^2.2.1": version: 2.2.1 resolution: "tapable@npm:2.2.1" @@ -19387,6 +19395,15 @@ __metadata: languageName: node linkType: hard +"type-fest@npm:^5.0.1": + version: 5.0.1 + resolution: "type-fest@npm:5.0.1" + dependencies: + tagged-tag: "npm:^1.0.0" + checksum: 10/5ec4def4ce82e6a33cf2e1a50f7ef512226fbe85314e402155aaedd70d4aa7ccea4224a72234d5351b1b4a730b36243d5b011c147e91795d2eee0dba291c6e51 + languageName: node + linkType: hard + "type-is@npm:~1.6.18": version: 1.6.18 resolution: "type-is@npm:1.6.18" From 4880222190f3681342344e6246ec90b182748ba8 Mon Sep 17 00:00:00 2001 From: Anastasia Beglova Date: Mon, 6 Oct 2025 16:40:11 -0400 Subject: [PATCH 04/16] csv problem files (#2560) --- learning_resources/constants.py | 2 ++ learning_resources/etl/canvas.py | 12 +++++++++--- learning_resources/etl/canvas_test.py | 25 +++++++++++++------------ learning_resources/etl/utils.py | 18 +++++++++++++----- learning_resources/etl/utils_test.py | 2 +- 5 files changed, 38 insertions(+), 21 deletions(-) diff --git a/learning_resources/constants.py b/learning_resources/constants.py index 58e52673d4..f23705ded6 100644 --- a/learning_resources/constants.py +++ b/learning_resources/constants.py @@ -158,6 +158,8 @@ class LearningResourceRelationTypes(TextChoices): ".xml", ] +VALID_TUTOR_PROBLEM_FILE_TYPES = [*VALID_TEXT_FILE_TYPES, ".csv"] + VALID_ALL_FILE_TYPES = list( zip( [*VALID_FILE_TYPES, *VALID_TEXT_FILE_TYPES], diff --git a/learning_resources/etl/canvas.py b/learning_resources/etl/canvas.py index 25860c2034..1a95959e88 100644 --- a/learning_resources/etl/canvas.py +++ b/learning_resources/etl/canvas.py @@ -15,6 +15,7 @@ from learning_resources.constants import ( TUTOR_PROBLEM_TYPE, TUTOR_SOLUTION_TYPE, + VALID_TUTOR_PROBLEM_FILE_TYPES, LearningResourceType, PlatformType, ) @@ -26,9 +27,9 @@ ) from learning_resources.etl.constants import ETLSource from learning_resources.etl.utils import ( - _process_olx_path, calc_checksum, get_edx_module_id, + process_olx_path, ) from learning_resources.models import ( LearningResource, @@ -162,7 +163,7 @@ def _generate_content(): else: log.debug("skipping unpublished file %s", member.filename) - for content_data in _process_olx_path(olx_path, run, overwrite=overwrite): + for content_data in process_olx_path(olx_path, run, overwrite=overwrite): url_path = content_data["source_path"].lstrip( content_data["source_path"].split("/")[0] ) @@ -207,7 +208,12 @@ def transform_canvas_problem_files( if member.filename.startswith(settings.CANVAS_TUTORBOT_FOLDER): course_archive.extract(member, path=olx_path) log.debug("processing active problem set file %s", member.filename) - for file_data in _process_olx_path(olx_path, run, overwrite=overwrite): + for file_data in process_olx_path( + olx_path, + run, + overwrite=overwrite, + valid_file_types=VALID_TUTOR_PROBLEM_FILE_TYPES, + ): keys_to_keep = [ "run", "content", diff --git a/learning_resources/etl/canvas_test.py b/learning_resources/etl/canvas_test.py index addff5affa..4a63745a8a 100644 --- a/learning_resources/etl/canvas_test.py +++ b/learning_resources/etl/canvas_test.py @@ -461,8 +461,9 @@ def test_transform_canvas_problem_files_pdf_calls_pdf_to_markdown( "source_path": f"tutorbot/{pdf_filename}", "file_extension": ".pdf", } + mocker.patch( - "learning_resources.etl.canvas._process_olx_path", + "learning_resources.etl.canvas.process_olx_path", return_value=iter([fake_file_data]), ) @@ -489,21 +490,21 @@ def test_transform_canvas_problem_files_non_pdf_does_not_call_pdf_to_markdown( """ settings.CANVAS_TUTORBOT_FOLDER = "tutorbot/" settings.CANVAS_PDF_TRANSCRIPTION_MODEL = "fake-model" - html_filename = "problemset2/problem.html" - html_content = b"problem" + csv_filename = "problemset2/problem.csv" + csv_content = "a,b,c\n1,2,3" zip_path = make_canvas_zip( - tmp_path, files=[(f"tutorbot/{html_filename}", html_content)] + tmp_path, files=[(f"tutorbot/{csv_filename}", csv_content)] ) fake_file_data = { "run": "run", - "content": "original html content", + "content": csv_content, "archive_checksum": "checksum", - "source_path": f"tutorbot/{html_filename}", - "file_extension": ".html", + "source_path": f"tutorbot/{csv_filename}", + "file_extension": ".csv", } mocker.patch( - "learning_resources.etl.canvas._process_olx_path", + "learning_resources.etl.canvas.process_olx_path", return_value=iter([fake_file_data]), ) @@ -514,7 +515,7 @@ def test_transform_canvas_problem_files_non_pdf_does_not_call_pdf_to_markdown( results = list(transform_canvas_problem_files(zip_path, run, overwrite=True)) pdf_to_md.assert_not_called() - assert results[0]["content"] == "original html content" + assert results[0]["content"] == csv_content assert results[0]["problem_title"] == "problemset2" @@ -526,12 +527,12 @@ def test_transform_canvas_content_files_url_assignment(mocker, tmp_path): run = MagicMock() run.id = 1 url_config = {"/folder/file1.html": {"url": "https://cdn.example.com/file1.html"}} - # Patch _process_olx_path to yield content_data with source_path + # Patch process_olx_path to yield content_data with source_path mock_content_data = [ {"source_path": "data/folder/file1.html", "key": "file1"}, ] mocker.patch( - "learning_resources.etl.canvas._process_olx_path", + "learning_resources.etl.canvas.process_olx_path", return_value=mock_content_data, ) mocker.patch( @@ -1570,7 +1571,7 @@ def test_transform_canvas_problem_files_skips_pdf_to_markdown_if_checksum_exists } mocker.patch( - "learning_resources.etl.canvas._process_olx_path", + "learning_resources.etl.canvas.process_olx_path", return_value=iter([fake_file_data]), ) diff --git a/learning_resources/etl/utils.py b/learning_resources/etl/utils.py index 2100b01387..e402b7a78f 100644 --- a/learning_resources/etl/utils.py +++ b/learning_resources/etl/utils.py @@ -313,7 +313,7 @@ def parse_dates(date_string, hour=12): def documents_from_olx( - olx_path: str, + olx_path: str, valid_file_types: list[str] = VALID_TEXT_FILE_TYPES ) -> Generator[tuple, None, None]: """ Extract text from OLX directory @@ -329,7 +329,7 @@ def documents_from_olx( for filename in files: extension_lower = Path(filename).suffix.lower() - if extension_lower in VALID_TEXT_FILE_TYPES and "draft" not in root: + if extension_lower in valid_file_types and "draft" not in root: with Path.open(Path(root, filename), "rb") as f: filebytes = f.read() @@ -529,9 +529,17 @@ def get_video_metadata(olx_path: str, run: LearningResourceRun) -> dict: return video_transcript_mapping -def _process_olx_path(olx_path: str, run: LearningResourceRun, *, overwrite): +def process_olx_path( + olx_path: str, + run: LearningResourceRun, + *, + overwrite, + valid_file_types=VALID_TEXT_FILE_TYPES, +) -> Generator[dict, None, None]: video_srt_metadata = get_video_metadata(olx_path, run) - for document, metadata in documents_from_olx(olx_path): + for document, metadata in documents_from_olx( + olx_path, valid_file_types=valid_file_types + ): source_path = metadata.get("source_path") edx_module_id = get_edx_module_id(source_path, run) key = edx_module_id @@ -611,7 +619,7 @@ def transform_content_files( with TemporaryDirectory(prefix=basedir) as inner_tempdir: check_call(["tar", "xf", course_tarpath], cwd=inner_tempdir) # noqa: S603,S607 olx_path = glob.glob(inner_tempdir + "/*")[0] # noqa: PTH207 - yield from _process_olx_path(olx_path, run, overwrite=overwrite) + yield from process_olx_path(olx_path, run, overwrite=overwrite) def get_learning_course_bucket_name(etl_source: str) -> str: diff --git a/learning_resources/etl/utils_test.py b/learning_resources/etl/utils_test.py index 52737c62e8..47b60b5f25 100644 --- a/learning_resources/etl/utils_test.py +++ b/learning_resources/etl/utils_test.py @@ -224,7 +224,7 @@ def test_transform_content_files( # noqa: PLR0913 "learning_resources.etl.utils.extract_text_metadata", return_value=tika_output ) - # Mock the new functions called by _process_olx_path + # Mock the new functions called by process_olx_path video_metadata = {"test": "video"} test_url = "https://example.com/test" From 28402ada8b8381be71a0393434892d6c691d7020 Mon Sep 17 00:00:00 2001 From: Anastasia Beglova Date: Mon, 6 Oct 2025 16:51:04 -0400 Subject: [PATCH 05/16] truncate csv files in tutor problems api (#2568) --- learning_resources/views.py | 27 +++++++++++++++++++-------- learning_resources/views_test.py | 21 ++++++++++++++++----- 2 files changed, 35 insertions(+), 13 deletions(-) diff --git a/learning_resources/views.py b/learning_resources/views.py index a6a6597c04..899c3914bc 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -1471,18 +1471,29 @@ def retrieve_problem(self, request, run_readable_id, problem_title): # noqa: AR return Response( { "problem_set_files": [ - { - "file_name": problem_file.file_name, - "content": problem_file.content, - } + problem_set_file_output(problem_file) for problem_file in problem_files ], "solution_set_files": [ - { - "file_name": solution_file.file_name, - "content": solution_file.content, - } + problem_set_file_output(solution_file) for solution_file in solution_files ], } ) + + +def problem_set_file_output(problem_set_file): + if problem_set_file.file_extension == ".csv": + csv_lines = problem_set_file.content.splitlines() + return { + "file_name": problem_set_file.file_name, + "file_extension": ".csv", + "truncated_content": "\n".join(csv_lines[0:5]), + "number_of_records": len(csv_lines) - 1, + "note": "The content of the data file has been truncated to the column headers and first 4 rows.", # noqa: E501 + } + return { + "file_name": problem_set_file.file_name, + "content": problem_set_file.content, + "file_extension": problem_set_file.file_extension, + } diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index 87ef7560d2..5ba2409108 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -1456,14 +1456,16 @@ def test_course_run_problems_endpoint(client, user_role, django_user_model): type="problem", content="Content for Problem Set 1", file_name="problem1.txt", + file_extension=".txt", ) TutorProblemFileFactory.create( run=course_run, problem_title="Problem Set 1", type="problem", - content="Content for Problem Set 1 Part 2", - file_name="problem1-b.txt", + content="a,b\n1,1\n2,2\n3,3\n4,4\n5,5\n6,6", + file_name="problem1-data.csv", + file_extension=".csv", ) TutorProblemFileFactory.create( @@ -1472,6 +1474,7 @@ def test_course_run_problems_endpoint(client, user_role, django_user_model): type="solution", content="Content for Problem Set 1 Solution", file_name="solution1.txt", + file_extension=".txt", ) TutorProblemFileFactory.create( run=course_run, problem_title="Problem Set 2", type="problem" @@ -1496,16 +1499,24 @@ def test_course_run_problems_endpoint(client, user_role, django_user_model): if user_role in ["admin", "group_tutor_problem_viewer"]: assert detail_resp.json() == { "problem_set_files": [ - {"file_name": "problem1.txt", "content": "Content for Problem Set 1"}, { - "file_name": "problem1-b.txt", - "content": "Content for Problem Set 1 Part 2", + "file_name": "problem1.txt", + "content": "Content for Problem Set 1", + "file_extension": ".txt", + }, + { + "file_name": "problem1-data.csv", + "truncated_content": "a,b\n1,1\n2,2\n3,3\n4,4", + "file_extension": ".csv", + "note": "The content of the data file has been truncated to the column headers and first 4 rows.", + "number_of_records": 6, }, ], "solution_set_files": [ { "file_name": "solution1.txt", "content": "Content for Problem Set 1 Solution", + "file_extension": ".txt", }, ], } From 9bcf3fa36a330c7f7d81d659d14f666c2ea4460b Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 7 Oct 2025 14:25:11 -0400 Subject: [PATCH 06/16] One-click enrollment via org dashboard card titles (#2569) * simplify nested ternaries to if/else * remove deprecated query * enroll on title click, too * add cartesian product * fix link, loaders --- .../CoursewareDisplay/DashboardCard.test.tsx | 234 +++++++++++++----- .../CoursewareDisplay/DashboardCard.tsx | 137 ++++++---- .../DashboardDialogs.test.tsx | 28 ++- .../CoursewareDisplay/DashboardDialogs.tsx | 42 ++-- .../CoursewareDisplay/EnrollmentDisplay.tsx | 2 + .../DashboardPage/OrganizationContent.tsx | 4 +- .../ol-test-utilities/src/cartesianProduct.ts | 56 +++++ .../src/cartestianProduct.test.ts | 38 +++ frontends/ol-test-utilities/src/index.ts | 1 + 9 files changed, 391 insertions(+), 151 deletions(-) create mode 100644 frontends/ol-test-utilities/src/cartesianProduct.ts create mode 100644 frontends/ol-test-utilities/src/cartestianProduct.test.ts diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx index d01b729fd8..1b8841fb19 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.test.tsx @@ -7,16 +7,13 @@ import { within, } from "@/test-utils" import * as mitxonline from "api/mitxonline-test-utils" -import { - urls as testUrls, - factories as testFactories, - mockAxiosInstance, -} from "api/test-utils" +import { mockAxiosInstance } from "api/test-utils" import { DashboardCard, getDefaultContextMenuItems } from "./DashboardCard" import { dashboardCourse } from "./test-utils" import { faker } from "@faker-js/faker/locale/en" import moment from "moment" import { EnrollmentMode, EnrollmentStatus } from "./types" +import { cartesianProduct } from "ol-test-utilities" const pastDashboardCourse: typeof dashboardCourse = (...overrides) => { return dashboardCourse( @@ -52,23 +49,45 @@ const futureDashboardCourse: typeof dashboardCourse = (...overrides) => { ) } -beforeEach(() => { - // Mock user API call - const user = testFactories.user.user() +const mitxUser = mitxonline.factories.user.user + +const setupUserApis = () => { const mitxUser = mitxonline.factories.user.user() - setMockResponse.get(testUrls.userMe.get(), user) setMockResponse.get(mitxonline.urls.userMe.get(), mitxUser) -}) +} describe.each([ { display: "desktop", testId: "enrollment-card-desktop" }, { display: "mobile", testId: "enrollment-card-mobile" }, -])("EnrollmentCard $display", ({ testId }) => { +])("DashboardCard $display", ({ testId }) => { const getCard = () => screen.getByTestId(testId) - test("It shows course title with link to marketing url", () => { - const course = dashboardCourse() - renderWithProviders() + const originalLocation = window.location + + beforeAll(() => { + Object.defineProperty(window, "location", { + configurable: true, + enumerable: true, + value: { ...originalLocation, assign: jest.fn() }, + }) + }) + + afterAll(() => { + Object.defineProperty(window, "location", { + configurable: true, + enumerable: true, + value: originalLocation, + }) + }) + + test("It shows course title and links to marketingUrl if titleAction is marketing", async () => { + setupUserApis() + const course = dashboardCourse({ + marketingUrl: "?some-marketing-url", + }) + renderWithProviders( + , + ) const card = getCard() @@ -78,7 +97,23 @@ describe.each([ expect(courseLink).toHaveAttribute("href", course.marketingUrl) }) + test("It shows course title and links to courseware if titleAction is courseware", async () => { + setupUserApis() + const course = dashboardCourse() + renderWithProviders( + , + ) + + const card = getCard() + + const courseLink = within(card).getByRole("link", { + name: course.title, + }) + expect(courseLink).toHaveAttribute("href", course.run.coursewareUrl) + }) + test("Accepts a classname", () => { + setupUserApis() const course = dashboardCourse() const TheComponent = faker.helpers.arrayElement([ "li", @@ -88,6 +123,7 @@ describe.each([ ]) renderWithProviders( { - renderWithProviders() + setupUserApis() + renderWithProviders( + , + ) const card = getCard() const coursewareCTA = within(card).getByTestId("courseware-button") @@ -154,8 +193,9 @@ describe.each([ ])( "Courseware CTA shows correct label based on courseNoun prop and dates (case $case)", ({ course, expected }) => { + setupUserApis() const { view } = renderWithProviders( - , + , ) const card = getCard() const coursewareCTA = within(card).getByTestId("courseware-button") @@ -173,7 +213,11 @@ describe.each([ const courseNoun = faker.word.noun() view.rerender( - , + , ) if ( @@ -221,8 +265,11 @@ describe.each([ ])( "Shows upgrade banner based on run.canUpgrade and not already upgraded (canUpgrade: $overrides.canUpgrade)", ({ overrides, expectation }) => { + setupUserApis() const course = dashboardCourse(overrides) - renderWithProviders() + renderWithProviders( + , + ) const card = getCard() const upgradeRoot = within(card).queryByTestId("upgrade-root") @@ -236,6 +283,7 @@ describe.each([ ])( "Never shows upgrade banner if `offerUpgrade` is false", ({ offerUpgrade, expected }) => { + setupUserApis() const course = dashboardCourse({ run: { canUpgrade: true, @@ -247,6 +295,7 @@ describe.each([ renderWithProviders( , @@ -259,6 +308,7 @@ describe.each([ ) test("Upgrade banner shows correct price and deadline", () => { + setupUserApis() const certificateUpgradePrice = faker.commerce.price() const certificateUpgradeDeadline = moment() .startOf("day") @@ -275,7 +325,9 @@ describe.each([ enrollment: { mode: EnrollmentMode.Audit }, }) - renderWithProviders() + renderWithProviders( + , + ) const card = getCard() const upgradeRoot = within(card).getByTestId("upgrade-root") @@ -287,13 +339,16 @@ describe.each([ }) test("Shows number of days until course starts", () => { + setupUserApis() const startDate = moment() .startOf("day") .add(5, "days") .add(3, "hours") .toISOString() const enrollment = dashboardCourse({ run: { startDate } }) - renderWithProviders() + renderWithProviders( + , + ) const card = getCard() expect(card).toHaveTextContent(/starts in 5 days/i) @@ -302,6 +357,7 @@ describe.each([ test.each([{ showNotComplete: true }, { showNotComplete: false }])( "Shows incomplete status when showNotComplete is true", ({ showNotComplete }) => { + setupUserApis() const enrollment = faker.helpers.arrayElement([ { status: EnrollmentStatus.NotEnrolled }, { status: EnrollmentStatus.Enrolled }, @@ -313,6 +369,7 @@ describe.each([ } const { view } = renderWithProviders( , @@ -324,6 +381,7 @@ describe.each([ view.rerender( { + setupUserApis() renderWithProviders( , ) @@ -389,6 +449,7 @@ describe.each([ ])( "getDefaultContextMenuItems returns correct items", async ({ contextMenuItems }) => { + setupUserApis() const course = dashboardCourse() course.enrollment = { id: faker.number.int(), @@ -397,6 +458,7 @@ describe.each([ } renderWithProviders( , @@ -431,8 +493,10 @@ describe.each([ ])( "Context menu button is not shown when enrollment status is not Completed or Enrolled", ({ status }) => { + setupUserApis() renderWithProviders( , ) @@ -457,13 +521,16 @@ describe.each([ ])( "CoursewareButton switches to Enroll functionality when enrollment status is not enrolled or undefined", ({ status }) => { + setupUserApis() const course = dashboardCourse() course.enrollment = { id: faker.number.int(), status: status, mode: EnrollmentMode.Audit, } - renderWithProviders() + renderWithProviders( + , + ) const card = getCard() const coursewareButton = within(card).getByTestId("courseware-button") @@ -479,58 +546,91 @@ describe.each([ }, ) - test("CoursewareButton hits enroll endpoint appropriately", async () => { - const course = dashboardCourse({ - coursewareId: faker.string.uuid(), - enrollment: { - id: faker.number.int(), - status: EnrollmentStatus.NotEnrolled, - }, - }) + const setupEnrollmentApis = (opts: { + user: ReturnType + course: ReturnType + }) => { + setMockResponse.get(mitxonline.urls.userMe.get(), opts.user) - // Mock user without country and year_of_birth to trigger JustInTimeDialog - const baseUser = mitxonline.factories.user.user() - const mitxUserWithoutRequiredFields = { - ...baseUser, - legal_address: { ...baseUser.legal_address, country: undefined }, - user_profile: { ...baseUser.user_profile, year_of_birth: undefined }, - } - setMockResponse.get( - mitxonline.urls.userMe.get(), - mitxUserWithoutRequiredFields, + const enrollmentUrl = mitxonline.urls.b2b.courseEnrollment( + opts.course.coursewareId ?? undefined, ) + setMockResponse.post(enrollmentUrl, { + result: "b2b-enroll-success", + order: 1, + }) - setMockResponse.post( - mitxonline.urls.b2b.courseEnrollment(course.coursewareId ?? undefined), - { result: "b2b-enroll-success", order: 1 }, - ) - // Mock countries data needed by JustInTimeDialog - setMockResponse.get(mitxonline.urls.countries.list(), [ + const countries = [ { code: "US", name: "United States" }, { code: "CA", name: "Canada" }, - ]) + ] + if (opts.user.legal_address?.country) { + countries.push({ + code: opts.user.legal_address.country, + name: "User's Country", + }) + } + // Mock countries data needed by JustInTimeDialog + setMockResponse.get(mitxonline.urls.countries.list(), countries) + return { enrollmentUrl } + } + + const ENROLLMENT_TRIGGERS = [ + { trigger: "button" as const }, + { trigger: "title-link" as const }, + ] + test.each(ENROLLMENT_TRIGGERS)( + "Enrollment for complete profile bypasses just-in-time dialog", + async ({ trigger }) => { + const userData = mitxUser() + const course = dashboardCourse({ + enrollment: { status: EnrollmentStatus.NotEnrolled }, + }) + const { enrollmentUrl } = setupEnrollmentApis({ user: userData, course }) + renderWithProviders( + , + ) + const card = getCard() + const triggerElement = + trigger === "button" + ? within(card).getByTestId("courseware-button") + : within(card).getByRole("link", { name: course.title }) - renderWithProviders() - const card = getCard() - const coursewareButton = within(card).getByTestId("courseware-button") + await user.click(triggerElement) - // Now the button should show the JustInTimeDialog instead of directly enrolling - await user.click(coursewareButton) + expect(mockAxiosInstance.request).toHaveBeenCalledWith( + expect.objectContaining({ method: "POST", url: enrollmentUrl }), + ) + }, + ) - // Verify the JustInTimeDialog appeared - const dialog = await screen.findByRole("dialog", { - name: "Just a Few More Details", - }) - expect(dialog).toBeInTheDocument() - - // The enrollment API should NOT be called yet (until dialog is completed) - expect(mockAxiosInstance.request).not.toHaveBeenCalledWith( - expect.objectContaining({ - method: "POST", - url: mitxonline.urls.b2b.courseEnrollment( - course.coursewareId ?? undefined, - ), - }), - ) - }) + test.each( + cartesianProduct(ENROLLMENT_TRIGGERS, [ + { userData: mitxUser({ legal_address: { country: "" } }) }, + { userData: mitxUser({ user_profile: { year_of_birth: null } }) }, + ]), + )( + "Enrollment for complete profile bypasses just-in-time dialog", + async ({ trigger, userData }) => { + const course = dashboardCourse({ + enrollment: { status: EnrollmentStatus.NotEnrolled }, + }) + setupEnrollmentApis({ user: userData, course }) + renderWithProviders( + , + ) + const card = getCard() + const triggerElement = + trigger === "button" + ? within(card).getByTestId("courseware-button") + : within(card).getByRole("link", { name: course.title }) + + await user.click(triggerElement) + + await screen.findByRole("dialog", { name: "Just a Few More Details" }) + expect(mockAxiosInstance.request).not.toHaveBeenCalledWith( + expect.objectContaining({ method: "POST" }), + ) + }, + ) }) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx index 32da8e1df9..1772f2f18d 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/DashboardCard.tsx @@ -32,7 +32,8 @@ import { } from "./DashboardDialogs" import NiceModal from "@ebay/nice-modal-react" import { useCreateEnrollment } from "api/mitxonline-hooks/enrollment" -import { useMitxOnlineUserMe } from "api/mitxonline-hooks/user" +import { mitxUserQueries } from "api/mitxonline-hooks/user" +import { useQuery } from "@tanstack/react-query" const CardRoot = styled.div<{ screenSize: "desktop" | "mobile" @@ -66,10 +67,6 @@ const CardRoot = styled.div<{ }, ]) -const SpinnerContainer = styled.div({ - marginLeft: "8px", -}) - const TitleLink = styled(Link)(({ theme }) => ({ [theme.breakpoints.down("md")]: { maxWidth: "calc(100% - 16px)", @@ -117,6 +114,40 @@ const getDefaultContextMenuItems = ( ] } +const useOneClickEnroll = () => { + const mitxOnlineUser = useQuery(mitxUserQueries.me()) + const createEnrollment = useCreateEnrollment() + const userCountry = mitxOnlineUser.data?.legal_address?.country + const userYearOfBirth = mitxOnlineUser.data?.user_profile?.year_of_birth + const showJustInTimeDialog = !userCountry || !userYearOfBirth + + const mutate = ({ + href, + coursewareId, + }: { + href: string + coursewareId: string + }) => { + if (showJustInTimeDialog) { + NiceModal.show(JustInTimeDialog, { + href: href, + readableId: coursewareId, + }) + return + } else { + createEnrollment.mutate( + { readable_id: coursewareId }, + { + onSuccess: () => { + window.location.assign(href) + }, + }, + ) + } + } + return { mutate, isPending: createEnrollment.isPending } +} + type CoursewareButtonProps = { coursewareId?: string | null startDate?: string | null @@ -161,62 +192,53 @@ const CoursewareButton = styled( courseNoun, enrollmentStatus, }) - const mitxOnlineUser = useMitxOnlineUserMe() const hasStarted = startDate && isInPast(startDate) const hasEnrolled = enrollmentStatus && enrollmentStatus !== EnrollmentStatus.NotEnrolled - const createEnrollment = useCreateEnrollment() - const userCountry = mitxOnlineUser.data?.legal_address?.country - const userYearOfBirth = mitxOnlineUser.data?.user_profile?.year_of_birth - const showJustInTimeDialog = !userCountry || !userYearOfBirth - return (hasStarted && href) || !hasEnrolled ? ( - hasEnrolled && href ? ( - } - href={href} className={className} + disabled={oneClickEnroll.isPending || !coursewareId} + onClick={() => { + if (!href || !coursewareId) return + oneClickEnroll.mutate({ href, coursewareId }) + }} + endIcon={ + oneClickEnroll.isPending ? ( + + ) : undefined + } {...others} > {coursewareText} - - ) : ( - + ) - ) : ( + } + // Disabled + return ( } @@ -169,17 +161,13 @@ const UnenrollDialogInner: React.FC = ({ variant="primary" type="submit" disabled={destroyEnrollment.isPending} + endIcon={ + destroyEnrollment.isPending ? ( + + ) : undefined + } > Unenroll - {destroyEnrollment.isPending && ( - - - - )} } @@ -275,13 +263,13 @@ const JustInTimeDialogInner: React.FC<{ href: string; readableId: string }> = ({ variant="primary" type="submit" disabled={formik.isSubmitting} + endIcon={ + formik.isSubmitting ? ( + + ) : undefined + } > Submit - {formik.isSubmitting && ( - - - - )} } diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx index a8b390335c..c5709ee8f8 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/EnrollmentDisplay.tsx @@ -139,6 +139,7 @@ const EnrollmentExpandCollapse: React.FC = ({ {shownEnrollments.map((course) => ( = ({ {hiddenEnrollments.map((course) => ( ))} @@ -326,7 +326,7 @@ const OrgProgramDisplay: React.FC<{ dashboardResource={course} courseNoun="Module" offerUpgrade={false} - titleHref={course.run?.coursewareUrl} + titleAction="courseware" buttonHref={course.run?.coursewareUrl} /> ))} diff --git a/frontends/ol-test-utilities/src/cartesianProduct.ts b/frontends/ol-test-utilities/src/cartesianProduct.ts new file mode 100644 index 0000000000..b4b189117e --- /dev/null +++ b/frontends/ol-test-utilities/src/cartesianProduct.ts @@ -0,0 +1,56 @@ +// Provide several overloads for better type inference with up to 5 arrays. +function cartesianProduct(a: A[], b: B[]): (A & B)[] +function cartesianProduct(a: A[], b: B[], c: C[]): (A & B & C)[] +function cartesianProduct( + a: A[], + b: B[], + c: C[], + d: D[], +): (A & B & C & D)[] +function cartesianProduct( + a: A[], + b: B[], + c: C[], + d: D[], + e: E[], +): (A & B & C & D & E)[] +function cartesianProduct(...arrays: T[][]): T[] + +/** + * Generates the cartesian product of multiple arrays of objects, merging the + * objects. This can be used with jest.each for an effect similar to multiple + * pytest.mark.parametrize calls. + * + * For example: + * ```ts + * cartesianProduct( + * [{ x: 1 }, { x: 2 }], + * [{ y: 'a' }, { y: 'b' }], + * [{ z: 3 }, { z: 4 }] + * ) + * ``` + * + * would yield: + * ``` + * [ + * { x: 1, y: 'a', z: 3 }, + * { x: 1, y: 'a', z: 4 }, + * { x: 1, y: 'b', z: 3 }, + * { x: 1, y: 'b', z: 4 }, + * { x: 2, y: 'a', z: 3 }, + * { x: 2, y: 'a', z: 4 }, + * { x: 2, y: 'b', z: 3 }, + * { x: 2, y: 'b', z: 4 } + * ] + * ``` + */ +function cartesianProduct(...arrays: T[][]): T[] { + return arrays.reduce( + (acc, curr) => { + return acc.flatMap((a) => curr.map((b) => ({ ...a, ...b }))) + }, + [{}] as T[], + ) +} + +export default cartesianProduct diff --git a/frontends/ol-test-utilities/src/cartestianProduct.test.ts b/frontends/ol-test-utilities/src/cartestianProduct.test.ts new file mode 100644 index 0000000000..3ba05d97f8 --- /dev/null +++ b/frontends/ol-test-utilities/src/cartestianProduct.test.ts @@ -0,0 +1,38 @@ +import cartesianProduct from "./cartesianProduct" + +describe("cartesianProduct", () => { + it("should return an empty array when given empty arrays", () => { + const result = cartesianProduct([], []) + expect(result).toEqual([]) + }) + + it("should handle single array", () => { + const result = cartesianProduct([{ a: 1 }, { a: 2 }]) + expect(result).toEqual([{ a: 1 }, { a: 2 }]) + }) + + it("should generate cartesian product of two arrays", () => { + const result = cartesianProduct( + [ + { a: 0, x: 10 }, + { a: 1, x: 20 }, + ], + [{ y: "a" }, { y: "b" }, { y: "c" }], + [{ z: true }, { z: false }], + ) + expect(result).toEqual([ + { a: 0, x: 10, y: "a", z: true }, + { a: 0, x: 10, y: "a", z: false }, + { a: 0, x: 10, y: "b", z: true }, + { a: 0, x: 10, y: "b", z: false }, + { a: 0, x: 10, y: "c", z: true }, + { a: 0, x: 10, y: "c", z: false }, + { a: 1, x: 20, y: "a", z: true }, + { a: 1, x: 20, y: "a", z: false }, + { a: 1, x: 20, y: "b", z: true }, + { a: 1, x: 20, y: "b", z: false }, + { a: 1, x: 20, y: "c", z: true }, + { a: 1, x: 20, y: "c", z: false }, + ]) + }) +}) diff --git a/frontends/ol-test-utilities/src/index.ts b/frontends/ol-test-utilities/src/index.ts index 4b28cc5aea..0570104320 100644 --- a/frontends/ol-test-utilities/src/index.ts +++ b/frontends/ol-test-utilities/src/index.ts @@ -6,6 +6,7 @@ export * from "./assertions" export * from "./domQueries/byImageSrc" export * from "./domQueries/byTerm" export * from "./domQueries/forms" +export { default as cartesianProduct } from "./cartesianProduct" /** * This is moment-timezone. From 1382330035e204b8cb716d8630eb4c3b44daa7da Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 7 Oct 2025 14:45:33 -0400 Subject: [PATCH 07/16] rename attach/[code] url to enrollmentcode/[code] (#2570) * rename attach/[code] url to enrollmentcode/[code] * comment, fix a url * use 308 * use permanent: true --- frontends/main/next.config.js | 10 ++++++++++ .../EnrollmentCodePage.test.tsx} | 10 +++++----- .../EnrollmentCodePage.tsx} | 6 +++--- .../src/app/{attach => enrollmentcode}/[code]/page.tsx | 8 +++++--- frontends/main/src/common/urls.ts | 2 +- 5 files changed, 24 insertions(+), 12 deletions(-) rename frontends/main/src/app-pages/{B2BAttachPage/B2BAttachPage.test.tsx => EnrollmentCodePage/EnrollmentCodePage.test.tsx} (90%) rename frontends/main/src/app-pages/{B2BAttachPage/B2BAttachPage.tsx => EnrollmentCodePage/EnrollmentCodePage.tsx} (93%) rename frontends/main/src/app/{attach => enrollmentcode}/[code]/page.tsx (60%) diff --git a/frontends/main/next.config.js b/frontends/main/next.config.js index 29bb4dff7a..5dce66095d 100644 --- a/frontends/main/next.config.js +++ b/frontends/main/next.config.js @@ -38,6 +38,16 @@ const nextConfig = { }, ] }, + async redirects() { + return [ + { + // can be removed once fastly redirect is in place + source: "/attach/:code", + destination: "/enrollmentcode/:code", + permanent: true, + }, + ] + }, async headers() { return [ diff --git a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx b/frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.test.tsx similarity index 90% rename from frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx rename to frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.test.tsx index 6732bb8e20..a81d6d7757 100644 --- a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.test.tsx +++ b/frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.test.tsx @@ -8,7 +8,7 @@ import { } from "api/mitxonline-test-utils" import * as commonUrls from "@/common/urls" import { Permission } from "api/hooks/user" -import B2BAttachPage from "./B2BAttachPage" +import EnrollmentCodePage from "./EnrollmentCodePage" import invariant from "tiny-invariant" // Mock next-nprogress-bar for App Router @@ -19,7 +19,7 @@ jest.mock("next-nprogress-bar", () => ({ }), })) -describe("B2BAttachPage", () => { +describe("EnrollmentCodePage", () => { beforeEach(() => { jest.clearAllMocks() mockPush.mockClear() @@ -33,7 +33,7 @@ describe("B2BAttachPage", () => { setMockResponse.get(mitxOnlineUrls.userMe.get(), null) setMockResponse.post(b2bUrls.b2bAttach.b2bAttachView("test-code"), []) - renderWithProviders(, { + renderWithProviders(, { url: commonUrls.B2B_ATTACH_VIEW, }) @@ -64,7 +64,7 @@ describe("B2BAttachPage", () => { setMockResponse.post(b2bUrls.b2bAttach.b2bAttachView("test-code"), []) - renderWithProviders(, { + renderWithProviders(, { url: commonUrls.B2B_ATTACH_VIEW, }) }) @@ -92,7 +92,7 @@ describe("B2BAttachPage", () => { setMockResponse.post(b2bUrls.b2bAttach.b2bAttachView("test-code"), []) - renderWithProviders(, { + renderWithProviders(, { url: commonUrls.B2B_ATTACH_VIEW, }) diff --git a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx b/frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.tsx similarity index 93% rename from frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx rename to frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.tsx index 54d7339e2c..2f95b44ff7 100644 --- a/frontends/main/src/app-pages/B2BAttachPage/B2BAttachPage.tsx +++ b/frontends/main/src/app-pages/EnrollmentCodePage/EnrollmentCodePage.tsx @@ -8,7 +8,7 @@ import { userQueries } from "api/hooks/user" import { useQuery } from "@tanstack/react-query" import { useRouter } from "next-nprogress-bar" -type B2BAttachPageProps = { +type EnrollmentCodePage = { code: string } @@ -17,7 +17,7 @@ const InterstitialMessage = styled(Typography)(({ theme }) => ({ textAlign: "center", })) -const B2BAttachPage: React.FC = ({ code }) => { +const EnrollmentCodePage: React.FC = ({ code }) => { const { mutate: attach, isSuccess, @@ -79,4 +79,4 @@ const B2BAttachPage: React.FC = ({ code }) => { ) } -export default B2BAttachPage +export default EnrollmentCodePage diff --git a/frontends/main/src/app/attach/[code]/page.tsx b/frontends/main/src/app/enrollmentcode/[code]/page.tsx similarity index 60% rename from frontends/main/src/app/attach/[code]/page.tsx rename to frontends/main/src/app/enrollmentcode/[code]/page.tsx index 023194dc92..175b606fc6 100644 --- a/frontends/main/src/app/attach/[code]/page.tsx +++ b/frontends/main/src/app/enrollmentcode/[code]/page.tsx @@ -2,16 +2,18 @@ import React from "react" import { Metadata } from "next" import { standardizeMetadata } from "@/common/metadata" import invariant from "tiny-invariant" -import B2BAttachPage from "@/app-pages/B2BAttachPage/B2BAttachPage" +import EnrollmentCodePage from "@/app-pages/EnrollmentCodePage/EnrollmentCodePage" export const metadata: Metadata = standardizeMetadata({ title: "Use Enrollment Code", }) -const Page: React.FC> = async ({ params }) => { +const Page: React.FC> = async ({ + params, +}) => { const resolved = await params invariant(resolved?.code, "code is required") - return + return } export default Page diff --git a/frontends/main/src/common/urls.ts b/frontends/main/src/common/urls.ts index dbe08c8832..c74d2c693b 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -189,7 +189,7 @@ export const auth = (opts: LoginUrlOpts) => { export const ECOMMERCE_CART = "/cart/" as const -export const B2B_ATTACH_VIEW = "/attach/[code]" +export const B2B_ATTACH_VIEW = "/enrollmentcode/[code]" export const b2bAttachView = (code: string) => generatePath(B2B_ATTACH_VIEW, { code: code }) From 88fd6565ed06aed01a31216b05c65bfe62da9e86 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Tue, 7 Oct 2025 14:46:50 -0400 Subject: [PATCH 08/16] Sort runs by start_date (#2567) --- learning_resources/factories.py | 6 +++-- learning_resources/models.py | 6 ++--- learning_resources/views.py | 6 ++--- learning_resources/views_test.py | 40 +++++++++++++++++++++++++------- 4 files changed, 41 insertions(+), 17 deletions(-) diff --git a/learning_resources/factories.py b/learning_resources/factories.py index b7131329de..5625306eb6 100644 --- a/learning_resources/factories.py +++ b/learning_resources/factories.py @@ -512,10 +512,12 @@ class LearningResourceRunFactory(DjangoModelFactory): ) ) start_date = factory.LazyAttribute( - lambda obj: obj.enrollment_start + timedelta(days=15) + lambda obj: obj.enrollment_start + timedelta(days=random.randint(15, 20)) # noqa: S311 ) end_date = factory.LazyAttribute( - lambda obj: obj.start_date + timedelta(days=90) if obj.start_date else None + lambda obj: obj.start_date + timedelta(days=random.randint(90, 100)) # noqa: S311 + if obj.start_date + else None ) prices = sorted( [ diff --git a/learning_resources/models.py b/learning_resources/models.py index 1dcc1e3f07..91b269d786 100644 --- a/learning_resources/models.py +++ b/learning_resources/models.py @@ -362,9 +362,9 @@ def for_serialization(self, *, user: Optional["User"] = None): "content_tags", Prefetch( "runs", - queryset=LearningResourceRun.objects.filter( - published=True - ).for_serialization(), + queryset=LearningResourceRun.objects.filter(published=True) + .order_by("start_date", "enrollment_start", "id") + .for_serialization(), ), Prefetch( "parents", diff --git a/learning_resources/views.py b/learning_resources/views.py index 899c3914bc..0812f0b15e 100644 --- a/learning_resources/views.py +++ b/learning_resources/views.py @@ -561,9 +561,9 @@ class ResourceListItemsViewSet(NestedViewSetMixin, viewsets.ReadOnlyModelViewSet ), Prefetch( "child__runs", - queryset=LearningResourceRun.objects.filter( - published=True - ).for_serialization(), + queryset=LearningResourceRun.objects.filter(published=True) + .order_by("start_date", "enrollment_start", "id") + .for_serialization(), ), "child__runs__instructors", "child__runs__resource_prices", diff --git a/learning_resources/views_test.py b/learning_resources/views_test.py index 5ba2409108..fc6ffb169b 100644 --- a/learning_resources/views_test.py +++ b/learning_resources/views_test.py @@ -80,8 +80,14 @@ def test_list_course_endpoint(client, url, params): resp = client.get(f"{reverse(url)}?{params}") assert resp.data.get("count") == 2 - for idx, course in enumerate(courses): - assert resp.data.get("results")[idx]["id"] == course.learning_resource.id + for course in resp.data.get("results"): + assert course.get("id") in [c.learning_resource.id for c in courses] + assert len(course.get("runs")) > 1 + for i in range(1, len(course.get("runs"))): + assert ( + course.get("runs")[i]["start_date"] + >= course.get("runs")[i - 1]["start_date"] + ) @pytest.mark.parametrize( @@ -94,6 +100,17 @@ def test_get_course_detail_endpoint(client, url): resp = client.get(reverse(url, args=[course.learning_resource.id])) assert resp.data.get("readable_id") == course.learning_resource.readable_id + assert len(resp.data.get("runs")) > 1 + assert [run["id"] for run in resp.data.get("runs")] == list( + course.learning_resource.runs.values_list("id", flat=True).order_by( + "start_date" + ) + ) + for i in range(1, len(resp.data.get("runs"))): + assert ( + resp.data.get("runs")[i]["start_date"] + >= resp.data.get("runs")[i - 1]["start_date"] + ) @pytest.mark.parametrize( @@ -1550,7 +1567,8 @@ def test_resource_items_only_shows_published_runs(client, user): [course], through_defaults={"relation_type": "program_courses"} ) - published_run = LearningResourceRunFactory.create( + published_runs = LearningResourceRunFactory.create_batch( + 4, published=True, learning_resource=course, ) @@ -1560,11 +1578,11 @@ def test_resource_items_only_shows_published_runs(client, user): ) course.runs.set( [ - published_run, + *published_runs, unpublished_run, ] ) - assert course.runs.count() == 2 + assert course.runs.count() == 5 client.force_login(user) url = reverse( @@ -1577,7 +1595,11 @@ def test_resource_items_only_shows_published_runs(client, user): assert len(results) == 1 child_data = results[0]["resource"] assert child_data["id"] == course.id - run_ids = [run["id"] for run in child_data["runs"]] - assert published_run.id in run_ids - assert unpublished_run.id not in run_ids - assert len(child_data["runs"]) == 1 + for idx, run in enumerate(child_data["runs"]): + assert run["id"] in [run.id for run in published_runs] + if idx > 0: + assert ( + child_data["runs"][idx]["start_date"] + >= child_data["runs"][idx - 1]["start_date"] + ) + assert len(child_data["runs"]) == 4 From 4118408620857bf165845837740553255b748393 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 8 Oct 2025 10:30:23 -0400 Subject: [PATCH 09/16] Expose version of frontend (#2575) * expose version * add default version --- env/frontend.env | 1 + frontends/main/src/app/healthcheck/route.ts | 10 +++++++++- frontends/main/src/app/layout.tsx | 4 ++++ 3 files changed, 14 insertions(+), 1 deletion(-) diff --git a/env/frontend.env b/env/frontend.env index 0225b9a761..0033b47629 100644 --- a/env/frontend.env +++ b/env/frontend.env @@ -25,3 +25,4 @@ NEXT_PUBLIC_LEARN_AI_RECOMMENDATION_ENDPOINT=https://api-learn-ai-qa.ol.mit.edu/ NEXT_PUBLIC_LEARN_AI_SYLLABUS_ENDPOINT=https://api-learn-ai-qa.ol.mit.edu/http/syllabus_agent/ NEXT_PUBLIC_MITX_ONLINE_BASE_URL=${MITX_ONLINE_BASE_URL} +NEXT_PUBLIC_VERSION="local-dev" diff --git a/frontends/main/src/app/healthcheck/route.ts b/frontends/main/src/app/healthcheck/route.ts index 42a44a4249..2eb9277042 100644 --- a/frontends/main/src/app/healthcheck/route.ts +++ b/frontends/main/src/app/healthcheck/route.ts @@ -1,3 +1,11 @@ +const VERSION = process.env.NEXT_PUBLIC_VERSION || "unknown" export async function GET() { - return Response.json({ status: "ok" }, { status: 200 }) + return Response.json( + { + status: "ok", + version: VERSION, + timestamp: new Date().toISOString(), // easily tell if response is cached + }, + { status: 200 }, + ) } diff --git a/frontends/main/src/app/layout.tsx b/frontends/main/src/app/layout.tsx index 9b4c7d2ef6..435709331b 100644 --- a/frontends/main/src/app/layout.tsx +++ b/frontends/main/src/app/layout.tsx @@ -35,6 +35,10 @@ export default function RootLayout({ rel="stylesheet" href="https://use.typekit.net/lbk1xay.css" > + From 7196cd31a6f9e7869d59da92a7db6719426b95d0 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 8 Oct 2025 10:32:11 -0400 Subject: [PATCH 10/16] chore(deps): update redis docker tag to v8.2.2 (#2566) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- docker-compose.services.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index a27dfd116d..55b92647aa 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -23,7 +23,7 @@ jobs: - 5432:5432 redis: - image: redis:8.2.1 + image: redis:8.2.2 ports: - 6379:6379 diff --git a/docker-compose.services.yml b/docker-compose.services.yml index 30e3b24d02..621597a5dd 100644 --- a/docker-compose.services.yml +++ b/docker-compose.services.yml @@ -25,7 +25,7 @@ services: redis: profiles: - backend - image: redis:8.2.1 + image: redis:8.2.2 healthcheck: test: ["CMD", "redis-cli", "ping", "|", "grep", "PONG"] interval: 3s From c152c6cef549dafdc9df4b30fd4c9c7e1d600e45 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 8 Oct 2025 10:53:06 -0400 Subject: [PATCH 11/16] chore(deps): update actions/setup-node action to v5 (#2576) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 8 ++++---- .github/workflows/publish-pages.yml | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 55b92647aa..8c3490b5d1 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -93,7 +93,7 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: "^22.0.0" cache: yarn @@ -201,7 +201,7 @@ jobs: - name: Checkout uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: "^22" cache: yarn @@ -224,7 +224,7 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: "^22.0.0" cache: yarn @@ -263,7 +263,7 @@ jobs: runs-on: ubuntu-22.04 steps: - uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: "^22.0.0" cache: yarn diff --git a/.github/workflows/publish-pages.yml b/.github/workflows/publish-pages.yml index 1de00b537f..4d18b976d3 100644 --- a/.github/workflows/publish-pages.yml +++ b/.github/workflows/publish-pages.yml @@ -15,7 +15,7 @@ jobs: - name: Checkout uses: actions/checkout@08c6903cd8c0fde910a37f88322edcfb5dd907a8 # v5 - - uses: actions/setup-node@49933ea5288caeca8642d1e84afbd3f7d6820020 # v4 + - uses: actions/setup-node@a0853c24544627f65ddf259abe73b1d18a591444 # v5 with: node-version: "^22.0.0" cache: yarn From 90c999668e145539eaa21f05d774df833bafeab1 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 8 Oct 2025 11:06:38 -0400 Subject: [PATCH 12/16] chore(deps): update actions/setup-python action to v6 (#2577) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 8c3490b5d1..bc2bbd0607 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -43,7 +43,7 @@ jobs: virtualenvs-create: true virtualenvs-in-project: true - - uses: actions/setup-python@a26af69be951a213d495a4c3e4e4022e16d87065 # v5 + - uses: actions/setup-python@e797f83bcb11b83ae66e0230d6156d7c80228e7c # v6 with: python-version-file: "pyproject.toml" cache: "poetry" From 8413c1b75642681ffa47f0c0a091c93cd3e32c26 Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 8 Oct 2025 11:18:32 -0400 Subject: [PATCH 13/16] chore(deps): update nginx docker tag to v1.29.2 (#2579) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- nginx/Dockerfile | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/nginx/Dockerfile b/nginx/Dockerfile index 62113e4f93..b944ee70be 100644 --- a/nginx/Dockerfile +++ b/nginx/Dockerfile @@ -3,7 +3,7 @@ # it's primary purpose is to emulate heroku-buildpack-nginx's # functionality that compiles config/nginx.conf.erb # See https://github.com/heroku/heroku-buildpack-nginx/blob/fefac6c569f28182b3459cb8e34b8ccafc403fde/bin/start-nginx -FROM nginx:1.29.1 +FROM nginx:1.29.2 # Logs are configured to a relatic path under /etc/nginx # but the container expects /var/log From 66c1375db7fb69b83d3714e0b62f6ad74ac8df7b Mon Sep 17 00:00:00 2001 From: "renovate[bot]" <29139614+renovate[bot]@users.noreply.github.com> Date: Wed, 8 Oct 2025 11:33:50 -0400 Subject: [PATCH 14/16] chore(deps): update codecov/codecov-action action to v5.5.1 (#2580) Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com> --- .github/workflows/ci.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bc2bbd0607..e83b93e8d5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -85,7 +85,7 @@ jobs: MITOL_COOKIE_NAME: cookie_monster - name: Upload coverage to CodeCov - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 with: file: ./coverage.xml @@ -142,7 +142,7 @@ jobs: NODE_ENV: test - name: Upload coverage to CodeCov - uses: codecov/codecov-action@18283e04ce6e62d37312384ff67231eb8fd56d24 # v5.4.3 + uses: codecov/codecov-action@5a1091511ad55cbe89839c7260b706298ca349f7 # v5.5.1 with: file: coverage/lcov.info From cd79ef839011d59767b4b11faf6c5f78581b5412 Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Wed, 8 Oct 2025 12:51:19 -0400 Subject: [PATCH 15/16] Do not overwrite existing canvas content (#2581) --- webhooks/views.py | 2 +- webhooks/views_test.py | 22 ++++++++++++++++++++++ 2 files changed, 23 insertions(+), 1 deletion(-) diff --git a/webhooks/views.py b/webhooks/views.py index bedf2f0bb6..d289e75f16 100644 --- a/webhooks/views.py +++ b/webhooks/views.py @@ -108,7 +108,7 @@ def process_create_content_file_request(data): content_path = data.get("content_path") if etl_source == ETLSource.canvas.name: log.info("Processing Canvas content file: %s", content_path) - ingest_canvas_course.apply_async([content_path, True]) + ingest_canvas_course.apply_async([content_path, False]) def process_delete_content_file_request(data): diff --git a/webhooks/views_test.py b/webhooks/views_test.py index df5ad15853..86ddceb348 100644 --- a/webhooks/views_test.py +++ b/webhooks/views_test.py @@ -75,3 +75,25 @@ def test_content_file_delete_webhook_view_canvas_resource_not_found( assert response.status_code == 200 assert mock_log.warning.called assert "does not exist" in mock_log.warning.call_args[0][0] + + +@pytest.mark.django_db +def test_content_file_webhook_view_canvas_success(settings, client, mocker): + """ + Test ContentFileWebhookView processes Canvas create webhook successfully + """ + url = reverse("webhooks:v1:content_file_webhook") + mock_ingest = mocker.patch("webhooks.views.ingest_canvas_course.apply_async") + + data = { + "source": ETLSource.canvas.name, + "content_path": "/path/to/canvas/course.tar.gz", + } + response = client.post( + url, + data=json.dumps(data), + content_type="application/json", + headers={"X-MITLearn-Signature": get_secret(data, settings)}, + ) + assert response.status_code == 200 + mock_ingest.assert_called_once_with(["/path/to/canvas/course.tar.gz", False]) From 2e0f1bd270e3b26c45300647fd7bdb45c5bf5677 Mon Sep 17 00:00:00 2001 From: Doof Date: Wed, 8 Oct 2025 17:11:17 +0000 Subject: [PATCH 16/16] Release 0.45.7 --- RELEASE.rst | 19 +++++++++++++++++++ main/settings.py | 2 +- 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 40d23981e7..cd90cac016 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,25 @@ Release Notes ============= +Version 0.45.7 +-------------- + +- Do not overwrite existing canvas content (#2581) +- chore(deps): update codecov/codecov-action action to v5.5.1 (#2580) +- chore(deps): update nginx docker tag to v1.29.2 (#2579) +- chore(deps): update actions/setup-python action to v6 (#2577) +- chore(deps): update actions/setup-node action to v5 (#2576) +- chore(deps): update redis docker tag to v8.2.2 (#2566) +- Expose version of frontend (#2575) +- Sort runs by start_date (#2567) +- rename attach/[code] url to enrollmentcode/[code] (#2570) +- One-click enrollment via org dashboard card titles (#2569) +- truncate csv files in tutor problems api (#2568) +- csv problem files (#2560) +- add "just in time" dialog (#2530) +- Program Page basic layout (#2556) +- Fix qdrant payload schema type and add mechanism to keep types in sync (#2553) + Version 0.45.4 (Released October 02, 2025) -------------- diff --git a/main/settings.py b/main/settings.py index 250d042d84..c767607372 100644 --- a/main/settings.py +++ b/main/settings.py @@ -34,7 +34,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.45.4" +VERSION = "0.45.7" log = logging.getLogger()