Skip to content

Server rendering for homepage, units and topics listing pages #1822

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 32 commits into from
Nov 20, 2024

Conversation

jonkafton
Copy link
Contributor

@jonkafton jonkafton commented Nov 12, 2024

What are the relevant tickets?

Closes:
Next.js: Homepage: Prefetch API calls #5920
Next.js: Topics, Units and Channels pages: Prefetch API calls #6036

Description (What does it do?)

Homepage

Makes API calls needed to render homepage learning resources on the server so that these prerender:

  • Featured Courses carousel "All"
  • Featured Courses carousel "Free"
  • Featured Courses carousel "With Certificate"
  • Featured Courses carousel "Professional & Executive Learning"
  • Media carousel "All"
  • Media carousel "Videos"
  • Media carousel "Podcasts"
  • Browse by Topic section

These are all learning resources, however the learning resource endpoints return user specific data, namely the user lists each resource may be a member of. This PR #1808 provides a new endpoint to query for user list relationships independently of the learning resources endpoints, enabling us to querying them publicly for server rendering.

There is the follow up task to implement these endpoints in the frontend so that the server rendered learning resources in this PR are then furnished with their user list memberships in the browser.

In the meantime, this PR fetches the public learning resources on the server, then invalidates the cache so that they are refetched on the client complete with the user specific data. This required commenting out the featured list shuffle behavior to prevent the content flicker as the client rendered resource cards replace the server rendered version. This will be reinstated with this task: https://github.com/mitodl/hq/issues/6032.

Unit Listing page /units

Updates to make a single call /api/v0/channels/?channel_type=unit, replacing calls to each channel detail page in the unit cards. Prefetches channels list and count by unit.

Topics Listing page /topics

Prefetches topics list and counts by topic.

How can this be tested?

These routes should server render all public content without any prefetch mismatch warnings in the browser console:

  • /
  • /units
  • /topics

Additional Context

Also adds reusable server PageParams type and cleans up the api key factory imports.

@jonkafton jonkafton marked this pull request as draft November 12, 2024 20:40
@jonkafton jonkafton changed the base branch from main to jk/5919-prefetch-mechanism November 12, 2024 20:41
@jonkafton jonkafton changed the base branch from jk/5919-prefetch-mechanism to main November 13, 2024 20:00
@jonkafton jonkafton changed the title Fetch homepage API calls on the server for prerendering Fetch API calls on the server for prerendering Nov 15, 2024
@jonkafton jonkafton force-pushed the jk/5920-prefetch-homepage branch from e59621f to 11d76cf Compare November 18, 2024 17:17
@jonkafton jonkafton marked this pull request as ready for review November 18, 2024 19:16
@jonkafton jonkafton added the Needs Review An open Pull Request that is ready for review label Nov 18, 2024
@jonkafton jonkafton changed the title Fetch API calls on the server for prerendering Server rendering for homepage, units and topics listing pages Nov 18, 2024
Copy link
Contributor

@gumaerc gumaerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just noticed one commented out mock in a test below that I think should be removed, but I was also wondering about the add to learning path / userlist buttons. #1808 has been merged, but what's left to be done on that before randomization can be turned back on? Is it just the buttons themselves that need to be updated to use the new API endpoints?

channel_url: `${window.location.origin}/units/${unit.code}`,
})
})
// units.forEach((unit) => {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Now that this page is using the channel list endpoint to get the details, this is meant to be removed right?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, missed that - removed.

@jonkafton
Copy link
Contributor Author

jonkafton commented Nov 19, 2024

#1808 has been merged, but what's left to be done on that before randomization can be turned back on?

This has to be done to make use of the new membership endpoint - https://github.com/mitodl/hq/issues/6032. The idea is that the learning resources will be fetched on the server and we'll query for memberships on the client to update the user lists. With that we can turn off the learning resource cache invalidation added with this PR to refetch for the memberships, so can reinstate the randomization.

@jonkafton jonkafton requested a review from gumaerc November 19, 2024 20:58
Copy link
Contributor

@gumaerc gumaerc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@jonkafton jonkafton force-pushed the jk/5920-prefetch-homepage branch from c92adf0 to f4b10d6 Compare November 20, 2024 15:27
@jonkafton jonkafton merged commit c09f9b5 into main Nov 20, 2024
17 checks passed
jonkafton added a commit that referenced this pull request Nov 20, 2024
mbertrand pushed a commit that referenced this pull request Nov 22, 2024
* 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
@odlbot odlbot mentioned this pull request Nov 25, 2024
19 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Needs Review An open Pull Request that is ready for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants