Add version cookie to remember user's version preference (#59532)

Author: heiskr

src/frame/lib/constants.ts

@@ -12,6 +12,7 @@ const DEFAULT_MAX_REQUEST_TIMEOUT = isDev ? 15_000 : 10_000
 
 export const ROOT = process.env.ROOT || '.'
 export const USER_LANGUAGE_COOKIE_NAME = 'user_language'
+export const USER_VERSION_COOKIE_NAME = 'user_version'
 export const TRANSLATIONS_ROOT = process.env.TRANSLATIONS_ROOT || 'translations'
 export const MAX_REQUEST_TIMEOUT = process.env.REQUEST_TIMEOUT
   ? parseInt(process.env.REQUEST_TIMEOUT, 10)

src/frame/middleware/cache-control.ts

@@ -93,6 +93,13 @@ export function languageCacheControl(res: Response): void {
   res.set('vary', 'accept-language, x-user-language')
 }
 
+// Vary on both language and version for homepage redirects
+// x-user-version is a custom request header derived from req.cookie:user_version
+export function languageAndVersionCacheControl(res: Response): void {
+  defaultCacheControl(res)
+  res.set('vary', 'accept-language, x-user-language, x-user-version')
+}
+
 // Long cache control for versioned assets: images, CSS, JS...
 export const assetCacheControl = cacheControlFactory(60 * 60 * 24 * 7, { immutable: true })
 

src/frame/middleware/index.ts

@@ -16,6 +16,7 @@ import {
 import handleErrors from '@/observability/middleware/handle-errors'
 import handleNextDataPath from './handle-next-data-path'
 import detectLanguage from '@/languages/middleware/detect-language'
+import detectVersion from '@/versions/middleware/detect-version'
 import reloadTree from './reload-tree'
 import context from './context/context'
 import shortVersions from '@/versions/middleware/short-versions'
@@ -213,6 +214,7 @@ export default function index(app: Express) {
   // *** Config and context for redirects ***
   app.use(urlDecode) // Must come before detectLanguage to decode @ symbols in version segments
   app.use(detectLanguage) // Must come before context, breadcrumbs, find-page, handle-errors, homepages
+  app.use(detectVersion) // Must come before handle-redirects for version cookie support
   app.use(asyncMiddleware(reloadTree)) // Must come before context
   app.use(asyncMiddleware(context)) // Must come before early-access-*, handle-redirects
   app.use(shortVersions) // Support version shorthands

src/observability/logger/lib/logger-context.ts

@@ -16,6 +16,7 @@ export type LoggerContext = {
   body?: any
   language?: string
   userLanguage?: string
+  userVersion?: string
   version?: string
   pagePath?: string
 }
@@ -51,6 +52,8 @@ const INCLUDE_HEADERS = [
   // Language
   'x-user-language',
   'accept-language',
+  // Version
+  'x-user-version',
   // Host
   'host',
   'x-host',

src/redirects/middleware/handle-redirects.ts

@@ -4,7 +4,11 @@ import patterns from '@/frame/lib/patterns'
 import { pathLanguagePrefixed } from '@/languages/lib/languages-server'
 import { deprecatedWithFunctionalRedirects } from '@/versions/lib/enterprise-server-releases'
 import getRedirect from '../lib/get-redirect'
-import { defaultCacheControl, languageCacheControl } from '@/frame/middleware/cache-control'
+import {
+  defaultCacheControl,
+  languageCacheControl,
+  languageAndVersionCacheControl,
+} from '@/frame/middleware/cache-control'
 import { ExtendedRequest, URLSearchParamsTypes } from '@/types'
 
 export default function handleRedirects(req: ExtendedRequest, res: Response, next: NextFunction) {
@@ -27,13 +31,21 @@ export default function handleRedirects(req: ExtendedRequest, res: Response, nex
   // blanket redirects for languageless homepage
   if (req.path === '/') {
     const language = getLanguage(req)
-    languageCacheControl(res)
+    languageAndVersionCacheControl(res)
+
+    // Build redirect path, optionally including user's preferred version
+    let redirectPath = `/${language}`
+    const userVersion = req.userVersion
+    if (userVersion && userVersion !== 'free-pro-team@latest') {
+      redirectPath += `/${userVersion}`
+    }
+
     // Forward query params to the new URL
     let queryParams = new URLSearchParams((req?.query as any) || '').toString()
     if (queryParams) {
       queryParams = `?${queryParams}`
     }
-    return res.redirect(302, `/${language}${queryParams}`)
+    return res.redirect(302, redirectPath + queryParams)
   }
 
   // begin redirect handling

src/types/types.ts

@@ -24,6 +24,7 @@ export type ExtendedRequest = Request & {
   context?: Context
   language?: string
   userLanguage?: string
+  userVersion?: string
   FailBot?: Failbot
 }
 

src/versions/components/VersionPicker.tsx

@@ -1,10 +1,14 @@
 import { useRouter } from 'next/router'
+import { useState } from 'react'
+import { ActionMenu, ActionList } from '@primer/react'
 import { ArrowRightIcon, InfoIcon } from '@primer/octicons-react'
+import cx from 'classnames'
 
+import Cookies from '@/frame/components/lib/cookies'
+import { USER_VERSION_COOKIE_NAME } from '@/frame/lib/constants'
 import { useMainContext } from '@/frame/components/context/MainContext'
 import { DEFAULT_VERSION, useVersion } from '@/versions/components/useVersion'
 import { useTranslation } from '@/languages/components/useTranslation'
-import { Picker } from '@/tools/components/Picker'
 
 import styles from './VersionPicker.module.scss'
 
@@ -16,8 +20,9 @@ export const VersionPicker = ({ xs }: Props) => {
   const router = useRouter()
   const { currentVersion } = useVersion()
   const mainContext = useMainContext()
-  // Use TypeScript's "not null assertion" because  mainContext.page` should
-  // will present in mainContext if it's gotten to the stage of React
+  const [open, setOpen] = useState(false)
+  // Use TypeScript's "not null assertion" because mainContext.page should
+  // be present in mainContext if it's gotten to the stage of React
   // rendering.
   const page = mainContext.page!
   const { allVersions, enterpriseServerVersions } = mainContext
@@ -32,13 +37,26 @@ export const VersionPicker = ({ xs }: Props) => {
     return prefix + router.asPath.replace(`/${currentVersion}`, '')
   }
 
-  const allLinks = (page.applicableVersions || []).map((pageVersion) => ({
+  type VersionPickerLink = {
+    text: string
+    selected: boolean
+    href: string
+    extra: {
+      arrow: boolean
+      info: boolean
+      version?: string
+    }
+    divider: boolean
+  }
+
+  const allLinks: VersionPickerLink[] = (page.applicableVersions || []).map((pageVersion) => ({
     text: allVersions[pageVersion].versionTitle,
     selected: currentVersion === pageVersion,
     href: versionToHref(pageVersion),
     extra: {
       arrow: false,
       info: false,
+      version: pageVersion,
     },
     divider: false,
   }))
@@ -54,6 +72,7 @@ export const VersionPicker = ({ xs }: Props) => {
     extra: {
       arrow: false,
       info: false,
+      version: undefined,
     },
     divider: true,
   })
@@ -66,6 +85,7 @@ export const VersionPicker = ({ xs }: Props) => {
       extra: {
         arrow: true,
         info: false,
+        version: undefined,
       },
       divider: false,
     })
@@ -81,32 +101,73 @@ export const VersionPicker = ({ xs }: Props) => {
       extra: {
         arrow: false,
         info: true,
+        version: undefined,
       },
       divider: false,
     })
   }
 
+  const selectedOption = allLinks.find((item) => item.selected)
+
+  const handleVersionSelect = (item: VersionPickerLink) => {
+    // Save the user's version preference when they actively select one
+    if (item.extra?.version) {
+      try {
+        Cookies.set(USER_VERSION_COOKIE_NAME, item.extra.version)
+      } catch (err) {
+        console.warn('Unable to set preferred version cookie', err)
+      }
+    }
+    setOpen(false)
+    // Navigate after setting cookie
+    if (item.href) {
+      router.push(item.href)
+    }
+  }
+
   return (
     <div data-testid="version-picker" className={xs ? 'd-flex' : ''}>
-      <Picker
-        defaultText={t('version_picker_default_text')}
-        items={allLinks}
-        alignment="end"
-        pickerLabel={xs ? `Version\n` : `Version: `}
-        dataTestId="field"
-        descriptionFontSize={xs ? 6 : 5}
-        renderItem={(item) => {
-          return (
-            <div data-testid="version-picker-item" className={styles.itemsWidth}>
-              {item.text}
-              {item.extra?.arrow && (
-                <ArrowRightIcon verticalAlign="middle" size={15} className="ml-1" />
-              )}
-              {item.extra?.info && <InfoIcon verticalAlign="middle" size={15} className="ml-1" />}
-            </div>
-          )
-        }}
-      />
+      <ActionMenu open={open} onOpenChange={setOpen}>
+        <ActionMenu.Button
+          variant="invisible"
+          className={`color-fg-default width-full p-1 pl-2 pr-2`}
+        >
+          <span className={styles.pickerLabel}>{xs ? `Version\n` : `Version: `}</span>
+          <span className={`f${xs ? 6 : 5} color-fg-muted text-normal`} data-testid="field">
+            {selectedOption?.text || t('version_picker_default_text')}
+          </span>
+        </ActionMenu.Button>
+        <ActionMenu.Overlay width="auto" align="end">
+          <ActionList selectionVariant="single" role="menu">
+            {allLinks.map((item, i) =>
+              item.divider ? (
+                <ActionList.Divider key={`divider${i}`} />
+              ) : (
+                <ActionList.Item
+                  key={item.text}
+                  active={item.selected}
+                  onSelect={(e) => {
+                    e.preventDefault()
+                    handleVersionSelect(item)
+                  }}
+                  className={cx((item.extra?.arrow || item.extra?.info) && styles.extrasDisplay)}
+                  role={item.extra?.arrow || item.extra?.info ? 'menuitem' : 'menuitemradio'}
+                >
+                  <div data-testid="version-picker-item" className={styles.itemsWidth}>
+                    {item.text}
+                    {item.extra?.arrow && (
+                      <ArrowRightIcon verticalAlign="middle" size={15} className="ml-1" />
+                    )}
+                    {item.extra?.info && (
+                      <InfoIcon verticalAlign="middle" size={15} className="ml-1" />
+                    )}
+                  </div>
+                </ActionList.Item>
+              ),
+            )}
+          </ActionList>
+        </ActionMenu.Overlay>
+      </ActionMenu>
     </div>
   )
 }

src/versions/middleware/detect-version.ts

@@ -0,0 +1,26 @@
+import type { Response, NextFunction } from 'express'
+
+import { USER_VERSION_COOKIE_NAME } from '@/frame/lib/constants'
+import { allVersionKeys } from '@/versions/lib/all-versions'
+import { updateLoggerContext } from '@/observability/logger/lib/logger-context'
+import type { ExtendedRequest } from '@/types'
+
+function isValidVersion(version: string): boolean {
+  return allVersionKeys.includes(version)
+}
+
+export function getUserVersionFromCookie(req: ExtendedRequest): string | undefined {
+  const value = req.cookies?.[USER_VERSION_COOKIE_NAME]
+  if (value && isValidVersion(value)) {
+    return value
+  }
+  return undefined
+}
+
+export default function detectVersion(req: ExtendedRequest, res: Response, next: NextFunction) {
+  req.userVersion = getUserVersionFromCookie(req)
+  updateLoggerContext({
+    userVersion: req.userVersion,
+  })
+  return next()
+}

src/versions/tests/version-cookie.ts

@@ -0,0 +1,58 @@
+import { describe, expect, test } from 'vitest'
+
+import { get } from '@/tests/helpers/e2etest'
+import { USER_VERSION_COOKIE_NAME } from '@/frame/lib/constants'
+
+describe('version cookie redirects', () => {
+  test('homepage redirects to preferred version from cookie', async () => {
+    const res = await get('/', {
+      headers: {
+        Cookie: `${USER_VERSION_COOKIE_NAME}=enterprise-cloud@latest`,
+      },
+      followRedirects: false,
+    })
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/en/enterprise-cloud@latest')
+    expect(res.headers.vary).toContain('x-user-version')
+  })
+
+  test('homepage redirects to /en when no version cookie', async () => {
+    const res = await get('/', { followRedirects: false })
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/en')
+    expect(res.headers.vary).toContain('x-user-version')
+  })
+
+  test('homepage redirects to /en when fpt version in cookie', async () => {
+    const res = await get('/', {
+      headers: {
+        Cookie: `${USER_VERSION_COOKIE_NAME}=free-pro-team@latest`,
+      },
+      followRedirects: false,
+    })
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/en')
+  })
+
+  test('ignores invalid version in cookie', async () => {
+    const res = await get('/', {
+      headers: {
+        Cookie: `${USER_VERSION_COOKIE_NAME}=invalid-version`,
+      },
+      followRedirects: false,
+    })
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/en')
+  })
+
+  test('homepage redirects to enterprise-server version from cookie', async () => {
+    const res = await get('/', {
+      headers: {
+        Cookie: `${USER_VERSION_COOKIE_NAME}=enterprise-server@3.15`,
+      },
+      followRedirects: false,
+    })
+    expect(res.statusCode).toBe(302)
+    expect(res.headers.location).toBe('/en/enterprise-server@3.15')
+  })
+})