Server rendering for homepage, units and topics listing pages (#1847)

* Warn on API calls during initial render not prefetched * Full prefetch for homepage (commented) * Prefetch utility * Check for queries prefetched that are not needed during render and warn * No need to stringify * Replace useQuery overrides with decoupled cache check (wip) * Observer count for unnecessary prefetch warnings * Remove useQuery override * Test prefetch warnings * Remove inadvertent/unnecessary diff * Remove comments * Prefetch homepage. Invalidate learning resource cache items * Remove comment * Update comment * Temp remove featured resource list shuffle * Remove unused * Prefetch calls * Prefetch topics page * Rename key factory re-exports * Units page prefetch * Single request for all unit channel details * Update unit listing page tests * Page arg types * Optional route params * Remove comment * Remove unused * Remove comment * Reinstate featured list shuffle and remove cache invalidation to refetch resources for user * Add news to server render
mitodl · Nov 25, 2024 · f2758b7 · f2758b7
1 parent 48cb5e0
commit f2758b7
Show file tree

Hide file tree

Showing 27 changed files with 272 additions and 173 deletions.
diff --git a/frontends/api/src/hooks/channels/index.ts b/frontends/api/src/hooks/channels/index.ts
@@ -55,5 +55,5 @@ export {
   useChannelsList,
   useChannelPartialUpdate,
   useChannelCounts,
-  channels as channelsKeyFactory,
+  channels,
 }
diff --git a/frontends/api/src/hooks/learningResources/index.ts b/frontends/api/src/hooks/learningResources/index.ts
@@ -150,5 +150,5 @@ export {
   useSchoolsList,
   useSimilarLearningResources,
   useVectorSimilarLearningResources,
-  learningResources as learningResourcesKeyFactory,
+  learningResources,
 }
diff --git a/frontends/api/src/hooks/newsEvents/index.ts b/frontends/api/src/hooks/newsEvents/index.ts
@@ -19,5 +19,5 @@ export {
   useNewsEventsList,
   useNewsEventsDetail,
   NewsEventsListFeedTypeEnum,
-  newsEvents as newsEventsKeyFactory,
+  newsEvents,
 }
diff --git a/frontends/api/src/hooks/testimonials/index.ts b/frontends/api/src/hooks/testimonials/index.ts
@@ -23,8 +23,4 @@ const useTestimonialDetail = (id: number | undefined) => {
   })
 }
 
-export {
-  useTestimonialDetail,
-  useTestimonialList,
-  testimonials as testimonialsKeyFactory,
-}
+export { useTestimonialDetail, useTestimonialList, testimonials }
diff --git a/frontends/api/src/ssr/prefetch.ts b/frontends/api/src/ssr/prefetch.ts
@@ -1,13 +1,19 @@
 import { QueryClient, dehydrate } from "@tanstack/react-query"
 import type { Query } from "@tanstack/react-query"
 
-// Utility to avoid repetition in server components
-export const prefetch = async (queries: (Query | unknown)[]) => {
-  const queryClient = new QueryClient()
+/* Utility to avoid repetition in server components
+ * Optionally pass the queryClient returned from a previous prefetch
+ * where queries are dependent on previous results
+ */
+export const prefetch = async (
+  queries: (Query | unknown)[],
+  queryClient?: QueryClient,
+) => {
+  queryClient = queryClient || new QueryClient()
 
   await Promise.all(
     queries.map((query) => queryClient.prefetchQuery(query as Query)),
   )
 
-  return dehydrate(queryClient)
+  return { dehydratedState: dehydrate(queryClient), queryClient }
 }
diff --git a/frontends/api/src/ssr/usePrefetchWarnings.test.ts b/frontends/api/src/ssr/usePrefetchWarnings.test.ts
@@ -4,7 +4,7 @@ import { usePrefetchWarnings } from "./usePrefetchWarnings"
 import { setupReactQueryTest } from "../hooks/test-utils"
 import { urls, factories, setMockResponse } from "../test-utils"
 import {
-  learningResourcesKeyFactory,
+  learningResources,
   useLearningResourcesDetail,
 } from "../hooks/learningResources"
 
@@ -45,7 +45,7 @@ describe("SSR prefetch warnings", () => {
         expect.objectContaining({
           disabled: false,
           initialStatus: "loading",
-          key: learningResourcesKeyFactory.detail(1).queryKey,
+          key: learningResources.detail(1).queryKey,
           observerCount: 1,
         }),
       ],
@@ -65,7 +65,7 @@ describe("SSR prefetch warnings", () => {
       wrapper,
       initialProps: {
         queryClient,
-        exemptions: [learningResourcesKeyFactory.detail(1).queryKey],
+        exemptions: [learningResources.detail(1).queryKey],
       },
     })
 
@@ -83,7 +83,7 @@ describe("SSR prefetch warnings", () => {
     const { unmount } = renderHook(
       () =>
         useQuery({
-          ...learningResourcesKeyFactory.detail(1),
+          ...learningResources.detail(1),
           initialData: data,
         }),
       { wrapper },
@@ -105,9 +105,9 @@ describe("SSR prefetch warnings", () => {
       [
         {
           disabled: false,
-          hash: JSON.stringify(learningResourcesKeyFactory.detail(1).queryKey),
+          hash: JSON.stringify(learningResources.detail(1).queryKey),
           initialStatus: "success",
-          key: learningResourcesKeyFactory.detail(1).queryKey,
+          key: learningResources.detail(1).queryKey,
           observerCount: 0,
           status: "success",
         },

diff --git a/frontends/api/src/test-utils/factories/channels.ts b/frontends/api/src/test-utils/factories/channels.ts
@@ -148,6 +148,7 @@ const _channelShared = (): Partial<Omit<Channel, "channel_type">> => {
       key: faker.lorem.slug(),
       value: faker.lorem.slug(),
     }),
+    channel_url: `${faker.internet.url({ appendSlash: false })}${faker.system.directoryPath()}`,
   }
 }
 

diff --git a/frontends/api/src/test-utils/urls.ts b/frontends/api/src/test-utils/urls.ts
@@ -8,6 +8,7 @@
 import type {
   NewsEventsApiNewsEventsListRequest,
   TestimonialsApi,
+  ChannelsApi,
 } from "../generated/v0"
 import type {
   LearningResourcesApi as LRApi,
@@ -186,6 +187,8 @@ const channels = {
   details: (channelType: string, name: string) =>
     `${API_BASE_URL}/api/v0/channels/type/${channelType}/${name}/`,
   patch: (id: number) => `${API_BASE_URL}/api/v0/channels/${id}/`,
+  list: (params?: Paramsv0<ChannelsApi, "channelsList">) =>
+    `${API_BASE_URL}/api/v0/channels/${query(params)}`,
 }
 
 const widgetLists = {

diff --git a/frontends/main/src/app-pages/UnitsListingPage/UnitCard.tsx b/frontends/main/src/app-pages/UnitsListingPage/UnitCard.tsx
@@ -1,5 +1,6 @@
 import React from "react"
-import { LearningResourceOfferorDetail, OfferedByEnum } from "api"
+import type { OfferedByEnum } from "api"
+import type { UnitChannel } from "api/v0"
 import {
   Card,
   Skeleton,
@@ -8,7 +9,6 @@ import {
   theme,
   UnitLogo,
 } from "ol-components"
-import { useChannelDetail } from "api/hooks/channels"
 import Link from "next/link"
 
 const CardStyled = styled(Card)({
@@ -102,25 +102,23 @@ const CountsText = styled(Typography)(({ theme }) => ({
 }))
 
 interface UnitCardsProps {
-  units: LearningResourceOfferorDetail[] | undefined
+  channels: UnitChannel[] | undefined
   courseCounts: Record<string, number>
   programCounts: Record<string, number>
 }
 
 interface UnitCardProps {
-  unit: LearningResourceOfferorDetail
+  channel: UnitChannel
   courseCount: number
   programCount: number
 }
 
 const UnitCard: React.FC<UnitCardProps> = (props) => {
-  const { unit, courseCount, programCount } = props
-  const channelDetailQuery = useChannelDetail("unit", unit.code)
-  const channelDetail = channelDetailQuery.data
-  const unitUrl = channelDetail?.channel_url
+  const { channel, courseCount, programCount } = props
+  const unit = channel.unit_detail.unit
 
-  if (!unitUrl) return null
-  const href = unitUrl && new URL(unitUrl).pathname
+  if (!channel.channel_url) return null
+  const href = new URL(channel.channel_url).pathname
 
   return (
     <CardStyled forwardClicksToLink data-testid={`unit-card-${unit.code}`}>
@@ -134,9 +132,7 @@ const UnitCard: React.FC<UnitCardProps> = (props) => {
             </LogoContainer>
             <CardBottom>
               <ValuePropContainer>
-                <HeadingText>
-                  {channelDetail?.configuration?.heading}
-                </HeadingText>
+                <HeadingText>{channel?.configuration?.heading}</HeadingText>
               </ValuePropContainer>
               <CountsTextContainer>
                 <CountsText data-testid={`course-count-${unit.code}`}>
@@ -174,17 +170,18 @@ export const UnitCardLoading = () => {
 }
 
 export const UnitCards: React.FC<UnitCardsProps> = (props) => {
-  const { units, courseCounts, programCounts } = props
+  const { channels, courseCounts, programCounts } = props
   return (
     <>
-      {units?.map((unit) => {
+      {channels?.map((channel) => {
+        const unit = channel.unit_detail.unit
         const courseCount = courseCounts[unit.code] || 0
         const programCount = programCounts[unit.code] || 0
 
         return unit.value_prop ? (
           <UnitCard
             key={unit.code}
-            unit={unit}
+            channel={channel}
             courseCount={courseCount}
             programCount={programCount}
           />

diff --git a/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx b/frontends/main/src/app-pages/UnitsListingPage/UnitsListingPage.test.tsx
@@ -2,44 +2,71 @@ import React from "react"
 import { renderWithProviders, screen, waitFor, within } from "@/test-utils"
 import UnitsListingPage from "./UnitsListingPage"
 import { factories, setMockResponse, urls } from "api/test-utils"
+import { ChannelTypeEnum } from "api/v0"
+import type { UnitChannel } from "api/v0"
 import { assertHeadings } from "ol-test-utilities"
 
-describe("DepartmentListingPage", () => {
+describe("UnitListingPage", () => {
   const setupApis = () => {
-    const make = factories.learningResources
-    const academicUnit1 = make.offeror({
-      code: "academicUnit1",
-      name: "Academic Unit 1",
-      value_prop: "Academic Unit 1 value prop",
-      professional: false,
+    const make = factories.channels
+    const academicUnit1 = make.channel({
+      channel_type: ChannelTypeEnum.Unit,
+      name: "academicUnit1",
+      title: "Academic Unit 1",
+      unit_detail: {
+        unit: {
+          value_prop: "Academic Unit 1 value prop",
+          professional: false,
+        },
+      },
     })
-    const academicUnit2 = make.offeror({
-      code: "academicUnit2",
-      name: "Academic Unit 2",
-      value_prop: "Academic Unit 2 value prop",
-      professional: false,
+    const academicUnit2 = make.channel({
+      channel_type: ChannelTypeEnum.Unit,
+      name: "academicUnit2",
+      title: "Academic Unit 2",
+      unit_detail: {
+        unit: {
+          value_prop: "Academic Unit 2 value prop",
+          professional: false,
+        },
+      },
     })
-    const academicUnit3 = make.offeror({
-      code: "academicUnit3",
-      name: "Academic Unit 3",
-      value_prop: "Academic Unit 3 value prop",
-      professional: false,
+    const academicUnit3 = make.channel({
+      channel_type: ChannelTypeEnum.Unit,
+      name: "academicUnit3",
+      title: "Academic Unit 3",
+      unit_detail: {
+        unit: {
+          value_prop: "Academic Unit 3 value prop",
+          professional: false,
+        },
+      },
     })
 
-    const professionalUnit1 = make.offeror({
-      code: "professionalUnit1",
-      name: "Professional Unit 1",
-      value_prop: "Professional Unit 1 value prop",
-      professional: true,
+    const professionalUnit1 = make.channel({
+      channel_type: ChannelTypeEnum.Unit,
+      name: "professionalUnit1",
+      title: "Professional Unit 1",
+      unit_detail: {
+        unit: {
+          value_prop: "Professional Unit 1 value prop",
+          professional: true,
+        },
+      },
     })
-    const professionalUnit2 = make.offeror({
-      code: "professionalUnit2",
-      name: "Professional Unit 2",
-      value_prop: "Professional Unit 2 value prop",
-      professional: true,
+    const professionalUnit2 = make.channel({
+      channel_type: ChannelTypeEnum.Unit,
+      name: "professionalUnit2",
+      title: "Professional Unit 2",
+      unit_detail: {
+        unit: {
+          value_prop: "Professional Unit 2 value prop",
+          professional: true,
+        },
+      },
     })
 
-    const units = [
+    const unitChannels = [
       academicUnit1,
       academicUnit2,
       academicUnit3,
@@ -63,29 +90,23 @@ describe("DepartmentListingPage", () => {
 
     setMockResponse.get(
       urls.channels.counts("unit"),
-      units.map((unit) => {
+      unitChannels.map((channel) => {
         return {
-          name: unit.code,
+          name: channel.name,
           counts: {
-            courses: courseCounts[unit.code],
-            programs: programCounts[unit.code],
+            courses: courseCounts[channel.name],
+            programs: programCounts[channel.name],
           },
         }
       }),
     )
-    setMockResponse.get(urls.offerors.list(), {
-      count: units.length,
-      results: units,
-    })
-
-    units.forEach((unit) => {
-      setMockResponse.get(urls.channels.details("unit", unit.code), {
-        channel_url: `${window.location.origin}/units/${unit.code}`,
-      })
+    setMockResponse.get(urls.channels.list({ channel_type: "unit" }), {
+      count: unitChannels.length,
+      results: unitChannels,
     })
 
     return {
-      units,
+      unitChannels,
       courseCounts,
       programCounts,
     }
@@ -98,7 +119,7 @@ describe("DepartmentListingPage", () => {
   })
 
   it("Shows unit properties within the proper section", async () => {
-    const { units, courseCounts, programCounts } = setupApis()
+    const { unitChannels, courseCounts, programCounts } = setupApis()
 
     renderWithProviders(<UnitsListingPage />)
 
@@ -116,11 +137,15 @@ describe("DepartmentListingPage", () => {
       return links
     })
 
-    units.forEach((unit) => {
+    unitChannels.forEach((channel) => {
+      const { unit } = (channel as UnitChannel).unit_detail
       const section = unit.professional ? professionalSection : academicSection
       const card = within(section).getByTestId(`unit-card-${unit.code}`)
       const link = within(card).getByRole("link")
-      expect(link).toHaveAttribute("href", `/units/${unit.code}`)
+      expect(link).toHaveAttribute(
+        "href",
+        new URL(channel.channel_url!).pathname,
+      )
 
       const courseCount = courseCounts[unit.code]
       const programCount = programCounts[unit.code]
-Original file line number
+Diff line change
@@ Expand Up @@
           key: faker.lorem.slug(),
           value: faker.lorem.slug(),
         }),
+        channel_url: `${faker.internet.url({ appendSlash: false })}${faker.system.directoryPath()}`,
       }
     }
@@ Expand Down @@