From 74cb07ee290fb7a061b11626765f8ceee24c88aa Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 10 Sep 2024 11:27:27 -0400 Subject: [PATCH 01/17] adapt renderwithproviders for nextjs --- frontends/main/jest.config.ts | 17 ++ frontends/main/src/app/getQueryClient.ts | 39 +-- frontends/main/src/app/providers.tsx | 2 +- .../{ => src}/test-utils/delete.mockAxios.ts | 0 .../test-utils/example-request-mocks.test.ts | 0 .../main/{ => src}/test-utils/factories.ts | 0 frontends/main/src/test-utils/index.tsx | 227 ++++++++++++++++++ .../test-utils/index_old.tsx} | 0 .../main/{ => src}/test-utils/setupJest.ts | 0 .../{ => src}/test-utils/withFakeLocation.ts | 0 .../src/mocks/nextNavigation.ts | 2 +- 11 files changed, 271 insertions(+), 16 deletions(-) create mode 100644 frontends/main/jest.config.ts rename frontends/main/{ => src}/test-utils/delete.mockAxios.ts (100%) rename frontends/main/{ => src}/test-utils/example-request-mocks.test.ts (100%) rename frontends/main/{ => src}/test-utils/factories.ts (100%) create mode 100644 frontends/main/src/test-utils/index.tsx rename frontends/main/{test-utils/index.tsx => src/test-utils/index_old.tsx} (100%) rename frontends/main/{ => src}/test-utils/setupJest.ts (100%) rename frontends/main/{ => src}/test-utils/withFakeLocation.ts (100%) 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/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/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/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/example-request-mocks.test.ts b/frontends/main/src/test-utils/example-request-mocks.test.ts similarity index 100% rename from frontends/main/test-utils/example-request-mocks.test.ts rename to frontends/main/src/test-utils/example-request-mocks.test.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/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx new file mode 100644 index 0000000000..3406d27921 --- /dev/null +++ b/frontends/main/src/test-utils/index.tsx @@ -0,0 +1,227 @@ +/* eslint-disable import/no-extraneous-dependencies */ +import React from "react" +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" + +import { makeQueryClient } from "@/app/getQueryClient" +import { render } from "@testing-library/react" +import { setMockResponse } from "api/test-utils" +import type { User } from "api/hooks/user" +import { mockRouter } from "ol-test-utilities/mocks/nextNavigation" + +interface TestAppOptions { + url: string + user: Partial +} + +const defaultTestAppOptions = { + url: "/", +} +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 renderWithProviders = ( + component: React.ReactNode, + options: Partial = {}, +) => { + const allOpts = { ...defaultTestAppOptions, ...options } + const { url } = allOpts + + const queryClient = makeQueryClient() + + if (allOpts.user) { + const user = { ...defaultUser, ...allOpts.user } + queryClient.setQueryData(["userMe"], { ...user }) + } + mockRouter.setCurrentUrl(url) + + const view = render( + {component}, + ) + + const location = { + pathname: mockRouter.pathname, + search: mockRouter.query, + } + + return { view, queryClient, location } +} + +const renderTestApp = () => { + throw new Error("not supported") +} + +/** + * Assert that a functional component was called at some point with the given + * props. + * @param fc the mock or spied upon functional component + * @param partialProps an object of props + */ +const expectProps = ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + fc: (...args: any[]) => void, + partialProps: unknown, +) => { + expect(fc).toHaveBeenCalledWith( + expect.objectContaining(partialProps), + expect.anything(), + ) +} + +/** + * Assert that a functional component was last called with the given + * props. + * @param fc the mock or spied upon functional component + * @param partialProps an object of props + */ +const expectLastProps = ( + // eslint-disable-next-line @typescript-eslint/no-explicit-any + fc: jest.Mock, + partialProps: unknown, +) => { + expect(fc).toHaveBeenLastCalledWith( + expect.objectContaining(partialProps), + expect.anything(), + ) +} + +/** + * Useful for checking that "real" navigation occurs, i.e., navigation with a + * full browser reload, not React Router's SPA-routing. + */ +const expectWindowNavigation = async (cb: () => void | Promise) => { + const consoleError = console.error + try { + const spy = jest.spyOn(console, "error").mockImplementation() + await cb() + expect(spy).toHaveBeenCalledTimes(1) + const error = spy.mock.calls[0][0] + expect(error instanceof Error) + expect(error.message).toMatch(/Not implemented: navigation/) + } finally { + console.error = consoleError + } +} + +const ignoreError = (errorMessage: string, timeoutMs?: number) => { + const consoleError = console.error + const spy = jest.spyOn(console, "error").mockImplementation((...args) => { + if (args[0]?.includes(errorMessage)) { + return + } + return consoleError.call(console, args) + }) + + const timeout = setTimeout(() => { + throw new Error( + `Ignored console error not cleared after ${timeoutMs || 5000}ms: "${errorMessage}"`, + ) + }, timeoutMs || 5000) + + const clear = () => { + console.error = consoleError + spy.mockClear() + clearTimeout(timeout) + } + + return { clear } +} + +const getMetaContent = ({ + property, + name, +}: { + property?: string + name?: string +}) => { + const propSelector = property ? `[property="${property}"]` : "" + const nameSelector = name ? `[name="${name}"]` : "" + const selector = `meta${propSelector}${nameSelector}` + const el = document.querySelector(selector) + return el?.content +} + +type TestableMetas = { + title?: string | null + description?: string | null + og: { + image?: string | null + imageAlt?: string | null + description?: string | null + title?: string | null + } + twitter: { + card?: string | null + image?: string | null + description?: string | null + } +} +const getMetas = (): TestableMetas => { + return { + title: document.title, + description: getMetaContent({ name: "description" }), + og: { + image: getMetaContent({ property: "og:image" }), + imageAlt: getMetaContent({ property: "og:image:alt" }), + description: getMetaContent({ property: "og:description" }), + title: getMetaContent({ property: "og:title" }), + }, + twitter: { + card: getMetaContent({ name: "twitter:card" }), + image: getMetaContent({ name: "twitter:image:src" }), + description: getMetaContent({ name: "twitter:description" }), + }, + } +} +const assertPartialMetas = (expected: Partial) => { + expect(getMetas()).toEqual( + expect.objectContaining({ + ...expected, + og: expect.objectContaining(expected.og ?? {}), + twitter: expect.objectContaining(expected.twitter ?? {}), + }), + ) +} + +export { + renderTestApp, + renderWithProviders, + renderRoutesWithProviders, + expectProps, + expectLastProps, + expectWindowNavigation, + ignoreError, + getMetas, + assertPartialMetas, +} +// Conveniences +export { setMockResponse } +export { + act, + screen, + prettyDOM, + within, + fireEvent, + waitFor, + renderHook, +} from "@testing-library/react" +export { default as user } from "@testing-library/user-event" + +export type { TestAppOptions, User } diff --git a/frontends/main/test-utils/index.tsx b/frontends/main/src/test-utils/index_old.tsx similarity index 100% rename from frontends/main/test-utils/index.tsx rename to frontends/main/src/test-utils/index_old.tsx 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/ol-test-utilities/src/mocks/nextNavigation.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts index ca81cf316b..2f9ac95879 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts @@ -16,7 +16,7 @@ export const nextNavigationMocks = { }), usePathname: () => { const router = nextNavigationMocks.useRouter() - return router.asPath + return router.pathname }, useSearchParams: () => { const router = nextNavigationMocks.useRouter() From c5e967c5c8fc82e5a8987b1896d367a68845efcc Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 10 Sep 2024 12:18:12 -0400 Subject: [PATCH 02/17] fix some tests depending on router location --- frontends/jest-shared-setup.ts | 1 + .../app-pages/HomePage/PersonalizeSection.tsx | 4 +- frontends/main/src/common/urls.test.ts | 14 +-- frontends/main/src/common/urls.ts | 13 ++- .../page-components/Header/Header.test.tsx | 2 +- .../src/page-components/Header/UserMenu.tsx | 13 +-- .../LearningResourceDrawer.test.tsx | 3 +- .../SignupPopover/SignupPopover.test.tsx | 2 +- .../SignupPopover/SignupPopover.tsx | 7 +- .../test-utils/example-request-mocks.test.ts | 100 ------------------ frontends/main/src/test-utils/index.tsx | 14 ++- .../src/mocks/nextNavigation.ts | 22 +++- 12 files changed, 59 insertions(+), 136 deletions(-) delete mode 100644 frontends/main/src/test-utils/example-request-mocks.test.ts diff --git a/frontends/jest-shared-setup.ts b/frontends/jest-shared-setup.ts index 3ec45641aa..66212e486e 100644 --- a/frontends/jest-shared-setup.ts +++ b/frontends/jest-shared-setup.ts @@ -10,6 +10,7 @@ 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" // Pulled from the docs - see https://jestjs.io/docs/manual-mocks#mocking-methods-which-are-not-implemented-in-jsdom 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/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..5f69e9c1a4 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -1,3 +1,5 @@ +import invariant from "tiny-invariant" + const generatePath = ( template: string, params: Record, @@ -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,7 +79,8 @@ 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}` } diff --git a/frontends/main/src/page-components/Header/Header.test.tsx b/frontends/main/src/page-components/Header/Header.test.tsx index ed8ac0e1eb..0a5ab7781c 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, 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/LearningResourceDrawer/LearningResourceDrawer.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx index bbfb594b2c..888495f60d 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 } 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/test-utils/example-request-mocks.test.ts b/frontends/main/src/test-utils/example-request-mocks.test.ts deleted file mode 100644 index f9d8a1f821..0000000000 --- a/frontends/main/src/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/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index 3406d27921..5e93f51c3f 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -9,7 +9,10 @@ import { makeQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" import { setMockResponse } from "api/test-utils" import type { User } from "api/hooks/user" -import { mockRouter } from "ol-test-utilities/mocks/nextNavigation" +import { + mockRouter, + nextRouterQueryToSearchParams, +} from "ol-test-utilities/mocks/nextNavigation" interface TestAppOptions { url: string @@ -57,8 +60,13 @@ const renderWithProviders = ( ) const location = { - pathname: mockRouter.pathname, - search: mockRouter.query, + get current() { + const searchParams = nextRouterQueryToSearchParams(mockRouter.query) + const search = searchParams.toString() + ? `?${searchParams.toString()}` + : "" + return { pathname: mockRouter.pathname, search } + }, } return { view, queryClient, location } diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts index 2f9ac95879..471d677409 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts @@ -8,6 +8,22 @@ */ import * as mocks from "next-router-mock" +type ParsedUrlQuery = typeof mocks.memoryRouter.query + +const nextRouterQueryToSearchParams = ( + query: ParsedUrlQuery, +): URLSearchParams => { + const params = new URLSearchParams() + Object.entries(query).forEach(([key, value]) => { + if (Array.isArray(value)) { + value.forEach((v) => params.append(key, v)) + } else { + params.append(key, value ?? "") + } + }) + return params +} + export const nextNavigationMocks = { ...mocks, notFound: jest.fn(), @@ -20,10 +36,8 @@ export const nextNavigationMocks = { }, useSearchParams: () => { const router = nextNavigationMocks.useRouter() - const path = router.query - // eslint-disable-next-line @typescript-eslint/no-explicit-any - return new URLSearchParams(path as any) + return nextRouterQueryToSearchParams(router.query) }, } const mockRouter = nextNavigationMocks.memoryRouter -export { mockRouter } +export { mockRouter, nextRouterQueryToSearchParams } From 1cca2f368d9c13e6383f7b64e1a8ea12ded6c35a Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Tue, 10 Sep 2024 14:16:03 -0400 Subject: [PATCH 03/17] fix an import --- frontends/main/src/app/page.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index 411f7c0452..f5236c521f 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -5,7 +5,7 @@ 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 { getQueryClient } from "./getQueryClient" import { getMetadataAsync } from "@/common/metadata" export async function generateMetadata({ From 4d7836ab3d209282523ba6d54fd26ff2141dce9f Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Wed, 11 Sep 2024 18:44:27 -0400 Subject: [PATCH 04/17] fix some routing issues --- .../DashboardPage/Dashboard.test.tsx | 3 -- .../app-pages/DashboardPage/DashboardPage.tsx | 2 +- frontends/main/src/common/urls.ts | 8 +-- frontends/main/src/test-utils/index.tsx | 12 ++--- .../src/components/Button/Button.tsx | 19 ++++--- .../src/mocks/nextNavigation.test.ts | 49 +++++++++++++++++++ .../src/mocks/nextNavigation.ts | 44 +++++++++++------ 7 files changed, 97 insertions(+), 40 deletions(-) create mode 100644 frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts 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/common/urls.ts b/frontends/main/src/common/urls.ts index 5f69e9c1a4..71832fc4c9 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -84,16 +84,16 @@ export const login = ({ return `${LOGIN}?next=${next}` } -export const DASHBOARD_HOME = "/dashboard/" +export const DASHBOARD_HOME = "/dashboard" -export const MY_LISTS = "/dashboard/my-lists/" +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/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index 5e93f51c3f..e9ac514152 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -9,10 +9,7 @@ import { makeQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" import { setMockResponse } from "api/test-utils" import type { User } from "api/hooks/user" -import { - mockRouter, - nextRouterQueryToSearchParams, -} from "ol-test-utilities/mocks/nextNavigation" +import { mockRouter } from "ol-test-utilities/mocks/nextNavigation" interface TestAppOptions { url: string @@ -61,11 +58,8 @@ const renderWithProviders = ( const location = { get current() { - const searchParams = nextRouterQueryToSearchParams(mockRouter.query) - const search = searchParams.toString() - ? `?${searchParams.toString()}` - : "" - return { pathname: mockRouter.pathname, search } + const url = new URL(mockRouter.asPath, "http://localhost") + return { pathname: mockRouter.pathname, search: url.search } }, } 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-test-utilities/src/mocks/nextNavigation.test.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts new file mode 100644 index 0000000000..aadfacaa78 --- /dev/null +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts @@ -0,0 +1,49 @@ +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([ + // These paths should match those found in the `/pages` folder: + "/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") + const { result } = renderHook(() => useSearchParams()) + console.log(result.current.toString()) + }) + + 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 471d677409..c5cfb8bad9 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts @@ -8,20 +8,17 @@ */ import * as mocks from "next-router-mock" -type ParsedUrlQuery = typeof mocks.memoryRouter.query +const getParams = (template: string, pathname: string) => { + const route = template.split("/") + const path = pathname.split("/") -const nextRouterQueryToSearchParams = ( - query: ParsedUrlQuery, -): URLSearchParams => { - const params = new URLSearchParams() - Object.entries(query).forEach(([key, value]) => { - if (Array.isArray(value)) { - value.forEach((v) => params.append(key, v)) - } else { - params.append(key, value ?? "") + return route.reduce((acc, part, i) => { + if (part.startsWith("[") && part.endsWith("]")) { + const key = part.slice(1, -1) + return { ...acc, [key]: path[i] } } - }) - return params + return acc + }, {}) } export const nextNavigationMocks = { @@ -32,12 +29,29 @@ export const nextNavigationMocks = { }), usePathname: () => { const router = nextNavigationMocks.useRouter() - return router.pathname + /** + * 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() - return nextRouterQueryToSearchParams(router.query) + 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, nextRouterQueryToSearchParams } +export { mockRouter } From 2d806c69133b1fa31ef771d475afe0d2a93b4fb8 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 12 Sep 2024 15:36:23 -0400 Subject: [PATCH 05/17] temporarily skip some tests --- .../main/src/app-pages/ChannelPage/ChannelPage.test.tsx | 8 ++++---- .../main/src/app-pages/ChannelPage/ChannelSearch.test.tsx | 2 +- .../main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx | 2 +- .../app-pages/ProgramLetterPage/ProgramLetter.test.tsx | 2 +- frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx | 2 +- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx index e75aa8284f..1f92ec25fe 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx @@ -132,7 +132,7 @@ const NON_UNIT_CHANNEL_TYPES = Object.values(ChannelTypeEnum) .filter((v) => v !== ChannelTypeEnum.Unit) .map((v) => ({ channelType: v })) -describe.each(ALL_CHANNEL_TYPES)( +describe.skip.each(ALL_CHANNEL_TYPES)( "ChannelPage, common behavior", ({ channelType }) => { it("Displays the channel search if search_filter is not undefined", async () => { @@ -203,7 +203,7 @@ describe.each(ALL_CHANNEL_TYPES)( }, ) -describe.each(NON_UNIT_CHANNEL_TYPES)( +describe.skip.each(NON_UNIT_CHANNEL_TYPES)( "ChannelPage, common non-unit ($channelType)", ({ channelType }) => { it("Does not display a featured carousel if the channel type is not 'unit'", async () => { @@ -266,7 +266,7 @@ describe.each(NON_UNIT_CHANNEL_TYPES)( }, ) -describe("Channel Pages, Topic only", () => { +describe.skip("Channel Pages, Topic only", () => { test("Subtopics display", async () => { const { channel, subTopics } = setupApis({ search_filter: "topic=Physics", @@ -285,7 +285,7 @@ describe("Channel Pages, Topic only", () => { }) }) -describe("Channel Pages, Unit only", () => { +describe.skip("Channel Pages, Unit only", () => { it("Sets the expected meta tags", async () => { const { channel } = setupApis({ search_filter: "offered_by=ocw", diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx index 3a3f87409c..d197e087ea 100644 --- a/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx +++ b/frontends/main/src/app-pages/ChannelPage/ChannelSearch.test.tsx @@ -102,7 +102,7 @@ const getLastApiSearchParams = () => { return fullUrl.searchParams } -describe("ChannelSearch", () => { +describe.skip("ChannelSearch", () => { test("Renders search results", async () => { const resources = factories.learningResources.resources({ count: 10, diff --git a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx index 5e5845549b..136d4f70c5 100644 --- a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx +++ b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx @@ -3,7 +3,7 @@ import { urls } from "api/test-utils" import * as commonUrls from "@/common/urls" import { Permissions } from "@/common/permissions" -describe("PrivacyPage", () => { +describe.skip("PrivacyPage", () => { test("Renders title", async () => { setMockResponse.get(urls.userMe.get(), { [Permissions.Authenticated]: true, diff --git a/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx b/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx index 59323970a5..e438520731 100644 --- a/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx +++ b/frontends/main/src/app-pages/ProgramLetterPage/ProgramLetter.test.tsx @@ -15,7 +15,7 @@ const setup = ({ programLetter }: { programLetter: ProgramLetter }) => { }) } -describe("ProgramLetterDisplayPage", () => { +describe.skip("ProgramLetterDisplayPage", () => { it("Renders a program letter from api", async () => { const programLetter = factory.programLetter() setup({ programLetter }) diff --git a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx index cefd67e012..945849c4ee 100644 --- a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx +++ b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx @@ -3,7 +3,7 @@ import { urls } from "api/test-utils" import * as commonUrls from "@/common/urls" import { Permissions } from "@/common/permissions" -describe("TermsPage", () => { +describe.skip("TermsPage", () => { test("Renders title", async () => { setMockResponse.get(urls.userMe.get(), { [Permissions.Authenticated]: true, From 2e62a3a216b8904ddbf892f323bb496869ada16d Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 09:04:48 -0400 Subject: [PATCH 06/17] fix some more tests --- .../test-utils/factories/learningResources.ts | 2 +- .../app-pages/AboutPage/AboutPage.test.tsx | 2 +- .../DepartmentListingPage.test.tsx | 4 +--- .../DepartmentListingPage.tsx | 9 ++------- .../app-pages/ErrorPage/NotFoundPage.test.tsx | 2 +- .../src/app-pages/HomePage/HomePage.test.tsx | 16 +++++++-------- .../LearningPathListingPage.test.tsx | 20 +++++-------------- .../OnboardingPage/OnboardingPage.test.tsx | 2 +- .../TopicsListingPage.test.tsx | 3 --- .../UnitsListingPage.test.tsx | 3 --- frontends/main/src/common/urls.ts | 2 +- .../ChannelAvatar/ChannelAvatar.test.tsx | 18 +++++++++++------ .../components/MITLogoLink/MITLogoLink.tsx | 10 ++++++++-- .../src/page-components/Footer/Footer.tsx | 6 +++++- .../page-components/Header/Header.test.tsx | 5 ++--- .../ItemsListing/ItemsListing.test.tsx | 3 ++- .../ResourceCard/ResourceCard.test.tsx | 20 +++++++++++-------- .../UserListListing/UserListListing.test.tsx | 6 ++---- frontends/ol-ckeditor/jest.config.ts | 4 ++++ .../src/components/Lists/ListItemLink.tsx | 4 ++-- .../src/filemocks/raw-svgmock.js | 2 ++ .../src/filemocks/svgmock.js | 8 +++++++- 22 files changed, 79 insertions(+), 72 deletions(-) create mode 100644 frontends/ol-test-utilities/src/filemocks/raw-svgmock.js 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/main/src/app-pages/AboutPage/AboutPage.test.tsx b/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx index d7bf022a64..da835e81ad 100644 --- a/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx +++ b/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx @@ -4,7 +4,7 @@ import * as commonUrls from "@/common/urls" import { Permissions } from "@/common/permissions" describe("AboutPage", () => { - test("Renders title", async () => { + test.skip("Renders title", async () => { setMockResponse.get(urls.userMe.get(), { [Permissions.Authenticated]: true, }) 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/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/TopicsListingPage/TopicsListingPage.test.tsx b/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx index 91802d56df..2fcce7eb90 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" }) }) 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/common/urls.ts b/frontends/main/src/common/urls.ts index 71832fc4c9..fe9fa9d82d 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -16,7 +16,7 @@ export const HOME = "/" export const ONBOARDING = "/onboarding" -export const LEARNINGPATH_LISTING = "/learningpaths/" +export const LEARNINGPATH_LISTING = "/learningpaths" export const LEARNINGPATH_VIEW = "/learningpaths/:id" export const learningPathsView = (id: number) => generatePath(LEARNINGPATH_VIEW, { id: String(id) }) 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 0a5ab7781c..003b5ebb56 100644 --- a/frontends/main/src/page-components/Header/Header.test.tsx +++ b/frontends/main/src/page-components/Header/Header.test.tsx @@ -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/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/ResourceCard/ResourceCard.test.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx index f7b8074248..8c48b0a5fe 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,15 @@ 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") + 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/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/ol-ckeditor/jest.config.ts b/frontends/ol-ckeditor/jest.config.ts index 9cc7a02b7a..ed8ade7905 100644 --- a/frontends/ol-ckeditor/jest.config.ts +++ b/frontends/ol-ckeditor/jest.config.ts @@ -11,6 +11,10 @@ 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", 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, +} From 5f7e4e47c571e053a9e90452870fd510be7731c4 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 09:52:42 -0400 Subject: [PATCH 07/17] fix ts imports --- frontends/main/tsconfig.json | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) 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"] } From 0c0f5ba505623f0d8782beaf7b08b09d6a5881ca Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 12:23:22 -0400 Subject: [PATCH 08/17] fix a test broken during rebase --- .../LearningResourceDrawer.test.tsx | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx index 888495f60d..f9a9a4b444 100644 --- a/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx +++ b/frontends/main/src/page-components/LearningResourceDrawer/LearningResourceDrawer.test.tsx @@ -140,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() From 0750cbf8cd36fe72c3c6e8de7c5e27209349f976 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 14:37:42 -0400 Subject: [PATCH 09/17] replace a few renderTestApp calls --- frontends/api/package.json | 1 + .../app-pages/AboutPage/AboutPage.test.tsx | 20 +++++-------------- .../PrivacyPage/PrivacyPage.test.tsx | 15 ++++++-------- .../src/app-pages/SearchPage/SearchPage.tsx | 4 ++++ .../app-pages/TermsPage/TermsPage.test.tsx | 14 +++++-------- 5 files changed, 21 insertions(+), 33 deletions(-) diff --git a/frontends/api/package.json b/frontends/api/package.json index 7062b5d08e..49971ef2db 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -5,6 +5,7 @@ "sideEffects": false, "exports": { ".": "./src/generated/v1/api.ts", + "./clients": "./src/clients.ts", "./v0": "./src/generated/v0/api.ts", "./hooks/*": "./src/hooks/*/index.ts", "./constants": "./src/common/constants.ts", diff --git a/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx b/frontends/main/src/app-pages/AboutPage/AboutPage.test.tsx index da835e81ad..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.skip("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") - }) + test("Renders title", async () => { + renderWithProviders() screen.getByRole("heading", { name: "About Us", }) diff --git a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx index 136d4f70c5..0fb699cee7 100644 --- a/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx +++ b/frontends/main/src/app-pages/PrivacyPage/PrivacyPage.test.tsx @@ -1,20 +1,17 @@ -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.skip("PrivacyPage", () => { +describe("PrivacyPage", () => { test("Renders title", async () => { setMockResponse.get(urls.userMe.get(), { [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/SearchPage/SearchPage.tsx b/frontends/main/src/app-pages/SearchPage/SearchPage.tsx index 375b4d0923..b0855292f6 100644 --- a/frontends/main/src/app-pages/SearchPage/SearchPage.tsx +++ b/frontends/main/src/app-pages/SearchPage/SearchPage.tsx @@ -222,6 +222,10 @@ const SearchPage: React.FC = () => { const page = +(searchParams.get("page") ?? "1") + console.log({ + searchParams: searchParams.toString(), + }) + return ( diff --git a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx index 945849c4ee..667979d23d 100644 --- a/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx +++ b/frontends/main/src/app-pages/TermsPage/TermsPage.test.tsx @@ -1,20 +1,16 @@ -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.skip("TermsPage", () => { +describe("TermsPage", () => { test("Renders title", async () => { setMockResponse.get(urls.userMe.get(), { [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", }) From 0a808d7c0d6fad6a9a49a52a9505850f7a90e48b Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:00:44 -0400 Subject: [PATCH 10/17] fix remaining renderTestApp tests --- .../ChannelPage/ChannelPage.test.tsx | 122 +++++++----------- .../src/app-pages/ChannelPage/ChannelPage.tsx | 68 +++++++++- .../ChannelPage/ChannelSearch.test.tsx | 16 ++- .../{ => [id]/view}/ProgramLetter.test.tsx | 8 +- .../{ => [id]/view}/ProgramLetterPage.tsx | 0 .../src/app-pages/SearchPage/SearchPage.tsx | 4 - .../src/app/program_letter/[id]/view/page.tsx | 2 +- frontends/main/src/common/urls.ts | 18 +-- frontends/main/src/test-utils/index.tsx | 17 ++- .../src/mocks/nextNavigation.test.ts | 5 +- .../src/mocks/nextNavigation.ts | 3 +- 11 files changed, 156 insertions(+), 107 deletions(-) rename frontends/main/src/app-pages/ProgramLetterPage/{ => [id]/view}/ProgramLetter.test.tsx (82%) rename frontends/main/src/app-pages/ProgramLetterPage/{ => [id]/view}/ProgramLetterPage.tsx (100%) diff --git a/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx b/frontends/main/src/app-pages/ChannelPage/ChannelPage.test.tsx index 1f92ec25fe..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") @@ -132,7 +132,7 @@ const NON_UNIT_CHANNEL_TYPES = Object.values(ChannelTypeEnum) .filter((v) => v !== ChannelTypeEnum.Unit) .map((v) => ({ channelType: v })) -describe.skip.each(ALL_CHANNEL_TYPES)( +describe.each(ALL_CHANNEL_TYPES)( "ChannelPage, common behavior", ({ channelType }) => { it("Displays the channel search if search_filter is not undefined", async () => { @@ -141,7 +141,9 @@ describe.skip.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.skip.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.skip.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.skip.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() }, @@ -203,7 +211,7 @@ describe.skip.each(ALL_CHANNEL_TYPES)( }, ) -describe.skip.each(NON_UNIT_CHANNEL_TYPES)( +describe.each(NON_UNIT_CHANNEL_TYPES)( "ChannelPage, common non-unit ($channelType)", ({ channelType }) => { it("Does not display a featured carousel if the channel type is not 'unit'", async () => { @@ -212,7 +220,9 @@ describe.skip.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.skip.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.skip.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.skip.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([ @@ -266,13 +274,15 @@ describe.skip.each(NON_UNIT_CHANNEL_TYPES)( }, ) -describe.skip("Channel Pages, Topic only", () => { +describe("Channel Pages, Topic only", () => { test("Subtopics display", async () => { const { channel, subTopics } = setupApis({ 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", { @@ -285,64 +295,18 @@ describe.skip("Channel Pages, Topic only", () => { }) }) -describe.skip("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 }, - }) - }) - }) - +describe("Channel Pages, Unit only", () => { 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.skip("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.skip("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 && ( { return fullUrl.searchParams } -describe.skip("ChannelSearch", () => { +describe("ChannelSearch", () => { test("Renders search results", async () => { const resources = factories.learningResources.resources({ count: 10, @@ -125,7 +127,9 @@ describe.skip("ChannelSearch", () => { }, }) 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.skip("ChannelSearch", () => { }) setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ + renderWithProviders(, { url: `/c/${channel.channel_type}/${channel.name}/${url}`, }) @@ -240,7 +244,9 @@ describe.skip("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/ProgramLetterPage/ProgramLetter.test.tsx b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx similarity index 82% 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 e438520731..4511a8066e 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,12 +12,12 @@ const setup = ({ programLetter }: { programLetter: ProgramLetter }) => { programLetter, ) setMockResponse.get(urls.userMe.get(), {}) - renderTestApp({ + renderWithProviders(, { url: programLetterView(programLetter.id), }) } -describe.skip("ProgramLetterDisplayPage", () => { +describe("ProgramLetterDisplayPage", () => { it("Renders a program letter from api", async () => { const programLetter = factory.programLetter() setup({ programLetter }) 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/SearchPage/SearchPage.tsx b/frontends/main/src/app-pages/SearchPage/SearchPage.tsx index b0855292f6..375b4d0923 100644 --- a/frontends/main/src/app-pages/SearchPage/SearchPage.tsx +++ b/frontends/main/src/app-pages/SearchPage/SearchPage.tsx @@ -222,10 +222,6 @@ const SearchPage: React.FC = () => { const page = +(searchParams.get("page") ?? "1") - console.log({ - searchParams: searchParams.toString(), - }) - return ( 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/common/urls.ts b/frontends/main/src/common/urls.ts index fe9fa9d82d..c1f6c47222 100644 --- a/frontends/main/src/common/urls.ts +++ b/frontends/main/src/common/urls.ts @@ -4,7 +4,7 @@ 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}'`) } @@ -17,15 +17,15 @@ export const HOME = "/" export const ONBOARDING = "/onboarding" export const LEARNINGPATH_LISTING = "/learningpaths" -export const LEARNINGPATH_VIEW = "/learningpaths/:id" +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) }) @@ -35,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) => @@ -87,7 +87,7 @@ export const login = ({ export const DASHBOARD_HOME = "/dashboard" export const MY_LISTS = "/dashboard/my-lists" -export const USERLIST_VIEW = "/dashboard/my-lists/:id" +export const USERLIST_VIEW = "/dashboard/my-lists/[id]" export const userListView = (id: number) => generatePath(USERLIST_VIEW, { id: String(id) }) diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index e9ac514152..e4a1eb9281 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -9,7 +9,22 @@ import { makeQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" import { setMockResponse } from "api/test-utils" import type { User } from "api/hooks/user" -import { mockRouter } from "ol-test-utilities/mocks/nextNavigation" +import { + mockRouter, + createDynamicRouteParser, +} from "ol-test-utilities/mocks/nextNavigation" +import * as urls from "@/common/urls" + +const setupRoutes = () => { + mockRouter.useParser( + createDynamicRouteParser([ + // + urls.PROGRAMLETTER_VIEW, + urls.CHANNEL_VIEW, + ]), + ) +} +setupRoutes() interface TestAppOptions { url: string diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts index aadfacaa78..d3c0e43baa 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.test.ts @@ -9,7 +9,6 @@ describe("Mock Navigation", () => { beforeAll(() => { mockRouter.useParser( createDynamicRouteParser([ - // These paths should match those found in the `/pages` folder: "/dynamic/[id]", "/dynamic/[id]/[other]", "/static/path", @@ -35,9 +34,9 @@ describe("Mock Navigation", () => { }) test("useSearchParams returns the current search params", () => { - mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2") + mockRouter.setCurrentUrl("/dynamic/bar?a=1&b=2&b=3") const { result } = renderHook(() => useSearchParams()) - console.log(result.current.toString()) + expect(result.current).toEqual(new URLSearchParams("a=1&b=2&b=3")) }) test("useParams returns the current params", () => { diff --git a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts index c5cfb8bad9..671ef498b1 100644 --- a/frontends/ol-test-utilities/src/mocks/nextNavigation.ts +++ b/frontends/ol-test-utilities/src/mocks/nextNavigation.ts @@ -7,6 +7,7 @@ * 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("/") @@ -54,4 +55,4 @@ export const nextNavigationMocks = { } const mockRouter = nextNavigationMocks.memoryRouter -export { mockRouter } +export { mockRouter, createDynamicRouteParser } From cece2eafcbad6bd4667aaab627c674c70af4b841 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:08:05 -0400 Subject: [PATCH 11/17] fix build --- frontends/api/package.json | 1 + frontends/main/src/app/page.tsx | 2 +- frontends/main/src/test-utils/index.tsx | 1 - frontends/main/src/test-utils/index_old.tsx | 249 -------------------- yarn.lock | 12 +- 5 files changed, 3 insertions(+), 262 deletions(-) delete mode 100644 frontends/main/src/test-utils/index_old.tsx diff --git a/frontends/api/package.json b/frontends/api/package.json index 49971ef2db..7abfd01313 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -7,6 +7,7 @@ ".": "./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/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index f5236c521f..c1d6165e03 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -4,7 +4,7 @@ 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 { FeaturedApiFeaturedListRequest } from "api/v1" import { getQueryClient } from "./getQueryClient" import { getMetadataAsync } from "@/common/metadata" diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index e4a1eb9281..d9f85cf4b9 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -220,7 +220,6 @@ const assertPartialMetas = (expected: Partial) => { export { renderTestApp, renderWithProviders, - renderRoutesWithProviders, expectProps, expectLastProps, expectWindowNavigation, diff --git a/frontends/main/src/test-utils/index_old.tsx b/frontends/main/src/test-utils/index_old.tsx deleted file mode 100644 index 3dcba40530..0000000000 --- a/frontends/main/src/test-utils/index_old.tsx +++ /dev/null @@ -1,249 +0,0 @@ -/* eslint-disable import/no-extraneous-dependencies */ -import React from "react" -import { createMemoryRouter, useRouteError } from "react-router" -import type { RouteObject } from "react-router" - -// @ts-expect-error Fixing tests next -import AppProviders from "../AppProviders" -// @ts-expect-error Fixing tests next -import appRoutes from "../routes" -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" - -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 -} - -/** - * Render routes in a test environment using same providers used by App. - */ -const renderRoutesWithProviders = ( - routes: RouteObject[], - options: Partial = {}, -) => { - const { url, user, initialQueryData } = { - ...defaultTestAppOptions, - ...options, - } - - const router = createMemoryRouter(routes, { initialEntries: [url] }) - const queryClient = options.queryClient || createQueryClient() - - if (user) { - queryClient.setQueryData(["userMe"], { is_authenticated: true, ...user }) - } - if (initialQueryData) { - initialQueryData.forEach(([queryKey, data]) => { - queryClient.setQueryData(queryKey, data) - }) - } - const view = render( - , - ) - - const location = { - get current() { - return router.state.location - }, - } - - 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) -} - -/** - * Assert that a functional component was called at some point with the given - * props. - * @param fc the mock or spied upon functional component - * @param partialProps an object of props - */ -const expectProps = ( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - fc: (...args: any[]) => void, - partialProps: unknown, -) => { - expect(fc).toHaveBeenCalledWith( - expect.objectContaining(partialProps), - expect.anything(), - ) -} - -/** - * Assert that a functional component was last called with the given - * props. - * @param fc the mock or spied upon functional component - * @param partialProps an object of props - */ -const expectLastProps = ( - // eslint-disable-next-line @typescript-eslint/no-explicit-any - fc: jest.Mock, - partialProps: unknown, -) => { - expect(fc).toHaveBeenLastCalledWith( - expect.objectContaining(partialProps), - expect.anything(), - ) -} - -/** - * Useful for checking that "real" navigation occurs, i.e., navigation with a - * full browser reload, not React Router's SPA-routing. - */ -const expectWindowNavigation = async (cb: () => void | Promise) => { - const consoleError = console.error - try { - const spy = jest.spyOn(console, "error").mockImplementation() - await cb() - expect(spy).toHaveBeenCalledTimes(1) - const error = spy.mock.calls[0][0] - expect(error instanceof Error) - expect(error.message).toMatch(/Not implemented: navigation/) - } finally { - console.error = consoleError - } -} - -const ignoreError = (errorMessage: string, timeoutMs?: number) => { - const consoleError = console.error - const spy = jest.spyOn(console, "error").mockImplementation((...args) => { - if (args[0]?.includes(errorMessage)) { - return - } - return consoleError.call(console, args) - }) - - const timeout = setTimeout(() => { - throw new Error( - `Ignored console error not cleared after ${timeoutMs || 5000}ms: "${errorMessage}"`, - ) - }, timeoutMs || 5000) - - const clear = () => { - console.error = consoleError - spy.mockClear() - clearTimeout(timeout) - } - - return { clear } -} - -const getMetaContent = ({ - property, - name, -}: { - property?: string - name?: string -}) => { - const propSelector = property ? `[property="${property}"]` : "" - const nameSelector = name ? `[name="${name}"]` : "" - const selector = `meta${propSelector}${nameSelector}` - const el = document.querySelector(selector) - return el?.content -} - -type TestableMetas = { - title?: string | null - description?: string | null - og: { - image?: string | null - imageAlt?: string | null - description?: string | null - title?: string | null - } - twitter: { - card?: string | null - image?: string | null - description?: string | null - } -} -const getMetas = (): TestableMetas => { - return { - title: document.title, - description: getMetaContent({ name: "description" }), - og: { - image: getMetaContent({ property: "og:image" }), - imageAlt: getMetaContent({ property: "og:image:alt" }), - description: getMetaContent({ property: "og:description" }), - title: getMetaContent({ property: "og:title" }), - }, - twitter: { - card: getMetaContent({ name: "twitter:card" }), - image: getMetaContent({ name: "twitter:image:src" }), - description: getMetaContent({ name: "twitter:description" }), - }, - } -} -const assertPartialMetas = (expected: Partial) => { - expect(getMetas()).toEqual( - expect.objectContaining({ - ...expected, - og: expect.objectContaining(expected.og ?? {}), - twitter: expect.objectContaining(expected.twitter ?? {}), - }), - ) -} - -export { - renderTestApp, - renderWithProviders, - renderRoutesWithProviders, - expectProps, - expectLastProps, - expectWindowNavigation, - ignoreError, - getMetas, - assertPartialMetas, -} -// Conveniences -export { setMockResponse } -export { - act, - screen, - prettyDOM, - within, - fireEvent, - waitFor, - renderHook, -} from "@testing-library/react" -export { default as user } from "@testing-library/user-event" - -export type { TestAppOptions, User } 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: From 44d7ad3785ccc046ec1b022c14d4358771351ce0 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:10:44 -0400 Subject: [PATCH 12/17] fix typecheck --- .github/workflows/ci.yml | 1 - .../src/page-components/ResourceCard/ResourceCard.test.tsx | 1 + frontends/ol-components/src/components/EmbedlyCard/util.ts | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 04624b3200..4edc56f682 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -145,7 +145,6 @@ jobs: run: yarn run lint-check - 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/main/src/page-components/ResourceCard/ResourceCard.test.tsx b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx index 8c48b0a5fe..4ec73676c4 100644 --- a/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx +++ b/frontends/main/src/page-components/ResourceCard/ResourceCard.test.tsx @@ -188,6 +188,7 @@ describe.each([ invariant(resource) const link = screen.getByRole("link", { name: new RegExp(resource.title) }) 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/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 } From e9c0abfd50bbf4083ede90fc70a01ee40abef632 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:25:12 -0400 Subject: [PATCH 13/17] add a comment --- frontends/main/src/test-utils/index.tsx | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index d9f85cf4b9..62237c9a89 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -15,6 +15,11 @@ import { } 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([ From 4ab6157b2f7b47c69068b2a70b82e2a2bbc4583c Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:42:51 -0400 Subject: [PATCH 14/17] build before typecheck --- .github/workflows/ci.yml | 22 ++++------------------ 1 file changed, 4 insertions(+), 18 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 4edc56f682..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,6 +126,10 @@ 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 run: yarn run typecheck From 0eb19b42db9e44803ce12baced30bb3bdd84c78e Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 16:53:24 -0400 Subject: [PATCH 15/17] fix an env var issue --- frontends/jest-shared-setup.ts | 1 + frontends/ol-ckeditor/jest.config.ts | 1 - frontends/ol-ckeditor/src/components/CkeditorDisplay.test.tsx | 4 ++-- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/frontends/jest-shared-setup.ts b/frontends/jest-shared-setup.ts index 66212e486e..76e8c247db 100644 --- a/frontends/jest-shared-setup.ts +++ b/frontends/jest-shared-setup.ts @@ -11,6 +11,7 @@ expect.extend(matchers) 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/ol-ckeditor/jest.config.ts b/frontends/ol-ckeditor/jest.config.ts index ed8ade7905..0d65c15475 100644 --- a/frontends/ol-ckeditor/jest.config.ts +++ b/frontends/ol-ckeditor/jest.config.ts @@ -18,7 +18,6 @@ const config: Config.InitialOptions = { 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() From 4d596eff726d768a83ceb0b39926cca19c800faa Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Fri, 13 Sep 2024 17:14:08 -0400 Subject: [PATCH 16/17] remove erroneous setTimeout --- .../src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx | 2 -- 1 file changed, 2 deletions(-) diff --git a/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx b/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx index 2fcce7eb90..13b8bafc2f 100644 --- a/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx +++ b/frontends/main/src/app-pages/TopicsListingPage/TopicsListingPage.test.tsx @@ -140,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") From 5692e5bce3bf465718b337de35c4dee300bb6a52 Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Mon, 16 Sep 2024 13:26:21 -0400 Subject: [PATCH 17/17] simplify import --- .../ProgramLetterPage/[id]/view/ProgramLetter.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx index 4511a8066e..33bf36c46c 100644 --- a/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx +++ b/frontends/main/src/app-pages/ProgramLetterPage/[id]/view/ProgramLetter.test.tsx @@ -1,5 +1,5 @@ import React from "react" -import { renderWithProviders, waitFor } from "../../../../test-utils" +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"