From 0bbc72b73fba353e5621ba9537ea644a53526e7a Mon Sep 17 00:00:00 2001 From: Chris Chudzicki Date: Thu, 30 Oct 2025 13:59:00 -0400 Subject: [PATCH 1/7] Eliminate build-time API calls, default to dynamic rendering (#2660) * force all pages into dynamic rendering, no build-time api calls * allow running docker in production mode * add temporary logging * remove temporary logging * fix typo * remove unnecessary revalidates --- docker-compose.apps.yml | 8 +++++++- frontends/main/src/app/layout.tsx | 10 ++++++++++ frontends/main/src/app/page.tsx | 12 ------------ frontends/main/src/app/search/page.tsx | 12 ------------ frontends/main/src/app/sitemaps/channels/sitemap.ts | 13 +++++-------- .../main/src/app/sitemaps/resources/sitemap.ts | 13 +++++-------- 6 files changed, 27 insertions(+), 41 deletions(-) diff --git a/docker-compose.apps.yml b/docker-compose.apps.yml index 5a57cc6449..0671b1cfc6 100644 --- a/docker-compose.apps.yml +++ b/docker-compose.apps.yml @@ -36,7 +36,13 @@ services: command: - | yarn install --immutable - yarn watch + if [ "$${NODE_ENV}" = "production" ]; then + yarn build + echo "WARNING: Production preview will NOT re-render on file changes." + yarn preview + else + yarn watch + fi ports: - "8062:8062" - "6006:6006" diff --git a/frontends/main/src/app/layout.tsx b/frontends/main/src/app/layout.tsx index 435709331b..9db5de12f1 100644 --- a/frontends/main/src/app/layout.tsx +++ b/frontends/main/src/app/layout.tsx @@ -17,6 +17,16 @@ export const metadata = { metadataBase: NEXT_PUBLIC_ORIGIN ? new URL(NEXT_PUBLIC_ORIGIN) : null, } +/** + * Force all pages to render dynamically (at request-time) rather than + * statically at build time. This simplifies the build, reduces build time, and + * ensures fresh data on each request to the NextJS server. + * + * It does increase server load, but this should not be significant since + * requests are cached on CDN. + */ +export const dynamic = "force-dynamic" + export default function RootLayout({ children, }: Readonly<{ diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index 09a0ab61dc..799728fd02 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -23,18 +23,6 @@ export async function generateMetadata({ }) } -/** - * Fallback to dynamic rendering to load as much of the page as possible. - * - * We could alternatively wrap the carousels (which use useSearchParams) in a - * suspense boundary. This ostensibly leads to a faster response, since the - * server can render the rest of the homepage at build time. - * - * But... We ache the result on CDN anyway, so it's essentially pre-build for - * most requests, anyway. - */ -export const dynamic = "force-dynamic" - const Page: React.FC> = async () => { const { dehydratedState } = await prefetch([ // Featured Courses carousel "All" diff --git a/frontends/main/src/app/search/page.tsx b/frontends/main/src/app/search/page.tsx index 3061a0794c..387d67c083 100644 --- a/frontends/main/src/app/search/page.tsx +++ b/frontends/main/src/app/search/page.tsx @@ -20,18 +20,6 @@ export async function generateMetadata({ searchParams }: PageProps<"/search">) { }) } -/** - * The search page uses Next's `useSearchParams`. This requires either: - * 1. wrap the in Suspense - * 2. or force-dynamic. - * - * (1) caused a hydration error for authenticated users. We have not found - * the root cause of the hydration error. - * - * (2) seems to work well. - */ -export const dynamic = "force-dynamic" - const Page: React.FC> = async ({ searchParams }) => { const search = (await searchParams) as ResourceSearchRequest & { page?: string diff --git a/frontends/main/src/app/sitemaps/channels/sitemap.ts b/frontends/main/src/app/sitemaps/channels/sitemap.ts index d1ad6d47e6..caf42aaf02 100644 --- a/frontends/main/src/app/sitemaps/channels/sitemap.ts +++ b/frontends/main/src/app/sitemaps/channels/sitemap.ts @@ -10,19 +10,16 @@ invariant(BASE_URL, "NEXT_PUBLIC_ORIGIN must be defined") const PAGE_SIZE = 100 /** - * By default in NextJS, sitemaps are statically generated at build time. - * We want to ensure up-to-date sitemaps. - * - * This forces the sitemaps to be rendered for each request. - * However, we we set s-maxage in the headers (next.config.js) to enable caching - * by a CDN. + * As of NextJS 15.5.3, sitemaps are ALWAYS generated at build time, even with + * the force-dynamic below (this may be a NextJS bug?). However, the + * force-dynamic does force re-generation when requests are made in production. */ export const dynamic = "force-dynamic" export async function generateSitemaps(): Promise { /** - * NextJS runs this at build time (despite force-dynamic below). - * Early exist here to avoid the useless build-time API calls. + * NextJS runs this at build time (despite force-dynamic above). + * Early exit here to avoid the useless build-time API calls. */ if (dangerouslyDetectProductionBuildPhase()) return [] const { count } = (await channelsApi.channelsList({ limit: PAGE_SIZE })).data diff --git a/frontends/main/src/app/sitemaps/resources/sitemap.ts b/frontends/main/src/app/sitemaps/resources/sitemap.ts index b757f2e8a8..871ea602a2 100644 --- a/frontends/main/src/app/sitemaps/resources/sitemap.ts +++ b/frontends/main/src/app/sitemaps/resources/sitemap.ts @@ -10,19 +10,16 @@ invariant(BASE_URL, "NEXT_PUBLIC_ORIGIN must be defined") const PAGE_SIZE = 1_000 /** - * By default in NextJS, sitemaps are statically generated at build time. - * We want to ensure up-to-date sitemaps. - * - * This forces the sitemaps to be rendered for each request. - * However, we we set s-maxage in the headers (next.config.js) to enable caching - * by a CDN. + * As of NextJS 15.5.3, sitemaps are ALWAYS generated at build time, even with + * the force-dynamic below (this may be a NextJS bug?). However, the + * force-dynamic does force re-generation when requests are made in production. */ export const dynamic = "force-dynamic" export async function generateSitemaps(): Promise { /** - * NextJS runs this at build time (despite force-dynamic below). - * Early exist here to avoid the useless build-time API calls. + * NextJS runs this at build time (despite force-dynamic above). + * Early exit here to avoid the useless build-time API calls. */ if (dangerouslyDetectProductionBuildPhase()) return [] const { count } = ( From 99b6b1c7fcf0dadacba64ec50db927f3f852775c Mon Sep 17 00:00:00 2001 From: Matt Bertrand Date: Thu, 30 Oct 2025 14:45:05 -0400 Subject: [PATCH 2/7] Get rid of debug code (#2661) --- webhooks/views.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/webhooks/views.py b/webhooks/views.py index 7fc042941b..eea1dca681 100644 --- a/webhooks/views.py +++ b/webhooks/views.py @@ -124,8 +124,6 @@ def get_data(self, request): """ Get data from the serializer """ - body = request.body - log.error("Webhook body: %s", body) serializer = VideoShortWebhookRequestSerializer(data=json.loads(request.body)) serializer.is_valid(raise_exception=True) return serializer.validated_data From 5ac5dbf64e83e9cb15a438e475a52c1cceeeeab9 Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Thu, 30 Oct 2025 15:53:11 -0400 Subject: [PATCH 3/7] alter org page header based on new requirements (#2659) --- .../DashboardPage/CoursewareDisplay/test-utils.ts | 6 ++++++ .../app-pages/DashboardPage/OrganizationContent.test.tsx | 9 ++++++--- .../src/app-pages/DashboardPage/OrganizationContent.tsx | 5 +++-- 3 files changed, 15 insertions(+), 5 deletions(-) diff --git a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts index 6f6c4f8bfc..617ddb1d65 100644 --- a/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts +++ b/frontends/main/src/app-pages/DashboardPage/CoursewareDisplay/test-utils.ts @@ -26,6 +26,7 @@ const makeProgram = factories.programs.program const makeProgramCollection = factories.programs.programCollection const makeCourseEnrollment = factories.enrollment.courseEnrollment const makeGrade = factories.enrollment.grade +const makeContract = factories.contracts.contract const dashboardCourse: PartialFactory = (...overrides) => { return mergeOverrides( @@ -127,6 +128,11 @@ const setupEnrollments = (includeExpired: boolean) => { const setupProgramsAndCourses = () => { const user = u.factories.user.user() const orgX = factories.organizations.organization({ name: "Org X" }) + const contract = makeContract({ + organization: orgX.id, + name: "Org X Contract", + }) + orgX.contracts = [contract] const mitxOnlineUser = factories.user.user({ b2b_organizations: [orgX] }) setMockResponse.get(u.urls.userMe.get(), user) setMockResponse.get(urls.userMe.get(), mitxOnlineUser) diff --git a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.test.tsx b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.test.tsx index 8a23f0968e..08b63060d7 100644 --- a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.test.tsx +++ b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.test.tsx @@ -37,8 +37,9 @@ describe("OrganizationContent", () => { renderWithProviders() await screen.findByRole("heading", { - name: `Your ${orgX.name} Home`, + name: orgX.name, }) + await screen.findByText(orgX.contracts[0].name) const programAHeader = await screen.findByText(programA.title) const programBHeader = await screen.findByText(programB.title) @@ -300,8 +301,9 @@ describe("OrganizationContent", () => { // Wait for the header to appear await screen.findByRole("heading", { - name: `Your ${orgX.name} Home`, + name: orgX.name, }) + await screen.findByText(orgX.contracts[0].name) // Since there are no programs or collections, no program/collection components should be rendered const programs = screen.queryAllByTestId("org-program-root") @@ -331,8 +333,9 @@ describe("OrganizationContent", () => { // Wait for the header to appear await screen.findByRole("heading", { - name: `Your ${orgX.name} Home`, + name: orgX.name, }) + await screen.findByText(orgX.contracts[0].name) // Should have no collections const collections = screen.queryAllByTestId("org-program-collection-root") diff --git a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx index 2c5d649d7c..e15bc040ef 100644 --- a/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx +++ b/frontends/main/src/app-pages/DashboardPage/OrganizationContent.tsx @@ -68,9 +68,10 @@ const OrganizationHeader: React.FC<{ org?: OrganizationPage }> = ({ org }) => { - Your {org?.name} Home + {org?.name} - MIT courses for {org?.name} + {/* For now we will use the first contract name until we refactor this to be based on contracts / offerings */} + {org?.contracts[0]?.name} ) From 56eac9bf95a76cf90daca8d78ba6c12aa5cc502a Mon Sep 17 00:00:00 2001 From: Jon Kafton <939376+jonkafton@users.noreply.github.com> Date: Mon, 3 Nov 2025 19:53:37 +0100 Subject: [PATCH 4/7] Server side query retry and caching with request scoped QueryClient (#2596) * Server side query retry and caching with request scoped QueryClient and native fetch for axios * Default to force-cache on fetch options. ESLint rule to prevent use of API outside a Query Client * Status on axios error * Update comment * Hydration issues with non-serializable error response. Sanitize axios errors and remove from query client * Remove catch - prefetch does not throw * Safe generate metadata utility with error catch * Remove use of native fetch * Update tests to expect console calls * Remove comment * Update comment Co-authored-by: Chris Chudzicki * Update to use query client for API calls / no direct API client import * Metadata fallback not used * Remove not used debug props. Update test for standardized metadata fallback --------- Co-authored-by: Chris Chudzicki --- frontends/.eslintrc.js | 11 +++ frontends/api/package.json | 2 - .../src/hooks/learningResources/queries.ts | 14 ++++ frontends/api/src/mitxonline/index.ts | 1 - frontends/api/src/ssr/prefetch.ts | 70 ++++------------- frontends/api/src/ssr/serverQueryClient.ts | 76 +++++++++++++++++++ frontends/main/package.json | 1 + .../(products)/courses/[readable_id]/page.tsx | 28 +++---- .../programs/[readable_id]/page.tsx | 30 +++----- .../src/app/c/[channelType]/[name]/page.tsx | 24 +++--- .../[certificateType]/[uuid]/page.tsx | 41 +++++----- .../[certificateType]/[uuid]/pdf/route.tsx | 24 +++--- frontends/main/src/app/error.test.tsx | 12 +++ frontends/main/src/app/error.tsx | 1 + .../main/src/app/getQueryClient.test.tsx | 4 +- frontends/main/src/app/getQueryClient.ts | 22 ++---- frontends/main/src/app/global-error.tsx | 6 +- frontends/main/src/app/page.tsx | 10 ++- frontends/main/src/app/search/page.tsx | 10 ++- .../main/src/app/sitemaps/channels/sitemap.ts | 20 +++-- .../src/app/sitemaps/resources/sitemap.ts | 24 +++--- .../main/src/common/handleNotFound.test.ts | 39 ---------- frontends/main/src/common/handleNotFound.ts | 24 ------ frontends/main/src/common/metadata.test.ts | 59 ++++++++++++++ frontends/main/src/common/metadata.ts | 65 ++++++++++------ frontends/main/src/middleware.ts | 25 ------ frontends/main/src/test-utils/index.tsx | 4 +- yarn.lock | 20 +++-- 28 files changed, 369 insertions(+), 298 deletions(-) delete mode 100644 frontends/api/src/mitxonline/index.ts create mode 100644 frontends/api/src/ssr/serverQueryClient.ts delete mode 100644 frontends/main/src/common/handleNotFound.test.ts delete mode 100644 frontends/main/src/common/handleNotFound.ts create mode 100644 frontends/main/src/common/metadata.test.ts delete mode 100644 frontends/main/src/middleware.ts diff --git a/frontends/.eslintrc.js b/frontends/.eslintrc.js index 75b0317415..074b4b193e 100644 --- a/frontends/.eslintrc.js +++ b/frontends/.eslintrc.js @@ -68,6 +68,11 @@ module.exports = { message: "The LearningResourceDrawer should be lazy loaded with dynamic import.", }, + { + group: ["api/clients"], + message: + "Direct import from 'api/clients' is not allowed. Use React Query hooks from 'api/hooks/*' instead for caching and error handling. In server components, use getServerQueryClient().fetchQuery(), passing the queryOptions.", + }, ], }), // This rule is disabled in the default a11y config, but unclear why. @@ -145,6 +150,12 @@ module.exports = { message: "Do not specify `fontFamily` manually. Prefer spreading `theme.typography.subtitle1` or similar. If using neue-haas-grotesk-text, this is ThemeProvider's default fontFamily.", }, + { + selector: + "FunctionDeclaration[id.name='generateMetadata'] > BlockStatement > ReturnStatement[argument.type!='CallExpression'], FunctionDeclaration[id.name='generateMetadata'] > BlockStatement > ReturnStatement[argument.callee.name!='safeGenerateMetadata']", + message: + "generateMetadata functions must return safeGenerateMetadata() to ensure proper error handling and fallback metadata.", + }, ], }, overrides: [ diff --git a/frontends/api/package.json b/frontends/api/package.json index 4ad76e4e40..e74ccebcf9 100644 --- a/frontends/api/package.json +++ b/frontends/api/package.json @@ -5,7 +5,6 @@ "sideEffects": false, "exports": { ".": "./src/generated/v1/api.ts", - "./clients": "./src/clients.ts", "./v0": "./src/generated/v0/api.ts", "./v1": "./src/generated/v1/api.ts", "./hooks/*": "./src/hooks/*/index.ts", @@ -13,7 +12,6 @@ "./ssr/*": "./src/ssr/*.ts", "./test-utils/factories": "./src/test-utils/factories/index.ts", "./test-utils": "./src/test-utils/index.ts", - "./mitxonline": "./src/mitxonline/index.ts", "./mitxonline-hooks/*": "./src/mitxonline/hooks/*/index.ts", "./mitxonline-test-utils": "./src/mitxonline/test-utils/index.ts" }, diff --git a/frontends/api/src/hooks/learningResources/queries.ts b/frontends/api/src/hooks/learningResources/queries.ts index e9ad586abf..91438cf8c3 100644 --- a/frontends/api/src/hooks/learningResources/queries.ts +++ b/frontends/api/src/hooks/learningResources/queries.ts @@ -18,6 +18,7 @@ import type { FeaturedApiFeaturedListRequest as FeaturedListParams, LearningResourcesApiLearningResourcesItemsListRequest as ItemsListRequest, LearningResourcesSearchResponse, + LearningResourcesApiLearningResourcesSummaryListRequest as LearningResourcesSummaryListRequest, } from "../../generated/v1" import { queryOptions } from "@tanstack/react-query" import { hasPosition, randomizeGroups } from "./util" @@ -46,6 +47,11 @@ const learningResourceKeys = { ...learningResourceKeys.listsRoot(), params, ], + summaryListRoot: () => [...learningResourceKeys.root, "summaryList"], + summaryList: (params: LearningResourcesSummaryListRequest) => [ + ...learningResourceKeys.summaryListRoot(), + params, + ], // detail detailsRoot: () => [...learningResourceKeys.root, "detail"], detail: (id: number) => [...learningResourceKeys.detailsRoot(), id], @@ -150,6 +156,14 @@ const learningResourceQueries = { results: res.data.results.map(clearListMemberships), })), }), + summaryList: (params: LearningResourcesSummaryListRequest) => + queryOptions({ + queryKey: learningResourceKeys.summaryList(params), + queryFn: () => + learningResourcesApi + .learningResourcesSummaryList(params) + .then((res) => res.data), + }), featured: (params: FeaturedListParams = {}) => queryOptions({ queryKey: learningResourceKeys.featured(params), diff --git a/frontends/api/src/mitxonline/index.ts b/frontends/api/src/mitxonline/index.ts deleted file mode 100644 index e66ee4c62f..0000000000 --- a/frontends/api/src/mitxonline/index.ts +++ /dev/null @@ -1 +0,0 @@ -export * from "./clients" diff --git a/frontends/api/src/ssr/prefetch.ts b/frontends/api/src/ssr/prefetch.ts index ff4de6b368..3cd719911c 100644 --- a/frontends/api/src/ssr/prefetch.ts +++ b/frontends/api/src/ssr/prefetch.ts @@ -1,51 +1,9 @@ -import { QueryClient, dehydrate } from "@tanstack/react-query" +import { dehydrate } from "@tanstack/react-query" +import { getServerQueryClient } from "./serverQueryClient" import type { Query } from "@tanstack/react-query" -const MAX_RETRIES = 3 -const NO_RETRY_CODES = [400, 401, 403, 404, 405, 409, 422] - -type MaybeHasStatus = { - response?: { - status?: number - } -} - -const getPrefetchQueryClient = () => { - return new QueryClient({ - defaultOptions: { - queries: { - /** - * React Query's default retry logic is only active in the browser. - * Here we explicitly configure it to retry MAX_RETRIES times on - * the server, with an exclusion list of statuses that we expect not - * to succeed on retry. - * - * Includes status undefined as we want to retry on network errors - */ - retry: (failureCount, error) => { - const status = (error as MaybeHasStatus)?.response?.status - const isNetworkError = status === undefined || status === 0 - - if (isNetworkError || !NO_RETRY_CODES.includes(status)) { - return failureCount < MAX_RETRIES - } - return false - }, - - /** - * By default, React Query gradually applies a backoff delay, though it is - * preferable that we do not significantly delay initial page renders (or - * indeed pages that are Statically Rendered during the build process) and - * instead allow the request to fail quickly so it can be subsequently - * fetched on the client. - */ - retryDelay: 1000, - }, - }, - }) -} - -/* Utility to avoid repetition in server components +/** + * Utility to avoid repetition in server components * Optionally pass the queryClient returned from a previous prefetch * where queries are dependent on previous results */ @@ -53,19 +11,21 @@ export const prefetch = async ( queries: (Query | unknown)[], /** - * The SSR QueryClient is transient - it is created only for prefetch - * while API requests are made to server render the page and discarded - * as the dehydrated state is produced and sent to the client. + * Unless passed, the SSR QueryClient uses React's cache() for reuse for the duration of the request. * - * Create a new query client if one is not provided. + * The QueryClient is garbage collected once the dehydrated state is produced and + * sent to the client and the request is complete. */ - queryClient = getPrefetchQueryClient(), + queryClient = getServerQueryClient(), ) => { await Promise.all( - queries - .filter(Boolean) - .map((query) => queryClient.prefetchQuery(query as Query)), + queries.filter(Boolean).map((query) => { + return queryClient.prefetchQuery(query as Query) + }), ) - return { dehydratedState: dehydrate(queryClient), queryClient } + return { + dehydratedState: dehydrate(queryClient), + queryClient, + } } diff --git a/frontends/api/src/ssr/serverQueryClient.ts b/frontends/api/src/ssr/serverQueryClient.ts new file mode 100644 index 0000000000..daa745891f --- /dev/null +++ b/frontends/api/src/ssr/serverQueryClient.ts @@ -0,0 +1,76 @@ +import { cache } from "react" +import { QueryClient } from "@tanstack/react-query" +import { AxiosError } from "axios" + +const MAX_RETRIES = 3 +const NO_RETRY_CODES = [400, 401, 403, 404, 405, 409, 422] + +/** + * Get or create a server-side QueryClient for consistent retry behavior. + * The server QueryClient should be used for all server-side API calls. + * + * Uses React's cache() to ensure the same QueryClient instance is reused + * throughout a single HTTP request, enabling: + * + * - Server API calls share the same QueryClient: + * - Prefetch runs in page server components + * - generateMetadata() + * - No duplicate API calls within the same request + * - Automatic cleanup when the request completes + * - Isolation between different HTTP requests + * + * The QueryClientProvider runs (during SSR) in a separate render pass as it's a + * client component and so the instance is not reused. On the server this does not + * make API calls and only sets up the hydration boundary and registers hooks in + * readiness for the dehydrated state to be sent to the client. + */ +const getServerQueryClient = cache(() => { + const queryClient = new QueryClient({ + defaultOptions: { + queries: { + /** + * React Query's default retry logic is only active in the browser. + * Here we explicitly configure it to retry MAX_RETRIES times on + * the server, with an exclusion list of statuses that we expect not + * to succeed on retry. + * + * Includes status undefined as we want to retry on network errors + */ + retry: (failureCount, error) => { + const axiosError = error as AxiosError + console.info("Retrying failed request", { + failureCount, + error: { + message: axiosError.message, + name: axiosError.name, + status: axiosError?.status, + code: axiosError.code, + method: axiosError.request?.method, + url: axiosError.request?.url, + }, + }) + const status = (error as AxiosError)?.response?.status + const isNetworkError = status === undefined || status === 0 + + if (isNetworkError || !NO_RETRY_CODES.includes(status)) { + return failureCount < MAX_RETRIES + } + return false + }, + + /** + * By default, React Query gradually applies a backoff delay, though it is + * preferable that we do not significantly delay initial page renders (or + * indeed pages that are Statically Rendered during the build process) and + * instead allow the request to fail quickly so it can be subsequently + * fetched on the client. + */ + retryDelay: 1000, + }, + }, + }) + + return queryClient +}) + +export { getServerQueryClient } diff --git a/frontends/main/package.json b/frontends/main/package.json index b53d266257..d2df5e29cb 100644 --- a/frontends/main/package.json +++ b/frontends/main/package.json @@ -22,6 +22,7 @@ "@sentry/nextjs": "^10.0.0", "@tanstack/react-query": "^5.66", "api": "workspace:*", + "async_hooks": "^1.0.0", "classnames": "^2.5.1", "formik": "^2.4.6", "iso-639-1": "^3.1.4", diff --git a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx index 79f3722a64..96e47759ca 100644 --- a/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/courses/[readable_id]/page.tsx @@ -1,29 +1,30 @@ import React from "react" import { HydrationBoundary } from "@tanstack/react-query" -import { standardizeMetadata } from "@/common/metadata" +import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" import { prefetch } from "api/ssr/prefetch" import CoursePage from "@/app-pages/ProductPages/CoursePage" -import { pagesApi } from "api/mitxonline" -import * as Sentry from "@sentry/nextjs" import { notFound } from "next/navigation" import { pagesQueries } from "api/mitxonline-hooks/pages" import { coursesQueries } from "api/mitxonline-hooks/courses" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" +import { getServerQueryClient } from "api/ssr/serverQueryClient" export const generateMetadata = async ( props: PageProps<"/courses/[readable_id]">, ) => { const params = await props.params - try { - const resp = await pagesApi.pagesfieldstypecmsCoursepageRetrieve({ - readable_id: decodeURIComponent(params.readable_id), - }) + return safeGenerateMetadata(async () => { + const queryClient = getServerQueryClient() + + const data = await queryClient.fetchQuery( + pagesQueries.coursePages(decodeURIComponent(params.readable_id)), + ) - if (resp.data.items.length === 0) { + if (data.items.length === 0) { notFound() } - const [course] = resp.data.items + const [course] = data.items const image = course.feature_image ? course.course_details.page.feature_image_src : DEFAULT_RESOURCE_IMG @@ -32,14 +33,7 @@ export const generateMetadata = async ( image, robots: "noindex, nofollow", }) - } catch (error) { - Sentry.captureException(error) - console.error( - "Error fetching course page metadata for", - params.readable_id, - error, - ) - } + }) } const Page: React.FC> = async (props) => { diff --git a/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx index 60b80e0dd4..094267e460 100644 --- a/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx +++ b/frontends/main/src/app/(products)/programs/[readable_id]/page.tsx @@ -1,29 +1,30 @@ import React from "react" import { HydrationBoundary } from "@tanstack/react-query" -import { standardizeMetadata } from "@/common/metadata" +import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" import { prefetch } from "api/ssr/prefetch" import ProgramPage from "@/app-pages/ProductPages/ProgramPage" -import { pagesApi } from "api/mitxonline" -import * as Sentry from "@sentry/nextjs" import { notFound } from "next/navigation" import { pagesQueries } from "api/mitxonline-hooks/pages" import { programsQueries } from "api/mitxonline-hooks/programs" import { DEFAULT_RESOURCE_IMG } from "ol-utilities" +import { getServerQueryClient } from "api/ssr/serverQueryClient" export const generateMetadata = async ( props: PageProps<"/programs/[readable_id]">, ) => { const params = await props.params - try { - const resp = await pagesApi.pagesfieldstypecmsProgrampageRetrieve({ - readable_id: decodeURIComponent(params.readable_id), - }) + return safeGenerateMetadata(async () => { + const queryClient = getServerQueryClient() + + const data = await queryClient.fetchQuery( + pagesQueries.programPages(decodeURIComponent(params.readable_id)), + ) - if (resp.data.items.length === 0) { + if (data.items.length === 0) { notFound() } - const [program] = resp.data.items + const [program] = data.items // Note: feature_image.src is relative to mitxonline root. const image = program.feature_image @@ -34,17 +35,10 @@ export const generateMetadata = async ( image, robots: "noindex, nofollow", }) - } catch (error) { - Sentry.captureException(error) - console.error( - "Error fetching course page metadata for", - params.readable_id, - error, - ) - } + }) } -const Page: React.FC> = async (props) => { +const Page: React.FC> = async (props) => { const params = await props.params const readableId = decodeURIComponent(params.readable_id) /** diff --git a/frontends/main/src/app/c/[channelType]/[name]/page.tsx b/frontends/main/src/app/c/[channelType]/[name]/page.tsx index 1ac3631877..236153b240 100644 --- a/frontends/main/src/app/c/[channelType]/[name]/page.tsx +++ b/frontends/main/src/app/c/[channelType]/[name]/page.tsx @@ -1,6 +1,6 @@ import React from "react" import ChannelPage from "@/app-pages/ChannelPage/ChannelPage" -import { channelsApi } from "api/clients" +import { getServerQueryClient } from "api/ssr/serverQueryClient" import { ChannelTypeEnum, UnitChannel } from "api/v0" import { FeaturedListOfferedByEnum, @@ -8,7 +8,7 @@ import { PaginatedLearningResourceOfferorDetailList, LearningResourceOfferorDetail, } from "api" -import { getMetadataAsync } from "@/common/metadata" +import { getMetadataAsync, safeGenerateMetadata } from "@/common/metadata" import { HydrationBoundary } from "@tanstack/react-query" import { prefetch } from "api/ssr/prefetch" import { @@ -17,7 +17,6 @@ import { } from "api/hooks/learningResources" import { channelQueries } from "api/hooks/channels" import { testimonialsQueries } from "api/hooks/testimonials" -import handleNotFound from "@/common/handleNotFound" import getSearchParams from "@/page-components/SearchDisplay/getSearchParams" import validateRequestParams from "@/page-components/SearchDisplay/validateRequestParams" import { @@ -33,14 +32,18 @@ export async function generateMetadata({ }: PageProps<"/c/[channelType]/[name]">) { const { channelType, name } = await params - const { data } = await handleNotFound( - channelsApi.channelsTypeRetrieve({ channel_type: channelType, name: name }), - ) + return safeGenerateMetadata(async () => { + const queryClient = getServerQueryClient() + + const data = await queryClient.fetchQuery( + channelQueries.detailByType(channelType, name), + ) - return getMetadataAsync({ - searchParams, - title: data.title, - description: data.public_description, + return getMetadataAsync({ + searchParams, + title: data.title, + description: data.public_description, + }) }) } @@ -103,6 +106,7 @@ const Page: React.FC> = async ({ [learningResourceQueries.search(searchRequest as LRSearchRequest)], queryClient, ) + return ( diff --git a/frontends/main/src/app/certificate/[certificateType]/[uuid]/page.tsx b/frontends/main/src/app/certificate/[certificateType]/[uuid]/page.tsx index 9cc044384f..60aca8da2f 100644 --- a/frontends/main/src/app/certificate/[certificateType]/[uuid]/page.tsx +++ b/frontends/main/src/app/certificate/[certificateType]/[uuid]/page.tsx @@ -6,9 +6,8 @@ import { certificateQueries } from "api/mitxonline-hooks/certificates" import { HydrationBoundary } from "@tanstack/react-query" import { isInEnum } from "@/common/utils" import { notFound } from "next/navigation" -import { standardizeMetadata } from "@/common/metadata" -import { courseCertificatesApi, programCertificatesApi } from "api/mitxonline" -import * as Sentry from "@sentry/nextjs" +import { safeGenerateMetadata, standardizeMetadata } from "@/common/metadata" +import { getServerQueryClient } from "api/ssr/serverQueryClient" const { NEXT_PUBLIC_ORIGIN } = process.env @@ -22,13 +21,17 @@ export async function generateMetadata({ }: PageProps<"/certificate/[certificateType]/[uuid]">): Promise { const { certificateType, uuid } = await params - let title, displayType, userName + return safeGenerateMetadata(async () => { + let title, displayType, userName + + const queryClient = getServerQueryClient() - try { if (certificateType === CertificateType.Course) { - const { data } = await courseCertificatesApi.courseCertificatesRetrieve({ - cert_uuid: uuid, - }) + const data = await queryClient.fetchQuery( + certificateQueries.courseCertificatesRetrieve({ + cert_uuid: uuid, + }), + ) title = data.course_run.course.title @@ -36,10 +39,10 @@ export async function generateMetadata({ userName = data?.user?.name } else { - const { data } = await programCertificatesApi.programCertificatesRetrieve( - { + const data = await queryClient.fetchQuery( + certificateQueries.programCertificatesRetrieve({ cert_uuid: uuid, - }, + }), ) title = data.program.title @@ -48,19 +51,11 @@ export async function generateMetadata({ userName = data.user.name } - } catch (error) { - Sentry.captureException(error) - console.error("Error fetching certificate for metadata", { - certificateType, - uuid, - error, - }) - return standardizeMetadata({}) - } - return standardizeMetadata({ - title: `${userName}'s ${displayType}`, - description: `${userName} has successfully completed the Universal Artificial Intelligence ${displayType}: ${title}`, + return standardizeMetadata({ + title: `${userName}'s ${displayType}`, + description: `${userName} has successfully completed the Universal Artificial Intelligence ${displayType}: ${title}`, + }) }) } diff --git a/frontends/main/src/app/certificate/[certificateType]/[uuid]/pdf/route.tsx b/frontends/main/src/app/certificate/[certificateType]/[uuid]/pdf/route.tsx index 189b016312..e5ee89232b 100644 --- a/frontends/main/src/app/certificate/[certificateType]/[uuid]/pdf/route.tsx +++ b/frontends/main/src/app/certificate/[certificateType]/[uuid]/pdf/route.tsx @@ -4,7 +4,8 @@ import type { AxiosError } from "axios" import type { NextRequest } from "next/server" import * as Sentry from "@sentry/nextjs" import moment from "moment" -import { courseCertificatesApi, programCertificatesApi } from "api/mitxonline" +import { getServerQueryClient } from "api/ssr/serverQueryClient" +import { certificateQueries } from "api/mitxonline-hooks/certificates" import { V2CourseRunCertificate, V2ProgramCertificate, @@ -561,21 +562,26 @@ type RouteContext = { export async function GET(req: NextRequest, ctx: RouteContext) { const { certificateType, uuid } = await ctx.params + const queryClient = getServerQueryClient() + let pdfDoc try { if (certificateType === CertificateType.Course) { - const certificate = - await courseCertificatesApi.courseCertificatesRetrieve({ + const certificate = await queryClient.fetchQuery( + certificateQueries.courseCertificatesRetrieve({ cert_uuid: uuid, - }) - pdfDoc = pdf() + }), + ) + + pdfDoc = pdf() } else { - const certificate = - await programCertificatesApi.programCertificatesRetrieve({ + const certificate = await queryClient.fetchQuery( + certificateQueries.programCertificatesRetrieve({ cert_uuid: uuid, - }) - pdfDoc = pdf() + }), + ) + pdfDoc = pdf() } } catch (error) { if ([400, 404].includes((error as AxiosError).status ?? -1)) { diff --git a/frontends/main/src/app/error.test.tsx b/frontends/main/src/app/error.test.tsx index 374ba8957c..8f4a544a19 100644 --- a/frontends/main/src/app/error.test.tsx +++ b/frontends/main/src/app/error.test.tsx @@ -8,15 +8,22 @@ import { setMockResponse, urls, factories } from "api/test-utils" const makeUser = factories.user.user test("The error page shows error message", () => { + const consoleSpy = jest.spyOn(console, "error").mockImplementation() const error = new Error("Ruh roh") renderWithProviders() screen.getByRole("heading", { name: "Something went wrong." }) screen.getByText("Oops!") const homeLink = screen.getByRole("link", { name: "Home" }) expect(homeLink).toHaveAttribute("href", HOME) + expect(consoleSpy).toHaveBeenCalledWith( + "Error encountered in React error boundary:", + error, + ) + consoleSpy.mockRestore() }) test("The Forbidden loads with a link that directs to HomePage", async () => { + const consoleSpy = jest.spyOn(console, "error").mockImplementation() const error = new ForbiddenError() setMockResponse.get(urls.userMe.get(), makeUser()) @@ -27,4 +34,9 @@ test("The Forbidden loads with a link that directs to HomePage", async () => { screen.getByText("Oops!") const homeLink = screen.getByRole("link", { name: "Home" }) expect(homeLink).toHaveAttribute("href", HOME) + expect(consoleSpy).toHaveBeenCalledWith( + "Error encountered in React error boundary:", + error, + ) + consoleSpy.mockRestore() }) diff --git a/frontends/main/src/app/error.tsx b/frontends/main/src/app/error.tsx index b92be75e66..970d257f88 100644 --- a/frontends/main/src/app/error.tsx +++ b/frontends/main/src/app/error.tsx @@ -21,6 +21,7 @@ export const metadata = { const Error = ({ error }: { error: Error }) => { useEffect(() => { + console.error("Error encountered in React error boundary:", error) Sentry.captureException(error) }, [error]) diff --git a/frontends/main/src/app/getQueryClient.test.tsx b/frontends/main/src/app/getQueryClient.test.tsx index e2ae9f9c52..52a3cf2625 100644 --- a/frontends/main/src/app/getQueryClient.test.tsx +++ b/frontends/main/src/app/getQueryClient.test.tsx @@ -2,10 +2,10 @@ import React from "react" import { act, renderHook, waitFor } from "@testing-library/react" import { allowConsoleErrors } from "ol-test-utilities" import { QueryClientProvider, useQuery } from "@tanstack/react-query" -import { makeQueryClient } from "./getQueryClient" +import { makeBrowserQueryClient } from "./getQueryClient" const getWrapper = () => { - const queryClient = makeQueryClient() + const queryClient = makeBrowserQueryClient() const wrapper: React.FC<{ children: React.ReactNode }> = ({ children }) => ( {children} ) diff --git a/frontends/main/src/app/getQueryClient.ts b/frontends/main/src/app/getQueryClient.ts index bcedcec418..4c2ce1f6ce 100644 --- a/frontends/main/src/app/getQueryClient.ts +++ b/frontends/main/src/app/getQueryClient.ts @@ -1,6 +1,7 @@ // Based on https://tanstack.com/query/v5/docs/framework/react/guides/advanced-ssr import { QueryClient, isServer, focusManager } from "@tanstack/react-query" +import { getServerQueryClient } from "api/ssr/serverQueryClient" type MaybeHasStatusAndDetail = { response?: { @@ -15,7 +16,7 @@ const MAX_RETRIES = 3 const THROW_ERROR_CODES = [400, 401, 403] const NO_RETRY_CODES = [400, 401, 403, 404, 405, 409, 422] -const makeQueryClient = (): QueryClient => { +const makeBrowserQueryClient = (): QueryClient => { return new QueryClient({ defaultOptions: { queries: { @@ -52,16 +53,6 @@ const makeQueryClient = (): QueryClient => { } return false }, - - /** - * By default, React Query gradually applies a backoff delay, though it is - * preferable that we do not significantly delay initial page renders on - * the server and instead allow the request to fail quickly so it can be - * subsequently fetched on the client. Note that we aim to prefetch any API - * content needed to render the page, so we don't generally expect the retry - * rules above to be in use on the server. - */ - retryDelay: isServer ? 1000 : undefined, }, }, }) @@ -71,14 +62,15 @@ let browserQueryClient: QueryClient | undefined = undefined function getQueryClient() { if (isServer) { - // Server: always make a new query client - return makeQueryClient() + return getServerQueryClient() } 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() + if (!browserQueryClient) { + browserQueryClient = makeBrowserQueryClient() + } return browserQueryClient } } @@ -135,5 +127,5 @@ focusManager.setEventListener((setFocused) => { } }) -export { makeQueryClient, getQueryClient } +export { makeBrowserQueryClient, getQueryClient } export type { MaybeHasStatusAndDetail } diff --git a/frontends/main/src/app/global-error.tsx b/frontends/main/src/app/global-error.tsx index 1ce3353074..299175dbcb 100644 --- a/frontends/main/src/app/global-error.tsx +++ b/frontends/main/src/app/global-error.tsx @@ -5,8 +5,8 @@ * (error.tsx is the fallback error page for UI within page content.) * * Notes: - * - does NOT use root layout (since error occured there!) - * - therefore, must definie its own HTML tags and providers + * - does NOT use root layout (since error occurred there!) + * - therefore, must define its own HTML tags and providers * Must define its own HTML tag * - NOT used in development mode * @@ -19,8 +19,10 @@ import { ThemeProvider, MITLearnGlobalStyles } from "ol-components" export default function GlobalError({ error }: { error: Error }) { useEffect(() => { + console.error("Error encountered in global error boundary:", error) Sentry.captureException(error) }, [error]) + return ( diff --git a/frontends/main/src/app/page.tsx b/frontends/main/src/app/page.tsx index 799728fd02..027d19e74a 100644 --- a/frontends/main/src/app/page.tsx +++ b/frontends/main/src/app/page.tsx @@ -1,7 +1,7 @@ import React from "react" import type { Metadata } from "next" import HomePage from "@/app-pages/HomePage/HomePage" -import { getMetadataAsync } from "@/common/metadata" +import { getMetadataAsync, safeGenerateMetadata } from "@/common/metadata" import { HydrationBoundary } from "@tanstack/react-query" import { learningResourceQueries, @@ -17,9 +17,11 @@ import { prefetch } from "api/ssr/prefetch" export async function generateMetadata({ searchParams, }: PageProps<"/">): Promise { - return await getMetadataAsync({ - title: "Learn with MIT", - searchParams, + return safeGenerateMetadata(async () => { + return await getMetadataAsync({ + title: "Learn with MIT", + searchParams, + }) }) } diff --git a/frontends/main/src/app/search/page.tsx b/frontends/main/src/app/search/page.tsx index 387d67c083..234f92e618 100644 --- a/frontends/main/src/app/search/page.tsx +++ b/frontends/main/src/app/search/page.tsx @@ -5,7 +5,7 @@ import { learningResourceQueries, offerorQueries, } from "api/hooks/learningResources" -import { getMetadataAsync } from "@/common/metadata" +import { getMetadataAsync, safeGenerateMetadata } from "@/common/metadata" import SearchPage from "@/app-pages/SearchPage/SearchPage" import { facetNames } from "@/app-pages/SearchPage/searchRequests" import getSearchParams from "@/page-components/SearchDisplay/getSearchParams" @@ -14,9 +14,11 @@ import type { ResourceSearchRequest } from "@/page-components/SearchDisplay/vali import { LearningResourcesSearchApiLearningResourcesSearchRetrieveRequest as LRSearchRequest } from "api" export async function generateMetadata({ searchParams }: PageProps<"/search">) { - return await getMetadataAsync({ - title: "Search", - searchParams, + return safeGenerateMetadata(async () => { + return getMetadataAsync({ + title: "Search", + searchParams, + }) }) } diff --git a/frontends/main/src/app/sitemaps/channels/sitemap.ts b/frontends/main/src/app/sitemaps/channels/sitemap.ts index caf42aaf02..5cf08fe599 100644 --- a/frontends/main/src/app/sitemaps/channels/sitemap.ts +++ b/frontends/main/src/app/sitemaps/channels/sitemap.ts @@ -1,8 +1,9 @@ import type { MetadataRoute } from "next" -import { channelsApi } from "api/clients" import invariant from "tiny-invariant" import type { GenerateSitemapResult } from "../types" import { dangerouslyDetectProductionBuildPhase } from "../util" +import { getServerQueryClient } from "api/ssr/serverQueryClient" +import { channelQueries } from "api/hooks/channels" const BASE_URL = process.env.NEXT_PUBLIC_ORIGIN invariant(BASE_URL, "NEXT_PUBLIC_ORIGIN must be defined") @@ -22,7 +23,10 @@ export async function generateSitemaps(): Promise { * Early exit here to avoid the useless build-time API calls. */ if (dangerouslyDetectProductionBuildPhase()) return [] - const { count } = (await channelsApi.channelsList({ limit: PAGE_SIZE })).data + const queryClient = getServerQueryClient() + const { count } = await queryClient.fetchQuery( + channelQueries.list({ limit: PAGE_SIZE }), + ) const pages = Math.ceil(count / PAGE_SIZE) return new Array(pages).fill(null).map((_, index) => ({ @@ -36,11 +40,13 @@ export default async function sitemap({ }: { id: string }): Promise { - const offset = +id * PAGE_SIZE - const { data } = await channelsApi.channelsList({ - limit: PAGE_SIZE, - offset, - }) + const queryClient = getServerQueryClient() + const data = await queryClient.fetchQuery( + channelQueries.list({ + limit: PAGE_SIZE, + offset: +id * PAGE_SIZE, + }), + ) return data.results.map((channel) => ({ url: channel.channel_url, diff --git a/frontends/main/src/app/sitemaps/resources/sitemap.ts b/frontends/main/src/app/sitemaps/resources/sitemap.ts index 871ea602a2..80975d14ca 100644 --- a/frontends/main/src/app/sitemaps/resources/sitemap.ts +++ b/frontends/main/src/app/sitemaps/resources/sitemap.ts @@ -1,5 +1,6 @@ import type { MetadataRoute } from "next" -import { learningResourcesApi } from "api/clients" +import { getServerQueryClient } from "api/ssr/serverQueryClient" +import { learningResourceQueries } from "api/hooks/learningResources" import invariant from "tiny-invariant" import { GenerateSitemapResult } from "../types" import { dangerouslyDetectProductionBuildPhase } from "../util" @@ -22,11 +23,12 @@ export async function generateSitemaps(): Promise { * Early exit here to avoid the useless build-time API calls. */ if (dangerouslyDetectProductionBuildPhase()) return [] - const { count } = ( - await learningResourcesApi.learningResourcesSummaryList({ + const queryClient = getServerQueryClient() + const { count } = await queryClient.fetchQuery( + learningResourceQueries.summaryList({ limit: PAGE_SIZE, - }) - ).data + }), + ) const pages = Math.ceil(count / PAGE_SIZE) @@ -42,11 +44,13 @@ export default async function sitemap({ }: { id: string }): Promise { - const offset = +id * PAGE_SIZE - const { data } = await learningResourcesApi.learningResourcesSummaryList({ - limit: PAGE_SIZE, - offset, - }) + const queryClient = getServerQueryClient() + const data = await queryClient.fetchQuery( + learningResourceQueries.summaryList({ + limit: PAGE_SIZE, + offset: +id * PAGE_SIZE, + }), + ) return data.results.map((resource) => ({ url: `${BASE_URL}/search?resource=${resource.id}`, diff --git a/frontends/main/src/common/handleNotFound.test.ts b/frontends/main/src/common/handleNotFound.test.ts deleted file mode 100644 index 7978b9e78b..0000000000 --- a/frontends/main/src/common/handleNotFound.test.ts +++ /dev/null @@ -1,39 +0,0 @@ -import type { AxiosError } from "axios" -import handleNotFound from "./handleNotFound" -import { nextNavigationMocks } from "ol-test-utilities/mocks/nextNavigation" - -describe("Handle not found wrapper utility", () => { - test("Should call notFound() for errors with status 404", async () => { - const error: Partial = { - status: 404, - message: "Not Found", - } - - const promise = Promise.reject(error) - - await handleNotFound(promise) - - expect(nextNavigationMocks.notFound).toHaveBeenCalled() - }) - - test("Should not call notFound() for success and return result", async () => { - const resolvedValue = { data: "success" } - const promise = Promise.resolve(resolvedValue) - - const result = await handleNotFound(promise) - - expect(result).toEqual(resolvedValue) - expect(nextNavigationMocks.notFound).not.toHaveBeenCalled() - }) - - test("Should rethrow non 404 errors", async () => { - const error = new Error("Something went wrong") - - const promise = Promise.reject(error) - - await expect(handleNotFound(promise)).rejects.toThrow( - "Something went wrong", - ) - expect(nextNavigationMocks.notFound).not.toHaveBeenCalled() - }) -}) diff --git a/frontends/main/src/common/handleNotFound.ts b/frontends/main/src/common/handleNotFound.ts deleted file mode 100644 index f34a77bece..0000000000 --- a/frontends/main/src/common/handleNotFound.ts +++ /dev/null @@ -1,24 +0,0 @@ -import type { AxiosError } from "axios" -import { notFound } from "next/navigation" - -/* This is intended to wrap API calls that fetch resources during server render, - * such as to gather metadata for the learning resource drawer. - * - * The ./app/global-error.tsx boundary for root layout errors is only supplied the - * error message so we cannot determine that it is a 404 to show the NotFoundPage. - * Instead we must handle at each point of use so need a utility function. Below we - * use next/navigation's notFound() to render ./app/not-found.tsx - */ - -const handleNotFound = async (promise: Promise): Promise => { - try { - return await promise - } catch (error) { - if ((error as AxiosError).status === 404) { - return notFound() - } - throw error - } -} - -export default handleNotFound diff --git a/frontends/main/src/common/metadata.test.ts b/frontends/main/src/common/metadata.test.ts new file mode 100644 index 0000000000..98db8cf252 --- /dev/null +++ b/frontends/main/src/common/metadata.test.ts @@ -0,0 +1,59 @@ +import type { AxiosError, AxiosResponse } from "axios" +import { safeGenerateMetadata, standardizeMetadata } from "./metadata" +import { nextNavigationMocks } from "ol-test-utilities/mocks/nextNavigation" + +describe("safeGenerateMetadata", () => { + const mockMetadata = { + title: "Test Title", + description: "Test Description", + } + + let consoleErrorSpy: jest.SpyInstance + + beforeEach(() => { + consoleErrorSpy = jest.spyOn(console, "error").mockImplementation() + }) + + afterEach(() => { + consoleErrorSpy.mockRestore() + }) + + test("Should call notFound() for errors with status 404", async () => { + const error: Partial = { + response: { status: 404 } as AxiosResponse, + message: "Not Found", + } + + const fn = jest.fn().mockRejectedValue(error) + + await safeGenerateMetadata(fn) + + expect(nextNavigationMocks.notFound).toHaveBeenCalled() + expect(consoleErrorSpy).not.toHaveBeenCalled() + }) + + test("Should return result on success", async () => { + const fn = jest.fn().mockResolvedValue(mockMetadata) + + const result = await safeGenerateMetadata(fn) + + expect(result).toEqual(mockMetadata) + expect(nextNavigationMocks.notFound).not.toHaveBeenCalled() + expect(consoleErrorSpy).not.toHaveBeenCalled() + }) + + test("Should return standardized metadata for non-404 errors", async () => { + const error = new Error("Something went wrong") + const fn = jest.fn().mockRejectedValue(error) + + const result = await safeGenerateMetadata(fn) + const standardizedMetadata = await standardizeMetadata() + + expect(result).toEqual(standardizedMetadata) + expect(nextNavigationMocks.notFound).not.toHaveBeenCalled() + expect(consoleErrorSpy).toHaveBeenCalledWith( + "Error fetching page metadata", + error, + ) + }) +}) diff --git a/frontends/main/src/common/metadata.ts b/frontends/main/src/common/metadata.ts index 1a881cd827..77f331f777 100644 --- a/frontends/main/src/common/metadata.ts +++ b/frontends/main/src/common/metadata.ts @@ -2,10 +2,12 @@ import { canonicalResourceDrawerUrl, RESOURCE_DRAWER_PARAMS, } from "@/common/urls" -import { learningResourcesApi } from "api/clients" +import type { AxiosError } from "axios" import type { Metadata } from "next" import * as Sentry from "@sentry/nextjs" -import handleNotFound from "./handleNotFound" +import { learningResourceQueries } from "api/hooks/learningResources" +import { getServerQueryClient } from "api/ssr/serverQueryClient" +import { notFound } from "next/navigation" const DEFAULT_OG_IMAGE = "/images/learn-og-image.jpg" @@ -18,6 +20,33 @@ type MetadataAsyncProps = { social?: boolean } & Metadata +/** + * Wraps the metadata generation function in a try/catch block. Uncaught or + * rethrown errors in generateMetadata result in showing the fallback error page, + * which is heavy handed for metadata generation errors. + * + * Axios error cannot be serialized as they contain function values and circular references. + * These result in "Functions cannot be passed directly to Client Components" errors (in production build). + * + * Instead, we catch the error and return the fallback of default metadata. + * + * If the error is a 404, we show the not found page. + */ +export async function safeGenerateMetadata( + fn: () => Promise, +): Promise { + try { + return await fn() + } catch (error: unknown) { + if ((error as AxiosError)?.response?.status === 404) { + return notFound() + } + console.error("Error fetching page metadata", error) + Sentry.captureException(error) + return await standardizeMetadata() + } +} + /* * Fetch metadata for the current page. * the method handles resource param override if necessary. @@ -40,27 +69,17 @@ export const getMetadataAsync = async ({ ? Number(learningResourceIds[0]) : Number(learningResourceIds) const alts = alternates ?? {} - if (learningResourceId) { - try { - const { data } = await handleNotFound( - learningResourcesApi.learningResourcesRetrieve({ - id: learningResourceId, - }), - ) - title = data?.title - description = data?.description?.replace(/<\/[^>]+(>|$)/g, "") ?? "" - image = data?.image?.url || image - imageAlt = image === data?.image?.url ? imageAlt : data?.image?.alt || "" - alts.canonical = canonicalResourceDrawerUrl(learningResourceId) - } catch (error) { - Sentry.captureException(error) - console.error( - "Error fetching learning resource for page metadata", - learningResourceId, - error, - ) - } + if (learningResourceId) { + const queryClient = getServerQueryClient() + const data = await queryClient.fetchQuery( + learningResourceQueries.detail(learningResourceId), + ) + title = data?.title + description = data?.description?.replace(/<\/[^>]+(>|$)/g, "") ?? "" + image = data?.image?.url || image + imageAlt = image === data?.image?.url ? imageAlt : data?.image?.alt || "" + alts.canonical = canonicalResourceDrawerUrl(learningResourceId) } return standardizeMetadata({ @@ -87,7 +106,7 @@ export const standardizeMetadata = ({ imageAlt, social = true, ...otherMeta -}: MetadataProps): Metadata => { +}: MetadataProps = {}): Metadata => { title = `${title} | ${process.env.NEXT_PUBLIC_SITE_NAME}` const socialMetadata = social ? { diff --git a/frontends/main/src/middleware.ts b/frontends/main/src/middleware.ts deleted file mode 100644 index f713676d2b..0000000000 --- a/frontends/main/src/middleware.ts +++ /dev/null @@ -1,25 +0,0 @@ -import { NextResponse } from "next/server" -import type { NextRequest } from "next/server" - -/* The CDNs handle SSL redirects, but this is intended to block content being served - * over HTTP if directly addressed at their origin server domains. - */ -export function middleware(request: NextRequest) { - const protocol = request.headers.get("x-forwarded-proto") - const host = request.headers.get("host") - - if ( - protocol !== "https" && - process.env.NODE_ENV === "production" && - /* This is included so we can test the build locally with yarn build; yarn start; - * without needing an additional APP_ENV variable (Next.js builds with NODE_ENV=production always). - * We're typically using open.odl.local or just localhost. - */ - !host?.includes("local") - ) { - const url = `https://${host}${request.nextUrl.pathname}${request.nextUrl.searchParams.toString() ? `?${request.nextUrl.searchParams}` : ""}` - return NextResponse.redirect(url, 301) - } - - return NextResponse.next() -} diff --git a/frontends/main/src/test-utils/index.tsx b/frontends/main/src/test-utils/index.tsx index c88bf7dc73..6658fc8c61 100644 --- a/frontends/main/src/test-utils/index.tsx +++ b/frontends/main/src/test-utils/index.tsx @@ -5,7 +5,7 @@ import { ThemeProvider } from "ol-components" import { Provider as NiceModalProvider } from "@ebay/nice-modal-react" import type { QueryClient } from "@tanstack/react-query" -import { makeQueryClient } from "@/app/getQueryClient" +import { makeBrowserQueryClient } from "@/app/getQueryClient" import { render } from "@testing-library/react" import { factories, setMockResponse } from "api/test-utils" import type { User } from "api/hooks/user" @@ -65,7 +65,7 @@ const renderWithProviders = ( const allOpts = { ...defaultTestAppOptions, ...options } const { url } = allOpts - const queryClient = makeQueryClient() + const queryClient = makeBrowserQueryClient() if (allOpts.user) { const user = { ...defaultUser, ...allOpts.user } diff --git a/yarn.lock b/yarn.lock index 482e84ed98..41b8652d24 100644 --- a/yarn.lock +++ b/yarn.lock @@ -1728,11 +1728,11 @@ __metadata: linkType: hard "@emnapi/runtime@npm:^1.5.0": - version: 1.6.0 - resolution: "@emnapi/runtime@npm:1.6.0" + version: 1.7.0 + resolution: "@emnapi/runtime@npm:1.7.0" dependencies: tslib: "npm:^2.4.0" - checksum: 10/88f685ecb23df070a61447bf61b12a113b7edecc248969e1dc18e4637ee8519389cde8b95c22b2144de41490b42aedc6a791fe1b00940a02fdeaadac1352bbf6 + checksum: 10/4dc726eb42fe2c7777fd32090f3e5e006c630e1a732538139caa18daf586e883e81c562cd69b0622db16e76bb572a2dde30711494edcee4a34059b62f5f46267 languageName: node linkType: hard @@ -7633,6 +7633,13 @@ __metadata: languageName: node linkType: hard +"async_hooks@npm:^1.0.0": + version: 1.0.0 + resolution: "async_hooks@npm:1.0.0" + checksum: 10/d31a4cb971a980b095cde18879598d1b196b32cb38905d1c2ed0aca7ebc7e040344af52fe2dedba410a1eaf4bc12d63db797b91285048387cac78319a6366901 + languageName: node + linkType: hard + "asynckit@npm:^0.4.0": version: 0.4.0 resolution: "asynckit@npm:0.4.0" @@ -11265,9 +11272,9 @@ __metadata: linkType: hard "fuse.js@npm:^7.0.0": - version: 7.1.0 - resolution: "fuse.js@npm:7.1.0" - checksum: 10/9f9105e54372897a46cb3e04074f0db5bd0a428320d4618276a57e6142d7502235a556f05cf87aa3c5d6d9c6fdfa06b901b78379c48aa0951672ccbc4a1bfe70 + version: 7.0.0 + resolution: "fuse.js@npm:7.0.0" + checksum: 10/d75d35f2d61afa85b8248f9cbfc7d4df29ae47ea574a15ad5c3c2a41930c5ed78668346295508b59ec4929fcb1a5cd6d9a8c649b5a3bc8b18e515f4e4cb9809d languageName: node linkType: hard @@ -14077,6 +14084,7 @@ __metadata: "@types/react-slick": "npm:^0.23.13" "@types/slick-carousel": "npm:^1" api: "workspace:*" + async_hooks: "npm:^1.0.0" classnames: "npm:^2.5.1" eslint: "npm:8.57.1" eslint-config-next: "npm:^14.2.7" From 5204f222a6a7bfdc2a931d27947a36fa60435ab0 Mon Sep 17 00:00:00 2001 From: Shankar Ambady Date: Mon, 3 Nov 2025 21:22:26 -0500 Subject: [PATCH 5/7] Shanbady/fix subscription groups (#2664) * making sure query is picked from something user is a member of * adding initial solution * adding some variance in grouping * test definition * docstring updates * docstring updates --- learning_resources_search/tasks.py | 28 ++--- learning_resources_search/tasks_test.py | 132 ++++++++++++++++++++++++ 2 files changed, 147 insertions(+), 13 deletions(-) diff --git a/learning_resources_search/tasks.py b/learning_resources_search/tasks.py index 9e9b2b1354..0685518561 100644 --- a/learning_resources_search/tasks.py +++ b/learning_resources_search/tasks.py @@ -221,15 +221,20 @@ def _get_percolated_rows(resources, subscription_type): if percolated.count() > 0: percolated_users = set(percolated.values_list("users", flat=True)) all_users.update(percolated_users) - query = percolated.first() - search_url = _infer_percolate_group_url(query) - req = PreparedRequest() - req.prepare_url(search_url, {"resource": resource.id}) - resource_url = req.url - source_channel = query.source_channel() - - rows.extend( - [ + for user in percolated_users: + if not user: + continue + user_instance = User.objects.get(id=user) + user_queries = user_instance.percolate_queries.values_list( + "id", flat=True + ) + query = percolated.filter(id__in=user_queries).order_by("?").first() + search_url = _infer_percolate_group_url(query) + req = PreparedRequest() + req.prepare_url(search_url, {"resource": resource.id}) + resource_url = req.url + source_channel = query.source_channel() + rows.append( { "resource_url": resource_url, "resource_title": resource.title, @@ -247,10 +252,7 @@ def _get_percolated_rows(resources, subscription_type): "group": _infer_percolate_group(query), "search_url": search_url, } - for user in percolated_users - ] - ) - + ) return rows diff --git a/learning_resources_search/tasks_test.py b/learning_resources_search/tasks_test.py index 2cde6227d9..bd106dbddd 100644 --- a/learning_resources_search/tasks_test.py +++ b/learning_resources_search/tasks_test.py @@ -16,6 +16,7 @@ LearningResourceDepartmentFactory, LearningResourceFactory, LearningResourceOfferorFactory, + LearningResourceTopicFactory, ProgramFactory, ) from learning_resources.models import LearningResource @@ -855,6 +856,137 @@ def test_infer_percolate_group(mocked_api): assert _infer_percolate_group(offerer_query) == offerer.name +def test_percolate_user_grouping(mocked_api, mocker): + """ + Test that each user receives an email with resources + they are supposed to recieve (based off of subscription) + """ + topic_name = "Mechanical Engineering" + topic = LearningResourceTopicFactory.create(name=topic_name) + alternate_topic = LearningResourceTopicFactory.create( + name=f"{topic_name}_alternate" + ) + offerer = LearningResourceOfferorFactory.create() + department = LearningResourceDepartmentFactory.create() + alternate_department = LearningResourceDepartmentFactory.create() + + resource_a = LearningResourceFactory.create( + title="resource A", + topics=[topic, alternate_topic], + is_course=True, + offered_by=offerer, + departments=[department], + ) + + resource_b = LearningResourceFactory.create( + title="resource B", + topics=[alternate_topic], + is_course=True, + ) + resource_c = LearningResourceFactory.create( + title="resource C", + departments=[alternate_department], + is_course=True, + topics=[], + ) + + user_a, user_b, user_c, user_d = UserFactory.create_batch(4) + + topic_query = PercolateQueryFactory.create() + topic_query.original_query["topic"] = [topic_name] + + alternate_topic_query = PercolateQueryFactory.create() + alternate_topic_query.original_query["topic"] = [f"{topic_name}_alternate"] + + department_query = PercolateQueryFactory.create() + department_query.original_query["department"] = [department.department_id] + + alternate_department_query = PercolateQueryFactory.create() + alternate_department_query.original_query["department"] = [ + alternate_department.department_id + ] + + offerer_query = PercolateQueryFactory.create() + offerer_query.source_type = PercolateQuery.CHANNEL_SUBSCRIPTION_TYPE + offerer_query.original_query["offered_by"] = [offerer.code] + + # user_a should have 1 percolated doc + topic_query.users.add(user_a) + + # user_b should have 3 percolated docs + topic_query.users.add(user_b) + alternate_topic_query.users.add(user_b) + alternate_department_query.users.add(user_b) + + # user_c should have 2 percolated doc + department_query.users.add(user_c) + alternate_department_query.users.add(user_c) + + # user_d should have 1 percolated doc + offerer_query.users.add(user_d) + + # save all the queries + for query in [ + department_query, + alternate_topic_query, + offerer_query, + topic_query, + alternate_department_query, + ]: + query.source_type = PercolateQuery.CHANNEL_SUBSCRIPTION_TYPE + query.save() + + percolate_matches_for_document_mock = mocker.patch( + "learning_resources_search.tasks.percolate_matches_for_document", + ) + + def _matches_for_document(resource_id): + """ + Mock percolation + """ + if resource_id == resource_a.id: + return PercolateQuery.objects.filter( + id__in=[ + topic_query.id, + alternate_topic_query.id, + department_query.id, + offerer_query.id, + ] + ) + elif resource_id == resource_b.id: + return PercolateQuery.objects.filter( + id__in=[ + alternate_topic_query.id, + ] + ) + elif resource_id == resource_c.id: + return PercolateQuery.objects.filter( + id__in=[ + alternate_department_query.id, + ] + ) + else: + return PercolateQuery.objects.none() + + percolate_matches_for_document_mock.side_effect = _matches_for_document + + rows = _get_percolated_rows( + [resource_a, resource_b, resource_c], "channel_subscription_type" + ) + grouped_by_user = _group_percolated_rows(rows) + resources_by_user = {} + # get the total number of resources for each user + for user in [user_a, user_b, user_c, user_d]: + resources_by_user[user.id] = sum( + [len(items) for items in grouped_by_user[user.id].values()] + ) + + assert resources_by_user[user_a.id] == 1 + assert resources_by_user[user_b.id] == 3 + assert resources_by_user[user_c.id] == 2 + assert resources_by_user[user_d.id] == 1 + + def test_email_grouping_function(mocked_api, mocker): """ Test that template data for digest emails are grouped correctly From 93763967de1162c9def75f06d77d667c89cec64c Mon Sep 17 00:00:00 2001 From: Carey P Gumaer Date: Tue, 4 Nov 2025 11:02:37 -0500 Subject: [PATCH 6/7] don't use next_start_date for "anytime" courses & programs (#2670) * don't use next_start_date for "anytime" courses & programs * add a test --- .../InfoSection.test.tsx | 27 +++++++++++++++++++ .../LearningResourceExpanded/InfoSection.tsx | 5 ++-- 2 files changed, 30 insertions(+), 2 deletions(-) diff --git a/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.test.tsx b/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.test.tsx index bf1c5645ed..865cd5df55 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.test.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.test.tsx @@ -226,6 +226,33 @@ describe("Learning resource info section start date", () => { await user.click(showMoreLink) expect(runDates.children.length).toBe(totalRuns + 1) }) + + test("Anytime courses with next_start_date should not replace first date in 'As taught in' section", () => { + const course = { + ...courses.free.anytime, + next_start_date: "2024-03-15T00:00:00Z", + } + + renderWithTheme() + + const section = screen.getByTestId("drawer-info-items") + + within(section).getByText("Starts:") + within(section).getByText("Anytime") + + within(section).getByText("As taught in:") + + expect(within(section).queryByText("March 15, 2024")).toBeNull() + + const runDates = within(section).getByTestId("drawer-run-dates") + expect(runDates).toBeInTheDocument() + + const firstRun = course.runs?.[0] + invariant(firstRun) + const firstRunDate = formatRunDate(firstRun, true) + invariant(firstRunDate) + expect(within(section).getByText(firstRunDate)).toBeInTheDocument() + }) }) describe("Learning resource info section format and location", () => { diff --git a/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.tsx b/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.tsx index f07ab87a22..e120fa771e 100644 --- a/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.tsx +++ b/frontends/main/src/page-components/LearningResourceExpanded/InfoSection.tsx @@ -180,6 +180,7 @@ const totalRunsWithDates = (resource: LearningResource) => { const RunDates: React.FC<{ resource: LearningResource }> = ({ resource }) => { const [showingMore, setShowingMore] = useState(false) + const anytime = showStartAnytime(resource) let sortedDates = resource.runs ?.sort((a, b) => { if (a?.start_date && b?.start_date) { @@ -187,14 +188,14 @@ const RunDates: React.FC<{ resource: LearningResource }> = ({ resource }) => { } return 0 }) - .map((run) => formatRunDate(run, showStartAnytime(resource))) + .map((run) => formatRunDate(run, anytime)) .filter((date) => date !== null) const nextStartDate = resource.next_start_date ? formatDate(resource.next_start_date, "MMMM DD, YYYY") : null - if (sortedDates && nextStartDate) { + if (sortedDates && nextStartDate && !anytime) { // Replace the first date with next_start_date sortedDates = [nextStartDate, ...sortedDates.slice(1)] } From 7e67fa3c9176e2b701b45d434edb8a48bb2a211d Mon Sep 17 00:00:00 2001 From: Doof Date: Tue, 4 Nov 2025 19:16:22 +0000 Subject: [PATCH 7/7] Release 0.47.9 --- RELEASE.rst | 10 ++++++++++ main/settings.py | 2 +- 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/RELEASE.rst b/RELEASE.rst index 715c8ab623..bb3c18bdc3 100644 --- a/RELEASE.rst +++ b/RELEASE.rst @@ -1,6 +1,16 @@ Release Notes ============= +Version 0.47.9 +-------------- + +- don't use next_start_date for "anytime" courses & programs (#2670) +- Shanbady/fix subscription groups (#2664) +- Server side query retry and caching with request scoped QueryClient (#2596) +- alter org page header based on new requirements (#2659) +- Get rid of debug code (#2661) +- Eliminate build-time API calls, default to dynamic rendering (#2660) + Version 0.47.8 (Released October 29, 2025) -------------- diff --git a/main/settings.py b/main/settings.py index c5d47ab0cd..8a501843ed 100644 --- a/main/settings.py +++ b/main/settings.py @@ -34,7 +34,7 @@ from main.settings_pluggy import * # noqa: F403 from openapi.settings_spectacular import open_spectacular_settings -VERSION = "0.47.8" +VERSION = "0.47.9" log = logging.getLogger()