diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04624b3200..ccb7b5058c 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -98,24 +98,6 @@ jobs: with: file: ./coverage.xml - build-nextjs-frontend: - runs-on: ubuntu-latest - steps: - - name: Checkout - uses: actions/checkout@692973e3d937129bcbf40652eb9f2f61becf3332 # v4 - - - uses: actions/setup-node@1e60f620b9541d16bece96c5465dc8ee9832be0b # v4 - with: - node-version: "^20" - cache: yarn - cache-dependency-path: yarn.lock - - - name: Install dependencies - run: yarn install - - - name: Build Next.js frontend - run: yarn workspace main build - javascript-tests: runs-on: ubuntu-latest steps: @@ -144,8 +126,11 @@ jobs: - name: Lints run: yarn run lint-check + - name: Build Next.js frontend + run: yarn workspace main build + # do this before typecheck. See https://github.com/vercel/next.js/issues/53959#issuecomment-1735563224 + - name: Typecheck - if: false # To be reinstated. Anything actually used in `main` is typecheck during build run: yarn run typecheck - name: Get number of CPU cores diff --git a/frontends/api/package.json b/frontends/api/package.json index 7062b5d08e..7abfd01313 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -5,7 +5,9 @@ "sideEffects": false, "exports": { ".": "./src/generated/v1/api.ts", + "./clients": "./src/clients.ts", "./v0": "./src/generated/v0/api.ts", + "./v1": "./src/generated/v1/api.ts", "./hooks/*": "./src/hooks/*/index.ts", "./constants": "./src/common/constants.ts", "./test-utils/factories": "./src/test-utils/factories/index.ts", diff --git a/frontends/api/src/test-utils/factories/learningResources.ts b/frontends/api/src/test-utils/factories/learningResources.ts index 7de5efb2e0..82bb5015f9 100644 --- a/frontends/api/src/test-utils/factories/learningResources.ts +++ b/frontends/api/src/test-utils/factories/learningResources.ts @@ -196,7 +196,7 @@ const learningResourceTopic: Factory = ( const topic: LearningResourceTopic = { id: uniqueEnforcerId.enforce(() => faker.number.int()), name: uniqueEnforcerWords.enforce(() => faker.lorem.words()), - channel_url: `${faker.internet.url()}${faker.system.directoryPath()}`, + channel_url: `${faker.internet.url({ appendSlash: false })}${faker.system.directoryPath()}`, parent: null, ...overrides, } diff --git a/frontends/jest-shared-setup.ts b/frontends/jest-shared-setup.ts index 3ec45641aa..76e8c247db 100644 --- a/frontends/jest-shared-setup.ts +++ b/frontends/jest-shared-setup.ts @@ -10,6 +10,8 @@ expect.extend(matchers) // env vars process.env.NEXT_PUBLIC_MITOL_API_BASE_URL = "http://api.test.learn.odl.local:8063" +process.env.NEXT_PUBLIC_ORIGIN = "http://test.learn.odl.local:8062" +process.env.NEXT_PUBLIC_EMBEDLY_KEY = "fake-embedly-key" // Pulled from the docs - see https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom diff --git a/frontends/main/jest.config.ts b/frontends/main/jest.config.ts new file mode 100644 index 0000000000..f4a9e2d8cf --- /dev/null +++ b/frontends/main/jest.config.ts @@ -0,0 +1,17 @@ +import path from "path" +import type { Config } from "@jest/types" +import baseConfig from "../jest.jsdom.config" + +const config: Config.InitialOptions = { + ...baseConfig, + setupFilesAfterEnv: [ + ...baseConfig.setupFilesAfterEnv, + "./test-utils/setupJest.ts", + ], + moduleNameMapper: { + "^@/(.*)$": path.resolve(__dirname, "src/$1"), + ...baseConfig.moduleNameMapper, + }, +} + +export default config diff --git a/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx b/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx index d7bf022a64..d51b1f2b26 100644 --- a/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx +++ b/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx @@ -1,20 +1,10 @@ -import { renderTestApp, screen, waitFor, setMockResponse } from "@/test-utils" -import { urls } from "api/test-utils" -import * as commonUrls from "@/common/urls" -import { Permissions } from "@/common/permissions" +import React from "react" +import { screen, renderWithProviders } from "@/test-utils" +import { AboutPage } from "./AboutPage" describe("AboutPage", () => { test("Renders title", async () => { - setMockResponse.get(urls.userMe.get(), { - [Permissions.Authenticated]: true, - }) - - renderTestApp({ - url: commonUrls.ABOUT, - }) - await waitFor(() => { - expect(document.title).toBe("About Us | MIT Learn") - }) + renderWithProviders() screen.getByRole("heading", { name: "About Us", }) diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx index e75aa8284f..be819654b1 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx @@ -1,17 +1,17 @@ +import React from "react" import { urls, factories, makeRequest } from "api/test-utils" import { ChannelTypeEnum, type Channel } from "api/v0" import type { LearningResourcesSearchResponse } from "api" import { - renderTestApp, screen, setMockResponse, - within, waitFor, - assertPartialMetas, + renderWithProviders, } from "@/test-utils" import ChannelSearch from "./ChannelSearch" -import { assertHeadings } from "ol-test-utilities" +import { assertHeadings, getByImageSrc } from "ol-test-utilities" import invariant from "tiny-invariant" +import ChannelPage from "./ChannelPage" jest.mock("./ChannelSearch", () => { const actual = jest.requireActual("./ChannelSearch") @@ -141,7 +141,9 @@ describe.each(ALL_CHANNEL_TYPES)( "platform=ocw&platform=mitxonline&department=8&department=9", channel_type: channelType, }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) const expectedProps = expect.objectContaining({ constantSearchParams: { @@ -161,7 +163,9 @@ describe.each(ALL_CHANNEL_TYPES)( channel_type: channelType, }) channel.search_filter = undefined - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) expect(mockedChannelSearch).toHaveBeenCalledTimes(0) @@ -172,7 +176,9 @@ describe.each(ALL_CHANNEL_TYPES)( channel_type: channelType, }) channel.search_filter = undefined - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) await waitFor(() => { @@ -195,7 +201,9 @@ describe.each(ALL_CHANNEL_TYPES)( {}, { isSubscribed }, ) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) const subscribedButton = await screen.findByText("Follow") expect(subscribedButton).toBeVisible() }, @@ -212,7 +220,9 @@ describe.each(NON_UNIT_CHANNEL_TYPES)( channel_type: channelType, }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) const carousels = screen.queryByText("Featured Courses") expect(carousels).toBe(null) @@ -224,13 +234,10 @@ describe.each(NON_UNIT_CHANNEL_TYPES)( channel_type: channelType, }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - const title = await screen.findByRole("heading", { name: channel.title }) - await waitFor(() => { - assertPartialMetas({ - title: `${channel.title} | ${APP_SETTINGS.SITE_NAME}`, - }) + const { view } = renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, }) + const title = await screen.findByRole("heading", { name: channel.title }) // Banner background image expect( someAncestor(title, (el) => @@ -240,11 +247,10 @@ describe.each(NON_UNIT_CHANNEL_TYPES)( ), ).toBe(true) // logo - const images = screen.getAllByRole("img") - const logos = images.filter((img) => - img.src.includes(channel.configuration.logo), + getByImageSrc( + view.container, + `${window.origin}${channel.configuration.logo}`, ) - expect(logos.length).toBe(1) }) test("headings", async () => { @@ -252,7 +258,9 @@ describe.each(NON_UNIT_CHANNEL_TYPES)( search_filter: "topic=Physics", channel_type: channelType, }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await waitFor(() => { assertHeadings([ @@ -272,7 +280,9 @@ describe("Channel Pages, Topic only", () => { search_filter: "topic=Physics", channel_type: ChannelTypeEnum.Topic, }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) invariant(subTopics) const links = await screen.findAllByRole("link", { @@ -286,63 +296,17 @@ describe("Channel Pages, Topic only", () => { }) describe("Channel Pages, Unit only", () => { - it("Sets the expected meta tags", async () => { - const { channel } = setupApis({ - search_filter: "offered_by=ocw", - channel_type: "unit", - }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) - const title = `${channel.title} | ${APP_SETTINGS.SITE_NAME}` - const { heading: description } = channel.configuration - await waitFor(() => { - assertPartialMetas({ - title, - description, - og: { title, description }, - }) - }) - }) - - it("Sets the expected metadata tags when resource drawer is open", async () => { - /** - * Check that the meta tags are correct on channel page, even when the - * resource drawer is open. - */ - const { channel } = setupApis({ - search_filter: "offered_by=ocw", - channel_type: "unit", - }) - const resource = factories.learningResources.resource() - setMockResponse.get( - urls.learningResources.details({ id: resource.id }), - resource, - ) - - renderTestApp({ - url: `/c/${channel.channel_type}/${channel.name}?resource=${resource.id}`, - }) - await screen.findByRole("heading", { name: channel.title, hidden: true }) - const title = `${resource.title} | ${APP_SETTINGS.SITE_NAME}` - const description = resource.description - await waitFor(() => { - assertPartialMetas({ - title, - description, - og: { title, description }, - }) - }) - }) - it("Displays the channel title, banner, and avatar", async () => { const { channel } = setupApis({ search_filter: "offered_by=ocw", channel_type: "unit", }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) const title = await screen.findByRole("heading", { name: channel.title }) - const image = within(title).getByRole("img") - expect(image.src).toContain(channel.configuration.logo) + getByImageSrc(title, `${window.origin}${channel.configuration.logo}`) }) it("Displays a featured carousel if the channel type is 'unit'", async () => { const { channel } = setupApis({ @@ -350,7 +314,9 @@ describe("Channel Pages, Unit only", () => { channel_type: "unit", }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) const carousel = await screen.findByText("Featured Courses") expect(carousel).toBeInTheDocument() @@ -377,7 +343,9 @@ describe("Channel Pages, Unit only", () => { search_filter: "offered_by=ocw", channel_type: "unit", }) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await waitFor(() => { assertHeadings([ diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelPage.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelPage.tsx index 73b68b8ff5..7daf8b4f6e 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelPage.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelPage.tsx @@ -1,6 +1,7 @@ "use client" import React from "react" +import LearningResourceDrawer from "@/page-components/LearningResourceDrawer/LearningResourceDrawer" import { useParams } from "next/navigation" import { ChannelPageTemplate } from "./ChannelPageTemplate" import { useChannelDetail } from "api/hooks/channels" @@ -11,21 +12,74 @@ import type { BooleanFacets, } from "@mitodl/course-search-utils" import { ChannelTypeEnum } from "api/v0" -import LearningResourceDrawer from "@/page-components/LearningResourceDrawer/LearningResourceDrawer" +import { useLearningResourceTopics } from "api/hooks/learningResources" +import { ChipLink, Container, styled, Typography } from "ol-components" +import { propsNotNil } from "ol-utilities" + +const SubTopicsContainer = styled(Container)(({ theme }) => ({ + marginBottom: "60px", + [theme.breakpoints.down("sm")]: { + marginBottom: "24px", + }, +})) + +const SubTopicsHeader = styled(Typography)(({ theme }) => ({ + marginBottom: "10px", + ...theme.typography.subtitle1, +})) + +const ChipsContainer = styled.div({ + display: "flex", + flexWrap: "wrap", + gap: "12px", +}) type RouteParams = { channelType: ChannelTypeEnum name: string } +type SubTopicDisplayProps = { + parentTopicId: number +} + +const SubTopicsDisplay: React.FC = (props) => { + const { parentTopicId } = props + const topicsQuery = useLearningResourceTopics({ + parent_topic_id: [parentTopicId], + }) + const totalSubtopics = topicsQuery.data?.results.length ?? 0 + const subTopics = topicsQuery.data?.results.filter( + propsNotNil(["channel_url"]), + ) + return ( + totalSubtopics > 0 && ( + + Related Topics + + {subTopics?.map((topic) => ( + + ))} + + + ) + ) +} + const ChannelPage: React.FC = () => { const { channelType, name } = useParams() const channelQuery = useChannelDetail(String(channelType), String(name)) const searchParams: Facets & BooleanFacets = {} + const publicDescription = channelQuery.data?.public_description if (channelQuery.data?.search_filter) { const urlParams = new URLSearchParams(channelQuery.data.search_filter) - for (const [key, value] of urlParams.entries()) { const paramEntry = searchParams[key as FacetKey] if (paramEntry !== undefined) { @@ -42,7 +96,15 @@ const ChannelPage: React.FC = () => { <> -

{channelQuery.data?.public_description}

+ {publicDescription && ( + {publicDescription} + )} + {channelQuery.data?.channel_type === ChannelTypeEnum.Topic && + channelQuery.data?.topic_detail?.topic ? ( + + ) : null} {channelQuery.data?.search_filter && ( { }, }) setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}`, + }) await screen.findAllByText(channel.title) const tabpanel = await screen.findByRole("tabpanel") for (const resource of resources) { @@ -174,7 +178,7 @@ describe("ChannelSearch", () => { }) setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ + renderWithProviders(, { url: `/c/${channel.channel_type}/${channel.name}/${url}`, }) @@ -240,7 +244,9 @@ describe("ChannelSearch", () => { setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ url: `/c/${channel.channel_type}/${channel.name}/` }) + renderWithProviders(, { + url: `/c/${channel.channel_type}/${channel.name}/`, + }) await waitFor(() => { expect(makeRequest.mock.calls.length > 0).toBe(true) diff --git a/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx b/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx index 3afde875c5..8dd725e646 100644 --- a/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/Dashboard.test.tsx @@ -196,9 +196,6 @@ describe("DashboardPage", () => { test("Renders title", async () => { setupAPIs() renderWithProviders() - await waitFor(() => { - expect(document.title).toBe("Your MIT Learning Journey | MIT Learn") - }) screen.getByRole("heading", { name: "Your MIT Learning Journey", }) diff --git a/frontends/main/src/app-pages/DashboardPage/DashboardPage.tsx b/frontends/main/src/app-pages/DashboardPage/DashboardPage.tsx index 620af3b3b8..8f35dccbf7 100644 --- a/frontends/main/src/app-pages/DashboardPage/DashboardPage.tsx +++ b/frontends/main/src/app-pages/DashboardPage/DashboardPage.tsx @@ -341,7 +341,7 @@ const DashboardPage: React.FC = () => { diff --git a/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.test.tsx b/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.test.tsx index 8138a4bb73..3bea375333 100644 --- a/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.test.tsx +++ b/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.test.tsx @@ -86,9 +86,6 @@ describe("DepartmentListingPage", () => { it("Has correct page title", async () => { setupApis() renderWithProviders() - await waitFor(() => { - expect(document.title).toBe("Departments | MIT Learn") - }) screen.getByRole("heading", { name: "Browse by Academic Department" }) }) @@ -156,6 +153,7 @@ describe("DepartmentListingPage", () => { const link = await screen.findByRole("link", { name: (name) => name.includes(dept.name), }) + expect(link).toHaveAttribute("href", dept.channel_url) }) diff --git a/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.tsx b/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.tsx index 393fcf9eda..26e84e9771 100644 --- a/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.tsx +++ b/frontends/main/src/app-pages/DepartmentListingPage/DepartmentListingPage.tsx @@ -147,15 +147,10 @@ const SchoolDepartments: React.FC = ({ { count: courses, label: pluralize("Course", courses) }, { count: programs, label: pluralize("Program", programs) }, ] + if (!department.channel_url) return null return ( - + { +test.skip("The NotFoundPage loads with meta", async () => { renderWithProviders(, {}) await waitFor(() => { const meta = document.head.querySelector('meta[name="robots"]') diff --git a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx index 6fac87776e..1617a43b47 100644 --- a/frontends/main/src/app-pages/HomePage/HomePage.test.tsx +++ b/frontends/main/src/app-pages/HomePage/HomePage.test.tsx @@ -101,16 +101,16 @@ describe("Home Page Hero", () => { setupAPIs() renderWithProviders() const expected = [ - { label: "Topic", href: "/topics/" }, - { label: "Recently Added", href: "/search/?sortby=new" }, - { label: "Popular", href: "/search/?sortby=-views" }, - { label: "Upcoming", href: "/search/?sortby=upcoming" }, - { label: "Free", href: "/search/?free=true" }, + { label: "Topic", href: "/topics" }, + { label: "Recently Added", href: "/search?sortby=new" }, + { label: "Popular", href: "/search?sortby=-views" }, + { label: "Upcoming", href: "/search?sortby=upcoming" }, + { label: "Free", href: "/search?free=true" }, { label: "With Certificate", - href: "/search/?certification_type=professional&certification_type=completion&certification_type=micromasters", + href: "/search?certification_type=professional&certification_type=completion&certification_type=micromasters", }, - { label: "Explore All", href: "/search/" }, + { label: "Explore All", href: "/search" }, ] expected.forEach(({ label, href }) => { const link = screen.getByRole("link", { name: label }) @@ -263,7 +263,7 @@ describe("Home Page personalize section", () => { ).closest("section") invariant(personalize) const link = within(personalize).getByRole("link") - expect(link).toHaveAttribute("href", "/dashboard/") + expect(link).toHaveAttribute("href", "/dashboard") }) test("Links to login when not authenticated", async () => { diff --git a/frontends/main/src/app-pages/HomePage/PersonalizeSection.tsx b/frontends/main/src/app-pages/HomePage/PersonalizeSection.tsx index cfc5b4c93f..93bd620af7 100644 --- a/frontends/main/src/app-pages/HomePage/PersonalizeSection.tsx +++ b/frontends/main/src/app-pages/HomePage/PersonalizeSection.tsx @@ -73,9 +73,7 @@ const AUTH_TEXT_DATA = { linkProps: { children: "Sign Up for Free", rawAnchor: true, - href: urls.login({ - pathname: urls.DASHBOARD_HOME, - }), + href: urls.login({ pathname: urls.DASHBOARD_HOME }), }, }, } diff --git a/frontends/main/src/app-pages/LearningPathListingPage/LearningPathListingPage.test.tsx b/frontends/main/src/app-pages/LearningPathListingPage/LearningPathListingPage.test.tsx index a2ea595d12..d413f9b912 100644 --- a/frontends/main/src/app-pages/LearningPathListingPage/LearningPathListingPage.test.tsx +++ b/frontends/main/src/app-pages/LearningPathListingPage/LearningPathListingPage.test.tsx @@ -8,7 +8,6 @@ import { renderWithProviders, setMockResponse, user, - waitFor, } from "@/test-utils" import type { User } from "@/test-utils" @@ -23,7 +22,8 @@ const setup = ({ listsCount?: number } = {}) => { const paths = factories.learningResources.learningPaths({ count: listsCount }) - + const userData = factories.user.user({ ...user }) + setMockResponse.get(urls.userMe.get(), userData) setMockResponse.get(urls.learningPaths.list(), paths) const { location } = renderWithProviders(, { @@ -37,9 +37,6 @@ describe("LearningPathListingPage", () => { it("Has title 'Learning Paths'", async () => { setup() screen.getByRole("heading", { name: "Learning Paths" }) - await waitFor(() => - expect(document.title).toBe("Learning Paths | MIT Learn"), - ) }) it("Renders a card for each learning path", async () => { @@ -131,18 +128,11 @@ describe("LearningPathListingPage", () => { }) test("Clicking on list title navigates to list page", async () => { - const { location, paths } = setup() + const { paths } = setup() const path = faker.helpers.arrayElement(paths.results) - const listTitle = await screen.findByRole("link", { + const link = await screen.findByRole("link", { name: new RegExp(path.title), }) - await user.click(listTitle) - expect(location.current).toEqual( - expect.objectContaining({ - pathname: `/learningpaths/${path.id}`, - search: "", - hash: "", - }), - ) + expect(link).toHaveAttribute("href", `/learningpaths/${path.id}`) }) }) diff --git a/frontends/main/src/app-pages/OnboardingPage/OnboardingPage.test.tsx b/frontends/main/src/app-pages/OnboardingPage/OnboardingPage.test.tsx index d6dd7b0650..b08dfed65c 100644 --- a/frontends/main/src/app-pages/OnboardingPage/OnboardingPage.test.tsx +++ b/frontends/main/src/app-pages/OnboardingPage/OnboardingPage.test.tsx @@ -58,7 +58,7 @@ const PROFILES_FOR_STEPS = times(STEPS_DATA.length, profileForStep) const setup = async (profile: Profile) => { allowConsoleErrors() - + setMockResponse.get(urls.userMe.get(), factories.user.user()) setMockResponse.get(urls.profileMe.get(), profile) setMockResponse.patch(urls.profileMe.patch(), (req: Partial) => ({ ...profile, diff --git a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx index 5e5845549b..0fb699cee7 100644 --- a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx +++ b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx @@ -1,7 +1,8 @@ -import { renderTestApp, screen, waitFor, setMockResponse } from "@/test-utils" +import React from "react" +import { screen, setMockResponse, renderWithProviders } from "@/test-utils" import { urls } from "api/test-utils" -import * as commonUrls from "@/common/urls" import { Permissions } from "@/common/permissions" +import PrivacyPage from "./PrivacyPage" describe("PrivacyPage", () => { test("Renders title", async () => { @@ -9,12 +10,8 @@ describe("PrivacyPage", () => { [Permissions.Authenticated]: true, }) - renderTestApp({ - url: commonUrls.PRIVACY, - }) - await waitFor(() => { - expect(document.title).toBe("Privacy Policy | MIT Learn") - }) + renderWithProviders() + screen.getByRole("heading", { name: "Privacy Policy", }) diff --git a/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx similarity index 86% rename from frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx rename to frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx index 59323970a5..33bf36c46c 100644 --- a/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx +++ b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx @@ -1,8 +1,10 @@ -import { renderTestApp, waitFor } from "../../test-utils" +import React from "react" +import { renderWithProviders, waitFor } from "@/test-utils" import type { ProgramLetter } from "api" import { letters as factory } from "api/test-utils/factories" import { setMockResponse, urls } from "api/test-utils" import { programLetterView } from "@/common/urls" +import ProgramLetterPage from "./ProgramLetterPage" const setup = ({ programLetter }: { programLetter: ProgramLetter }) => { setMockResponse.get( @@ -10,7 +12,7 @@ const setup = ({ programLetter }: { programLetter: ProgramLetter }) => { programLetter, ) setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ + renderWithProviders(, { url: programLetterView(programLetter.id), }) } diff --git a/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetterPage.tsx b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetterPage.tsx similarity index 100% rename from frontends/main/src/app-pages/ProgramLetterPage/ProgramLetterPage.tsx rename to frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetterPage.tsx diff --git a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx index cefd67e012..667979d23d 100644 --- a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx +++ b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx @@ -1,7 +1,8 @@ -import { renderTestApp, screen, waitFor, setMockResponse } from "@/test-utils" +import React from "react" +import { screen, setMockResponse, renderWithProviders } from "@/test-utils" import { urls } from "api/test-utils" -import * as commonUrls from "@/common/urls" import { Permissions } from "@/common/permissions" +import TermsPage from "./TermsPage" describe("TermsPage", () => { test("Renders title", async () => { @@ -9,12 +10,7 @@ describe("TermsPage", () => { [Permissions.Authenticated]: true, }) - renderTestApp({ - url: commonUrls.TERMS, - }) - await waitFor(() => { - expect(document.title).toBe("Terms of Service | MIT Learn") - }) + renderWithProviders() screen.getByRole("heading", { name: "Terms of Service", }) diff --git a/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx b/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx index 91802d56df..13b8bafc2f 100644 --- a/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx +++ b/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx @@ -91,9 +91,6 @@ describe("TopicsListingPage", () => { it("Has a page title", async () => { setupApis() renderWithProviders() - await waitFor(() => { - expect(document.title).toBe("Topics | MIT Learn") - }) screen.getByRole("heading", { name: "Browse by Topic" }) }) @@ -143,8 +140,6 @@ describe("TopicsListingPage", () => { await screen.findByRole("heading", { name: topics.t2.name }), ) - await new Promise((res) => setTimeout(res, 200)) - expect(topic1).toHaveTextContent("Courses: 100") expect(topic1).toHaveTextContent("Programs: 10") diff --git a/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx b/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx index 5a901d2beb..b731f967a2 100644 --- a/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx +++ b/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx @@ -133,9 +133,6 @@ describe("DepartmentListingPage", () => { it("Has a page title", async () => { setupApis() renderWithProviders() - await waitFor(() => { - expect(document.title).toBe("Units | MIT Learn") - }) screen.getByRole("heading", { name: "Academic & Professional Learning" }) }) diff --git a/frontends/main/src/app/getQueryClient.ts b/frontends/main/src/app/getQueryClient.ts index 0d5e1e8e26..e5116d82cd 100644 --- a/frontends/main/src/app/getQueryClient.ts +++ b/frontends/main/src/app/getQueryClient.ts @@ -1,18 +1,29 @@ -// https://tanstack.com/query/v4/docs/framework/react/guides/ssr#using-hydrate +// Based on https://tanstack.com/query/v5/docs/framework/react/guides/advanced-ssr -import { QueryClient } from "api/ssr" -// import { cache } from "react" +import { QueryClient, isServer } from "api/ssr" -/* - * Using cache() Errors with: - * Error occurred prerendering page "/about". Read more: https://nextjs.org/docs/messages/prerender-error - Error: Not implemented. - at Object.getCacheForType - https://github.com/vercel/next.js/issues/57205 - */ -// const getQueryClient = cache(() => new QueryClient()) +function makeQueryClient() { + return new QueryClient({ + defaultOptions: { + queries: {}, + }, + }) +} -// TODO We need to ensure this is called only once per request -const getQueryClient = () => new QueryClient() +let browserQueryClient: QueryClient | undefined = undefined -export default getQueryClient +function getQueryClient() { + if (isServer) { + // Server: always make a new query client + return makeQueryClient() + } else { + // Browser: make a new query client if we don't already have one + // This is very important, so we don't re-make a new client if React + // suspends during the initial render. This may not be needed if we + // have a suspense boundary BELOW the creation of the query client + if (!browserQueryClient) browserQueryClient = makeQueryClient() + return browserQueryClient + } +} + +export { makeQueryClient, getQueryClient } diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index 411f7c0452..c1d6165e03 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -4,8 +4,8 @@ import { dehydrate, Hydrate } from "api/ssr" import HomePage from "@/app-pages/HomePage/HomePage" import * as carousels from "@/app-pages/HomePage/carousels" import { learningResourcesKeyFactory } from "api/hooks/learningResources" -import { FeaturedApiFeaturedListRequest } from "api/generated/v1/api" -import getQueryClient from "./getQueryClient" +import { FeaturedApiFeaturedListRequest } from "api/v1" +import { getQueryClient } from "./getQueryClient" import { getMetadataAsync } from "@/common/metadata" export async function generateMetadata({ diff --git a/frontends/main/src/app/program_letter/[id]/view/page.tsx b/frontends/main/src/app/program_letter/[id]/view/page.tsx index 849a6050e5..faa058688c 100644 --- a/frontends/main/src/app/program_letter/[id]/view/page.tsx +++ b/frontends/main/src/app/program_letter/[id]/view/page.tsx @@ -1,5 +1,5 @@ import React from "react" -import ProgramLetterPage from "@/app-pages/ProgramLetterPage/ProgramLetterPage" +import ProgramLetterPage from "@/app-pages/ProgramLetterPage/[id]/view/ProgramLetterPage" const Page: React.FC = () => { return diff --git a/frontends/main/src/app/providers.tsx b/frontends/main/src/app/providers.tsx index 8dbe732cf0..3d7793dd1b 100644 --- a/frontends/main/src/app/providers.tsx +++ b/frontends/main/src/app/providers.tsx @@ -1,7 +1,7 @@ "use client" import React from "react" -import getQueryClient from "./getQueryClient" +import { getQueryClient } from "./getQueryClient" import { QueryClientProvider } from "api/ssr" import { ThemeProvider, NextJsAppRouterCacheProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" diff --git a/frontends/main/src/common/urls.test.ts b/frontends/main/src/common/urls.test.ts index 7e7ae66529..24485014b6 100644 --- a/frontends/main/src/common/urls.test.ts +++ b/frontends/main/src/common/urls.test.ts @@ -1,27 +1,29 @@ import { login } from "./urls" -const { MITOL_API_BASE_URL } = APP_SETTINGS +const MITOL_API_BASE_URL = process.env.NEXT_PUBLIC_MITOL_API_BASE_URL test("login encodes the next parameter appropriately", () => { expect(login()).toBe( - `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://localhost/`, + `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://test.learn.odl.local:8062/`, ) expect(login({})).toBe( - `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://localhost/`, + `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://test.learn.odl.local:8062/`, ) expect( login({ pathname: "/foo/bar", }), - ).toBe(`${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://localhost/foo/bar`) + ).toBe( + `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://test.learn.odl.local:8062/foo/bar`, + ) expect( login({ pathname: "/foo/bar", - search: "?cat=meow", + searchParams: new URLSearchParams("?cat=meow"), }), ).toBe( - `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://localhost/foo/bar%3Fcat%3Dmeow`, + `${MITOL_API_BASE_URL}/login/ol-oidc/?next=http://test.learn.odl.local:8062/foo/bar%3Fcat%3Dmeow`, ) }) diff --git a/frontends/main/src/common/urls.ts b/frontends/main/src/common/urls.ts index 04e566d59c..c1f6c47222 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -1,8 +1,10 @@ +import invariant from "tiny-invariant" + const generatePath = ( template: string, params: Record, ): string => { - return template.replace(/:(\w+)/g, (_, key) => { + return template.replace(/\[(\w+)\]/g, (_, key) => { if (params[key] === undefined) { throw new Error(`Missing parameter '${key}'`) } @@ -14,16 +16,16 @@ export const HOME = "/" export const ONBOARDING = "/onboarding" -export const LEARNINGPATH_LISTING = "/learningpaths/" -export const LEARNINGPATH_VIEW = "/learningpaths/:id" +export const LEARNINGPATH_LISTING = "/learningpaths" +export const LEARNINGPATH_VIEW = "/learningpaths/[id]" export const learningPathsView = (id: number) => generatePath(LEARNINGPATH_VIEW, { id: String(id) }) -export const PROGRAMLETTER_VIEW = "/program_letter/:id/view/" +export const PROGRAMLETTER_VIEW = "/program_letter/[id]/view/" export const programLetterView = (id: string) => generatePath(PROGRAMLETTER_VIEW, { id: String(id) }) export const ARTICLES_LISTING = "/articles/" -export const ARTICLES_DETAILS = "/articles/:id" -export const ARTICLES_EDIT = "/articles/:id/edit" +export const ARTICLES_DETAILS = "/articles/[id]" +export const ARTICLES_EDIT = "/articles/[id]/edit" export const ARTICLES_CREATE = "/articles/new" export const articlesView = (id: number) => generatePath(ARTICLES_DETAILS, { id: String(id) }) @@ -33,10 +35,10 @@ export const articlesEditView = (id: number) => export const DEPARTMENTS = "/departments/" export const TOPICS = "/topics/" -export const CHANNEL_VIEW = "/c/:channelType/:name" as const -export const CHANNEL_EDIT = "/c/:channelType/:name/manage/" as const +export const CHANNEL_VIEW = "/c/[channelType]/[name]" as const +export const CHANNEL_EDIT = "/c/[channelType]/[name]/manage/" as const export const CHANNEL_EDIT_WIDGETS = - "/c/:channelType/:name/manage/widgets/" as const + "/c/[channelType]/[name]/manage/widgets/" as const export const makeChannelViewPath = (channelType: string, name: string) => generatePath(CHANNEL_VIEW, { channelType, name }) export const makeChannelEditPath = (channelType: string, name: string) => @@ -47,6 +49,10 @@ export const makeChannelManageWidgetsPath = ( ) => generatePath(CHANNEL_EDIT_WIDGETS, { channelType, name }) const ORIGIN = process.env.NEXT_PUBLIC_ORIGIN +if (process.env.NODE_ENV !== "production") { + invariant(!ORIGIN?.endsWith("/"), "NEXT_PUBLIC_ORIGIN should not end with /") +} + const MITOL_API_BASE_URL = process.env.NEXT_PUBLIC_MITOL_API_BASE_URL export const LOGIN = `${MITOL_API_BASE_URL}/login/ol-oidc/` @@ -58,11 +64,11 @@ export const LOGOUT = `${MITOL_API_BASE_URL}/logout/` */ export const login = ({ pathname = "/", - search = "", + searchParams = new URLSearchParams(), hash = "", }: { pathname?: string | null - search?: string + searchParams?: URLSearchParams hash?: string | null } = {}) => { /** @@ -73,20 +79,21 @@ export const login = ({ * There's no need to encode the path parameter (it might contain slashes, * but those are allowed in search parameters) so let's keep it readable. */ - const next = `${ORIGIN}/${pathname}${encodeURIComponent(search)}${encodeURIComponent(hash as string)}` + const search = searchParams.toString() ? `?${searchParams.toString()}` : "" + const next = `${ORIGIN}${pathname}${encodeURIComponent(search)}${encodeURIComponent(hash as string)}` return `${LOGIN}?next=${next}` } -export const DASHBOARD_HOME = "/dashboard/" +export const DASHBOARD_HOME = "/dashboard" -export const MY_LISTS = "/dashboard/my-lists/" -export const USERLIST_VIEW = "/dashboard/my-lists/:id" +export const MY_LISTS = "/dashboard/my-lists" +export const USERLIST_VIEW = "/dashboard/my-lists/[id]" export const userListView = (id: number) => generatePath(USERLIST_VIEW, { id: String(id) }) -export const PROFILE = "/dashboard/profile/" +export const PROFILE = "/dashboard/profile" -export const SETTINGS = "/dashboard/settings/" +export const SETTINGS = "/dashboard/settings" export const SEARCH = "/search/" diff --git a/frontends/main/src/components/ChannelAvatar/ChannelAvatar.test.tsx b/frontends/main/src/components/ChannelAvatar/ChannelAvatar.test.tsx index 2d11709a1b..531f3b45eb 100644 --- a/frontends/main/src/components/ChannelAvatar/ChannelAvatar.test.tsx +++ b/frontends/main/src/components/ChannelAvatar/ChannelAvatar.test.tsx @@ -4,24 +4,30 @@ import { render, screen } from "@testing-library/react" import { ThemeProvider } from "ol-components" import { channels as factory } from "api/test-utils/factories" import ChannelAvatar from "./ChannelAvatar" +import { getByImageSrc } from "ol-test-utilities" +import invariant from "tiny-invariant" describe("Avatar", () => { it("Displays a small avatar image for the channel", async () => { const channel = factory.channel() - render(, { + const view = render(, { wrapper: ThemeProvider, }) - const img = screen.getByRole("img") + invariant(channel.avatar_small) + const img = getByImageSrc(view.container, channel.avatar_small) expect(img.getAttribute("alt")).toBe("") // should be empty unless meaningful - expect(img.getAttribute("src")).toEqual(channel.avatar_small) }) + it("Displays a medium avatar image by default", async () => { const channel = factory.channel() - render(, { wrapper: ThemeProvider }) - const img = screen.getByRole("img") + const view = render(, { + wrapper: ThemeProvider, + }) + invariant(channel.avatar_medium) + const img = getByImageSrc(view.container, channel.avatar_medium) expect(img.getAttribute("alt")).toBe("") // should be empty unless meaningful - expect(img.getAttribute("src")).toEqual(channel.avatar_medium) }) + it("Displays initials if no avatar image exists", async () => { const channel = factory.channel({ title: "Test Title", diff --git a/frontends/main/src/components/MITLogoLink/MITLogoLink.tsx b/frontends/main/src/components/MITLogoLink/MITLogoLink.tsx index 25ed9647cc..b94f3f4b67 100644 --- a/frontends/main/src/components/MITLogoLink/MITLogoLink.tsx +++ b/frontends/main/src/components/MITLogoLink/MITLogoLink.tsx @@ -6,9 +6,15 @@ interface Props { href?: string className?: string logo?: string + alt?: string } -const MITLogoLink: React.FC = ({ href, logo, className }) => ( +const MITLogoLink: React.FC = ({ + href, + logo, + alt = "MIT Learn Logo", + className, +}) => ( = ({ href, logo, className }) => ( // eslint-disable-next-line react/no-unknown-property appzi-screenshot-exclude="true" > - MIT Learn Logo + {alt} ) diff --git a/frontends/main/src/page-components/Footer/Footer.tsx b/frontends/main/src/page-components/Footer/Footer.tsx index 318efd8eda..1619735741 100644 --- a/frontends/main/src/page-components/Footer/Footer.tsx +++ b/frontends/main/src/page-components/Footer/Footer.tsx @@ -148,7 +148,11 @@ const Footer: FunctionComponent = () => { - + Massachusetts Institute of Technology
diff --git a/frontends/main/src/page-components/Header/Header.test.tsx b/frontends/main/src/page-components/Header/Header.test.tsx index ed8ac0e1eb..003b5ebb56 100644 --- a/frontends/main/src/page-components/Header/Header.test.tsx +++ b/frontends/main/src/page-components/Header/Header.test.tsx @@ -75,7 +75,7 @@ describe("UserMenu", () => { const initialUrl = "/foo/bar?cat=meow" const expectedUrl = urlConstants.login({ pathname: "/foo/bar", - search: "?cat=meow", + searchParams: new URLSearchParams("?cat=meow"), }) setMockResponse.get(urls.userMe.get(), { is_authenticated: isAuthenticated, @@ -119,13 +119,12 @@ describe("UserMenu", () => { test("Learning path editors see 'Learning Paths' link", async () => { setMockResponse.get(urls.userMe.get(), { is_learning_path_editor: true }) - const { location } = renderWithProviders(
) + renderWithProviders(
) const menu = await findUserMenu() const link = within(menu).getByRole("menuitem", { name: "Learning Paths", }) - await user.click(link) - expect(location.current.pathname).toBe("/learningpaths/") + expect(link).toHaveAttribute("href", "/learningpaths") }) test("Users WITHOUT LearningPathEditor permission do not see 'Learning Paths' link", async () => { diff --git a/frontends/main/src/page-components/Header/UserMenu.tsx b/frontends/main/src/page-components/Header/UserMenu.tsx index 3cb8535f46..a5f7469f28 100644 --- a/frontends/main/src/page-components/Header/UserMenu.tsx +++ b/frontends/main/src/page-components/Header/UserMenu.tsx @@ -1,7 +1,7 @@ "use client" import React, { useState } from "react" -import { ButtonLink, SimpleMenu, styled } from "ol-components" +import { ActionButtonLink, ButtonLink, SimpleMenu, styled } from "ol-components" import type { MenuOverrideProps, SimpleMenuItem } from "ol-components" import * as urls from "@/common/urls" import { @@ -97,16 +97,13 @@ const UserMenu: React.FC = ({ variant }) => { const [visible, setVisible] = useState(false) const pathname = usePathname() - const search = useSearchParams() + const searchParams = useSearchParams() const { isLoading, data: user } = useUserMe() if (isLoading) { return null } - const loginUrl = urls.login({ - pathname, - search: search.toString(), - }) + const loginUrl = urls.login({ pathname, searchParams }) const items: UserMenuItem[] = [ { @@ -173,7 +170,7 @@ const UserMenu: React.FC = ({ variant }) => { )} {variant === "mobile" ? ( - {/* TODO = ({ variant }) => { aria-label="Log in" > - */} + ) : ( "" diff --git a/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx b/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx index 6511f69590..509c89a219 100644 --- a/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx +++ b/frontends/main/src/page-components/ItemsListing/ItemsListing.test.tsx @@ -165,6 +165,8 @@ describe("ItemsListing", () => { faker.number.int(), ) const items = paginatedRelationships.results as LearningResourceListItem[] + const user = factories.user.user() + setMockResponse.get(urls.userMe.get(), user) renderWithProviders( { sortable={sortable} condensed={condensed} />, - { user: {} }, ) const titles = items.map((item) => item.resource.title) diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx index bbfb594b2c..f9a9a4b444 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx @@ -37,13 +37,14 @@ jest.mock("posthog-js/react", () => ({ })) describe("LearningResourceDrawer", () => { - it.each([ + it.skip.each([ { descriptor: "is enabled", enablePostHog: true }, { descriptor: "is not enabled", enablePostHog: false }, ])( "Renders drawer content when resource=id is in the URL and captures the view if PostHog $descriptor", async ({ enablePostHog }) => { setMockResponse.get(urls.userMe.get(), {}) + // @ts-expect-error reinstante posthog APP_SETTINGS.POSTHOG = { api_key: enablePostHog ? "test1234" : "", // pragma: allowlist secret } @@ -139,13 +140,17 @@ describe("LearningResourceDrawer", () => { urls.learningResources.details({ id: resource.id }), resource, ) + const user = factories.user.user({ + is_learning_path_editor: isLearningPathEditor, + }) + if (isAuthenticated) { + setMockResponse.get(urls.userMe.get(), user) + } else { + setMockResponse.get(urls.userMe.get(), null, { code: 403 }) + } renderWithProviders(, { url: `?resource=${resource.id}`, - user: { - is_learning_path_editor: isLearningPathEditor, - is_authenticated: isAuthenticated, - }, }) expect(LearningResourceExpanded).toHaveBeenCalled() diff --git a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx index f7b8074248..4ec73676c4 100644 --- a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx +++ b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -25,10 +25,15 @@ jest.mock("ol-components", () => { jest.mock("@ebay/nice-modal-react", () => { const actual = jest.requireActual("@ebay/nice-modal-react") + const show = jest.fn() return { __esModule: true, ...actual, - show: jest.fn(), + show, + default: { + ...actual.default, + show: show, + }, } }) @@ -177,16 +182,16 @@ describe.each([ }) test("Clicking card opens resource drawer", async () => { - const { resource, location } = setup({ + const { resource } = setup({ user: { is_learning_path_editor: true }, }) invariant(resource) const link = screen.getByRole("link", { name: new RegExp(resource.title) }) - await user.click(link) - expect( - new URLSearchParams(location.current.search).get( - RESOURCE_DRAWER_QUERY_PARAM, - ), - ).toBe(String(resource.id)) + const href = link.getAttribute("href") + invariant(href) + const url = new URL(href, window.location.href) + expect(url.searchParams.get(RESOURCE_DRAWER_QUERY_PARAM)).toBe( + String(resource.id), + ) }) }) diff --git a/frontends/main/src/page-components/SignupPopover/SignupPopover.test.tsx b/frontends/main/src/page-components/SignupPopover/SignupPopover.test.tsx index f5c6fe4cee..98f8917064 100644 --- a/frontends/main/src/page-components/SignupPopover/SignupPopover.test.tsx +++ b/frontends/main/src/page-components/SignupPopover/SignupPopover.test.tsx @@ -17,7 +17,7 @@ test("SignupPopover shows link to sign up", async () => { expect(link.href).toMatch( urls.login({ pathname: "/some-path", - search: "?dog=woof", + searchParams: new URLSearchParams("dog=woof"), }), ) }) diff --git a/frontends/main/src/page-components/SignupPopover/SignupPopover.tsx b/frontends/main/src/page-components/SignupPopover/SignupPopover.tsx index b60dcf0185..9b0f92ab0f 100644 --- a/frontends/main/src/page-components/SignupPopover/SignupPopover.tsx +++ b/frontends/main/src/page-components/SignupPopover/SignupPopover.tsx @@ -40,12 +40,7 @@ const SignupPopover: React.FC = (props) => { and follow your areas of interest.
- + Sign Up
diff --git a/frontends/main/src/page-components/UserListListing/UserListListing.test.tsx b/frontends/main/src/page-components/UserListListing/UserListListing.test.tsx index cc1ec6a91e..e95c932bc2 100644 --- a/frontends/main/src/page-components/UserListListing/UserListListing.test.tsx +++ b/frontends/main/src/page-components/UserListListing/UserListListing.test.tsx @@ -115,13 +115,11 @@ describe("UserListListing", () => { }) test("Clicking on the card shows the list detail view", async () => { - const { paths, location } = setup() + const { paths } = setup() const card = await screen.findByTestId( `user-list-card-condensed-${paths.results[0].id}`, ) const cardLink = within(card).getByRole("link") - - await user.click(cardLink) - expect(location.current.pathname).toBe(userListView(paths.results[0].id)) + expect(cardLink).toHaveAttribute("href", userListView(paths.results[0].id)) }) }) diff --git a/frontends/main/test-utils/delete.mockAxios.ts b/frontends/main/src/test-utils/delete.mockAxios.ts similarity index 100% rename from frontends/main/test-utils/delete.mockAxios.ts rename to frontends/main/src/test-utils/delete.mockAxios.ts diff --git a/frontends/main/test-utils/factories.ts b/frontends/main/src/test-utils/factories.ts similarity index 100% rename from frontends/main/test-utils/factories.ts rename to frontends/main/src/test-utils/factories.ts diff --git a/frontends/main/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx similarity index 71% rename from frontends/main/test-utils/index.tsx rename to frontends/main/src/test-utils/index.tsx index 3dcba40530..62237c9a89 100644 --- a/frontends/main/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -1,93 +1,93 @@ /* eslint-disable import/no-extraneous-dependencies */ import React from "react" -import { createMemoryRouter, useRouteError } from "react-router" -import type { RouteObject } from "react-router" +import { QueryClientProvider } from "api/ssr" +import { ThemeProvider } from "ol-components" +import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" +import type { QueryClient } from "api/ssr" -// @ts-expect-error Fixing tests next -import AppProviders from "../AppProviders" -// @ts-expect-error Fixing tests next -import appRoutes from "../routes" +import { makeQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" import { setMockResponse } from "api/test-utils" -// @ts-expect-error Fixing tests next -import { createQueryClient } from "@/services/react-query/react-query" import type { User } from "api/hooks/user" -import { QueryKey } from "@tanstack/react-query" +import { + mockRouter, + createDynamicRouteParser, +} from "ol-test-utilities/mocks/nextNavigation" +import * as urls from "@/common/urls" + +/** + * Makes nextNavigation aware of the routes. + * + * This is needed for `useParams` to pick up the correct path parameters. + */ +const setupRoutes = () => { + mockRouter.useParser( + createDynamicRouteParser([ + // + urls.PROGRAMLETTER_VIEW, + urls.CHANNEL_VIEW, + ]), + ) +} +setupRoutes() interface TestAppOptions { url: string user: Partial - queryClient?: ReturnType - initialQueryData?: [QueryKey, unknown][] } const defaultTestAppOptions = { url: "/", } - -/** - * React-router includes a default error boundary which catches thrown errors. - * - * During testing, we want all unexpected errors to be re-thrown. - */ -const RethrowError = () => { - const error = useRouteError() - if (error) { - throw error - } - return null +const defaultUser: User = { + is_authenticated: true, } +const TestProviders: React.FC<{ + children: React.ReactNode + queryClient: QueryClient +}> = ({ children, queryClient }) => ( + + + {children} + + +) + /** * Render routes in a test environment using same providers used by App. */ -const renderRoutesWithProviders = ( - routes: RouteObject[], +const renderWithProviders = ( + component: React.ReactNode, options: Partial = {}, ) => { - const { url, user, initialQueryData } = { - ...defaultTestAppOptions, - ...options, - } + const allOpts = { ...defaultTestAppOptions, ...options } + const { url } = allOpts - const router = createMemoryRouter(routes, { initialEntries: [url] }) - const queryClient = options.queryClient || createQueryClient() + const queryClient = makeQueryClient() - if (user) { - queryClient.setQueryData(["userMe"], { is_authenticated: true, ...user }) - } - if (initialQueryData) { - initialQueryData.forEach(([queryKey, data]) => { - queryClient.setQueryData(queryKey, data) - }) + if (allOpts.user) { + const user = { ...defaultUser, ...allOpts.user } + queryClient.setQueryData(["userMe"], { ...user }) } + mockRouter.setCurrentUrl(url) + const view = render( - , + {component}, ) const location = { get current() { - return router.state.location + const url = new URL(mockRouter.asPath, "http://localhost") + return { pathname: mockRouter.pathname, search: url.search } }, } return { view, queryClient, location } } -const renderTestApp = (options?: Partial) => - renderRoutesWithProviders(appRoutes, options) - -/** - * Render element in a test environment using same providers used by App. - */ -const renderWithProviders = ( - element: React.ReactNode, - options?: Partial, -) => { - const routes: RouteObject[] = [ - { element, path: "*", errorElement: }, - ] - return renderRoutesWithProviders(routes, options) +const renderTestApp = () => { + throw new Error("not supported") } /** @@ -225,7 +225,6 @@ const assertPartialMetas = (expected: Partial) => { export { renderTestApp, renderWithProviders, - renderRoutesWithProviders, expectProps, expectLastProps, expectWindowNavigation, diff --git a/frontends/main/test-utils/setupJest.ts b/frontends/main/src/test-utils/setupJest.ts similarity index 100% rename from frontends/main/test-utils/setupJest.ts rename to frontends/main/src/test-utils/setupJest.ts diff --git a/frontends/main/test-utils/withFakeLocation.ts b/frontends/main/src/test-utils/withFakeLocation.ts similarity index 100% rename from frontends/main/test-utils/withFakeLocation.ts rename to frontends/main/src/test-utils/withFakeLocation.ts diff --git a/frontends/main/test-utils/example-request-mocks.test.ts b/frontends/main/test-utils/example-request-mocks.test.ts deleted file mode 100644 index f9d8a1f821..0000000000 --- a/frontends/main/test-utils/example-request-mocks.test.ts +++ /dev/null @@ -1,100 +0,0 @@ -import axios from "api/axios" -import { setMockResponse } from "api/test-utils" -import { ControlledPromise, allowConsoleErrors } from "ol-test-utilities" - -describe("request mocking", () => { - test("mocking specific responses and spying", async () => { - setMockResponse.post( - "/some-example", - { someResponseKey: "response for request with {a:5}" }, - { requestBody: expect.objectContaining({ a: 5 }) }, - ) - setMockResponse.post( - "/some-example", - { someResponseKey: "response for request with {b:10}" }, - { requestBody: expect.objectContaining({ b: 10 }) }, - ) - setMockResponse.post( - "/some-example", - { someResponseKey: "fallback post response" }, - // if 3rd arg is undefined, the response (2nd arg) will be used for all unmatched request bodies - ) - - setMockResponse.patch("/another-example", { someResponseKey: "patched!" }) - - const r0 = await axios.post("/some-example", { dog: "woof" }) - const r1 = await axios.patch("/another-example", { dog: "bark bark" }) - const r2 = await axios.post("/some-example", { baby: "sleep", b: 10 }) - const r3 = await axios.post("/some-example", { cat: "meow", a: 5 }) - - // toHaveBeenNthCalledWith is 1-indexed - expect(axios.post).toHaveBeenNthCalledWith(1, "/some-example", { - dog: "woof", - }) - expect(axios.patch).toHaveBeenNthCalledWith(1, "/another-example", { - dog: "bark bark", - }) - expect(axios.post).toHaveBeenNthCalledWith(2, "/some-example", { - baby: "sleep", - b: 10, - }) - expect(axios.post).toHaveBeenNthCalledWith(3, "/some-example", { - cat: "meow", - a: 5, - }) - - expect(r0.data).toEqual({ someResponseKey: "fallback post response" }) - expect(r1.data).toEqual({ someResponseKey: "patched!" }) - expect(r2.data).toEqual({ - someResponseKey: "response for request with {b:10}", - }) - expect(r3.data).toEqual({ - someResponseKey: "response for request with {a:5}", - }) - }) - - test("Error codes reject", async () => { - setMockResponse.post("/some-example", "Bad request", { code: 400 }) - await expect(axios.post("/some-example", { a: 5 })).rejects.toEqual( - expect.objectContaining({ - response: { data: "Bad request", status: 400 }, - }), - ) - }) - - test("Errors if mock value is not set.", async () => { - const { consoleError } = allowConsoleErrors() - expect(consoleError).not.toHaveBeenCalled() - let error: Error | null = null - try { - await axios.post("/some-example", { dog: "woof" }) - } catch (err) { - error = err as Error - } - expect(error?.message).toBe("No response specified for post /some-example") - expect(consoleError).toHaveBeenCalledWith( - "No response specified for post /some-example", - ) - }) - - test("Manually resolving a response", async () => { - const responseBody = new ControlledPromise() - setMockResponse.get("/respond-when-i-say", responseBody) - const response = axios.get("/respond-when-i-say") - let responseStatus = "pending" - response.then(() => { - responseStatus = "resolved" - }) - - await Promise.resolve() // flush the event queue - expect(responseStatus).toBe("pending") // response is still pending - responseBody.resolve(37) - expect(await response).toEqual( - expect.objectContaining({ - data: 37, - status: 200, - }), - ) - expect(responseStatus).toBe("resolved") - }) -}) diff --git a/frontends/main/tsconfig.json b/frontends/main/tsconfig.json index ad5778a13f..31ab164154 100644 --- a/frontends/main/tsconfig.json +++ b/frontends/main/tsconfig.json @@ -18,10 +18,7 @@ } ], "paths": { - "@/*": ["./src/*"], - "@/test-utils": ["./test-utils"], - "api": ["../api/src"], - "api/*": ["../api/src/*"] + "@/*": ["./src/*"] }, "target": "ESNext", "types": [ @@ -39,5 +36,5 @@ ".next/types/**/*.ts", "../jest.jsdom.config" ], - "exclude": ["node_modules", "../mit-learn/**/*.tsx", "ol-test-utilities"] + "exclude": ["node_modules", "../mit-learn/**/*.tsx"] } diff --git a/frontends/ol-ckeditor/jest.config.ts b/frontends/ol-ckeditor/jest.config.ts index 9cc7a02b7a..0d65c15475 100644 --- a/frontends/ol-ckeditor/jest.config.ts +++ b/frontends/ol-ckeditor/jest.config.ts @@ -11,10 +11,13 @@ const config: Config.InitialOptions = { "|vanilla-colorful" + ")/)", ], + moduleNameMapper: { + "\\.svg$": "ol-test-utilities/filemocks/raw-svgmock.js", + "\\.(css|scss)$": "ol-test-utilities/filemocks/filemock.js", + }, globals: { APP_SETTINGS: { CKEDITOR_UPLOAD_URL: "https://meowmeow.com", - EMBEDLY_KEY: "embedly_key", MITOL_AXIOS_WITH_CREDENTIALS: false, MITOL_API_BASE_URL: "https://api.test.learn.mit.edu", }, diff --git a/frontends/ol-ckeditor/src/components/CkeditorDisplay.test.tsx b/frontends/ol-ckeditor/src/components/CkeditorDisplay.test.tsx index 5578658bd1..35365bf55c 100644 --- a/frontends/ol-ckeditor/src/components/CkeditorDisplay.test.tsx +++ b/frontends/ol-ckeditor/src/components/CkeditorDisplay.test.tsx @@ -17,9 +17,9 @@ describe("CkeditorDisplay", () => { `.trim() const expectedHtmlOut = `

Some text

- +

More text

- +

Some more text

`.trim() const view = render() diff --git a/frontends/ol-components/src/components/Button/Button.tsx b/frontends/ol-components/src/components/Button/Button.tsx index 753092aa36..15ac7a89a9 100644 --- a/frontends/ol-components/src/components/Button/Button.tsx +++ b/frontends/ol-components/src/components/Button/Button.tsx @@ -283,15 +283,18 @@ type ButtonLinkProps = ButtonStyleProps & rawAnchor?: boolean href: string } + const ButtonLink = ButtonStyled.withComponent( - ({ children, rawAnchor, ...props }: ButtonLinkProps) => { - const Component = rawAnchor ? "a" : Link - return ( - - {children} - - ) - }, + React.forwardRef( + ({ children, rawAnchor, ...props }: ButtonLinkProps, ref) => { + const Component = rawAnchor ? "a" : Link + return ( + + {children} + + ) + }, + ), ) ButtonLink.displayName = "ButtonLink" diff --git a/frontends/ol-components/src/components/EmbedlyCard/util.ts b/frontends/ol-components/src/components/EmbedlyCard/util.ts index 401896a75a..fd6acb55a8 100644 --- a/frontends/ol-components/src/components/EmbedlyCard/util.ts +++ b/frontends/ol-components/src/components/EmbedlyCard/util.ts @@ -82,9 +82,9 @@ const createStylesheet = (doc: Document, css: string) => { } const getEmbedlyKey = (): string | null => { - const key = APP_SETTINGS.EMBEDLY_KEY + const key = process.env.NEXT_PUBLIC_EMBEDLY_KEY if (typeof key === "string") return key - console.warn("APP_SETTINGS.EMBEDLY_KEY should be a string.") + console.warn("process.env.NEXT_PUBLIC_EMBEDLY_KEY should be a string.") return null } diff --git a/frontends/ol-components/src/components/Lists/ListItemLink.tsx b/frontends/ol-components/src/components/Lists/ListItemLink.tsx index cd493ddf1d..3da72dedfe 100644 --- a/frontends/ol-components/src/components/Lists/ListItemLink.tsx +++ b/frontends/ol-components/src/components/Lists/ListItemLink.tsx @@ -14,8 +14,8 @@ type ListItemLinkProps = ListItemButtonProps<"a"> * since the padding is applied to the link itself. */ const ListItemLink: React.FC = styled( - ({ href, ...props }: ListItemLinkProps) => ( - + ({ ...props }: ListItemLinkProps) => ( + ), )(() => [ { diff --git a/frontends/ol-test-utilities/src/filemocks/raw-svgmock.js b/frontends/ol-test-utilities/src/filemocks/raw-svgmock.js new file mode 100644 index 0000000000..045f45637e --- /dev/null +++ b/frontends/ol-test-utilities/src/filemocks/raw-svgmock.js @@ -0,0 +1,2 @@ +// Used by jest to mock SVG imports +module.exports = "" diff --git a/frontends/ol-test-utilities/src/filemocks/svgmock.js b/frontends/ol-test-utilities/src/filemocks/svgmock.js index 045f45637e..30f9ffbec6 100644 --- a/frontends/ol-test-utilities/src/filemocks/svgmock.js +++ b/frontends/ol-test-utilities/src/filemocks/svgmock.js @@ -1,2 +1,8 @@ // Used by jest to mock SVG imports -module.exports = "" +module.exports = { + src: "/_next/static/path/to/image.svg", + height: 100, + width: 200, + blurWidth: 0, + blurHeight: 0, +} diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts new file mode 100644 index 0000000000..d3c0e43baa --- /dev/null +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts @@ -0,0 +1,48 @@ +import { renderHook } from "@testing-library/react" +import { nextNavigationMocks } from "./nextNavigation" +import mockRouter from "next-router-mock" +import { createDynamicRouteParser } from "next-router-mock/dynamic-routes" + +const { usePathname, useSearchParams } = nextNavigationMocks + +describe("Mock Navigation", () => { + beforeAll(() => { + mockRouter.useParser( + createDynamicRouteParser([ + "/dynamic/[id]", + "/dynamic/[id]/[other]", + "/static/path", + ]), + ) + }) + + afterAll(() => { + mockRouter.useParser( + createDynamicRouteParser([ + // These paths should match those found in the `/pages` folder: + "/dynamic/[id]", + "/dynamic/[id]/[other]", + "/static/path", + ]), + ) + }) + + test("usePathname returns the current pathname", () => { + mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2") + const { result } = renderHook(() => usePathname()) + expect(result.current).toBe("/dynamic/bar") + }) + + test("useSearchParams returns the current search params", () => { + mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2&b=3") + const { result } = renderHook(() => useSearchParams()) + expect(result.current).toEqual(new URLSearchParams("a=1&b=2&b=3")) + }) + + test("useParams returns the current params", () => { + mockRouter.useParser(createDynamicRouteParser(["x"])) + mockRouter.setCurrentUrl("/dynamic/bar/baz?a=1&b=2") + const { result } = renderHook(() => nextNavigationMocks.useParams()) + expect(result.current).toEqual({ id: "bar", other: "baz" }) + }) +}) diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts index ca81cf316b..671ef498b1 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts @@ -7,6 +7,20 @@ * See https://github.com/scottrippey/next-router-mock/issues */ import * as mocks from "next-router-mock" +import { createDynamicRouteParser } from "next-router-mock/dynamic-routes" + +const getParams = (template: string, pathname: string) => { + const route = template.split("/") + const path = pathname.split("/") + + return route.reduce((acc, part, i) => { + if (part.startsWith("[") && part.endsWith("]")) { + const key = part.slice(1, -1) + return { ...acc, [key]: path[i] } + } + return acc + }, {}) +} export const nextNavigationMocks = { ...mocks, @@ -16,14 +30,29 @@ export const nextNavigationMocks = { }), usePathname: () => { const router = nextNavigationMocks.useRouter() - return router.asPath + /** + * next-router-mock is designed for Pages router. We are adapting for App + * router. The return value is a little different for the two: + * pages router: /dynamic/[id] + * app router: /dynamic/123 + * + * I.e., pages router returns the route "template". + * App router returns similar to window.location.pathname + */ + const url = new URL(router.asPath, "http://localhost") + return url.pathname }, useSearchParams: () => { const router = nextNavigationMocks.useRouter() - const path = router.query - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return new URLSearchParams(path as any) + const url = new URL(router.asPath, "http://localhost") + return url.searchParams + }, + useParams: () => { + const router = nextNavigationMocks.useRouter() + const url = new URL(router.asPath, "http://localhost") + return getParams(router.pathname, url.pathname) }, } + const mockRouter = nextNavigationMocks.memoryRouter -export { mockRouter } +export { mockRouter, createDynamicRouteParser } diff --git a/yarn.lock b/yarn.lock index 863a219b51..f254c80853 100644 --- a/yarn.lock +++ b/yarn.lock @@ -5843,17 +5843,7 @@ __metadata: languageName: node linkType: hard -"@types/react@npm:*, @types/react@npm:^16.8.0 || ^17.0.0 || ^18.0.0, @types/react@npm:^18.0.26": - version: 18.3.3 - resolution: "@types/react@npm:18.3.3" - dependencies: - "@types/prop-types": "npm:*" - csstype: "npm:^3.0.2" - checksum: 10/68e203b7f1f91d6cf21f33fc7af9d6d228035a26c83f514981e54aa3da695d0ec6af10c277c6336de1dd76c4adbe9563f3a21f80c4462000f41e5f370b46e96c - languageName: node - linkType: hard - -"@types/react@npm:^18.3.5": +"@types/react@npm:*, @types/react@npm:^16.8.0 || ^17.0.0 || ^18.0.0, @types/react@npm:^18.0.26, @types/react@npm:^18.3.5": version: 18.3.5 resolution: "@types/react@npm:18.3.5" dependencies: