Skip to content

Commit aac022f

Browse files
authored
fix landing page carousel links (#58128)
1 parent bba2c9c commit aac022f

File tree

3 files changed

+68
-11
lines changed

3 files changed

+68
-11
lines changed

src/frame/middleware/resolve-recommended.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import type { ExtendedRequest, ResolvedArticle } from '@/types'
22
import type { Response, NextFunction } from 'express'
33
import findPage from '@/frame/lib/find-page'
44
import { renderContent } from '@/content-render/index'
5+
import Permalink from '@/frame/lib/permalink'
56

67
import { createLogger } from '@/observability/logger/index'
78

@@ -83,12 +84,11 @@ function tryResolveArticlePath(
8384
}
8485

8586
/**
86-
* Get the href for a page from its permalinks
87+
* Get the path for a page (without language/version)
8788
*/
88-
function getPageHref(page: any, currentLanguage: string = 'en'): string {
89-
if (page.permalinks?.length > 0) {
90-
const permalink = page.permalinks.find((p: any) => p.languageCode === currentLanguage)
91-
return permalink ? permalink.href : page.permalinks[0].href
89+
function getPageHref(page: any): string {
90+
if (page.relativePath) {
91+
return Permalink.relativePathToSuffix(page.relativePath)
9292
}
9393
return '' // fallback
9494
}
@@ -126,15 +126,14 @@ async function resolveRecommended(
126126
return next()
127127
}
128128

129-
const currentLanguage = req.context?.currentLanguage || 'en'
130129
const resolved: ResolvedArticle[] = []
131130

132131
for (const rawPath of articlePaths) {
133132
try {
134133
const foundPage = tryResolveArticlePath(rawPath, page?.relativePath, req)
135134

136135
if (foundPage) {
137-
const href = getPageHref(foundPage, currentLanguage)
136+
const href = getPageHref(foundPage)
138137
const category = foundPage.relativePath
139138
? foundPage.relativePath.split('/').slice(0, -1).filter(Boolean)
140139
: []

src/frame/tests/resolve-recommended.test.ts

Lines changed: 57 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ describe('resolveRecommended middleware', () => {
120120
{
121121
title: 'Test Article',
122122
intro: '<p>Test intro</p>',
123-
href: '/en/copilot/tutorials/article',
123+
href: '/copilot/tutorials/article',
124124
category: ['copilot', 'tutorials'],
125125
},
126126
])
@@ -195,7 +195,7 @@ describe('resolveRecommended middleware', () => {
195195
{
196196
title: 'Valid Article',
197197
intro: '<p>Valid intro</p>',
198-
href: '/en/test/valid',
198+
href: '/test/valid',
199199
category: ['test'],
200200
},
201201
])
@@ -256,10 +256,64 @@ describe('resolveRecommended middleware', () => {
256256
{
257257
title: 'Relative Article',
258258
intro: '<p>Relative intro</p>',
259-
href: '/en/copilot/relative-article',
259+
href: '/copilot/relative-article', // Updated to clean path
260260
category: ['copilot'],
261261
},
262262
])
263263
expect(mockNext).toHaveBeenCalled()
264264
})
265+
266+
test('returns paths without language or version prefixes', async () => {
267+
const testPage: Partial<import('@/types').Page> = {
268+
mtime: Date.now(),
269+
title: 'Tutorial Page',
270+
rawTitle: 'Tutorial Page',
271+
intro: 'Tutorial intro',
272+
rawIntro: 'Tutorial intro',
273+
relativePath: 'copilot/tutorials/tutorial-page/index.md',
274+
fullPath: '/full/path/copilot/tutorials/tutorial-page/index.md',
275+
languageCode: 'en',
276+
documentType: 'article',
277+
markdown: 'Tutorial content',
278+
versions: {},
279+
applicableVersions: ['free-pro-team@latest'],
280+
permalinks: [
281+
{
282+
languageCode: 'en',
283+
pageVersion: 'free-pro-team@latest',
284+
title: 'Tutorial Page',
285+
href: '/en/copilot/tutorials/tutorial-page',
286+
hrefWithoutLanguage: '/copilot/tutorials/tutorial-page',
287+
},
288+
],
289+
renderProp: vi.fn().mockResolvedValue('rendered'),
290+
renderTitle: vi.fn().mockResolvedValue('Tutorial Page'),
291+
render: vi.fn().mockResolvedValue('rendered content'),
292+
buildRedirects: vi.fn().mockReturnValue({}),
293+
}
294+
295+
mockFindPage.mockReturnValue(testPage as any)
296+
297+
const req = createMockRequest({ rawRecommended: ['/copilot/tutorials/tutorial-page'] })
298+
299+
await resolveRecommended(req, mockRes, mockNext)
300+
301+
expect(mockFindPage).toHaveBeenCalledWith(
302+
'/en/copilot/tutorials/tutorial-page',
303+
req.context!.pages,
304+
req.context!.redirects,
305+
)
306+
307+
// Verify that the href is a clean path without language/version, that gets
308+
// added on the React side.
309+
expect((req.context!.page as any).recommended).toEqual([
310+
{
311+
title: 'Tutorial Page',
312+
intro: '<p>Tutorial intro</p>',
313+
href: '/copilot/tutorials/tutorial-page',
314+
category: ['copilot', 'tutorials', 'tutorial-page'],
315+
},
316+
])
317+
expect(mockNext).toHaveBeenCalled()
318+
})
265319
})

src/landings/components/shared/LandingCarousel.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import { useState, useEffect, useRef } from 'react'
2+
import { useRouter } from 'next/router'
23
import { ChevronLeftIcon, ChevronRightIcon } from '@primer/octicons-react'
34
import cx from 'classnames'
45
import type { ResolvedArticle } from '@/types'
56
import { useTranslation } from '@/languages/components/useTranslation'
7+
import { useVersion } from '@/versions/components/useVersion'
68
import styles from './LandingCarousel.module.scss'
79

810
type LandingCarouselProps = {
@@ -42,6 +44,8 @@ export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselPr
4244
const [isAnimating, setIsAnimating] = useState(false)
4345
const itemsPerView = useResponsiveItemsPerView()
4446
const { t } = useTranslation('product_landing')
47+
const router = useRouter()
48+
const { currentVersion } = useVersion()
4549
const headingText = heading || t('carousel.recommended')
4650
// Ref to store timeout IDs for cleanup
4751
const animationTimeoutRef = useRef<NodeJS.Timeout | null>(null)
@@ -145,7 +149,7 @@ export const LandingCarousel = ({ heading = '', recommended }: LandingCarouselPr
145149
{visibleItems.map((article: ResolvedArticle, index) => (
146150
<a
147151
key={startIndex + index}
148-
href={article.href}
152+
href={`/${router.locale}/${currentVersion}${article.href}`}
149153
className={cx(styles.articleCard, 'border', 'border-default', 'rounded-2')}
150154
>
151155
<h3 className={styles.articleTitle}>

0 commit comments

Comments
 (0)