Skip to content

Conversation

@jonkafton
Copy link
Contributor

@jonkafton jonkafton commented Oct 17, 2024

What are the relevant tickets?

Closes https://github.com/mitodl/hq/issues/5738

Description (What does it do?)

This PR replaces the existing static front end build with Next.js by updating the CI jobs to build and push to Heroku (previously pushing to S3).

This can be merged to main when we are confident that we are ready to deploy Next.js to Production and will be making no further changes to the existing static site build.

Note that once this PR is merged, changes to the nextjs branch will no longer deploy to the Next.js RC - this change moves these steps to the workflows triggered on the release-candidate and release branches.

Changes

  • In the CI workflow (all push) it builds the Docker image as a dry run before changes are merged.

  • In the Production Deploy and Release Candidate Deploy workflows:

    • Builds and pushes the Docker image with Heroku's cli (replacing the static build with Webpack).

    • Purges the Fastly cache (FASTLY_SERVICE_ID_PROD_NEXTJS replacing FASTLY_SERVICE_ID_PROD).

Make it Live

The actual go live will happen once we update Fastly to use mitopen-production-nextjs-6b0d663f9110.herokuapp.com as the origin for requests on learn.mit.edu (currently at https://nextjs.learn.mit.edu/)

When this is merged we also want to do the same for rc.learn.mit.edu. The RC Next.js server is at mitopen-rc-nextjs-4414d6bdb4ac.herokuapp.com.

Post Release

Some infrastructure/Pulumi clean up items:

  • Drop the Fastly references to S3 ol-mitlearn-app-storage-production and ol-mitlearn-app-storage-rc, if they haven't been replaced. We could remove the /frontend dir in these buckets, though they're not doing much harm.

  • We can remove these DNS entries and their respective Fastly config:

    • next.rc.learn.mit.edu
    • nextjs-rc.learn.mit.edu
    • nextjs.learn.mit.edu
  • We can remove these from the GitHub Actions secrets:

    • FASTLY_API_KEY_PROD
    • FASTLY_API_KEY_RC
    • FASTLY_SERVICE_ID_PROD
    • FASTLY_SERVICE_ID_RC

In this repo:

  • We can remove the ./frontends/mit-learn workspace. This contains items we did not migrate to Next.js - article pages and edit channel pages, which we could pull from history if we needed to reanimate.

How can this be tested?

The commit history on this branch shows the CI running and deploying the new Production Next.js service to Heroku (app name: mitopen-production-nextjs) - we have been able to do this in parallel to existing Production as DNS will not point to it until we put it live.

The steps have been moved from the CI workflow to Release Candidate Deploy and Production Deploy, though can only be tested fully when we merge to main.

jonkafton and others added 8 commits October 15, 2024 17:26
* Add location field to LearningResource and LearningResourceRun models (#1604)

* Always delete past events during ETL, and filter them out from api results too just in case (#1660)

* Release 0.21.1

* Release date for 0.21.1

* remove unnecessary padding adjustment on Input base styles (#1666)

* remove unnecessary padding adjustment on Input base styles

* adjust actual textarea padding to match Figma

* Content File Score Adjustment (#1667)

* Release 0.21.2

* Restore program letter intercept view (#1643)

* Restore program letter intercept view

* Generate schema

* Fix test

* fixing import

* Revert "fixing import"

This reverts commit cf56807.

---------

Co-authored-by: shankar ambady <ambady@mit.edu>

* Release date for 0.21.2

* topic detail banner / subtopic logic revisions (#1646)

* add new darker chip variant

* move subtopic display to the banner

* remove max width on banner subtitle texts

* query subtopics only on parent topic channels, and on child topic channels query "related topics" aka the topic's siblings

* don't display the current topic in related topics

* show parent topic breadcrumbs on child topic channel pages

* fix tests

* only do subtopics mocking if necessary

* fix editchannelpage test

* move action buttons under the title on mobile

* fix test

* add new default channel background image

* "toopics"

* fix background position

* remove accidentally committed file

* fix background size on mobile

* remove unnecessary order css rule

* fix banner background on mobile

* fix breadcrumb props

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* remove unnecessary topic id filter

* remove unnecessary breadcrumb fallback

* set the default banner background image in the banner component itself, and only apply large and up 140% background size if the default image is being used

* properly set default background image

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Update Yarn to v4.5.0 (#1624)

* Update Yarn to v4.5.0

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

---------

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

* Clear Cache on Deploy (#1668)

* adding initial cache clear command

* adding cache clear command to heroku release script

* fix docstrings

* removing invalid flag from clear cache command (#1675)

* prevent featured course carousel from re-randomizing (#1673)

* use skipFeatured when invalidating userlists / learning paths after update

* add a new function for updating query data for user_list_parents and learning_path_parents on the featured courses list when either is edited without re-randomizing

* fix learning path invalidations

* fix test

* Shuffling around where the search_update event is fired so it happens in more places (#1679)

- Changed HeroSearch to use the SearchField component instead of SearchInput, so we get the event capture stuff
- Updated SearchInput to not require setPage (for use on the homepage)
- Moved facet events out of the SearchPage and into SearchDisplay so they get called on other pages that use it
- Also trigger the facet events on more things - should also now get sorting changes, choosing between categories, and clearing facets

* Reinstate background steps image

---------

Co-authored-by: Matt Bertrand <mrbertrand@gmail.com>
Co-authored-by: Doof <mitx-devops@mit.edu>
Co-authored-by: Carey P Gumaer <gumaerc@mit.edu>
Co-authored-by: Anastasia Beglova <abeglova@mit.edu>
Co-authored-by: Nathan Levesque <rhysyngsun@users.noreply.github.com>
Co-authored-by: shankar ambady <ambady@mit.edu>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Co-authored-by: Shankar Ambady <shanbady@gmail.com>
Co-authored-by: James Kachel <james@jkachel.com>
* Middleware to set cache headers

* Set headers in the config

* Upgrade Next.js

* Regex to exclude paths with a file extension to apply only to routes

* Remove middleware file

* Instruct to only store in shared cache (not browsers)
* Copy yarn releases

* Build the docker image on all push
* fixing program letter template

* re-enabling test

* adding space
* Increase cache durations

* Update comment
@jonkafton jonkafton marked this pull request as draft October 17, 2024 19:05
ChristopherChudzicki and others added 3 commits October 17, 2024 16:30
* configure sentry, fix error pages

* add env vars to CI

* add global styles to global-error

* TEMPORARY self destruct button

* cleanup

* cleanup error boundary, other misc cleanup

* remove a few useless things

* add a simple test i guess

* remove intentional error triggers
@jonkafton jonkafton marked this pull request as ready for review October 18, 2024 17:38
@jonkafton jonkafton force-pushed the jk/5738-ci-config-nextjs-prod branch from bbfdb77 to 6e1277a Compare October 21, 2024 12:31
@shanbady shanbady self-requested a review October 21, 2024 13:17
@shanbady shanbady self-assigned this Oct 21, 2024
file: coverage/lcov.info

build-deploy-frontend:
build-nextjs-container:
Copy link
Contributor

Choose a reason for hiding this comment

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

I'd be happy to just remove this step—we have yarn workspace main build above. (Which, annoyingly, we do need to run separately. It has to run before typecheck. vercel/next.js#53959 (comment))

If you want to keep it, I would remove the heroku vars.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The intention is to catch issues building the container before that happens on the release branches, but:

  • As you say, we end up doing 2 builds on push (albeit in parallel).
  • It's not the exact same as release branches as there we build using Heroku's CLI heroku container:push (they don't have a build only / dry run command).

It would be good to have some check on the docker build before we get to release-candidate. Can we rethink once this is released? Good call with Heroku vars - removed.

Copy link
Contributor

@shanbady shanbady left a comment

Choose a reason for hiding this comment

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

left a small note about the hard-coded sentry sampling rate

--build-arg NEXT_PUBLIC_POSTHOG_API_KEY=$POSTHOG_API_KEY \
--build-arg NEXT_PUBLIC_SENTRY_DSN=$SENTRY_DSN \
--build-arg NEXT_PUBLIC_SENTRY_ENV=$SENTRY_ENV \
--build-arg NEXT_PUBLIC_SENTRY_PROFILES_SAMPLE_RATE=$SENTRY_PROFILES_SAMPLE_RATE \
Copy link
Contributor

Choose a reason for hiding this comment

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

noting that this appears to currently be hardcoded to 0 in the env section

@jonkafton jonkafton requested a review from shanbady October 21, 2024 14:59
Copy link
Contributor

@shanbady shanbady left a comment

Choose a reason for hiding this comment

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

looks good 👍

@jonkafton jonkafton merged commit 6965103 into nextjs Oct 21, 2024
12 checks passed
@shanbady shanbady deleted the jk/5738-ci-config-nextjs-prod branch October 21, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants