NextJS: add /about and /projects base paths #2519

eatyourgreens · 2021-11-05T10:31:06Z

Set next.config.basePath to /about for content pages and /projects for projects.
Remove the corresponding page paths so that page URLs don't change.
Update asset prefix URLs so that static assets don't break.

Package:
app-project
app-content-pages

Towards #2518.

Review Checklist

General

Are the tests passing locally and on Travis?
Is the documentation up to date?

Components

Has a storybook story been created or updated?
Is the component accessible?
- Can it be used with a screen reader? BBC guide to testing with VoiceOver
- Can it be used from the keyboard? WebAIM guide to keyboard testing
- Is it passing accessibility checks in the storybook?

Apps

Does it work in all major browsers: Firefox, Chrome, Edge, Safari?
Does it work on mobile?
Can you yarn panic && yarn bootstrap or docker-compose up --build and app works as expected?

Publishing

Is the changelog updated?
Are the dependencies updated for apps and libraries that are using the newly published library?

Post-merging

Did the app deploy to https://frontend.preview.zooniverse.org/projects/:project-name/:owner or https://frontend.preview.zooniverse.org/about?
Is the new feature working or bug now fixed?
- Is there a Talk or blog post written to announce the new feature(s)?
Is the design working across browsers (Firefox, Chrome, Edge, Safari) and mobile?
- Is this approved by our designer?
Is this ready for production deployment?

eatyourgreens · 2021-11-05T10:34:58Z

Running a test project locally:
https://local.zooniverse.org:3000/projects/eatyourgreens/-project-testing-ground/classify/workflow/3223

I'm seeing asset requests prefixed as expected 👍
eg. https://local.zooniverse.org:3000/projects/_next/static/chunks/webpack.js?ts=1636108424200

eatyourgreens · 2021-11-05T10:47:59Z

kubernetes/deployment-production.tmpl

@@ -61,7 +61,7 @@ spec:
            - containerPort: 3000
          env:
            - name: PROJECT_ASSET_PREFIX
-              value: https://fe-project.zooniverse.org
+              value: https://fe-project.zooniverse.org/projects


This can be changed to https://www.zooniverse.org/projects, once routing is set up.

eatyourgreens · 2021-11-05T10:49:12Z

kubernetes/deployment-production.tmpl

@@ -153,7 +153,7 @@ spec:
            - containerPort: 3000
          env:
            - name: CONTENT_ASSET_PREFIX
-              value: https://fe-content-pages.zooniverse.org
+              value: https://fe-content-pages.zooniverse.org/about


This can change to https://www.zooniverse.org/about.

eatyourgreens · 2021-11-05T10:49:50Z

kubernetes/deployment-staging.tmpl

@@ -51,7 +51,7 @@ spec:
            - containerPort: 3000
          env:
            - name: PROJECT_ASSET_PREFIX
-              value: https://fe-project.preview.zooniverse.org
+              value: https://fe-project.preview.zooniverse.org/projects


This can change to https://frontend.preview.zooniverse.org/projects.

eatyourgreens · 2021-11-05T10:50:23Z

kubernetes/deployment-staging.tmpl

@@ -135,7 +135,7 @@ spec:
            - containerPort: 3000
          env:
            - name: CONTENT_ASSET_PREFIX
-              value: https://fe-content-pages.preview.zooniverse.org
+              value: https://fe-content-pages.preview.zooniverse.org/about


This can change to https://frontend.preview.zooniverse.org/about.

- Set `next.config.basePath` to `/about` for content pages and `/projects` for projects. - Remove the corresponding page paths so that page URLs don't change. - Update asset prefix URLs so that static assets don't break.

docker-compose.yml

eatyourgreens · 2021-11-10T09:16:18Z

@camallen would it be ok to merge this? I’d like to see how it works behind nginx, on staging.

eatyourgreens · 2021-11-10T09:39:11Z

Looking at the placeholder images for subjects, on a staging page, those are being served directly from https://fe-project.preview.zooniverse.org/subject-placeholder.png. That won't break here, they'll still be served to the browser from fe-project, but that's another static asset that should be served via the CDN.

https://frontend.preview.zooniverse.org/projects/eatyourgreens/-project-testing-ground/classify/workflow/3223?env=staging

eatyourgreens · 2021-11-10T09:46:59Z

There's one other static image that's served directly from the origin at the moment: https://fe-project.preview.zooniverse.org/simple-avatar.png on the team pages.

https://frontend.preview.zooniverse.org/projects/eatyourgreens/-project-testing-ground/about/team?env=staging

camallen

LGTM - happy to get this to staging and iterate. One assumption is this won't impact any production deployment when it ships - we need to be quick to fix any issues if they arise on production.

camallen · 2021-11-10T09:58:49Z

packages/app-content-pages/next.config.js

@@ -29,6 +29,7 @@ const COMMIT_ID = process.env.COMMIT_ID || commitID()

 const nextConfig = {
  assetPrefix,
+  basePath: '/about', 


Should we allow this to be modified by ENV var with the default /about as fallback?

I'm not sure. It changes the URL of all the pages served by the app and I'm not sure I see the value of changing those from /about/team and /about/publications to a different URL.

camallen · 2021-11-10T09:59:36Z

packages/app-project/next.config.js

@@ -19,9 +19,10 @@ function commitID () {
  }
 }

+const basePath = '/projects'


Again - any use in allowing this to be set via ENV var with the above as fallback?

Same here. I'm not sure about the value of changing project URLs away from /projects/:owner:/:project:, with the associated overhead of updating our static setup.

eatyourgreens · 2021-11-10T10:13:56Z

One assumption is this won't impact any production deployment when it ships - we need to be quick to fix any issues if they arise on production.

Agreed. That's why I'd like to get it out on staging: so we can test it out when URL rewriting is in place.

#2519 added /projects as a base path for the project app. This PR updates links, which have been hardcoded with `/projects` and now point to `/projects/projects/:owner:/:name:`.

eatyourgreens added enhancement New feature or request ops Changes that affect how something is built or deployed labels Nov 5, 2021

eatyourgreens requested review from camallen and a team November 5, 2021 10:31

eatyourgreens force-pushed the next-basepaths branch from 28ef9b7 to 27177a2 Compare November 5, 2021 10:36

eatyourgreens commented Nov 5, 2021

View reviewed changes

eatyourgreens mentioned this pull request Nov 5, 2021

Set up routing for NextJS static assets zooniverse/static#251

Closed

NextJS: add /about and /projects base paths

12f85f9

- Set `next.config.basePath` to `/about` for content pages and `/projects` for projects. - Remove the corresponding page paths so that page URLs don't change. - Update asset prefix URLs so that static assets don't break.

eatyourgreens force-pushed the next-basepaths branch from 27177a2 to 12f85f9 Compare November 5, 2021 12:17

camallen reviewed Nov 5, 2021

View reviewed changes

docker-compose.yml Show resolved Hide resolved

eatyourgreens added 2 commits November 9, 2021 10:58

Merge branch 'master' into next-basepaths

391cb61

Merge branch 'master' into next-basepaths

1865fd2

camallen approved these changes Nov 10, 2021

View reviewed changes

github-actions bot added the approved This PR is approved for merging label Nov 10, 2021

eatyourgreens merged commit 3825231 into master Nov 10, 2021

eatyourgreens deleted the next-basepaths branch November 10, 2021 10:14

eatyourgreens mentioned this pull request Nov 10, 2021

Projects: Remove /projects from internal links #2529

Merged

18 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

NextJS: add /about and /projects base paths #2519

NextJS: add /about and /projects base paths #2519

eatyourgreens commented Nov 5, 2021

eatyourgreens commented Nov 5, 2021

eatyourgreens Nov 5, 2021

eatyourgreens Nov 5, 2021

eatyourgreens Nov 5, 2021

eatyourgreens Nov 5, 2021

eatyourgreens commented Nov 10, 2021

eatyourgreens commented Nov 10, 2021

eatyourgreens commented Nov 10, 2021

camallen left a comment

camallen Nov 10, 2021

eatyourgreens Nov 10, 2021

camallen Nov 10, 2021

eatyourgreens Nov 10, 2021

eatyourgreens commented Nov 10, 2021

NextJS: add /about and /projects base paths #2519

NextJS: add /about and /projects base paths #2519

Conversation

eatyourgreens commented Nov 5, 2021

Review Checklist

General

Components

Apps

Publishing

Post-merging

eatyourgreens commented Nov 5, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eatyourgreens commented Nov 10, 2021

eatyourgreens commented Nov 10, 2021

eatyourgreens commented Nov 10, 2021

camallen left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eatyourgreens commented Nov 10, 2021